[ZEPPELIN-1465] Add an option to allow S3 server-side encryption#1969
Closed
jeff-cyft wants to merge 2 commits into
Closed
[ZEPPELIN-1465] Add an option to allow S3 server-side encryption#1969jeff-cyft wants to merge 2 commits into
jeff-cyft wants to merge 2 commits into
Conversation
Member
|
Thanks @jeff-cyft for the contribution! |
felixcheung
reviewed
Feb 4, 2017
| if (useServerSideEncryption) { | ||
| // Request server-side encryption. | ||
| ObjectMetadata objectMetadata = new ObjectMetadata(); | ||
| objectMetadata.setSSEAlgorithm(ObjectMetadata.AES_256_SERVER_SIDE_ENCRYPTION); |
Member
There was a problem hiding this comment.
very minor q: should SSEAlogrithm be configurable too?
Contributor
Author
There was a problem hiding this comment.
it's a good question.
- There is currently only the one documented valid option. http://docs.aws.amazon.com/AWSJavaSDK/latest/javadoc/com/amazonaws/services/s3/model/ObjectMetadata.html
- There's no enumeration in the API that can be searched by String so in the future this code would need amendment anyway to support potential new options. I thought about letting the zeppelin configurer specify any string they'd like but it seemed error-prone to expect someone configuring zeppelin to be aware of constant string literals in the S3 Java API.
Member
There was a problem hiding this comment.
great! thanks for the details!
Member
|
LGTM |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this PR for?
Provide a configuration option that will cause the S3 Notebook repo to request server-side encryption of saved notebooks.
What type of PR is it?
Improvement
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-1465
How should this be tested?
Enable the configuration option, save a notebook in zeppelin, and confirm in the AWS S3 Console that the related file was saved with AES-256 encryption on the server-side. (Properties tab, Detail section)
Questions:
Does the licenses files need update?
No
Is there breaking changes for older versions?
No.
Does this needs documentation?
I added mentions of the new option in existing documentation.
Thank you!