Rationalize all the deprecated and not deprecated versions of solr ur…

…l and zk host Add bats tests Use all the parameters in cli in the same order added crednetials, need to add testing that basic auth works for all!
apache · Jun 10, 2024 · d514536 · d514536
1 parent cf622f2
commit d514536
Show file tree

Hide file tree

Showing 19 changed files with 101 additions and 24 deletions.
diff --git a/solr/core/src/java/org/apache/solr/cli/AuthTool.java b/solr/core/src/java/org/apache/solr/cli/AuthTool.java
@@ -111,7 +111,9 @@ public List<Option> getOptions() {
                 "This is where any authentication related configuration files, if any, would be placed.")
             .build(),
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
         SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
         SolrCLI.OPTION_CREDENTIALS);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetDownloadTool.java
@@ -57,8 +57,11 @@ public List<Option> getOptions() {
             .required(true)
             .desc("Local directory with configs.")
             .build(),
-        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java
@@ -65,8 +65,11 @@ public List<Option> getOptions() {
             .required(false)
             .desc("Parent directory of example configsets.")
             .build(),
-        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ConfigTool.java b/solr/core/src/java/org/apache/solr/cli/ConfigTool.java
@@ -82,7 +82,9 @@ public List<Option> getOptions() {
             .desc("Set the property to this value; accepts JSON objects and strings.")
             .build(),
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
         SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
         SolrCLI.OPTION_CREDENTIALS);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/CreateTool.java b/solr/core/src/java/org/apache/solr/cli/CreateTool.java
@@ -68,10 +68,6 @@ public String getName() {
   @Override
   public List<Option> getOptions() {
     return List.of(
-        SolrCLI.OPTION_ZKHOST,
-        SolrCLI.OPTION_SOLRURL,
-        SolrCLI.OPTION_ZKHOST_DEPRECATED,
-        SolrCLI.OPTION_SOLRURL,
         Option.builder("c")
             .longOpt("name")
             .argName("NAME")
@@ -111,6 +107,10 @@ public List<Option> getOptions() {
             .required(false)
             .desc("Configuration name; default is the collection name.")
             .build(),
+        SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
         SolrCLI.OPTION_CREDENTIALS);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java
@@ -61,7 +61,6 @@ public String getName() {
   @Override
   public List<Option> getOptions() {
     return List.of(
-        SolrCLI.OPTION_SOLRURL,
         Option.builder("c")
             .longOpt("name")
             .argName("NAME")
@@ -83,7 +82,10 @@ public List<Option> getOptions() {
             .desc(
                 "Skip safety checks when deleting the configuration directory used by a collection.")
             .build(),
+        SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
         SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
         SolrCLI.OPTION_CREDENTIALS);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java b/solr/core/src/java/org/apache/solr/cli/HealthcheckTool.java
@@ -56,15 +56,18 @@ public class HealthcheckTool extends ToolBase {
   @Override
   public List<Option> getOptions() {
     return List.of(
-        SolrCLI.OPTION_SOLRURL,
-        SolrCLI.OPTION_ZKHOST,
         Option.builder("c")
             .longOpt("name")
             .argName("NAME")
             .hasArg()
             .required(true)
             .desc("Name of the collection to check.")
-            .build());
+            .build(),
+        SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS);
   }
 
   enum ShardState {

diff --git a/solr/core/src/java/org/apache/solr/cli/LinkConfigTool.java b/solr/core/src/java/org/apache/solr/cli/LinkConfigTool.java
@@ -59,7 +59,8 @@ public List<Option> getOptions() {
             .required(true)
             .desc("Configset name in ZooKeeper.")
             .build(),
-        SolrCLI.OPTION_ZKHOST);
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED);
   }
 
   @Override

diff --git a/solr/core/src/java/org/apache/solr/cli/PackageTool.java b/solr/core/src/java/org/apache/solr/cli/PackageTool.java
@@ -308,7 +308,6 @@ private Pair<String, String> parsePackageVersion(String arg) {
   @Override
   public List<Option> getOptions() {
     return List.of(
-        SolrCLI.OPTION_SOLRURL,
         Option.builder()
             .longOpt("collections")
             .argName("COLLECTIONS")
@@ -344,6 +343,10 @@ public List<Option> getOptions() {
             .desc("Don't prompt for input; accept all default choices, defaults to false.")
             .longOpt("noprompt")
             .build(),
+        SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
         SolrCLI.OPTION_CREDENTIALS);
   }
 }
diff --git a/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java b/solr/core/src/java/org/apache/solr/cli/RunExampleTool.java
@@ -154,7 +154,6 @@ public List<Option> getOptions() {
             .desc("Specify the hostname for this Solr instance.")
             .longOpt("host")
             .build(),
-        SolrCLI.OPTION_ZKHOST,
         Option.builder("c")
             .required(false)
             .desc(
@@ -176,7 +175,9 @@ public List<Option> getOptions() {
             .desc(
                 "Additional options to be passed to the JVM when starting example Solr server(s).")
             .longOpt("addlopts")
-            .build());
+            .build(),
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED);
   }
 
   @Override

diff --git a/solr/core/src/java/org/apache/solr/cli/SolrCLI.java b/solr/core/src/java/org/apache/solr/cli/SolrCLI.java
@@ -595,7 +595,8 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception {
     String solrUrl =
         cli.hasOption("solr-url") ? cli.getOptionValue("solr-url") : cli.getOptionValue("solrUrl");
     if (solrUrl == null) {
-      String zkHost = cli.hasOption("z") ? cli.getOptionValue("z") : cli.getOptionValue("zkHost");
+      String zkHost =
+          cli.hasOption("zk-host") ? cli.getOptionValue("zk-host") : cli.getOptionValue("zkHost");
       if (zkHost == null) {
         solrUrl = SolrCLI.getDefaultSolrUrl();
         CLIO.getOutStream()
@@ -628,7 +629,8 @@ public static String normalizeSolrUrl(CommandLine cli) throws Exception {
    */
   public static String getZkHost(CommandLine cli) throws Exception {
 
-    String zkHost = cli.getOptionValue("zk-host");
+    String zkHost =
+        cli.hasOption("zk-host") ? cli.getOptionValue("zk-host") : cli.getOptionValue("zkHost");
     if (zkHost != null && !zkHost.isBlank()) {
       return zkHost;
     }

diff --git a/solr/core/src/java/org/apache/solr/cli/UpdateACLTool.java b/solr/core/src/java/org/apache/solr/cli/UpdateACLTool.java
@@ -57,7 +57,8 @@ public List<Option> getOptions() {
             .required(true)
             .desc("The path to update.")
             .build(),
-        SolrCLI.OPTION_ZKHOST);
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED);
   }
 
   @Override

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java b/solr/core/src/java/org/apache/solr/cli/ZkCpTool.java
@@ -75,9 +75,11 @@ public List<Option> getOptions() {
             .desc("Required to look up configuration for compressing state.json.")
             .build(),
         SolrCLI.OPTION_RECURSE,
-        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_SOLRURL,
-        SolrCLI.OPTION_VERBOSE);
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS);
   }
 
   @Override

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java b/solr/core/src/java/org/apache/solr/cli/ZkLsTool.java
@@ -47,8 +47,11 @@ public List<Option> getOptions() {
             .desc("Path to list.")
             .build(),
         SolrCLI.OPTION_RECURSE,
-        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMkrootTool.java
@@ -53,8 +53,12 @@ public List<Option> getOptions() {
             .required(false)
             .desc("Raise an error if the root exists.  Defaults to false.")
             .build(),
-        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_RECURSE,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java b/solr/core/src/java/org/apache/solr/cli/ZkMvTool.java
@@ -56,8 +56,12 @@ public List<Option> getOptions() {
             .required(true)
             .desc("Destination Znode to move to.")
             .build(),
-        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_RECURSE,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java b/solr/core/src/java/org/apache/solr/cli/ZkRmTool.java
@@ -49,8 +49,11 @@ public List<Option> getOptions() {
             .desc("Path to remove.")
             .build(),
         SolrCLI.OPTION_RECURSE,
-        SolrCLI.OPTION_ZKHOST,
         SolrCLI.OPTION_SOLRURL,
+        SolrCLI.OPTION_SOLRURL_DEPRECATED,
+        SolrCLI.OPTION_ZKHOST,
+        SolrCLI.OPTION_ZKHOST_DEPRECATED,
+        SolrCLI.OPTION_CREDENTIALS,
         SolrCLI.OPTION_VERBOSE);
   }
 

diff --git a/solr/packaging/test/test_config.bats b/solr/packaging/test/test_config.bats
@@ -52,3 +52,26 @@ teardown() {
   assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
   assert_output --partial "assuming solr url is http://localhost:${SOLR_PORT}."
 }
+
+# This test is to validate the deprecated and non deprecated options for connecting to Solr
+@test "connecting to solr via various solr urls and zk hosts" {
+  solr create -c COLL_NAME
+
+  run solr config -c COLL_NAME --property updateHandler.autoCommit.maxDocs -v 100 -solrUrl http://localhost:${SOLR_PORT}
+  assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
+  assert_output --partial "Deprecated for removal since 9.7: Use --solr-url instead"
+
+  run solr config -c COLL_NAME --property updateHandler.autoCommit.maxDocs -v 100 --solr-url http://localhost:${SOLR_PORT}
+  assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
+
+  run solr config -c COLL_NAME --property updateHandler.autoCommit.maxDocs -v 100 -zkHost localhost:${ZK_PORT}
+  assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
+  assert_output --partial "Deprecated for removal since 9.7: Use --zk-host instead"
+
+  run solr config -c COLL_NAME --property updateHandler.autoCommit.maxDocs -v 100 -z localhost:${ZK_PORT}
+  assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
+
+  run solr config -c COLL_NAME --property updateHandler.autoCommit.maxDocs -v 100 --zk-host localhost:${ZK_PORT}
+  assert_output --partial "Successfully set-property updateHandler.autoCommit.maxDocs to 100"
+
+}
diff --git a/solr/packaging/test/test_zk.bats b/solr/packaging/test/test_zk.bats
@@ -47,19 +47,34 @@ teardown() {
   assert_output --partial "aliases.json"
 }
 
-@test "get zk host using solr url" {
+@test "connecting to solr via various solr urls and zk hosts" {
   sleep 1
   run solr zk ls / -solrUrl http://localhost:${SOLR_PORT}
   assert_output --partial "aliases.json"
+  # We do mapping in bin/solr script from -solrUrl to --solr-url that prevents deprecation warning
+  #assert_output --partial "Deprecated for removal since 9.7: Use --solr-url instead"
 
   run solr zk ls / -s http://localhost:${SOLR_PORT}
   assert_output --partial "aliases.json"
+  # We do mapping in bin/solr script from -solrUrl to --solr-url that prevents deprecation warning
+  #assert_output --partial "Deprecated for removal since 9.7: Use --solr-url instead"
 
   run solr zk ls / --solr-url http://localhost:${SOLR_PORT}
   assert_output --partial "aliases.json"
 
   run solr zk ls /
   assert_output --partial "aliases.json"
+
+  run solr zk ls / -z localhost:${ZK_PORT}
+  assert_output --partial "aliases.json"
+
+  run solr zk ls / --zk-host localhost:${ZK_PORT}
+  assert_output --partial "aliases.json"
+
+  run solr zk ls / -zkHost localhost:${ZK_PORT}
+  assert_output --partial "aliases.json"
+  # We do mapping in bin/solr script from -zkHost to --zk-host that prevents deprecation warning
+  #assert_output --partial "Deprecated for removal since 9.7: Use --zk-host instead"
 }
 
 @test "copying files around" {