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

RolloverSignerSecretProvider.LOG should be @VisibleForTesting

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.8.0
    • Fix Version/s: 2.9.0, 3.0.0-alpha4
    • Component/s: security
    • Labels:
    • Hadoop Flags:
      Reviewed
    1. HADOOP-14310.002.patch
      2 kB
      Arun Shanmugam Kumar
    2. HADOOP-14310.001.patch
      2 kB
      Arun Shanmugam Kumar

      Activity

      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Hi, I am a newbie. I can take this issue. Can someone assign this to me pls.

      Thanks,
      Arun

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Hi, I am a newbie. I can take this issue. Can someone assign this to me pls. Thanks, Arun
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Daniel Templeton Shouldn't we make the LOG variable private and static instead of making it "@VisibleForTesting" ? Please let me know why it should be "@VisibleForTesting".

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton Shouldn't we make the LOG variable private and static instead of making it "@VisibleForTesting" ? Please let me know why it should be "@VisibleForTesting".
      Hide
      templedf Daniel Templeton added a comment -

      Typically it should be private, but this class' logger is used in TestZKSignerSecretProvider and TestRandomSignerSecretProvider.

      Show
      templedf Daniel Templeton added a comment - Typically it should be private, but this class' logger is used in TestZKSignerSecretProvider and TestRandomSignerSecretProvider .
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Daniel Templeton Attaching the patch. Please provide code review for the same.

      Note: I have added explicit guava dependency in the pom.xml. I did a dependency:analyze to verify it is clean. I did found few warnings from it unrelated to my patch. Let me know if you like me to log a separate JIRA for that.

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton Attaching the patch. Please provide code review for the same. Note: I have added explicit guava dependency in the pom.xml. I did a dependency:analyze to verify it is clean. I did found few warnings from it unrelated to my patch. Let me know if you like me to log a separate JIRA for that.
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Daniel Templeton any luck reviewing my patch pls ?

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton any luck reviewing my patch pls ?
      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 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 13m 25s trunk passed
      +1 compile 16m 52s trunk passed
      +1 checkstyle 0m 19s trunk passed
      +1 mvnsite 0m 26s trunk passed
      +1 mvneclipse 0m 21s trunk passed
      -1 findbugs 0m 29s hadoop-common-project/hadoop-auth in trunk has 1 extant Findbugs warnings.
      +1 javadoc 0m 19s trunk passed
      +1 mvninstall 0m 16s the patch passed
      +1 compile 14m 37s the patch passed
      +1 javac 14m 37s the patch passed
      +1 checkstyle 0m 19s hadoop-common-project/hadoop-auth: The patch generated 0 new + 2 unchanged - 1 fixed = 2 total (was 3)
      +1 mvnsite 0m 25s the patch passed
      +1 mvneclipse 0m 21s 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 0m 41s the patch passed
      +1 javadoc 0m 18s the patch passed
      +1 unit 2m 53s hadoop-auth in the patch passed.
      +1 asflicense 0m 32s The patch does not generate ASF License warnings.
      54m 49s



      Subsystem Report/Notes
      Docker Image:yetus/hadoop:14b5c93
      JIRA Issue HADOOP-14310
      JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864885/HADOOP-14310.001.patch
      Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
      uname Linux de8313199473 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
      Build tool maven
      Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
      git revision trunk / d9014bd
      Default Java 1.8.0_131
      findbugs v3.1.0-RC1
      findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-auth-warnings.html
      Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/testReport/
      modules C: hadoop-common-project/hadoop-auth U: hadoop-common-project/hadoop-auth
      Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/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 13s 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 13m 25s trunk passed +1 compile 16m 52s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 26s trunk passed +1 mvneclipse 0m 21s trunk passed -1 findbugs 0m 29s hadoop-common-project/hadoop-auth in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 16s the patch passed +1 compile 14m 37s the patch passed +1 javac 14m 37s the patch passed +1 checkstyle 0m 19s hadoop-common-project/hadoop-auth: The patch generated 0 new + 2 unchanged - 1 fixed = 2 total (was 3) +1 mvnsite 0m 25s the patch passed +1 mvneclipse 0m 21s 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 0m 41s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 2m 53s hadoop-auth in the patch passed. +1 asflicense 0m 32s The patch does not generate ASF License warnings. 54m 49s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HADOOP-14310 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12864885/HADOOP-14310.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux de8313199473 3.13.0-116-generic #163-Ubuntu SMP Fri Mar 31 14:13:22 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / d9014bd Default Java 1.8.0_131 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-auth-warnings.html Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/testReport/ modules C: hadoop-common-project/hadoop-auth U: hadoop-common-project/hadoop-auth Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12231/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Daniel Templeton Thanks. Looks like findbugs warning is not due to my patch. I can open a separate jira to clean those up.

      Regards,
      Arun

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton Thanks. Looks like findbugs warning is not due to my patch. I can open a separate jira to clean those up. Regards, Arun
      Hide
      templedf Daniel Templeton added a comment -

      Sorry for the slow response. The patch looks good, but please leave the java imports before the org imports. That's the preferred order.

      Show
      templedf Daniel Templeton added a comment - Sorry for the slow response. The patch looks good, but please leave the java imports before the org imports. That's the preferred order.
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -

      Daniel Templeton Attaching the latest cleaned up patch as per your suggestion.

      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton Attaching the latest cleaned up patch as per your suggestion.
      Hide
      hadoopqa Hadoop QA added a comment -
      -1 overall



      Vote Subsystem Runtime Comment
      0 reexec 0m 23s 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 52s trunk passed
      +1 compile 18m 44s trunk passed
      +1 checkstyle 0m 19s trunk passed
      +1 mvnsite 0m 25s trunk passed
      +1 mvneclipse 0m 23s trunk passed
      -1 findbugs 0m 33s hadoop-common-project/hadoop-auth in trunk has 1 extant Findbugs warnings.
      +1 javadoc 0m 19s trunk passed
      +1 mvninstall 0m 15s the patch passed
      +1 compile 13m 50s the patch passed
      +1 javac 13m 50s the patch passed
      +1 checkstyle 0m 17s hadoop-common-project/hadoop-auth: The patch generated 0 new + 2 unchanged - 1 fixed = 2 total (was 3)
      +1 mvnsite 0m 24s the patch passed
      +1 mvneclipse 0m 20s 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 0m 41s the patch passed
      +1 javadoc 0m 18s the patch passed
      +1 unit 2m 50s hadoop-auth in the patch passed.
      +1 asflicense 0m 33s The patch does not generate ASF License warnings.
      57m 27s



      Subsystem Report/Notes
      Docker Image:yetus/hadoop:14b5c93
      JIRA Issue HADOOP-14310
      JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867964/HADOOP-14310.002.patch
      Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle
      uname Linux 656c620de6f3 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
      Build tool maven
      Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
      git revision trunk / 6600abb
      Default Java 1.8.0_121
      findbugs v3.1.0-RC1
      findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-auth-warnings.html
      Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/testReport/
      modules C: hadoop-common-project/hadoop-auth U: hadoop-common-project/hadoop-auth
      Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/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 23s 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 52s trunk passed +1 compile 18m 44s trunk passed +1 checkstyle 0m 19s trunk passed +1 mvnsite 0m 25s trunk passed +1 mvneclipse 0m 23s trunk passed -1 findbugs 0m 33s hadoop-common-project/hadoop-auth in trunk has 1 extant Findbugs warnings. +1 javadoc 0m 19s trunk passed +1 mvninstall 0m 15s the patch passed +1 compile 13m 50s the patch passed +1 javac 13m 50s the patch passed +1 checkstyle 0m 17s hadoop-common-project/hadoop-auth: The patch generated 0 new + 2 unchanged - 1 fixed = 2 total (was 3) +1 mvnsite 0m 24s the patch passed +1 mvneclipse 0m 20s 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 0m 41s the patch passed +1 javadoc 0m 18s the patch passed +1 unit 2m 50s hadoop-auth in the patch passed. +1 asflicense 0m 33s The patch does not generate ASF License warnings. 57m 27s Subsystem Report/Notes Docker Image:yetus/hadoop:14b5c93 JIRA Issue HADOOP-14310 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12867964/HADOOP-14310.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit xml findbugs checkstyle uname Linux 656c620de6f3 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 6600abb Default Java 1.8.0_121 findbugs v3.1.0-RC1 findbugs https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/artifact/patchprocess/branch-findbugs-hadoop-common-project_hadoop-auth-warnings.html Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/testReport/ modules C: hadoop-common-project/hadoop-auth U: hadoop-common-project/hadoop-auth Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/12308/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
      Hide
      beyonddream Arun Shanmugam Kumar added a comment -
      Show
      beyonddream Arun Shanmugam Kumar added a comment - Daniel Templeton ping
      Hide
      templedf Daniel Templeton added a comment -

      LGTM +1

      Show
      templedf Daniel Templeton added a comment - LGTM +1
      Hide
      templedf Daniel Templeton added a comment -

      Thanks for the patch, Arun Shanmugam Kumar. Committed to trunk and branch-2.

      Show
      templedf Daniel Templeton added a comment - Thanks for the patch, Arun Shanmugam Kumar . Committed to trunk and branch-2.
      Hide
      hudson Hudson added a comment -

      SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11857 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11857/)
      HADOOP-14310. RolloverSignerSecretProvider.LOG should be (templedf: rev 86368cc7667adfe224df77b039c2ffe8de7f889a)

      • (edit) hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java
      • (edit) hadoop-common-project/hadoop-auth/pom.xml
      Show
      hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11857 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11857/ ) HADOOP-14310 . RolloverSignerSecretProvider.LOG should be (templedf: rev 86368cc7667adfe224df77b039c2ffe8de7f889a) (edit) hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/util/RolloverSignerSecretProvider.java (edit) hadoop-common-project/hadoop-auth/pom.xml

        People

        • Assignee:
          beyonddream Arun Shanmugam Kumar
          Reporter:
          templedf Daniel Templeton
        • Votes:
          0 Vote for this issue
          Watchers:
          3 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development