Skip to content

Commit a68b232

Browse files
adwk67claude
andcommitted
refactor: move product image resolution from dereference to validate
Image resolution is a pure computation, not an I/O dereference, so it belongs in validate_cluster alongside the other config validation. This also aligns with the pattern used by the trino operator. The dereference error variants were renamed to drop the `Invalid` prefix (e.g. InvalidAuthenticationConfig → AuthenticationConfig) because removing ResolveProductImage left all remaining variants sharing the same prefix, triggering clippy::enum_variant_names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent a958ce2 commit a68b232

3 files changed

Lines changed: 37 additions & 45 deletions

File tree

rust/operator-binary/src/airflow_controller.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -325,15 +325,9 @@ pub async fn reconcile_airflow(
325325

326326
let client = &ctx.client;
327327

328-
let dereferenced = crate::controller::dereference::dereference(
329-
client,
330-
airflow,
331-
CONTAINER_IMAGE_BASE_NAME,
332-
&ctx.operator_environment.image_repository,
333-
crate::built_info::PKG_VERSION,
334-
)
335-
.await
336-
.context(DereferenceSnafu)?;
328+
let dereferenced = crate::controller::dereference::dereference(client, airflow)
329+
.await
330+
.context(DereferenceSnafu)?;
337331

338332
let cluster_operation_cond_builder =
339333
ClusterOperationsConditionBuilder::new(&airflow.spec.cluster_operation);
@@ -375,9 +369,14 @@ pub async fn reconcile_airflow(
375369
None
376370
};
377371

378-
let validated =
379-
crate::controller::validate::validate_cluster(airflow, &dereferenced, &ctx.product_config)
380-
.context(ValidateSnafu)?;
372+
let validated = crate::controller::validate::validate_cluster(
373+
airflow,
374+
CONTAINER_IMAGE_BASE_NAME,
375+
&ctx.operator_environment.image_repository,
376+
crate::built_info::PKG_VERSION,
377+
&ctx.product_config,
378+
)
379+
.context(ValidateSnafu)?;
381380

382381
let mut cluster_resources = ClusterResources::new(
383382
APP_NAME,

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

Lines changed: 9 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
use snafu::{ResultExt, Snafu};
2-
use stackable_operator::commons::{
3-
product_image_selection::{self, ResolvedProductImage},
4-
random_secret_creation,
5-
};
2+
use stackable_operator::commons::random_secret_creation;
63

74
use crate::crd::{
85
authentication::AirflowClientAuthenticationDetailsResolved,
@@ -13,61 +10,46 @@ use crate::crd::{
1310

1411
#[derive(Snafu, Debug)]
1512
pub enum Error {
16-
#[snafu(display("failed to resolve product image"))]
17-
ResolveProductImage {
18-
source: product_image_selection::Error,
19-
},
20-
2113
#[snafu(display("failed to apply authentication configuration"))]
22-
InvalidAuthenticationConfig {
14+
AuthenticationConfig {
2315
source: crate::crd::authentication::Error,
2416
},
2517

2618
#[snafu(display("invalid authorization config"))]
27-
InvalidAuthorizationConfig {
19+
AuthorizationConfig {
2820
source: stackable_operator::commons::opa::Error,
2921
},
3022

3123
#[snafu(display("failed to create internal secret"))]
32-
InvalidInternalSecret {
24+
InternalSecret {
3325
source: random_secret_creation::Error,
3426
},
3527
}
3628

3729
/// External references resolved during the dereference step.
3830
pub struct DereferencedObjects {
39-
pub resolved_product_image: ResolvedProductImage,
4031
pub authentication_config: AirflowClientAuthenticationDetailsResolved,
4132
pub authorization_config: AirflowAuthorizationResolved,
4233
}
4334

4435
pub async fn dereference(
4536
client: &stackable_operator::client::Client,
4637
airflow: &v1alpha2::AirflowCluster,
47-
image_base_name: &str,
48-
image_repository: &str,
49-
pkg_version: &str,
5038
) -> Result<DereferencedObjects, Error> {
51-
let resolved_product_image = airflow
52-
.spec
53-
.image
54-
.resolve(image_base_name, image_repository, pkg_version)
55-
.context(ResolveProductImageSnafu)?;
56-
5739
let authentication_config = AirflowClientAuthenticationDetailsResolved::from(
5840
&airflow.spec.cluster_config.authentication,
5941
client,
6042
)
6143
.await
62-
.context(InvalidAuthenticationConfigSnafu)?;
44+
.context(AuthenticationConfigSnafu)?;
6345

6446
let authorization_config = AirflowAuthorizationResolved::from_authorization_config(
6547
client,
6648
airflow,
6749
&airflow.spec.cluster_config.authorization,
6850
)
6951
.await
70-
.context(InvalidAuthorizationConfigSnafu)?;
52+
.context(AuthorizationConfigSnafu)?;
7153

7254
random_secret_creation::create_random_secret_if_not_exists(
7355
&airflow.shared_internal_secret_secret_name(),
@@ -77,7 +59,7 @@ pub async fn dereference(
7759
client,
7860
)
7961
.await
80-
.context(InvalidInternalSecretSnafu)?;
62+
.context(InternalSecretSnafu)?;
8163

8264
random_secret_creation::create_random_secret_if_not_exists(
8365
&airflow.shared_jwt_secret_secret_name(),
@@ -87,7 +69,7 @@ pub async fn dereference(
8769
client,
8870
)
8971
.await
90-
.context(InvalidInternalSecretSnafu)?;
72+
.context(InternalSecretSnafu)?;
9173

9274
// https://airflow.apache.org/docs/apache-airflow/stable/security/secrets/fernet.html#security-fernet
9375
// does not document how long the fernet key should be, but recommends using
@@ -101,10 +83,9 @@ pub async fn dereference(
10183
client,
10284
)
10385
.await
104-
.context(InvalidInternalSecretSnafu)?;
86+
.context(InternalSecretSnafu)?;
10587

10688
Ok(DereferencedObjects {
107-
resolved_product_image,
10889
authentication_config,
10990
authorization_config,
11091
})

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

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,21 @@ use std::{
66
use product_config::{ProductConfigManager, types::PropertyNameKind};
77
use snafu::{ResultExt, Snafu};
88
use stackable_operator::{
9-
commons::product_image_selection::ResolvedProductImage,
9+
commons::product_image_selection::{self, ResolvedProductImage},
1010
product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config},
1111
role_utils::RoleGroupRef,
1212
};
1313
use strum::IntoEnumIterator;
1414

15-
use super::dereference::DereferencedObjects;
1615
use crate::crd::{AIRFLOW_CONFIG_FILENAME, AirflowConfig, AirflowExecutor, AirflowRole, v1alpha2};
1716

1817
#[derive(Snafu, Debug)]
1918
pub enum Error {
19+
#[snafu(display("failed to resolve product image"))]
20+
ResolveProductImage {
21+
source: product_image_selection::Error,
22+
},
23+
2024
#[snafu(display("invalid product config"))]
2125
InvalidProductConfig {
2226
source: stackable_operator::product_config_utils::Error,
@@ -64,9 +68,17 @@ pub struct ValidatedAirflowCluster {
6468

6569
pub fn validate_cluster(
6670
airflow: &v1alpha2::AirflowCluster,
67-
dereferenced: &DereferencedObjects,
71+
image_base_name: &str,
72+
image_repository: &str,
73+
pkg_version: &str,
6874
product_config_manager: &ProductConfigManager,
6975
) -> Result<ValidatedAirflowCluster, Error> {
76+
let resolved_product_image = airflow
77+
.spec
78+
.image
79+
.resolve(image_base_name, image_repository, pkg_version)
80+
.context(ResolveProductImageSnafu)?;
81+
7082
let mut roles = HashMap::new();
7183

7284
// if the kubernetes executor is specified there will be no worker role as the pods
@@ -88,7 +100,7 @@ pub fn validate_cluster(
88100

89101
let role_config = transform_all_roles_to_config(airflow, &roles);
90102
let validated_role_config = validate_all_roles_and_groups_config(
91-
&dereferenced.resolved_product_image.product_version,
103+
&resolved_product_image.product_version,
92104
&role_config.context(GenerateProductConfigSnafu)?,
93105
product_config_manager,
94106
false,
@@ -143,7 +155,7 @@ pub fn validate_cluster(
143155
}
144156

145157
Ok(ValidatedAirflowCluster {
146-
image: dereferenced.resolved_product_image.clone(),
158+
image: resolved_product_image,
147159
role_groups,
148160
role_configs,
149161
executor: airflow.spec.executor.clone(),

0 commit comments

Comments
 (0)