Skip to content

Commit 660373f

Browse files
maltesanderclaude
andcommitted
refactor: Fold discovery into the build step (ValidatedCluster)
Move discovery.rs into controller/build/discovery.rs and refactor build_discovery_config_map to take the ValidatedCluster + role Service + cluster_info, reading the TLS scheme/port and secret class from cluster_config and the app version from the resolved image. The owner object is used only for the owner reference and object metadata. ValidatedClusterConfig gains a `tls` field (alongside `user_info`), mirroring trino/hdfs. The discovery ConfigMap name now comes from `cluster.name`, which finally consumes that field and removes its `#[allow(dead_code)]`. Adds unit tests asserting the discovery URL (http/https + port) and the OPA_SECRET_CLASS entry with and without TLS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2f5f7cb commit 660373f

6 files changed

Lines changed: 183 additions & 128 deletions

File tree

rust/operator-binary/src/controller.rs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ use crate::{
7272
OpaClusterStatus, OpaConfig, OpaRole, user_info_fetcher, v1alpha2,
7373
},
7474
controller::build::properties::logging::BundleBuilderLogLevel,
75-
discovery::{self, build_discovery_configmaps},
7675
operations::graceful_shutdown::add_graceful_shutdown_config,
7776
service::{
7877
self, APP_PORT, APP_PORT_NAME, build_rolegroup_headless_service,
@@ -228,7 +227,7 @@ pub enum Error {
228227
},
229228

230229
#[snafu(display("failed to build discovery ConfigMap"))]
231-
BuildDiscoveryConfig { source: discovery::Error },
230+
BuildDiscoveryConfig { source: build::discovery::Error },
232231

233232
#[snafu(display("failed to apply discovery ConfigMap"))]
234233
ApplyDiscoveryConfig {
@@ -473,20 +472,17 @@ pub async fn reconcile_opa(
473472
.context(ApplyPatchRoleGroupDaemonSetSnafu { rolegroup })?;
474473
}
475474

476-
for discovery_cm in build_discovery_configmaps(
477-
opa,
478-
opa,
479-
&validated.image,
475+
let discovery_cm = build::discovery::build_discovery_config_map(
476+
&validated,
480477
&server_role_service,
481478
&client.kubernetes_cluster_info,
479+
opa,
482480
)
483-
.context(BuildDiscoveryConfigSnafu)?
484-
{
485-
cluster_resources
486-
.add(client, discovery_cm)
487-
.await
488-
.context(ApplyDiscoveryConfigSnafu)?;
489-
}
481+
.context(BuildDiscoveryConfigSnafu)?;
482+
cluster_resources
483+
.add(client, discovery_cm)
484+
.await
485+
.context(ApplyDiscoveryConfigSnafu)?;
490486

491487
let cluster_operation_cond_builder =
492488
ClusterOperationsConditionBuilder::new(&opa.spec.cluster_operation);

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,5 @@
22
//! Kubernetes resource specifications.
33
44
pub mod config_map;
5+
pub mod discovery;
56
pub mod properties;
Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
//! Builds the discovery [`ConfigMap`] clients use to connect to an `OpaCluster`.
2+
//!
3+
//! The content comes entirely from the [`ValidatedCluster`] (plus the externally-resolved role
4+
//! [`Service`] and `cluster_info`); the owner object is only used for the owner reference and
5+
//! object metadata.
6+
7+
use snafu::{OptionExt, ResultExt, Snafu};
8+
use stackable_operator::{
9+
builder::{configmap::ConfigMapBuilder, meta::ObjectMetaBuilder},
10+
k8s_openapi::api::core::v1::{ConfigMap, Service},
11+
kube::runtime::reflector::ObjectRef,
12+
utils::cluster_info::KubernetesClusterInfo,
13+
};
14+
15+
use crate::{
16+
controller::{build_recommended_labels, validate::ValidatedCluster},
17+
crd::{OpaRole, v1alpha2},
18+
service::{APP_PORT, APP_TLS_PORT},
19+
};
20+
21+
#[derive(Snafu, Debug)]
22+
pub enum Error {
23+
#[snafu(display("object {} is missing metadata to build owner reference", opa))]
24+
ObjectMissingMetadataForOwnerRef {
25+
source: stackable_operator::builder::meta::Error,
26+
opa: ObjectRef<v1alpha2::OpaCluster>,
27+
},
28+
29+
#[snafu(display("the role Service has no name associated"))]
30+
NoServiceName,
31+
32+
#[snafu(display("the role Service has no namespace associated"))]
33+
NoServiceNamespace,
34+
35+
#[snafu(display("failed to build ConfigMap"))]
36+
BuildConfigMap {
37+
source: stackable_operator::builder::configmap::Error,
38+
},
39+
40+
#[snafu(display("failed to build object meta data"))]
41+
ObjectMeta {
42+
source: stackable_operator::builder::meta::Error,
43+
},
44+
}
45+
46+
type Result<T, E = Error> = std::result::Result<T, E>;
47+
48+
/// Builds the discovery [`ConfigMap`] containing the URL (and, when TLS is enabled, the secret
49+
/// class) clients need to connect to the cluster.
50+
pub fn build_discovery_config_map(
51+
cluster: &ValidatedCluster,
52+
svc: &Service,
53+
cluster_info: &KubernetesClusterInfo,
54+
owner: &v1alpha2::OpaCluster,
55+
) -> Result<ConfigMap> {
56+
let name = cluster.name.to_string();
57+
58+
let (scheme, port) = if cluster.cluster_config.tls.is_some() {
59+
("https", APP_TLS_PORT)
60+
} else {
61+
("http", APP_PORT)
62+
};
63+
64+
let url = format!(
65+
"{scheme}://{service_name}.{namespace}.svc.{cluster_domain}:{port}/",
66+
service_name = svc.metadata.name.as_deref().context(NoServiceNameSnafu)?,
67+
namespace = svc
68+
.metadata
69+
.namespace
70+
.as_deref()
71+
.context(NoServiceNamespaceSnafu)?,
72+
cluster_domain = cluster_info.cluster_domain,
73+
);
74+
75+
let metadata = ObjectMetaBuilder::new()
76+
.name_and_namespace(owner)
77+
.name(&name)
78+
.ownerreference_from_resource(owner, None, Some(true))
79+
.with_context(|_| ObjectMissingMetadataForOwnerRefSnafu {
80+
opa: ObjectRef::from_obj(owner),
81+
})?
82+
.with_recommended_labels(&build_recommended_labels(
83+
owner,
84+
&cluster.image.app_version_label_value,
85+
&OpaRole::Server.to_string(),
86+
"discovery",
87+
))
88+
.context(ObjectMetaSnafu)?
89+
.build();
90+
91+
let mut cm_builder = ConfigMapBuilder::new();
92+
cm_builder.metadata(metadata).add_data("OPA", url);
93+
94+
if let Some(tls) = &cluster.cluster_config.tls {
95+
cm_builder.add_data("OPA_SECRET_CLASS", &tls.server_secret_class);
96+
}
97+
98+
cm_builder.build().context(BuildConfigMapSnafu)
99+
}
100+
101+
#[cfg(test)]
102+
mod tests {
103+
use stackable_operator::{
104+
commons::networking::DomainName,
105+
k8s_openapi::api::core::v1::{Service, ServiceSpec},
106+
kube::api::ObjectMeta,
107+
utils::cluster_info::KubernetesClusterInfo,
108+
};
109+
110+
use super::*;
111+
use crate::controller::build::properties::test_support::validated_cluster_from_spec;
112+
113+
fn role_service() -> Service {
114+
Service {
115+
metadata: ObjectMeta {
116+
name: Some("test-opa-server".to_owned()),
117+
namespace: Some("default".to_owned()),
118+
..ObjectMeta::default()
119+
},
120+
spec: Some(ServiceSpec::default()),
121+
status: None,
122+
}
123+
}
124+
125+
fn cluster_info() -> KubernetesClusterInfo {
126+
KubernetesClusterInfo {
127+
cluster_domain: DomainName::try_from("cluster.local").unwrap(),
128+
}
129+
}
130+
131+
#[test]
132+
fn renders_http_url_without_tls() {
133+
let (opa, validated) = validated_cluster_from_spec(serde_json::json!({
134+
"image": { "productVersion": "1.2.3" },
135+
"servers": { "roleGroups": { "default": {} } },
136+
}));
137+
138+
let cm =
139+
build_discovery_config_map(&validated, &role_service(), &cluster_info(), &opa).unwrap();
140+
let data = cm.data.unwrap();
141+
142+
assert_eq!(
143+
data.get("OPA").map(String::as_str),
144+
Some("http://test-opa-server.default.svc.cluster.local:8081/")
145+
);
146+
assert!(!data.contains_key("OPA_SECRET_CLASS"));
147+
}
148+
149+
#[test]
150+
fn renders_https_url_and_secret_class_with_tls() {
151+
let (opa, validated) = validated_cluster_from_spec(serde_json::json!({
152+
"image": { "productVersion": "1.2.3" },
153+
"clusterConfig": { "tls": { "serverSecretClass": "tls" } },
154+
"servers": { "roleGroups": { "default": {} } },
155+
}));
156+
157+
let cm =
158+
build_discovery_config_map(&validated, &role_service(), &cluster_info(), &opa).unwrap();
159+
let data = cm.data.unwrap();
160+
161+
assert_eq!(
162+
data.get("OPA").map(String::as_str),
163+
Some("https://test-opa-server.default.svc.cluster.local:8443/")
164+
);
165+
assert_eq!(data.get("OPA_SECRET_CLASS").map(String::as_str), Some("tls"));
166+
}
167+
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,10 @@ use stackable_operator::{
1717
};
1818
use strum::IntoEnumIterator;
1919

20-
use crate::crd::{OpaConfig, OpaConfigOverrides, OpaRole, user_info_fetcher, v1alpha2};
20+
use crate::crd::{
21+
OpaConfig, OpaConfigOverrides, OpaRole, user_info_fetcher,
22+
v1alpha2::{self, OpaTls},
23+
};
2124

2225
#[derive(Snafu, Debug)]
2326
pub enum Error {
@@ -43,8 +46,6 @@ type Result<T, E = Error> = std::result::Result<T, E>;
4346
/// for every role group, ready to be turned into Kubernetes resources without touching the raw
4447
/// `OpaCluster` spec again (except for owner references).
4548
pub struct ValidatedCluster {
46-
// TODO: consumed by the Service / StatefulSet build steps in the follow-up PR.
47-
#[allow(dead_code)]
4849
pub name: ClusterName,
4950
pub image: ResolvedProductImage,
5051
pub cluster_config: ValidatedClusterConfig,
@@ -55,6 +56,7 @@ pub struct ValidatedCluster {
5556
/// raw `OpaCluster` to render config (except for owner references).
5657
pub struct ValidatedClusterConfig {
5758
pub user_info: Option<user_info_fetcher::v1alpha2::Config>,
59+
pub tls: Option<OpaTls>,
5860
}
5961

6062
/// The validated configuration of a single role group.
@@ -118,6 +120,7 @@ pub fn validate(
118120
image,
119121
cluster_config: ValidatedClusterConfig {
120122
user_info: opa.spec.cluster_config.user_info.clone(),
123+
tls: opa.spec.cluster_config.tls.clone(),
121124
},
122125
role_group_configs,
123126
})

rust/operator-binary/src/discovery.rs

Lines changed: 0 additions & 111 deletions
This file was deleted.

rust/operator-binary/src/main.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ use crate::{
3838

3939
mod controller;
4040
mod crd;
41-
mod discovery;
4241
mod operations;
4342
mod service;
4443
mod webhooks;

0 commit comments

Comments
 (0)