Support AssumeRole when credential_source = Environment
#231
Conversation
| @@ -52,13 +53,21 @@ func (self ECRHelper) Get(serverURL string) (string, string, error) { | |||
| return "", "", credentials.NewErrCredentialsNotFound() | |||
| } | |||
|
|
|||
| profile, profile_exists := os.LookupEnv("AWS_PROFILE") | |||
There was a problem hiding this comment.
nit: profile_exists -> profileExists
https://golang.org/doc/effective_go.html#mixed-caps
Finally, the convention in Go is to use MixedCaps or mixedCaps rather than underscores to write multiword names.
| if registry.FIPS { | ||
| client, err = self.ClientFactory.NewClientWithFipsEndpoint(registry.Region) | ||
| if err != nil { | ||
| logrus.WithError(err).Error("Error resolving FIPS endpoint") | ||
| return "", "", credentials.NewErrCredentialsNotFound() | ||
| } | ||
| } else if profile_exists { | ||
| client, err = self.ClientFactory.NewClientWithExplicitProfile(profile) | ||
| if err != nil { | ||
| logrus.WithError(err).Error("Error creating client with explicit profile") | ||
| return "", "", credentials.NewErrCredentialsNotFound() | ||
| } |
There was a problem hiding this comment.
This is probably a question for @samuelkarp - Right now FIPS and AWS_PROFILE are mutually-exclusive. Is there a case where a customer want to use AWS_PROFILE, but with FIPS?
There was a problem hiding this comment.
Yes, these options should not be mutually exclusive.
|
@benhowes Are you able to update this to address the mutual-exclusivity problem? |
|
@samuelkarp I've actually left the company where I was developing this since then, I don't think I'll find time to work on this PR any further. Sorry! |
|
@benhowes Thanks for letting me know! I'm going to close this PR for now, but we'd be open to looking at changes like this again in the future. |
From #181
Description of changes:
This PR is a slight variation on the code outlined in #181, which just always passes the value of
AWS_PROFILEto the creation of a session. To the best of my knowledge, this will work in the regular cases which are already supported, as well as when assuming a role withcredential_source = Environment.I've not currently added any tests, because I cannot see any examples of tests which use the
~/.aws/configor any tests which setAWS_PROFILE.Opening this for discussion and to hopefully arrive at a solution which is mergeable.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.