HADOOP-13112. Change CredentialShell to use CommandShell base class (Matthew Paduano via aw)

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

@@ -19,7 +19,6 @@
 package org.apache.hadoop.crypto.key;
 
 import java.io.IOException;
-import java.io.PrintStream;
 import java.security.InvalidParameterException;
 import java.security.NoSuchAlgorithmException;
 import java.util.HashMap;
@@ -27,17 +26,19 @@ import java.util.List;
 import java.util.Map;
 
 import com.google.common.annotations.VisibleForTesting;
+
+import org.apache.commons.lang.StringUtils;
+
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.conf.Configured;
 import org.apache.hadoop.crypto.key.KeyProvider.Metadata;
 import org.apache.hadoop.crypto.key.KeyProvider.Options;
-import org.apache.hadoop.util.Tool;
+import org.apache.hadoop.tools.CommandShell;
 import org.apache.hadoop.util.ToolRunner;
 
 /**
  * This program is the CLI utility for the KeyProvider facilities in Hadoop.
  */
-public class KeyShell extends Configured implements Tool {
+public class KeyShell extends CommandShell {
   final static private String USAGE_PREFIX = "Usage: hadoop key " +
       "[generic options]\n";
   final static private String COMMANDS =
@@ -55,50 +56,12 @@ public class KeyShell extends Configured implements Tool {
       "MUST use the -provider argument.";
 
   private boolean interactive = true;
-  private Command command = null;
 
   /** If true, fail if the provider requires a password and none is given. */
   private boolean strict = false;
 
-  /** allows stdout to be captured if necessary. */
-  @VisibleForTesting
-  public PrintStream out = System.out;
-  /** allows stderr to be captured if necessary. */
-  @VisibleForTesting
-  public PrintStream err = System.err;
-
   private boolean userSuppliedProvider = false;
 
-  /**
-   * Primary entry point for the KeyShell; called via main().
-   *
-   * @param args Command line arguments.
-   * @return 0 on success and 1 on failure.  This value is passed back to
-   * the unix shell, so we must follow shell return code conventions:
-   * the return code is an unsigned character, and 0 means success, and
-   * small positive integers mean failure.
-   * @throws Exception
-   */
-  @Override
-  public int run(String[] args) throws Exception {
-    int exitCode = 0;
-    try {
-      exitCode = init(args);
-      if (exitCode != 0) {
-        return exitCode;
-      }
-      if (command.validate()) {
-        command.execute();
-      } else {
-        exitCode = 1;
-      }
-    } catch (Exception e) {
-      e.printStackTrace(err);
-      return 1;
-    }
-    return exitCode;
-  }
-
   /**
    * Parse the command line arguments and initialize the data.
    * <pre>
@@ -112,7 +75,8 @@ public class KeyShell extends Configured implements Tool {
    * @return 0 on success, 1 on failure.
    * @throws IOException
    */
-  private int init(String[] args) throws IOException {
+  @Override
+  protected int init(String[] args) throws IOException {
     final Options options = KeyProvider.options(getConf());
     final Map<String, String> attributes = new HashMap<String, String>();
 
@@ -123,10 +87,8 @@ public class KeyShell extends Configured implements Tool {
         if (moreTokens) {
           keyName = args[++i];
         }
-
-        command = new CreateCommand(keyName, options);
+        setSubCommand(new CreateCommand(keyName, options));
         if ("-help".equals(keyName)) {
-          printKeyShellUsage();
           return 1;
         }
       } else if (args[i].equals("delete")) {
@@ -134,10 +96,8 @@ public class KeyShell extends Configured implements Tool {
         if (moreTokens) {
           keyName = args[++i];
         }
-
-        command = new DeleteCommand(keyName);
+        setSubCommand(new DeleteCommand(keyName));
         if ("-help".equals(keyName)) {
-          printKeyShellUsage();
           return 1;
         }
       } else if (args[i].equals("roll")) {
@@ -145,14 +105,12 @@ public class KeyShell extends Configured implements Tool {
         if (moreTokens) {
           keyName = args[++i];
         }
-
-        command = new RollCommand(keyName);
+        setSubCommand(new RollCommand(keyName));
         if ("-help".equals(keyName)) {
-          printKeyShellUsage();
           return 1;
         }
       } else if ("list".equals(args[i])) {
-        command = new ListCommand();
+        setSubCommand(new ListCommand());
       } else if ("-size".equals(args[i]) && moreTokens) {
         options.setBitLength(Integer.parseInt(args[++i]));
       } else if ("-cipher".equals(args[i]) && moreTokens) {
@@ -164,15 +122,13 @@ public class KeyShell extends Configured implements Tool {
         final String attr = attrval[0].trim();
         final String val = attrval[1].trim();
         if (attr.isEmpty() || val.isEmpty()) {
-          out.println("\nAttributes must be in attribute=value form, " +
-                  "or quoted\nlike \"attribute = value\"\n");
-          printKeyShellUsage();
+          getOut().println("\nAttributes must be in attribute=value form, " +
+              "or quoted\nlike \"attribute = value\"\n");
           return 1;
         }
         if (attributes.containsKey(attr)) {
-          out.println("\nEach attribute must correspond to only one value:\n" +
-                  "atttribute \"" + attr + "\" was repeated\n" );
-          printKeyShellUsage();
+          getOut().println("\nEach attribute must correspond to only one " +
+              "value:\natttribute \"" + attr + "\" was repeated\n");
           return 1;
         }
         attributes.put(attr, val);
@@ -186,20 +142,13 @@ public class KeyShell extends Configured implements Tool {
       } else if (args[i].equals("-strict")) {
         strict = true;
       } else if ("-help".equals(args[i])) {
-        printKeyShellUsage();
         return 1;
       } else {
-        printKeyShellUsage();
-        ToolRunner.printGenericCommandUsage(System.err);
+        ToolRunner.printGenericCommandUsage(getErr());
         return 1;
       }
     }
 
-    if (command == null) {
-      printKeyShellUsage();
-      return 1;
-    }
-
     if (!attributes.isEmpty()) {
       options.setAttributes(attributes);
     }
@@ -207,33 +156,24 @@ public class KeyShell extends Configured implements Tool {
     return 0;
   }
 
-  private void printKeyShellUsage() {
-    out.println(USAGE_PREFIX + COMMANDS);
-    if (command != null) {
-      out.println(command.getUsage());
-    } else {
-      out.println("=========================================================" +
-          "======");
-      out.println(CreateCommand.USAGE + ":\n\n" + CreateCommand.DESC);
-      out.println("=========================================================" +
-          "======");
-      out.println(RollCommand.USAGE + ":\n\n" + RollCommand.DESC);
-      out.println("=========================================================" +
-          "======");
-      out.println(DeleteCommand.USAGE + ":\n\n" + DeleteCommand.DESC);
-      out.println("=========================================================" +
-          "======");
-      out.println(ListCommand.USAGE + ":\n\n" + ListCommand.DESC);
-    }
+  @Override
+  public String getCommandUsage() {
+    StringBuffer sbuf = new StringBuffer(USAGE_PREFIX + COMMANDS);
+    String banner = StringUtils.repeat("=", 66);
+    sbuf.append(banner + "\n");
+    sbuf.append(CreateCommand.USAGE + ":\n\n" + CreateCommand.DESC + "\n");
+    sbuf.append(banner + "\n");
+    sbuf.append(RollCommand.USAGE + ":\n\n" + RollCommand.DESC + "\n");
+    sbuf.append(banner + "\n");
+    sbuf.append(DeleteCommand.USAGE + ":\n\n" + DeleteCommand.DESC + "\n");
+    sbuf.append(banner + "\n");
+    sbuf.append(ListCommand.USAGE + ":\n\n" + ListCommand.DESC + "\n");
+    return sbuf.toString();
   }
 
-  private abstract class Command {
+  private abstract class Command extends SubCommand {
     protected KeyProvider provider = null;
 
-    public boolean validate() {
-      return true;
-    }
-
     protected KeyProvider getKeyProvider() {
       KeyProvider prov = null;
       List<KeyProvider> providers;
@@ -250,21 +190,21 @@ public class KeyShell extends Configured implements Tool {
           }
         }
       } catch (IOException e) {
-        e.printStackTrace(err);
+        e.printStackTrace(getErr());
       }
       if (prov == null) {
-        out.println(NO_VALID_PROVIDERS);
+        getOut().println(NO_VALID_PROVIDERS);
       }
       return prov;
     }
 
     protected void printProviderWritten() {
-      out.println(provider + " has been updated.");
+      getOut().println(provider + " has been updated.");
     }
 
     protected void warnIfTransientProvider() {
       if (provider.isTransient()) {
-        out.println("WARNING: you are modifying a transient provider.");
+        getOut().println("WARNING: you are modifying a transient provider.");
       }
     }
 
@@ -298,20 +238,20 @@ public class KeyShell extends Configured implements Tool {
     public void execute() throws IOException {
       try {
         final List<String> keys = provider.getKeys();
-        out.println("Listing keys for KeyProvider: " + provider);
+        getOut().println("Listing keys for KeyProvider: " + provider);
         if (metadata) {
           final Metadata[] meta =
             provider.getKeysMetadata(keys.toArray(new String[keys.size()]));
           for (int i = 0; i < meta.length; ++i) {
-            out.println(keys.get(i) + " : " + meta[i]);
+            getOut().println(keys.get(i) + " : " + meta[i]);
           }
         } else {
           for (String keyName : keys) {
-            out.println(keyName);
+            getOut().println(keyName);
           }
         }
       } catch (IOException e) {
-        out.println("Cannot list keys for KeyProvider: " + provider
+        getOut().println("Cannot list keys for KeyProvider: " + provider
             + ": " + e.toString());
         throw e;
       }
@@ -345,7 +285,7 @@ public class KeyShell extends Configured implements Tool {
         rc = false;
       }
       if (keyName == null) {
-        out.println("Please provide a <keyname>.\n" +
+        getOut().println("Please provide a <keyname>.\n" +
             "See the usage description by using -help.");
         rc = false;
       }
@@ -355,20 +295,20 @@ public class KeyShell extends Configured implements Tool {
     public void execute() throws NoSuchAlgorithmException, IOException {
       try {
         warnIfTransientProvider();
-        out.println("Rolling key version from KeyProvider: "
+        getOut().println("Rolling key version from KeyProvider: "
             + provider + "\n  for key name: " + keyName);
         try {
           provider.rollNewVersion(keyName);
           provider.flush();
-          out.println(keyName + " has been successfully rolled.");
+          getOut().println(keyName + " has been successfully rolled.");
           printProviderWritten();
         } catch (NoSuchAlgorithmException e) {
-          out.println("Cannot roll key: " + keyName + " within KeyProvider: "
-              + provider + ". " + e.toString());
+          getOut().println("Cannot roll key: " + keyName +
+              " within KeyProvider: " + provider + ". " + e.toString());
           throw e;
         }
       } catch (IOException e1) {
-        out.println("Cannot roll key: " + keyName + " within KeyProvider: "
+        getOut().println("Cannot roll key: " + keyName + " within KeyProvider: "
             + provider + ". " + e1.toString());
         throw e1;
       }
@@ -405,7 +345,7 @@ public class KeyShell extends Configured implements Tool {
         return false;
       }
       if (keyName == null) {
-        out.println("There is no keyName specified. Please specify a " +
+        getOut().println("There is no keyName specified. Please specify a " +
             "<keyname>. See the usage description with -help.");
         return false;
       }
@@ -416,12 +356,12 @@ public class KeyShell extends Configured implements Tool {
                   + " key " + keyName + " from KeyProvider "
                   + provider + ". Continue? ");
           if (!cont) {
-            out.println(keyName + " has not been deleted.");
+            getOut().println(keyName + " has not been deleted.");
           }
           return cont;
         } catch (IOException e) {
-          out.println(keyName + " will not be deleted.");
-          e.printStackTrace(err);
+          getOut().println(keyName + " will not be deleted.");
+          e.printStackTrace(getErr());
         }
       }
       return true;
@@ -429,16 +369,16 @@ public class KeyShell extends Configured implements Tool {
 
     public void execute() throws IOException {
       warnIfTransientProvider();
-      out.println("Deleting key: " + keyName + " from KeyProvider: "
+      getOut().println("Deleting key: " + keyName + " from KeyProvider: "
           + provider);
       if (cont) {
         try {
           provider.deleteKey(keyName);
           provider.flush();
-          out.println(keyName + " has been successfully deleted.");
+          getOut().println(keyName + " has been successfully deleted.");
           printProviderWritten();
         } catch (IOException e) {
-          out.println(keyName + " has not been deleted. " + e.toString());
+          getOut().println(keyName + " has not been deleted. " + e.toString());
           throw e;
         }
       }
@@ -483,18 +423,18 @@ public class KeyShell extends Configured implements Tool {
           rc = false;
         } else if (provider.needsPassword()) {
           if (strict) {
-            out.println(provider.noPasswordError());
+            getOut().println(provider.noPasswordError());
             rc = false;
           } else {
-            out.println(provider.noPasswordWarning());
+            getOut().println(provider.noPasswordWarning());
           }
         }
       } catch (IOException e) {
-        e.printStackTrace(err);
+        e.printStackTrace(getErr());
       }
       if (keyName == null) {
-        out.println("Please provide a <keyname>. See the usage description" +
-            " with -help.");
+        getOut().println("Please provide a <keyname>. " +
+            " See the usage description with -help.");
         rc = false;
       }
       return rc;
@@ -505,17 +445,17 @@ public class KeyShell extends Configured implements Tool {
       try {
         provider.createKey(keyName, options);
         provider.flush();
-        out.println(keyName + " has been successfully created with options "
-            + options.toString() + ".");
+        getOut().println(keyName + " has been successfully created " +
+            "with options " + options.toString() + ".");
         printProviderWritten();
       } catch (InvalidParameterException e) {
-        out.println(keyName + " has not been created. " + e.toString());
+        getOut().println(keyName + " has not been created. " + e.toString());
         throw e;
       } catch (IOException e) {
-        out.println(keyName + " has not been created. " + e.toString());
+        getOut().println(keyName + " has not been created. " + e.toString());
         throw e;
       } catch (NoSuchAlgorithmException e) {
-        out.println(keyName + " has not been created. " + e.toString());
+        getOut().println(keyName + " has not been created. " + e.toString());
         throw e;
       }
     }

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/alias/CredentialShell.java

@@ -20,23 +20,24 @@ package org.apache.hadoop.security.alias;
 
 import java.io.Console;
 import java.io.IOException;
-import java.io.PrintStream;
 import java.security.InvalidParameterException;
 import java.security.NoSuchAlgorithmException;
 import java.util.Arrays;
 import java.util.List;
 
 import com.google.common.annotations.VisibleForTesting;
+
+import org.apache.commons.lang.StringUtils;
+
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.conf.Configured;
-import org.apache.hadoop.util.Tool;
+import org.apache.hadoop.tools.CommandShell;
 import org.apache.hadoop.util.ToolRunner;
 
 /**
- * This program is the CLI utility for the CredentialProvider facilities in 
+ * This program is the CLI utility for the CredentialProvider facilities in
  * Hadoop.
  */
-public class CredentialShell extends Configured implements Tool {
+public class CredentialShell extends CommandShell {
   final static private String USAGE_PREFIX = "Usage: hadoop credential " +
       "[generic options]\n";
   final static private String COMMANDS =
@@ -52,44 +53,13 @@ public class CredentialShell extends Configured implements Tool {
       "MUST use the -provider argument.";
 
   private boolean interactive = true;
-  private Command command = null;
 
   /** If true, fail if the provider requires a password and none is given. */
   private boolean strict = false;
 
-  /** Allows stdout to be captured if necessary. */
-  @VisibleForTesting
-  public PrintStream out = System.out;
-  /** Allows stderr to be captured if necessary. */
-  @VisibleForTesting
-  public PrintStream err = System.err;
-
   private boolean userSuppliedProvider = false;
   private String value = null;
   private PasswordReader passwordReader;
-  private boolean isHelp = false;
-
-  @Override
-  public int run(String[] args) throws Exception {
-    int exitCode = 0;
-    try {
-      exitCode = init(args);
-      if (exitCode != 0) {
-        return exitCode;
-      }
-      if (!isHelp) {
-        if (command.validate()) {
-          command.execute();
-        } else {
-          exitCode = 1;
-        }
-      }
-    } catch (Exception e) {
-      e.printStackTrace(err);
-      return 1;
-    }
-    return exitCode;
-  }
 
   /**
    * Parse the command line arguments and initialize the data.
@@ -102,46 +72,33 @@ public class CredentialShell extends Configured implements Tool {
    * @return 0 if the argument(s) were recognized, 1 otherwise
    * @throws IOException
    */
+  @Override
   protected int init(String[] args) throws IOException {
     // no args should print the help message
     if (0 == args.length) {
-      printCredShellUsage();
-      ToolRunner.printGenericCommandUsage(System.err);
+      ToolRunner.printGenericCommandUsage(getErr());
       return 1;
     }
 
     for (int i = 0; i < args.length; i++) { // parse command line
       if (args[i].equals("create")) {
         if (i == args.length - 1) {
-          printCredShellUsage();
           return 1;
         }
-        String alias = args[++i];
-        command = new CreateCommand(alias);
-        if (alias.equals("-help")) {
-          printCredShellUsage();
-          return 0;
-        }
+        setSubCommand(new CreateCommand(args[++i]));
       } else if (args[i].equals("delete")) {
         if (i == args.length - 1) {
-          printCredShellUsage();
           return 1;
         }
-        String alias = args[++i];
-        command = new DeleteCommand(alias);
-        if (alias.equals("-help")) {
-          printCredShellUsage();
-          return 0;
-        }
+        setSubCommand(new DeleteCommand(args[++i]));
       } else if (args[i].equals("list")) {
-        command = new ListCommand();
+        setSubCommand(new ListCommand());
       } else if (args[i].equals("-provider")) {
         if (i == args.length - 1) {
-          printCredShellUsage();
           return 1;
         }
         userSuppliedProvider = true;
-        getConf().set(CredentialProviderFactory.CREDENTIAL_PROVIDER_PATH, 
+        getConf().set(CredentialProviderFactory.CREDENTIAL_PROVIDER_PATH,
             args[++i]);
       } else if (args[i].equals("-f") || (args[i].equals("-force"))) {
         interactive = false;
@@ -150,42 +107,32 @@ public class CredentialShell extends Configured implements Tool {
       } else if (args[i].equals("-v") || (args[i].equals("-value"))) {
         value = args[++i];
       } else if (args[i].equals("-help")) {
-        printCredShellUsage();
+        printShellUsage();
         return 0;
       } else {
-        printCredShellUsage();
-        ToolRunner.printGenericCommandUsage(System.err);
+        ToolRunner.printGenericCommandUsage(getErr());
         return 1;
       }
     }
     return 0;
   }
 
-  private void printCredShellUsage() {
-    isHelp = true;
-    out.println(USAGE_PREFIX + COMMANDS);
-    if (command != null) {
-      out.println(command.getUsage());
-    } else {
-      out.println("=========================================================" +
-          "======");
-      out.println(CreateCommand.USAGE + ":\n\n" + CreateCommand.DESC);
-      out.println("=========================================================" +
-          "======");
-      out.println(DeleteCommand.USAGE + ":\n\n" + DeleteCommand.DESC);
-      out.println("=========================================================" +
-          "======");
-      out.println(ListCommand.USAGE + ":\n\n" + ListCommand.DESC);
-    }
+  @Override
+  public String getCommandUsage() {
+    StringBuffer sbuf = new StringBuffer(USAGE_PREFIX + COMMANDS);
+    String banner = StringUtils.repeat("=", 66);
+    sbuf.append(banner + "\n");
+    sbuf.append(CreateCommand.USAGE + ":\n\n" + CreateCommand.DESC + "\n");
+    sbuf.append(banner + "\n");
+    sbuf.append(DeleteCommand.USAGE + ":\n\n" + DeleteCommand.DESC + "\n");
+    sbuf.append(banner + "\n");
+    sbuf.append(ListCommand.USAGE + ":\n\n" + ListCommand.DESC + "\n");
+    return sbuf.toString();
   }
 
-  private abstract class Command {
+  private abstract class Command extends SubCommand {
     protected CredentialProvider provider = null;
 
-    public boolean validate() {
-      return true;
-    }
-
     protected CredentialProvider getCredentialProvider() {
       CredentialProvider prov = null;
       List<CredentialProvider> providers;
@@ -202,24 +149,29 @@ public class CredentialShell extends Configured implements Tool {
           }
         }
       } catch (IOException e) {
-        e.printStackTrace(err);
+        e.printStackTrace(getErr());
       }
       if (prov == null) {
-        out.println(NO_VALID_PROVIDERS);
+        getOut().println(NO_VALID_PROVIDERS);
       }
       return prov;
     }
 
     protected void printProviderWritten() {
-      out.println("Provider " + provider.toString() + " has been updated.");
+      getOut().println("Provider " + provider.toString() + " was updated.");
     }
 
     protected void warnIfTransientProvider() {
       if (provider.isTransient()) {
-        out.println("WARNING: you are modifying a transient provider.");
+        getOut().println("WARNING: you are modifying a transient provider.");
       }
     }
 
+    protected void doHelp() {
+      getOut().println(USAGE_PREFIX + COMMANDS);
+      printShellUsage();
+    }
+
     public abstract void execute() throws Exception;
 
     public abstract String getUsage();
@@ -244,13 +196,13 @@ public class CredentialShell extends Configured implements Tool {
       List<String> aliases;
       try {
         aliases = provider.getAliases();
-        out.println("Listing aliases for CredentialProvider: " +
+        getOut().println("Listing aliases for CredentialProvider: " +
             provider.toString());
         for (String alias : aliases) {
-          out.println(alias);
+          getOut().println(alias);
         }
       } catch (IOException e) {
-        out.println("Cannot list aliases for CredentialProvider: " +
+        getOut().println("Cannot list aliases for CredentialProvider: " +
             provider.toString()
             + ": " + e.getMessage());
         throw e;
@@ -283,15 +235,18 @@ public class CredentialShell extends Configured implements Tool {
 
     @Override
     public boolean validate() {
-      provider = getCredentialProvider();
-      if (provider == null) {
-        return false;
-      }
       if (alias == null) {
-        out.println("There is no alias specified. Please provide the" +
+        getOut().println("There is no alias specified. Please provide the" +
             "mandatory <alias>. See the usage description with -help.");
         return false;
       }
+      if (alias.equals("-help")) {
+        return true;
+      }
+      provider = getCredentialProvider();
+      if (provider == null) {
+        return false;
+      }
       if (interactive) {
         try {
           cont = ToolRunner
@@ -299,30 +254,34 @@ public class CredentialShell extends Configured implements Tool {
                   alias + " from CredentialProvider " + provider.toString() +
                   ". Continue? ");
           if (!cont) {
-            out.println("Nothing has been deleted.");
+            getOut().println("Nothing has been deleted.");
           }
           return cont;
         } catch (IOException e) {
-          out.println(alias + " will not be deleted.");
-          e.printStackTrace(err);
+          getOut().println(alias + " will not be deleted.");
+          e.printStackTrace(getErr());
         }
       }
       return true;
     }
 
     public void execute() throws IOException {
+      if (alias.equals("-help")) {
+        doHelp();
+        return;
+      }
       warnIfTransientProvider();
-      out.println("Deleting credential: " + alias +
+      getOut().println("Deleting credential: " + alias +
           " from CredentialProvider: " + provider.toString());
       if (cont) {
         try {
           provider.deleteCredentialEntry(alias);
-          out.println("Credential " + alias +
+          getOut().println("Credential " + alias +
               " has been successfully deleted.");
           provider.flush();
           printProviderWritten();
         } catch (IOException e) {
-          out.println("Credential " + alias + " has NOT been deleted.");
+          getOut().println("Credential " + alias + " has NOT been deleted.");
           throw e;
         }
       }
@@ -352,31 +311,37 @@ public class CredentialShell extends Configured implements Tool {
     }
 
     public boolean validate() {
-      boolean rc = true;
+      if (alias == null) {
+        getOut().println("There is no alias specified. Please provide the" +
+            "mandatory <alias>. See the usage description with -help.");
+        return false;
+      }
+      if (alias.equals("-help")) {
+        return true;
+      }
       try {
         provider = getCredentialProvider();
         if (provider == null) {
-          rc = false;
+          return false;
         } else if (provider.needsPassword()) {
           if (strict) {
-            out.println(provider.noPasswordError());
-            rc = false;
+            getOut().println(provider.noPasswordError());
+            return false;
           } else {
-            out.println(provider.noPasswordWarning());
+            getOut().println(provider.noPasswordWarning());
           }
         }
       } catch (IOException e) {
-        e.printStackTrace(err);
-      }
-      if (alias == null) {
-        out.println("There is no alias specified. Please provide the" +
-            "mandatory <alias>. See the usage description with -help.");
-        rc = false;
+        e.printStackTrace(getErr());
       }
-      return rc;
+      return true;
     }
 
     public void execute() throws IOException, NoSuchAlgorithmException {
+      if (alias.equals("-help")) {
+        doHelp();
+        return;
+      }
       warnIfTransientProvider();
       try {
         char[] credential = null;
@@ -388,14 +353,14 @@ public class CredentialShell extends Configured implements Tool {
         }
         provider.createCredentialEntry(alias, credential);
         provider.flush();
-        out.println(alias + " has been successfully created.");
+        getOut().println(alias + " has been successfully created.");
         printProviderWritten();
       } catch (InvalidParameterException e) {
-        out.println("Credential " + alias + " has NOT been created. " +
+        getOut().println("Credential " + alias + " has NOT been created. " +
             e.getMessage());
         throw e;
       } catch (IOException e) {
-        out.println("Credential " + alias + " has NOT been created. " +
+        getOut().println("Credential " + alias + " has NOT been created. " +
             e.getMessage());
         throw e;
       }
@@ -406,13 +371,13 @@ public class CredentialShell extends Configured implements Tool {
       return USAGE + ":\n\n" + DESC;
     }
   }
-  
+
   protected char[] promptForCredential() throws IOException {
     PasswordReader c = getPasswordReader();
     if (c == null) {
       throw new IOException("No console available for prompting user.");
     }
-    
+
     char[] cred = null;
 
     boolean noMatch;
@@ -434,18 +399,18 @@ public class CredentialShell extends Configured implements Tool {
     } while (noMatch);
     return cred;
   }
-  
+
   public PasswordReader getPasswordReader() {
     if (passwordReader == null) {
       passwordReader = new PasswordReader();
     }
     return passwordReader;
   }
-  
+
   public void setPasswordReader(PasswordReader reader) {
     passwordReader = reader;
   }
-  
+
   /** To facilitate testing since Console is a final class. */
   public static class PasswordReader {
     public char[] readPassword(String prompt) {
@@ -459,8 +424,8 @@ public class CredentialShell extends Configured implements Tool {
       console.format(message);
     }
   }
-  
-  
+
+
   /**
    * Main program.
    *

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/tools/CommandShell.java

@@ -64,7 +64,7 @@ public abstract class CommandShell extends Configured implements Tool {
     int exitCode = 0;
     try {
       exitCode = init(args);
-      if (exitCode != 0) {
+      if (exitCode != 0 || subcommand == null) {
         printShellUsage();
         return exitCode;
       }
@@ -89,7 +89,7 @@ public abstract class CommandShell extends Configured implements Tool {
    */
   protected abstract int init(String[] args) throws Exception;
 
-  private void printShellUsage() {
+  protected final void printShellUsage() {
     if (subcommand != null) {
       out.println(subcommand.getUsage());
     } else {