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

KMS should set UGI's Configuration object properly

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: kms
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      We found that the Configuration object in UGI in KMS server is not initialized properly, therefore it does not load core-site.xml from KMSConfiguration.KMS_CONFIG_DIR.

      This becomes a problem when the Hadoop cluster uses LdapGroupsMapping for group resolution, because the UGI in KMS falls back to the default JniBasedUnixGroupsMappingWithFallback (defined in core-default.xml) and is thus not consistent with the Hadoop cluster.

      1. HADOOP-13638.001.patch
        1 kB
        Wei-Chiu Chuang
      2. HADOOP-13638.002.patch
        11 kB
        Wei-Chiu Chuang

        Activity

        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        v01. a one-liner fixer to set UGI configuration correctly when KMS starts.

        I don't think it's easy to unit test it, but I've manually verified this fix works on a CDH 5.7.0 cluster.

        Show
        jojochuang Wei-Chiu Chuang added a comment - v01. a one-liner fixer to set UGI configuration correctly when KMS starts. I don't think it's easy to unit test it, but I've manually verified this fix works on a CDH 5.7.0 cluster.
        Hide
        xiaochen Xiao Chen added a comment -

        +1 pending jenkins. Nice work Wei-Chiu!
        This is clearly a bug and I think manual testing is fine. Could you elaborate a little bit on the manual testings you've done? Thanks.

        Show
        xiaochen Xiao Chen added a comment - +1 pending jenkins. Nice work Wei-Chiu! This is clearly a bug and I think manual testing is fine. Could you elaborate a little bit on the manual testings you've done? Thanks.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        Here's what I did to verify the patch:

        Configure a CDH Hadoop cluster using LdapGroupsMapping and KMS. The KMS ACL rule denies "group1" from decrypting the key. I added additional log at Groups#<init> to print the class name of the GroupMapping resolution object. Subsequently, I started KMS and do a few operations in a HDFS encryption zone to observe the class name printed.

        Show
        jojochuang Wei-Chiu Chuang added a comment - Here's what I did to verify the patch: Configure a CDH Hadoop cluster using LdapGroupsMapping and KMS. The KMS ACL rule denies "group1" from decrypting the key. I added additional log at Groups#<init> to print the class name of the GroupMapping resolution object. Subsequently, I started KMS and do a few operations in a HDFS encryption zone to observe the class name printed.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 12s 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 52s trunk passed
        +1 compile 7m 19s trunk passed
        +1 checkstyle 0m 11s trunk passed
        +1 mvnsite 0m 18s trunk passed
        +1 mvneclipse 0m 11s trunk passed
        +1 findbugs 0m 22s trunk passed
        +1 javadoc 0m 11s trunk passed
        +1 mvninstall 0m 16s the patch passed
        +1 compile 7m 13s the patch passed
        +1 javac 7m 13s the patch passed
        +1 checkstyle 0m 13s the patch passed
        +1 mvnsite 0m 18s the patch passed
        +1 mvneclipse 0m 13s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 31s the patch passed
        +1 javadoc 0m 14s the patch passed
        -1 unit 1m 32s hadoop-kms in the patch failed.
        +1 asflicense 0m 22s The patch does not generate ASF License warnings.
        27m 52s



        Reason Tests
        Failed junit tests hadoop.crypto.key.kms.server.TestKMS



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HADOOP-13638
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829690/HADOOP-13638.001.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 9ddaad62c252 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 964e546
        Default Java 1.8.0_101
        findbugs v3.0.0
        unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-kms.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/testReport/
        modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/console
        Powered by Apache Yetus 0.4.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 12s 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 52s trunk passed +1 compile 7m 19s trunk passed +1 checkstyle 0m 11s trunk passed +1 mvnsite 0m 18s trunk passed +1 mvneclipse 0m 11s trunk passed +1 findbugs 0m 22s trunk passed +1 javadoc 0m 11s trunk passed +1 mvninstall 0m 16s the patch passed +1 compile 7m 13s the patch passed +1 javac 7m 13s the patch passed +1 checkstyle 0m 13s the patch passed +1 mvnsite 0m 18s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 31s the patch passed +1 javadoc 0m 14s the patch passed -1 unit 1m 32s hadoop-kms in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 27m 52s Reason Tests Failed junit tests hadoop.crypto.key.kms.server.TestKMS Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13638 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829690/HADOOP-13638.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 9ddaad62c252 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 964e546 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-kms.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/testReport/ modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10559/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks for the explanation Wei-Chiu, sgtm.
        Failed test looks related.

        Show
        xiaochen Xiao Chen added a comment - Thanks for the explanation Wei-Chiu, sgtm. Failed test looks related.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        The tests failed because UserGroupInformation.setConfiguration(conf) sets a global variable, and each test has a client side configuration which is overwritten by it.

        Need to find a way to avoid interference between client and server.

        Show
        jojochuang Wei-Chiu Chuang added a comment - The tests failed because UserGroupInformation.setConfiguration(conf) sets a global variable, and each test has a client side configuration which is overwritten by it. Need to find a way to avoid interference between client and server.
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks for looking into the test Wei-Chiu. Looking forward to the fix.
        This will also give audience more time to review this.

        Show
        xiaochen Xiao Chen added a comment - Thanks for looking into the test Wei-Chiu. Looking forward to the fix. This will also give audience more time to review this.
        Hide
        jojochuang Wei-Chiu Chuang added a comment - - edited

        v02: The Configuration object is shared by both KMS client and server in unit tests because UGI gets/sets it to a static variable. I don't see a way to isolate client's configuration from server.

        Fortunately, UGI-sensitive configuration names are independent between KMS client and server. As a workaround, make sure the client configurations are copied to the server's so that client can read them.

        Show
        jojochuang Wei-Chiu Chuang added a comment - - edited v02: The Configuration object is shared by both KMS client and server in unit tests because UGI gets/sets it to a static variable. I don't see a way to isolate client's configuration from server. Fortunately, UGI-sensitive configuration names are independent between KMS client and server. As a workaround, make sure the client configurations are copied to the server's so that client can read them.
        Hide
        hadoopqa Hadoop QA added a comment -
        +1 overall



        Vote Subsystem Runtime Comment
        0 reexec 0m 20s Docker mode activated.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 7m 43s trunk passed
        +1 compile 7m 32s trunk passed
        +1 checkstyle 0m 14s trunk passed
        +1 mvnsite 0m 18s trunk passed
        +1 mvneclipse 0m 12s trunk passed
        +1 findbugs 0m 23s trunk passed
        +1 javadoc 0m 11s trunk passed
        +1 mvninstall 0m 15s the patch passed
        +1 compile 7m 5s the patch passed
        +1 javac 7m 5s the patch passed
        -0 checkstyle 0m 14s hadoop-common-project/hadoop-kms: The patch generated 3 new + 114 unchanged - 3 fixed = 117 total (was 117)
        +1 mvnsite 0m 18s the patch passed
        +1 mvneclipse 0m 12s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 findbugs 0m 29s the patch passed
        +1 javadoc 0m 12s the patch passed
        +1 unit 2m 8s hadoop-kms in the patch passed.
        +1 asflicense 0m 21s The patch does not generate ASF License warnings.
        29m 36s



        Subsystem Report/Notes
        Docker Image:yetus/hadoop:9560f25
        JIRA Issue HADOOP-13638
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829906/HADOOP-13638.002.patch
        Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
        uname Linux 4f77a4ead812 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
        git revision trunk / 40acace
        Default Java 1.8.0_101
        findbugs v3.0.0
        checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt
        Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/testReport/
        modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms
        Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/console
        Powered by Apache Yetus 0.4.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 20s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 7m 43s trunk passed +1 compile 7m 32s trunk passed +1 checkstyle 0m 14s trunk passed +1 mvnsite 0m 18s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 0m 23s trunk passed +1 javadoc 0m 11s trunk passed +1 mvninstall 0m 15s the patch passed +1 compile 7m 5s the patch passed +1 javac 7m 5s the patch passed -0 checkstyle 0m 14s hadoop-common-project/hadoop-kms: The patch generated 3 new + 114 unchanged - 3 fixed = 117 total (was 117) +1 mvnsite 0m 18s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 29s the patch passed +1 javadoc 0m 12s the patch passed +1 unit 2m 8s hadoop-kms in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 29m 36s Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13638 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12829906/HADOOP-13638.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4f77a4ead812 3.13.0-92-generic #139-Ubuntu SMP Tue Jun 28 20:42:26 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 40acace Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-kms.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/testReport/ modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10574/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        Checkstyle warnings can't be removed unless we refactor test methods.

        Show
        jojochuang Wei-Chiu Chuang added a comment - Checkstyle warnings can't be removed unless we refactor test methods.
        Hide
        xiaochen Xiao Chen added a comment -

        Thanks Wei-Chiu Chuang for the new patch. Could you explain why do we need newConf = new Configuration(false); instead of the default ctor?

        Show
        xiaochen Xiao Chen added a comment - Thanks Wei-Chiu Chuang for the new patch. Could you explain why do we need newConf = new Configuration(false); instead of the default ctor?
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        That was copied from an existing test case.

        Show
        jojochuang Wei-Chiu Chuang added a comment - That was copied from an existing test case.
        Hide
        xiaochen Xiao Chen added a comment -

        Checkstyle is out-of-scope as Wei-Chiu mentioned. +1 to patch 2, committing this.

        Show
        xiaochen Xiao Chen added a comment - Checkstyle is out-of-scope as Wei-Chiu mentioned. +1 to patch 2, committing this.
        Hide
        xiaochen Xiao Chen added a comment -

        I have committed this to trunk, branch-2 and branch-2.8. Let me know if you feel this should go to earlier branches, from the Target Versions field.

        Thanks for the contribution Wei-Chiu Chuang!

        Show
        xiaochen Xiao Chen added a comment - I have committed this to trunk, branch-2 and branch-2.8. Let me know if you feel this should go to earlier branches, from the Target Versions field. Thanks for the contribution Wei-Chiu Chuang !
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10489 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10489/)
        HADOOP-13638. KMS should set UGI's Configuration object properly. (xiao: rev fa397e74fe988bcbb05c816de73eb738794ace4b)

        • (edit) hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java
        • (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebApp.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10489 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10489/ ) HADOOP-13638 . KMS should set UGI's Configuration object properly. (xiao: rev fa397e74fe988bcbb05c816de73eb738794ace4b) (edit) hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java (edit) hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSWebApp.java
        Hide
        jojochuang Wei-Chiu Chuang added a comment -

        Thanks very much Xiao Chen for reviewing and committing the patch!

        Show
        jojochuang Wei-Chiu Chuang added a comment - Thanks very much Xiao Chen for reviewing and committing the patch!

          People

          • Assignee:
            jojochuang Wei-Chiu Chuang
            Reporter:
            jojochuang Wei-Chiu Chuang
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development