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

Improvements to KMS logging to help debug authorization errors

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.9.0, 3.0.0-alpha2
    • Fix Version/s: 2.9.0, 3.0.0-alpha2
    • Component/s: kms
    • Labels:
      None
    • Flags:
      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.

      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

        Hide
        tmgstev Tristan Stevens added a comment -

        version 1 of patch

        Show
        tmgstev Tristan Stevens added a comment - version 1 of patch
        Hide
        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).
        Show
        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).
        Hide
        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.

        Show
        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.
        Hide
        tmgstev Tristan Stevens added a comment -

        Thanks Steve Loughran

        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?

        Show
        tmgstev Tristan Stevens added a comment - Thanks Steve Loughran 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?
        Hide
        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.

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

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

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

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

        Show
        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.
        Hide
        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?

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

        Version3 of patch

        Show
        tmgstev Tristan Stevens added a comment - Version3 of patch
        Hide
        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.

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

        Show
        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..
        Hide
        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).

        Show
        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).
        Hide
        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.

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

        Thanks for updating the patch Tristan Stevens. 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.

        Show
        asuresh Arun Suresh added a comment - Thanks for updating the patch Tristan Stevens . 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.
        Hide
        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.

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

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

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

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

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

        +1 for the latest patch. Will commit this to trunk tomorrow if no objections from anyone else.
        Tristan Stevens, will you be posting a branch-2 patch (since this is targeted for 2.9.0 as well) ?

        Show
        asuresh Arun Suresh added a comment - - edited +1 for the latest patch. Will commit this to trunk tomorrow if no objections from anyone else. Tristan Stevens , will you be posting a branch-2 patch (since this is targeted for 2.9.0 as well) ?
        Hide
        tmgstev Tristan Stevens added a comment -

        Thanks Arun Suresh.

        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.

        Show
        tmgstev Tristan Stevens added a comment - Thanks Arun Suresh . 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.
        Hide
        xyao Xiaoyu Yao added a comment -

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

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

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

        Show
        asuresh Arun Suresh added a comment - Committing this shortly (I'll take care of the unused imports and the javadoc when i check in)
        Hide
        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)

        Show
        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)
        Hide
        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 Tristan Stevens and for the reviews Steve Loughran and Xiaoyu Yao.

        Show
        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 Tristan Stevens and for the reviews Steve Loughran and Xiaoyu Yao .
        Hide
        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
        Show
        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

          • Assignee:
            tmgstev Tristan Stevens
            Reporter:
            tmgstev Tristan Stevens
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development