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

Implement TokenRenewer to renew and cancel delegation tokens in KMS

    Details

    • Target Version/s:
    • Release Note:
      Enables renewal and cancellation of KMS delegation tokens. hadoop.security.key.provider.path needs to be configured to reach the key provider.

      Description

      Service DelegationToken (DT) renewal is done in Yarn by org.apache.hadoop.yarn.server.resourcemanager.security.DelegationTokenRenewer, where it calls Token#renew and uses ServiceLoader to find the renewer class (code), and invokes the renew method from it.

      We seem to miss the token renewer class in KMS / HttpFSFileSystem, and hence Yarn defaults to TrivialRenewer for DT of such kinds, resulting in the token not being renewed.

      As a side note, HttpFSFileSystem does have a renewDelegationToken API, but I don't see it invoked in hadoop code base. KMS does not have any renew hook.

      1. HADOOP-13155.pre.patch
        27 kB
        Xiao Chen
      2. HADOOP-13155.01.patch
        28 kB
        Xiao Chen
      3. HADOOP-13155.02.patch
        30 kB
        Xiao Chen
      4. HADOOP-13155.03.patch
        30 kB
        Xiao Chen
      5. HADOOP-13155.04.patch
        31 kB
        Xiao Chen
      6. HADOOP-13155.05.patch
        32 kB
        Xiao Chen
      7. HADOOP-13155.06.patch
        33 kB
        Xiao Chen
      8. HADOOP-13155.07.patch
        27 kB
        Xiao Chen

        Issue Links

          Activity

          Hide
          xiaochen Xiao Chen added a comment -

          FYI created HDFS-10489 for the config deprecation.

          Show
          xiaochen Xiao Chen added a comment - FYI created HDFS-10489 for the config deprecation.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Andrew Wang for the review, commit and discussions! I added a release note to this jira. Will create a new one targeting 3.0.0 to deprecate the config keys.

          Also thanks Arun, Yongjun, Wei-Chiu and Allen for your valuable inputs / reviews.

          Show
          xiaochen Xiao Chen added a comment - Thanks Andrew Wang for the review, commit and discussions! I added a release note to this jira. Will create a new one targeting 3.0.0 to deprecate the config keys. Also thanks Arun, Yongjun, Wei-Chiu and Allen for your valuable inputs / reviews.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #9911 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9911/)
          HADOOP-13155. Implement TokenRenewer to renew and cancel delegation (wang: rev 713cb71820ad94a5436f35824d07aa12fcba5cc6)

          • hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java
          • hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java
          • hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSAuthenticationFilter.java
          • hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.security.token.TokenRenewer
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/LoadBalancingKMSClientProvider.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/KMSUtil.java
          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderDelegationTokenExtension.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #9911 (See https://builds.apache.org/job/Hadoop-trunk-Commit/9911/ ) HADOOP-13155 . Implement TokenRenewer to renew and cancel delegation (wang: rev 713cb71820ad94a5436f35824d07aa12fcba5cc6) hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/DFSUtilClient.java hadoop-common-project/hadoop-kms/src/test/java/org/apache/hadoop/crypto/key/kms/server/TestKMS.java hadoop-common-project/hadoop-kms/src/main/java/org/apache/hadoop/crypto/key/kms/server/KMSAuthenticationFilter.java hadoop-common-project/hadoop-common/src/main/resources/META-INF/services/org.apache.hadoop.security.token.TokenRenewer hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/KMSClientProvider.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/kms/LoadBalancingKMSClientProvider.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/KMSUtil.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyProviderDelegationTokenExtension.java
          Hide
          andrew.wang Andrew Wang added a comment -

          Pushed back through branch-2.8. Some small fixups for the backports. Thanks again Xiao for the patch and Allen for also commenting!

          Show
          andrew.wang Andrew Wang added a comment - Pushed back through branch-2.8. Some small fixups for the backports. Thanks again Xiao for the patch and Allen for also commenting!
          Hide
          andrew.wang Andrew Wang added a comment -

          LGTM +1, will commit shortly. Thanks for working on this Xiao.

          Could you also add a release note about the new config requirement for renewal?

          Also, should we do anything about this for Hadoop 3? For instance, deprecate and remove the "dfs." key in favor of the "hadoop." key?

          Show
          andrew.wang Andrew Wang added a comment - LGTM +1, will commit shortly. Thanks for working on this Xiao. Could you also add a release note about the new config requirement for renewal? Also, should we do anything about this for Hadoop 3? For instance, deprecate and remove the "dfs. " key in favor of the "hadoop. " key?
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s 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.
          0 mvndep 1m 12s Maven dependency ordering for branch
          +1 mvninstall 5m 57s trunk passed
          +1 compile 6m 9s trunk passed
          +1 checkstyle 1m 21s trunk passed
          +1 mvnsite 1m 42s trunk passed
          +1 mvneclipse 0m 33s trunk passed
          +1 findbugs 2m 56s trunk passed
          +1 javadoc 1m 25s trunk passed
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 20s the patch passed
          +1 compile 6m 43s the patch passed
          +1 javac 6m 43s the patch passed
          -1 checkstyle 1m 23s root: The patch generated 2 new + 160 unchanged - 6 fixed = 162 total (was 166)
          +1 mvnsite 1m 48s the patch passed
          +1 mvneclipse 0m 34s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 37s the patch passed
          +1 javadoc 1m 27s the patch passed
          +1 unit 7m 14s hadoop-common in the patch passed.
          +1 unit 1m 28s hadoop-kms in the patch passed.
          +1 unit 0m 53s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          49m 22s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807604/HADOOP-13155.07.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 3b27fd943a0a 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 / 16b1cc7
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/artifact/patchprocess/diff-checkstyle-root.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/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 13s 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. 0 mvndep 1m 12s Maven dependency ordering for branch +1 mvninstall 5m 57s trunk passed +1 compile 6m 9s trunk passed +1 checkstyle 1m 21s trunk passed +1 mvnsite 1m 42s trunk passed +1 mvneclipse 0m 33s trunk passed +1 findbugs 2m 56s trunk passed +1 javadoc 1m 25s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 20s the patch passed +1 compile 6m 43s the patch passed +1 javac 6m 43s the patch passed -1 checkstyle 1m 23s root: The patch generated 2 new + 160 unchanged - 6 fixed = 162 total (was 166) +1 mvnsite 1m 48s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 37s the patch passed +1 javadoc 1m 27s the patch passed +1 unit 7m 14s hadoop-common in the patch passed. +1 unit 1m 28s hadoop-kms in the patch passed. +1 unit 0m 53s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 49m 22s Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807604/HADOOP-13155.07.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 3b27fd943a0a 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 / 16b1cc7 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/artifact/patchprocess/diff-checkstyle-root.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9644/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 again Andrew Wang for the discussion, and for getting HADOOP-13228 in.
          Attached patch 7 is a rebase on latest trunk.

          Regarding the config and compat:

          • DFSUtilClient now has a static variable for storing the config name, and defaults to dfs._ for compat. It can be set via DFSUtilClient#setKeyProviderUriKeyName.
          • The provider creation logic is extracted to a new class KMSUtil in common.
          • The newly added renewer classes uses KMSUtil, but since they're in common, they use the hadoop._ configs. Of course, this means one will need to have both configs set correctly so that the token can be renewed, but this seems to be the most compatible way.
          Show
          xiaochen Xiao Chen added a comment - Thanks again Andrew Wang for the discussion, and for getting HADOOP-13228 in. Attached patch 7 is a rebase on latest trunk. Regarding the config and compat: DFSUtilClient now has a static variable for storing the config name, and defaults to dfs._ for compat. It can be set via DFSUtilClient#setKeyProviderUriKeyName . The provider creation logic is extracted to a new class KMSUtil in common. The newly added renewer classes uses KMSUtil , but since they're in common, they use the hadoop._ configs. Of course, this means one will need to have both configs set correctly so that the token can be renewed, but this seems to be the most compatible way.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Allen Wittenauer for the suggestion.
          Given that DtUtilShell invokes the interface of org.apache.hadoop.security.token.Token#renew in DtFileOperations#renewTokenFile (and cancel in the same way), the current test in TestKMS covers it by calling the same interface.

          Show
          xiaochen Xiao Chen added a comment - Thanks Allen Wittenauer for the suggestion. Given that DtUtilShell invokes the interface of org.apache.hadoop.security.token.Token#renew in DtFileOperations#renewTokenFile (and cancel in the same way), the current test in TestKMS covers it by calling the same interface.
          Hide
          aw Allen Wittenauer added a comment -

          We should make sure this works with hadoop dtutil.

          Show
          aw Allen Wittenauer added a comment - We should make sure this works with hadoop dtutil.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks Andrew for the summary. I'm working on the test for DTAuthenticator, it's not straight forward, I'll create a new jira and link here + ping you when ready.

          Show
          xiaochen Xiao Chen added a comment - Thanks Andrew for the summary. I'm working on the test for DTAuthenticator, it's not straight forward, I'll create a new jira and link here + ping you when ready.
          Hide
          andrew.wang Andrew Wang added a comment -

          I talked with Xiao about this patch offline, here's our notes:

          • Setting the DT in the query string is deprecated, and only preserved for testing. So we don't need to support that in this new renewer functionality.
          • We should consider splitting out that bug fix into a separate JIRA, I promise to quickly review and commit since this one is dependent. A unit test would be good too.
          • We need to keep using the old "dfs..." config key for compatibility, so can't just swap to the "hadoop..." config key. I haven't seen a situation where we'd want to configure these differently, since people normally only have a single KMS instance for the entire cluster. But, compat is compat, so we think a setter function in DFSClientUtil will work. One day we will probably want per-DFS KMS configuration for cross-cluster distcp, in which case we'd also need this.
          Show
          andrew.wang Andrew Wang added a comment - I talked with Xiao about this patch offline, here's our notes: Setting the DT in the query string is deprecated, and only preserved for testing. So we don't need to support that in this new renewer functionality. We should consider splitting out that bug fix into a separate JIRA, I promise to quickly review and commit since this one is dependent. A unit test would be good too. We need to keep using the old "dfs..." config key for compatibility, so can't just swap to the "hadoop..." config key. I haven't seen a situation where we'd want to configure these differently, since people normally only have a single KMS instance for the entire cluster. But, compat is compat, so we think a setter function in DFSClientUtil will work. One day we will probably want per-DFS KMS configuration for cross-cluster distcp, in which case we'd also need this.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s 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.
          0 mvndep 0m 42s Maven dependency ordering for branch
          +1 mvninstall 6m 15s trunk passed
          +1 compile 6m 10s trunk passed
          +1 checkstyle 1m 20s trunk passed
          +1 mvnsite 1m 52s trunk passed
          +1 mvneclipse 0m 36s trunk passed
          +1 findbugs 3m 23s trunk passed
          +1 javadoc 1m 25s trunk passed
          0 mvndep 0m 10s Maven dependency ordering for patch
          +1 mvninstall 1m 22s the patch passed
          +1 compile 6m 46s the patch passed
          +1 javac 6m 46s the patch passed
          -1 checkstyle 1m 26s root: The patch generated 2 new + 232 unchanged - 6 fixed = 234 total (was 238)
          +1 mvnsite 1m 52s the patch passed
          +1 mvneclipse 0m 34s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 3m 50s the patch passed
          +1 javadoc 1m 26s the patch passed
          -1 unit 7m 16s hadoop-common in the patch failed.
          +1 unit 1m 28s hadoop-kms in the patch passed.
          +1 unit 0m 55s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          50m 20s



          Reason Tests
          Failed junit tests hadoop.security.token.delegation.web.TestWebDelegationToken



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807203/HADOOP-13155.06.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux e84f95c15e43 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 / bca31fe
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/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 14s 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. 0 mvndep 0m 42s Maven dependency ordering for branch +1 mvninstall 6m 15s trunk passed +1 compile 6m 10s trunk passed +1 checkstyle 1m 20s trunk passed +1 mvnsite 1m 52s trunk passed +1 mvneclipse 0m 36s trunk passed +1 findbugs 3m 23s trunk passed +1 javadoc 1m 25s trunk passed 0 mvndep 0m 10s Maven dependency ordering for patch +1 mvninstall 1m 22s the patch passed +1 compile 6m 46s the patch passed +1 javac 6m 46s the patch passed -1 checkstyle 1m 26s root: The patch generated 2 new + 232 unchanged - 6 fixed = 234 total (was 238) +1 mvnsite 1m 52s the patch passed +1 mvneclipse 0m 34s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 3m 50s the patch passed +1 javadoc 1m 26s the patch passed -1 unit 7m 16s hadoop-common in the patch failed. +1 unit 1m 28s hadoop-kms in the patch passed. +1 unit 0m 55s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 50m 20s Reason Tests Failed junit tests hadoop.security.token.delegation.web.TestWebDelegationToken Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807203/HADOOP-13155.06.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux e84f95c15e43 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 / bca31fe Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9624/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 -

          Patch 6 just to make jenkins happy. I know it may need further change based on further discussions.

          Show
          xiaochen Xiao Chen added a comment - Patch 6 just to make jenkins happy. I know it may need further change based on further discussions.
          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 appears to include 1 new or modified test files.
          0 mvndep 0m 48s Maven dependency ordering for branch
          +1 mvninstall 6m 31s trunk passed
          +1 compile 6m 14s trunk passed
          +1 checkstyle 1m 22s trunk passed
          +1 mvnsite 1m 49s trunk passed
          +1 mvneclipse 0m 35s trunk passed
          +1 findbugs 2m 58s trunk passed
          +1 javadoc 1m 25s trunk passed
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 1m 21s the patch passed
          +1 compile 6m 44s the patch passed
          +1 javac 6m 44s the patch passed
          -1 checkstyle 1m 23s root: The patch generated 7 new + 233 unchanged - 6 fixed = 240 total (was 239)
          +1 mvnsite 1m 42s the patch passed
          +1 mvneclipse 0m 33s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 27s hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)
          +1 javadoc 1m 25s the patch passed
          -1 unit 7m 18s hadoop-common in the patch failed.
          +1 unit 1m 27s hadoop-kms in the patch passed.
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          49m 35s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:[line 189]
            Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:[line 174]
            Possible null pointer dereference of dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:[line 146]
          Failed junit tests hadoop.security.token.delegation.web.TestWebDelegationToken



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807053/HADOOP-13155.05.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 84315fca92c4 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 / 93d8a7f
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/diff-checkstyle-root.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/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 11s 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. 0 mvndep 0m 48s Maven dependency ordering for branch +1 mvninstall 6m 31s trunk passed +1 compile 6m 14s trunk passed +1 checkstyle 1m 22s trunk passed +1 mvnsite 1m 49s trunk passed +1 mvneclipse 0m 35s trunk passed +1 findbugs 2m 58s trunk passed +1 javadoc 1m 25s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 1m 21s the patch passed +1 compile 6m 44s the patch passed +1 javac 6m 44s the patch passed -1 checkstyle 1m 23s root: The patch generated 7 new + 233 unchanged - 6 fixed = 240 total (was 239) +1 mvnsite 1m 42s the patch passed +1 mvneclipse 0m 33s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 27s hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) +1 javadoc 1m 25s the patch passed -1 unit 7m 18s hadoop-common in the patch failed. +1 unit 1m 27s hadoop-kms in the patch passed. +1 unit 0m 54s hadoop-hdfs-client in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 49m 35s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java: [line 189]   Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java: [line 174]   Possible null pointer dereference of dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java: [line 146] Failed junit tests hadoop.security.token.delegation.web.TestWebDelegationToken Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807053/HADOOP-13155.05.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 84315fca92c4 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 / 93d8a7f Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/diff-checkstyle-root.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9617/console Powered by Apache Yetus 0.4.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 17s 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.
          0 mvndep 0m 13s Maven dependency ordering for branch
          +1 mvninstall 8m 0s trunk passed
          +1 compile 8m 47s trunk passed
          +1 checkstyle 0m 33s trunk passed
          +1 mvnsite 1m 33s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 findbugs 1m 56s trunk passed
          +1 javadoc 1m 13s trunk passed
          0 mvndep 0m 7s Maven dependency ordering for patch
          +1 mvninstall 1m 13s the patch passed
          +1 compile 8m 24s the patch passed
          +1 javac 8m 24s the patch passed
          -1 checkstyle 0m 31s hadoop-common-project: The patch generated 5 new + 211 unchanged - 5 fixed = 216 total (was 216)
          +1 mvnsite 1m 26s the patch passed
          +1 mvneclipse 0m 24s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          -1 findbugs 1m 54s hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0)
          +1 javadoc 1m 20s the patch passed
          -1 unit 8m 45s hadoop-common in the patch failed.
          +1 unit 1m 40s hadoop-kms in the patch passed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          50m 31s



          Reason Tests
          FindBugs module:hadoop-common-project/hadoop-common
            Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:[line 187]
            Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:[line 173]
            Possible null pointer dereference of dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:[line 146]
          Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager
            hadoop.security.token.delegation.web.TestWebDelegationToken



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807046/HADOOP-13155.05.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 4196906aee6a 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 / 93d8a7f
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/diff-checkstyle-hadoop-common-project.txt
          findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/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 17s 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. 0 mvndep 0m 13s Maven dependency ordering for branch +1 mvninstall 8m 0s trunk passed +1 compile 8m 47s trunk passed +1 checkstyle 0m 33s trunk passed +1 mvnsite 1m 33s trunk passed +1 mvneclipse 0m 22s trunk passed +1 findbugs 1m 56s trunk passed +1 javadoc 1m 13s trunk passed 0 mvndep 0m 7s Maven dependency ordering for patch +1 mvninstall 1m 13s the patch passed +1 compile 8m 24s the patch passed +1 javac 8m 24s the patch passed -1 checkstyle 0m 31s hadoop-common-project: The patch generated 5 new + 211 unchanged - 5 fixed = 216 total (was 216) +1 mvnsite 1m 26s the patch passed +1 mvneclipse 0m 24s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. -1 findbugs 1m 54s hadoop-common-project/hadoop-common generated 3 new + 0 unchanged - 0 fixed = 3 total (was 0) +1 javadoc 1m 20s the patch passed -1 unit 8m 45s hadoop-common in the patch failed. +1 unit 1m 40s hadoop-kms in the patch passed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 50m 31s Reason Tests FindBugs module:hadoop-common-project/hadoop-common   Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.cancel(Token, Configuration) At KMSClientProvider.java: [line 187]   Load of known null value in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java:in org.apache.hadoop.crypto.key.kms.KMSClientProvider$KMSTokenRenewer.renew(Token, Configuration) At KMSClientProvider.java: [line 173]   Possible null pointer dereference of dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java:dToken in org.apache.hadoop.security.token.delegation.web.DelegationTokenAuthenticator.appendDelegationToken(URL, AuthenticatedURL$Token, Token, HttpURLConnection) Dereferenced at DelegationTokenAuthenticator.java: [line 146] Failed junit tests hadoop.security.ssl.TestReloadingX509TrustManager   hadoop.security.token.delegation.web.TestWebDelegationToken Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12807046/HADOOP-13155.05.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 4196906aee6a 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 / 93d8a7f Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/diff-checkstyle-hadoop-common-project.txt findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/new-findbugs-hadoop-common-project_hadoop-common.html unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms U: hadoop-common-project Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9616/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 - - edited

          Thanks Andrew Wang for the review! Appreciate your comments.
          Patch 5 addresses some of them, with the exceptions open for discuss:

          Regarding moving the config key, I think the other Renewers get around this by embedding the static class within the parent class and accessing required state statically. I think the parent here would be KMSClientProvider. It wouldn't be good to tie renewal to this HDFS key anyway, since the KMS is used for more than just HDFS encryption.

          Let's talk about this in more details.
          I think there're 2 things mixed here:

          1. The renewer could be a static inner class of KMSClientProvider.
            Sure, I moved the renewer into KMSCP.
          2. ...accessing required state statically...
            I don't think this is true. Since it's static it wouldn't read any states, just configs, right?
            Among those renewers, TokenAspect and MRDelegationTokenRenewer are not embedded. Both of them plus the renewers embedded in TimelineDelegationTokenIdentifier and RMDelegationTokenIdentifier all create the instance to renewDelegationToken using conf and token. For KMS, we can't get the url from the token because 1) it doesn't have proto (i.e. http v.s. https) 2) it's not HA-aware.
            So I think we need to read from config, which is what KMSUtil#createKeyProvider does when it calls into KMSCP#createProvider.
            I think Wei-Chiu Chuang's proposal may work (thanks Wei-Chiu!), but I also found HDFS-7004 when the dfs config was added, and the linked HADOOP-11054 saying 'In the case of HDFS encryption, it would be desirable to be explicitly specify a KeyProvider URI to avoid obscure misconfigurations.'. IMO, KMS being a general component should just use the common hadoop.security.key.provider.path. Reused the logic of DFSUtilClient in KMSUtil, and updated callers with proper keys.

          BTW,

          Why does renewal in particularly need a URL with USER_NAME set? IIUC this is needed for PseudoAuthentication, but here we're doing DT authentication?

          Good catch!! I guess I was not super clear on the concept so I missed it: Initially PseudoAuth was failing and I debugged and added user name to let it pass. However, since we're using DT here we definitely should just use DTAuth. After more background work, I found out theres a bug in DelegationTokenAuthenticatedURL when handling delegation tokens for renew/cancel/add. (openConnection was done right, in HADOOP-10880, but seems missed renew/cancel/add.). I fixed it in DTAuthenticatedURL and DTAuthenticator, by using a similar fall back logic. (Can't do it in DTAuthenticatedURL along because the connection object is created in DTAuthenticator for those methods.)
          Unit test on this when DelegationTokenAuthenticatedURL#useQueryStringforDelegationToken == false seems to be non-trivial and not easy to mock/spy, since it's all created on the fly. Considering this is supposed to be deprecated, I manually tested it by setting the default to true.

          Show
          xiaochen Xiao Chen added a comment - - edited Thanks Andrew Wang for the review! Appreciate your comments. Patch 5 addresses some of them, with the exceptions open for discuss: Regarding moving the config key, I think the other Renewers get around this by embedding the static class within the parent class and accessing required state statically. I think the parent here would be KMSClientProvider. It wouldn't be good to tie renewal to this HDFS key anyway, since the KMS is used for more than just HDFS encryption. Let's talk about this in more details. I think there're 2 things mixed here: The renewer could be a static inner class of KMSClientProvider. Sure, I moved the renewer into KMSCP. ...accessing required state statically... I don't think this is true. Since it's static it wouldn't read any states, just configs, right? Among those renewers, TokenAspect and MRDelegationTokenRenewer are not embedded. Both of them plus the renewers embedded in TimelineDelegationTokenIdentifier and RMDelegationTokenIdentifier all create the instance to renewDelegationToken using conf and token . For KMS, we can't get the url from the token because 1) it doesn't have proto (i.e. http v.s. https) 2) it's not HA-aware. So I think we need to read from config, which is what KMSUtil#createKeyProvider does when it calls into KMSCP#createProvider . I think Wei-Chiu Chuang 's proposal may work (thanks Wei-Chiu!), but I also found HDFS-7004 when the dfs config was added, and the linked HADOOP-11054 saying 'In the case of HDFS encryption, it would be desirable to be explicitly specify a KeyProvider URI to avoid obscure misconfigurations.'. IMO, KMS being a general component should just use the common hadoop.security.key.provider.path . Reused the logic of DFSUtilClient in KMSUtil, and updated callers with proper keys. BTW, Why does renewal in particularly need a URL with USER_NAME set? IIUC this is needed for PseudoAuthentication, but here we're doing DT authentication? Good catch!! I guess I was not super clear on the concept so I missed it: Initially PseudoAuth was failing and I debugged and added user name to let it pass. However, since we're using DT here we definitely should just use DTAuth. After more background work, I found out theres a bug in DelegationTokenAuthenticatedURL when handling delegation tokens for renew/cancel/add. ( openConnection was done right, in HADOOP-10880 , but seems missed renew/cancel/add.). I fixed it in DTAuthenticatedURL and DTAuthenticator, by using a similar fall back logic. (Can't do it in DTAuthenticatedURL along because the connection object is created in DTAuthenticator for those methods.) Unit test on this when DelegationTokenAuthenticatedURL#useQueryStringforDelegationToken == false seems to be non-trivial and not easy to mock/spy, since it's all created on the fly. Considering this is supposed to be deprecated, I manually tested it by setting the default to true.
          Hide
          andrew.wang Andrew Wang added a comment -

          Thanks for working on this patch Xiao, and thanks Yongjun, Arun, and Wei-Chiu for weighing in. I had a few review comments:

          • Mildly prefer to keep the newline at the top of KMSClientProvider
          • Regarding moving the config key, I think the other Renewers get around this by embedding the static class within the parent class and accessing required state statically. I think the parent here would be KMSClientProvider. It wouldn't be good to tie renewal to this HDFS key anyway, since the KMS is used for more than just HDFS encryption.
          • KMSClientProvider#addDelegationToken and cancelDT just pass a dummy url to the authUrl call. Why does renewal in particularly need a URL with USER_NAME set? IIUC this is needed for PseudoAuthentication, but here we're doing DT authentication?
          • Extra newline in declaration of generateDT in KMSClientProvider
          • In the new test in TestKMS, can we configure the Kerberos config in testDTOKerberized, and then pass the Configuration to testDelegationTokensOps? I think that's cleaner.
          • Also recommend doubling the timeout Rule, since things often run slower on overloaded Jenkins servers and we don't want a new flake.
          Show
          andrew.wang Andrew Wang added a comment - Thanks for working on this patch Xiao, and thanks Yongjun, Arun, and Wei-Chiu for weighing in. I had a few review comments: Mildly prefer to keep the newline at the top of KMSClientProvider Regarding moving the config key, I think the other Renewers get around this by embedding the static class within the parent class and accessing required state statically. I think the parent here would be KMSClientProvider. It wouldn't be good to tie renewal to this HDFS key anyway, since the KMS is used for more than just HDFS encryption. KMSClientProvider#addDelegationToken and cancelDT just pass a dummy url to the authUrl call. Why does renewal in particularly need a URL with USER_NAME set? IIUC this is needed for PseudoAuthentication, but here we're doing DT authentication? Extra newline in declaration of generateDT in KMSClientProvider In the new test in TestKMS, can we configure the Kerberos config in testDTOKerberized, and then pass the Configuration to testDelegationTokensOps? I think that's cleaner. Also recommend doubling the timeout Rule, since things often run slower on overloaded Jenkins servers and we don't want a new flake.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 docker 3m 12s Docker failed to build yetus/hadoop:2c91fd8.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12806771/HADOOP-13155.04.patch
          JIRA Issue HADOOP-13155
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9611/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 0s Docker mode activated. -1 docker 3m 12s Docker failed to build yetus/hadoop:2c91fd8. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12806771/HADOOP-13155.04.patch JIRA Issue HADOOP-13155 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9611/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 -

          What about using hadoop.security.key.provider.path instead of dfs.encryption.key.provider.uri? Reading some documentation, it seems both values should be configured the same. This way you don't need hdfs configs.

          Show
          jojochuang Wei-Chiu Chuang added a comment - What about using hadoop.security.key.provider.path instead of dfs.encryption.key.provider.uri ? Reading some documentation, it seems both values should be configured the same. This way you don't need hdfs configs.
          Hide
          xiaochen Xiao Chen added a comment -

          Had an offline review with Yongjun Zhang, and patch 4 addressing his comments:

          • KMSTokenRenewer, use its own logger
          • Added more logs when KMSTokenRenewer found the keyProvider is not a DTExt instance
          • Regarding the template usage when creating delegation tokens:
            • The way of creating a new Token<AbstractDelegationTokenIdentifier> for DelegationTokenAuthenticatedURL$Token#setDelegationToken seems verbose.
              Since we're accepting a generic type, I think this is the safe way to go. Casting may end up throwing exceptions. I refactored KMSCP with a generateDelegationToken method to do this for both the renew and cancel.
            • Also, constructing the Token<ADTI> using the 4 parameters seems non-optimal
              However, I don't feel changing its copy constructor to accepting Token<?> is a good idea... IIUC the template class Token is supposed to only accept T. For this reason, I didn't change anything. Feel free to comment if you think otherwise.

          One thing Yongjun also brought up is the move of dfs.encryption.key.provider.uri from HdfsClientConfigKeys to CommonConfigurationKeys.

          • The reason of this move is that the renewer is in common (and kms), hence we need the util method to create provider in common, hence the need of reading that config from common.
          • I left the name dfs.xxx for compatibility, but it's a bit weird to have a dfs.* in common configurations. Not sure what's the best way of handling this.. Andrew Wang, do you have any advice on it? Thanks!
          Show
          xiaochen Xiao Chen added a comment - Had an offline review with Yongjun Zhang , and patch 4 addressing his comments: KMSTokenRenewer , use its own logger Added more logs when KMSTokenRenewer found the keyProvider is not a DTExt instance Regarding the template usage when creating delegation tokens: The way of creating a new Token<AbstractDelegationTokenIdentifier> for DelegationTokenAuthenticatedURL$Token#setDelegationToken seems verbose. Since we're accepting a generic type, I think this is the safe way to go. Casting may end up throwing exceptions. I refactored KMSCP with a generateDelegationToken method to do this for both the renew and cancel. Also, constructing the Token<ADTI> using the 4 parameters seems non-optimal However, I don't feel changing its copy constructor to accepting Token<?> is a good idea... IIUC the template class Token is supposed to only accept T . For this reason, I didn't change anything. Feel free to comment if you think otherwise. One thing Yongjun also brought up is the move of dfs.encryption.key.provider.uri from HdfsClientConfigKeys to CommonConfigurationKeys . The reason of this move is that the renewer is in common (and kms), hence we need the util method to create provider in common, hence the need of reading that config from common. I left the name dfs.xxx for compatibility, but it's a bit weird to have a dfs.* in common configurations. Not sure what's the best way of handling this.. Andrew Wang , do you have any advice on it? Thanks!
          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 appears to include 1 new or modified test files.
          0 mvndep 0m 32s Maven dependency ordering for branch
          +1 mvninstall 6m 57s trunk passed
          +1 compile 6m 55s trunk passed
          +1 checkstyle 1m 25s trunk passed
          +1 mvnsite 2m 50s trunk passed
          +1 mvneclipse 0m 46s trunk passed
          +1 findbugs 5m 3s trunk passed
          +1 javadoc 2m 36s trunk passed
          0 mvndep 0m 12s Maven dependency ordering for patch
          +1 mvninstall 2m 20s the patch passed
          +1 compile 6m 54s the patch passed
          +1 javac 6m 54s the patch passed
          -1 checkstyle 1m 25s root: The patch generated 3 new + 315 unchanged - 6 fixed = 318 total (was 321)
          +1 mvnsite 2m 42s the patch passed
          +1 mvneclipse 0m 48s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 5m 33s the patch passed
          +1 javadoc 2m 36s the patch passed
          -1 unit 8m 7s hadoop-common in the patch failed.
          +1 unit 1m 32s hadoop-kms in the patch passed.
          +1 unit 0m 56s hadoop-hdfs-client in the patch passed.
          -1 unit 62m 25s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          124m 13s



          Reason Tests
          Failed junit tests hadoop.net.TestDNS
            hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock
            hadoop.hdfs.web.TestWebHdfsTimeouts
            hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery
            hadoop.hdfs.TestAsyncDFSRename



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805935/HADOOP-13155.03.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux 0ba26c401938 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 / 57c31a3
          Default Java 1.8.0_91
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/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 appears to include 1 new or modified test files. 0 mvndep 0m 32s Maven dependency ordering for branch +1 mvninstall 6m 57s trunk passed +1 compile 6m 55s trunk passed +1 checkstyle 1m 25s trunk passed +1 mvnsite 2m 50s trunk passed +1 mvneclipse 0m 46s trunk passed +1 findbugs 5m 3s trunk passed +1 javadoc 2m 36s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 2m 20s the patch passed +1 compile 6m 54s the patch passed +1 javac 6m 54s the patch passed -1 checkstyle 1m 25s root: The patch generated 3 new + 315 unchanged - 6 fixed = 318 total (was 321) +1 mvnsite 2m 42s the patch passed +1 mvneclipse 0m 48s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 5m 33s the patch passed +1 javadoc 2m 36s the patch passed -1 unit 8m 7s hadoop-common in the patch failed. +1 unit 1m 32s hadoop-kms in the patch passed. +1 unit 0m 56s hadoop-hdfs-client in the patch passed. -1 unit 62m 25s hadoop-hdfs in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 124m 13s Reason Tests Failed junit tests hadoop.net.TestDNS   hadoop.hdfs.server.blockmanagement.TestPendingInvalidateBlock   hadoop.hdfs.web.TestWebHdfsTimeouts   hadoop.hdfs.server.datanode.fsdataset.impl.TestLazyPersistReplicaRecovery   hadoop.hdfs.TestAsyncDFSRename Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805935/HADOOP-13155.03.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux 0ba26c401938 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 / 57c31a3 Default Java 1.8.0_91 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9568/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 -

          Fixing the javac warning

          Show
          xiaochen Xiao Chen added a comment - Fixing the javac warning
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 10s 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.
          0 mvndep 0m 32s Maven dependency ordering for branch
          +1 mvninstall 6m 30s trunk passed
          +1 compile 6m 33s trunk passed
          +1 checkstyle 1m 24s trunk passed
          +1 mvnsite 2m 40s trunk passed
          +1 mvneclipse 0m 47s trunk passed
          +1 findbugs 4m 55s trunk passed
          +1 javadoc 2m 30s trunk passed
          0 mvndep 0m 12s Maven dependency ordering for patch
          +1 mvninstall 2m 16s the patch passed
          +1 compile 6m 34s the patch passed
          -1 javac 6m 34s root generated 1 new + 696 unchanged - 1 fixed = 697 total (was 697)
          -1 checkstyle 1m 23s root: The patch generated 3 new + 315 unchanged - 6 fixed = 318 total (was 321)
          +1 mvnsite 2m 39s the patch passed
          +1 mvneclipse 0m 46s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 xml 0m 1s The patch has no ill-formed XML file.
          +1 findbugs 5m 27s the patch passed
          +1 javadoc 2m 35s the patch passed
          +1 unit 8m 44s hadoop-common in the patch passed.
          +1 unit 1m 33s hadoop-kms in the patch passed.
          +1 unit 0m 55s hadoop-hdfs-client in the patch passed.
          -1 unit 60m 56s hadoop-hdfs in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          121m 25s



          Reason Tests
          Failed junit tests hadoop.hdfs.TestDFSClientRetries
            hadoop.hdfs.shortcircuit.TestShortCircuitCache



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805818/HADOOP-13155.02.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml
          uname Linux c48de7f763ae 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 / b4078bd
          Default Java 1.8.0_91
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/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 10s 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. 0 mvndep 0m 32s Maven dependency ordering for branch +1 mvninstall 6m 30s trunk passed +1 compile 6m 33s trunk passed +1 checkstyle 1m 24s trunk passed +1 mvnsite 2m 40s trunk passed +1 mvneclipse 0m 47s trunk passed +1 findbugs 4m 55s trunk passed +1 javadoc 2m 30s trunk passed 0 mvndep 0m 12s Maven dependency ordering for patch +1 mvninstall 2m 16s the patch passed +1 compile 6m 34s the patch passed -1 javac 6m 34s root generated 1 new + 696 unchanged - 1 fixed = 697 total (was 697) -1 checkstyle 1m 23s root: The patch generated 3 new + 315 unchanged - 6 fixed = 318 total (was 321) +1 mvnsite 2m 39s the patch passed +1 mvneclipse 0m 46s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 xml 0m 1s The patch has no ill-formed XML file. +1 findbugs 5m 27s the patch passed +1 javadoc 2m 35s the patch passed +1 unit 8m 44s hadoop-common in the patch passed. +1 unit 1m 33s hadoop-kms in the patch passed. +1 unit 0m 55s hadoop-hdfs-client in the patch passed. -1 unit 60m 56s hadoop-hdfs in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 121m 25s Reason Tests Failed junit tests hadoop.hdfs.TestDFSClientRetries   hadoop.hdfs.shortcircuit.TestShortCircuitCache Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805818/HADOOP-13155.02.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle xml uname Linux c48de7f763ae 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 / b4078bd Default Java 1.8.0_91 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/artifact/patchprocess/patch-unit-hadoop-hdfs-project_hadoop-hdfs.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9565/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 Wei-Chiu for the review comments.

          1. I think the API in KMS documentation are for the HTTP REST APIs, which is on org.apache.hadoop.crypto.key.kms.server.KMS, so no need to update.
          2. Good catch on the xml, added it to core-defaults.xml as well. The only intention was that KMSUtil cannot access HDFSConfigKeys, so I had to expose it into a header that common can use. I guess we have to keep it in both places...

          Patch 2 also fixes javac/checkstyle above. Please take a look and provide your feedback. Thanks!

          Show
          xiaochen Xiao Chen added a comment - Thanks Wei-Chiu for the review comments. I think the API in KMS documentation are for the HTTP REST APIs, which is on org.apache.hadoop.crypto.key.kms.server.KMS , so no need to update. Good catch on the xml, added it to core-defaults.xml as well. The only intention was that KMSUtil cannot access HDFSConfigKeys, so I had to expose it into a header that common can use. I guess we have to keep it in both places... Patch 2 also fixes javac/checkstyle above. Please take a look and provide your feedback. Thanks!
          Hide
          jojochuang Wei-Chiu Chuang added a comment -

          Hi Xiao Chen thanks for contributing this patch.

          1. Could you also update kms documentation to include these new APIs? It would be nice to have that, though I am ok without it.
          2. If you want to move the property dfs.encryption.key.provider.uri to CommonConfigurationKeys, I think you should also move this property from hdfs-default.xml to core-default.xml.
          Show
          jojochuang Wei-Chiu Chuang added a comment - Hi Xiao Chen thanks for contributing this patch. Could you also update kms documentation to include these new APIs? It would be nice to have that, though I am ok without it. If you want to move the property dfs.encryption.key.provider.uri to CommonConfigurationKeys, I think you should also move this property from hdfs-default.xml to core-default.xml.
          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 appears to include 1 new or modified test files.
          0 mvndep 0m 35s Maven dependency ordering for branch
          +1 mvninstall 6m 35s trunk passed
          +1 compile 6m 27s trunk passed
          +1 checkstyle 1m 23s trunk passed
          +1 mvnsite 2m 39s trunk passed
          +1 mvneclipse 0m 44s trunk passed
          +1 findbugs 4m 50s trunk passed
          +1 javadoc 2m 31s trunk passed
          0 mvndep 0m 11s Maven dependency ordering for patch
          +1 mvninstall 2m 14s the patch passed
          +1 compile 6m 20s the patch passed
          -1 javac 6m 20s root generated 1 new + 696 unchanged - 1 fixed = 697 total (was 697)
          -1 checkstyle 1m 23s root: The patch generated 16 new + 315 unchanged - 5 fixed = 331 total (was 320)
          +1 mvnsite 2m 39s the patch passed
          +1 mvneclipse 0m 45s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 5m 26s the patch passed
          +1 javadoc 2m 29s the patch passed
          -1 unit 6m 55s hadoop-common in the patch failed.
          +1 unit 1m 30s hadoop-kms in the patch passed.
          +1 unit 0m 54s hadoop-hdfs-client in the patch passed.
          +1 unit 58m 25s hadoop-hdfs in the patch passed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          116m 33s



          Reason Tests
          Failed junit tests hadoop.ha.TestZKFailoverController



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:2c91fd8
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805160/HADOOP-13155.01.patch
          JIRA Issue HADOOP-13155
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux ffc261e5d524 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 / 9325845
          Default Java 1.8.0_91
          findbugs v3.0.0
          javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/diff-compile-javac-root.txt
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/diff-checkstyle-root.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/testReport/
          modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: .
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/console
          Powered by Apache Yetus 0.3.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 appears to include 1 new or modified test files. 0 mvndep 0m 35s Maven dependency ordering for branch +1 mvninstall 6m 35s trunk passed +1 compile 6m 27s trunk passed +1 checkstyle 1m 23s trunk passed +1 mvnsite 2m 39s trunk passed +1 mvneclipse 0m 44s trunk passed +1 findbugs 4m 50s trunk passed +1 javadoc 2m 31s trunk passed 0 mvndep 0m 11s Maven dependency ordering for patch +1 mvninstall 2m 14s the patch passed +1 compile 6m 20s the patch passed -1 javac 6m 20s root generated 1 new + 696 unchanged - 1 fixed = 697 total (was 697) -1 checkstyle 1m 23s root: The patch generated 16 new + 315 unchanged - 5 fixed = 331 total (was 320) +1 mvnsite 2m 39s the patch passed +1 mvneclipse 0m 45s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 5m 26s the patch passed +1 javadoc 2m 29s the patch passed -1 unit 6m 55s hadoop-common in the patch failed. +1 unit 1m 30s hadoop-kms in the patch passed. +1 unit 0m 54s hadoop-hdfs-client in the patch passed. +1 unit 58m 25s hadoop-hdfs in the patch passed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 116m 33s Reason Tests Failed junit tests hadoop.ha.TestZKFailoverController Subsystem Report/Notes Docker Image:yetus/hadoop:2c91fd8 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12805160/HADOOP-13155.01.patch JIRA Issue HADOOP-13155 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux ffc261e5d524 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 / 9325845 Default Java 1.8.0_91 findbugs v3.0.0 javac https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/diff-compile-javac-root.txt checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/diff-checkstyle-root.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt unit test logs https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/testReport/ modules C: hadoop-common-project/hadoop-common hadoop-common-project/hadoop-kms hadoop-hdfs-project/hadoop-hdfs-client hadoop-hdfs-project/hadoop-hdfs U: . Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9532/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thank you Arun Suresh for the helpful comment! I really appreciate your time.

          Patch 1 attached is a more mature version and passes tests for kerberos + simple. I moved delegation token operations to a separate test case. I will verify that in a cluster with yarn as well.

          No modification is done to HttpFs, since I don't think HttpFS has it's own kind. It extends FileSystem and implements renew/cancel there.

          Show
          xiaochen Xiao Chen added a comment - Thank you Arun Suresh for the helpful comment! I really appreciate your time. Patch 1 attached is a more mature version and passes tests for kerberos + simple. I moved delegation token operations to a separate test case. I will verify that in a cluster with yarn as well. No modification is done to HttpFs, since I don't think HttpFS has it's own kind. It extends FileSystem and implements renew/cancel there.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



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



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804893/HADOOP-13155.pre.patch
          JIRA Issue HADOOP-13155
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9521/console
          Powered by Apache Yetus 0.3.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 4s HADOOP-13155 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12804893/HADOOP-13155.pre.patch JIRA Issue HADOOP-13155 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/9521/console Powered by Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          asuresh Arun Suresh added a comment -

          Thanks for taking this up Xiao Chen..

          IIRC, the reason why we had not implemented the renewer for KMS was that we were catering to FileSystem and DistributedFileSystem API which at that point had only an addDelegationTokens().

          So, I guess it is a good idea to add a Renewer for KMS-DTs. Will go over you patch in more detail over the weekend.

          Since looks like MiniKMS is setup with PseudoAuthenticationHandler by default (didn't find a way to configure auth type before starting MiniKMS.

          I remember testing KMS via TestKMS using the KerberosDelegationTokenAuthenticationHandler. Take a look at TestKMS::testStartStop method where we do a conf.set("hadoop.kms.authentication.type", "kerberos") before running the KMS server.

          Show
          asuresh Arun Suresh added a comment - Thanks for taking this up Xiao Chen .. IIRC, the reason why we had not implemented the renewer for KMS was that we were catering to FileSystem and DistributedFileSystem API which at that point had only an addDelegationTokens() . So, I guess it is a good idea to add a Renewer for KMS-DTs. Will go over you patch in more detail over the weekend. Since looks like MiniKMS is setup with PseudoAuthenticationHandler by default (didn't find a way to configure auth type before starting MiniKMS. I remember testing KMS via TestKMS using the KerberosDelegationTokenAuthenticationHandler . Take a look at TestKMS::testStartStop method where we do a conf.set("hadoop.kms.authentication.type", "kerberos") before running the KMS server.
          Hide
          xiaochen Xiao Chen added a comment -

          Attaching a preliminary patch to express the idea.
          Arun Suresh and Alejandro Abdelnur, could you please take a look and provide your valuable feedback? I just want to make sure I'm on the right path... Thanks in advance!

          The patch does:

          • Added a new TokenRenewer implementation that handles kms-dt.
          • Moved some necessary utils from hdfs to common.
          • The test mimics what yarn's DelegationTokenRenewer does - directly calling renew/cancel on the token.
          • Currently I've only made it work with SIMPLE authentication. Since looks like MiniKMS is setup with PseudoAuthenticationHandler by default (didn't find a way to configure auth type before starting MiniKMS.

          Will do:

          • I imagine more work is needed in KMSCP to correctly create the authURL.
          • Plan to manual test using a MR job to verify end-to-end.
          Show
          xiaochen Xiao Chen added a comment - Attaching a preliminary patch to express the idea. Arun Suresh and Alejandro Abdelnur , could you please take a look and provide your valuable feedback? I just want to make sure I'm on the right path... Thanks in advance! The patch does: Added a new TokenRenewer implementation that handles kms-dt. Moved some necessary utils from hdfs to common. The test mimics what yarn's DelegationTokenRenewer does - directly calling renew/cancel on the token. Currently I've only made it work with SIMPLE authentication. Since looks like MiniKMS is setup with PseudoAuthenticationHandler by default (didn't find a way to configure auth type before starting MiniKMS. Will do: I imagine more work is needed in KMSCP to correctly create the authURL. Plan to manual test using a MR job to verify end-to-end.
          Hide
          xiaochen Xiao Chen added a comment -

          Following a quick chat and advice from Yongjun Zhang, here's more details on how other tokens are handled:

          • DFSClient$Renewer handles HDFS_DELEGATION_TOKEN
          • TokenAspect$TokenManager handles HFTP delegation, HSFTP delegation, WEBHDFS delegation, SWEBHDFS delegation. (No httpfs, but didn't find httpfs-specific kind either)
          • There are other similar classes in yarn/mr to handle corresponding tokens
          • As mentioned above, no renewer class handles kms-dt
          Show
          xiaochen Xiao Chen added a comment - Following a quick chat and advice from Yongjun Zhang , here's more details on how other tokens are handled: DFSClient$Renewer handles HDFS_DELEGATION_TOKEN TokenAspect$TokenManager handles HFTP delegation , HSFTP delegation , WEBHDFS delegation , SWEBHDFS delegation . (No httpfs, but didn't find httpfs-specific kind either) There are other similar classes in yarn/mr to handle corresponding tokens As mentioned above, no renewer class handles kms-dt
          Hide
          xiaochen Xiao Chen added a comment -

          Hm, seems I can't edit the description. Sorry for message flooding, but here's more details:

          I found this problem when debugging an issue with KMS DT renewal.

          Token's code snippet to get renewer is like this:

            private static ServiceLoader<TokenRenewer> renewers =
                ServiceLoader.load(TokenRenewer.class);
          
            private synchronized TokenRenewer getRenewer() throws IOException {
              if (renewer != null) {
                return renewer;
              }
              renewer = TRIVIAL_RENEWER;
              synchronized (renewers) {
                for (TokenRenewer canidate : renewers) {
                  if (canidate.handleKind(this.kind)) {
                    renewer = canidate;
                    return renewer;
                  }
                }
              }
              LOG.warn("No TokenRenewer defined for token kind " + this.kind);
              return renewer;
          

          And META-INF/services/org.apache.hadoop.security.token.TokenRenewer defines each implementation. I didn't find any TokenRenewer implementation that handles kind kms-dt. I wanted to look at HttpFSFileSystem for a reference since they reuse the same auth codes in hadoop-common, but found out there's none.

          (There's no TokenKind in HttpFS either, so may be that's not needed...) The main intention of this jira is to allow KMS DTs to be able to get renewed.

          Show
          xiaochen Xiao Chen added a comment - Hm, seems I can't edit the description. Sorry for message flooding, but here's more details: I found this problem when debugging an issue with KMS DT renewal. Token's code snippet to get renewer is like this: private static ServiceLoader<TokenRenewer> renewers = ServiceLoader.load(TokenRenewer.class); private synchronized TokenRenewer getRenewer() throws IOException { if (renewer != null ) { return renewer; } renewer = TRIVIAL_RENEWER; synchronized (renewers) { for (TokenRenewer canidate : renewers) { if (canidate.handleKind( this .kind)) { renewer = canidate; return renewer; } } } LOG.warn( "No TokenRenewer defined for token kind " + this .kind); return renewer; And META-INF/services/org.apache.hadoop.security.token.TokenRenewer defines each implementation. I didn't find any TokenRenewer implementation that handles kind kms-dt . I wanted to look at HttpFSFileSystem for a reference since they reuse the same auth codes in hadoop-common, but found out there's none. (There's no TokenKind in HttpFS either, so may be that's not needed...) The main intention of this jira is to allow KMS DTs to be able to get renewed.
          Hide
          xiaochen Xiao Chen added a comment -

          Ping Alejandro Abdelnur and Arun Suresh for advice. Am I correct that we're missing the hook?

          I see AuthenticationFilter#doFilter can handle DT operation requests using its authHandler. So I imagine we can just add renewer implementations to call DelegationTokenAuthenticatedURL#renewDelegationToken.

          Show
          xiaochen Xiao Chen added a comment - Ping Alejandro Abdelnur and Arun Suresh for advice. Am I correct that we're missing the hook? I see AuthenticationFilter#doFilter can handle DT operation requests using its authHandler . So I imagine we can just add renewer implementations to call DelegationTokenAuthenticatedURL#renewDelegationToken .

            People

            • Assignee:
              xiaochen Xiao Chen
              Reporter:
              xiaochen Xiao Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development