SOLR-8415: Provide command to switch between non/secure mode in ZK

solr/CHANGES.txt

@@ -116,6 +116,9 @@ New Features
 * SOLR-8560: Added RequestStatusState enum which can be used when comparing states of 
   asynchronous requests. (Shai Erera)
 
+* SOLR-8415: Provide command to switch between non/secure mode in ZK
+  (Mike Drob, Gregory Chanan)
+
 Bug Fixes
 ----------------------
 

solr/core/src/java/org/apache/solr/cloud/ZkCLI.java

@@ -73,6 +73,7 @@ public class ZkCLI {
   private static final String LIST = "list";
   private static final String CMD = "cmd";
   private static final String CLUSTERPROP = "clusterprop";
+  private static final String UPDATEACLS = "updateacls";
   
   /**
    * Allows you to perform a variety of zookeeper related tasks, such as:
@@ -100,7 +101,8 @@ public class ZkCLI {
         .withDescription(
             "cmd to run: " + BOOTSTRAP + ", " + UPCONFIG + ", " + DOWNCONFIG
                 + ", " + LINKCONFIG + ", " + MAKEPATH + ", " + PUT + ", " + PUT_FILE + ","
-                + GET + "," + GET_FILE + ", " + LIST + ", " + CLEAR).create(CMD));
+                + GET + "," + GET_FILE + ", " + LIST + ", " + CLEAR
+                + ", " + UPDATEACLS).create(CMD));
 
     Option zkHostOption = new Option("z", ZKHOST, true,
         "ZooKeeper host address");
@@ -152,6 +154,7 @@ public class ZkCLI {
         System.out.println("zkcli.sh -zkhost localhost:9983 -cmd " + CLEAR + " /solr");
         System.out.println("zkcli.sh -zkhost localhost:9983 -cmd " + LIST);
         System.out.println("zkcli.sh -zkhost localhost:9983 -cmd " + CLUSTERPROP + " -" + NAME + " urlScheme -" + VALUE_LONG + " https" );
+        System.out.println("zkcli.sh -zkhost localhost:9983 -cmd " + UPDATEACLS + " /solr");
         return;
       }
       
@@ -301,6 +304,13 @@ public class ZkCLI {
           }
           byte [] data = zkClient.getData(arglist.get(0).toString(), null, null, true);
           FileUtils.writeByteArrayToFile(new File(arglist.get(1).toString()), data);
+        } else if (line.getOptionValue(CMD).equals(UPDATEACLS)) {
+          List arglist = line.getArgList();
+          if (arglist.size() != 1) {
+            System.out.println("-" + UPDATEACLS + " requires one arg - the path to update");
+            System.exit(1);
+          }
+          zkClient.updateACLs(arglist.get(0).toString());
         } else if (line.getOptionValue(CMD).equalsIgnoreCase(CLUSTERPROP)) {
           if(!line.hasOption(NAME)) {
             System.out.println("-" + NAME + " is required for " + CLUSTERPROP);

solr/core/src/test/org/apache/solr/cloud/ZkCLITest.java

@@ -25,6 +25,7 @@ import org.apache.solr.SolrJettyTestBase;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.VMParamsAllAndReadonlyDigestZkACLProvider;
 import org.apache.solr.common.cloud.ZkConfigManager;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
@@ -317,6 +318,31 @@ public class ZkCLITest extends SolrTestCaseJ4 {
       reader.close();
     }
   }
+  
+  @Test
+  public void testUpdateAcls() throws Exception {
+    try {
+      System.setProperty(SolrZkClient.ZK_ACL_PROVIDER_CLASS_NAME_VM_PARAM_NAME, VMParamsAllAndReadonlyDigestZkACLProvider.class.getName());
+      System.setProperty(VMParamsAllAndReadonlyDigestZkACLProvider.DEFAULT_DIGEST_READONLY_USERNAME_VM_PARAM_NAME, "user");
+      System.setProperty(VMParamsAllAndReadonlyDigestZkACLProvider.DEFAULT_DIGEST_READONLY_PASSWORD_VM_PARAM_NAME, "pass");
+
+      String[] args = new String[] {"-zkhost", zkServer.getZkAddress(), "-cmd", "updateacls", "/"};
+      ZkCLI.main(args);
+    } finally {
+      // Need to clear these before we open the next SolrZkClient
+      System.clearProperty(SolrZkClient.ZK_ACL_PROVIDER_CLASS_NAME_VM_PARAM_NAME);
+      System.clearProperty(VMParamsAllAndReadonlyDigestZkACLProvider.DEFAULT_DIGEST_READONLY_USERNAME_VM_PARAM_NAME);
+      System.clearProperty(VMParamsAllAndReadonlyDigestZkACLProvider.DEFAULT_DIGEST_READONLY_PASSWORD_VM_PARAM_NAME);
+    }
+    
+    boolean excepted = false;
+    try (SolrZkClient zkClient = new SolrZkClient(zkServer.getZkAddress(), AbstractDistribZkTestBase.DEFAULT_CONNECTION_TIMEOUT)) {
+      zkClient.getData("/", null, null, true);
+    } catch (KeeperException.NoAuthException e) {
+      excepted = true;
+    }
+    assertTrue("Did not fail to read.", excepted);
+  }
 
   @Override
   public void tearDown() throws Exception {

solr/solrj/src/java/org/apache/solr/common/cloud/SolrZkClient.java

@@ -45,6 +45,7 @@ import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.stream.StreamResult;
 import javax.xml.transform.stream.StreamSource;
+
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
@@ -702,32 +703,22 @@ public class SolrZkClient implements Closeable {
 
   // yeah, it's recursive :(
   public void clean(String path) throws InterruptedException, KeeperException {
-    List<String> children;
-    try {
-      children = getChildren(path, null, true);
-    } catch (NoNodeException r) {
-      return;
-    }
-    for (String string : children) {
-      // we can't clean the built-in zookeeper node
-      if (path.equals("/") && string.equals("zookeeper")) continue;
-      if (path.equals("/")) {
-        clean(path + string);
-      } else {
-        clean(path + "/" + string);
-      }
-    }
-    try {
-      if (!path.equals("/")) {
+    traverseZkTree(path, new ZkVisitor() {
+      @Override
+      public void visit(String znode) throws InterruptedException, KeeperException {
         try {
-          delete(path, -1, true);
-        } catch (NotEmptyException e) {
-          clean(path);
+          if (!znode.equals("/")) {
+            try {
+              delete(znode, -1, true);
+            } catch (NotEmptyException e) {
+              clean(znode);
+            }
+          }
+        } catch (NoNodeException r) {
+          return;
         }
       }
-    } catch (NoNodeException r) {
-      return;
-    }
+    });
   }
   
   /**
@@ -758,4 +749,77 @@ public class SolrZkClient implements Closeable {
   public ZkACLProvider getZkACLProvider() {
     return zkACLProvider;
   }
+
+  /**
+   * Set the ACL on a single node in ZooKeeper. This will replace all existing ACL on that node.
+   *
+   * @param path path to set ACL on e.g. /solr/conf/solrconfig.xml
+   * @param acls a list of {@link ACL} to be applied
+   * @param retryOnConnLoss true if the command should be retried on connection loss
+   */
+  public Stat setACL(final String path, final List<ACL> acls, boolean retryOnConnLoss) throws InterruptedException, KeeperException  {
+    if (retryOnConnLoss) {
+      return zkCmdExecutor.retryOperation(new ZkOperation() {
+        @Override
+        public Stat execute() throws KeeperException, InterruptedException {
+          return keeper.setACL(path, acls, -1);
+        }
+      });
+    } else {
+      return keeper.setACL(path, acls, -1);
+    }
+  }
+
+  /**
+   * Update all ACLs for a zk tree based on our configured {@link ZkACLProvider}.
+   * @param root the root node to recursively update
+   */
+  public void updateACLs(final String root) throws KeeperException, InterruptedException {
+    traverseZkTree(root, new ZkVisitor() {
+      @Override
+      public void visit(String path) throws InterruptedException, KeeperException {
+        try {
+          setACL(path, getZkACLProvider().getACLsToAdd(path), true);
+          log.info("Updated ACL on " + path);
+        } catch (NoNodeException e) {
+          // If a node was deleted, don't bother trying to set ACLs on it.
+          return;
+        }
+      }
+    });
+  }
+
+  private interface ZkVisitor {
+    /**
+     * Visit the target path
+     * @param path the path to visit
+     */
+    void visit(String path) throws InterruptedException, KeeperException;
+  }
+
+  /**
+   * Recursively visit a zk tree rooted at path and apply the given visitor to each path. Exists as a separate method
+   * because some of the logic can get nuanced.
+   *
+   * @param path the path to start from
+   * @param visitor the operation to perform on each path
+   */
+  private void traverseZkTree(final String path, final ZkVisitor visitor) throws InterruptedException, KeeperException {
+    List<String> children;
+    try {
+      children = getChildren(path, null, true);
+    } catch (NoNodeException r) {
+      return;
+    }
+    for (String string : children) {
+      // we can't do anything to the built-in zookeeper node
+      if (path.equals("/") && string.equals("zookeeper")) continue;
+      if (path.equals("/")) {
+        traverseZkTree(path + string, visitor);
+      } else {
+        traverseZkTree(path + "/" + string, visitor);
+      }
+    }
+    visitor.visit(path);
+  }
 }

