-
-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Automatically log in & add aws-vault
support
#68
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooo this is awesome!!! auto-logging in is definitely something i've wanted on this workflow. i'm not familiar with aws-vault
, but it seems interesting. i took a preliminary pass, and conceptually it makes sense, but i need to dig in and understand aws-vault
a bit more before incorporating it into a release. some top-of-mind thoughts:
- i see
aws-vault
is written in go. does it provide programmatic access such that we could potentially include it as a dependency to a) remove the extra step to installaws-vault
; and b) remove the need to fork a process just to invoke it. this may simpify the implementation and would probably provide better performance for authing - what alternatives to
aws-vault
are there? are there ones that satisfy issues 1.a and 1.b from above? - regardless of underlying auth implementation, i would love to see what kind of test coverage we could get here, if any (even if mocking out calls to the actual auth API utilizing go-vcr like other tests do)
any insights you can give me to the above before i take a dive would be appreciated!
beyond that, here are some thoughts on the extras in your fork you mentioned:
Opening S3 Static Website URL if the S3 bucket has that function enabled.
this could definitely be useful! i am curious how your workflow does this though. for example, is this behavior exposed via a modifier key + enter? if so, my thought for setting modifiers like any given resource should have their behavior configurable because everyone's use cases are different. i wouldn't want to force this behavior into a modifier if another user has no use for opening these URLs and there's some other action they'd like to perform. an alternative here would be to have a subcommand or the entity such that people can select from a list of actions to perform, but i'm definitely open to both if we can satisfy the constraints
Directly going to the CloudWatch logs of any service
there is some functionality that exists for this currently... can you elaborate on what you mean here?
Being able to open an SSH session into any EC2 instance
this definitely seems useful, though this behavior should confirm to what i outlined about opening S3 Static Website URLs
i'll try to take a look at this sometime soon... life has just been busy recently 😅. whatever the case, i greatly appreciate your contribution here!!! 🙏
|
||
loginUrl := url.URL{ | ||
Scheme: "https", | ||
Host: "us-east-1.signin.aws.amazon.com", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here and below: will this work for other regions as well? it feels like we should either a) make this URL region-aware, or b) otherwise strip it if AWS handles that automatically (though if there are redirects that slows that down, we should instead just make the URL region aware)
func validateAWSUrl(url string) error { | ||
// TODO | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what should this function be doing?
if err := validateAWSUrl(finalUrl.String()); err != nil { | ||
panic(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
because most of these are hardcoded values that are specific to the function body, it feels like we should be validating the input (that is, destinationUrl
) earlier on. as-is, this is the surface area for the validation is larger than it needs to be
func executeAwsVaultCommand(profile string) (string, error) { | ||
cmd := exec.Command("aws-vault", "exec", profile, "--no-session", "--", "env") | ||
output, err := cmd.Output() | ||
if err != nil { | ||
return "", err | ||
} | ||
|
||
return string(output), nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add better error messaging for if aws-vault
was not resolvable. something like, "Credential provider was configured as aws-vault
, but aws-vault
was not installed or on your $PATH
" (though, read my top-level PR comment associated with this review bc this may not be relevant if, for example, aws-vault
provides programmatic access)
Hi @rkoval,
I've been a fan of your Alfred Workflow for quite some time now. It's an invaluable tool in my specific use case, where - as a freelancer - I find myself switching between many different AWS environments in a day.
I've made some enhancements that have been well-received among my colleagues. I've been considering a pull request for a while now, and given the positive feedback, I decided it's time to share these features with the community. Here are the details:
1. AWS Vault as credential provider
The environment variable
ALFRED_AWS_CONSOLE_SERVICES_WORKFLOW_AWS_AUTH_PROVIDER
can be set to aws-vault to enable this feature. The workflow will then utilize AWS Vault for credential management.The reason I have implemented this feature in my own workflow is because I do not like to store any IAM credentials in plain text, nor do I like exposing them to the terminal/applications running AWS operations.
2. Federated URLs with Custom Identity Broker Access
I think all developers can agree that getting presented with the following screen when trying to sign in is annoying to say the least.
Using the custom identity broker access API endpoints, it is possible to construct a - what I like to call -
LogoutLoginURL
. This makes it possible to never get presented with that pesky annoying page ever again.Notes
I understand the importance of code quality and testing. However, this PR is a starting point to gauge your interest. If these features align with your vision for the workflow, I'd be more than happy to refine the code and add necessary tests.
Looking forward to your thoughts!
P.S. This is PR is a very stripped down version of my custom setup of this workflow, as I do not think they would fit the vision of "keep it simple". But if you want to work together on creating a general implementation of those, let me know! To give you an idea: