[ZEPPELIN-848] Add support for encrypted data stored in Amazon S3 #886
Conversation
Adds support for using the AWS KMS or a custom encryption materials provider class to encrypt data stored in Amazon S3. Also a minor improvement to logic inside the S3 notebook repo.
|
I'm not sure what happened with the CI build, I can't get any details about the failure. Is there anything I can do from my end? |
| emp = (EncryptionMaterialsProvider) empInstance; | ||
| } | ||
| else { | ||
| throw new IllegalArgumentException("Class " + empClassname + " does not implement " |
There was a problem hiding this comment.
Why not to throw IOException since constructor is already throwing it and upper layer may rely on that
There was a problem hiding this comment.
I'll make this change, thanks
Catch AmazonClientException and re-throw as IOException since ACE is a runtime exception.
NotebookServer was not logging the exception thrown when reloading notebooks
|
I made changes to re-throw the AmazonClientExceptions as IOException, then made a change in NotebookServer so that the exception is logged. It looks like elsewhere exceptions are getting logged, so now S3 issues should get logged nicely. |
|
Any updates on this? |
|
@natesammons-nasdaq Thanks for useful improvement. CI fails with error Could you take care of it? |
Wrap at 100 chars
|
Sorry about that! |
|
thanks for handling exceptions, LGTM. may need to re-trigger CI, although the build failure isn't related at all. |
|
While all build profile failed for last commit with some network issue, It's good idea to re-trigger CI and see how it goes. @natesammons-nasdaq could you retrigger CI? you can do it by either close->reopen this pullrequest or add more commits. |
|
Sure, I'll try that. I haven't been able to see any build status yet, they all appear to fail with a timeout under Travis. ? [edit] nevermind, I see build status going now. Might be some corporate network shenanigans, as I can see status just fine from home. Sent from my iPhone On May 17, 2016, at 8:25 PM, Lee moon soo <notifications@github.commailto:notifications@github.com> wrote: While all build profile failedhttps://travis-ci.org/apache/incubator-zeppelin/builds/130849266 for last commit with some network issue, It's good idea to re-trigger CI and see how it goes. @natesammons-nasdaqhttps://github.com/natesammons-nasdaq could you retrigger CI? you can do it by either close->reopen this pullrequest or add more commits. You are receiving this because you were mentioned. CONFIDENTIALITY NOTICE: This e-mail and any attachments are for the exclusive and confidential use of the intended recipient and may constitute non-public information. If you received this e-mail in error, disclosing, copying, distributing or taking any action in reliance of this e-mail is strictly prohibited and may be unlawful. Instead, please notify us immediately by return e-mail and promptly delete this message and its attachments from your computer system. We do not waive any work product or other applicable legal privilege(s) by the transmission of this message. |
|
Closing request to re-open it |
|
Re-opening to trigger CI |
|
Build is still failing with the following: [ERROR] Failed to execute goal org.apache.maven.plugins:maven-remote-resources-plugin:1.4:process (default) on project zeppelin: Error downloading resources archive. Could not transfer artifact org.apache.apache.resources:apache-jar-resource-bundle:jar:1.5-SNAPSHOT from/to apache-snapshots (https://repository.apache.org/snapshots/): Failed to transfer file: https://repository.apache.org/snapshots/org/apache/apache/resources/apache-jar-resource-bundle/1.5-SNAPSHOT/apache-jar-resource-bundle-1.5-SNAPSHOT.jar. Return code is: 503 , ReasonPhrase:Service Unavailable. I had issues getting this artifact when building locally as well, I think I ended up just manually adding it to the local maven repo |
|
I think there were some problem on "repository.apache.org" and i believe now it is stable. @natesammons-nasdaq Could you try once again re-trigger the CI? |
|
Merged changes to trigger another build |
|
I looked at the latest failure, not sure what the cause is. Looks like: Tests run: 6, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 81.399 sec <<< FAILURE! - in org.apache.zeppelin.rest.ZeppelinSparkClusterTest |
|
Looks like the last build passed! |
|
Is there anything else I need to do from my end? Thanks |
|
Tested and working well for me. One more thing, could you add new env variables and properties to https://github.com/apache/incubator-zeppelin/blob/master/docs/install/install.md#zeppelin-configuration ? |
Add S3 encryption documentation to the config section in install.md
|
The build is still failing, looks like a selenium test. Any ideas? |
|
Hmmm, still failing with a FF selenium test |
|
Travis build is still failing. Not sure what to do as I am pretty confident that it is unrelated. |
|
Thanks @natesammons-nasdaq for the improvement! CI failure is unrelated to this change and tracked by https://issues.apache.org/jira/browse/ZEPPELIN-878 LGTM and merge if there're no more discussions |
|
Thanks! Very happy to contribute to this project! |
|
Great stuff! CI has been recently fixed on latest master, @natesammons-nasdaq could you please rebase and see if that helps? |
|
Looks like the tests are passing now! Nice |
What is this PR for?
Adds support for using the AWS KMS or a custom encryption materials
provider class to encrypt data stored in Amazon S3. Also a minor
improvement to logic inside the S3 notebook repo when dealing with local files.
What type of PR is it?
Improvement
What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-848
How should this be tested?
Running in EMR or another system in AWS is easiest. Make appropriate changes to the config and use an AWS KMS key
Questions: