[HADOOP-13157] Follow-on improvements to hadoop credential commands

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.8.0
• Fix Version/s: 2.8.0, 3.0.0-alpha1
• Component/s: security
• Target Version/s: 2.8.0
• Assignee: Mike Yoder
• Reporter: Mike Yoder
• Created: 16/May/16 18:28
• Updated: 30/Aug/16 01:14
• Resolved: 18/May/16 21:58

Description

andrew.wang had some follow-up code review comments from HADOOP-12942. Hence this issue.

Ping lmccay 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.

Issue Links

• relates to: HADOOP-12942 hadoop credential commands non-obviously use password of "none"