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

Update to boto3. Add AWS Profile support #174

Merged
merged 7 commits into from
Jul 24, 2016

Conversation

acmcelwee
Copy link
Member

Updating to boto3 provides support for the standard AWS multi-profile capabilities.

When I first started trying to use stacker, I spun my wheels for quite a while to figure out how to make it work with our multi-profile support. I eventually stumbled on the closed issue #106, which pointed me in the right direction, but it was certainly big hurdle that I could see stopping a lot of people from trying to pick up a new tool. All of our AWS credentials are temporary security credentials so I ended up having to refresh my credentials (with our usual tool) and then do a nasty dance along the lines of export AWS_SECURITY_TOKEN="$(cat ~/.aws/credentials | grep aws_session_token | head -n2 | tail -n1 | awk '{print $3}')". Supporting the modern credentials management with multi-profile support would probably ease adoption going forward.

@phobologic
Copy link
Member

Thanks @acmcelwee for this - I actually had a boto3 branch I've been working on, but glad to have you knock this one out :) One question I have, since I haven't played w/ boto3's profile support:

Will this break things for those folks who use environment variables? What happens when you don't have a profile setup? I know plenty of people who use the environment variables (including myself - mostly because I like keeping all my AWS credentials encrypted at rest: http://signal0.com/2013/05/14/_more__securely_storing_my_aws_credentials.html), and I wouldn't want this to be non-backwards compatible for them.

I'll dive into the code a bit more today/this weekend. Thanks again!

@phobologic
Copy link
Member

Actually - one thing that might make this easier, if my concerns are correct, is to do away with the --profile flag, and instead rely on the AWS_PROFILE environment variable instead. That should get you the functionality you want, while not requiring a special flag (and I believe that that would be a more consistent way to use profiles across tools in general). What do you think?

@acmcelwee
Copy link
Member Author

@phobologic boto3 has a nice tiered approach to resolving credentials, where they actually read from environment variables before they resolve credentials from ~/.ssh/credentials. I might need to do a second pass to treat the profile more of an optional, so I'll try to look at that over the next few days. My suspicion is that that because I've defaulted it to default, it might break things for your scenario.

Regarding the AWS_PROFILE env var, I could be on board with that, but if you want to mirror the way the aws cli works, it always uses your env vars first, followed by the default profile if it's unspecified, but it always lets you specify a profile context override. For me, I'm often interacting with multiple aws profiles (dev=default, stage, and prod), and it's really nice to make a context switch via a flag, rather than a re-export of my context via an env var. That's just my two cents.

@phobologic
Copy link
Member

Yeah - I also waffle on this because profile is such a generic term, I could see it colliding with other tools. I'm not sure that using --profile is the suggestion from AWS, just the way they do it in the awscli, which makes sense since that's the only profile they'll ever use.

I will say that you don't necessarily have to re-export your variable every time - you'd just have to do:

AWS_PROFILE=dev ./stacker ...
AWS_PROFILE=prod ./stacker ...

We actually do this quite a bit, though using other tools. For our MFA/assumable roles, we use https://github.com/remind101/assume-role - so, for example, when I'm rolling out in prod I do:

assume-role prod ./stacker ...

The great thing about that is that assume-role uses the environment variables, which means it's basically usable with any tool that uses any form of boto/boto3/standard AWS functionality. The other reason I like it is because I often find myself doing work in a specific environment for a while, in which case I can:

eval $(assume-role prod) and it loads it into my environment (for an hour, till the creds expire). I tend to load this up, with the shell magic I pointed at in that signal0 post, with a variable that shows what AWS account I'm working with on my prompt.

@acmcelwee
Copy link
Member Author

I'd never actually used the AWS_PROFILE var, but I dig it. The latest commit drops all of the explicit profile support to let the profile support functionality remain purely at the boto3 level. This should be the best of both of all scenarios:

  • Modern aws library
  • Built-in multi-profile support with no additional work by the stacker codebase
  • Backwards compatibility with the practice of pure env var aws credential management

@phobologic
Copy link
Member

Awesome - yeah, this kicks ass. I'm swamped right now, but I'll try to get to reviewing this this weekend. Thanks again for all your help!

key = self.cfn_bucket.new_key(key_name)
key.set_contents_from_string(blueprint.rendered, encrypt_key=True)
self.s3_conn.put_object(Bucket=self.bucket_name, Key=key_name,
Body=blueprint.rendered, ServerSideEncryption='AES256')
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will pass pep8 - in general, can you also make lines < 80. We've kept the code base at that for the most part, despite the limit being raised to 110.

@phobologic
Copy link
Member

Other than the pep8 changes, this looks good.

@acmcelwee
Copy link
Member Author

@phobologic - pep8 reformatting pushed

@phobologic
Copy link
Member

Awesome, thanks @acmcelwee!

@phobologic phobologic merged commit 083bb37 into cloudtools:master Jul 24, 2016
@acmcelwee acmcelwee deleted the boto3 branch June 27, 2019 22:05
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.

2 participants