SOLR-18320: Use Configsets V2 API instead of direct ZooKeeper access in bin/solr create

changelog/unreleased/SOLR-18320-skip-zk-for-configset-upload.yml

@@ -0,0 +1,7 @@
+title: bin/solr create no longer requires a direct ZooKeeper connection to upload a configset; it now uses the Configsets V2 API.
+type: changed
+authors:
+  - name: Eric Pugh
+links:
+  - name: SOLR-18320
+    url: https://issues.apache.org/jira/browse/SOLR-18320

solr/core/src/java/org/apache/solr/cli/ConfigSetUploadTool.java

@@ -31,7 +31,14 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-/** Supports zk upconfig command in the bin/solr script. */
+/**
+ * Supports zk upconfig command in the bin/solr script.
+ *
+ * <p>Unlike {@link CreateTool}, this uploads directly to ZooKeeper rather than going through the
+ * Configsets V2 API. That's intentional: this command is meant to work against a bare ZooKeeper
+ * ensemble with no Solr node running yet (e.g. provisioning configs before the cluster starts), and
+ * the V2 API requires an HTTP endpoint on an already-running node to PUT the zip to.
+ */
 @SuppressWarnings("UnnecessarilyFullyQualified")
 @picocli.CommandLine.Command(
     name = "upconfig",

solr/core/src/java/org/apache/solr/cli/CreateTool.java

@@ -16,6 +16,7 @@
  */
 package org.apache.solr.cli;
 
+import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
@@ -34,12 +35,13 @@
 import org.apache.solr.client.solrj.impl.CloudSolrClient;
 import org.apache.solr.client.solrj.jetty.HttpJettySolrClient;
 import org.apache.solr.client.solrj.request.CollectionsApi;
+import org.apache.solr.client.solrj.request.ConfigsetsApi;
 import org.apache.solr.client.solrj.request.CoresApi;
 import org.apache.solr.client.solrj.request.SystemInfoRequest;
 import org.apache.solr.client.solrj.response.SystemInfoResponse;
-import org.apache.solr.cloud.ZkConfigSetService;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.util.EnvUtils;
+import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.ConfigSetService;
 
 /** Supports create command in the bin/solr script. */
@@ -317,24 +319,22 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para
     int replicationFactor = params.replicationFactor;
     String confName = params.confName;
 
-    boolean configExistsInZk =
+    boolean configExists =
         confName != null
             && !confName.trim().isEmpty()
-            && ZkStateReader.from(cloudSolrClient).getZkClient().exists("/configs/" + confName);
+            && new ConfigsetsApi.ListConfigSet()
+                .process(cloudSolrClient)
+                .configSets
+                .contains(confName);
 
-    if (configExistsInZk) {
+    if (configExists) {
       echo("Re-using existing configuration directory " + confName);
-    } else { // if (confdir != null && !confdir.trim().isEmpty()) {
+    } else {
       if (confName == null || confName.trim().isEmpty()) {
         confName = params.name;
       }
 
-      // TODO: This should be done using the configSet API.  This would let us remove
-      // the direct dependency on ZooKeeper APIs.  Unlike the bin/solr zk comamnds that
-      // work directly with ZooKeeper.
       final Path configsetsDirPath = CLIUtils.getConfigSetsDir(solrInstallDirPath);
-      ConfigSetService configSetService =
-          new ZkConfigSetService(ZkStateReader.from(cloudSolrClient).getZkClient());
       Path confPath =
           ConfigSetService.getConfigsetPath(params.confDir, configsetsDirPath.toString());
 
@@ -343,10 +343,11 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para
               + confPath.toAbsolutePath()
               + " for config "
               + confName
-              + " to ZooKeeper at "
-              + cloudSolrClient.getClusterStateProvider().getQuorumHosts());
-      // We will trust the config since we have the Zookeeper Address
-      configSetService.uploadConfig(confName, confPath);
+              + " using the Configsets V2 API");
+      var uploadReq =
+          new ConfigsetsApi.UploadConfigSet(
+              confName, new ByteArrayInputStream(zipConfigSet(confPath)));
+      uploadReq.process(cloudSolrClient);
     }
 
     // since creating a collection is a heavy-weight operation, check for existence first
@@ -387,6 +388,11 @@ private void createCollection(CloudSolrClient cloudSolrClient, CreateParams para
     echo(endMessage);
   }
 
+  /** Zips the contents of a configset directory for upload. */
+  static byte[] zipConfigSet(Path confPath) throws IOException {
+    return Utils.zipDirectory(confPath);
+  }
+
   private Path getFullConfDir(Path solrInstallDir, Path confDirName) {
     return CLIUtils.getConfigSetsDir(solrInstallDir).resolve(confDirName);
   }

solr/core/src/java/org/apache/solr/handler/configsets/DownloadConfigSet.java

@@ -21,20 +21,14 @@
 import jakarta.inject.Inject;
 import jakarta.ws.rs.core.Response;
 import jakarta.ws.rs.core.StreamingOutput;
-import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.io.InputStream;
-import java.nio.file.FileVisitResult;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.SimpleFileVisitor;
-import java.nio.file.attribute.BasicFileAttributes;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
 import org.apache.commons.io.file.PathUtils;
 import org.apache.solr.client.api.endpoint.ConfigsetsApi;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.StrUtils;
+import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.ConfigSetService;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.jersey.PermissionName;
@@ -86,48 +80,12 @@ public static Response buildZipResponse(ConfigSetService configSetService, Strin
    */
   public static byte[] zipConfigSet(ConfigSetService configSetService, String configSetName)
       throws IOException {
-    ByteArrayOutputStream baos = new ByteArrayOutputStream();
     Path tmpDirectory = Files.createTempDirectory("configset-download-");
     try {
       configSetService.downloadConfig(configSetName, tmpDirectory);
-      try (ZipOutputStream zipOut = new ZipOutputStream(baos)) {
-        Files.walkFileTree(
-            tmpDirectory,
-            new SimpleFileVisitor<>() {
-              @Override
-              public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
-                  throws IOException {
-                if (Files.isHidden(dir)) {
-                  return FileVisitResult.SKIP_SUBTREE;
-                }
-                String dirName = tmpDirectory.relativize(dir).toString();
-                if (!dirName.isEmpty()) {
-                  if (!dirName.endsWith("/")) {
-                    dirName += "/";
-                  }
-                  zipOut.putNextEntry(new ZipEntry(dirName));
-                  zipOut.closeEntry();
-                }
-                return FileVisitResult.CONTINUE;
-              }
-
-              @Override
-              public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
-                  throws IOException {
-                if (!Files.isHidden(file)) {
-                  try (InputStream fis = Files.newInputStream(file)) {
-                    ZipEntry zipEntry = new ZipEntry(tmpDirectory.relativize(file).toString());
-                    zipOut.putNextEntry(zipEntry);
-                    fis.transferTo(zipOut);
-                  }
-                }
-                return FileVisitResult.CONTINUE;
-              }
-            });
-      }
+      return Utils.zipDirectory(tmpDirectory);
     } finally {
       PathUtils.deleteDirectory(tmpDirectory);
     }
-    return baos.toByteArray();
   }
 }

solr/core/src/test/org/apache/solr/cli/CreateToolTest.java

@@ -17,6 +17,13 @@
 
 package org.apache.solr.cli;
 
+import java.io.ByteArrayInputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.util.SecurityJson;
 import org.junit.BeforeClass;
@@ -57,4 +64,52 @@ public void testCreateCollectionWithBasicAuth() throws Exception {
 
     assertEquals(0, runTool(args, CreateTool.class));
   }
+
+  @Test
+  public void testCreateCollectionUploadsNewConfigSet() throws Exception {
+    String[] args = {
+      "create",
+      "-c",
+      "testCreateCollectionUploadsNewConfigSet",
+      "-d",
+      configset("cloud-minimal").toString(),
+      "-n",
+      "cloud-minimal-uploaded",
+      "-z",
+      cluster.getZkClient().getZkServerAddress(),
+      "--credentials",
+      SecurityJson.USER_PASS,
+      "--verbose"
+    };
+
+    assertEquals(0, CLITestHelper.runTool(args, CreateTool.class));
+    assertTrue(cluster.getZkClient().exists("/configs/cloud-minimal-uploaded"));
+  }
+
+  @Test
+  public void testZipConfigSetSkipsHiddenFilesAndIncludesDirectoryEntries() throws Exception {
+    Path confDir = createTempDir("zipConfigSetTest");
+    Files.writeString(confDir.resolve("solrconfig.xml"), "<config/>");
+    Files.writeString(confDir.resolve(".hidden-file"), "should not be zipped");
+    Path langDir = Files.createDirectory(confDir.resolve("lang"));
+    Files.writeString(langDir.resolve("stopwords.txt"), "the\na\n");
+    Path hiddenDir = Files.createDirectory(confDir.resolve(".hiddenDir"));
+    Files.writeString(hiddenDir.resolve("nope.txt"), "should not be zipped either");
+
+    byte[] zipBytes = CreateTool.zipConfigSet(confDir);
+
+    Set<String> entryNames = new HashSet<>();
+    try (ZipInputStream zipIn = new ZipInputStream(new ByteArrayInputStream(zipBytes))) {
+      ZipEntry entry;
+      while ((entry = zipIn.getNextEntry()) != null) {
+        entryNames.add(entry.getName());
+      }
+    }
+
+    assertTrue(entryNames.contains("solrconfig.xml"));
+    assertTrue(entryNames.contains("lang/"));
+    assertTrue(entryNames.contains("lang/stopwords.txt"));
+    assertFalse(entryNames.contains(".hidden-file"));
+    assertTrue(entryNames.stream().noneMatch(name -> name.startsWith(".hiddenDir")));
+  }
 }

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -43,6 +43,11 @@
 import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.SimpleFileVisitor;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.AbstractMap;
