This is an automated email from the ASF dual-hosted git repository.
pauloricardomg pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra-sidecar.git
The following commit(s) were added to refs/heads/trunk by this push:
new b4fa381c CASSSIDECAR-429: Add RFC 6902-inspired JSON Patch support to
ConfigurationManager (#369)
b4fa381c is described below
commit b4fa381c39b26ba57c7cc4c10f89b17b7560b5f3
Author: Paulo Motta <[email protected]>
AuthorDate: Tue Aug 11 17:35:17 2026 -0300
CASSSIDECAR-429: Add RFC 6902-inspired JSON Patch support to
ConfigurationManager (#369)
Patch by Paulo Motta; Reviewed by Francisco Guerrero, Shailaja Koppu for
CASSSIDECAR-429
---
CHANGES.txt | 1 +
.../CassandraConfigurationOverlay.java | 71 +--
.../sidecar/configmanagement/ConfigUtils.java | 38 ++
.../ConfigurationConflictException.java | 54 ++
.../configmanagement/ConfigurationManager.java | 103 +++-
.../ConfigurationPatchApplier.java | 465 ++++++++++++++
.../ConfigurationPatchException.java | 45 ++
.../ConfigurationPatchOperation.java | 117 ++++
.../ConfigurationPatchValidator.java | 300 +++++++++
.../configmanagement/ConfigurationProvider.java | 6 +-
.../CassandraConfigurationOverlayTest.java | 116 +---
.../configmanagement/ConfigurationManagerTest.java | 466 ++++++++++++++
.../ConfigurationPatchApplierTest.java | 667 +++++++++++++++++++++
.../ConfigurationPatchValidatorTest.java | 437 ++++++++++++++
14 files changed, 2715 insertions(+), 171 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 6c43ac33..d440bcea 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,5 +1,6 @@
0.5.0
-----
+ * Add RFC 6902-inspired JSON Patch support to ConfigurationManager
(CASSSIDECAR-429)
* Implement job coordination for cluster-wide operations (CASSSIDECAR-377)
* Fix dead/dropped CDC lifecycle metrics and duplicate SidecarCdcStats
interface (CASSSIDECAR-489)
* Wire CDC configs in configs table to
SidecarCdcOptions/SidecarStatePersister (CASSSIDECAR-483)
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 ad518b90..57a06137 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 @@ import org.jetbrains.annotations.Nullable;
* <p>The {@code extraJvmOpts} field contains JVM options that are appended to
the Cassandra JVM startup
* command. Each entry maps the full option flag (e.g. {@code
-Dcassandra.jmx.local.port}) to its value
* (e.g. {@code 7199}). These are opaque strings not subject to schema
validation.
+ *
+ * <p>This class is immutable. Mutations are performed by {@link
ConfigurationPatchApplier} which
+ * constructs a new instance with the desired changes.
*/
public class CassandraConfigurationOverlay
{
@@ -59,7 +62,7 @@ public class CassandraConfigurationOverlay
/**
* Returns the cassandra.yaml overlay as a version-agnostic JSON object.
Callers must not mutate the
- * returned object; use {@link #updated} to produce a new overlay with
changes applied.
+ * returned object; use {@link ConfigurationPatchApplier} to produce a new
overlay with changes applied.
*
* @return the cassandra.yaml overlay as a version-agnostic JSON object
*/
@@ -114,59 +117,6 @@ public class CassandraConfigurationOverlay
return new CassandraConfigurationOverlay(cassandraYaml, extraJvmOpts);
}
- /**
- * Returns a new overlay with the given updates applied. The current
instance is not modified.
- *
- * <p>Both parameters follow the same semantics: a {@code null} value for
a key removes that entry,
- * a non-null value upserts it.
- *
- * @param cassandraYamlUpdates field-level changes to cassandra.yaml: key
= field name, value = new value.
- * A {@code null} value removes the field.
Pass {@code null} for no yaml changes.
- * @param extraJvmOptsUpdates JVM option changes: key = option name,
value = new option value.
- * A {@code null} value removes the option.
Pass {@code null} for no changes.
- * @return a new overlay with the updates applied
- */
- @NotNull
- public CassandraConfigurationOverlay updated(@Nullable Map<String, Object>
cassandraYamlUpdates,
- @Nullable Map<String, String>
extraJvmOptsUpdates)
- {
- JsonObject mergedYaml = cassandraYaml.copy();
- if (cassandraYamlUpdates != null)
- {
- for (Map.Entry<String, Object> entry :
cassandraYamlUpdates.entrySet())
- {
- if (entry.getValue() == null)
- {
- mergedYaml.remove(entry.getKey());
- }
- else
- {
- mergedYaml.put(entry.getKey(), entry.getValue());
- }
- }
- }
-
- LinkedHashMap<String, String> mergedOpts = new
LinkedHashMap<>(extraJvmOpts);
- if (extraJvmOptsUpdates != null)
- {
- for (Map.Entry<String, String> entry :
extraJvmOptsUpdates.entrySet())
- {
- if (entry.getValue() == null)
- {
- mergedOpts.remove(entry.getKey());
- }
- else
- {
- mergedOpts.put(entry.getKey(), entry.getValue());
- }
- }
- }
-
- validateNoConflictingBooleanOpts(mergedOpts);
-
- return new CassandraConfigurationOverlay(mergedYaml, mergedOpts);
- }
-
static boolean hasConflictingBooleanOpt(Map<String, String> existing,
String key)
{
String conflicting = conflictingBooleanOpt(key);
@@ -186,19 +136,6 @@ public class CassandraConfigurationOverlay
return null;
}
- private static void validateNoConflictingBooleanOpts(Map<String, String>
opts)
- {
- for (String key : opts.keySet())
- {
- if (hasConflictingBooleanOpt(opts, key))
- {
- String option = key.substring(5);
- throw new IllegalArgumentException(
- "Conflicting boolean JVM options: -XX:+" + option + " and
-XX:-" + option);
- }
- }
- }
-
@Override
public boolean equals(Object o)
{
diff --git
a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigUtils.java
b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigUtils.java
index f511c70e..5762e7d6 100644
---
a/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigUtils.java
+++
b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigUtils.java
@@ -24,9 +24,13 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Instant;
import java.util.Collections;
+import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator;
@@ -40,6 +44,8 @@ import org.jetbrains.annotations.Nullable;
*/
public final class ConfigUtils
{
+ private static final Logger LOGGER =
LoggerFactory.getLogger(ConfigUtils.class);
+
static final YAMLFactory YAML_FACTORY = new YAMLFactory()
.disable(YAMLGenerator.Feature.WRITE_DOC_START_MARKER);
@@ -125,4 +131,36 @@ public final class ConfigUtils
result.mergeIn(overlay, true);
return result;
}
+
+ /**
+ * Merges overlay JVM options onto base JVM options, with overlay entries
overriding base entries
+ * on key conflict. When an overlay entry conflicts with a base boolean
option (e.g. base
+ * {@code -XX:+UseG1GC} and overlay {@code -XX:-UseG1GC}), the base option
is preserved and the
+ * overlay entry is skipped, matching {@link
ConfigurationOverlaySnapshot#overlay}. Insertion order
+ * is preserved. Neither input map is modified.
+ *
+ * @param base the base JVM options
+ * @param overlay the overlay JVM options whose values take precedence
+ * @return a new map with the merged result
+ */
+ @NotNull
+ public static Map<String, String> mergeOpts(@NotNull Map<String, String>
base,
+ @NotNull Map<String, String>
overlay)
+ {
+ Objects.requireNonNull(base, "base must not be null");
+ Objects.requireNonNull(overlay, "overlay must not be null");
+ Map<String, String> merged = new LinkedHashMap<>(base);
+ for (Map.Entry<String, String> entry : overlay.entrySet())
+ {
+ if (CassandraConfigurationOverlay.hasConflictingBooleanOpt(merged,
entry.getKey()))
+ {
+ LOGGER.warn("Conflicting boolean JVM option '{}' in overlay
conflicts with base option '{}'. " +
+ "Preserving base option and skipping overlay
entry.",
+ entry.getKey(),
CassandraConfigurationOverlay.conflictingBooleanOpt(entry.getKey()));
+ continue; // preserve base boolean option, skip conflicting
overlay entry
+ }
+ merged.put(entry.getKey(), entry.getValue());
+ }
+ return merged;
+ }
}
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 00000000..beeb701a
--- /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 0ae2b6ce..5a0d6fc9 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;
@@ -26,7 +28,11 @@ import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
/**
- * Manages configuration of Cassandra instances via Sidecar
+ * Manages configuration of Cassandra instances via Sidecar.
+ *
+ * <p>The <em>effective configuration</em> is the result of merging the base
cassandra.yaml template
+ * with the instance overlay retrieved from the {@link ConfigurationProvider}.
It is what the instance
+ * actually sees; the overlay is only the delta persisted on top of the base
template.
*/
public class ConfigurationManager
{
@@ -48,8 +54,7 @@ public class ConfigurationManager
}
/**
- * Computes the effective configuration for the given instance by merging
the base template
- * with the overlay from the {@link ConfigurationProvider}.
+ * Computes the effective configuration for the given instance.
*
* @param instance the Cassandra instance metadata
* @return a snapshot of the effective configuration with its SHA-256 hash
and last modified timestamp
@@ -84,4 +89,96 @@ public class ConfigurationManager
cachedBaseSnapshot = snapshot;
return snapshot;
}
+
+ /**
+ * Patches the effective configuration for the given instance using RFC
6902-inspired JSON Patch
+ * operations. Validates the caller's expected hash against the current
effective configuration,
+ * validates and applies the patch operations to the overlay, persists via
the
+ * {@link ConfigurationProvider}, and returns the new effective
configuration.
+ *
+ * <p>Paths reference the effective configuration structure, but mutations
target the overlay.
+ * For nested paths within a top-level {@code cassandraYaml} key,
+ * the entire top-level key value is copied from the effective config into
the overlay before
+ * applying the leaf change (copy-siblings strategy). This ensures the
overlay is always
+ * self-contained at the top-level key granularity. As a consequence,
editing a nested leaf pins
+ * its sibling leaves against later base-template drift, and removing an
overlaid leaf reverts it to
+ * the current base value; see {@link ConfigurationPatchApplier} for
details.
+ *
+ * <p>All operations are validated atomically: either all succeed or none
are applied.
+ *
+ * @param instance the Cassandra instance metadata
+ * @param expectedHash the hash of the effective configuration as last
seen by the caller
+ * @param operations the patch operations to apply
+ * @return a snapshot of the new effective configuration with its SHA-256
hash and last modified timestamp
+ * @throws ConfigurationConflictException if the expectedHash does not
match the current effective hash
+ * @throws ConfigurationPatchException if any patch operation fails
validation or application
+ * @throws ConfigurationManagerException if the provider fails during the
operation
+ */
+ @NotNull
+ public ConfigurationOverlaySnapshot patchConfiguration(@NotNull
InstanceMetadata instance,
+ @NotNull String
expectedHash,
+ @NotNull
List<ConfigurationPatchOperation> operations)
+ {
+ Objects.requireNonNull(instance, "instance must not be null");
+ Objects.requireNonNull(expectedHash, "expectedHash must not be null");
+ Objects.requireNonNull(operations, "operations must not be null");
+
+ try
+ {
+ // 1. Validate operations structure (path format, value presence,
duplicates)
+ List<ConfigurationPatchValidator.ParsedPatchOperation> parsedOps =
+ ConfigurationPatchValidator.validate(operations);
+
+ // 2. Read current state and validate the caller's hash matches
+ ConfigurationOverlaySnapshot baseSnapshot = getBaseSnapshot();
+ ConfigurationOverlaySnapshot currentOverlay =
provider.getOverlay(instance);
+
+ ConfigurationOverlaySnapshot effectiveConfig = currentOverlay !=
null
+ ?
baseSnapshot.overlay(currentOverlay, instance.id())
+ : baseSnapshot;
+
+ if (!expectedHash.equals(effectiveConfig.hash()))
+ {
+ throw new ConfigurationConflictException(expectedHash,
effectiveConfig.hash());
+ }
+
+ // 3. Apply patch operations to the overlay (precondition checks +
mutations)
+ CassandraConfigurationOverlay currentOverlayConfig =
currentOverlay != null
+ ?
currentOverlay.configuration()
+ : new
CassandraConfigurationOverlay(null, null);
+ CassandraConfigurationOverlay updatedOverlay =
+ ConfigurationPatchApplier.apply(parsedOps,
baseSnapshot.configuration(), currentOverlayConfig);
+ ConfigurationOverlaySnapshot newOverlaySnapshot = new
ConfigurationOverlaySnapshot(Instant.now(),
+
updatedOverlay);
+
+ // 4. CAS via provider — concurrent patches are resolved by the
provider's own atomicity
+ String currentOverlayHash = currentOverlay != null ?
currentOverlay.hash() : null;
+ boolean stored = provider.storeOverlay(instance,
currentOverlayHash, newOverlaySnapshot);
+
+ // 5. Store rejected — re-read to determine if this is a true
conflict or an unexpected failure
+ if (!stored)
+ {
+ ConfigurationOverlaySnapshot updatedCurrent =
provider.getOverlay(instance);
+ ConfigurationOverlaySnapshot newEffective = updatedCurrent !=
null
+ ?
baseSnapshot.overlay(updatedCurrent, instance.id())
+ : baseSnapshot;
+ if (!expectedHash.equals(newEffective.hash()))
+ {
+ throw new ConfigurationConflictException(expectedHash,
newEffective.hash());
+ }
+ throw new ConfigurationManagerException(
+ "Provider rejected the overlay store unexpectedly",
null);
+ }
+
+ return baseSnapshot.overlay(newOverlaySnapshot, instance.id());
+ }
+ catch (ConfigurationManagerException e)
+ {
+ throw e;
+ }
+ catch (Exception e)
+ {
+ throw new ConfigurationManagerException("Failed to patch
configuration", e);
+ }
+ }
}
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 00000000..b5698fa0
--- /dev/null
+++
b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplier.java
@@ -0,0 +1,465 @@
+/*
+ * 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.
+ *
+ * <p>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).
+ *
+ * <p>Because copy-siblings captures a snapshot of the whole top-level block
into the overlay, and the
+ * overlay is deep-merged over the base ({@link
ConfigUtils#mergeConfigurations}), two behaviors follow
+ * that callers should be aware of:
+ * <ul>
+ * <li><b>Editing a nested leaf pins its sibling leaves against base
drift.</b> Every leaf present in
+ * the top-level block at edit time is copied into the overlay and
thereafter shadows the base, so
+ * later changes to those same leaves in the base template no longer
surface in the effective
+ * configuration. Keys added to the base block <em>after</em> the edit
are not pinned - the deep
+ * merge still surfaces them.</li>
+ * <li><b>Removing an overlaid leaf reverts it to the current base
value.</b> The leaf is deleted from
+ * the overlay, so the effective value falls back to whatever the base
template currently holds,
+ * which may differ from the value that was in effect when the overlay
was written.</li>
+ * </ul>
+ *
+ * <p>Operations are applied sequentially (RFC 6902 section 5): each operation
is validated and
+ * applied against the effective configuration produced by the previous
operation, so later
+ * operations observe the effects of earlier ones. Mutations target a
throwaway copy of the overlay;
+ * if any operation fails, the exception propagates and the copy is discarded,
so nothing is
+ * persisted (all-or-nothing atomicity).
+ */
+public final class ConfigurationPatchApplier
+{
+ private ConfigurationPatchApplier()
+ {
+ throw new UnsupportedOperationException();
+ }
+
+ /**
+ * Applies a list of validated patch operations and returns the updated
overlay.
+ *
+ * <p>The effective configuration is re-derived from {@code baseConfig}
and the evolving overlay
+ * before each operation, so an operation's preconditions are checked
against the state left by the
+ * preceding operations rather than the initial snapshot.
+ *
+ * @param parsedOps the validated and parsed operations
+ * @param baseConfig the base configuration (base template) the
overlay is applied on top of
+ * @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<ConfigurationPatchValidator.ParsedPatchOperation>
parsedOps,
+ @NotNull CassandraConfigurationOverlay baseConfig,
+ @NotNull CassandraConfigurationOverlay currentOverlay)
+ {
+ JsonObject overlayYaml = currentOverlay.cassandraYaml().copy();
+ Map<String, String> overlayOpts = new
LinkedHashMap<>(currentOverlay.extraJvmOpts());
+
+ for (ConfigurationPatchValidator.ParsedPatchOperation parsed :
parsedOps)
+ {
+ // Re-derive the effective configuration so each op sees the
effects of the previous ones.
+ JsonObject effectiveYaml =
ConfigUtils.mergeConfigurations(baseConfig.cassandraYaml(), overlayYaml);
+ Map<String, String> effectiveOpts =
ConfigUtils.mergeOpts(baseConfig.extraJvmOpts(), overlayOpts);
+
+ checkPreconditions(parsed, effectiveYaml, effectiveOpts,
overlayYaml, overlayOpts);
+ applyMutation(parsed, effectiveYaml, overlayYaml, overlayOpts);
+ }
+
+ return new CassandraConfigurationOverlay(overlayYaml, overlayOpts);
+ }
+
+ private static void
checkPreconditions(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ JsonObject effectiveYaml,
+ Map<String, String> effectiveOpts,
+ JsonObject overlayYaml,
+ Map<String, String> overlayOpts)
+ {
+ if (parsed.section().equals(CASSANDRA_YAML_SECTION))
+ {
+ checkYamlPreconditions(parsed, effectiveYaml, overlayYaml);
+ }
+ else if (parsed.section().equals(EXTRA_JVM_OPTS_SECTION))
+ {
+ checkJvmOptsPreconditions(parsed, effectiveOpts, overlayOpts);
+ }
+ }
+
+ 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);
+ }
+ // Both sides are Vert.x JSON types: resolveValue returns
JsonObject/JsonArray/scalars,
+ // and op.value() is expected to be read from the request via
JsonObject.getValue,
+ // which wraps objects/arrays the same way.
+ if (!Objects.equals(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<String> 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))
+ {
+ 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<String, String>
effectiveOpts,
+ Map<String, String>
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);
+ }
+ checkNoConflictingBooleanOpt(effectiveOpts, key, 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:
+ // extraJvmOpts is flat, so the parent always exists. Only
guard against introducing a
+ // boolean option that conflicts with one already in the
effective configuration
+ // (e.g. adding -XX:-UseG1GC while -XX:+UseG1GC is set by the
base or a prior op).
+ checkNoConflictingBooleanOpt(effectiveOpts, key, op);
+ break;
+ }
+ }
+
+ private static void checkNoConflictingBooleanOpt(Map<String, String>
effectiveOpts, String key,
+
ConfigurationPatchOperation op)
+ {
+ if
(CassandraConfigurationOverlay.hasConflictingBooleanOpt(effectiveOpts, key))
+ {
+ String conflicting =
CassandraConfigurationOverlay.conflictingBooleanOpt(key);
+ throw new ConfigurationPatchException(
+ "Conflicting boolean JVM option: '" + key + "' conflicts
with existing '"
+ + conflicting + "' in the effective configuration", op);
+ }
+ }
+
+ private static void
applyMutation(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ JsonObject effectiveYaml,
+ JsonObject updatedYaml,
+ Map<String, String> 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, effectiveYaml, 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 topLevelObject =
asJsonObject(updatedYaml.getValue(parsed.topLevelKey()));
+ if (topLevelObject != null)
+ {
+ removeAtPath(topLevelObject, 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 topLevelObject;
+ if (updatedYaml.containsKey(parsed.topLevelKey()))
+ {
+ topLevelObject =
asJsonObject(updatedYaml.getValue(parsed.topLevelKey()));
+ }
+ else
+ {
+ topLevelObject =
deepCopyValue(effectiveYaml.getValue(parsed.topLevelKey()));
+ }
+ if (topLevelObject == null)
+ {
+ topLevelObject = new JsonObject();
+ }
+ setAtPath(topLevelObject, parsed.nestedSegments(), op.value());
+ updatedYaml.put(parsed.topLevelKey(), topLevelObject);
+ }
+ }
+
+ private static void
applyJvmOptsMutation(ConfigurationPatchValidator.ParsedPatchOperation parsed,
+ Map<String, String> updatedOpts)
+ {
+ ConfigurationPatchOperation op = parsed.operation();
+ String key = parsed.topLevelKey();
+
+ if (op.op() == ConfigurationPatchOperation.Op.REMOVE)
+ {
+ updatedOpts.remove(key);
+ }
+ else
+ {
+ // Boolean-opt conflicts are rejected by checkJvmOptsPreconditions
against the current
+ // effective configuration, so the mutation here is unconditional.
+ 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<String> 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<String> 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<String> 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)
+ {
+ // Unreachable: the remove precondition already verified the
full path exists in the
+ // overlay against the current (post-previous-op) state, so
every intermediate is present.
+ throw new IllegalStateException("Overlay path segment '" +
segments.get(i)
+ + "' is missing or not an
object during remove");
+ }
+ parent = childObj;
+ }
+ parent.remove(segments.get(segments.size() - 1));
+ }
+
+ // Values are always read via JsonObject.getValue, which wraps nested Maps
into JsonObject,
+ // so a raw Map never reaches here; only JsonObject (or a non-object such
as JsonArray/scalar).
+ @Nullable
+ private static JsonObject asJsonObject(@Nullable Object value)
+ {
+ return value instanceof JsonObject ? (JsonObject) value : null;
+ }
+
+ private static JsonObject deepCopyValue(@Nullable Object value)
+ {
+ return value instanceof JsonObject ? ((JsonObject) value).copy() : new
JsonObject();
+ }
+
+}
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 00000000..6a234ba9
--- /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 00000000..581abb60
--- /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}.
+ *
+ * <p>Paths use JSON Pointer syntax (RFC 6901) and reference the effective
configuration structure.
+ * Operations are interpreted as overlay mutations:
+ * <ul>
+ * <li>{@code add} — upsert value in overlay; parent must exist in effective
config</li>
+ * <li>{@code remove} — remove from overlay; fails if key only exists in
base template</li>
+ * <li>{@code replace} — set value in overlay; fails if key absent from
effective config</li>
+ * <li>{@code test} — assert effective config value matches; no mutation</li>
+ * </ul>
+ */
+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 00000000..889d0eba
--- /dev/null
+++
b/server/src/main/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidator.java
@@ -0,0 +1,300 @@
+/*
+ * 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.
+ *
+ * <p>Validation includes:
+ * <ul>
+ * <li>Path format: must start with {@code /configuration/cassandraYaml/} or
+ * {@code /configuration/extraJvmOpts/}</li>
+ * <li>Value presence: required for add/replace/test, must be absent for
remove</li>
+ * <li>Duplicate path detection: multiple mutation ops targeting the same
path are rejected</li>
+ * <li>Path segments split by {@code /}; empty segments are rejected</li>
+ * </ul>
+ */
+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_]*)$");
+
+ // JVM options that are rejected because they execute arbitrary commands
or write to
+ // arbitrary filesystem paths. The value pattern permits absolute paths
(Cassandra system
+ // properties legitimately need them), so path-bearing flags must be
blocked by key instead.
+ static final Set<String> BLOCKED_JVM_OPTS = Set.of(
+ // Execute arbitrary commands
+ "-XX:OnOutOfMemoryError",
+ "-XX:OnError",
+ // Write to arbitrary filesystem paths
+ "-XX:ErrorFile",
+ "-XX:HeapDumpPath",
+ "-XX:LogFile",
+ "-XX:FlightRecorderOptions",
+ "-XX:StartFlightRecording",
+ "-Xloggc",
+ "-Xlog",
+ "-Xbootclasspath");
+
+ // Allows: alphanumeric, dots, colons, slashes, @, +, commas, hyphens,
braces, brackets, quotes (max 512 chars).
+ // Quotes/braces/brackets permit JSON values. Whitespace is rejected
because the Cassandra launcher
+ // word-splits it; shell metacharacters (;|&$`), newlines and other
control characters are also rejected.
+ 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<ParsedPatchOperation> validate(@NotNull
List<ConfigurationPatchOperation> operations)
+ {
+ if (operations.isEmpty())
+ {
+ throw new ConfigurationPatchException("Patch operations list must
not be empty", null);
+ }
+
+ Set<String> seenMutationPaths = new HashSet<>();
+ List<ParsedPatchOperation> 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<String> 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<String> 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<String> 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_JVM_OPTS.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<String> parsePointerSegments(String pointer,
ConfigurationPatchOperation op)
+ {
+ if (pointer.isEmpty())
+ {
+ return List.of();
+ }
+
+ String[] parts = pointer.split("/", -1);
+ List<String> 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<String> nestedSegments;
+
+ ParsedPatchOperation(@NotNull ConfigurationPatchOperation operation,
+ @NotNull String section,
+ @NotNull String topLevelKey,
+ @NotNull List<String> 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.
+ * For example, path {@code
/configuration/cassandraYaml/memtable/heap_pool} yields top-level key
+ * {@code memtable} and nested segments {@code [heap_pool]}.
+ */
+ @NotNull
+ public List<String> 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 86fcb3b8..e639eef7 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 @@ import org.jetbrains.annotations.Nullable;
*
* <p>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.
*
* <p>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 26f9f886..450f5b5a 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 org.junit.jupiter.api.Test;
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 @@ import static
org.assertj.core.api.Assertions.assertThatThrownBy;
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<String, Object> 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<String, String> jvmOpts = new LinkedHashMap<>();
- jvmOpts.put("-Dcassandra.jmx.local.port", "7199");
- jvmOpts.put("-Xmx", "4G");
- CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(null, jvmOpts);
-
- Map<String, String> updates = new LinkedHashMap<>();
- updates.put("-Xmx", "8G");
-
- CassandraConfigurationOverlay updated = overlay.updated(null, updates);
-
- Map<String, String> expected = new LinkedHashMap<>();
- expected.put("-Dcassandra.jmx.local.port", "7199");
- expected.put("-Xmx", "8G");
- assertThat(updated.extraJvmOpts()).containsExactlyEntriesOf(expected);
- }
-
- @Test
- void testUpdatedRemovesJvmOptWithNullValue()
- {
- Map<String, String> jvmOpts = new LinkedHashMap<>();
- jvmOpts.put("-Dcassandra.jmx.local.port", "7199");
- jvmOpts.put("-Xmx", "4G");
- CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(null, jvmOpts);
-
- Map<String, String> 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<String, String> 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<String, String> 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 49080915..719f6b0f 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.io.UncheckedIOException;
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,464 @@ class ConfigurationManagerTest
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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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 testPatchConflictingBooleanJvmOptRejected()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ // Existing overlay enables G1GC; a patch that adds the opposite flag
must be rejected outright
+ // rather than being stored and silently dropped when merged with the
base.
+ Map<String, String> jvmOpts = new LinkedHashMap<>();
+ jvmOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay initial = new
CassandraConfigurationOverlay(null, jvmOpts);
+ provider.storeOverlay(instance, null, new
ConfigurationOverlaySnapshot(Instant.now(), initial));
+
+ ConfigurationManager manager = new ConfigurationManager(provider,
BASE_TEMPLATE);
+ String effectiveHash =
manager.getEffectiveConfiguration(instance).hash();
+
+ List<ConfigurationPatchOperation> ops = List.of(
+ new
ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+
"/configuration/extraJvmOpts/-XX:-UseG1GC", ""));
+
+ assertThatThrownBy(() -> manager.patchConfiguration(instance,
effectiveHash, ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Conflicting boolean JVM option");
+
+ // Nothing was stored: the overlay still enables G1GC and does not
contain the conflicting flag
+ CassandraConfigurationOverlay storedOverlay =
provider.getOverlay(instance).configuration();
+ assertThat(storedOverlay.extraJvmOpts()).containsEntry("-XX:+UseG1GC",
"");
+
assertThat(storedOverlay.extraJvmOpts()).doesNotContainKey("-XX:-UseG1GC");
+ }
+
+ @Test
+ void testPatchReturnedConfigMatchesStoredConfig()
+ {
+ InstanceMetadata instance = mockInstance(1);
+
+ Map<String, String> jvmOpts = new LinkedHashMap<>();
+ jvmOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay initial = new
CassandraConfigurationOverlay(null, jvmOpts);
+ provider.storeOverlay(instance, null, new
ConfigurationOverlaySnapshot(Instant.now(), initial));
+
+ ConfigurationManager manager = new ConfigurationManager(provider,
BASE_TEMPLATE);
+ String effectiveHash =
manager.getEffectiveConfiguration(instance).hash();
+
+ List<ConfigurationPatchOperation> ops = List.of(
+ new
ConfigurationPatchOperation(ConfigurationPatchOperation.Op.ADD,
+
"/configuration/extraJvmOpts/-Xmx", "8g"));
+
+ ConfigurationOverlaySnapshot returned =
manager.patchConfiguration(instance, effectiveHash, ops);
+
+ // The effective config returned to the caller must equal the
effective config computed from the
+ // persisted overlay on a subsequent read (no silent divergence
between stored and returned state).
+ ConfigurationOverlaySnapshot reread =
manager.getEffectiveConfiguration(instance);
+ assertThat(returned.hash()).isEqualTo(reread.hash());
+ assertThat(returned.configuration()).isEqualTo(reread.configuration());
+ assertThat(returned.configuration().extraJvmOpts())
+ .containsEntry("-XX:+UseG1GC", "")
+ .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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<Future<?>> futures = new ArrayList<>();
+ for (int i = 0; i < threadCount; i++)
+ {
+ int value = i;
+ futures.add(executor.submit(() -> {
+ try
+ {
+ startLatch.await();
+ List<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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 00000000..fe6f25de
--- /dev/null
+++
b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchApplierTest.java
@@ -0,0 +1,667 @@
+/*
+ * 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.JsonArray;
+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 baseConfig;
+
+ @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<String, String> effectiveOpts = new LinkedHashMap<>();
+ effectiveOpts.put("-Xmx", "4g");
+ effectiveOpts.put("-Dcassandra.ring_delay_ms", "60000");
+
+ baseConfig = new CassandraConfigurationOverlay(effectiveYaml,
effectiveOpts);
+ }
+
+ // --- Top-level cassandraYaml operations ---
+
+ @Test
+ void testAddTopLevelKey()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
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(baseConfig,
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(baseConfig,
overlay,
+ new ConfigurationPatchOperation(REMOVE,
"/configuration/cassandraYaml/concurrent_reads", null));
+
+
assertThat(newOverlay.cassandraYaml().containsKey("concurrent_reads")).isFalse();
+ }
+
+ @Test
+ void testRemoveTemplateOnlyKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, 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(baseConfig,
EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/concurrent_reads", 128));
+
+
assertThat(newOverlay.cassandraYaml().getInteger("concurrent_reads")).isEqualTo(128);
+ }
+
+ @Test
+ void testReplaceAbsentKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, 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(baseConfig,
EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
"/configuration/cassandraYaml/concurrent_reads", 32));
+
+ assertThat(newOverlay.cassandraYaml()).isEmpty();
+ }
+
+ @Test
+ void testTestMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, 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(baseConfig, 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(baseConfig,
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(baseConfig,
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 testNestedEditPinsSiblingsAgainstBaseDrift()
+ {
+ // Editing one nested leaf copies the whole top-level block's leaves
into the overlay, pinning
+ // the siblings that existed at edit time against later base-template
drift. Keys added to the
+ // base block afterwards are not pinned - the deep merge still
surfaces them.
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
+
"/configuration/cassandraYaml/memtable/configurations/trie/class_name",
"ShardedMemtable"));
+
+ // Base template drifts after the overlay was written: an existing
sibling changes value and a
+ // brand-new sibling key appears.
+ JsonObject driftedBaseYaml = baseConfig.cassandraYaml().copy();
+ JsonObject driftedTrie = driftedBaseYaml.getJsonObject("memtable")
+
.getJsonObject("configurations").getJsonObject("trie");
+ driftedTrie.put("max_shard_count", 99); // existing sibling
drifts
+ driftedTrie.put("new_base_field", "fromBase"); // new key added to base
+
+ JsonObject effective =
ConfigUtils.mergeConfigurations(driftedBaseYaml, newOverlay.cassandraYaml());
+ JsonObject trie =
effective.getJsonObject("memtable").getJsonObject("configurations").getJsonObject("trie");
+
+ assertThat(trie.getString("class_name")).isEqualTo("ShardedMemtable");
// the edit
+ assertThat(trie.getInteger("max_shard_count")).isEqualTo(4);
// pinned, not the drifted 99
+ assertThat(trie.getString("new_base_field")).isEqualTo("fromBase");
// new base key still surfaces
+ }
+
+ @Test
+ void testRemoveOverlaidLeafRevertsToCurrentBaseValue()
+ {
+ // trie/max_shard_count is overlaid at 8; removing it from the overlay
lets the base value surface
+ // again - and specifically the base value at merge time, not the
value the overlay held.
+ CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(new JsonObject()
+ .put("memtable", new JsonObject()
+ .put("configurations", new JsonObject()
+ .put("trie", new
JsonObject().put("max_shard_count", 8)))), null);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
overlay,
+ new ConfigurationPatchOperation(REMOVE,
+
"/configuration/cassandraYaml/memtable/configurations/trie/max_shard_count",
null));
+
+ // Base drifts to 16 after removal; the effective value tracks the
current base, not the removed 8.
+ JsonObject driftedBaseYaml = baseConfig.cassandraYaml().copy();
+
driftedBaseYaml.getJsonObject("memtable").getJsonObject("configurations")
+ .getJsonObject("trie").put("max_shard_count", 16);
+
+ JsonObject effective =
ConfigUtils.mergeConfigurations(driftedBaseYaml, newOverlay.cassandraYaml());
+ JsonObject trie =
effective.getJsonObject("memtable").getJsonObject("configurations").getJsonObject("trie");
+
+ assertThat(trie.getInteger("max_shard_count")).isEqualTo(16); //
reverted to current base value
+ }
+
+ @Test
+ void testMultipleNestedOpsOnSameTopLevelKey()
+ {
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
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(baseConfig,
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(baseConfig, 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(baseConfig, 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(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+
"/configuration/cassandraYaml/memtable/configurations/trie/class_name",
"TrieMemtable"));
+ }
+
+ @Test
+ void testTestObjectValue()
+ {
+ // TEST against an object-valued path. The request handler reads the
value via JsonObject.getValue,
+ // so the expected value is a JsonObject - the same type resolveValue
returns.
+ applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+
"/configuration/cassandraYaml/memtable/configurations/trie",
+ new JsonObject().put("class_name",
"TrieMemtable").put("max_shard_count", 4)));
+ }
+
+ @Test
+ void testTestObjectValueMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+
"/configuration/cassandraYaml/memtable/configurations/trie",
+ new JsonObject().put("class_name",
"TrieMemtable").put("max_shard_count", 99))))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testTestArrayValue()
+ {
+ // TEST against an array-valued path. The expected value is a
JsonArray, matching what
+ // resolveValue returns for array values.
+ JsonObject effectiveYaml = baseConfig.cassandraYaml().copy()
+ .put("data_file_directories", new
JsonArray().add("/data1").add("/data2"));
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ effectiveYaml, baseConfig.extraJvmOpts());
+
+ applyOps(effective, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+ "/configuration/cassandraYaml/data_file_directories",
+ new JsonArray().add("/data1").add("/data2")));
+ }
+
+ @Test
+ void testTestArrayValueMismatchFails()
+ {
+ JsonObject effectiveYaml = baseConfig.cassandraYaml().copy()
+ .put("data_file_directories", new
JsonArray().add("/data1").add("/data2"));
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ effectiveYaml, baseConfig.extraJvmOpts());
+
+ assertThatThrownBy(() -> applyOps(effective, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
+ "/configuration/cassandraYaml/data_file_directories",
+ new JsonArray().add("/data1").add("/other"))))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testTestNestedValueMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, 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<String> directories = List.of("/data1", "/data2");
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
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 = baseConfig.cassandraYaml().copy()
+ .put("data_file_directories", List.of("/old_data"));
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ effectiveYaml, baseConfig.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 = baseConfig.cassandraYaml().copy()
+ .put("seed_provider", List.of(Map.of("class_name",
"SimpleSeedProvider")));
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ effectiveYaml, baseConfig.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(baseConfig,
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(baseConfig,
overlay,
+ new ConfigurationPatchOperation(REMOVE,
"/configuration/extraJvmOpts/-Xmx", null));
+
+ assertThat(newOverlay.extraJvmOpts()).doesNotContainKey("-Xmx");
+ }
+
+ @Test
+ void testReplaceJvmOptAbsentFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/extraJvmOpts/-Xms", "2g")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in effective
extraJvmOpts");
+ }
+
+ @Test
+ void testTestJvmOptValue()
+ {
+ applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
"/configuration/extraJvmOpts/-Xmx", "4g"));
+ }
+
+ @Test
+ void testTestJvmOptMismatchFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(TEST,
"/configuration/extraJvmOpts/-Xmx", "16g")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testAddInvalidJvmOptKeyFails()
+ {
+ assertThatThrownBy(() -> applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/invalidKey", "value")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Invalid JVM option key 'invalidKey'");
+ }
+
+ @Test
+ void testAddConflictingBooleanJvmOptFails()
+ {
+ Map<String, String> effectiveOpts = new LinkedHashMap<>();
+ effectiveOpts.put("-Xmx", "4g");
+ effectiveOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ baseConfig.cassandraYaml(), effectiveOpts);
+ CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-XX:+UseG1GC", "")));
+
+ assertThatThrownBy(() -> applyOps(effective, overlay,
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-XX:-UseG1GC", "")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Conflicting boolean JVM option");
+ }
+
+ @Test
+ void testAddConflictingBooleanJvmOptAgainstBaseFails()
+ {
+ // Conflicting option exists only in the effective config (i.e. base
template), not the overlay.
+ // The overlay-only check would miss this; the effective-config check
must catch it.
+ Map<String, String> effectiveOpts = new LinkedHashMap<>();
+ effectiveOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay effective = new
CassandraConfigurationOverlay(
+ baseConfig.cassandraYaml(), effectiveOpts);
+
+ assertThatThrownBy(() -> applyOps(effective, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-XX:-UseG1GC", "")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Conflicting boolean JVM option");
+ }
+
+ @Test
+ void testReplaceBooleanJvmOptByRemovingThenAdding()
+ {
+ // -XX:+UseG1GC lives only in the overlay (not the base template), so
removing it from the
+ // overlay clears it from the effective config, and the subsequent add
of -XX:-UseG1GC applies
+ // against the now-conflict-free effective configuration.
+ CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-XX:+UseG1GC", "")));
+
+ Map<String, String> baseOpts = new LinkedHashMap<>();
+ baseOpts.put("-Xmx", "4g");
+ CassandraConfigurationOverlay base = new CassandraConfigurationOverlay(
+ baseConfig.cassandraYaml(), baseOpts);
+
+ CassandraConfigurationOverlay newOverlay = applyOps(base, 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(baseConfig, 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(baseConfig,
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");
+ }
+
+ // --- Sequential (RFC 6902 section 5) operation semantics ---
+
+ @Test
+ void testReplaceThenTestSeesUpdatedValue()
+ {
+ // "Change X, then assert X now holds the new value" - the test op
must observe the prior replace.
+ applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/concurrent_reads", 64),
+ new ConfigurationPatchOperation(TEST,
"/configuration/cassandraYaml/concurrent_reads", 64));
+ }
+
+ @Test
+ void testTestAgainstStaleValueFails()
+ {
+ // After the replace, the value is 64, so a test for the original 32
must fail.
+ assertThatThrownBy(() -> applyOps(baseConfig, EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/concurrent_reads", 64),
+ new ConfigurationPatchOperation(TEST,
"/configuration/cassandraYaml/concurrent_reads", 32)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Test failed");
+ }
+
+ @Test
+ void testAddObjectThenAddChild()
+ {
+ // Build-up: create a new nested object, then add a child into it in
the same patch.
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
+
"/configuration/cassandraYaml/memtable/configurations/custom", new
JsonObject()),
+ new ConfigurationPatchOperation(ADD,
+
"/configuration/cassandraYaml/memtable/configurations/custom/class_name",
"CustomMemtable"));
+
+ JsonObject custom =
newOverlay.cassandraYaml().getJsonObject("memtable")
+
.getJsonObject("configurations").getJsonObject("custom");
+ assertThat(custom.getString("class_name")).isEqualTo("CustomMemtable");
+ }
+
+ @Test
+ void testAddTopLevelThenReplaceChild()
+ {
+ // Create a new top-level section, then replace a field within it.
+ CassandraConfigurationOverlay newOverlay = applyOps(baseConfig,
EMPTY_OVERLAY,
+ new ConfigurationPatchOperation(ADD,
"/configuration/cassandraYaml/new_section",
+ new JsonObject().put("a", 1)),
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/new_section/a", 2));
+
+
assertThat(newOverlay.cassandraYaml().getJsonObject("new_section").getInteger("a")).isEqualTo(2);
+ }
+
+ @Test
+ void testRemoveThenRemoveChildFails()
+ {
+ 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)))), null);
+
+ // Removing 'trie' first makes the subsequent remove of
trie/class_name reference a path that no
+ // longer exists in the overlay - it must fail rather than silently
no-op.
+ assertThatThrownBy(() -> applyOps(baseConfig, overlay,
+ new ConfigurationPatchOperation(REMOVE,
+
"/configuration/cassandraYaml/memtable/configurations/trie", null),
+ new ConfigurationPatchOperation(REMOVE,
+
"/configuration/cassandraYaml/memtable/configurations/trie/class_name", null)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in overlay");
+ }
+
+ @Test
+ void testRemoveOverlayOnlyTopLevelThenWriteChildFails()
+ {
+ // 'custom_section' exists only in the overlay. Removing it clears it
from the effective config,
+ // so replacing a child of it afterwards must fail instead of
resurrecting it.
+ CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(new JsonObject()
+ .put("custom_section", new JsonObject().put("a", 1)), null);
+
+ assertThatThrownBy(() -> applyOps(baseConfig, overlay,
+ new ConfigurationPatchOperation(REMOVE,
"/configuration/cassandraYaml/custom_section", null),
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/custom_section/a", 2)))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("does not exist in effective config");
+ }
+
+ @Test
+ void testRemoveOverlayBooleanOptThenAddOppositeStillConflictsAgainstBase()
+ {
+ // +UseG1GC exists in BOTH the base and the overlay. Removing the
overlay copy leaves the base's
+ // +UseG1GC in the effective config, so adding -UseG1GC must still be
rejected as a conflict.
+ Map<String, String> baseOpts = new LinkedHashMap<>();
+ baseOpts.put("-XX:+UseG1GC", "");
+ CassandraConfigurationOverlay base = new CassandraConfigurationOverlay(
+ baseConfig.cassandraYaml(), baseOpts);
+ CassandraConfigurationOverlay overlay = new
CassandraConfigurationOverlay(null,
+ new LinkedHashMap<>(Map.of("-XX:+UseG1GC", "")));
+
+ assertThatThrownBy(() -> applyOps(base, overlay,
+ new ConfigurationPatchOperation(REMOVE,
"/configuration/extraJvmOpts/-XX:+UseG1GC", null),
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-XX:-UseG1GC", "")))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("Conflicting boolean JVM option");
+ }
+
+ private static CassandraConfigurationOverlay
applyOps(CassandraConfigurationOverlay base,
+
CassandraConfigurationOverlay overlay,
+
ConfigurationPatchOperation... ops)
+ {
+ List<ConfigurationPatchValidator.ParsedPatchOperation> parsed =
+ ConfigurationPatchValidator.validate(List.of(ops));
+ return ConfigurationPatchApplier.apply(parsed, base, 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 00000000..391b51e8
--- /dev/null
+++
b/server/src/test/java/org/apache/cassandra/sidecar/configmanagement/ConfigurationPatchValidatorTest.java
@@ -0,0 +1,437 @@
+/*
+ * 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/cassandraYaml//key", "value"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("empty segment");
+ }
+
+ @Test
+ void testRejectsDuplicateMutationPaths()
+ {
+ List<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(TEST,
"/configuration/cassandraYaml/key", "old"),
+ new ConfigurationPatchOperation(REPLACE,
"/configuration/cassandraYaml/key", "new"));
+
+ List<ConfigurationPatchValidator.ParsedPatchOperation> parsed =
ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed).hasSize(2);
+ }
+
+ @Test
+ void testParsesTopLevelCassandraYamlPath()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/cassandraYaml/concurrent_reads", 64));
+
+ List<ConfigurationPatchValidator.ParsedPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
+
"/configuration/cassandraYaml/memtable/configurations/trie/class_name",
"value"));
+
+ List<ConfigurationPatchValidator.ParsedPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Xmx", "8g"));
+
+ List<ConfigurationPatchValidator.ParsedPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dcassandra.jmx.local.port", "7199"));
+
+ List<ConfigurationPatchValidator.ParsedPatchOperation> parsed =
ConfigurationPatchValidator.validate(ops);
+
+ assertThat(parsed).hasSize(1);
+
assertThat(parsed.get(0).topLevelKey()).isEqualTo("-Dcassandra.jmx.local.port");
+ }
+
+ @Test
+ void testAcceptsValidXFlag()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Xms", "2g"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsValidXXBooleanFlag()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-XX:+UseG1GC", ""));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsValidXXValueFlag()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-XX:MaxGCPauseMillis", "200"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testRejectsJavaAgentKey()
+ {
+ List<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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 testRejectsPathBearingJvmOpts()
+ {
+ // Options that write to arbitrary filesystem paths must be blocked by
key, since the value
+ // pattern legitimately permits absolute paths for Cassandra system
properties.
+ List<String> blockedPathOpts = List.of(
+ "-XX:ErrorFile", "-XX:HeapDumpPath", "-XX:LogFile",
+ "-XX:FlightRecorderOptions", "-XX:StartFlightRecording",
+ "-Xloggc", "-Xlog", "-Xbootclasspath");
+
+ for (String key : blockedPathOpts)
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/" + key, "/tmp/evil"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessage("Blocked extraJvmOpts key '" + key + "': this
JVM option is not allowed");
+ }
+ }
+
+ @Test
+ void testRejectsKeyWithEqualsSign()
+ {
+ List<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Xmx", "4g"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsJvmOptValueWithAbsolutePath()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dcassandra.logdir", "/var/log/cassandra"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsJvmOptValueWithJson()
+ {
+ String jsonValue = "{\"class_name\":\"SizeTieredCompactionStrategy\","
+ +
"\"parameters\":{\"min_threshold\":\"4\",\"max_threshold\":\"32\"}}";
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dcassandra.settings.default_compaction",
+ jsonValue));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testAcceptsJvmOptValueWithCommas()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dcassandra.seed_provider",
+
"org.apache.cassandra.locator.SimpleSeedProvider:127.0.0.1,127.0.0.2"));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithWhitespace()
+ {
+ // Whitespace is rejected because bin/cassandra word-splits unquoted
values during `eval`,
+ // silently truncating a value like "hello world" to "hello".
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dfoo", "hello world"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("disallowed characters");
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithPathTraversal()
+ {
+ List<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(ADD,
"/configuration/extraJvmOpts/-Dcassandra.logdir", "/tmp/../../etc/cron.d"));
+
+ assertThatThrownBy(() -> ConfigurationPatchValidator.validate(ops))
+ .isInstanceOf(ConfigurationPatchException.class)
+ .hasMessageContaining("path traversal");
+ }
+
+ @Test
+ void testRejectsJvmOptValueWithShellMetacharacters()
+ {
+ List<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> 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<ConfigurationPatchOperation> ops = List.of(
+ new ConfigurationPatchOperation(REMOVE,
"/configuration/extraJvmOpts/-Xmx", null));
+
+ assertThat(ConfigurationPatchValidator.validate(ops)).hasSize(1);
+ }
+
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
