CASSSIDECAR-429: Add RFC 6902-inspired JSON Patch support to ConfigurationManager

CHANGES.txt

@@ -1,5 +1,6 @@
 0.5.0
 -----
+ * Add RFC 6902-inspired JSON Patch support to ConfigurationManager (CASSSIDECAR-429)
  * Wire CDC configs in configs table to SidecarCdcOptions/SidecarStatePersister (CASSSIDECAR-483)
  * Implement durable operational job tracker (CASSSIDECAR-374)
  * Remove filesystem path from Http response (CASSSIDECAR-477)

server/src/main/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlay.java

@@ -39,6 +39,9 @@
  * <p>The {@code extraJvmOpts} field contains JVM options that are appended to the Cassandra JVM startup
  * command. Each entry maps the full option flag (e.g. {@code -Dcassandra.jmx.local.port}) to its value
  * (e.g. {@code 7199}). These are opaque strings not subject to schema validation.
+ *
+ * <p>This class is immutable. Mutations are performed by {@link ConfigurationPatchApplier} which
+ * constructs a new instance with the desired changes.
  */
 public class CassandraConfigurationOverlay
 {
@@ -59,7 +62,7 @@ public CassandraConfigurationOverlay(@Nullable JsonObject cassandraYaml,
 
     /**
      * Returns the cassandra.yaml overlay as a version-agnostic JSON object. Callers must not mutate the
-     * returned object; use {@link #updated} to produce a new overlay with changes applied.
+     * returned object; use {@link ConfigurationPatchApplier} to produce a new overlay with changes applied.
      *
      * @return the cassandra.yaml overlay as a version-agnostic JSON object
      */
@@ -114,59 +117,6 @@ public static CassandraConfigurationOverlay fromJson(@NotNull JsonObject json)
         return new CassandraConfigurationOverlay(cassandraYaml, extraJvmOpts);
     }
 
-    /**
-     * Returns a new overlay with the given updates applied. The current instance is not modified.
-     *
-     * <p>Both parameters follow the same semantics: a {@code null} value for a key removes that entry,
-     * a non-null value upserts it.
-     *
-     * @param cassandraYamlUpdates field-level changes to cassandra.yaml: key = field name, value = new value.
-     *                             A {@code null} value removes the field. Pass {@code null} for no yaml changes.
-     * @param extraJvmOptsUpdates  JVM option changes: key = option name, value = new option value.
-     *                             A {@code null} value removes the option. Pass {@code null} for no changes.
-     * @return a new overlay with the updates applied
-     */
-    @NotNull
-    public CassandraConfigurationOverlay updated(@Nullable Map<String, Object> cassandraYamlUpdates,
-                                                 @Nullable Map<String, String> extraJvmOptsUpdates)
-    {
-        JsonObject mergedYaml = cassandraYaml.copy();
-        if (cassandraYamlUpdates != null)
-        {
-            for (Map.Entry<String, Object> entry : cassandraYamlUpdates.entrySet())
-            {
-                if (entry.getValue() == null)
-                {
-                    mergedYaml.remove(entry.getKey());
-                }
-                else
-                {
-                    mergedYaml.put(entry.getKey(), entry.getValue());
-                }
-            }
-        }
-
-        LinkedHashMap<String, String> mergedOpts = new LinkedHashMap<>(extraJvmOpts);
-        if (extraJvmOptsUpdates != null)
-        {
-            for (Map.Entry<String, String> entry : extraJvmOptsUpdates.entrySet())
-            {
-                if (entry.getValue() == null)
-                {
-                    mergedOpts.remove(entry.getKey());
-                }
-                else
-                {
-                    mergedOpts.put(entry.getKey(), entry.getValue());
-                }
-            }
-        }
-
-        validateNoConflictingBooleanOpts(mergedOpts);
-
-        return new CassandraConfigurationOverlay(mergedYaml, mergedOpts);
-    }
-
     static boolean hasConflictingBooleanOpt(Map<String, String> existing, String key)
     {
         String conflicting = conflictingBooleanOpt(key);
@@ -186,19 +136,6 @@ static String conflictingBooleanOpt(String key)
         return null;
     }
 
-    private static void validateNoConflictingBooleanOpts(Map<String, String> opts)
-    {
-        for (String key : opts.keySet())
-        {
-            if (hasConflictingBooleanOpt(opts, key))
-            {
-                String option = key.substring(5);
-                throw new IllegalArgumentException(
-                    "Conflicting boolean JVM options: -XX:+" + option + " and -XX:-" + option);
-            }
-        }
-    }
-
     @Override
     public boolean equals(Object o)
     {

server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigUtils.java

@@ -24,9 +24,13 @@
 import java.nio.file.Path;
 import java.time.Instant;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import com.fasterxml.jackson.core.JsonParser;
 import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
 import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
@@ -40,6 +44,8 @@
  */
 public final class ConfigUtils
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(ConfigUtils.class);
+
     static final YAMLFactory YAML_FACTORY = new YAMLFactory()
             .disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER);
 
@@ -125,4 +131,36 @@ public static JsonObject mergeConfigurations(@NotNull JsonObject base, @NotNull
         result.mergeIn(overlay, true);
         return result;
     }
+
+    /**
+     * Merges overlay JVM options onto base JVM options, with overlay entries overriding base entries
+     * on key conflict. When an overlay entry conflicts with a base boolean option (e.g. base
+     * {@code -XX:+UseG1GC} and overlay {@code -XX:-UseG1GC}), the base option is preserved and the
+     * overlay entry is skipped, matching {@link ConfigurationOverlaySnapshot#overlay}. Insertion order
+     * is preserved. Neither input map is modified.
+     *
+     * @param base    the base JVM options
+     * @param overlay the overlay JVM options whose values take precedence
+     * @return a new map with the merged result
+     */
+    @NotNull
+    public static Map<String, String> mergeOpts(@NotNull Map<String, String> base,
+                                                @NotNull Map<String, String> overlay)
+    {
+        Objects.requireNonNull(base, "base must not be null");
+        Objects.requireNonNull(overlay, "overlay must not be null");
+        Map<String, String> merged = new LinkedHashMap<>(base);
+        for (Map.Entry<String, String> entry : overlay.entrySet())
+        {
+            if (CassandraConfigurationOverlay.hasConflictingBooleanOpt(merged, entry.getKey()))
+            {
+                LOGGER.warn("Conflicting boolean JVM option '{}' in overlay conflicts with base option '{}'. " +
+                            "Preserving base option and skipping overlay entry.",
+                            entry.getKey(), CassandraConfigurationOverlay.conflictingBooleanOpt(entry.getKey()));
+                continue; // preserve base boolean option, skip conflicting overlay entry
+            }
+            merged.put(entry.getKey(), entry.getValue());
+        }
+        return merged;
+    }
 }

