Details
-
Bug
-
Status: Resolved
-
Minor
-
Resolution: Fixed
-
2.9.0, 3.0.0-alpha2
-
None
-
Patch
Description
At the moment there is no debug or trace level logs generated for KMS authorisation decisions. In order for users to understand what is going on in given scenarios this would be invaluable.
Code should endeavour to keep as much work off the sunny-day-code-path as much as possible.
Attachments
Attachments
- HADOOP-13903.patch
- 3 kB
- Tristan Stevens
- HADOOP-13903-2.patch
- 3 kB
- Tristan Stevens
- HADOOP-13903-3.patch
- 3 kB
- Tristan Stevens
- HADOOP-13903-5.patch
- 7 kB
- Tristan Stevens
- HADOOP-13903-5-branch2.7.patch
- 6 kB
- Tristan Stevens
- HADOOP-13903-6.patch
- 10 kB
- Tristan Stevens
- HADOOP-13903-6.patch
- 10 kB
- Tristan Stevens
- HADOOP-13903-7.patch
- 8 kB
- Tristan Stevens
- HADOOP-13903-branch2.7-6.patch
- 7 kB
- Tristan Stevens
- HADOOP-13903-branch2.7-7.patch
- 7 kB
- Tristan Stevens
Activity
I like extra logs, especially those for debugging security issues, so this is welcome.
- we normally just use LOG.debug(), over trace
- now we are moving to SL4J as classes get updated, you can move off the LOG.trace("User: [" + ugi.getShortUserName() + "] ") code and the .isTraceEnabled() guard, and just go: LOG.debug("User [{}]", ugi.getShortUserName). No string concat will take place unless debug is enabled, so cost is minimal (one string object instantiation is all).
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 17s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 8m 50s | trunk passed |
+1 | compile | 11m 31s | trunk passed |
+1 | checkstyle | 0m 18s | trunk passed |
+1 | mvnsite | 0m 24s | trunk passed |
+1 | mvneclipse | 0m 19s | trunk passed |
+1 | findbugs | 0m 30s | trunk passed |
+1 | javadoc | 0m 19s | trunk passed |
+1 | mvninstall | 0m 20s | the patch passed |
+1 | compile | 11m 31s | the patch passed |
+1 | javac | 11m 31s | the patch passed |
-0 | checkstyle | 0m 18s | hadoop-common-project/hadoop-kms: The patch generated 5 new + 6 unchanged - 0 fixed = 11 total (was 6) |
+1 | mvnsite | 0m 24s | the patch passed |
+1 | mvneclipse | 0m 17s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 41s | the patch passed |
+1 | javadoc | 0m 16s | the patch passed |
-1 | unit | 2m 2s | hadoop-kms in the patch failed. |
+1 | asflicense | 0m 34s | The patch does not generate ASF License warnings. |
40m 36s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.crypto.key.kms.server.TestKMS |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12843204/HADOOP-13903.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 4a0efdd0e0ef 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 72bff19 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/11273/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/11273/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11273/testReport/ |
modules | C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11273/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Thanks stevel@apache.org
I can change to LOG.debug("Checking user {}, groups: {} for: {} " + acl.getAclString(), ugi.getShortUserName(), Arrays.toString(ugi.getGroupNames()), opType.toString() ); but we'd still be evaluating those three functions (including Arrays.toString() etc) on every access.
To that end, I'd be tempted to keep the .isDebugEnabled() statements in. What do you think?
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 18s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 6m 47s | trunk passed |
+1 | compile | 9m 49s | trunk passed |
+1 | checkstyle | 0m 17s | trunk passed |
+1 | mvnsite | 0m 23s | trunk passed |
+1 | mvneclipse | 0m 17s | trunk passed |
+1 | findbugs | 0m 27s | trunk passed |
+1 | javadoc | 0m 17s | trunk passed |
+1 | mvninstall | 0m 17s | the patch passed |
+1 | compile | 9m 13s | the patch passed |
+1 | javac | 9m 13s | the patch passed |
-0 | checkstyle | 0m 17s | hadoop-common-project/hadoop-kms: The patch generated 3 new + 6 unchanged - 0 fixed = 9 total (was 6) |
+1 | mvnsite | 0m 23s | the patch passed |
+1 | mvneclipse | 0m 18s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 36s | the patch passed |
+1 | javadoc | 0m 18s | the patch passed |
-1 | unit | 2m 0s | hadoop-kms in the patch failed. |
+1 | asflicense | 0m 32s | The patch does not generate ASF License warnings. |
34m 11s |
Reason | Tests |
---|---|
Failed junit tests | hadoop.crypto.key.kms.server.TestKMS |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12843247/HADOOP-13903-2.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux a3e3387c87e8 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 72bff19 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/11275/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/11275/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11275/testReport/ |
modules | C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11275/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Doesn't the KMSAudit expose all this via the ok() / unauthorized() and error() methods ? They seem to already be invoked by the KMSACLs::assertAccess() method
asuresh The KMSACLs::assertAccess() method audits unauthorized operations (not ok() or error()), but doesn't give any information as to why the operation was unauthorized (also, KMSACLs::hasAccessToKey() doesn't appear to audit anything for some reason).
This patch gives detailed information about the reasons why operations were or were not authorised, including the full decision tree, which groups a user has, and whether they match whitelist or blacklists or key specific ACLs.
KMSACLs::hasAccessToKey() doesn't appear to audit anything for some reason..
Then I guess, we should should call KMSAudit::unauthorized() there if the check fails.
... if (LOG.isDebugEnabled()) { if (blacklist == null) { LOG.debug("No blacklist for {}", type.toString()); } else if (access) { LOG.debug("user is in {}" , blacklist.getAclString()); } else { LOG.debug("user is not in {}" , blacklist.getAclString()); } ....
Not really sure if we should explicitly print a user and his/her associated acl in the logs.
SImilarly:
.... if (LOG.isDebugEnabled()) { LOG.debug("Checking user [{}], groups: {} for: {} {} ", ugi.getShortUserName(), Arrays.toString(ugi.getGroupNames()), type.toString(), acls.get(type).getAclString() ); } ....
Printing the users group information explicitly in the log might have security implications.
I've updated the patch to remove the ugi.getGroupNames(). The usernames are all over the logs in most things anyway (good examples in NameNode logs, Hive Server2 or in Apache Sentry logs, and the ACLs are already in the logs. Of course you'd only do this in a controlled scenario - you'd hope that only an authorised user would be able to change the log4j.properties. Personally I'd rather keep the ugi.getGroupNames in there, but happy to cede to your judgement there.
Then I guess, we should should call KMSAudit::unauthorized() there if the check fails.
I agree with that, however we'd need to add a new method to KMSAudit as there is no unauthorized method that takes KeyOpType - I propose that goes in a separate JIRA, although I can add that in here if you'd like?
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 17m 2s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 12m 33s | trunk passed |
+1 | compile | 9m 30s | trunk passed |
+1 | checkstyle | 0m 17s | trunk passed |
+1 | mvnsite | 0m 22s | trunk passed |
+1 | mvneclipse | 0m 17s | trunk passed |
+1 | findbugs | 0m 27s | trunk passed |
+1 | javadoc | 0m 18s | trunk passed |
+1 | mvninstall | 0m 16s | the patch passed |
+1 | compile | 9m 18s | the patch passed |
+1 | javac | 9m 18s | the patch passed |
-0 | checkstyle | 0m 17s | hadoop-common-project/hadoop-kms: The patch generated 1 new + 6 unchanged - 0 fixed = 7 total (was 6) |
+1 | mvnsite | 0m 22s | the patch passed |
+1 | mvneclipse | 0m 15s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 35s | the patch passed |
+1 | javadoc | 0m 18s | the patch passed |
+1 | unit | 2m 12s | hadoop-kms in the patch passed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
56m 35s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12843462/HADOOP-13903-3.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 6522cdfed191 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / 70ca1f1 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/11287/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11287/testReport/ |
modules | C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11287/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
KMSAudit as there is no unauthorized method that takes KeyOpType - I propose that goes in a separate JIRA, although I can add that in here if you'd like?
I guess it should be fine to add it here..
Made changes to log failed Key ACL requests, although because there's separate enums for Key operations as to KMS Operations, the audit logger now takes either using Object.
I've attached patches based on trunk (3.x alpha and also 2.7).
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 11s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 12m 54s | trunk passed |
+1 | compile | 9m 49s | trunk passed |
+1 | checkstyle | 0m 18s | trunk passed |
+1 | mvnsite | 0m 22s | trunk passed |
+1 | mvneclipse | 0m 18s | trunk passed |
+1 | findbugs | 0m 28s | trunk passed |
+1 | javadoc | 0m 16s | trunk passed |
+1 | mvninstall | 0m 16s | the patch passed |
+1 | compile | 9m 14s | the patch passed |
+1 | javac | 9m 14s | the patch passed |
-0 | checkstyle | 0m 17s | hadoop-common-project/hadoop-kms: The patch generated 1 new + 7 unchanged - 0 fixed = 8 total (was 7) |
+1 | mvnsite | 0m 22s | the patch passed |
+1 | mvneclipse | 0m 17s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 34s | the patch passed |
+1 | javadoc | 0m 18s | the patch passed |
+1 | unit | 2m 13s | hadoop-kms in the patch passed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
40m 22s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12845504/HADOOP-13903-5.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux d35dee17ef1c 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / e49e0a6 |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/11353/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11353/testReport/ |
modules | C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11353/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Thanks for updating the patch tmgstev. Am not really in favor of passing Object arguments, but since the change is restricted to private methods I guess its fine. But it would probably be a good idea to probably put a javadoc note in the AuditEvent constructor.
+1 pending that and the whitespace fix.
Updated both patches to resolve whitespace issue and to add Javadoc for public method with Object.
Also, changed some of the Object signatures to String where possible.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 0s | Docker mode activated. |
-1 | patch | 0m 6s | |
Subsystem | Report/Notes |
---|---|
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12846322/HADOOP-13903-branch2.7-6.patch |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11399/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 15s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
0 | mvndep | 1m 47s | Maven dependency ordering for branch |
+1 | mvninstall | 12m 49s | trunk passed |
+1 | compile | 9m 52s | trunk passed |
+1 | checkstyle | 1m 34s | trunk passed |
+1 | mvnsite | 1m 24s | trunk passed |
+1 | mvneclipse | 0m 35s | trunk passed |
+1 | findbugs | 1m 43s | trunk passed |
+1 | javadoc | 0m 44s | trunk passed |
0 | mvndep | 0m 17s | Maven dependency ordering for patch |
+1 | mvninstall | 0m 44s | the patch passed |
+1 | compile | 9m 35s | the patch passed |
+1 | javac | 9m 35s | the patch passed |
+1 | checkstyle | 1m 35s | the patch passed |
+1 | mvnsite | 1m 29s | the patch passed |
+1 | mvneclipse | 0m 42s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
-1 | findbugs | 0m 41s | hadoop-common-project/hadoop-kms generated 4 new + 0 unchanged - 0 fixed = 4 total (was 0) |
+1 | javadoc | 0m 51s | the patch passed |
-1 | unit | 2m 18s | hadoop-kms in the patch failed. |
+1 | unit | 0m 37s | hadoop-yarn-api in the patch passed. |
+1 | asflicense | 0m 38s | The patch does not generate ASF License warnings. |
75m 38s |
Reason | Tests |
---|---|
FindBugs | module:hadoop-common-project/hadoop-kms |
Possible null pointer dereference of op in org.apache.hadoop.crypto.key.kms.server.KMSAudit.op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) Dereferenced at KMSAudit.java:op in org.apache.hadoop.crypto.key.kms.server.KMSAudit.op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) Dereferenced at KMSAudit.java:[line 224] | |
Non-virtual method call in org.apache.hadoop.crypto.key.kms.server.KMSAudit.error(UserGroupInformation, String, String, String) passes null for non-null parameter of op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) At KMSAudit.java:String, String, String) passes null for non-null parameter of op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) At KMSAudit.java:[line 249] | |
Non-virtual method call in org.apache.hadoop.crypto.key.kms.server.KMSAudit.unauthenticated(String, String, String, String) passes null for non-null parameter of op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) At KMSAudit.java:String, String, String) passes null for non-null parameter of op(KMSAuditLogger$OpStatus, Object, UserGroupInformation, String, String, String) At KMSAudit.java:[line 255] | |
String is incompatible with expected argument type KMS$KMSOp in org.apache.hadoop.crypto.key.kms.server.SimpleKMSAuditLogger.logAuditEvent(KMSAuditLogger$OpStatus, KMSAuditLogger$AuditEvent) At SimpleKMSAuditLogger.java:argument type KMS$KMSOp in org.apache.hadoop.crypto.key.kms.server.SimpleKMSAuditLogger.logAuditEvent(KMSAuditLogger$OpStatus, KMSAuditLogger$AuditEvent) At SimpleKMSAuditLogger.java:[line 56] | |
Failed junit tests | hadoop.crypto.key.kms.server.TestKMSAudit |
hadoop.crypto.key.kms.server.TestKMS |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12846325/HADOOP-13903-6.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux 7d7e7bdaa4c0 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / db490ec |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
findbugs | https://builds.apache.org/job/PreCommit-HADOOP-Build/11400/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-kms.html |
unit | https://builds.apache.org/job/PreCommit-HADOOP-Build/11400/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11400/testReport/ |
modules | C: hadoop-common-project/hadoop-kms hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api U: . |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11400/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
Last patches were bad - replacing with String was not easily feasible. Reverted to just fix the whitespace. New patches attached that address Javadoc and whitespace issues. Unit tests and checkstyle appear to pass on both trunk and branch-2.7.3.
-1 overall |
Vote | Subsystem | Runtime | Comment |
---|---|---|---|
0 | reexec | 0m 17s | Docker mode activated. |
+1 | @author | 0m 0s | The patch does not contain any @author tags. |
-1 | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
+1 | mvninstall | 12m 53s | trunk passed |
+1 | compile | 9m 34s | trunk passed |
+1 | checkstyle | 0m 18s | trunk passed |
+1 | mvnsite | 0m 53s | trunk passed |
+1 | mvneclipse | 0m 18s | trunk passed |
+1 | findbugs | 0m 27s | trunk passed |
+1 | javadoc | 0m 17s | trunk passed |
+1 | mvninstall | 0m 13s | the patch passed |
+1 | compile | 9m 8s | the patch passed |
+1 | javac | 9m 8s | the patch passed |
-0 | checkstyle | 0m 18s | hadoop-common-project/hadoop-kms: The patch generated 3 new + 7 unchanged - 0 fixed = 10 total (was 7) |
+1 | mvnsite | 0m 53s | the patch passed |
+1 | mvneclipse | 0m 17s | the patch passed |
+1 | whitespace | 0m 0s | The patch has no whitespace issues. |
+1 | findbugs | 0m 35s | the patch passed |
+1 | javadoc | 0m 17s | the patch passed |
+1 | unit | 2m 13s | hadoop-kms in the patch passed. |
+1 | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
41m 7s |
Subsystem | Report/Notes |
---|---|
Docker | Image:yetus/hadoop:a9ad5d6 |
JIRA Issue | |
JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12846617/HADOOP-13903-7.patch |
Optional Tests | asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle |
uname | Linux ecbbec02f279 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
Build tool | maven |
Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh |
git revision | trunk / c18590f |
Default Java | 1.8.0_111 |
findbugs | v3.0.0 |
checkstyle | https://builds.apache.org/job/PreCommit-HADOOP-Build/11411/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt |
Test Results | https://builds.apache.org/job/PreCommit-HADOOP-Build/11411/testReport/ |
modules | C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms |
Console output | https://builds.apache.org/job/PreCommit-HADOOP-Build/11411/console |
Powered by | Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
+1 for the latest patch. Will commit this to trunk tomorrow if no objections from anyone else.
tmgstev, will you be posting a branch-2 patch (since this is targeted for 2.9.0 as well) ?
Thanks asuresh.
The patches we have apply as follows:
- trunk: HADOOP-13903-7.patch applies and tests cleanly.
- branch-2: HADOOP-13903-7.patch applies and tests cleanly.
- branch-2.8: No patch but the repo doesn't seem to compile cleanly at the moment anyway. Errors in org.apache.hadoop.crypto.key.kms.server.KMSAuthenticationFilter.java.
- branch-2.7: HADOOP-13903-branch2.7-7.patch applies and tests cleanly.
If you want me to provide a patch for branch-2.8 then I can do.
Thanks tmgstev for adding the traces. Can you fix the checkstyle issues from Jenkins? Otherwise, the latest patch v7 looks good to me.
Committing this shortly (I'll take care of the unused imports and the javadoc when i check in)
The unused imports are actually for the javadoc. The two long lines are
import statements which I think are okay?
On 11 Jan 2017 08:08, "Arun Suresh (JIRA)" <jira@apache.org> wrote:
[ https://issues.apache.org/jira/browse/HADOOP-13903?page=
com.atlassian.jira.plugin.system.issuetabpanels:comment-
tabpanel&focusedCommentId=15817602#comment-15817602 ]
Arun Suresh commented on HADOOP-13903:
--------------------------------------
Committing this shortly (I'll take care of the unused imports and the
javadoc when i check in)
HADOOP-13903-5-branch2.7.patch, HADOOP-13903-5.patch, HADOOP-13903-6.patch,
HADOOP-13903-6.patch, HADOOP-13903-7.patch, HADOOP-13903-branch2.7-6.patch,
HADOOP-13903-branch2.7-7.patch, HADOOP-13903.patch
authorisation decisions. In order for users to understand what is going on
in given scenarios this would be invaluable.
much as possible.
–
This message was sent by Atlassian JIRA
(v6.3.4#6332)
As Tristan pointed out, looks like the "unused" imports are actually needed, since they are referred to in the javadocs.
Committed this to trunk and branch-2 after fixing the remaining checkstyle (to add the '.' at the end of the sentence). Thanks again for working on this tmgstev and for the reviews steve_l and xyao.
SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11105 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11105/)
HADOOP-13903. Improvements to KMS logging to help debug authorization (arun suresh: rev be529dade182dd2f3718fc52133f43e83dce191f)
- (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSAuditLogger.java
- (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSACLs.java
- (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSAudit.java
version 1 of patch