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

Feature/sse kms #107

Closed
wants to merge 13 commits into from
Closed

Feature/sse kms #107

wants to merge 13 commits into from

Conversation

robparrott
Copy link

For Issue #102

This builds on the PR for SSE-S3.

Adds two more options: --use-kms and --kms-key-id with expanded options for server side encryption of objects in S3.

  • Requires that --use-sse is specified.
  • When only the --use-kms is given, then uses the KMS server-side encryption mode (SSE-KMS) but with the default, customer master key (CMK).
  • When --kms-key-id is specified in addition to --use-kms it use use that specified KMS instead of the master key. Requires that the user that is connected to S3 via goofys has permission to access and use that KMS key.

All three cases tested in basic fashion with small and large file sizes.

kahing added a commit that referenced this pull request Sep 12, 2016
also got rid of the --use and prefix --kms with --sse, and
made --sse-kms imply --sse because it doesn't make sense
otherwise

refs #107
@kahing
Copy link
Owner

kahing commented Sep 12, 2016

I squashed this and #106 and massaged it a little. Could you take a look and let me know what you think?

@robparrott
Copy link
Author

This makes sense given that the --sse-kms string can be left as empty.

kahing added a commit that referenced this pull request Sep 13, 2016
also got rid of the --use and prefix --kms with --sse, and
made --sse-kms imply --sse because it doesn't make sense
otherwise

refs #107
@kahing
Copy link
Owner

kahing commented Sep 13, 2016

merged via 55c235c

@kahing kahing closed this Sep 13, 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