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

Follow-on improvements to hadoop credential commands

VotersWatch issueWatchersCreate sub-taskLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Resolved
    • Major
    • Resolution: Fixed
    • 2.8.0
    • 2.8.0, 3.0.0-alpha1
    • security
    • None

    Description

      Andrew Wang had some follow-up code review comments from HADOOP-12942. Hence this issue.

      Ping Larry McCay as well.

      The comments:

      Overall this looks okay, the only correctness question I have is about the difference in behavior when the pwfile doesn't exist.

      The rest are all nits, would be nice to do these cleanups though.

      File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/JavaKeyStoreProvider.java:

      Line 147:
      Could this be a static helper?

      Line 161: new
      The javadoc says it returns null in this situation. This is also a difference from the implementation in the AbstractJKSP. Intentional?

      Line 175: private void locateKeystore() throws IOException {
      static helper? for the construct*Path methods too?

      File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java:

      Line 50: @VisibleForTesting public static final String NO_VALID_PROVIDERS =
      FYI for the future, our coding style is to put annotations on their own separate line.

      File hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/AbstractJavaKeyStoreProvider.java:

      Line 326: private char[] locatePassword() throws IOException {
      this method looks very similar to the one in JavaKeyStoreProvider, except the env var it looks for is different, is there potential for code reuse?

      Line 394: " o In the environment variable " +
      Using a "*" is the usual way of doing a bullet point, e.g. markdown and wiki syntax.

      Line 399: "http://hadoop.apache.org/docs/current/hadoop-project-dist/" +
      This link is not tied to a version, so could be inaccurate.

      Attachments

        1. HADOOP-13157.001.patch
          21 kB
          Mike Yoder
        2. HADOOP-13157.002.patch
          21 kB
          Mike Yoder
        3. HADOOP-13157.003.branch-2.8.patch
          23 kB
          Mike Yoder
        4. HADOOP-13157.004.branch-2.patch
          23 kB
          Mike Yoder

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            yoderme Mike Yoder
            yoderme Mike Yoder
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment