diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlay.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlay.java
index ad518b905..0734babbb 100644
--- a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlay.java
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlay.java
@@ -39,6 +39,9 @@
* 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.
+ *
+ *
This class is immutable. Mutations are performed by {@link ConfigurationPatchApplier} which
+ * constructs a new instance with the desired changes.
*/
public class CassandraConfigurationOverlay
{
@@ -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.
- *
- *
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 cassandraYamlUpdates,
- @Nullable Map extraJvmOptsUpdates)
- {
- JsonObject mergedYaml = cassandraYaml.copy();
- if (cassandraYamlUpdates != null)
- {
- for (Map.Entry entry : cassandraYamlUpdates.entrySet())
- {
- if (entry.getValue() == null)
- {
- mergedYaml.remove(entry.getKey());
- }
- else
- {
- mergedYaml.put(entry.getKey(), entry.getValue());
- }
- }
- }
-
- LinkedHashMap mergedOpts = new LinkedHashMap<>(extraJvmOpts);
- if (extraJvmOptsUpdates != null)
- {
- for (Map.Entry 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 existing, String key)
{
String conflicting = conflictingBooleanOpt(key);
@@ -186,19 +136,6 @@ static String conflictingBooleanOpt(String key)
return null;
}
- private static void validateNoConflictingBooleanOpts(Map 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)
{
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationConflictException.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationConflictException.java
new file mode 100644
index 000000000..beeb701ab
--- /dev/null
+++ b/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;
+ }
+}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManager.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManager.java
index 0ae2b6ce8..15600aea0 100644
--- a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManager.java
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManager.java
@@ -19,6 +19,8 @@
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;
@@ -84,4 +86,94 @@ 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.
+ *
+ * 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.
+ *
+ *
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 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 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, effectiveConfig.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);
+ }
+ }
}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplier.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplier.java
new file mode 100644
index 000000000..db57ae9c8
--- /dev/null
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplier.java
@@ -0,0 +1,451 @@
+/*
+ * 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 java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+
+import io.vertx.core.json.JsonObject;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchValidator.CASSANDRA_YAML_SECTION;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchValidator.EXTRA_JVM_OPTS_SECTION;
+
+/**
+ * Applies validated patch operations to the configuration overlay.
+ *
+ * Paths reference the effective configuration (base + overlay merged),
+ * but mutations target the overlay only. 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).
+ *
+ *
All precondition checks are performed before any mutations, ensuring atomicity:
+ * either all operations succeed or none are applied.
+ */
+public final class ConfigurationPatchApplier
+{
+ private ConfigurationPatchApplier()
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * Applies a list of validated patch operations and returns the updated overlay.
+ *
+ * @param parsedOps the validated and parsed operations
+ * @param effectiveConfig the current effective configuration (base + overlay merged)
+ * @param currentOverlay the current overlay (may be empty)
+ * @return the updated overlay with all operations applied
+ * @throws ConfigurationPatchException if any precondition check fails
+ */
+ @NotNull
+ public static CassandraConfigurationOverlay apply(
+ @NotNull List parsedOps,
+ @NotNull CassandraConfigurationOverlay effectiveConfig,
+ @NotNull CassandraConfigurationOverlay currentOverlay)
+ {
+ // Phase 1: Precondition checks (no mutations)
+ for (ConfigurationPatchValidator.ParsedPatchOperation parsed : parsedOps)
+ {
+ checkPreconditions(parsed, effectiveConfig, currentOverlay);
+ }
+
+ // Phase 2: Apply mutations
+ JsonObject updatedYaml = currentOverlay.cassandraYaml().copy();
+ Map updatedOpts = new LinkedHashMap<>(currentOverlay.extraJvmOpts());
+
+ for (ConfigurationPatchValidator.ParsedPatchOperation parsed : parsedOps)
+ {
+ applyMutation(parsed, effectiveConfig, updatedYaml, updatedOpts);
+ }
+
+ return new CassandraConfigurationOverlay(updatedYaml, updatedOpts);
+ }
+
+ private static void checkPreconditions(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ CassandraConfigurationOverlay effectiveConfig,
+ CassandraConfigurationOverlay currentOverlay)
+ {
+ if (parsed.section().equals(CASSANDRA_YAML_SECTION))
+ {
+ checkYamlPreconditions(parsed, effectiveConfig.cassandraYaml(), currentOverlay.cassandraYaml());
+ }
+ else if (parsed.section().equals(EXTRA_JVM_OPTS_SECTION))
+ {
+ checkJvmOptsPreconditions(parsed, effectiveConfig.extraJvmOpts(), currentOverlay.extraJvmOpts());
+ }
+ }
+
+ private static void checkYamlPreconditions(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ JsonObject effectiveYaml,
+ JsonObject overlayYaml)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+
+ switch (op.op())
+ {
+ case TEST:
+ checkNestedPathTraversable(effectiveYaml, parsed);
+ Object actualValue = resolveValue(effectiveYaml, parsed.topLevelKey(), parsed.nestedSegments());
+ if (actualValue == null)
+ {
+ throw new ConfigurationPatchException(
+ "Test failed: path does not exist in effective config: '" + op.path() + "'", op);
+ }
+ if (!valueEquals(actualValue, op.value()))
+ {
+ throw new ConfigurationPatchException(
+ "Test failed: expected " + op.value() + " but found " + actualValue
+ + " at path '" + op.path() + "'", op);
+ }
+ break;
+
+ case REPLACE:
+ checkNestedPathTraversable(effectiveYaml, parsed);
+ Object existingValue = resolveValue(effectiveYaml, parsed.topLevelKey(), parsed.nestedSegments());
+ if (existingValue == null)
+ {
+ throw new ConfigurationPatchException(
+ "Replace failed: path does not exist in effective config: '" + op.path() + "'", op);
+ }
+ break;
+
+ case REMOVE:
+ checkNestedPathTraversable(overlayYaml, parsed);
+ Object overlayValue = resolveValue(overlayYaml, parsed.topLevelKey(), parsed.nestedSegments());
+ if (overlayValue == null)
+ {
+ throw new ConfigurationPatchException(
+ "Remove failed: path does not exist in overlay (may only exist in base template): '"
+ + op.path() + "'", op);
+ }
+ break;
+
+ case ADD:
+ if (parsed.isNested())
+ {
+ // Parent must exist in effective config (RFC 6902)
+ List parentSegments = parsed.nestedSegments()
+ .subList(0, parsed.nestedSegments().size() - 1);
+ checkNestedPathTraversable(effectiveYaml, parsed);
+ Object parent = resolveValue(effectiveYaml, parsed.topLevelKey(), parentSegments);
+ if (parent == null && !parentSegments.isEmpty())
+ {
+ throw new ConfigurationPatchException(
+ "Add failed: parent path does not exist in effective config: '" + op.path() + "'", op);
+ }
+ if (parent != null && !(parent instanceof JsonObject) && !(parent instanceof Map))
+ {
+ throw new ConfigurationPatchException(
+ "Add failed: parent is not an object at path: '" + op.path() + "'", op);
+ }
+ }
+ break;
+ }
+ }
+
+ /**
+ * Validates that the nested path can be traversed without hitting a non-object (e.g., an array)
+ * at an intermediate segment. Throws a clear error if a segment resolves to a value that
+ * cannot be traversed further.
+ */
+ private static void checkNestedPathTraversable(JsonObject yaml,
+ ConfigurationPatchValidator.ParsedPatchOperation parsed)
+ {
+ if (!parsed.isNested())
+ {
+ return;
+ }
+ ConfigurationPatchOperation op = parsed.operation();
+ Object current = yaml.getValue(parsed.topLevelKey());
+ if (current == null)
+ {
+ return; // will be caught by subsequent resolveValue null check
+ }
+
+ for (int i = 0; i < parsed.nestedSegments().size() - 1; i++)
+ {
+ JsonObject currentObj = asJsonObject(current);
+ if (currentObj == null)
+ {
+ throw new ConfigurationPatchException(
+ "Path traversal failed: intermediate value is not an object at path: '"
+ + op.path() + "'", op);
+ }
+ current = currentObj.getValue(parsed.nestedSegments().get(i));
+ if (current == null)
+ {
+ return; // will be caught by subsequent resolveValue null check
+ }
+ }
+ }
+
+ private static void checkJvmOptsPreconditions(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ Map effectiveOpts,
+ Map overlayOpts)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+ String key = parsed.topLevelKey();
+
+ switch (op.op())
+ {
+ case TEST:
+ if (!effectiveOpts.containsKey(key))
+ {
+ throw new ConfigurationPatchException(
+ "Test failed: key does not exist in effective extraJvmOpts: '" + op.path() + "'", op);
+ }
+ String actual = effectiveOpts.get(key);
+ if (!Objects.equals(actual, op.value()))
+ {
+ throw new ConfigurationPatchException(
+ "Test failed: expected '" + op.value() + "' but found '" + actual
+ + "' at path '" + op.path() + "'", op);
+ }
+ break;
+
+ case REPLACE:
+ if (!effectiveOpts.containsKey(key))
+ {
+ throw new ConfigurationPatchException(
+ "Replace failed: key does not exist in effective extraJvmOpts: '" + op.path() + "'", op);
+ }
+ break;
+
+ case REMOVE:
+ if (!overlayOpts.containsKey(key))
+ {
+ throw new ConfigurationPatchException(
+ "Remove failed: key does not exist in overlay extraJvmOpts "
+ + "(may only exist in base template): '" + op.path() + "'", op);
+ }
+ break;
+
+ case ADD:
+ // No precondition — extraJvmOpts is flat, parent always exists
+ break;
+ }
+ }
+
+ private static void applyMutation(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ CassandraConfigurationOverlay effectiveConfig,
+ JsonObject updatedYaml,
+ Map updatedOpts)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+
+ if (op.op() == ConfigurationPatchOperation.Op.TEST)
+ {
+ return; // test is read-only
+ }
+
+ if (parsed.section().equals(CASSANDRA_YAML_SECTION))
+ {
+ applyYamlMutation(parsed, effectiveConfig.cassandraYaml(), updatedYaml);
+ }
+ else if (parsed.section().equals(EXTRA_JVM_OPTS_SECTION))
+ {
+ applyJvmOptsMutation(parsed, updatedOpts);
+ }
+ }
+
+ private static void applyYamlMutation(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ JsonObject effectiveYaml,
+ JsonObject updatedYaml)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+
+ if (!parsed.isNested())
+ {
+ // Top-level key: direct put/remove on overlay yaml
+ if (op.op() == ConfigurationPatchOperation.Op.REMOVE)
+ {
+ updatedYaml.remove(parsed.topLevelKey());
+ }
+ else
+ {
+ updatedYaml.put(parsed.topLevelKey(), op.value());
+ }
+ }
+ else if (op.op() == ConfigurationPatchOperation.Op.REMOVE)
+ {
+ // Nested remove: modify the existing overlay subtree in-place.
+ // The precondition check already verified the leaf exists in the overlay.
+ // updatedYaml is already a deep copy so in-place mutation is safe.
+ JsonObject topLevel = asJsonObject(updatedYaml.getValue(parsed.topLevelKey()));
+ if (topLevel != null)
+ {
+ removeAtPath(topLevel, parsed.nestedSegments());
+ }
+ }
+ else
+ {
+ // Nested add/replace: copy-siblings strategy.
+ // If a prior op in this batch already copied the top-level key into the overlay,
+ // operate on that copy. Otherwise, copy from effective config to ensure the overlay
+ // is self-contained at the top-level key granularity.
+ JsonObject topLevel;
+ if (updatedYaml.containsKey(parsed.topLevelKey()))
+ {
+ topLevel = asJsonObject(updatedYaml.getValue(parsed.topLevelKey()));
+ }
+ else
+ {
+ topLevel = deepCopyValue(effectiveYaml.getValue(parsed.topLevelKey()));
+ }
+ if (topLevel == null)
+ {
+ topLevel = new JsonObject();
+ }
+ setAtPath(topLevel, parsed.nestedSegments(), op.value());
+ updatedYaml.put(parsed.topLevelKey(), topLevel);
+ }
+ }
+
+ private static void applyJvmOptsMutation(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ Map updatedOpts)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+ String key = parsed.topLevelKey();
+
+ if (op.op() == ConfigurationPatchOperation.Op.REMOVE)
+ {
+ updatedOpts.remove(key);
+ }
+ else
+ {
+ if (CassandraConfigurationOverlay.hasConflictingBooleanOpt(updatedOpts, key))
+ {
+ String conflicting = CassandraConfigurationOverlay.conflictingBooleanOpt(key);
+ throw new ConfigurationPatchException(
+ "Conflicting boolean JVM option: '" + key + "' conflicts with existing '"
+ + conflicting + "'", op);
+ }
+ updatedOpts.put(key, String.valueOf(op.value()));
+ }
+ }
+
+ /**
+ * Resolves a value at the given path within a JsonObject.
+ *
+ * @param root the root object to traverse
+ * @param topLevelKey the first key to look up
+ * @param nestedSegments remaining path segments after the top-level key
+ * @return the value at the path, or {@code null} if any segment is missing
+ */
+ @Nullable
+ @SuppressWarnings("unchecked")
+ static Object resolveValue(JsonObject root, String topLevelKey, List nestedSegments)
+ {
+ Object current = root.getValue(topLevelKey);
+ if (current == null)
+ {
+ return null;
+ }
+
+ for (String segment : nestedSegments)
+ {
+ JsonObject obj = asJsonObject(current);
+ if (obj == null)
+ {
+ return null;
+ }
+ current = obj.getValue(segment);
+ if (current == null)
+ {
+ return null;
+ }
+ }
+ return current;
+ }
+
+ private static void setAtPath(JsonObject root, List segments, Object value)
+ {
+ JsonObject parent = root;
+ for (int i = 0; i < segments.size() - 1; i++)
+ {
+ Object child = parent.getValue(segments.get(i));
+ JsonObject childObj = asJsonObject(child);
+ if (childObj == null)
+ {
+ childObj = new JsonObject();
+ parent.put(segments.get(i), childObj);
+ }
+ parent = childObj;
+ }
+ parent.put(segments.get(segments.size() - 1), value);
+ }
+
+ private static void removeAtPath(JsonObject root, List segments)
+ {
+ JsonObject parent = root;
+ for (int i = 0; i < segments.size() - 1; i++)
+ {
+ Object child = parent.getValue(segments.get(i));
+ JsonObject childObj = asJsonObject(child);
+ if (childObj == null)
+ {
+ return;
+ }
+ parent = childObj;
+ }
+ parent.remove(segments.get(segments.size() - 1));
+ }
+
+ @SuppressWarnings("unchecked")
+ @Nullable
+ private static JsonObject asJsonObject(@Nullable Object value)
+ {
+ if (value instanceof JsonObject)
+ {
+ return (JsonObject) value;
+ }
+ if (value instanceof Map)
+ {
+ return new JsonObject((Map) value);
+ }
+ return null;
+ }
+
+ @SuppressWarnings("unchecked")
+ private static JsonObject deepCopyValue(@Nullable Object value)
+ {
+ if (value instanceof JsonObject)
+ {
+ return ((JsonObject) value).copy();
+ }
+ if (value instanceof Map)
+ {
+ return new JsonObject((Map) value).copy();
+ }
+ return new JsonObject();
+ }
+
+ private static boolean valueEquals(@NotNull Object actual, @Nullable Object expected)
+ {
+ if (actual instanceof JsonObject && expected instanceof Map)
+ {
+ return actual.equals(new JsonObject((Map) expected));
+ }
+ return Objects.equals(actual, expected);
+ }
+}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchException.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchException.java
new file mode 100644
index 000000000..6a234ba9e
--- /dev/null
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchException.java
@@ -0,0 +1,45 @@
+/*
+ * 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;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Thrown when a configuration patch operation fails validation or application.
+ * Contains the failing operation for error reporting.
+ */
+public class ConfigurationPatchException extends ConfigurationManagerException
+{
+ @Nullable
+ private final ConfigurationPatchOperation failedOperation;
+
+ public ConfigurationPatchException(@NotNull String message,
+ @Nullable ConfigurationPatchOperation failedOperation)
+ {
+ super(message, null);
+ this.failedOperation = failedOperation;
+ }
+
+ @Nullable
+ public ConfigurationPatchOperation failedOperation()
+ {
+ return failedOperation;
+ }
+}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchOperation.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchOperation.java
new file mode 100644
index 000000000..581abb600
--- /dev/null
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchOperation.java
@@ -0,0 +1,117 @@
+/*
+ * 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 java.util.Objects;
+
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Represents a single JSON Patch operation (RFC 6902-inspired) for configuration updates.
+ * Supported operations: {@code add}, {@code remove}, {@code replace}, {@code test}.
+ *
+ * Paths use JSON Pointer syntax (RFC 6901) and reference the effective configuration structure.
+ * Operations are interpreted as overlay mutations:
+ *
+ * - {@code add} — upsert value in overlay; parent must exist in effective config
+ * - {@code remove} — remove from overlay; fails if key only exists in base template
+ * - {@code replace} — set value in overlay; fails if key absent from effective config
+ * - {@code test} — assert effective config value matches; no mutation
+ *
+ */
+public class ConfigurationPatchOperation
+{
+ /**
+ * The supported patch operations.
+ */
+ public enum Op
+ {
+ ADD,
+ REMOVE,
+ REPLACE,
+ TEST
+ }
+
+ @NotNull
+ private final Op op;
+
+ @NotNull
+ private final String path;
+
+ @Nullable
+ private final Object value;
+
+ /**
+ * @param op the operation type
+ * @param path the JSON Pointer path (e.g. "/configuration/cassandraYaml/concurrent_reads")
+ * @param value the value for add/replace/test operations; must be {@code null} for remove
+ */
+ public ConfigurationPatchOperation(@NotNull Op op, @NotNull String path, @Nullable Object value)
+ {
+ this.op = Objects.requireNonNull(op, "op must not be null");
+ this.path = Objects.requireNonNull(path, "path must not be null");
+ this.value = value;
+ }
+
+ @NotNull
+ public Op op()
+ {
+ return op;
+ }
+
+ @NotNull
+ public String path()
+ {
+ return path;
+ }
+
+ @Nullable
+ public Object value()
+ {
+ return value;
+ }
+
+ @Override
+ public String toString()
+ {
+ return "ConfigurationPatchOperation{op=" + op + ", path='" + path + "', value=" + value + '}';
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o)
+ {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass())
+ {
+ return false;
+ }
+ ConfigurationPatchOperation that = (ConfigurationPatchOperation) o;
+ return op == that.op && Objects.equals(path, that.path) && Objects.equals(value, that.value);
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(op, path, value);
+ }
+}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidator.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidator.java
new file mode 100644
index 000000000..d6802f2ab
--- /dev/null
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidator.java
@@ -0,0 +1,283 @@
+/*
+ * 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 java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import org.jetbrains.annotations.NotNull;
+
+/**
+ * Validates a list of {@link ConfigurationPatchOperation} instances before application.
+ *
+ * Validation includes:
+ *
+ * - Path format: must start with {@code /configuration/cassandraYaml/} or
+ * {@code /configuration/extraJvmOpts/}
+ * - Value presence: required for add/replace/test, must be absent for remove
+ * - Duplicate path detection: multiple mutation ops targeting the same path are rejected
+ * - Path segments split by {@code /}; empty segments are rejected
+ *
+ */
+public final class ConfigurationPatchValidator
+{
+ static final String PATH_PREFIX = "/configuration/";
+ static final String CASSANDRA_YAML_SECTION = "cassandraYaml";
+ static final String EXTRA_JVM_OPTS_SECTION = "extraJvmOpts";
+ static final String CASSANDRA_YAML_PREFIX = PATH_PREFIX + CASSANDRA_YAML_SECTION + "/";
+ static final String EXTRA_JVM_OPTS_PREFIX = PATH_PREFIX + EXTRA_JVM_OPTS_SECTION + "/";
+
+ // Allows: -Dproperty.name, -Xmx, -Xss, -XX:+Flag, -XX:-Flag, -XX:Flag
+ // Rejects: -javaagent, -agentpath, -agentlib, keys with = or shell metacharacters
+ static final Pattern JVM_OPT_KEY_PATTERN = Pattern.compile(
+ "^-(D[a-zA-Z][a-zA-Z0-9._-]*|X[a-z][a-zA-Z0-9]*|XX:[+-]?[a-zA-Z][a-zA-Z0-9_]*)$");
+
+ // XX flags that accept arbitrary commands as values
+ static final Set BLOCKED_XX_FLAGS = Set.of(
+ "-XX:OnOutOfMemoryError",
+ "-XX:OnError");
+
+ // Allows: alphanumeric, dots, colons, slashes, @, +, commas, spaces, hyphens (max 512 chars)
+ // Rejects: shell metacharacters (;|&$`), quotes, newlines, and other control characters
+ static final Pattern JVM_OPT_VALUE_PATTERN = Pattern.compile("^[a-zA-Z0-9._:/@+, -]{0,512}$");
+
+ // Matches /../ path traversal sequences (start, middle, or end of path)
+ static final Pattern PATH_TRAVERSAL_PATTERN = Pattern.compile("(?:^|/)\\.\\.(?:/|$)");
+
+ private ConfigurationPatchValidator()
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * Validates a list of patch operations and returns their parsed representations.
+ *
+ * @param operations the raw patch operations to validate
+ * @return the list of parsed operations with extracted section, top-level key, and nested segments
+ * @throws ConfigurationPatchException if any validation check fails
+ */
+ @NotNull
+ public static List validate(@NotNull List operations)
+ {
+ if (operations.isEmpty())
+ {
+ throw new ConfigurationPatchException("Patch operations list must not be empty", null);
+ }
+
+ Set seenMutationPaths = new HashSet<>();
+ List parsed = new ArrayList<>(operations.size());
+
+ for (ConfigurationPatchOperation op : operations)
+ {
+ validateValuePresence(op);
+ ParsedPatchOperation parsedOp = parsePath(op);
+ // Only mutation ops (add/remove/replace) are checked for duplicates.
+ // test is read-only and may target the same path as a subsequent mutation.
+ if (op.op() != ConfigurationPatchOperation.Op.TEST && !seenMutationPaths.add(op.path()))
+ {
+ throw new ConfigurationPatchException(
+ "Duplicate path in patch: '" + op.path() + "'", op);
+ }
+ parsed.add(parsedOp);
+ }
+ return parsed;
+ }
+
+ private static void validateValuePresence(ConfigurationPatchOperation op)
+ {
+ switch (op.op())
+ {
+ case ADD:
+ case REPLACE:
+ case TEST:
+ if (op.value() == null)
+ {
+ throw new ConfigurationPatchException(
+ "Operation '" + op.op() + "' requires a value", op);
+ }
+ break;
+ case REMOVE:
+ if (op.value() != null)
+ {
+ throw new ConfigurationPatchException(
+ "Operation 'REMOVE' must not have a value", op);
+ }
+ break;
+ }
+ }
+
+ private static ParsedPatchOperation parsePath(ConfigurationPatchOperation op)
+ {
+ String path = op.path();
+
+ if (path.startsWith(CASSANDRA_YAML_PREFIX))
+ {
+ String remaining = path.substring(CASSANDRA_YAML_PREFIX.length());
+ List segments = parsePointerSegments(remaining, op);
+ if (segments.isEmpty())
+ {
+ throw new ConfigurationPatchException(
+ "Path must specify at least one key after section: '" + path + "'", op);
+ }
+ String topLevelKey = segments.get(0);
+ List nestedSegments = segments.size() > 1 ? segments.subList(1, segments.size())
+ : List.of();
+ return new ParsedPatchOperation(op, CASSANDRA_YAML_SECTION, topLevelKey, nestedSegments);
+ }
+ else if (path.startsWith(EXTRA_JVM_OPTS_PREFIX))
+ {
+ String remaining = path.substring(EXTRA_JVM_OPTS_PREFIX.length());
+ List segments = parsePointerSegments(remaining, op);
+ if (segments.isEmpty())
+ {
+ throw new ConfigurationPatchException(
+ "Path must specify at least one key after section: '" + path + "'", op);
+ }
+ if (segments.size() > 1)
+ {
+ throw new ConfigurationPatchException(
+ "extraJvmOpts paths must be flat (no nested segments): '" + path + "'", op);
+ }
+ String jvmOptKey = segments.get(0);
+ validateJvmOptKey(jvmOptKey, op);
+ if (op.value() != null)
+ {
+ validateJvmOptValue(String.valueOf(op.value()), op);
+ }
+ return new ParsedPatchOperation(op, EXTRA_JVM_OPTS_SECTION, jvmOptKey, List.of());
+ }
+ else
+ {
+ throw new ConfigurationPatchException(
+ "Path must start with '/configuration/cassandraYaml/' or "
+ + "'/configuration/extraJvmOpts/': '" + path + "'", op);
+ }
+ }
+
+ private static void validateJvmOptKey(String key, ConfigurationPatchOperation op)
+ {
+ if (!JVM_OPT_KEY_PATTERN.matcher(key).matches())
+ {
+ throw new ConfigurationPatchException(
+ "Invalid JVM option key '" + key + "': must be a valid JVM option "
+ + "(-Dproperty.name, -Xflag, or -XX:[+-]Flag)", op);
+ }
+ if (BLOCKED_XX_FLAGS.contains(key))
+ {
+ throw new ConfigurationPatchException(
+ "Blocked extraJvmOpts key '" + key + "': this JVM option is not allowed", op);
+ }
+ }
+
+ private static void validateJvmOptValue(String value, ConfigurationPatchOperation op)
+ {
+ if (!JVM_OPT_VALUE_PATTERN.matcher(value).matches())
+ {
+ throw new ConfigurationPatchException(
+ "Invalid extraJvmOpts value: contains disallowed characters or exceeds 512 characters", op);
+ }
+ if (PATH_TRAVERSAL_PATTERN.matcher(value).find())
+ {
+ throw new ConfigurationPatchException(
+ "Invalid extraJvmOpts value: path traversal ('..') is not allowed", op);
+ }
+ }
+
+ /**
+ * Splits a path remainder into segments by {@code /}.
+ */
+ static List parsePointerSegments(String pointer, ConfigurationPatchOperation op)
+ {
+ if (pointer.isEmpty())
+ {
+ return List.of();
+ }
+
+ String[] parts = pointer.split("/", -1);
+ List segments = new ArrayList<>(parts.length);
+ for (String part : parts)
+ {
+ if (part.isEmpty())
+ {
+ throw new ConfigurationPatchException(
+ "Path contains empty segment: '" + op.path() + "'", op);
+ }
+ segments.add(part);
+ }
+ return segments;
+ }
+
+ /**
+ * A validated and parsed patch operation with its path decomposed into section, top-level key,
+ * and optional nested segments.
+ */
+ public static class ParsedPatchOperation
+ {
+ private final ConfigurationPatchOperation operation;
+ private final String section;
+ private final String topLevelKey;
+ private final List nestedSegments;
+
+ ParsedPatchOperation(@NotNull ConfigurationPatchOperation operation,
+ @NotNull String section,
+ @NotNull String topLevelKey,
+ @NotNull List nestedSegments)
+ {
+ this.operation = operation;
+ this.section = section;
+ this.topLevelKey = topLevelKey;
+ this.nestedSegments = nestedSegments;
+ }
+
+ @NotNull
+ public ConfigurationPatchOperation operation()
+ {
+ return operation;
+ }
+
+ /** Either "cassandraYaml" or "extraJvmOpts" */
+ @NotNull
+ public String section()
+ {
+ return section;
+ }
+
+ /** The first path segment after the section (e.g. "memtable", "concurrent_reads") */
+ @NotNull
+ public String topLevelKey()
+ {
+ return topLevelKey;
+ }
+
+ /** Segments after the top-level key; empty for flat paths */
+ @NotNull
+ public List nestedSegments()
+ {
+ return nestedSegments;
+ }
+
+ public boolean isNested()
+ {
+ return !nestedSegments.isEmpty();
+ }
+ }
+}
diff --git a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationProvider.java b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationProvider.java
index 86fcb3b81..e639eef7c 100644
--- a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationProvider.java
+++ b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationProvider.java
@@ -31,7 +31,7 @@
*
* The provider stores version-agnostic overlays and does not perform version-specific
* validation or merge logic. Validation against a version-aware schema and computing updated
- * overlays (via {@link CassandraConfigurationOverlay#updated}) are the responsibility of the
+ * overlays (via {@link ConfigurationPatchApplier}) are the responsibility of the
* Configuration Manager.
*/
public interface ConfigurationProvider
@@ -50,8 +50,8 @@ public interface ConfigurationProvider
* subject to hash-based optimistic concurrency control.
*
*
The caller is responsible for computing the new snapshot (via
- * {@link CassandraConfigurationOverlay#updated}). The provider only validates
- * the original hash against the currently stored version and persists the result.
+ * {@link ConfigurationPatchApplier}). The provider only validates the original
+ * hash against the currently stored version and persists the result.
*
* @param instance the Cassandra instance metadata
* @param originalHash the overlay hash from the previously read snapshot,
diff --git a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlayTest.java b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlayTest.java
index 26f9f886d..450f5b5a0 100644
--- a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlayTest.java
+++ b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/CassandraConfigurationOverlayTest.java
@@ -18,8 +18,6 @@
package org.apache.cassandra.sidecar.configmanagement;
-import java.util.Collections;
-import java.util.LinkedHashMap;
import java.util.Map;
import org.junit.jupiter.api.Test;
@@ -27,7 +25,6 @@
import io.vertx.core.json.JsonObject;
import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatThrownBy;
/**
* Tests for {@link CassandraConfigurationOverlay}
@@ -35,117 +32,40 @@
class CassandraConfigurationOverlayTest
{
@Test
- void testUpdatedAppliesCassandraYamlChanges()
+ void testConstructorDeepCopiesCassandraYaml()
{
- JsonObject yaml = new JsonObject()
- .put("concurrent_reads", 32)
- .put("memtable_flush_writers", 4)
- .put("storage_compatibility_mode", "CASSANDRA_4");
+ JsonObject yaml = new JsonObject().put("concurrent_reads", 32);
CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(yaml, null);
- Map updates = new LinkedHashMap<>();
- updates.put("concurrent_reads", 64);
- updates.put("storage_compatibility_mode", null);
-
- CassandraConfigurationOverlay updated = overlay.updated(updates, null);
-
- // concurrent_reads updated
- assertThat(updated.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(64);
- // storage_compatibility_mode removed
- assertThat(updated.cassandraYaml().containsKey("storage_compatibility_mode")).isFalse();
- // memtable_flush_writers preserved
- assertThat(updated.cassandraYaml().getInteger("memtable_flush_writers")).isEqualTo(4);
- }
-
- @Test
- void testUpdatedUpsertsAndRemovesJvmOpts()
- {
- Map jvmOpts = new LinkedHashMap<>();
- jvmOpts.put("-Dcassandra.jmx.local.port", "7199");
- jvmOpts.put("-Xmx", "4G");
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null, jvmOpts);
-
- Map updates = new LinkedHashMap<>();
- updates.put("-Xmx", "8G");
-
- CassandraConfigurationOverlay updated = overlay.updated(null, updates);
-
- Map expected = new LinkedHashMap<>();
- expected.put("-Dcassandra.jmx.local.port", "7199");
- expected.put("-Xmx", "8G");
- assertThat(updated.extraJvmOpts()).containsExactlyEntriesOf(expected);
- }
-
- @Test
- void testUpdatedRemovesJvmOptWithNullValue()
- {
- Map jvmOpts = new LinkedHashMap<>();
- jvmOpts.put("-Dcassandra.jmx.local.port", "7199");
- jvmOpts.put("-Xmx", "4G");
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null, jvmOpts);
-
- Map updates = new LinkedHashMap<>();
- updates.put("-Xmx", null);
-
- CassandraConfigurationOverlay updated = overlay.updated(null, updates);
-
- assertThat(updated.extraJvmOpts()).containsExactlyEntriesOf(Map.of(
- "-Dcassandra.jmx.local.port", "7199"));
- }
-
- @Test
- void testUpdatedRejectsConflictingBooleanOpts()
- {
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null, Map.of("-XX:+UseG1GC", ""));
-
- Map updates = new LinkedHashMap<>();
- updates.put("-XX:-UseG1GC", "");
-
- assertThatThrownBy(() -> overlay.updated(null, updates))
- .isInstanceOf(IllegalArgumentException.class)
- .hasMessageContaining("-XX:+UseG1GC")
- .hasMessageContaining("-XX:-UseG1GC");
- }
-
- @Test
- void testUpdatedAllowsReplacingBooleanOpt()
- {
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null, Map.of("-XX:+UseG1GC", ""));
-
- Map updates = new LinkedHashMap<>();
- updates.put("-XX:+UseG1GC", null);
- updates.put("-XX:-UseG1GC", "");
-
- CassandraConfigurationOverlay updated = overlay.updated(null, updates);
+ yaml.put("concurrent_reads", 64);
- assertThat(updated.extraJvmOpts()).containsExactlyEntriesOf(Map.of("-XX:-UseG1GC", ""));
+ assertThat(overlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(32);
}
@Test
- void testConstructorDeepCopiesCassandraYaml()
+ void testFromJsonRoundTrip()
{
- JsonObject yaml = new JsonObject().put("concurrent_reads", 32);
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(yaml, null);
+ JsonObject yaml = new JsonObject()
+ .put("concurrent_reads", 32)
+ .put("commitlog_sync", "periodic");
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(yaml, Map.of("-Xmx", "4G"));
- yaml.put("concurrent_reads", 64);
+ CassandraConfigurationOverlay fromJson = CassandraConfigurationOverlay.fromJson(overlay.toJson());
- assertThat(overlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(32);
+ assertThat(fromJson.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(32);
+ assertThat(fromJson.cassandraYaml().getString("commitlog_sync")).isEqualTo("periodic");
+ assertThat(fromJson.extraJvmOpts()).containsEntry("-Xmx", "4G");
}
@Test
- void testUpdatedReturnsNewInstance()
+ void testEqualsAndHashCode()
{
JsonObject yaml = new JsonObject().put("concurrent_reads", 32);
- CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(yaml, Map.of("-Xmx", "4G"));
+ CassandraConfigurationOverlay a = new CassandraConfigurationOverlay(yaml, Map.of("-Xmx", "4G"));
+ CassandraConfigurationOverlay b = new CassandraConfigurationOverlay(yaml.copy(), Map.of("-Xmx", "4G"));
- CassandraConfigurationOverlay updated = overlay.updated(
- Collections.singletonMap("concurrent_reads", 64),
- null);
-
- assertThat(updated).isNotSameAs(overlay);
- assertThat(updated.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(64);
- // Original is not modified by updated() — deep copy used internally
- assertThat(overlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(32);
+ assertThat(a).isEqualTo(b);
+ assertThat(a.hashCode()).isEqualTo(b.hashCode());
}
@Test
diff --git a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManagerTest.java b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManagerTest.java
index 49080915a..96137f0fa 100644
--- a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManagerTest.java
+++ b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationManagerTest.java
@@ -23,15 +23,23 @@
import java.nio.file.Path;
import java.nio.file.Paths;
import java.time.Instant;
+import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedHashMap;
+import java.util.List;
import java.util.Map;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import io.vertx.core.json.JsonObject;
import org.apache.cassandra.sidecar.cluster.instance.InstanceMetadata;
+import org.jetbrains.annotations.NotNull;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -173,6 +181,406 @@ void testGetEffectiveConfigurationCachesBaseSnapshot()
assertThat(second).isSameAs(first);
}
+ @Test
+ void testPatchAddTopLevelKey()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+
+ ConfigurationOverlaySnapshot baseEffective = manager.getEffectiveConfiguration(instance);
+ String baseHash = baseEffective.hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 64));
+
+ ConfigurationOverlaySnapshot result = manager.patchConfiguration(instance, baseHash, ops);
+
+ assertThat(result.configuration().cassandraYaml().getInteger("concurrent_reads")).isEqualTo(64);
+ assertThat(result.configuration().cassandraYaml().getString("cluster_name")).isEqualTo("Test Cluster");
+ assertThat(result.hash()).startsWith("sha256:");
+ assertThat(result.hash()).isNotEqualTo(baseHash);
+ }
+
+ @Test
+ void testPatchAddJvmOpt()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/extraJvmOpts/-Xmx", "8g"));
+
+ ConfigurationOverlaySnapshot result = manager.patchConfiguration(instance, baseHash, ops);
+
+ assertThat(result.configuration().extraJvmOpts()).containsEntry("-Xmx", "8g");
+ }
+
+ @Test
+ void testPatchRemoveTopLevelKeyFromOverlay()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ JsonObject initialYaml = new JsonObject().put("concurrent_reads", 128);
+ CassandraConfigurationOverlay initial = new CassandraConfigurationOverlay(initialYaml, null);
+ provider.storeOverlay(instance, null, new ConfigurationOverlaySnapshot(Instant.now(), initial));
+
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ String effectiveHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.REMOVE,
+ "/configuration/cassandraYaml/concurrent_reads", null));
+
+ ConfigurationOverlaySnapshot result = manager.patchConfiguration(instance, effectiveHash, ops);
+
+ // Falls back to base template value
+ assertThat(result.configuration().cassandraYaml().getInteger("concurrent_reads")).isEqualTo(32);
+ }
+
+ @Test
+ void testPatchRemoveTemplateOnlyKeyFails()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.REMOVE,
+ "/configuration/cassandraYaml/cluster_name", null));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, baseHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in overlay");
+ }
+
+ @Test
+ void testPatchReplaceExistingKey()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ // concurrent_reads=32 exists in base template
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.REPLACE,
+ "/configuration/cassandraYaml/concurrent_reads", 256));
+
+ ConfigurationOverlaySnapshot result = manager.patchConfiguration(instance, baseHash, ops);
+
+ assertThat(result.configuration().cassandraYaml().getInteger("concurrent_reads")).isEqualTo(256);
+ }
+
+ @Test
+ void testPatchReplaceAbsentKeyFails()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.REPLACE,
+ "/configuration/cassandraYaml/nonexistent_key", 42));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, baseHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in effective config");
+ }
+
+ @Test
+ void testPatchTestMatchingValue()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.TEST,
+ "/configuration/cassandraYaml/concurrent_reads", 32),
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ ConfigurationOverlaySnapshot result = manager.patchConfiguration(instance, baseHash, ops);
+
+ assertThat(result.configuration().cassandraYaml().getInteger("concurrent_reads")).isEqualTo(128);
+ }
+
+ @Test
+ void testPatchTestMismatchRejectsEntirePatch()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ JsonObject initialYaml = new JsonObject().put("concurrent_reads", 64);
+ CassandraConfigurationOverlay initial = new CassandraConfigurationOverlay(initialYaml, null);
+ provider.storeOverlay(instance, null, new ConfigurationOverlaySnapshot(Instant.now(), initial));
+
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ String effectiveHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.TEST,
+ "/configuration/cassandraYaml/concurrent_reads", 999),
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/memtable_flush_writers", 16));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, effectiveHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+
+ // Verify no mutation occurred
+ ConfigurationOverlaySnapshot current = manager.getEffectiveConfiguration(instance);
+ assertThat(current.configuration().cassandraYaml().getInteger("concurrent_reads")).isEqualTo(64);
+ assertThat(current.configuration().cassandraYaml().containsKey("memtable_flush_writers")).isFalse();
+ }
+
+ @Test
+ void testPatchConflictStaleHash()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ JsonObject initialYaml = new JsonObject().put("concurrent_reads", 64);
+ CassandraConfigurationOverlay initial = new CassandraConfigurationOverlay(initialYaml, null);
+ provider.storeOverlay(instance, null, new ConfigurationOverlaySnapshot(Instant.now(), initial));
+
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ String actualHash = manager.getEffectiveConfiguration(instance).hash();
+ String staleHash = "sha256:0000000000000000000000000000000000000000000000000000000000000000";
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, staleHash, ops))
+ .isInstanceOf(ConfigurationConflictException.class)
+ .satisfies(e -> {
+ ConfigurationConflictException conflict = (ConfigurationConflictException) e;
+ assertThat(conflict.expectedHash()).isEqualTo(staleHash);
+ assertThat(conflict.actualHash()).isEqualTo(actualHash);
+ });
+
+ // Overlay unchanged
+ assertThat(provider.getOverlay(instance).configuration().cassandraYaml().getInteger("concurrent_reads"))
+ .isEqualTo(64);
+ }
+
+ @Test
+ void testPatchStoreOverlayReturnsFalse()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ JsonObject initialYaml = new JsonObject().put("concurrent_reads", 64);
+ CassandraConfigurationOverlay initial = new CassandraConfigurationOverlay(initialYaml, null);
+ ConfigurationOverlaySnapshot initialSnapshot = new ConfigurationOverlaySnapshot(Instant.now(), initial);
+
+ ConfigurationProvider rejectingProvider = new ConfigurationProvider()
+ {
+ private ConfigurationOverlaySnapshot stored = initialSnapshot;
+
+ @Override
+ public ConfigurationOverlaySnapshot getOverlay(InstanceMetadata inst)
+ {
+ return stored;
+ }
+
+ @Override
+ public boolean storeOverlay(InstanceMetadata inst, String originalHash,
+ ConfigurationOverlaySnapshot newSnapshot)
+ {
+ return false;
+ }
+ };
+
+ ConfigurationManager manager = new ConfigurationManager(rejectingProvider, BASE_TEMPLATE);
+ String effectiveHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, effectiveHash, ops))
+ .isInstanceOf(ConfigurationManagerException.class)
+ .isNotInstanceOf(ConfigurationConflictException.class)
+ .hasMessageContaining("Provider rejected the overlay store unexpectedly");
+ }
+
+ @Test
+ void testPatchStoreOverlayReturnsFalseWithConflict()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ JsonObject initialYaml = new JsonObject().put("concurrent_reads", 64);
+ CassandraConfigurationOverlay initial = new CassandraConfigurationOverlay(initialYaml, null);
+ ConfigurationOverlaySnapshot initialSnapshot = new ConfigurationOverlaySnapshot(Instant.now(), initial);
+
+ // Overlay changes between storeOverlay rejection and re-read
+ JsonObject changedYaml = new JsonObject().put("concurrent_reads", 256);
+ CassandraConfigurationOverlay changed = new CassandraConfigurationOverlay(changedYaml, null);
+ ConfigurationOverlaySnapshot changedSnapshot = new ConfigurationOverlaySnapshot(Instant.now(), changed);
+
+ AtomicInteger getOverlayCallCount = new AtomicInteger(0);
+ ConfigurationProvider conflictingProvider = new ConfigurationProvider()
+ {
+ @Override
+ public ConfigurationOverlaySnapshot getOverlay(InstanceMetadata inst)
+ {
+ // First call returns initial, second call (after store rejection) returns changed
+ return getOverlayCallCount.incrementAndGet() <= 1 ? initialSnapshot : changedSnapshot;
+ }
+
+ @Override
+ public boolean storeOverlay(InstanceMetadata inst, String originalHash,
+ ConfigurationOverlaySnapshot newSnapshot)
+ {
+ return false;
+ }
+ };
+
+ ConfigurationManager manager = new ConfigurationManager(conflictingProvider, BASE_TEMPLATE);
+ String effectiveHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, effectiveHash, ops))
+ .isInstanceOf(ConfigurationConflictException.class)
+ .satisfies(e -> {
+ ConfigurationConflictException conflict = (ConfigurationConflictException) e;
+ assertThat(conflict.expectedHash()).isEqualTo(effectiveHash);
+ assertThat(conflict.actualHash()).isNotEqualTo(effectiveHash);
+ });
+ }
+
+ @Test
+ void testPatchProviderFailure()
+ {
+ ConfigurationProvider failingProvider = new ConfigurationProvider()
+ {
+ @Override
+ public ConfigurationOverlaySnapshot getOverlay(InstanceMetadata instance)
+ {
+ throw new UncheckedIOException(new IOException("provider unavailable"));
+ }
+
+ @Override
+ public boolean storeOverlay(InstanceMetadata instance, String originalHash,
+ @NotNull ConfigurationOverlaySnapshot newSnapshot)
+ {
+ throw new UnsupportedOperationException();
+ }
+ };
+
+ ConfigurationManager manager = new ConfigurationManager(failingProvider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, "sha256:abc", ops))
+ .isInstanceOf(ConfigurationManagerException.class)
+ .isNotInstanceOf(ConfigurationConflictException.class)
+ .hasMessageContaining("Failed to patch configuration")
+ .hasCauseInstanceOf(UncheckedIOException.class);
+ }
+
+ @Test
+ void testPatchConcurrentSameInstance() throws Exception
+ {
+ InstanceMetadata instance = mockInstance(1);
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ int threadCount = 10;
+ ExecutorService executor = Executors.newFixedThreadPool(threadCount);
+ CountDownLatch startLatch = new CountDownLatch(1);
+ AtomicInteger successCount = new AtomicInteger(0);
+ AtomicInteger conflictCount = new AtomicInteger(0);
+
+ List> futures = new ArrayList<>();
+ for (int i = 0; i < threadCount; i++)
+ {
+ int value = i;
+ futures.add(executor.submit(() -> {
+ try
+ {
+ startLatch.await();
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", value));
+ manager.patchConfiguration(instance, baseHash, ops);
+ successCount.incrementAndGet();
+ }
+ catch (ConfigurationConflictException e)
+ {
+ conflictCount.incrementAndGet();
+ }
+ catch (InterruptedException e)
+ {
+ Thread.currentThread().interrupt();
+ }
+ }));
+ }
+
+ startLatch.countDown();
+ for (Future future : futures)
+ {
+ future.get();
+ }
+ executor.shutdown();
+
+ assertThat(successCount.get()).isEqualTo(1);
+ assertThat(conflictCount.get()).isEqualTo(threadCount - 1);
+ }
+
+ @Test
+ void testPatchDuplicatePathsRejected()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 64),
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, baseHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Duplicate path");
+ }
+
+ @Test
+ void testPatchInvalidPathFormat()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ List ops = List.of(
+ new ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+ "/invalid/path", 42));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, baseHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Path must start with");
+ }
+
+ @Test
+ void testPatchEmptyOperationsRejected()
+ {
+ ConfigurationManager manager = new ConfigurationManager(provider, BASE_TEMPLATE);
+ InstanceMetadata instance = mockInstance(1);
+ String baseHash = manager.getEffectiveConfiguration(instance).hash();
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance, baseHash, List.of()))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("must not be empty");
+ }
+
private static InstanceMetadata mockInstance(int id)
{
InstanceMetadata instance = mock(InstanceMetadata.class);
diff --git a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplierTest.java b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplierTest.java
new file mode 100644
index 000000000..0a45ace8a
--- /dev/null
+++ b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplierTest.java
@@ -0,0 +1,434 @@
+/*
+ * 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 java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import io.vertx.core.json.JsonObject;
+
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.ADD;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.REMOVE;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.REPLACE;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.TEST;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests for {@link ConfigurationPatchApplier}
+ */
+class ConfigurationPatchApplierTest
+{
+ private static final CassandraConfigurationOverlay EMPTY_OVERLAY = new CassandraConfigurationOverlay(null, null);
+
+ private CassandraConfigurationOverlay effectiveConfig;
+
+ @BeforeEach
+ void setUp()
+ {
+ JsonObject effectiveYaml = new JsonObject()
+ .put("cluster_name", "TestCluster")
+ .put("concurrent_reads", 32)
+ .put("memtable", new JsonObject()
+ .put("configurations", new JsonObject()
+ .put("trie", new JsonObject()
+ .put("class_name", "TrieMemtable")
+ .put("max_shard_count", 4))
+ .put("skiplist", new JsonObject()
+ .put("class_name", "SkipListMemtable"))));
+
+ Map effectiveOpts = new LinkedHashMap<>();
+ effectiveOpts.put("-Xmx", "4g");
+ effectiveOpts.put("-Dcassandra.ring_delay_ms", "60000");
+
+ effectiveConfig = new CassandraConfigurationOverlay(effectiveYaml, effectiveOpts);
+ }
+
+ // --- Top-level cassandraYaml operations ---
+
+ @Test
+ void testAddTopLevelKey()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/new_key", "new_value"));
+
+ assertThat(newOverlay.cassandraYaml().getString("new_key")).isEqualTo("new_value");
+ }
+
+ @Test
+ void testAddOverwritesExistingOverlayValue()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(
+ new JsonObject().put("concurrent_reads", 64), null);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, overlay,
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/concurrent_reads", 256));
+
+ assertThat(newOverlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(256);
+ }
+
+ @Test
+ void testRemoveTopLevelKey()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(
+ new JsonObject().put("concurrent_reads", 64), null);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, overlay,
+ new ConfigurationPatchOperation(REMOVE, "/configuration/cassandraYaml/concurrent_reads", null));
+
+ assertThat(newOverlay.cassandraYaml().containsKey("concurrent_reads")).isFalse();
+ }
+
+ @Test
+ void testRemoveTemplateOnlyKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REMOVE, "/configuration/cassandraYaml/cluster_name", null)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in overlay");
+ }
+
+ @Test
+ void testReplaceExistingKey()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE, "/configuration/cassandraYaml/concurrent_reads", 128));
+
+ assertThat(newOverlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(128);
+ }
+
+ @Test
+ void testReplaceAbsentKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE, "/configuration/cassandraYaml/nonexistent", 42)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in effective config");
+ }
+
+ @Test
+ void testTestMatchingValue()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/concurrent_reads", 32));
+
+ assertThat(newOverlay.cassandraYaml()).isEmpty();
+ }
+
+ @Test
+ void testTestMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/concurrent_reads", 999)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed: expected 999 but found 32");
+ }
+
+ @Test
+ void testTestAbsentPathFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/nonexistent", "x")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("path does not exist in effective config");
+ }
+
+ // --- Nested cassandraYaml operations (copy-siblings) ---
+
+ @Test
+ void testAddNestedKeyCopiesSiblings()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
+ "/configuration/cassandraYaml/memtable/configurations/trie/compression", "lz4"));
+
+ JsonObject memtable = newOverlay.cassandraYaml().getJsonObject("memtable");
+ JsonObject trie = memtable.getJsonObject("configurations").getJsonObject("trie");
+ // New key added
+ assertThat(trie.getString("compression")).isEqualTo("lz4");
+ // Siblings copied from effective config
+ assertThat(trie.getString("class_name")).isEqualTo("TrieMemtable");
+ assertThat(trie.getInteger("max_shard_count")).isEqualTo(4);
+
+ JsonObject skiplist = memtable.getJsonObject("configurations").getJsonObject("skiplist");
+ assertThat(skiplist.getString("class_name")).isEqualTo("SkipListMemtable");
+ }
+
+ @Test
+ void testReplaceNestedKeyCopiesSiblings()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", "ShardedMemtable"));
+
+ JsonObject trie = newOverlay.cassandraYaml().getJsonObject("memtable")
+ .getJsonObject("configurations").getJsonObject("trie");
+ assertThat(trie.getString("class_name")).isEqualTo("ShardedMemtable");
+ assertThat(trie.getInteger("max_shard_count")).isEqualTo(4);
+ }
+
+ @Test
+ void testMultipleNestedOpsOnSameTopLevelKey()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", "ShardedMemtable"),
+ new ConfigurationPatchOperation(REPLACE,
+ "/configuration/cassandraYaml/memtable/configurations/trie/max_shard_count", 8));
+
+ JsonObject trie = newOverlay.cassandraYaml().getJsonObject("memtable")
+ .getJsonObject("configurations").getJsonObject("trie");
+ assertThat(trie.getString("class_name")).isEqualTo("ShardedMemtable");
+ assertThat(trie.getInteger("max_shard_count")).isEqualTo(8);
+
+ JsonObject skiplist = newOverlay.cassandraYaml().getJsonObject("memtable")
+ .getJsonObject("configurations").getJsonObject("skiplist");
+ assertThat(skiplist.getString("class_name")).isEqualTo("SkipListMemtable");
+ }
+
+ @Test
+ void testRemoveNestedKeyFromOverlay()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(new JsonObject()
+ .put("memtable", new JsonObject()
+ .put("configurations", new JsonObject()
+ .put("trie", new JsonObject()
+ .put("class_name", "ShardedMemtable")
+ .put("max_shard_count", 8))
+ .put("skiplist", new JsonObject()
+ .put("class_name", "SkipListMemtable")))), null);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, overlay,
+ new ConfigurationPatchOperation(REMOVE,
+ "/configuration/cassandraYaml/memtable/configurations/trie/max_shard_count", null));
+
+ JsonObject trie = newOverlay.cassandraYaml().getJsonObject("memtable")
+ .getJsonObject("configurations").getJsonObject("trie");
+ assertThat(trie.containsKey("max_shard_count")).isFalse();
+ assertThat(trie.getString("class_name")).isEqualTo("ShardedMemtable");
+ }
+
+ @Test
+ void testRemoveNestedKeyNotInOverlayFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REMOVE,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", null)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in overlay");
+ }
+
+ @Test
+ void testAddNestedKeyParentAbsentFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
+ "/configuration/cassandraYaml/nonexistent_parent/child/leaf", "value")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("parent path does not exist");
+ }
+
+ @Test
+ void testTestNestedValue()
+ {
+ applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", "TrieMemtable"));
+ }
+
+ @Test
+ void testTestNestedValueMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", "WrongValue")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ // --- Array-valued top-level keys ---
+
+ @Test
+ void testAddTopLevelArrayValue()
+ {
+ List directories = List.of("/data1", "/data2");
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/data_file_directories", directories));
+
+ assertThat(newOverlay.cassandraYaml().getJsonArray("data_file_directories"))
+ .containsExactly("/data1", "/data2");
+ }
+
+ @Test
+ void testReplaceTopLevelArrayValue()
+ {
+ JsonObject effectiveYaml = effectiveConfig.cassandraYaml().copy()
+ .put("data_file_directories", List.of("/old_data"));
+ CassandraConfigurationOverlay effective = new CassandraConfigurationOverlay(
+ effectiveYaml, effectiveConfig.extraJvmOpts());
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effective, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE, "/configuration/cassandraYaml/data_file_directories",
+ List.of("/new_data1", "/new_data2")));
+
+ assertThat(newOverlay.cassandraYaml().getJsonArray("data_file_directories"))
+ .containsExactly("/new_data1", "/new_data2");
+ }
+
+ @Test
+ void testNestedPathIntoArrayValueFails()
+ {
+ JsonObject effectiveYaml = effectiveConfig.cassandraYaml().copy()
+ .put("seed_provider", List.of(Map.of("class_name", "SimpleSeedProvider")));
+ CassandraConfigurationOverlay effective = new CassandraConfigurationOverlay(
+ effectiveYaml, effectiveConfig.extraJvmOpts());
+
+ assertThatThrownBy(() -> applyOps(effective, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
+ "/configuration/cassandraYaml/seed_provider/0/class_name", "OtherProvider")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("intermediate value is not an object");
+ }
+
+ // --- extraJvmOpts operations ---
+
+ @Test
+ void testAddJvmOpt()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xms", "2g"));
+
+ assertThat(newOverlay.extraJvmOpts()).containsEntry("-Xms", "2g");
+ }
+
+ @Test
+ void testRemoveJvmOptFromOverlay()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-Xmx", "8g")));
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, overlay,
+ new ConfigurationPatchOperation(REMOVE, "/configuration/extraJvmOpts/-Xmx", null));
+
+ assertThat(newOverlay.extraJvmOpts()).doesNotContainKey("-Xmx");
+ }
+
+ @Test
+ void testReplaceJvmOptAbsentFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE, "/configuration/extraJvmOpts/-Xms", "2g")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in effective extraJvmOpts");
+ }
+
+ @Test
+ void testTestJvmOptValue()
+ {
+ applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST, "/configuration/extraJvmOpts/-Xmx", "4g"));
+ }
+
+ @Test
+ void testTestJvmOptMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST, "/configuration/extraJvmOpts/-Xmx", "16g")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testAddInvalidJvmOptKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/invalidKey", "value")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key 'invalidKey'");
+ }
+
+ @Test
+ void testAddConflictingBooleanJvmOptFails()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-XX:+UseG1GC", "")));
+
+ assertThatThrownBy(() -> applyOps(effectiveConfig, overlay,
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:-UseG1GC", "")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Conflicting boolean JVM option");
+ }
+
+ @Test
+ void testReplaceBooleanJvmOptByRemovingThenAdding()
+ {
+ CassandraConfigurationOverlay overlay = new CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-XX:+UseG1GC", "")));
+
+ Map effectiveOpts = new LinkedHashMap<>();
+ effectiveOpts.put("-Xmx", "4g");
+ effectiveOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay effective = new CassandraConfigurationOverlay(
+ effectiveConfig.cassandraYaml(), effectiveOpts);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(effective, overlay,
+ new ConfigurationPatchOperation(REMOVE, "/configuration/extraJvmOpts/-XX:+UseG1GC", null),
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:-UseG1GC", ""));
+
+ assertThat(newOverlay.extraJvmOpts()).doesNotContainKey("-XX:+UseG1GC");
+ assertThat(newOverlay.extraJvmOpts()).containsEntry("-XX:-UseG1GC", "");
+ }
+
+ // --- Atomicity ---
+
+ @Test
+ void testTestFailurePreventsAllMutations()
+ {
+ assertThatThrownBy(() -> applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/new_key", "value"),
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/concurrent_reads", 999)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testMultipleOpsAppliedAtomically()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(effectiveConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/concurrent_writes", 64),
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xms", "2g"),
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/concurrent_reads", 32));
+
+ assertThat(newOverlay.cassandraYaml().getInteger("concurrent_writes")).isEqualTo(64);
+ assertThat(newOverlay.extraJvmOpts()).containsEntry("-Xms", "2g");
+ }
+
+ private static CassandraConfigurationOverlay applyOps(CassandraConfigurationOverlay effective,
+ CassandraConfigurationOverlay overlay,
+ ConfigurationPatchOperation... ops)
+ {
+ List parsed =
+ ConfigurationPatchValidator.validate(List.of(ops));
+ return ConfigurationPatchApplier.apply(parsed, effective, overlay);
+ }
+}
diff --git a/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidatorTest.java b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidatorTest.java
new file mode 100644
index 000000000..425a2f807
--- /dev/null
+++ b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidatorTest.java
@@ -0,0 +1,381 @@
+/*
+ * 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 java.util.List;
+
+import org.junit.jupiter.api.Test;
+
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.ADD;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.REMOVE;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.REPLACE;
+import static org.apache.cassandra.sidecar.configmanagement.ConfigurationPatchOperation.Op.TEST;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+/**
+ * Tests for {@link ConfigurationPatchValidator}
+ */
+class ConfigurationPatchValidatorTest
+{
+ @Test
+ void testRejectsEmptyOperationsList()
+ {
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(List.of()))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("must not be empty");
+ }
+
+ @Test
+ void testRejectsRemoveWithValue()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(REMOVE, "/configuration/cassandraYaml/key", "value"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("REMOVE' must not have a value");
+ }
+
+ @Test
+ void testRejectsAddWithoutValue()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/key", null));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("requires a value");
+ }
+
+ @Test
+ void testRejectsReplaceWithoutValue()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(REPLACE, "/configuration/cassandraYaml/key", null));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("requires a value");
+ }
+
+ @Test
+ void testRejectsTestWithoutValue()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/key", null));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("requires a value");
+ }
+
+ @Test
+ void testRejectsInvalidPathPrefix()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/invalid/path", 42));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Path must start with");
+ }
+
+ @Test
+ void testRejectsPathWithNoKeyAfterSection()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/", 42));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Path must specify at least one key after section");
+ }
+
+ @Test
+ void testRejectsNestedExtraJvmOpts()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dfoo/nested", "bar"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("extraJvmOpts paths must be flat");
+ }
+
+ @Test
+ void testRejectsEmptySegmentInPath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml//key", "value"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("empty segment");
+ }
+
+ @Test
+ void testRejectsDuplicateMutationPaths()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/key", "a"),
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/key", "b"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Duplicate path");
+ }
+
+ @Test
+ void testAllowsTestAndMutationOnSamePath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(TEST, "/configuration/cassandraYaml/key", "old"),
+ new ConfigurationPatchOperation(REPLACE, "/configuration/cassandraYaml/key", "new"));
+
+ List parsed = ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed).hasSize(2);
+ }
+
+ @Test
+ void testParsesTopLevelCassandraYamlPath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/cassandraYaml/concurrent_reads", 64));
+
+ List parsed = ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed).hasSize(1);
+ assertThat(parsed.get(0).section()).isEqualTo("cassandraYaml");
+ assertThat(parsed.get(0).topLevelKey()).isEqualTo("concurrent_reads");
+ assertThat(parsed.get(0).nestedSegments()).isEmpty();
+ assertThat(parsed.get(0).isNested()).isFalse();
+ }
+
+ @Test
+ void testParsesNestedCassandraYamlPath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD,
+ "/configuration/cassandraYaml/memtable/configurations/trie/class_name", "value"));
+
+ List parsed = ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed.get(0).section()).isEqualTo("cassandraYaml");
+ assertThat(parsed.get(0).topLevelKey()).isEqualTo("memtable");
+ assertThat(parsed.get(0).nestedSegments()).containsExactly("configurations", "trie", "class_name");
+ assertThat(parsed.get(0).isNested()).isTrue();
+ }
+
+ @Test
+ void testParsesExtraJvmOptsPath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xmx", "8g"));
+
+ List parsed = ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed.get(0).section()).isEqualTo("extraJvmOpts");
+ assertThat(parsed.get(0).topLevelKey()).isEqualTo("-Xmx");
+ assertThat(parsed.get(0).nestedSegments()).isEmpty();
+ }
+
+ // --- extraJvmOpts key allowlist tests ---
+
+ @Test
+ void testAcceptsValidSystemPropertyKey()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dcassandra.jmx.local.port", "7199"));
+
+ List parsed = ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed).hasSize(1);
+ assertThat(parsed.get(0).topLevelKey()).isEqualTo("-Dcassandra.jmx.local.port");
+ }
+
+ @Test
+ void testAcceptsValidXFlag()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xms", "2g"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsValidXXBooleanFlag()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:+UseG1GC", ""));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsValidXXValueFlag()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:MaxGCPauseMillis", "200"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testRejectsJavaAgentKey()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-javaagent", "/tmp/malicious.jar"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key '-javaagent'");
+ }
+
+ @Test
+ void testRejectsAgentPathKey()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-agentpath", "/tmp/agent.so"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key '-agentpath'");
+ }
+
+ @Test
+ void testRejectsBlockedOnOutOfMemoryError()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:OnOutOfMemoryError", "kill -9 %p"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessage("Blocked extraJvmOpts key '-XX:OnOutOfMemoryError': this JVM option is not allowed");
+ }
+
+ @Test
+ void testRejectsBlockedOnError()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:OnError", "rm -rf /"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessage("Blocked extraJvmOpts key '-XX:OnError': this JVM option is not allowed");
+ }
+
+ @Test
+ void testRejectsKeyWithEqualsSign()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dfoo=bar", "value"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key '-Dfoo=bar'");
+ }
+
+ @Test
+ void testRejectsKeyWithShellMetacharacters()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dfoo;echo pwned", "value"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key '-Dfoo;echo pwned'");
+ }
+
+ // --- extraJvmOpts value validation tests ---
+
+ @Test
+ void testAcceptsValidJvmOptValue()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xmx", "4g"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsJvmOptValueWithAbsolutePath()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:HeapDumpPath", "/var/log/cassandra"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithPathTraversal()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-XX:HeapDumpPath", "/tmp/../../etc/cron.d"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("path traversal");
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithShellMetacharacters()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dfoo", "val;rm -rf /"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("disallowed characters");
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithBacktick()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Dfoo", "`whoami`"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("disallowed characters");
+ }
+
+ @Test
+ void testRejectsJvmOptValueExceedingMaxLength()
+ {
+ String longValue = "a".repeat(513);
+ List ops = List.of(
+ new ConfigurationPatchOperation(ADD, "/configuration/extraJvmOpts/-Xmx", longValue));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("disallowed characters or exceeds 512");
+ }
+
+ @Test
+ void testAllowsRemoveWithoutValueValidation()
+ {
+ List ops = List.of(
+ new ConfigurationPatchOperation(REMOVE, "/configuration/extraJvmOpts/-Xmx", null));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+}