@@ -69,6 +74,8 @@
 import java.util.function.Predicate;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipOutputStream;
 import org.apache.solr.common.IteratorWriter;
 import org.apache.solr.common.LinkedHashMapWriter;
 import org.apache.solr.common.MapWriter;
@@ -1045,4 +1052,53 @@ public static ByteBuffer toByteArray(InputStream is) throws IOException {
       return bos.getByteBuffer();
     }
   }
+
+  /**
+   * Zips the contents of {@code rootPath} into an in-memory archive. Hidden files and directories
+   * (as determined by {@link Files#isHidden}) are skipped, directory entries are written for
+   * non-empty subdirectories, and zip entry names are normalized to use {@code /} separators
+   * regardless of platform.
+   *
+   * @param rootPath the directory to zip
+   * @return the zipped bytes
+   */
+  public static byte[] zipDirectory(Path rootPath) throws IOException {
+    ByteArrayOutputStream baos = new ByteArrayOutputStream();
+    try (ZipOutputStream zipOut = new ZipOutputStream(baos)) {
+      Files.walkFileTree(
+          rootPath,
+          new SimpleFileVisitor<>() {
+            @Override
+            public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs)
+                throws IOException {
+              if (Files.isHidden(dir)) {
+                return FileVisitResult.SKIP_SUBTREE;
+              }
+              String dirName = rootPath.relativize(dir).toString().replace('\\', '/');
+              if (!dirName.isEmpty()) {
+                if (!dirName.endsWith("/")) {
+                  dirName += "/";
+                }
+                zipOut.putNextEntry(new ZipEntry(dirName));
+                zipOut.closeEntry();
+              }
+              return FileVisitResult.CONTINUE;
+            }
+
+            @Override
+            public FileVisitResult visitFile(Path file, BasicFileAttributes attrs)
+                throws IOException {
+              if (Files.isHidden(file)) {
+                return FileVisitResult.CONTINUE;
+              }
+              String entryName = rootPath.relativize(file).toString().replace('\\', '/');
+              zipOut.putNextEntry(new ZipEntry(entryName));
+              Files.copy(file, zipOut);
+              zipOut.closeEntry();
+              return FileVisitResult.CONTINUE;
+            }
+          });
+    }
+    return baos.toByteArray();
+  }
 }

solr/test-framework/src/java/org/apache/solr/cloud/RemoteSolrBackend.java

@@ -20,8 +20,6 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.List;
-import java.util.zip.ZipEntry;
-import java.util.zip.ZipOutputStream;
 import org.apache.solr.SolrBackend;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrServerException;
@@ -30,6 +28,7 @@
 import org.apache.solr.client.solrj.request.ConfigSetAdminRequest;
 import org.apache.solr.common.util.IOUtils;
 import org.apache.solr.common.util.URLUtil;
+import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreContainer;
 
 /**
@@ -68,7 +67,7 @@ public SolrClient getSolrClient() {
   public void uploadConfigSet(Path configDir, String name) throws SolrServerException, IOException {
     Path tempZip = Files.createTempFile("configset-", ".zip");
     try {
-      zipDirectory(configDir, tempZip);
+      Files.write(tempZip, Utils.zipDirectory(configDir));
       new ConfigSetAdminRequest.Upload()
           .setConfigSetName(name)
           .setUploadFile(tempZip, "application/zip")
@@ -83,24 +82,6 @@ public boolean hasConfigSet(String name) throws SolrServerException, IOException
     return new ConfigSetAdminRequest.List().process(adminClient).getConfigSets().contains(name);
   }
 
-  /** Packages {@code sourceDir} contents into a zip file. */
-  private static void zipDirectory(Path sourceDir, Path targetZip) throws IOException {
-    try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(targetZip))) {
-      Files.walk(sourceDir)
-          .filter(Files::isRegularFile)
-          .forEach(
-              file -> {
-                try {
-                  zos.putNextEntry(new ZipEntry(sourceDir.relativize(file).toString()));
-                  Files.copy(file, zos);
-                  zos.closeEntry();
-                } catch (IOException e) {
-                  throw new RuntimeException(e);
-                }
-              });
-    }
-  }
-
   @Override
   public void createCollection(CollectionAdminRequest.Create create)
       throws SolrServerException, IOException {