Skip to content

Commit 81598e1

Browse files
dervoeticlaude
andcommitted
docs: update PR description for breaking ConfigOverrides changes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent aaa1c23 commit 81598e1

1 file changed

Lines changed: 106 additions & 0 deletions

File tree

config-override-pr-description.md

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
# Description
2+
3+
Implementation of https://github.com/stackabletech/decisions/issues/73, needed for https://github.com/stackabletech/opa-operator/issues/756
4+
5+
## Problem
6+
7+
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`.
8+
9+
## Solution
10+
11+
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.
12+
13+
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`).
14+
15+
### Changes to `CommonConfiguration`, `Role`, and `RoleGroup`
16+
17+
These structs gained a new `ConfigOverrides` type parameter that **must be specified by every operator**. There is no default — operators are required to define a typed override struct (e.g. `OpaConfigOverrides`) that precisely describes which config files can be overridden and how. On `Role`, `ConfigOverrides` is the second type parameter (right after `Config`), so that `RoleConfig` and `ProductSpecificCommonConfig` can still use their existing defaults:
18+
19+
```rust
20+
// Role<Config, ConfigOverrides, RoleConfig = GenericRoleConfig, ProductSpecificCommonConfig = GenericProductSpecificCommonConfig>
21+
pub servers: Role<OpaConfigFragment, OpaConfigOverrides, EmptyRoleConfig>,
22+
```
23+
24+
`CommonConfiguration` and `RoleGroup` also require `ConfigOverrides` as the third type parameter (no default):
25+
26+
```rust
27+
// CommonConfiguration<Config, ProductSpecificCommonConfig, ConfigOverrides>
28+
// RoleGroup<Config, ProductSpecificCommonConfig, ConfigOverrides>
29+
```
30+
31+
This is a **breaking change** — all existing operators must be migrated to define and use typed config overrides.
32+
33+
### New `config_overrides` module
34+
35+
It contains:
36+
37+
New patch strategies for JSON files:
38+
- `JsonConfigOverrides`: enum supporting three strategies for JSON config files:
39+
- `jsonMergePatch`: [RFC 7396](https://datatracker.ietf.org/doc/html/rfc7396), simple nested overrides expressed as YAML/JSON
40+
- `jsonPatches`: [RFC 6902](https://datatracker.ietf.org/doc/html/rfc6902), fine-grained operations (add, remove, replace, move, test)
41+
- `userProvided`: full file replacement escape hatch
42+
43+
- Typed key-value overrides:
44+
- `KeyValueConfigOverrides`: typed wrapper for flat key-value files (`.properties`, Hadoop XML).
45+
Uses `#[serde(flatten)]` so it serializes identically to the old `HashMap<String, String>`.
46+
47+
- `KeyValueOverridesProvider` trait:
48+
- 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.
49+
- 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. The `impl KeyValueOverridesProvider for OpaConfigOverrides {}` line is the small price for having exact types in the CRD.
50+
51+
Example for a hypothetical NiFi with the new typed overrides, which uses `KeyValueConfigOverrides`:
52+
53+
```rust
54+
struct NifiConfigOverrides {
55+
#[serde(rename = "nifi.properties")]
56+
nifi_properties: Option<KeyValueConfigOverrides>,
57+
58+
#[serde(rename = "authorizers.xml")]
59+
authorizers_xml: Option<XmlConfigOverrides>,
60+
}
61+
62+
impl KeyValueOverridesProvider for NifiConfigOverrides {
63+
fn get_key_value_overrides(&self, file: &str) -> BTreeMap<String, Option<String>> {
64+
match file {
65+
"nifi.properties" => self.nifi_properties
66+
.as_ref()
67+
.map(|kv| kv.as_overrides())
68+
.unwrap_or_default(),
69+
_ => BTreeMap::new(),
70+
}
71+
}
72+
}
73+
```
74+
75+
### Why a generic type parameter instead of an enum?
76+
77+
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.
78+
79+
### What is NOT included
80+
81+
- `XmlConfigOverrides` / XML patch strategy (RFC 5261): will be added when needed (e.g. for NiFi).
82+
83+
## Definition of Done Checklist
84+
85+
### Author
86+
87+
- [ ] Changes are OpenShift compatible
88+
- [x] CRD changes approved
89+
- [x] CRD documentation for all fields, following the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs/style-guide).
90+
- [ ] Integration tests passed (for non trivial changes)
91+
- [ ] Changes need to be "offline" compatible
92+
93+
### Reviewer
94+
95+
- [ ] Code contains useful comments
96+
- [ ] Code contains useful logging statements
97+
- [ ] (Integration-)Test cases added
98+
- [ ] Documentation added or updated. Follows the [style guide](https://docs.stackable.tech/home/nightly/contributor/docs/style-guide).
99+
- [ ] Changelog updated
100+
- [ ] Cargo.toml only contains references to git tags (not specific commits or branches)
101+
102+
### Acceptance
103+
104+
- [ ] Feature Tracker has been updated
105+
- [ ] Proper release label has been added
106+

0 commit comments

Comments
 (0)