Skip to content
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

Implement AWS EXTERNAL ID feature #142

Merged
merged 3 commits into from
Feb 27, 2021

Conversation

alemuro
Copy link
Contributor

@alemuro alemuro commented Feb 22, 2021

Hello,

The variable --app-role-arn provides the feature of assuming an AWS role during runtime. This allows the service to access services from a different account where it is running. Moreover, AWS recommends using this funcionality along with what they call external id. This is a random string or number the client must provide to assume an specific role. It acts like a password.

https://docs.aws.amazon.com/IAM/latest/UserGuide/id_roles_create_for-user_externalid.html

This PR implements this functionality. I've added a new environment variable called AWS_EXTERNAL_ID, a new CLI option --aws-external-id and a new YAML parameter: aws.external-id.

If the value is not provided, it will use the default value in a Go string (an empty string ""), so it should work always.

Regards,

@coveralls
Copy link

coveralls commented Feb 22, 2021

Coverage Status

Coverage decreased (-0.02%) to 13.601% when pulling 6678e2c on alemuro:add-aws-external-id into d4da387 on camptocamp:master.

state/aws.go Outdated
@@ -35,7 +35,9 @@ func NewAWS(c *config.Config) AWS {

if len(c.AWS.APPRoleArn) > 0 {
log.Debugf("Using %s role", c.AWS.APPRoleArn)
creds := stscreds.NewCredentials(sess, c.AWS.APPRoleArn)
creds := stscreds.NewCredentials(sess, c.AWS.APPRoleArn, func(p *stscreds.AssumeRoleProvider) {
p.ExternalID = aws_sdk.String(c.AWS.ExternalID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when the ExternalID is not specified? Does it behave the same as today?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be the same, as the default value is always an empty string. Moreover, let me do some tests to verify it, and I will come back to you! 😄

thanks

@alemuro
Copy link
Contributor Author

alemuro commented Feb 26, 2021

Hello again, after some tests I realised it is also necessary to put an if-statement. In case ExternalID is not provided, we will let the default value (which is nil), and it will be filled only if the value is provided. We need this if-statement because c.AWS.ExternalID is an empty string by default, but not nil, and this cause the test to fail.

Regards,

@raphink raphink merged commit 695ab95 into camptocamp:master Feb 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants