feat: config overrides for structured config files by dervoeti · Pull Request #1177 · stackabletech/operator-rs

dervoeti · 2026-03-17T21:01:24Z

Description

Implementation of https://github.com/stackabletech/decisions/issues/73, needed for stackabletech/opa-operator#756

Problem

The existing configOverrides mechanism uses HashMap<String, HashMap<String, String>> (filename → flat key-value pairs). This works well for flat formats like .properties files and Hadoop XML, but it cannot express modifications to nested or structured formats like JSON. For example, there is no clean way to override min_delay_seconds inside a deeply nested OPA config.json.

Solution

This PR adds strategy-based configOverrides building blocks to operator-rs. Instead of a single flat map, operators can now compose typed override structs that choose a patch strategy per config file. The CRD schema explicitly encodes which strategies are supported for which files, which means invalid input is rejected by the Kubernetes API server before the operator ever sees it.

The general architecture for config overrides is: operator-rs handles merging (combining base config with user overrides), while the operator handles rendering (turning the merged result into the actual file content). For key-value files, the merging happens inside the existing product config pipeline. For structured files like JSON, the merging happens via e.g. JsonConfigOverrides::apply(). Both are operator-rs code. For rendering, the operator picks the right format: sometimes using shared helpers from operator-rs (e.g. properties file writers), sometimes doing it directly (e.g. serde_json::to_string_pretty).

Breaking changes to `CommonConfiguration`, `Role`, and `RoleGroup`

These structs gained a new required ConfigOverrides type parameter. There is no default, which means every operator must explicitly specify its ConfigOverrides type. This is a breaking change: all existing operators need to define their own config overrides struct and pass it as a type parameter.

The type parameters were also renamed for clarity: T to Config, U to RoleConfig, and ProductSpecificCommonConfig to CommonConfig.

New `config_overrides` module

It contains:

New patch strategies for JSON files:

JsonConfigOverrides: enum supporting three strategies for JSON config files:
- jsonMergePatch: RFC 7396, simple nested overrides expressed as YAML/JSON
- jsonPatches: RFC 6902, fine-grained operations (add, remove, replace, move, test)
- userProvided: full file replacement escape hatch
Typed key-value overrides:
- KeyValueConfigOverrides: typed wrapper for flat key-value files (.properties, Hadoop XML).
  Uses #[serde(flatten)] so it serializes identically to the old HashMap<String, String>.
KeyValueOverridesProvider trait:
- A uniform interface the product config pipeline uses to extract flat key-value overrides from any ConfigOverrides type. The default implementation returns an empty map, so operators that only use structured overrides (like OPA) don't need any custom logic.
  The shared product config pipeline (transform_all_roles_to_config) processes PropertyNameKind::File entries by calling get_key_value_overrides on the ConfigOverrides type. Rust requires this trait bound at compile time even if a particular operator never passes PropertyNameKind::File entries. Operators that only use structured overrides (like OPA with JsonConfigOverrides) can rely on the default no-op implementation. Every operator's config overrides type must implement this trait, but impl KeyValueOverridesProvider for OpaConfigOverrides {} (using the default no-op) is sufficient for operators that don't use key-value overrides.

Example for a hypothetical NiFi with the new typed overrides, which uses KeyValueConfigOverrides:

  struct NifiConfigOverrides {
      #[serde(rename = "nifi.properties")]
      nifi_properties: Option<KeyValueConfigOverrides>,

      #[serde(rename = "authorizers.xml")]
      authorizers_xml: Option<XmlConfigOverrides>,
  }

  impl KeyValueOverridesProvider for NifiConfigOverrides {
      fn get_key_value_overrides(&self, file: &str) -> BTreeMap<String, Option<String>> {
          match file {
              "nifi.properties" => self.nifi_properties
                  .as_ref()
                  .map(|kv| kv.as_overrides())
                  .unwrap_or_default(),
              _ => BTreeMap::new(),
          }
      }
  }

Why a generic type parameter instead of an enum?

An enum containing all strategies would mean every operator advertises support for every strategy on every file. The current approach (operators compose a struct from building blocks) lets the CRD schema precisely reflect what is actually supported. Invalid combinations are rejected at admission time, not at runtime.

What is NOT included

XmlConfigOverrides / XML patch strategy (RFC 5261): will be added when needed (e.g. for NiFi).

Definition of Done Checklist

Author

Changes are OpenShift compatible
CRD changes approved
CRD documentation for all fields, following the style guide.
Integration tests passed (for non trivial changes)
Changes need to be "offline" compatible

Reviewer

Code contains useful comments
Code contains useful logging statements
(Integration-)Test cases added
Documentation added or updated. Follows the style guide.
Changelog updated
Cargo.toml only contains references to git tags (not specific commits or branches)

Acceptance

Feature Tracker has been updated
Proper release label has been added

sbernauer

Only small stuff noticed during daily, real review will follow

crates/stackable-operator/src/config_overrides.rs

crates/stackable-operator/crds/DummyCluster.yaml

sbernauer · 2026-03-20T13:04:01Z

crates/stackable-operator/src/config_overrides.rs

+        ]);
+
+        let result = overrides.apply(&base);
+        assert!(result.is_err(), "removing a nonexistent path should fail");


Suggested change

assert!(result.is_err(), "removing a nonexistent path should fail");

