Details
-
Bug
-
Status: Resolved
-
Minor
-
Resolution: Fixed
-
2.0.0
-
None
-
Reviewed
Description
There is duplicated code from MobUtils.createEncryptionContext in HStore, and there is a subset of that code in HFileReaderImpl.
Refactored key selection
Moved both to EncryptionUtil.java
Can't figure out how to write a unit test for this, but there's no new code just refactoring.
Attachments
Attachments
- HBASE-14901.1.patch
- 19 kB
- Nate Edel
- HBASE-14901.2.patch
- 19 kB
- Nate Edel
- HBASE-14901.3.patch
- 23 kB
- Nate Edel
- HBASE-14901.5.patch
- 23 kB
- Nate Edel
- HBASE-14901.6.patch
- 23 kB
- Nate Edel
Issue Links
- is cloned by
-
HBASE-14964 Backport HBASE-14901 (duplicated code to create/manage encryption keys) to branch-1
- Resolved
Activity
How about renaming EncryptionUtils.getgetKeyFromBytesOrMasterKey(...) to just EncryptionUtils.unwrapKey(Configuration, byte[]), since it's just delegating to unwrap key with known config values. Since it's a public method it should have some javadoc as well. The rest of the refactor looks good to me.
Seems okay to me to relocate these to the hbase-client module since the other unwrapKey() methods are already there.
apurtell any thoughts?
I concur ghelmling, rename that method as you suggest, put in hbase-client, and lgtm
Changes made as requested to name and (yes, that's much better naming) and JavaDoc added. Submitting to get QABot's opinion.
apurtell - EncryptionUtil.java is already in hbase-client; is there anything else that should be moved?
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12775191/HBASE-14901.2.patch
against master branch at commit cbbad6e5fbcf95f585c850a688d9cce51da06e24.
ATTACHMENT ID: 12775191
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 checkstyle. The applied patch generated new checkstyle errors. Check build console for list of new errors.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn post-site goal succeeds with this patch.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16726//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16726//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16726//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16726//console
This message is automatically generated.
Fixed checkstyle errors (bad imports.)
Fixed some existing checkstyle errors (bad import order, no private constructor on EncryptionUtil and MobUtils).
Fixed "protected" LOG on EncryptionUtil to private
+1 on patch v3. Will wait for HadoopQA to come back on that patch before committing.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12775384/HBASE-14901.3.patch
against master branch at commit da0cc598feab995eed12527d90805dd627674035.
ATTACHMENT ID: 12775384
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
-1 checkstyle. The applied patch generated new checkstyle errors. Check build console for list of new errors.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn post-site goal succeeds with this patch.
+1 core tests. The patch passed unit tests in .
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16733//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16733//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16733//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16733//console
This message is automatically generated.
Cascading checkstyles errors... added final to the two utility classes. QABot, what say you?
Sorry about that nkedel Looks like your test run got thrown out of the queue because of my messing w/ hadoopqa scripts. Thanks for requeuing.
Didn't re-queue after cancel/subit; re-uploading the same patch (no change from .4) and resubmitting.
-1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12775827/HBASE-14901.5.patch
against master branch at commit 03e4712f0ca08d57586b3fc4d93cf02c999515d8.
ATTACHMENT ID: 12775827
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
-1 patch. The patch command could not apply the patch.
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16767//console
This message is automatically generated.
+1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12775846/HBASE-14901.6.patch
against master branch at commit d18c1af3bb12d6bf1800f22a5289bcb71020bb65.
ATTACHMENT ID: 12775846
+1 @author. The patch does not contain any @author tags.
+1 tests included. The patch appears to include 4 new or modified tests.
+1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.0 2.4.1 2.5.0 2.5.1 2.5.2 2.6.0 2.6.1 2.7.0 2.7.1)
+1 javac. The applied patch does not increase the total number of javac compiler warnings.
+1 protoc. The applied patch does not increase the total number of protoc compiler warnings.
+1 javadoc. The javadoc tool did not generate any warning messages.
+1 checkstyle. The applied patch does not generate new checkstyle errors.
+1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) warnings.
+1 release audit. The applied patch does not increase the total number of release audit warnings.
+1 lineLengths. The patch does not introduce lines longer than 100
+1 site. The mvn post-site goal succeeds with this patch.
+1 core tests. The patch passed unit tests in .
+1 zombies. No zombie tests found running at the end of the build.
Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/16771//testReport/
Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/16771//artifact/patchprocess/newFindbugsWarnings.html
Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/16771//artifact/patchprocess/checkstyle-aggregate.html
Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/16771//console
This message is automatically generated.
FAILURE: Integrated in HBase-Trunk_matrix #545 (See https://builds.apache.org/job/HBase-Trunk_matrix/545/)
HBASE-14901 Remove duplicate code to create/manage encryption keys (garyh: rev 9511150bd60e5149856c23c90422e2da7114892e)
- hbase-server/src/main/java/org/apache/hadoop/hbase/mob/mapreduce/MemStoreWrapper.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
- hbase-client/src/test/java/org/apache/hadoop/hbase/security/TestEncryptionUtil.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderImpl.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/mob/MobUtils.java
- hbase-client/src/main/java/org/apache/hadoop/hbase/security/EncryptionUtil.java
- hbase-server/src/main/java/org/apache/hadoop/hbase/mob/compactions/PartitionedMobCompactor.java
ghelmling or eclark can one of you take a quick look at this?