-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
HADOOP-17922. move to fs.s3a.encryption.algorithm - JCEKS integration #3466
HADOOP-17922. move to fs.s3a.encryption.algorithm - JCEKS integration #3466
Conversation
693f9eb
to
7793694
Compare
checkstyle is unused import; fixed |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall. Just added a few clarification comments. And nice tests.
BTW, just to be sure once again. The key resolution follows the following order, right?
- bucket level new key whether it is present in jceks or xml.
- bucket level old key whether it is present in jceks or xml.
- global new key whether it is present in jceks or xml.
- global old key whether it is present in jceks or xml.
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AEncryptionMethods.java
Show resolved
Hide resolved
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/S3AUtils.java
Show resolved
Hide resolved
💔 -1 overall
This message was automatically generated. |
New unit test suite for propagation * adds TestBucketConfiguration for conf file propagation (succeeds) * one for jecks (fails; needs to sync up with the other patch) * moves over some test cases from ITestS3AConfiguration which don't need an FS. Change-Id: Ie1b6e0d1c655d00fc6d47ce054a1aeba01c71044
Explicit ordering of resolution of current & deprecated encryption config keys when reading from configuration files, which ensures that semantics for jcecks-stored values match those of XML options. Test expanded for password resolution Docs updated Change-Id: I3bf9697ff07d8cb830dca5a1d2c224a79f89fe90
Change-Id: I4a1465b65c2b1c532e2ba5d2497c88b614247ccd
Change-Id: I840344205abdc70cadbe4635a0bec9a80bccd918
7ce5895
to
3e71af7
Compare
had to rebase |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 All good.
@steveloughran, Seems like one test method ITestS3AFileContextStatistics#verifyWrittenBytes() still uses the old |
…apache#3466) The ordering of the resolution of new and deprecated s3a encryption options & secrets is the same when JCEKS and other hadoop credentials stores are used to store them as when they are in XML files: per-bucket settings always take priority over global values, even when the bucket-level options use the old option names. Contributed by Mehakmeet Singh and Steve Loughran
…#3466) The ordering of the resolution of new and deprecated s3a encryption options & secrets is the same when JCEKS and other hadoop credentials stores are used to store them as when they are in XML files: per-bucket settings always take priority over global values, even when the bucket-level options use the old option names. Contributed by Mehakmeet Singh and Steve Loughran Change-Id: I871672071efa2eb6b600cb2658fceeef57f658a3
New unit test suite for propagation
don't need an FS.
Change-Id: Ie1b6e0d1c655d00fc6d47ce054a1aeba01c71044
Description of PR
For code changes:
new test and modified original yes: s3 london