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

[pauloricardomg]

Summary

Implements patch operations for the Configuration Manager using RFC 6902-inspired semantics. Paths reference the effective configuration structure (base template + overlay merged) but mutations target the overlay only. Supported operations: add, remove, replace, test. move and copy are intentionally excluded as they introduce ambiguous semantics in the two-tier configuration model.

Configuration Model

The configuration uses a two-tier model:

• Base template: the stock cassandra.yaml provided by the operator, read-only via this API.
• Overlay: a sparse set of values explicitly set via patch operations, stored by the ConfigurationProvider.
• Effective config: the result of deep-merging base template + overlay. Overlay values take precedence. This is what the GET API returns and what Cassandra uses at startup.

Base Template          Overlay              Effective Config
+-----------------+    +-------------+      +-------------------+
| cluster: "prod" |    | reads: 128  |  =>  | cluster: "prod"   |
| reads: 32       | +  | flush: 8    |      | reads: 128        |
| flush: 4        |    +-------------+      | flush: 8          |
+-----------------+                         +-------------------+

Patch operations target paths in the effective config (what the client sees from GET) but only mutate the overlay. The base template is never modified by this API.

Overview

Patch Operations

Operation	Behavior	Precondition
add	Upsert value in overlay	Parent must exist in effective config (RFC 6902)
remove	Remove key from overlay; effective config falls back to template	Key must exist in overlay (not just template)
replace	Set value in overlay	Key must exist in effective config
test	Assert effective config value matches expected; no mutation	Path must exist in effective config

Copy-Siblings Strategy (nested paths)

When a patch targets a nested path within a top-level cassandraYaml key, the entire top-level key value is copied from the effective config into the overlay before applying the leaf change. This ensures the overlay is always self-contained at the top-level key granularity, preventing orphan fragments if the base template later removes the parent structure.

Example:

Template:

{"memtable": {"configurations": {"trie": {"class_name": "TrieMemtable", "max_shard_count": 4}, "skiplist": {"class_name": "SkipListMemtable"}}}}

Patch: replace /configuration/cassandraYaml/memtable/configurations/trie/max_shard_count with value 8

Resulting overlay (entire memtable key stored, not just the leaf):

{"memtable": {"configurations": {"trie": {"class_name": "TrieMemtable", "max_shard_count": 8}, "skiplist": {"class_name": "SkipListMemtable"}}}}

The trade-off is that sibling fields are "pinned" in the overlay. If the base template later modifies skiplist, the overlay's copy takes precedence. Operators can remove the sibling path from the overlay to re-sync with the template.

Main Classes

Class	Responsibility
ConfigurationPatchOperation	Value object: Op enum (ADD, REMOVE, REPLACE, TEST) + path + value
ConfigurationPatchValidator	Validates ops before application: path format, value presence, duplicate detection, segment parsing
ConfigurationPatchApplier	Applies validated ops: precondition checks (phase 1), then mutations (phase 2) for atomicity
ConfigurationPatchException	Thrown on validation/application failures with the failing operation attached
ConfigurationConflictException	Thrown on hash mismatch (optimistic concurrency)
ConfigurationManager.patchConfiguration	Orchestrates: validate ops, check hash, apply patch, CAS via provider

Validation Flow

1. Structural validation (ConfigurationPatchValidator): path prefix, segment parsing, value presence, duplicate mutation paths
2. Hash check (ConfigurationManager): caller's expectedHash vs current effective config hash
3. Precondition checks (ConfigurationPatchApplier phase 1): per-op checks against effective config and overlay
4. Mutations (ConfigurationPatchApplier phase 2): only if all preconditions pass

Additional Validations

• JVM option key format: must start with -D, -X, or -XX:
• Conflicting boolean JVM opts: -XX:+Foo and -XX:-Foo cannot coexist (remove first, then add)
• Nested paths into array values: rejected with clear error (array index traversal not supported)
• extraJvmOpts paths must be flat: no nested segments allowed

Test Overview

Test Class	Focus
ConfigurationPatchValidatorTest	Path parsing, value presence, duplicates, error cases
ConfigurationPatchApplierTest	All 4 ops at top-level and nested, copy-siblings, arrays, JVM validation, atomicity
ConfigurationManagerTest	End-to-end through the manager: hash conflicts, provider failures, concurrency

[skoppu22]

If incoming patch request has a conflicting jvm option against base yaml, this ConfigurationPatchApplier.apply only checks against overlay and doesn't detect conflict. storeOverlay call below stores it. Then baseSnapshot.overlay call below detects conflict with the base, prints warning and returns config without the conflicting option to the caller. The same thing happens in subsequent calls as well.
Also, returned config should be same as stored config, which is not true in this case.

[skoppu22]

Good to have test case for the same

[pauloricardomg]

Good catch! Addressed in 17fb889.
apply() now checks boolean JVM opt conflicts against the effective config (base + overlay), not just the overlay, and rejects the patch before storeOverlay is called. Stored and returned configs no longer diverge, and the warn-and-drop path in baseSnapshot.overlay() is no longer hit.

Tests: ConfigurationManagerTest.testPatchConflictingBooleanJvmOptRejected (rejected, nothing stored) and testPatchReturnedConfigMatchesStoredConfig (returned == re-read).

[skoppu22]

There are more options which can take paths etc e.g. -XX:ErrorFile, -XX:HeapDumpPath, -XX:LogFile, -Xloggc, and -D-style system properties. All absolute paths are allowed. Good to have allowlist instead of blocklist to be specific about which options are allowed and which paths are allowed.

[pauloricardomg]

Addressed in 5c67693 by expanding the blocklist to cover path-bearing/file-writing options (-XX:ErrorFile, -XX:HeapDumpPath, -XX:LogFile, -XX:FlightRecorderOptions, -XX:StartFlightRecording, -Xloggc, -Xlog, -Xbootclasspath) alongside the existing command-executing -XX:OnError/-XX:OnOutOfMemoryError.

I honestly don't know the full set of valid JVM configs, and an allowlist would mean enumerating every legitimate Cassandra JVM option (all the -Xms/-Xmx/-Xss/-XX GC and heap tuning flags plus the whole -Dcassandra.* / -Dcom.sun.management.* / -Djava.* system-property surface), which would make the list massive and hard to keep current. This isn't a security issue in practice: the patch endpoint requires admin credentials, so we're just removing the obviously insecure options and can add more as we find them. If you feel strongly about an allowlist, I'm happy to switch to one.

[skoppu22]

As we anyhow converting expected to JsonObject, can we expect the caller to pass both actual and expected as JsonObject, so we don't have to worry if passed params are not exactly matching instanceof expectations?

[pauloricardomg]

Addressed in d973ec4. The patch handler builds each operation's value via JsonObject.getValue() (from context.body().asJsonArray()), which returns Vert.x JSON types - JsonObject for objects, JsonArray for arrays, or scalars - the same types resolveValue returns for the effective config. So both sides are already comparable and the TEST precondition is now a plain Objects.equals with no instanceof/conversion. Added coverage for object- and array-valued TEST operations.

[skoppu22]

Well structured code !