Skip to content

Commit 5ff22fb

Browse files
adwk67claude
andcommitted
refactor: remove product-config validation from reconcile
Replaces transform_all_roles_to_config / validate_all_roles_and_groups_config with direct role iteration plus merged_overrides(). Folds the dereferenced authentication/authorization objects into ValidatedAirflowCluster so downstream build steps read them from the validated cluster. The product-config crate is still used for the Flask config writer (removed in a later step). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 95be852 commit 5ff22fb

6 files changed

Lines changed: 73 additions & 153 deletions

File tree

rust/operator-binary/src/airflow_controller.rs

Lines changed: 20 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,7 @@ use std::{
66
};
77

88
use const_format::concatcp;
9-
use product_config::{
10-
ProductConfigManager,
11-
flask_app_config_writer::{self, FlaskAppConfigWriterError},
12-
types::PropertyNameKind,
13-
};
9+
use product_config::flask_app_config_writer::{self, FlaskAppConfigWriterError};
1410
use snafu::{OptionExt, ResultExt, Snafu};
1511
use stackable_operator::{
1612
builder::{
@@ -61,10 +57,7 @@ use stackable_operator::{
6157
},
6258
kvp::{Annotation, Label, LabelError, Labels, ObjectLabels},
6359
logging::controller::ReconcilerError,
64-
product_config_utils::{
65-
CONFIG_OVERRIDE_FILE_FOOTER_KEY, CONFIG_OVERRIDE_FILE_HEADER_KEY, env_vars_from,
66-
env_vars_from_rolegroup_config,
67-
},
60+
product_config_utils::env_vars_from,
6861
product_logging::{
6962
self,
7063
framework::LoggingError,
@@ -118,9 +111,11 @@ pub const CONTAINER_IMAGE_BASE_NAME: &str = "airflow";
118111
pub const AIRFLOW_FULL_CONTROLLER_NAME: &str =
119112
concatcp!(AIRFLOW_CONTROLLER_NAME, '.', OPERATOR_NAME);
120113

114+
const CONFIG_OVERRIDE_FILE_HEADER_KEY: &str = "FILE_HEADER";
115+
const CONFIG_OVERRIDE_FILE_FOOTER_KEY: &str = "FILE_FOOTER";
116+
121117
pub struct Ctx {
122118
pub client: stackable_operator::client::Client,
123-
pub product_config: ProductConfigManager,
124119
pub operator_environment: OperatorEnvironmentOptions,
125120
}
126121

@@ -386,7 +381,7 @@ pub async fn reconcile_airflow(
386381
CONTAINER_IMAGE_BASE_NAME,
387382
&ctx.operator_environment.image_repository,
388383
crate::built_info::PKG_VERSION,
389-
&ctx.product_config,
384+
dereferenced,
390385
)
391386
.context(ValidateSnafu)?;
392387

@@ -464,8 +459,8 @@ pub async fn reconcile_airflow(
464459
common_configuration,
465460
&metadata_database_connection_details,
466461
&validated.image,
467-
&dereferenced.authentication_config,
468-
&dereferenced.authorization_config,
462+
&validated.authentication_config,
463+
&validated.authorization_config,
469464
&mut cluster_resources,
470465
client,
471466
&rbac_sa,
@@ -515,7 +510,7 @@ pub async fn reconcile_airflow(
515510
let git_sync_resources = git_sync::v1alpha2::GitSyncResources::new(
516511
&airflow.spec.cluster_config.dags_git_sync,
517512
&validated.image,
518-
&env_vars_from_rolegroup_config(&validated_rg_config.product_config_properties),
513+
&env_vars_from(&validated_rg_config.overrides.env_overrides),
519514
&airflow.volume_mounts(),
520515
LOG_VOLUME_NAME,
521516
&validated_rg_config
@@ -574,9 +569,9 @@ pub async fn reconcile_airflow(
574569
airflow,
575570
&validated.image,
576571
&rolegroup,
577-
&validated_rg_config.product_config_properties,
578-
&dereferenced.authentication_config,
579-
&dereferenced.authorization_config,
572+
&validated_rg_config.overrides.config_file_overrides,
573+
&validated.authentication_config,
574+
&validated.authorization_config,
580575
&validated_rg_config.merged_config.logging,
581576
&Container::Airflow,
582577
)?;
@@ -592,9 +587,9 @@ pub async fn reconcile_airflow(
592587
&validated.image,
593588
airflow_role,
594589
&rolegroup,
595-
&validated_rg_config.product_config_properties,
596-
&dereferenced.authentication_config,
597-
&dereferenced.authorization_config,
590+
&validated_rg_config.overrides.env_overrides,
591+
&validated.authentication_config,
592+
&validated.authorization_config,
598593
&metadata_database_connection_details,
599594
&celery_database_connection_details,
600595
&rbac_sa,
@@ -659,7 +654,7 @@ async fn build_executor_template(
659654
airflow,
660655
resolved_product_image,
661656
&rolegroup,
662-
&HashMap::new(),
657+
&BTreeMap::new(),
663658
authentication_config,
664659
authorization_config,
665660
&merged_executor_config.logging,
@@ -709,7 +704,7 @@ fn build_rolegroup_config_map(
709704
airflow: &v1alpha2::AirflowCluster,
710705
resolved_product_image: &ResolvedProductImage,
711706
rolegroup: &RoleGroupRef<v1alpha2::AirflowCluster>,
712-
rolegroup_config: &HashMap<PropertyNameKind, BTreeMap<String, String>>,
707+
config_file_overrides: &BTreeMap<String, String>,
713708
authentication_config: &AirflowClientAuthenticationDetailsResolved,
714709
authorization_config: &AirflowAuthorizationResolved,
715710
logging: &Logging<Container>,
@@ -732,10 +727,7 @@ fn build_rolegroup_config_map(
732727
config
733728
);
734729

735-
let mut file_config = rolegroup_config
736-
.get(&PropertyNameKind::File(AIRFLOW_CONFIG_FILENAME.to_string()))
737-
.cloned()
738-
.unwrap_or_default();
730+
let mut file_config = config_file_overrides.clone();
739731

740732
tracing::debug!(
741733
"Config overrides for {}: {:?}",
@@ -884,7 +876,7 @@ fn build_server_rolegroup_statefulset(
884876
resolved_product_image: &ResolvedProductImage,
885877
airflow_role: &AirflowRole,
886878
rolegroup_ref: &RoleGroupRef<v1alpha2::AirflowCluster>,
887-
rolegroup_config: &HashMap<PropertyNameKind, BTreeMap<String, String>>,
879+
env_overrides: &HashMap<String, String>,
888880
authentication_config: &AirflowClientAuthenticationDetailsResolved,
889881
authorization_config: &AirflowAuthorizationResolved,
890882
metadata_database_connection_details: &SqlAlchemyDatabaseConnectionDetails,
@@ -974,7 +966,7 @@ fn build_server_rolegroup_statefulset(
974966
env_vars::build_airflow_statefulset_envs(
975967
airflow,
976968
airflow_role,
977-
rolegroup_config,
969+
env_overrides,
978970
executor,
979971
authentication_config,
980972
authorization_config,
Lines changed: 40 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
1-
use std::{
2-
collections::{BTreeMap, HashMap},
3-
str::FromStr,
4-
};
1+
use std::collections::BTreeMap;
52

6-
use product_config::{ProductConfigManager, types::PropertyNameKind};
73
use snafu::{ResultExt, Snafu};
84
use stackable_operator::{
95
commons::product_image_selection::{self, ResolvedProductImage},
10-
product_config_utils::{transform_all_roles_to_config, validate_all_roles_and_groups_config},
116
role_utils::RoleGroupRef,
127
};
138
use strum::IntoEnumIterator;
149

15-
use crate::crd::{AIRFLOW_CONFIG_FILENAME, AirflowConfig, AirflowExecutor, AirflowRole, v1alpha2};
10+
use super::dereference::DereferencedObjects;
11+
use crate::crd::{
12+
AirflowConfig, AirflowExecutor, AirflowRole, MergedOverrides,
13+
authentication::AirflowClientAuthenticationDetailsResolved,
14+
authorization::AirflowAuthorizationResolved, v1alpha2,
15+
};
1616

1717
#[derive(Snafu, Debug)]
1818
pub enum Error {
@@ -21,24 +21,8 @@ pub enum Error {
2121
source: product_image_selection::Error,
2222
},
2323

24-
#[snafu(display("invalid product config"))]
25-
InvalidProductConfig {
26-
source: stackable_operator::product_config_utils::Error,
27-
},
28-
29-
#[snafu(display("failed to generate product config"))]
30-
GenerateProductConfig {
31-
source: stackable_operator::product_config_utils::Error,
32-
},
33-
3424
#[snafu(display("failed to resolve and merge config for role and role group"))]
3525
FailedToResolveConfig { source: crate::crd::Error },
36-
37-
#[snafu(display("could not parse Airflow role [{role}]"))]
38-
UnidentifiedAirflowRole {
39-
source: strum::ParseError,
40-
role: String,
41-
},
4226
}
4327

4428
/// Per-role configuration extracted during validation.
@@ -49,115 +33,99 @@ pub struct ValidatedRoleConfig {
4933
pub group_listener_name: Option<String>,
5034
}
5135

52-
/// Per-rolegroup configuration: the merged CRD config plus the product-config properties.
36+
/// Per-rolegroup configuration: the merged CRD config plus overrides.
5337
#[derive(Clone, Debug)]
5438
pub struct ValidatedRoleGroupConfig {
5539
pub merged_config: AirflowConfig,
56-
pub product_config_properties: HashMap<PropertyNameKind, BTreeMap<String, String>>,
40+
pub overrides: MergedOverrides,
5741
}
5842

59-
/// The validated cluster: proves that product-config validation and config merging
60-
/// succeeded for every role and role group before any resources are created.
43+
/// The validated cluster: proves that config merging succeeded for every role and
44+
/// role group before any resources are created. It also carries the dereferenced
45+
/// external references, so every downstream build step reads them from here.
6146
#[derive(Clone, Debug)]
6247
pub struct ValidatedAirflowCluster {
6348
pub image: ResolvedProductImage,
6449
pub role_groups: BTreeMap<AirflowRole, BTreeMap<String, ValidatedRoleGroupConfig>>,
6550
pub role_configs: BTreeMap<AirflowRole, ValidatedRoleConfig>,
6651
pub executor: AirflowExecutor,
52+
pub authentication_config: AirflowClientAuthenticationDetailsResolved,
53+
pub authorization_config: AirflowAuthorizationResolved,
6754
}
6855

6956
pub fn validate_cluster(
7057
airflow: &v1alpha2::AirflowCluster,
7158
image_base_name: &str,
7259
image_repository: &str,
7360
pkg_version: &str,
74-
product_config_manager: &ProductConfigManager,
61+
dereferenced: DereferencedObjects,
7562
) -> Result<ValidatedAirflowCluster, Error> {
7663
let resolved_product_image = airflow
7764
.spec
7865
.image
7966
.resolve(image_base_name, image_repository, pkg_version)
8067
.context(ResolveProductImageSnafu)?;
8168

82-
let mut roles = HashMap::new();
69+
let mut role_groups = BTreeMap::new();
70+
let mut role_configs = BTreeMap::new();
8371

8472
// if the kubernetes executor is specified there will be no worker role as the pods
8573
// are provisioned by airflow as defined by the task (default: one pod per task)
8674
for role in AirflowRole::iter() {
87-
if let Some(resolved_role) = airflow.get_role(&role) {
88-
roles.insert(
89-
role.to_string(),
90-
(
91-
vec![
92-
PropertyNameKind::Env,
93-
PropertyNameKind::File(AIRFLOW_CONFIG_FILENAME.into()),
94-
],
95-
resolved_role.clone(),
96-
),
97-
);
98-
}
99-
}
100-
101-
let role_config = transform_all_roles_to_config(airflow, &roles);
102-
let validated_role_config = validate_all_roles_and_groups_config(
103-
&resolved_product_image.product_version,
104-
&role_config.context(GenerateProductConfigSnafu)?,
105-
product_config_manager,
106-
false,
107-
false,
108-
)
109-
.context(InvalidProductConfigSnafu)?;
110-
111-
let mut role_groups = BTreeMap::new();
112-
let mut role_configs = BTreeMap::new();
113-
114-
for (role_name, rolegroup_configs) in validated_role_config.iter() {
115-
let airflow_role =
116-
AirflowRole::from_str(role_name).context(UnidentifiedAirflowRoleSnafu {
117-
role: role_name.to_string(),
118-
})?;
75+
let Some(resolved_role) = airflow.get_role(&role) else {
76+
continue;
77+
};
11978

12079
role_configs.insert(
121-
airflow_role.clone(),
80+
role.clone(),
12281
ValidatedRoleConfig {
12382
pdb: airflow
124-
.role_config(&airflow_role)
83+
.role_config(&role)
12584
.map(|rc| rc.pod_disruption_budget),
126-
listener_class: airflow_role
127-
.listener_class_name(airflow)
128-
.map(|s| s.to_string()),
129-
group_listener_name: airflow.group_listener_name(&airflow_role),
85+
listener_class: role.listener_class_name(airflow).map(|s| s.to_string()),
86+
group_listener_name: airflow.group_listener_name(&role),
13087
},
13188
);
13289

13390
let mut group_configs = BTreeMap::new();
134-
for (rolegroup_name, rolegroup_config) in rolegroup_configs.iter() {
91+
for rolegroup_name in resolved_role.role_groups.keys() {
13592
let rolegroup_ref = RoleGroupRef {
13693
cluster: stackable_operator::kube::runtime::reflector::ObjectRef::from_obj(airflow),
137-
role: role_name.into(),
94+
role: role.to_string(),
13895
role_group: rolegroup_name.into(),
13996
};
14097

14198
let merged_config = airflow
142-
.merged_config(&airflow_role, &rolegroup_ref)
99+
.merged_config(&role, &rolegroup_ref)
100+
.context(FailedToResolveConfigSnafu)?;
101+
102+
let overrides = airflow
103+
.merged_overrides(&role, rolegroup_name)
143104
.context(FailedToResolveConfigSnafu)?;
144105

145106
group_configs.insert(
146107
rolegroup_name.clone(),
147108
ValidatedRoleGroupConfig {
148109
merged_config,
149-
product_config_properties: rolegroup_config.clone(),
110+
overrides,
150111
},
151112
);
152113
}
153114

154-
role_groups.insert(airflow_role, group_configs);
115+
role_groups.insert(role, group_configs);
155116
}
156117

118+
let DereferencedObjects {
119+
authentication_config,
120+
authorization_config,
121+
} = dereferenced;
122+
157123
Ok(ValidatedAirflowCluster {
158124
image: resolved_product_image,
159125
role_groups,
160126
role_configs,
161127
executor: airflow.spec.executor.clone(),
128+
authentication_config,
129+
authorization_config,
162130
})
163131
}

rust/operator-binary/src/crd/authorization.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use stackable_operator::{client::Client, commons::opa::OpaApiVersion, shared::ti
22

33
use crate::crd::{AirflowAuthorization, AirflowOpaConfig, v1alpha2};
44

5+
#[derive(Clone, Debug)]
56
pub struct AirflowAuthorizationResolved {
67
pub opa: Option<OpaConfigResolved>,
78
}
@@ -24,6 +25,7 @@ impl AirflowAuthorizationResolved {
2425
}
2526
}
2627

28+
#[derive(Clone, Debug)]
2729
pub struct OpaConfigResolved {
2830
pub connection_string: String,
2931
pub cache_entry_time_to_live: Duration,

0 commit comments

Comments
 (0)