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

Allowing users to enable s3 server side encryption when uploading CF templates #167

Merged
merged 4 commits into from
Jun 29, 2016
Merged

Conversation

oliviervg1
Copy link
Contributor

Some enterprises enforce the use of server side encryption when uploading objects to S3. This change is to allow Stacker to work in such environments.

@phobologic
Copy link
Member

Hey @oliviervg1 - thanks for this! One question I have: Is there any reason to not make this the default? I'm kind of embarrassed we didn't make this the default already :) The encryption is per object, right?

@oliviervg1
Copy link
Contributor Author

oliviervg1 commented Jun 27, 2016

Hi @phobologic, yep - the encryption is on a per object basis. I'm not against making it the default behaviour, however I'd just like to double check something first if we go ahead with it:

Consider we have account A and account B. Stacker uploads server side encrypted templates to a bucket in account A. We then make the bucket available to account B using a bucket policy. Can account B read the encrypted objects from the bucket in account A?

@phobologic
Copy link
Member

I actually don't know the answer to that question, though I'm curious when you think that situation would arise?

@oliviervg1
Copy link
Contributor Author

oliviervg1 commented Jun 28, 2016

I've confirmed that cross account bucket access still works with AWS' default server side encryption. I'd never recommend anyone to use bucket policies to do cross account S3 access (using IAM role assumption is far better). I just wanted to make sure that we cover potential edge cases and not break anything for people using stacker in weird and wonderful ways :)

I'll update the pull request to force server side encryption if that's ok with you!

@phobologic
Copy link
Member

Awesome. Looks good to me. Thanks @oliviervg1 !

@phobologic phobologic merged commit 38f723e into cloudtools:master Jun 29, 2016
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