Skip to content

Commit 8e1be27

Browse files
maltesanderclaude
andcommitted
refactor: Split config file building into build/properties
Introduce `controller/build/properties/` with one build step per config file and a `ConfigFileName` enum, mirroring trino/hdfs (adapted for OPA's JSON files, so the builders return serialized `String`s and there is no key-value writer). - `properties/config_json.rs`: the `config.json` builder (the former `build_config_file` plus the `OpaClusterConfigFile` struct family) with its own error type and the default/override-precedence unit tests. - `properties/user_info_fetcher.rs`: serializes `user-info-fetcher.json`. - `properties/mod.rs`: the `ConfigFileName` enum and a shared `test_support::validated_cluster_from_spec` helper. - `config_map.rs` shrinks to metadata + dispatch + assembly, wrapping the per-file builder errors. The Vector config file keeps coming from the existing logging call for now; it is folded into the build step in a follow-up commit. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f436d17 commit 8e1be27

5 files changed

Lines changed: 320 additions & 220 deletions

File tree

rust/operator-binary/src/controller/build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,4 @@
22
//! Kubernetes resource specifications.
33
44
pub mod config_map;
5+
pub mod properties;
Lines changed: 41 additions & 220 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,22 @@
1-
//! Builds the rolegroup [`ConfigMap`] (the OPA `config.json` and sidecar config) from the
2-
//! [`ValidatedCluster`], without reaching into the raw `OpaCluster` spec (the owner object is
3-
//! only used for the owner reference and object metadata).
1+
//! Assembles the rolegroup [`ConfigMap`] from the [`ValidatedCluster`], dispatching to the
2+
//! per-file builders in [`super::properties`]. The owner object is only used for the owner
3+
//! reference and object metadata, never for config content.
44
5-
use serde::{Deserialize, Serialize};
65
use snafu::{ResultExt, Snafu};
76
use stackable_operator::{
87
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
98
k8s_openapi::api::core::v1::ConfigMap,
109
kvp::ObjectLabels,
11-
product_logging::spec::{ContainerLogConfig, ContainerLogConfigChoice, LogLevel},
1210
role_utils::RoleGroupRef,
13-
v2::config_overrides::JsonConfigOverrides,
1411
};
1512

13+
use super::properties::{ConfigFileName, config_json, user_info_fetcher};
1614
use crate::{
17-
controller::{
18-
CONFIG_FILE, OPA_STACKABLE_SERVICE_NAME,
19-
validate::{OpaRoleGroupConfig, ValidatedCluster},
20-
},
21-
crd::{Container, OpaConfig, OpaConfigOverrides, v1alpha2},
15+
controller::validate::{OpaRoleGroupConfig, ValidatedCluster},
16+
crd::v1alpha2,
2217
product_logging::extend_role_group_config_map,
2318
};
2419

25-
/// Decision logging is disabled by default. It is enabled when the `decision` logger is set to a
26-
/// level other than `NONE`.
27-
const DEFAULT_DECISION_LOGGING_ENABLED: bool = false;
28-
2920
#[derive(Snafu, Debug)]
3021
pub enum Error {
3122
#[snafu(display("object is missing metadata to build owner reference"))]
@@ -38,8 +29,11 @@ pub enum Error {
3829
source: stackable_operator::builder::meta::Error,
3930
},
4031

41-
#[snafu(display("failed to serialize user info fetcher configuration"))]
42-
SerializeUserInfoFetcherConfig { source: serde_json::Error },
32+
#[snafu(display("failed to build config.json"))]
33+
BuildConfigJson { source: config_json::Error },
34+
35+
#[snafu(display("failed to build user-info-fetcher.json"))]
36+
BuildUserInfoFetcher { source: user_info_fetcher::Error },
4337

4438
#[snafu(display("failed to add the logging configuration to the ConfigMap [{cm_name}]"))]
4539
InvalidLoggingConfig {
@@ -52,12 +46,6 @@ pub enum Error {
5246
source: stackable_operator::builder::configmap::Error,
5347
rolegroup: RoleGroupRef<v1alpha2::OpaCluster>,
5448
},
55-
56-
#[snafu(display("failed to serialize config file {file:?}"))]
57-
SerializeConfigFile {
58-
source: serde_json::Error,
59-
file: String,
60-
},
6149
}
6250

6351
type Result<T, E = Error> = std::result::Result<T, E>;
@@ -83,17 +71,18 @@ pub fn build_rolegroup_config_map(
8371
.build();
8472

8573
cm_builder.metadata(metadata).add_data(
86-
CONFIG_FILE,
87-
build_config_file(
74+
ConfigFileName::ConfigJson.to_string(),
75+
config_json::build(
8876
&rolegroup_config.merged_config,
8977
&rolegroup_config.config_overrides,
90-
)?,
78+
)
79+
.context(BuildConfigJsonSnafu)?,
9180
);
9281

9382
if let Some(user_info) = &cluster.cluster_config.user_info {
9483
cm_builder.add_data(
95-
"user-info-fetcher.json",
96-
serde_json::to_string_pretty(user_info).context(SerializeUserInfoFetcherConfigSnafu)?,
84+
ConfigFileName::UserInfoFetcher.to_string(),
85+
user_info_fetcher::build(user_info).context(BuildUserInfoFetcherSnafu)?,
9786
);
9887
}
9988

@@ -113,161 +102,28 @@ pub fn build_rolegroup_config_map(
113102
})
114103
}
115104

116-
/// Builds the OPA `config.json` from the operator defaults and the merged user `configOverrides`.
117-
fn build_config_file(
118-
merged_config: &OpaConfig,
119-
config_overrides: &OpaConfigOverrides,
120-
) -> Result<String> {
121-
let mut decision_logging_enabled = DEFAULT_DECISION_LOGGING_ENABLED;
122-
123-
if let Some(ContainerLogConfig {
124-
choice: Some(ContainerLogConfigChoice::Automatic(log_config)),
125-
}) = merged_config.logging.containers.get(&Container::Opa)
126-
{
127-
if let Some(config) = log_config.loggers.get("decision") {
128-
decision_logging_enabled = config.level != LogLevel::NONE;
129-
}
130-
}
131-
132-
let decision_logging = if decision_logging_enabled {
133-
Some(OpaClusterConfigDecisionLog { console: true })
134-
} else {
135-
None
136-
};
137-
138-
let config = OpaClusterConfigFile::new(decision_logging);
139-
140-
let config_value =
141-
serde_json::to_value(&config).with_context(|_| SerializeConfigFileSnafu {
142-
file: CONFIG_FILE.to_string(),
143-
})?;
144-
145-
// Apply the merged user `configOverrides`. The merge built a sequence that applies the
146-
// role-level patch first, then the role-group-level patch on top. `apply` is infallible; an
147-
// invalid patch is logged and skipped.
148-
let config_json = JsonConfigOverrides::from(config_overrides.config_json.clone());
149-
let config_value = config_json.apply(&config_value);
150-
151-
serde_json::to_string_pretty(&config_value).with_context(|_| SerializeConfigFileSnafu {
152-
file: CONFIG_FILE.to_string(),
153-
})
154-
}
155-
156-
#[derive(Serialize, Deserialize)]
157-
pub struct OpaClusterConfigFile {
158-
services: Vec<OpaClusterConfigService>,
159-
bundles: OpaClusterBundle,
160-
#[serde(skip_serializing_if = "Option::is_none")]
161-
decision_logs: Option<OpaClusterConfigDecisionLog>,
162-
status: Option<OpaClusterConfigStatus>,
163-
}
164-
165-
impl OpaClusterConfigFile {
166-
pub fn new(decision_logging: Option<OpaClusterConfigDecisionLog>) -> Self {
167-
Self {
168-
services: vec![OpaClusterConfigService {
169-
name: OPA_STACKABLE_SERVICE_NAME.to_owned(),
170-
url: "http://localhost:3030/opa/v1".to_owned(),
171-
}],
172-
bundles: OpaClusterBundle {
173-
stackable: OpaClusterBundleConfig {
174-
service: OPA_STACKABLE_SERVICE_NAME.to_owned(),
175-
resource: "opa/bundle.tar.gz".to_owned(),
176-
persist: true,
177-
polling: OpaClusterBundleConfigPolling {
178-
min_delay_seconds: 10,
179-
max_delay_seconds: 20,
180-
},
181-
},
182-
},
183-
decision_logs: decision_logging,
184-
// Enable more Prometheus metrics, such as bundle loads
185-
// See https://www.openpolicyagent.org/docs/monitoring#status-metrics
186-
status: Some(OpaClusterConfigStatus { prometheus: true }),
187-
}
188-
}
189-
}
190-
191-
#[derive(Serialize, Deserialize)]
192-
struct OpaClusterConfigService {
193-
name: String,
194-
url: String,
195-
}
196-
197-
#[derive(Serialize, Deserialize)]
198-
struct OpaClusterBundle {
199-
stackable: OpaClusterBundleConfig,
200-
}
201-
202-
#[derive(Serialize, Deserialize)]
203-
struct OpaClusterBundleConfig {
204-
service: String,
205-
resource: String,
206-
persist: bool,
207-
polling: OpaClusterBundleConfigPolling,
208-
}
209-
210-
#[derive(Serialize, Deserialize)]
211-
struct OpaClusterBundleConfigPolling {
212-
min_delay_seconds: i32,
213-
max_delay_seconds: i32,
214-
}
215-
216-
#[derive(Serialize, Deserialize)]
217-
pub struct OpaClusterConfigDecisionLog {
218-
console: bool,
219-
}
220-
221-
#[derive(Serialize, Deserialize)]
222-
struct OpaClusterConfigStatus {
223-
prometheus: bool,
224-
}
225-
226105
#[cfg(test)]
227106
mod tests {
228107
use serde_json::{Value, json};
229-
use stackable_operator::{
230-
cli::OperatorEnvironmentOptions, kube::runtime::reflector::ObjectRef,
231-
};
108+
use stackable_operator::kube::runtime::reflector::ObjectRef;
232109

233110
use super::*;
234111
use crate::{
235-
controller::{build_recommended_labels, validate::validate},
112+
controller::{build::properties::test_support::validated_cluster_from_spec,
113+
build_recommended_labels},
236114
crd::OpaRole,
237115
};
238116

239-
/// Validates an `OpaCluster` built from the given `spec` and renders the ConfigMap of its first
240-
/// (and, in these tests, only) server role group.
117+
/// Renders the ConfigMap of the `default` server role group of an `OpaCluster` built from `spec`.
241118
fn build_config_map(spec: Value) -> ConfigMap {
242-
let opa: v1alpha2::OpaCluster = serde_json::from_value(json!({
243-
"apiVersion": "opa.stackable.tech/v1alpha2",
244-
"kind": "OpaCluster",
245-
"metadata": { "name": "test-opa", "namespace": "default", "uid": "42" },
246-
"spec": spec,
247-
}))
248-
.expect("invalid test input");
249-
250-
let operator_environment = OperatorEnvironmentOptions {
251-
operator_namespace: "stackable-operators".to_string(),
252-
operator_service_name: "opa-operator".to_string(),
253-
image_repository: "oci.stackable.tech/sdp".to_string(),
254-
};
255-
256-
let validated = validate(&opa, &operator_environment).expect("validation should succeed");
119+
let (opa, validated) = validated_cluster_from_spec(spec);
257120

258121
let role = OpaRole::Server;
259-
let role_group_configs = validated
260-
.role_group_configs
261-
.get(&role)
262-
.expect("the server role should be present");
263-
let (rg_name, rg_config) = role_group_configs
264-
.iter()
265-
.next()
266-
.expect("at least one role group");
122+
let rg_config = &validated.role_group_configs[&role]["default"];
267123
let rolegroup_ref = RoleGroupRef {
268124
cluster: ObjectRef::from_obj(&opa),
269125
role: role.to_string(),
270-
role_group: rg_name.clone(),
126+
role_group: "default".to_string(),
271127
};
272128
let recommended_labels = build_recommended_labels(
273129
&opa,
@@ -286,72 +142,37 @@ mod tests {
286142
.expect("the config map should build")
287143
}
288144

289-
/// Extracts and parses the rendered `config.json` from a ConfigMap.
290-
fn config_json(config_map: &ConfigMap) -> Value {
291-
let raw = config_map
292-
.data
293-
.as_ref()
294-
.expect("the config map should have data")
295-
.get(CONFIG_FILE)
296-
.expect("config.json should be present");
297-
serde_json::from_str(raw).expect("config.json should be valid JSON")
298-
}
299-
300145
#[test]
301-
fn renders_default_config_json() {
146+
fn renders_config_json_without_user_info() {
302147
let cm = build_config_map(json!({
303148
"image": { "productVersion": "1.2.3" },
304149
"servers": { "roleGroups": { "default": {} } },
305150
}));
306-
let config = config_json(&cm);
151+
let data = cm.data.as_ref().expect("config map data");
307152

308-
// The bundled stackable service and its default polling values.
309-
assert_eq!(config["services"][0]["name"], "stackable");
310-
assert_eq!(config["bundles"]["stackable"]["polling"]["min_delay_seconds"], 10);
311-
assert_eq!(config["bundles"]["stackable"]["polling"]["max_delay_seconds"], 20);
312-
// Prometheus status metrics are enabled, decision logs are off by default.
313-
assert_eq!(config["status"]["prometheus"], true);
314-
assert!(config.get("decision_logs").is_none_or(Value::is_null));
315-
// No user info configured -> no user-info-fetcher.json.
316-
assert!(!cm.data.as_ref().unwrap().contains_key("user-info-fetcher.json"));
153+
assert!(data.contains_key("config.json"));
154+
assert!(!data.contains_key("user-info-fetcher.json"));
317155
}
318156

319157
#[test]
320-
fn applies_role_then_role_group_config_json_overrides() {
158+
fn renders_user_info_fetcher_json_when_configured() {
321159
let cm = build_config_map(json!({
322160
"image": { "productVersion": "1.2.3" },
323-
"servers": {
324-
"configOverrides": {
325-
"config.json": { "jsonMergePatch": {
326-
"bundles": { "stackable": { "polling": {
327-
"min_delay_seconds": 3,
328-
"max_delay_seconds": 7,
329-
} } },
330-
"default_decision": "test/hello",
331-
} }
332-
},
333-
"roleGroups": { "default": {
334-
"configOverrides": {
335-
"config.json": { "jsonMergePatch": {
336-
"bundles": { "stackable": { "polling": {
337-
"max_delay_seconds": 5,
338-
} } },
339-
"labels": { "rolegroup": "default" },
340-
} }
161+
"clusterConfig": {
162+
"userInfo": {
163+
"backend": {
164+
"experimentalXfscAas": {
165+
"hostname": "aas.default.svc.cluster.local",
166+
"port": 5000,
167+
}
341168
}
342-
} }
169+
}
343170
},
171+
"servers": { "roleGroups": { "default": {} } },
344172
}));
345-
let config = config_json(&cm);
173+
let data = cm.data.as_ref().expect("config map data");
346174

347-
let polling = &config["bundles"]["stackable"]["polling"];
348-
// Role-only key survives.
349-
assert_eq!(polling["min_delay_seconds"], 3);
350-
// Role group wins over role for the shared key.
351-
assert_eq!(polling["max_delay_seconds"], 5);
352-
// Role-level addition is kept.
353-
assert_eq!(config["default_decision"], "test/hello");
354-
// Role-group-level addition is applied on top.
355-
assert_eq!(config["labels"]["rolegroup"], "default");
175+
assert!(data.contains_key("config.json"));
176+
assert!(data.contains_key("user-info-fetcher.json"));
356177
}
357178
}

0 commit comments

Comments
 (0)