solr/solrj/src/test/org/apache/solr/common/cloud/SolrZkClientTest.java

@@ -0,0 +1,143 @@
+package org.apache.solr.common.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.lang.invoke.MethodHandles;
+import java.nio.charset.StandardCharsets;
+import java.security.NoSuchAlgorithmException;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.cloud.AbstractZkTestCase;
+import org.apache.solr.cloud.ZkTestServer;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Id;
+import org.apache.zookeeper.server.auth.DigestAuthenticationProvider;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class SolrZkClientTest extends SolrTestCaseJ4 {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+  
+  private static final String ROOT = "/";
+  private static final String PATH = "/collections/collection1";
+  
+  protected ZkTestServer zkServer;
+
+  SolrZkClient aclClient;
+  SolrZkClient credentialsClient;
+  SolrZkClient defaultClient;
+  
+
+  @Override
+  public void setUp() throws Exception {
+    super.setUp();
+    
+    final String SCHEME = "digest";
+    final String AUTH = "user:pass";
+
+    String zkDir = createTempDir().toString();
+    log.info("ZooKeeper dataDir:" + zkDir);
+    zkServer = new ZkTestServer(zkDir);
+    zkServer.run();
+    
+    try (SolrZkClient client = new SolrZkClient(zkServer.getZkHost(), AbstractZkTestCase.TIMEOUT)) {
+      // Set up chroot
+      client.makePath("/solr", false, true);
+    }
+
+    defaultClient = new SolrZkClient(zkServer.getZkAddress(), AbstractZkTestCase.TIMEOUT);
+    defaultClient.makePath(PATH, true);
+    
+    aclClient = new SolrZkClient(zkServer.getZkAddress(), AbstractZkTestCase.TIMEOUT) {
+      @Override
+      protected ZkACLProvider createZkACLProvider() {
+        return new DefaultZkACLProvider() {
+          @Override
+          protected List<ACL> createGlobalACLsToAdd() {
+            try {
+              Id id = new Id(SCHEME, DigestAuthenticationProvider.generateDigest(AUTH));
+              return Collections.singletonList(new ACL(ZooDefs.Perms.ALL, id));
+            } catch (NoSuchAlgorithmException e) {
+              throw new RuntimeException(e);
+            }
+          }
+        };
+      }
+    };
+    
+    credentialsClient = new SolrZkClient(zkServer.getZkAddress(), AbstractZkTestCase.TIMEOUT) {
+      @Override
+      protected ZkCredentialsProvider createZkCredentialsToAddAutomatically() {
+        return new DefaultZkCredentialsProvider() {
+          @Override
+          protected Collection<ZkCredentials> createCredentials() {
+            return Collections.singleton(new ZkCredentials(SCHEME, AUTH.getBytes(StandardCharsets.UTF_8)));
+          }
+        };
+      }
+    };
+  }
+  
+  @Override
+  public void tearDown() throws Exception {
+    aclClient.close();
+    credentialsClient.close();
+    defaultClient.close();
+    zkServer.shutdown();
+    super.tearDown();
+  }
+
+  
+  @Test
+  public void testSimpleUpdateACLs() throws KeeperException, InterruptedException {
+    assertTrue("Initial create was in secure mode; please check the test", canRead(defaultClient, PATH));
+    assertTrue("Credentialed client should always be able to read", canRead(credentialsClient, PATH));
+
+    // convert to secure
+    aclClient.updateACLs(ROOT);
+    assertFalse("Default client should not be able to read root in secure mode", canRead(defaultClient, ROOT));
+    assertFalse("Default client should not be able to read children in secure mode", canRead(defaultClient, PATH));
+    assertTrue("Credentialed client should always be able to read root in secure mode", canRead(credentialsClient, ROOT));
+    assertTrue("Credentialed client should always be able to read in secure mode", canRead(credentialsClient, PATH));
+
+    // convert to non-secure
+    credentialsClient.updateACLs(ROOT);
+    assertTrue("Default client should work again after clearing ACLs", canRead(defaultClient, PATH));
+    assertTrue("Credentialed client should always be able to read", canRead(credentialsClient, PATH));
+
+    // convert a subtree to secure
+    aclClient.updateACLs("/collections");
+    assertTrue("Default client should read unaffected paths", canRead(defaultClient, ROOT));
+    assertFalse("Default client should not read secure children", canRead(defaultClient, PATH));
+  }
+  
+  private static boolean canRead(SolrZkClient zkClient, String path) throws KeeperException, InterruptedException {
+    try {
+      zkClient.getData(path, null, null, true);
+      return true;
+    } catch (KeeperException.NoAuthException e) {
+      return false;
+    }
+  }
+}