[accumulo] branch 2.1 updated: Rework upgrade acl validation (#3109)

[dl...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dlmarion pushed a commit to branch 2.1
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/2.1 by this push:
     new b8b3c33625 Rework upgrade acl validation  (#3109)
b8b3c33625 is described below

commit b8b3c33625db0ef7a0c0edf3cb4dbf429a6bc8cc
Author: Dave Marion <dl...@apache.org>
AuthorDate: Fri Dec 16 12:54:43 2022 -0500

    Rework upgrade acl validation  (#3109)
    
    Rework upgrade acl validation to ensure that accumulo
    has ALL perms on each znode
    
    Related to #3108
---
 .../accumulo/manager/upgrade/Upgrader9to10.java    | 47 ++++++++++++----------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
index c406766445..45b281aa6e 100644
--- a/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
+++ b/server/manager/src/main/java/org/apache/accumulo/manager/upgrade/Upgrader9to10.java
@@ -34,6 +34,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Objects;
+import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.BiConsumer;
 
@@ -53,7 +54,6 @@ import org.apache.accumulo.core.data.TableId;
 import org.apache.accumulo.core.data.Value;
 import org.apache.accumulo.core.dataImpl.KeyExtent;
 import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter;
-import org.apache.accumulo.core.fate.zookeeper.ZooUtil;
 import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy;
 import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeMissingPolicy;
 import org.apache.accumulo.core.file.FileOperations;
@@ -94,7 +94,6 @@ import org.apache.zookeeper.ZKUtil;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
 import org.apache.zookeeper.data.ACL;
-import org.apache.zookeeper.data.Id;
 import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -143,33 +142,38 @@ public class Upgrader9to10 implements Upgrader {
     createScanServerNodes(context);
   }
 
+  private static String extractAuthName(ACL acl) {
+    Objects.requireNonNull(acl, "provided ACL cannot be null");
+    try {
+      return acl.getId().getId().trim().split(":")[0];
+    } catch (Exception ex) {
+      log.debug("Invalid ACL passed, cannot parse id from '{}'", acl);
+      return "";
+    }
+  }
+
+  private static boolean hasAllPermissions(final Set<String> users, final List<ACL> acls) {
+    return acls.stream()
+        .anyMatch(a -> users.contains(extractAuthName(a)) && a.getPerms() == ZooDefs.Perms.ALL);
+  }
+
   private void validateACLs(ServerContext context) {
 
     final AtomicBoolean aclErrorOccurred = new AtomicBoolean(false);
     final ZooReaderWriter zrw = context.getZooReaderWriter();
     final ZooKeeper zk = zrw.getZooKeeper();
     final String rootPath = context.getZooKeeperRoot();
-
-    final Id zkDigest =
-        ZooUtil.getZkDigestAuthId(context.getConfiguration().get(Property.INSTANCE_SECRET));
-    final List<ACL> privateWithAuth = new ArrayList<>();
-    privateWithAuth.add(new ACL(ZooDefs.Perms.ALL, zkDigest));
-    final List<ACL> publicWithAuth = new ArrayList<>(privateWithAuth);
-    publicWithAuth.add(new ACL(ZooDefs.Perms.READ, ZooDefs.Ids.ANYONE_ID_UNSAFE));
+    final Set<String> users = Set.of("accumulo", "anyone");
 
     try {
       ZKUtil.visitSubTreeDFS(zk, rootPath, false, (rc, path, ctx, name) -> {
         try {
-          final Stat stat = new Stat();
-          final List<ACL> acls = zk.getACL(path, stat);
-
-          if (((path.equals(Constants.ZROOT) || path.equals(Constants.ZROOT + Constants.ZINSTANCES))
-              && !acls.equals(ZooDefs.Ids.OPEN_ACL_UNSAFE))
-              || (!privateWithAuth.equals(acls) && !publicWithAuth.equals(acls))) {
-            log.error("ZNode at {} has unexpected ACL: {}", path, acls);
+          final List<ACL> acls = zk.getACL(path, new Stat());
+          if (!hasAllPermissions(users, acls)) {
+            log.error(
+                "ZNode at {} does not have an ACL that allows accumulo to write to it. ZNode ACL will need to be modified. Current ACLs: {}",
+                path, acls);
             aclErrorOccurred.set(true);
-          } else {
-            log.trace("ZNode at {} has expected ACL.", path);
           }
         } catch (KeeperException | InterruptedException e) {
           log.error("Error getting ACL for path: {}", path, e);
@@ -177,9 +181,10 @@ public class Upgrader9to10 implements Upgrader {
         }
       });
       if (aclErrorOccurred.get()) {
-        throw new RuntimeException("Upgrade Failed! Error validating ZNode ACLs. "
-            + "Check the log for specific failed paths, check ZooKeeper troubleshooting in user documentation "
-            + "for instructions on how to fix.");
+        throw new RuntimeException(
+            "Upgrade precondition failed! ACLs will need to be modified for some ZooKeeper nodes. "
+                + "Check the log for specific failed paths, check ZooKeeper troubleshooting in user documentation "
+                + "for instructions on how to fix.");
       }
     } catch (KeeperException | InterruptedException e) {
       throw new RuntimeException("Upgrade Failed! Error validating nodes under " + rootPath, e);