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

Remove TRUSTSTORE_PASSWORD related scripts from KMS

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.0-alpha2
    • Fix Version/s: 3.0.0-alpha2
    • Component/s: kms
    • Labels:
      None

      Description

      Now that HADOOP-13864 is fixed, it's unnecessary to set the truststore password. Let's remove it from kms.sh.

      1. HADOOP-13911.001.patch
        3 kB
        John Zhuge
      2. HADOOP-13911.002.patch
        4 kB
        John Zhuge
      3. HADOOP-13911.003.patch
        4 kB
        John Zhuge

        Issue Links

          Activity

          Hide
          jzhuge John Zhuge added a comment -

          Patch 001:

          • Remove truststore password related stuff from kms-env.sh, kms.sh, and ssl-server.xml.conf
          Show
          jzhuge John Zhuge added a comment - Patch 001: Remove truststore password related stuff from kms-env.sh, kms.sh, and ssl-server.xml.conf
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 12m 48s trunk passed
          +1 compile 9m 42s trunk passed
          +1 mvnsite 0m 27s trunk passed
          +1 mvneclipse 0m 22s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 17s the patch passed
          +1 compile 9m 14s the patch passed
          +1 javac 9m 14s the patch passed
          +1 mvnsite 0m 28s the patch passed
          +1 mvneclipse 0m 23s the patch passed
          +1 shellcheck 0m 16s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576)
          +1 shelldocs 0m 25s There were no new shelldocs issues.
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 javadoc 0m 22s the patch passed
          +1 unit 2m 16s hadoop-kms in the patch passed.
          +1 asflicense 0m 36s The patch does not generate ASF License warnings.
          39m 37s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13911
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12843824/HADOOP-13911.001.patch
          Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall
          uname Linux 483ad4a8869f 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / ef2dd7b
          Default Java 1.8.0_111
          shellcheck v0.4.5
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11299/testReport/
          modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11299/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 12m 48s trunk passed +1 compile 9m 42s trunk passed +1 mvnsite 0m 27s trunk passed +1 mvneclipse 0m 22s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 17s the patch passed +1 compile 9m 14s the patch passed +1 javac 9m 14s the patch passed +1 mvnsite 0m 28s the patch passed +1 mvneclipse 0m 23s the patch passed +1 shellcheck 0m 16s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576) +1 shelldocs 0m 25s There were no new shelldocs issues. +1 whitespace 0m 0s The patch has no whitespace issues. +1 javadoc 0m 22s the patch passed +1 unit 2m 16s hadoop-kms in the patch passed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 39m 37s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13911 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12843824/HADOOP-13911.001.patch Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall uname Linux 483ad4a8869f 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ef2dd7b Default Java 1.8.0_111 shellcheck v0.4.5 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11299/testReport/ modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11299/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John Zhuge for working on this. Could you remove the reference (and comments) in kms-config.sh too? +1 pending.

          Show
          xiaochen Xiao Chen added a comment - Thanks John Zhuge for working on this. Could you remove the reference (and comments) in kms-config.sh too? +1 pending.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 002:

          • Update kms-config.sh

          Tested kms.sh in both non-ssl and ssl mode.

          Xiao Chen, should we do the same for HttpFS as well?

          Show
          jzhuge John Zhuge added a comment - Patch 002: Update kms-config.sh Tested kms.sh in both non-ssl and ssl mode. Xiao Chen , should we do the same for HttpFS as well?
          Hide
          xiaochen Xiao Chen added a comment -

          Thanks John Zhuge, +1 will commit on Tuesday.

          should we do the same for HttpFS as well?

          I think so, we can do the HADOOP-13864 and HADOOP-13911 equivalents in the same HDFS jira if you'd like to work on.

          Show
          xiaochen Xiao Chen added a comment - Thanks John Zhuge , +1 will commit on Tuesday. should we do the same for HttpFS as well? I think so, we can do the HADOOP-13864 and HADOOP-13911 equivalents in the same HDFS jira if you'd like to work on.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 14m 21s trunk passed
          +1 compile 11m 12s trunk passed
          +1 mvnsite 0m 29s trunk passed
          +1 mvneclipse 0m 23s trunk passed
          +1 javadoc 0m 23s trunk passed
          +1 mvninstall 0m 18s the patch passed
          +1 compile 10m 1s the patch passed
          +1 javac 10m 1s the patch passed
          +1 mvnsite 0m 27s the patch passed
          +1 mvneclipse 0m 23s the patch passed
          +1 shellcheck 0m 15s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576)
          +1 shelldocs 0m 25s There were no new shelldocs issues.
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 javadoc 0m 22s the patch passed
          +1 unit 2m 18s hadoop-kms in the patch passed.
          +1 asflicense 0m 36s The patch does not generate ASF License warnings.
          43m 33s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13911
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12843936/HADOOP-13911.002.patch
          Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall
          uname Linux e58a56304e62 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / ef2dd7b
          Default Java 1.8.0_111
          shellcheck v0.4.5
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11300/testReport/
          modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11300/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 14m 21s trunk passed +1 compile 11m 12s trunk passed +1 mvnsite 0m 29s trunk passed +1 mvneclipse 0m 23s trunk passed +1 javadoc 0m 23s trunk passed +1 mvninstall 0m 18s the patch passed +1 compile 10m 1s the patch passed +1 javac 10m 1s the patch passed +1 mvnsite 0m 27s the patch passed +1 mvneclipse 0m 23s the patch passed +1 shellcheck 0m 15s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576) +1 shelldocs 0m 25s There were no new shelldocs issues. +1 whitespace 0m 0s The patch has no whitespace issues. +1 javadoc 0m 22s the patch passed +1 unit 2m 18s hadoop-kms in the patch passed. +1 asflicense 0m 36s The patch does not generate ASF License warnings. 43m 33s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13911 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12843936/HADOOP-13911.002.patch Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall uname Linux e58a56304e62 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / ef2dd7b Default Java 1.8.0_111 shellcheck v0.4.5 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11300/testReport/ modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11300/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment -

          Sorry, having my final pass before committing. This in kms.sh looks wrong.

          -# Mask the trustStorePassword
          -# shellcheck disable=SC2086
          -CATALINA_OPTS_DISP="$(echo ${CATALINA_OPTS} | sed -e 's/trustStorePassword=[^ ]*/trustStorePassword=***/')"
          -
           hadoop_debug "Using   CATALINA_OPTS:       ${CATALINA_OPTS_DISP}"
          

          We should just remove the mask logic, but print the CATALINA_OPTS.

          Show
          xiaochen Xiao Chen added a comment - Sorry, having my final pass before committing. This in kms.sh looks wrong. -# Mask the trustStorePassword -# shellcheck disable=SC2086 -CATALINA_OPTS_DISP= "$(echo ${CATALINA_OPTS} | sed -e 's/trustStorePassword=[^ ]*/trustStorePassword=***/')" - hadoop_debug "Using CATALINA_OPTS: ${CATALINA_OPTS_DISP}" We should just remove the mask logic, but print the CATALINA_OPTS.
          Hide
          jzhuge John Zhuge added a comment -

          My bad, thanks for catching it, Xiao Chen.

          Show
          jzhuge John Zhuge added a comment - My bad, thanks for catching it, Xiao Chen .
          Hide
          jzhuge John Zhuge added a comment -

          The following line in kms.sh is redundant:

          hadoop_debug "Using   CATALINA_OPTS:       ${CATALINA_OPTS_DISP}"
          
          $ CATALINA_OPTS='-config abc.xml' sbin/kms.sh --debug run
          ...
          DEBUG: Setting CATALINA_OPTS to -config abc.xml
          DEBUG: Using   CATALINA_OPTS:       -config abc.xml
          

          kms-config.sh prints the line Setting CATALINA_OPTS to ....

          Show
          jzhuge John Zhuge added a comment - The following line in kms.sh is redundant: hadoop_debug "Using CATALINA_OPTS: ${CATALINA_OPTS_DISP}" $ CATALINA_OPTS='-config abc.xml' sbin/kms.sh --debug run ... DEBUG: Setting CATALINA_OPTS to -config abc.xml DEBUG: Using CATALINA_OPTS: -config abc.xml kms-config.sh prints the line Setting CATALINA_OPTS to ... .
          Hide
          xiaochen Xiao Chen added a comment -

          I think Setting XXX is per var, and Using XXX is specific to the final CATALINA_OPTS used. Feels like we can keep leave this untouched for the removal patch, and just discard the CATALINA_OPTS_DISP mask.

          Show
          xiaochen Xiao Chen added a comment - I think Setting XXX is per var, and Using XXX is specific to the final CATALINA_OPTS used. Feels like we can keep leave this untouched for the removal patch, and just discard the CATALINA_OPTS_DISP mask.
          Hide
          jzhuge John Zhuge added a comment -

          Patch 003:

          • Xiao's comment
          Show
          jzhuge John Zhuge added a comment - Patch 003: Xiao's comment
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 15s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
          +1 mvninstall 12m 37s trunk passed
          +1 compile 10m 5s trunk passed
          +1 mvnsite 0m 29s trunk passed
          +1 mvneclipse 0m 23s trunk passed
          +1 javadoc 0m 22s trunk passed
          +1 mvninstall 0m 18s the patch passed
          +1 compile 10m 39s the patch passed
          +1 javac 10m 39s the patch passed
          +1 mvnsite 0m 26s the patch passed
          +1 mvneclipse 0m 21s the patch passed
          +1 shellcheck 0m 15s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576)
          +1 shelldocs 0m 24s There were no new shelldocs issues.
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 javadoc 0m 20s the patch passed
          +1 unit 2m 18s hadoop-kms in the patch passed.
          +1 asflicense 0m 33s The patch does not generate ASF License warnings.
          41m 7s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13911
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12844146/HADOOP-13911.003.patch
          Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall
          uname Linux c568c2acb572 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / f678080
          Default Java 1.8.0_111
          shellcheck v0.4.5
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11305/testReport/
          modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11305/console
          Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 15s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. -1 test4tests 0m 0s The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 mvninstall 12m 37s trunk passed +1 compile 10m 5s trunk passed +1 mvnsite 0m 29s trunk passed +1 mvneclipse 0m 23s trunk passed +1 javadoc 0m 22s trunk passed +1 mvninstall 0m 18s the patch passed +1 compile 10m 39s the patch passed +1 javac 10m 39s the patch passed +1 mvnsite 0m 26s the patch passed +1 mvneclipse 0m 21s the patch passed +1 shellcheck 0m 15s The patch generated 0 new + 575 unchanged - 1 fixed = 575 total (was 576) +1 shelldocs 0m 24s There were no new shelldocs issues. +1 whitespace 0m 0s The patch has no whitespace issues. +1 javadoc 0m 20s the patch passed +1 unit 2m 18s hadoop-kms in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 41m 7s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13911 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12844146/HADOOP-13911.003.patch Optional Tests asflicense mvnsite unit shellcheck shelldocs compile javac javadoc mvninstall uname Linux c568c2acb572 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / f678080 Default Java 1.8.0_111 shellcheck v0.4.5 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11305/testReport/ modules C: hadoop-common-project/hadoop-kms U: hadoop-common-project/hadoop-kms Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11305/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          xiaochen Xiao Chen added a comment - - edited

          +1 on patch 3, committing this.
          No-test is fine IMO, since we're just removing an var no longer used. Current tests passing should be sufficient.

          Show
          xiaochen Xiao Chen added a comment - - edited +1 on patch 3, committing this. No-test is fine IMO, since we're just removing an var no longer used. Current tests passing should be sufficient.
          Hide
          xiaochen Xiao Chen added a comment -

          Committed to trunk, thanks for the contribution John!

          Show
          xiaochen Xiao Chen added a comment - Committed to trunk, thanks for the contribution John!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11023 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11023/)
          HADOOP-13911. Remove TRUSTSTORE_PASSWORD related scripts from KMS. (xiao: rev 30f85d7a88a110637757cf7a1f4cdc9ed40f59fb)

          • (edit) hadoop-common-project/hadoop-kms/src/main/tomcat/ssl-server.xml.conf
          • (edit) hadoop-common-project/hadoop-kms/src/main/sbin/kms.sh
          • (edit) hadoop-common-project/hadoop-kms/src/main/libexec/kms-config.sh
          • (edit) hadoop-common-project/hadoop-kms/src/main/conf/kms-env.sh
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11023 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11023/ ) HADOOP-13911 . Remove TRUSTSTORE_PASSWORD related scripts from KMS. (xiao: rev 30f85d7a88a110637757cf7a1f4cdc9ed40f59fb) (edit) hadoop-common-project/hadoop-kms/src/main/tomcat/ssl-server.xml.conf (edit) hadoop-common-project/hadoop-kms/src/main/sbin/kms.sh (edit) hadoop-common-project/hadoop-kms/src/main/libexec/kms-config.sh (edit) hadoop-common-project/hadoop-kms/src/main/conf/kms-env.sh
          Hide
          jzhuge John Zhuge added a comment -

          Thanks Xiao Chen for reporting, review, and commit!

          Show
          jzhuge John Zhuge added a comment - Thanks Xiao Chen for reporting, review, and commit!

            People

            • Assignee:
              jzhuge John Zhuge
              Reporter:
              xiaochen Xiao Chen
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development