server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationConflictException.java

@@ -0,0 +1,54 @@
+/*
+ * 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.
+ */
+
+package org.apache.cassandra.sidecar.configmanagement;
+
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Thrown when a configuration patch fails due to a hash mismatch,
+ * indicating a concurrent modification to the effective configuration.
+ */
+public class ConfigurationConflictException extends ConfigurationManagerException
+{
+    @NotNull
+    private final String expectedHash;
+
+    @NotNull
+    private final String actualHash;
+
+    public ConfigurationConflictException(@NotNull String expectedHash, @NotNull String actualHash)
+    {
+        super("Configuration conflict: expected hash [" + expectedHash
+              + "] but found [" + actualHash + "]", null);
+        this.expectedHash = expectedHash;
+        this.actualHash = actualHash;
+    }
+
+    @NotNull
+    public String expectedHash()
+    {
+        return expectedHash;
+    }
+
+    @NotNull
+    public String actualHash()
+    {
+        return actualHash;
+    }
+}

server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManager.java

@@ -19,14 +19,20 @@
 package org.apache.cassandra.sidecar.configmanagement;
 
 import java.nio.file.Path;
+import java.time.Instant;
+import java.util.List;
 import java.util.Objects;
 
 import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 /**
- * Manages configuration of Cassandra instances via Sidecar
+ * Manages configuration of Cassandra instances via Sidecar.
+ *
+ * <p>The <em>effective configuration</em> is the result of merging the base cassandra.yaml template
+ * with the instance overlay retrieved from the {@link ConfigurationProvider}. It is what the instance
+ * actually sees; the overlay is only the delta persisted on top of the base template.
  */
 public class ConfigurationManager
 {
@@ -48,8 +54,7 @@ public ConfigurationManager(ConfigurationProvider provider, @Nullable Path baseT
     }
 
     /**
-     * Computes the effective configuration for the given instance by merging the base template
-     * with the overlay from the {@link ConfigurationProvider}.
+     * Computes the effective configuration for the given instance.
      *
      * @param instance the Cassandra instance metadata
      * @return a snapshot of the effective configuration with its SHA-256 hash and last modified timestamp
@@ -84,4 +89,96 @@ private ConfigurationOverlaySnapshot getBaseSnapshot()
         cachedBaseSnapshot = snapshot;
         return snapshot;
     }
+
+    /**
+     * Patches the effective configuration for the given instance using RFC 6902-inspired JSON Patch
+     * operations. Validates the caller's expected hash against the current effective configuration,
+     * validates and applies the patch operations to the overlay, persists via the
+     * {@link ConfigurationProvider}, and returns the new effective configuration.
+     *
+     * <p>Paths reference the effective configuration structure, but mutations target the overlay.
+     * For nested paths within a top-level {@code cassandraYaml} key,
+     * the entire top-level key value is copied from the effective config into the overlay before
+     * applying the leaf change (copy-siblings strategy). This ensures the overlay is always
+     * self-contained at the top-level key granularity. As a consequence, editing a nested leaf pins
+     * its sibling leaves against later base-template drift, and removing an overlaid leaf reverts it to
+     * the current base value; see {@link ConfigurationPatchApplier} for details.
+     *
+     * <p>All operations are validated atomically: either all succeed or none are applied.
+     *
+     * @param instance     the Cassandra instance metadata
+     * @param expectedHash the hash of the effective configuration as last seen by the caller
+     * @param operations   the patch operations to apply
+     * @return a snapshot of the new effective configuration with its SHA-256 hash and last modified timestamp
+     * @throws ConfigurationConflictException if the expectedHash does not match the current effective hash
+     * @throws ConfigurationPatchException    if any patch operation fails validation or application
+     * @throws ConfigurationManagerException  if the provider fails during the operation
+     */
+    @NotNull
+    public ConfigurationOverlaySnapshot patchConfiguration(@NotNull InstanceMetadata instance,
+                                                           @NotNull String expectedHash,
+                                                           @NotNull List<ConfigurationPatchOperation> operations)
+    {
+        Objects.requireNonNull(instance, "instance must not be null");
+        Objects.requireNonNull(expectedHash, "expectedHash must not be null");
+        Objects.requireNonNull(operations, "operations must not be null");
+
+        try
+        {
+            // 1. Validate operations structure (path format, value presence, duplicates)
+            List<ConfigurationPatchValidator.ParsedPatchOperation> parsedOps =
+                    ConfigurationPatchValidator.validate(operations);
+
+            // 2. Read current state and validate the caller's hash matches
+            ConfigurationOverlaySnapshot baseSnapshot = getBaseSnapshot();
+            ConfigurationOverlaySnapshot currentOverlay = provider.getOverlay(instance);
+
+            ConfigurationOverlaySnapshot effectiveConfig = currentOverlay != null
+                                                           ? baseSnapshot.overlay(currentOverlay, instance.id())
+                                                           : baseSnapshot;
+
+            if (!expectedHash.equals(effectiveConfig.hash()))
+            {
+                throw new ConfigurationConflictException(expectedHash, effectiveConfig.hash());
+            }
+
+            // 3. Apply patch operations to the overlay (precondition checks + mutations)
+            CassandraConfigurationOverlay currentOverlayConfig = currentOverlay != null
+                                                                 ? currentOverlay.configuration()
+                                                                 : new CassandraConfigurationOverlay(null, null);
+            CassandraConfigurationOverlay updatedOverlay =
+                    ConfigurationPatchApplier.apply(parsedOps, baseSnapshot.configuration(), currentOverlayConfig);
+            ConfigurationOverlaySnapshot newOverlaySnapshot = new ConfigurationOverlaySnapshot(Instant.now(),
+                                                                                               updatedOverlay);
+
+            // 4. CAS via provider — concurrent patches are resolved by the provider's own atomicity
+            String currentOverlayHash = currentOverlay != null ? currentOverlay.hash() : null;
+            boolean stored = provider.storeOverlay(instance, currentOverlayHash, newOverlaySnapshot);
+
+            // 5. Store rejected — re-read to determine if this is a true conflict or an unexpected failure
+            if (!stored)
+            {
+                ConfigurationOverlaySnapshot updatedCurrent = provider.getOverlay(instance);
+                ConfigurationOverlaySnapshot newEffective = updatedCurrent != null
+                                                            ? baseSnapshot.overlay(updatedCurrent, instance.id())
+                                                            : baseSnapshot;
+                if (!expectedHash.equals(newEffective.hash()))
+                {
+                    throw new ConfigurationConflictException(expectedHash, newEffective.hash());
+                }
+                throw new ConfigurationManagerException(
+                        "Provider rejected the overlay store unexpectedly", null);
+            }
+
+            return baseSnapshot.overlay(newOverlaySnapshot, instance.id());
+        }
+        catch (ConfigurationManagerException e)
+        {
+            throw e;
+        }
+        catch (Exception e)
+        {
+            throw new ConfigurationManagerException("Failed to patch configuration", e);
+        }
+    }
 }