Skip to content

Conversation

@johnf
Copy link
Contributor

@johnf johnf commented Dec 8, 2017

Support specifying aws profiles in the config

@patrobinson
Copy link
Contributor

I believe the AWS SDK will respect the use of the AWS_PROFILE environment variable, this seems like a much cleaner way to utilise profiles without writing any code for StackMaster to handle credentials.

@johnf
Copy link
Contributor Author

johnf commented Dec 8, 2017

@patrobinson yes by default the SDK will use the environment variable. This approach is handy though one use case the China region where you will need different credentials for that stack. Means you can throw them in the stack_master config rather than having to remember to change AWS_PROFILE in your shell.

@et304383
Copy link

et304383 commented Feb 1, 2018

Specifying a profile in a file that is supposed to be portable across multiple users is never a good idea. I don't name my profile the same as you.

@johnf
Copy link
Contributor Author

johnf commented Feb 1, 2018

I'm coming from a corporate environment where when staff start part of the setup process is setting their profile for the company to be a standard string.

Copy link
Contributor

@patrobinson patrobinson left a comment

Choose a reason for hiding this comment

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

I've thought about similar proposals a lot. Generally I'm not in favour of more tightly coupling the authentication mechanism to StackMaster. At present we do not know nor care about the value of AWS_PROFILE, that's all handled transparently by the SDK. Pushing credential handling to the SDK as much as possible is desirable.

While this is a fairly simple implementation, it's geared towards only those that have standardised credential settings. Internally we take a different approach, each user can configure a bash alias, that calls aws-vault with the appropriate arguments, to get the right credentials.

credential-alias stack_master apply us-east-1 my-stack

Indeed this could work the same way here

AWS_PROFILE=credentials stack_master apply us-east-1 my-stack

So I'm on the fence on whether to accept this PR. At a glance it doesn't make things too difficult going forwards, but I'd prefer to avoid needing to pass profile to every place we call the SDK. I might be convinced if some tests are added to ensure the profile setting takes affect.

region: region,
retry_limit: 10,
}
params[:credentials] = Aws::SharedCredentials.new(profile_name: profile_name) if profile_name
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like it would be easier to just over-write the environment variable rather than pass it down the stack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From memory I did it this way so that if you simply do stack_master apply different stacks can have different profiles. i.e. the Chine situation

@et304383
Copy link

et304383 commented Feb 2, 2018

I worry that if you add this the next thing people will want is the ability to pass credentials via access key and secret parameters.

Then, they'll want you to have profile prompt for an MFA code if one is required.

I was on the side of wanting all these args a while back when using tools like this, but now I've learned to keep things modular and portable by just using tools like aws-runas. As long as the tool supports env vars for key, secret key, and session token (like stack_master does) then you can use aws-runas.

@johnf
Copy link
Contributor Author

johnf commented Feb 2, 2018

Happy to add tests if that helps get it landed.

@patrobinson
Copy link
Contributor

patrobinson commented Feb 5, 2018

@et304383 This is my concern too. But I'm hoping we can tackle this problem in a 2.0 release of StackMaster where we better handle "environments" to make it easier while keeping the authentication mechanisms de-coupled. I'm not sure how we'll do that yet, open to ideas!

In the mean time I think it should be OK to keep some features, like this, which do not impact other users in the 1.x release. It's likely we'll cull them though if/when there's a better way of managing it.

But as you say, I'm not happy to tightly couple StackMaster to the authentication tool someone chooses.

@patrobinson
Copy link
Contributor

This PR is a bit stale. Happy to re-review when tests are added, in the mean time closing it.

@patrobinson patrobinson closed this Feb 2, 2019
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