Adds expected file to import config command

server/base/src/main/java/org/apache/accumulo/server/conf/store/PropStore.java

@@ -82,6 +82,21 @@ public interface PropStore {
   void replaceAll(PropStoreKey propStoreKey, long version, Map<String,String> props)
       throws ConcurrentModificationException;
 
+  /**
+   * Replaces all current properties with map provided. If a property is not included in the new
+   * map, the property will not be set.
+   *
+   * @param propStoreKey the prop cache key
+   * @param expected expected current properties
+   * @param props a map of property k,v pairs
+   * @throws IllegalStateException if the values cannot be written or if an underlying store
+   *         exception occurs.
+   * @throws java.util.ConcurrentModificationException if the properties currently in the store do
+   *         not match the expected props passed. No changes are made when this happens.
+   */
+  void replaceAll(PropStoreKey propStoreKey, Map<String,String> expected, Map<String,String> props)
+      throws ConcurrentModificationException;
+
   /**
    * Unconditionally replaces all properties with the map provided. If a property is not included in
    * the new map, the property will not be set.

server/base/src/main/java/org/apache/accumulo/server/conf/store/impl/ZooPropStore.java

@@ -217,6 +217,21 @@ public void replaceAll(@NonNull PropStoreKey propStoreKey, @NonNull Map<String,S
     mutateVersionedProps(propStoreKey, VersionedProperties::replaceAll, props);
   }
 
+  @Override
+  public void replaceAll(@NonNull PropStoreKey propStoreKey, Map<String,String> expected,
+      @NonNull Map<String,String> props) {
+    BiFunction<VersionedProperties,Map<String,String>,VersionedProperties> action =
+        (current, p) -> {
+          if (!current.asMap().equals(expected)) {
+            throw new ConcurrentModificationException(
+                "Current properties do not match expected, key:" + propStoreKey);
+          }
+          return current.replaceAll(p);
+        };
+
+    mutateVersionedProps(propStoreKey, action, props);
+  }
+
   @Override
   public void replaceAll(@NonNull PropStoreKey propStoreKey, long version,
       @NonNull Map<String,String> props) {

server/base/src/main/java/org/apache/accumulo/server/conf/util/ImportConfigCommand.java

@@ -18,8 +18,15 @@
  */
 package org.apache.accumulo.server.conf.util;
 
+import java.io.BufferedInputStream;
+import java.io.IOException;
 import java.io.InputStream;
+import java.io.UncheckedIOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.util.ArrayList;
+import java.util.ConcurrentModificationException;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -44,6 +51,7 @@
 import org.apache.accumulo.start.spi.KeywordExecutable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.Yaml;
 
 import com.beust.jcommander.JCommander;
@@ -77,6 +85,12 @@ public String description() {
   }
 
   public static class Opts extends ServerOpts {
+    @Parameter(names = "--input",
+        description = "Yaml file containing configuration data. If not specified will read from stdin.")
+    public String inputFile;
+    @Parameter(names = "--expected",
+        description = "Yaml file containing expected current config.  Changes are only made if config in zookeeper matches whats in this file.")
+    public String expectedFile;
     @Parameter(names = "--ignore-extra",
         description = "Proceed when Accumulo has extra tables, resource groups, or namespaces that are not in yaml")
     public boolean ignoreExtra = false;
@@ -99,6 +113,9 @@ static PropStoreKey getKey(Scope scope, String name, ServerContext context) {
   }
 
   record ScopeName(Scope scope, String name) {
+    public ScopeName(ScopedProperties sp) {
+      this(sp.scope(), sp.name());
+    }
   }
 
   private static Set<ScopeName> getAllScopeNames(ServerContext context) {
@@ -124,9 +141,15 @@ private static Set<ScopeName> getAllScopeNames(ServerContext context) {
   }
 
   private static void validate(ServerContext serverContext, List<ScopedProperties> allProps,
-      boolean ignoreExtra) {
+      Map<ScopeName,ScopedProperties> expectedProps, boolean ignoreExtra,
+      boolean precheckExpected) {
     var scopeNamesInYaml = new HashSet<ScopeName>();
-    allProps.forEach(sp -> scopeNamesInYaml.add(new ScopeName(sp.scope(), sp.name())));
+    allProps.forEach(sp -> {
+      if (!scopeNamesInYaml.add(new ScopeName(sp))) {
+        throw new IllegalArgumentException(
+            "Duplicate scope+name in input, scope:" + sp.scope() + " name:" + sp.name());
+      }
+    });
     var scopeNamesInAccumulo = getAllScopeNames(serverContext);
 
     if (!scopeNamesInYaml.equals(scopeNamesInAccumulo)) {
@@ -149,29 +172,111 @@ private static void validate(ServerContext serverContext, List<ScopedProperties>
       }
     }
 
+    if (expectedProps != null) {
+      for (var scopedProps : allProps) {
+        var key = new ScopeName(scopedProps.scope(), scopedProps.name());
+        if (!expectedProps.containsKey(key)) {
+          throw new IllegalArgumentException(
+              "Scope+name present in input but not present in expected file, scope:" + key.scope()
+                  + " name:" + key.name());
+        }
+      }
+    }
+
     // validate all scope+name before attempting to update any scope+name
+    var propStore = serverContext.getPropStore();
     for (var scopedProps : allProps) {
       var propStoreKey = getKey(scopedProps.scope(), scopedProps.name(), serverContext);
       PropUtil.validateProperties(serverContext, propStoreKey, scopedProps.props());
+      // precheckExpected is only used in testing, it allows test code to bypass this code and
+      // exercise the expected check done in the atomic zookeeper update.
+      if (expectedProps != null && precheckExpected) {
+        // This check serves two purposes. First it runs during dry-run. Second it avoids changing
+        // anything when a subset does not match the expected. Changes could be made after this
+        // check and before the update. If this happens the update still fail because another check
+        // is done in the atomic zookeeper update, however that could lead to a subset that match
+        // the expected being updated.
+        if (!propStore.get(propStoreKey).asMap()
+            .equals(expectedProps.get(new ScopeName(scopedProps)).props())) {
+          throw new ConcurrentModificationException(
+              createUnexpectedMessage(scopedProps.scope(), scopedProps.name()));
+        }
+      }
+    }
+  }
+
+  private static List<ScopedProperties> read(Yaml yaml, String file, InputStream in) {
+    List<ScopedProperties> allProps = new ArrayList<>();
+    if (file != null) {
+      try (var fileIn = new BufferedInputStream(Files.newInputStream(Path.of(file)))) {
+        for (var obj : yaml.loadAll(fileIn)) {
+          allProps.add(new ScopedProperties((Map<?,?>) obj));
+        }
+      } catch (IOException e) {
+        throw new UncheckedIOException(e);
+      }
+    } else {
+      for (var obj : yaml.loadAll(in)) {
+        allProps.add(new ScopedProperties((Map<?,?>) obj));
+      }
     }
+
+    return allProps;
   }
 
   @VisibleForTesting
   public static void load(ServerContext serverContext, InputStream in, Opts options) {
-    Yaml yaml = new Yaml();
-    List<ScopedProperties> allProps = new ArrayList<>();
-    for (var obj : yaml.loadAll(in)) {
-      allProps.add(new ScopedProperties((Map<?,?>) obj));
+    load(serverContext, in, options, true);
+  }
+
+  private static String createUnexpectedMessage(Scope scope, String name) {
+    return "Properties in scope:" + scope + " name:" + name
+        + " do not match the expected values. To diagnose, export current config to a new file and diff with expected file.";
+  }
+
+  @VisibleForTesting
+  public static void load(ServerContext serverContext, InputStream in, Opts options,
+      boolean precheckExpected) {
+    var loaderOpts = new LoaderOptions();
+    loaderOpts.setAllowDuplicateKeys(false);
+    Yaml yaml = new Yaml(loaderOpts);
+    List<ScopedProperties> allProps = read(yaml, options.inputFile, in);
+
+    Map<ScopeName,ScopedProperties> expectedProps;
+    if (options.expectedFile == null) {
+      expectedProps = null;
+    } else {
+      var grouped = new HashMap<ScopeName,ScopedProperties>();
+      for (var sp : read(yaml, options.expectedFile, null)) {
+        var key = new ScopeName(sp);
+        if (grouped.put(key, sp) != null) {
+          throw new IllegalArgumentException(
+              "Duplicate scope+name in expected file, scope:" + sp.scope() + " name:" + sp.name());
+        }
+      }
+      expectedProps = grouped;
     }
 
-    validate(serverContext, allProps, options.ignoreExtra);
+    validate(serverContext, allProps, expectedProps, options.ignoreExtra, precheckExpected);
 
     if (!options.dryRun) {
       var propStore = serverContext.getPropStore();
 
       for (var sp : allProps) {
         var propStoreKey = getKey(sp.scope(), sp.name(), serverContext);
-        propStore.replaceAll(propStoreKey, sp.props());
+        if (expectedProps == null) {
+          // Unconditionally replace properties
+          propStore.replaceAll(propStoreKey, sp.props());
+        } else {
+          try {
+            // Only replace properties if they match the expected values
+            propStore.replaceAll(propStoreKey, expectedProps.get(new ScopeName(sp)).props(),
+                sp.props());
+          } catch (ConcurrentModificationException cme) {
+            throw new ConcurrentModificationException(
+                createUnexpectedMessage(sp.scope(), sp.name()), cme);
+          }
+        }
       }
     }
   }

test/pom.xml

@@ -231,6 +231,10 @@
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
+    <dependency>
+      <groupId>org.yaml</groupId>
+      <artifactId>snakeyaml</artifactId>
+    </dependency>
     <dependency>
       <groupId>io.opentelemetry.javaagent</groupId>
       <artifactId>opentelemetry-javaagent</artifactId>