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

demo role_arn fixes #52

Closed
wants to merge 4 commits into from
Closed

Conversation

dustinrue
Copy link
Contributor

@dustinrue dustinrue commented Jun 2, 2023

I'm not familiar enough with the design pattern used here to properly modify the code, so for demo purposes and testing I have created this PR for #47 which contains a hack to get the role_arn value from the wpsnaphots config file to where it needs to be to assume roles.

These changes, in my testing, allow for the original design to work using profiles while also supporting ENV vars and role_arns when present in the config file. What does not work is having credentials hardcoded in the wpsnapshots config file. Adding the ability to load this information from wpsnapshots config would provide backwards compatibility, however.

I tested the following scenarios:

  • wpsnapshots configuration contains a valid "roleArn" value and no profile value
  • wpsnapshots configuration contains valid "profile" value with hard coded AWS credentials in ~/.aws/credentials
  • wpsnapshots configuration with no roleArn or profile and will use the default AWS profile if configured in ~/.aws
  • wpsnapshots configuration with no roleArn or profile but with AWS environment variables set
  • wpsnapshots has invalid roleArn configured
  • also tested against an AWS account with MFA enabled

I did not test this on an ec2 instance with an IAM role assigned but presumably it would work.

@johnwatkins0 johnwatkins0 self-requested a review June 9, 2023 13:23
@dustinrue dustinrue closed this Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant