Repository: storm
Updated Branches:
  refs/heads/master 6b043d794 -> 96b702dfc


STORM-3134: Improve upload-creds user experience


Project: http://git-wip-us.apache.org/repos/asf/storm/repo
Commit: http://git-wip-us.apache.org/repos/asf/storm/commit/aa4d93c9
Tree: http://git-wip-us.apache.org/repos/asf/storm/tree/aa4d93c9
Diff: http://git-wip-us.apache.org/repos/asf/storm/diff/aa4d93c9

Branch: refs/heads/master
Commit: aa4d93c9c3d75b12c00cbd8966f976e5ea107fbe
Parents: 10f01f4
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Authored: Fri Jun 29 18:06:34 2018 -0500
Committer: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Committed: Fri Jun 29 18:06:34 2018 -0500

----------------------------------------------------------------------
 .../org/apache/storm/command/AdminCommands.java |  25 +++++
 .../src/jvm/org/apache/storm/command/CLI.java   | 105 +++++++++++++++++--
 .../apache/storm/command/UploadCredentials.java |  43 +++++++-
 .../jvm/org/apache/storm/command/TestCLI.java   |  44 ++++++++
 4 files changed, 206 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/storm/blob/aa4d93c9/storm-core/src/jvm/org/apache/storm/command/AdminCommands.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/command/AdminCommands.java 
b/storm-core/src/jvm/org/apache/storm/command/AdminCommands.java
index 10eaedc..8fabe6c 100644
--- a/storm-core/src/jvm/org/apache/storm/command/AdminCommands.java
+++ b/storm-core/src/jvm/org/apache/storm/command/AdminCommands.java
@@ -30,6 +30,7 @@ import org.apache.storm.cluster.ClusterStateContext;
 import org.apache.storm.cluster.ClusterUtils;
 import org.apache.storm.cluster.DaemonType;
 import org.apache.storm.cluster.IStormClusterState;
+import org.apache.storm.generated.Credentials;
 import org.apache.storm.nimbus.NimbusInfo;
 import org.apache.storm.shade.org.apache.zookeeper.ZkCli;
 import org.apache.storm.utils.ConfigUtils;
@@ -80,6 +81,29 @@ public class AdminCommands {
         }
     }
 
+    private static class CredentialsDebug implements AdminCommand {
+        @Override
+        public void run(String[] args, Map<String, Object> conf, String 
command) throws Exception {
+            // We are pretending to be nimbus here.
+            IStormClusterState state = ClusterUtils.mkStormClusterState(conf, 
new ClusterStateContext(DaemonType.NIMBUS, conf));
+            for (String topologyId: args) {
+                System.out.println(topologyId + ":");
+                Credentials creds = state.credentials(topologyId, null);
+                if (creds != null) {
+                    for (String key : creds.get_creds().keySet()) {
+                        System.out.println("\t" + key);
+                    }
+                }
+            }
+        }
+
+        @Override
+        public void printCliHelp(String command, PrintStream out) {
+            out.println(command + " topology_id:");
+            out.println("\tPrint the credential keys for a topology.");
+        }
+    }
+
     private static class Help implements AdminCommand {
 
         @Override
@@ -109,6 +133,7 @@ public class AdminCommands {
     static {
         COMMANDS.put("remove_corrupt_topologies", new 
RemoveCorruptTopologies());
         COMMANDS.put("zk_cli", new ZkCli());
+        COMMANDS.put("creds", new CredentialsDebug());
         COMMANDS.put("help", new Help());
     }
 

http://git-wip-us.apache.org/repos/asf/storm/blob/aa4d93c9/storm-core/src/jvm/org/apache/storm/command/CLI.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/command/CLI.java 
b/storm-core/src/jvm/org/apache/storm/command/CLI.java
index 2c3311e..4510d29 100644
--- a/storm-core/src/jvm/org/apache/storm/command/CLI.java
+++ b/storm-core/src/jvm/org/apache/storm/command/CLI.java
@@ -109,7 +109,7 @@ public class CLI {
      * @param longName the multi character name of the option (no `--` 
characters proceed it).
      * @return a builder to be used to continue creating the command line.
      */
-    public CLIBuilder boolOpt(String shortName, String longName) {
+    public static CLIBuilder boolOpt(String shortName, String longName) {
         return new CLIBuilder().boolOpt(shortName, longName);
     }
 
@@ -118,7 +118,7 @@ public class CLI {
      * @param name the name of the argument.
      * @return a builder to be used to continue creating the command line.
      */
-    public CLIBuilder arg(String name) {
+    public static CLIBuilder arg(String name) {
         return new CLIBuilder().arg(name);
     }
 
@@ -128,7 +128,7 @@ public class CLI {
      * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
      * @return a builder to be used to continue creating the command line.
      */
-    public CLIBuilder arg(String name, Assoc assoc) {
+    public static CLIBuilder arg(String name, Assoc assoc) {
         return new CLIBuilder().arg(name, assoc);
     }
 
@@ -138,7 +138,7 @@ public class CLI {
      * @param parse an optional function to transform the string to something 
else. If null a NOOP is used.
      * @return a builder to be used to continue creating the command line.
      */
-    public CLIBuilder arg(String name, Parse parse) {
+    public static CLIBuilder arg(String name, Parse parse) {
         return new CLIBuilder().arg(name, parse);
     }
 
@@ -149,10 +149,50 @@ public class CLI {
      * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
      * @return a builder to be used to continue creating the command line.
      */
-    public CLIBuilder arg(String name, Parse parse, Assoc assoc) {
+    public static CLIBuilder arg(String name, Parse parse, Assoc assoc) {
         return new CLIBuilder().arg(name, parse, assoc);
     }
 
+    /**
+     * Add a named argument that is optional.
+     * @param name the name of the argument.
+     * @return a builder to be used to continue creating the command line.
+     */
+    public static CLIBuilder optionalArg(String name) {
+        return new CLIBuilder().optionalArg(name);
+    }
+
+    /**
+     * Add a named argument that is optional.
+     * @param name the name of the argument.
+     * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
+     * @return a builder to be used to continue creating the command line.
+     */
+    public static CLIBuilder optionalArg(String name, Assoc assoc) {
+        return new CLIBuilder().optionalArg(name, assoc);
+    }
+
+    /**
+     * Add a named argument that is optional.
+     * @param name the name of the argument.
+     * @param parse an optional function to transform the string to something 
else. If null a NOOP is used.
+     * @return a builder to be used to continue creating the command line.
+     */
+    public static CLIBuilder optionalArg(String name, Parse parse) {
+        return new CLIBuilder().optionalArg(name, parse);
+    }
+
+    /**
+     * Add a named argument that is optional.
+     * @param name the name of the argument.
+     * @param parse an optional function to transform the string to something 
else. If null a NOOP is used.
+     * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
+     * @return a builder to be used to continue creating the command line.
+     */
+    public static CLIBuilder optionalArg(String name, Parse parse, Assoc 
assoc) {
+        return new CLIBuilder().optionalArg(name, parse, assoc);
+    }
+
     public interface Parse {
         /**
          * Parse a String to the type you want it to be.
@@ -213,6 +253,7 @@ public class CLI {
     public static class CLIBuilder {
         private final ArrayList<Opt> opts = new ArrayList<>();
         private final ArrayList<Arg> args = new ArrayList<>();
+        private final ArrayList<Arg> optionalArgs = new ArrayList<>();
 
         /**
          * Add an option to be parsed.
@@ -299,11 +340,55 @@ public class CLI {
          * @return a builder to be used to continue creating the command line.
          */
         public CLIBuilder arg(String name, Parse parse, Assoc assoc) {
+            if (!optionalArgs.isEmpty()) {
+                throw new IllegalStateException("Cannot have a required 
argument after adding in an optional argument");
+            }
             args.add(new Arg(name, parse, assoc));
             return this;
         }
 
         /**
+         * Add a named argument that is optional.
+         * @param name the name of the argument.
+         * @return a builder to be used to continue creating the command line.
+         */
+        public CLIBuilder optionalArg(String name) {
+            return optionalArg(name, null, null);
+        }
+
+        /**
+         * Add a named argument that is optional.
+         * @param name the name of the argument.
+         * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
+         * @return a builder to be used to continue creating the command line.
+         */
+        public CLIBuilder optionalArg(String name, Assoc assoc) {
+            return optionalArg(name, null, assoc);
+        }
+
+        /**
+         * Add a named argument that is optional.
+         * @param name the name of the argument.
+         * @param parse an optional function to transform the string to 
something else. If null a NOOP is used.
+         * @return a builder to be used to continue creating the command line.
+         */
+        public CLIBuilder optionalArg(String name, Parse parse) {
+            return optionalArg(name, parse, null);
+        }
+
+        /**
+         * Add a named argument that is optional.
+         * @param name the name of the argument.
+         * @param parse an optional function to transform the string to 
something else. If null a NOOP is used.
+         * @param assoc an association command to decide what to do if the 
argument appears multiple times.  If null INTO_LIST is used.
+         * @return a builder to be used to continue creating the command line.
+         */
+        public CLIBuilder optionalArg(String name, Parse parse, Assoc assoc) {
+            optionalArgs.add(new Arg(name, parse, assoc));
+            return this;
+        }
+
+        /**
          * Parse the command line arguments.
          * @param rawArgs the string arguments to be parsed.
          * @return The parsed command line.
@@ -341,6 +426,8 @@ public class CLI {
                     ret.put(opt.shortName, current);
                 }
             }
+            List<Arg> fullArgs = new ArrayList<>(args);
+            fullArgs.addAll(optionalArgs);
             List<String> stringArgs = cl.getArgList();
             if (args.size() > stringArgs.size()) {
                 throw new RuntimeException("Wrong number of arguments at least 
" + args.size()
@@ -349,10 +436,10 @@ public class CLI {
 
             int argIndex = 0;
             int stringArgIndex = 0;
-            if (args.size() > 0) {
-                while (argIndex < args.size()) {
-                    Arg arg = args.get(argIndex);
-                    boolean isLastArg = (argIndex == (args.size() - 1));
+            if (fullArgs.size() > 0) {
+                while (argIndex < fullArgs.size()) {
+                    Arg arg = fullArgs.get(argIndex);
+                    boolean isLastArg = (argIndex == (fullArgs.size() - 1));
                     Object current = null;
                     int maxStringIndex = isLastArg ? stringArgs.size() : 
(stringArgIndex + 1);
                     for (; stringArgIndex < maxStringIndex; stringArgIndex++) {

http://git-wip-us.apache.org/repos/asf/storm/blob/aa4d93c9/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java
----------------------------------------------------------------------
diff --git a/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java 
b/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java
index 9a71fac..7eb89f2 100644
--- a/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java
+++ b/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java
@@ -14,10 +14,19 @@ package org.apache.storm.command;
 
 import java.io.FileReader;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
+import java.util.Set;
+import org.apache.storm.Config;
 import org.apache.storm.StormSubmitter;
+import org.apache.storm.generated.ClusterSummary;
+import org.apache.storm.generated.Nimbus;
+import org.apache.storm.generated.TopologySummary;
+import org.apache.storm.utils.NimbusClient;
+import org.apache.storm.utils.Utils;
+import org.json.simple.JSONValue;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -28,7 +37,7 @@ public class UploadCredentials {
     public static void main(String[] args) throws Exception {
         Map<String, Object> cl = CLI.opt("f", "file", null)
                                     .arg("topologyName", CLI.FIRST_WINS)
-                                    .arg("rawCredentials", CLI.INTO_LIST)
+                                    .optionalArg("rawCredentials", 
CLI.INTO_LIST)
                                     .parse(args);
 
         String credentialFile = (String) cl.get("f");
@@ -52,7 +61,37 @@ public class UploadCredentials {
                 credentialsMap.put(rawCredentials.get(i), rawCredentials.get(i 
+ 1));
             }
         }
-        StormSubmitter.pushCredentials(topologyName, new HashMap<>(), 
credentialsMap);
+
+        Map<String, Object> topologyConf = new HashMap<>();
+        //Try to get the topology conf from nimbus, so we can reuse it.
+        try (NimbusClient nc = NimbusClient.getConfiguredClient(new 
HashMap<>())) {
+            Nimbus.Iface client = nc.getClient();
+            ClusterSummary summary = client.getClusterInfo();
+            for (TopologySummary topo : summary.get_topologies()) {
+                if (topologyName.equals(topo.get_name())) {
+                    //We found the topology, lets get the conf
+                    String topologyId = topo.get_id();
+                    topologyConf = (Map<String, Object>) 
JSONValue.parse(client.getTopologyConf(topologyId));
+                    LOG.info("Using topology conf from {} as basis for getting 
new creds", topologyId);
+
+                    Map<String, Object> commandLine = 
Utils.readCommandLineOpts();
+                    List<String> clCreds = 
(List<String>)commandLine.get(Config.TOPOLOGY_AUTO_CREDENTIALS);
+                    List<String> topoCreds = 
(List<String>)topologyConf.get(Config.TOPOLOGY_AUTO_CREDENTIALS);
+                    
+                    if (clCreds != null) {
+                        Set<String> extra = new HashSet<>(clCreds);
+                        if (topoCreds != null) {
+                            extra.removeAll(topoCreds);
+                        }
+                        if (!extra.isEmpty()) {
+                            LOG.warn("The topology {} is not using {} but they 
were included here.", topologyId, extra);
+                        }
+                    }
+                    break;
+                }
+            }
+        }
+        StormSubmitter.pushCredentials(topologyName, topologyConf, 
credentialsMap);
         LOG.info("Uploaded new creds to topology: {}", topologyName);
     }
 }

http://git-wip-us.apache.org/repos/asf/storm/blob/aa4d93c9/storm-core/test/jvm/org/apache/storm/command/TestCLI.java
----------------------------------------------------------------------
diff --git a/storm-core/test/jvm/org/apache/storm/command/TestCLI.java 
b/storm-core/test/jvm/org/apache/storm/command/TestCLI.java
index fd06cbe..09e2db8 100644
--- a/storm-core/test/jvm/org/apache/storm/command/TestCLI.java
+++ b/storm-core/test/jvm/org/apache/storm/command/TestCLI.java
@@ -18,6 +18,7 @@ import java.util.Map;
 import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
 
 public class TestCLI {
 
@@ -62,6 +63,49 @@ public class TestCLI {
         assertEquals("value2", f.get("key2"));
     }
 
+
+    @Test
+    public void testOptional() throws Exception {
+        Map<String, Object> values = CLI.optionalArg("A", CLI.LAST_WINS)
+            .parse("TEST");
+
+        assertEquals(1, values.size());
+        assertEquals("TEST", values.get("A"));
+
+        values = CLI.optionalArg("A", CLI.LAST_WINS)
+            .parse();
+
+        assertEquals(1, values.size());
+        assertEquals(null, values.get("A"));
+
+
+        values = CLI.optionalArg("A", CLI.LAST_WINS)
+            .parse("THIS", "IS", "A", "TEST");
+
+        assertEquals(1, values.size());
+        assertEquals("TEST", values.get("A"));
+
+        values = CLI.arg("A", CLI.LAST_WINS)
+            .optionalArg("B", CLI.LAST_WINS)
+            .parse("THIS", "IS", "A", "TEST");
+
+        assertEquals(2, values.size());
+        assertEquals("THIS", values.get("A"));
+        assertEquals("TEST", values.get("B"));
+    }
+
+    @Test
+    public void argAfterOptional() throws Exception {
+        try {
+            CLI.optionalArg("A", CLI.LAST_WINS)
+                .arg("B");
+
+            fail("Expected an exception to be thrown by now");
+        } catch (IllegalStateException is) {
+            //Expected
+        }
+    }
+
     private static final class PairParse implements CLI.Parse {
 
         @Override

Reply via email to