Skip to content

Commit 28ad585

Browse files
authored
refactor: extract dereference/validate pipeline from reconcile (#836)
* feat: extract dereference and validate steps * fix: update crate hashes * feat: add smoke snapshot test * doc: adapted changelog * fix(test): remove hardcoded 1 replica from 1 node test cluster * fix: remove empty dereference step
1 parent 343d350 commit 28ad585

9 files changed

Lines changed: 489 additions & 116 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ All notable changes to this project will be documented in this file.
1515
- Set `maxSurge=1` and `maxUnavailable=0` on the OPA DaemonSet rolling update strategy to eliminate
1616
availability gaps during rolling updates ([#819]).
1717
- Document Helm deployed RBAC permissions and remove unnecessary permissions ([#820]).
18+
- Internal operator refactoring: introduce dereference() and validate() steps in the reconciler ([#836]).
1819

1920
[#818]: https://github.com/stackabletech/opa-operator/pull/818
2021
[#819]: https://github.com/stackabletech/opa-operator/pull/819
2122
[#820]: https://github.com/stackabletech/opa-operator/pull/820
2223
[#830]: https://github.com/stackabletech/opa-operator/pull/830
2324
[#831]: https://github.com/stackabletech/opa-operator/pull/831
25+
[#836]: https://github.com/stackabletech/opa-operator/pull/836
2426

2527
## [26.3.0] - 2026-03-16
2628

Cargo.nix

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crate-hashes.json

Lines changed: 9 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

rust/operator-binary/src/controller.rs

Lines changed: 22 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use stackable_operator::{
2626
cli::OperatorEnvironmentOptions,
2727
cluster_resources::{ClusterResourceApplyStrategy, ClusterResources},
2828
commons::{
29-
product_image_selection::{self, ResolvedProductImage},
29+
product_image_selection::ResolvedProductImage,
3030
rbac::build_rbac_resources,
3131
secret_class::{
3232
SecretClassVolume, SecretClassVolumeProvisionParts, SecretClassVolumeScope,
@@ -54,7 +54,6 @@ use stackable_operator::{
5454
kvp::{LabelError, Labels, ObjectLabels},
5555
logging::controller::ReconcilerError,
5656
memory::{BinaryMultiple, MemoryQuantity},
57-
product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config},
5857
product_logging::{
5958
self,
6059
framework::{
@@ -89,6 +88,8 @@ use crate::{
8988
},
9089
};
9190

91+
mod validate;
92+
9293
pub const OPA_CONTROLLER_NAME: &str = "opacluster";
9394
pub const OPA_FULL_CONTROLLER_NAME: &str = concatcp!(OPA_CONTROLLER_NAME, '.', OPERATOR_NAME);
9495

@@ -230,11 +231,6 @@ pub enum Error {
230231
source: stackable_operator::client::Error,
231232
},
232233

233-
#[snafu(display("invalid product config"))]
234-
InvalidProductConfig {
235-
source: stackable_operator::product_config_utils::Error,
236-
},
237-
238234
#[snafu(display("object is missing metadata to build owner reference"))]
239235
ObjectMissingMetadataForOwnerRef {
240236
source: stackable_operator::builder::meta::Error,
@@ -248,11 +244,6 @@ pub enum Error {
248244
source: stackable_operator::cluster_resources::Error,
249245
},
250246

251-
#[snafu(display("failed to transform configs"))]
252-
ProductConfigTransform {
253-
source: stackable_operator::product_config_utils::Error,
254-
},
255-
256247
#[snafu(display("failed to resolve and merge config for role and role group"))]
257248
FailedToResolveConfig { source: crate::crd::Error },
258249

@@ -332,14 +323,12 @@ pub enum Error {
332323
source: builder::pod::container::Error,
333324
},
334325

335-
#[snafu(display("failed to resolve product image"))]
336-
ResolveProductImage {
337-
source: product_image_selection::Error,
338-
},
339-
340326
#[snafu(display("failed to build service"))]
341327
BuildService { source: service::Error },
342328

329+
#[snafu(display("failed to validate cluster"))]
330+
ValidateCluster { source: validate::Error },
331+
343332
#[snafu(display("failed to build TLS volume"))]
344333
TlsVolumeBuild {
345334
source: builder::pod::volume::SecretOperatorVolumeSourceBuilderError,
@@ -448,15 +437,12 @@ pub async fn reconcile_opa(
448437
let opa_ref = ObjectRef::from_obj(opa);
449438

450439
let client = &ctx.client;
451-
let resolved_product_image = opa
452-
.spec
453-
.image
454-
.resolve(
455-
CONTAINER_IMAGE_BASE_NAME,
456-
&ctx.operator_environment.image_repository,
457-
crate::built_info::PKG_VERSION,
458-
)
459-
.context(ResolveProductImageSnafu)?;
440+
441+
// NOTE(@maltesander): There currently is no dereference (client required) step for OPA.
442+
// validate (no client required)
443+
let validated = validate::validate(opa, &ctx.operator_environment, &ctx.product_config)
444+
.context(ValidateClusterSnafu)?;
445+
460446
let opa_role = OpaRole::Server;
461447

462448
let mut cluster_resources = ClusterResources::new(
@@ -469,36 +455,14 @@ pub async fn reconcile_opa(
469455
)
470456
.context(FailedToCreateClusterResourcesSnafu)?;
471457

472-
let validated_config = validate_all_roles_and_groups_config(
473-
&resolved_product_image.product_version,
474-
&transform_all_roles_to_config(
475-
opa,
476-
&[(
477-
opa_role.to_string(),
478-
(
479-
vec![
480-
PropertyNameKind::File(CONFIG_FILE.to_string()),
481-
PropertyNameKind::Env,
482-
PropertyNameKind::Cli,
483-
],
484-
opa.spec.servers.clone(),
485-
),
486-
)]
487-
.into(),
488-
)
489-
.context(ProductConfigTransformSnafu)?,
490-
&ctx.product_config,
491-
false,
492-
false,
493-
)
494-
.context(InvalidProductConfigSnafu)?;
495-
let role_server_config = validated_config
458+
let role_server_config = validated
459+
.validated_role_config
496460
.get(&opa_role.to_string())
497461
.map(Cow::Borrowed)
498462
.unwrap_or_default();
499463

500464
let server_role_service =
501-
build_server_role_service(opa, &resolved_product_image).context(BuildServiceSnafu)?;
465+
build_server_role_service(opa, &validated.image).context(BuildServiceSnafu)?;
502466
// required for discovery config map later
503467
let server_role_service = cluster_resources
504468
.add(client, server_role_service)
@@ -534,20 +498,15 @@ pub async fn reconcile_opa(
534498
.merged_config(&opa_role, &rolegroup)
535499
.context(FailedToResolveConfigSnafu)?;
536500

537-
let rg_configmap = build_server_rolegroup_config_map(
538-
opa,
539-
&resolved_product_image,
540-
&rolegroup,
541-
&merged_config,
542-
)?;
543-
let rg_service = build_rolegroup_headless_service(opa, &resolved_product_image, &rolegroup)
501+
let rg_configmap =
502+
build_server_rolegroup_config_map(opa, &validated.image, &rolegroup, &merged_config)?;
503+
let rg_service = build_rolegroup_headless_service(opa, &validated.image, &rolegroup)
504+
.context(BuildServiceSnafu)?;
505+
let rg_metrics_service = build_rolegroup_metrics_service(opa, &validated.image, &rolegroup)
544506
.context(BuildServiceSnafu)?;
545-
let rg_metrics_service =
546-
build_rolegroup_metrics_service(opa, &resolved_product_image, &rolegroup)
547-
.context(BuildServiceSnafu)?;
548507
let rg_daemonset = build_server_rolegroup_daemonset(
549508
opa,
550-
&resolved_product_image,
509+
&validated.image,
551510
&opa_role,
552511
&rolegroup,
553512
rolegroup_config,
@@ -610,7 +569,7 @@ pub async fn reconcile_opa(
610569
for discovery_cm in build_discovery_configmaps(
611570
opa,
612571
opa,
613-
&resolved_product_image,
572+
&validated.image,
614573
&server_role_service,
615574
&client.kubernetes_cluster_info,
616575
)
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
//! The validate step in the OpaCluster controller
2+
//!
3+
//! Synchronously validates inputs that don't require a Kubernetes client. Produces
4+
//! [`ValidatedInputs`], consumed by the rest of `reconcile_opa`.
5+
6+
use product_config::{ProductConfigManager, types::PropertyNameKind};
7+
use snafu::{ResultExt, Snafu};
8+
use stackable_operator::{
9+
cli::OperatorEnvironmentOptions,
10+
commons::product_image_selection::{self, ResolvedProductImage},
11+
product_config_utils::{
12+
ValidatedRoleConfigByPropertyKind, transform_all_roles_to_config,
13+
validate_all_roles_and_groups_config,
14+
},
15+
};
16+
17+
use crate::crd::{OpaRole, v1alpha2};
18+
19+
#[derive(Snafu, Debug)]
20+
pub enum Error {
21+
#[snafu(display("failed to resolve product image"))]
22+
ResolveProductImage {
23+
source: product_image_selection::Error,
24+
},
25+
26+
#[snafu(display("failed to transform configs"))]
27+
ProductConfigTransform {
28+
source: stackable_operator::product_config_utils::Error,
29+
},
30+
31+
#[snafu(display("invalid product config"))]
32+
InvalidProductConfig {
33+
source: stackable_operator::product_config_utils::Error,
34+
},
35+
}
36+
37+
type Result<T, E = Error> = std::result::Result<T, E>;
38+
39+
/// Synchronous inputs the rest of `reconcile_opa` needs after dereferencing.
40+
pub struct ValidatedInputs {
41+
pub image: ResolvedProductImage,
42+
pub validated_role_config: ValidatedRoleConfigByPropertyKind,
43+
}
44+
45+
/// Validates the cluster spec and the dereferenced inputs.
46+
pub fn validate(
47+
opa: &v1alpha2::OpaCluster,
48+
operator_environment: &OperatorEnvironmentOptions,
49+
product_config: &ProductConfigManager,
50+
) -> Result<ValidatedInputs> {
51+
let image = opa
52+
.spec
53+
.image
54+
.resolve(
55+
super::CONTAINER_IMAGE_BASE_NAME,
56+
&operator_environment.image_repository,
57+
crate::built_info::PKG_VERSION,
58+
)
59+
.context(ResolveProductImageSnafu)?;
60+
61+
let validated_role_config = validate_all_roles_and_groups_config(
62+
&image.product_version,
63+
&transform_all_roles_to_config(
64+
opa,
65+
&[(
66+
OpaRole::Server.to_string(),
67+
(
68+
vec![
69+
PropertyNameKind::File(super::CONFIG_FILE.to_string()),
70+
PropertyNameKind::Env,
71+
PropertyNameKind::Cli,
72+
],
73+
opa.spec.servers.clone(),
74+
),
75+
)]
76+
.into(),
77+
)
78+
.context(ProductConfigTransformSnafu)?,
79+
product_config,
80+
false,
81+
false,
82+
)
83+
.context(InvalidProductConfigSnafu)?;
84+
85+
Ok(ValidatedInputs {
86+
image,
87+
validated_role_config,
88+
})
89+
}

0 commit comments

Comments
 (0)