assert!(

matches!(result.unwrap_err(), Error::ApplyJsonPatch { source } if source.to_string()

== "operation '/0' failed at path '/nonexistent': path is invalid"),

"removing a nonexistent path should fail"

);

sbernauer · 2026-03-20T13:08:27Z

crates/stackable-operator/src/config_overrides.rs

+        assert!(
+            result.is_err(),
+            "invalid patch operation should return an error"
+        );


Suggested change

assert!(

result.is_err(),

"invalid patch operation should return an error"

);

assert!(

matches!(result.unwrap_err(), Error::DeserializeJsonPatchOperation { source, index: 0 } if source.to_string()

== "missing field `op` at line 1 column 19"),

"invalid patch operation should return an error"

);

sbernauer · 2026-03-20T13:09:02Z

crates/stackable-operator/src/config_overrides.rs

+        let overrides = JsonConfigOverrides::UserProvided("not valid json".to_owned());
+
+        let result = overrides.apply(&base);
+        assert!(result.is_err(), "invalid JSON should return an error");


Suggested change

assert!(result.is_err(), "invalid JSON should return an error");

assert!(

matches!(result.unwrap_err(), Error::ParseUserProvidedJson { source } if source.to_string()

== "expected ident at line 1 column 2"),

"invalid JSON should return an error"

);

sbernauer · 2026-03-20T14:16:15Z

crates/stackable-operator/src/role_utils.rs

    T,
    U = GenericRoleConfig,


nit: It would be great if we could rename all T and U to Config and RoleConfg accordingly.

sbernauer · 2026-03-20T14:18:26Z

crates/stackable-operator/src/role_utils.rs

@@ -302,32 +311,36 @@ pub struct Role<
    T,
    U = GenericRoleConfig,
    ProductSpecificCommonConfig = GenericProductSpecificCommonConfig,


nit: It would be great to rename ProductSpecificCommonConfig to CommonConfig for consitency

sbernauer · 2026-03-20T14:19:01Z

crates/xtask/src/crd/dummy.rs

        #[serde(default)]
        pub object_overrides: ObjectOverrides,

+        json_config_overrides: Option<stackable_operator::config_overrides::JsonConfigOverrides>,


Please use this struct instead (this fields can than be removed)

#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize, JsonSchema)] #[schemars(crate = "stackable_operator::schemars")] pub struct ProductConfigOverrides { #[serde(rename = "my.json")] my_json: Option<JsonConfigOverrides>, }

a839d7e

(this is also the commit that switches from backwards-compatible config overrides to making it break for existing operators, so they are forced to opt in to the new config overrides)

…Config

dervoeti mentioned this pull request Mar 18, 2026

feat: config overrides stackabletech/opa-operator#818

Open

20 tasks

dervoeti moved this to Development: Waiting for Review in Stackable Engineering Mar 19, 2026

dervoeti added this to Stackable Engineering Mar 19, 2026

dervoeti self-assigned this Mar 19, 2026

sbernauer self-requested a review March 19, 2026 08:06

dervoeti moved this from Development: Waiting for Review to Development: In Review in Stackable Engineering Mar 19, 2026

sbernauer reviewed Mar 19, 2026

View reviewed changes

crates/stackable-operator/src/config_overrides.rs Outdated Show resolved Hide resolved

crates/stackable-operator/src/config_overrides.rs Outdated Show resolved Hide resolved

crates/stackable-operator/crds/DummyCluster.yaml Show resolved Hide resolved

dervoeti force-pushed the feat/config-overrides-json branch from b0e1dc8 to 8d9c6cd Compare March 19, 2026 13:27

sbernauer reviewed Mar 20, 2026

View reviewed changes

Techassi self-requested a review March 23, 2026 15:32

dervoeti force-pushed the feat/config-overrides-json branch 2 times, most recently from 88710df to abf16cd Compare March 26, 2026 15:27

dervoeti added 4 commits March 26, 2026 16:27

feat: config overrides for structured config files

4d18a9a

chore: regenerate CRD preview

b3a92e7

chore: apply nightly rustfmt formatting

e10c6db

fix: addressed first round of review feedback

68f5837

dervoeti force-pushed the feat/config-overrides-json branch from abf16cd to 20e94e3 Compare March 26, 2026 15:28

dervoeti added 4 commits March 26, 2026 16:44

refactor!: remove HashMap default for ConfigOverrides type parameter

a839d7e

refactor: rename ProductSpecificCommonConfig type parameter to Common…

48ac144

…Config

fix: use matches! assertions to verify specific error variants in tests

0c5ac02

refactor: rename T and U type parameters to Config and RoleConfig

4b84601

dervoeti force-pushed the feat/config-overrides-json branch from 20e94e3 to 4b84601 Compare March 26, 2026 16:36

fix: apply rustfmt formatting

b19c190

dervoeti mentioned this pull request Mar 27, 2026

feat: typed config overrides stackabletech/nifi-operator#917

Draft

20 tasks

Uh oh!

Conversation

dervoeti commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Breaking changes to CommonConfiguration, Role, and RoleGroup

New config_overrides module

Why a generic type parameter instead of an enum?

What is NOT included

Definition of Done Checklist

Author

Reviewer

Acceptance

Uh oh!

sbernauer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dervoeti commented Mar 17, 2026 •

edited

Loading

Breaking changes to `CommonConfiguration`, `Role`, and `RoleGroup`

New `config_overrides` module