Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-13903

Improvements to KMS logging to help debug authorization errors

Details

    • Bug
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • 2.9.0, 3.0.0-alpha2
    • 2.9.0, 3.0.0-alpha2
    • kms
    • 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

        1. HADOOP-13903.patch
          3 kB
          Tristan Stevens
        2. HADOOP-13903-2.patch
          3 kB
          Tristan Stevens
        3. HADOOP-13903-3.patch
          3 kB
          Tristan Stevens
        4. HADOOP-13903-5.patch
          7 kB
          Tristan Stevens
        5. HADOOP-13903-5-branch2.7.patch
          6 kB
          Tristan Stevens
        6. HADOOP-13903-6.patch
          10 kB
          Tristan Stevens
        7. HADOOP-13903-6.patch
          10 kB
          Tristan Stevens
        8. HADOOP-13903-7.patch
          8 kB
          Tristan Stevens
        9. HADOOP-13903-branch2.7-6.patch
          7 kB
          Tristan Stevens
        10. HADOOP-13903-branch2.7-7.patch
          7 kB
          Tristan Stevens

        Activity

          version 1 of patch

          tmgstev Tristan Stevens added a comment - version 1 of patch
          stevel@apache.org Steve Loughran added a comment -

          I like extra logs, especially those for debugging security issues, so this is welcome.

          1. we normally just use LOG.debug(), over trace
          2. 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).
          stevel@apache.org Steve Loughran added a comment - 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).
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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?

          tmgstev Tristan Stevens added a comment - 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?
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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.
          asuresh Arun Suresh added a comment -

          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 Arun Suresh added a comment - 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.

          tmgstev Tristan Stevens added a comment - 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.
          asuresh Arun Suresh added a comment -

          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.

          asuresh Arun Suresh added a comment - 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?

          tmgstev Tristan Stevens added a comment - 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?

          Version3 of patch

          tmgstev Tristan Stevens added a comment - Version3 of patch
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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.
          asuresh Arun Suresh added a comment -

          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..

          asuresh Arun Suresh added a comment - 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).

          tmgstev Tristan Stevens added a comment - 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).
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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.
          asuresh Arun Suresh added a comment -

          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.

          asuresh Arun Suresh added a comment - 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.

          tmgstev Tristan Stevens added a comment - 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.
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 6s HADOOP-13903 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          This message was automatically generated.

          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 6s HADOOP-13903 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-13903 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.
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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.

          tmgstev Tristan Stevens added a comment - 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.
          hadoopqa Hadoop QA added a comment -
          -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 HADOOP-13903
          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.

          hadoopqa Hadoop QA added a comment - -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 HADOOP-13903 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.
          asuresh Arun Suresh added a comment - - edited

          +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) ?

          asuresh Arun Suresh added a comment - - edited +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.

          tmgstev Tristan Stevens added a comment - 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.
          xyao Xiaoyu Yao added a comment -

          Thanks tmgstev for adding the traces. Can you fix the checkstyle issues from Jenkins? Otherwise, the latest patch v7 looks good to me.

          xyao Xiaoyu Yao added a comment - Thanks tmgstev for adding the traces. Can you fix the checkstyle issues from Jenkins? Otherwise, the latest patch v7 looks good to me.
          asuresh Arun Suresh added a comment -

          Committing this shortly (I'll take care of the unused imports and the javadoc when i check in)

          asuresh Arun Suresh added a comment - 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)

          tmgstev Tristan Stevens added a comment - 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)
          asuresh Arun Suresh added a comment -

          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.

          asuresh Arun Suresh added a comment - 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 .
          hudson Hudson added a comment -

          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
          hudson Hudson added a comment - 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

          People

            tmgstev Tristan Stevens
            tmgstev Tristan Stevens
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: