Skip to content

Commit 9ab5f01

Browse files
adwk67claude
andcommitted
refactor: extract webserver_config.py builder into config/webserver_config
Moves the webserver_config.py rendering (defaults + config overrides + the FILE_HEADER/FILE_FOOTER python blocks + the Flask writer call) out of build_rolegroup_config_map into a dedicated config::webserver_config::build(). The header/footer key constants and the related error variants move with it. Drops three debug! traces of intermediate config maps that no longer have a call site after the extraction; the rendered webserver_config.py is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 18f138f commit 9ab5f01

3 files changed

Lines changed: 91 additions & 75 deletions

File tree

rust/operator-binary/src/airflow_controller.rs

Lines changed: 14 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//! Ensures that `Pod`s are configured and running for each [`v1alpha2::AirflowCluster`]
22
use std::{
33
collections::{BTreeMap, BTreeSet, HashMap},
4-
io::Write,
54
sync::Arc,
65
};
76

@@ -72,17 +71,14 @@ use stackable_operator::{
7271
use strum::{EnumDiscriminants, IntoStaticStr};
7372

7473
use crate::{
75-
config::{
76-
self, PYTHON_IMPORTS,
77-
writer::{self, FlaskAppConfigWriterError},
78-
},
74+
config,
7975
controller_commons::{self, CONFIG_VOLUME_NAME, LOG_CONFIG_VOLUME_NAME, LOG_VOLUME_NAME},
8076
crd::{
8177
self, AIRFLOW_CONFIG_FILENAME, APP_NAME, AirflowClusterStatus, AirflowConfig,
82-
AirflowConfigOptions, AirflowExecutor, AirflowExecutorCommonConfiguration, AirflowRole,
83-
CONFIG_PATH, Container, ExecutorConfig, HTTP_PORT, HTTP_PORT_NAME, LISTENER_VOLUME_DIR,
84-
LISTENER_VOLUME_NAME, LOG_CONFIG_DIR, METRICS_PORT, METRICS_PORT_NAME, OPERATOR_NAME,
85-
STACKABLE_LOG_DIR, TEMPLATE_LOCATION, TEMPLATE_NAME, TEMPLATE_VOLUME_NAME,
78+
AirflowExecutor, AirflowExecutorCommonConfiguration, AirflowRole, CONFIG_PATH, Container,
79+
ExecutorConfig, HTTP_PORT, HTTP_PORT_NAME, LISTENER_VOLUME_DIR, LISTENER_VOLUME_NAME,
80+
LOG_CONFIG_DIR, METRICS_PORT, METRICS_PORT_NAME, OPERATOR_NAME, STACKABLE_LOG_DIR,
81+
TEMPLATE_LOCATION, TEMPLATE_NAME, TEMPLATE_VOLUME_NAME,
8682
authentication::{
8783
AirflowAuthenticationClassResolved, AirflowClientAuthenticationDetailsResolved,
8884
},
@@ -112,9 +108,6 @@ pub const CONTAINER_IMAGE_BASE_NAME: &str = "airflow";
112108
pub const AIRFLOW_FULL_CONTROLLER_NAME: &str =
113109
concatcp!(AIRFLOW_CONTROLLER_NAME, '.', OPERATOR_NAME);
114110

115-
const CONFIG_OVERRIDE_FILE_HEADER_KEY: &str = "FILE_HEADER";
116-
const CONFIG_OVERRIDE_FILE_FOOTER_KEY: &str = "FILE_FOOTER";
117-
118111
pub struct Ctx {
119112
pub client: stackable_operator::client::Client,
120113
pub operator_environment: OperatorEnvironmentOptions,
@@ -164,9 +157,9 @@ pub enum Error {
164157
source: stackable_operator::commons::rbac::Error,
165158
},
166159

167-
#[snafu(display("failed to build config file for {rolegroup}"))]
168-
BuildRoleGroupConfigFile {
169-
source: FlaskAppConfigWriterError,
160+
#[snafu(display("failed to build webserver config for {rolegroup}"))]
161+
BuildWebserverConfig {
162+
source: config::webserver_config::Error,
170163
rolegroup: RoleGroupRef<v1alpha2::AirflowCluster>,
171164
},
172165

@@ -257,14 +250,6 @@ pub enum Error {
257250
source: stackable_operator::builder::meta::Error,
258251
},
259252

260-
#[snafu(display("failed to construct config"))]
261-
ConstructConfig { source: config::Error },
262-
263-
#[snafu(display(
264-
"failed to write to String (Vec<u8> to be precise) containing Airflow config"
265-
))]
266-
WriteToConfigFileString { source: std::io::Error },
267-
268253
#[snafu(display("failed to configure logging"))]
269254
ConfigureLogging { source: LoggingError },
270255

@@ -728,58 +713,15 @@ fn build_rolegroup_config_map(
728713
logging: &Logging<Container>,
729714
container: &Container,
730715
) -> Result<ConfigMap, Error> {
731-
let mut config: BTreeMap<String, String> = BTreeMap::new();
732-
733-
// this will call default values from AirflowClientAuthenticationDetails
734-
config::add_airflow_config(
735-
&mut config,
716+
let config_file = config::webserver_config::build(
736717
authentication_config,
737718
authorization_config,
738719
&resolved_product_image.product_version,
720+
config_file_overrides,
739721
)
740-
.context(ConstructConfigSnafu)?;
741-
742-
tracing::debug!(
743-
"Default config for {}: {:?}",
744-
rolegroup.object_name(),
745-
config
746-
);
747-
748-
let mut file_config = config_file_overrides.clone();
749-
750-
tracing::debug!(
751-
"Config overrides for {}: {:?}",
752-
rolegroup.object_name(),
753-
file_config
754-
);
755-
756-
// now add any overrides, replacing any defaults
757-
config.append(&mut file_config);
758-
759-
tracing::debug!(
760-
"Merged config for {}: {:?}",
761-
rolegroup.object_name(),
762-
config
763-
);
764-
765-
let mut config_file = Vec::new();
766-
767-
// By removing the keys from `config_properties`, we avoid pasting the Python code into a Python variable as well
768-
// (which would be bad)
769-
if let Some(header) = config.remove(CONFIG_OVERRIDE_FILE_HEADER_KEY) {
770-
writeln!(config_file, "{}", header).context(WriteToConfigFileStringSnafu)?;
771-
}
772-
773-
let temp_file_footer: Option<String> = config.remove(CONFIG_OVERRIDE_FILE_FOOTER_KEY);
774-
775-
writer::write::<AirflowConfigOptions, _, _>(&mut config_file, config.iter(), PYTHON_IMPORTS)
776-
.with_context(|_| BuildRoleGroupConfigFileSnafu {
777-
rolegroup: rolegroup.clone(),
778-
})?;
779-
780-
if let Some(footer) = temp_file_footer {
781-
writeln!(config_file, "{}", footer).context(WriteToConfigFileStringSnafu)?;
782-
}
722+
.with_context(|_| BuildWebserverConfigSnafu {
723+
rolegroup: rolegroup.clone(),
724+
})?;
783725

784726
let mut cm_builder = ConfigMapBuilder::new();
785727

@@ -800,10 +742,7 @@ fn build_rolegroup_config_map(
800742
.context(ObjectMetaSnafu)?
801743
.build(),
802744
)
803-
.add_data(
804-
AIRFLOW_CONFIG_FILENAME,
805-
String::from_utf8(config_file).unwrap(),
806-
);
745+
.add_data(AIRFLOW_CONFIG_FILENAME, config_file);
807746

808747
extend_config_map_with_log_config(
809748
rolegroup,

rust/operator-binary/src/config/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
pub mod webserver_config;
12
pub mod writer;
23

34
use std::collections::BTreeMap;
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
//! Builds the `webserver_config.py` Flask configuration file from the resolved
2+
//! authentication/authorization config plus user-provided config overrides.
3+
4+
use std::{collections::BTreeMap, io::Write};
5+
6+
use snafu::{ResultExt, Snafu};
7+
8+
use super::{PYTHON_IMPORTS, add_airflow_config, writer};
9+
use crate::crd::{
10+
AirflowConfigOptions, authentication::AirflowClientAuthenticationDetailsResolved,
11+
authorization::AirflowAuthorizationResolved,
12+
};
13+
14+
/// Marks arbitrary Python code to prepend verbatim to the generated file.
15+
const CONFIG_OVERRIDE_FILE_HEADER_KEY: &str = "FILE_HEADER";
16+
/// Marks arbitrary Python code to append verbatim to the generated file.
17+
const CONFIG_OVERRIDE_FILE_FOOTER_KEY: &str = "FILE_FOOTER";
18+
19+
#[derive(Snafu, Debug)]
20+
pub enum Error {
21+
#[snafu(display("failed to construct the webserver config"))]
22+
ConstructConfig { source: super::Error },
23+
24+
#[snafu(display("failed to write the webserver config file"))]
25+
WriteConfigFile {
26+
source: writer::FlaskAppConfigWriterError,
27+
},
28+
29+
#[snafu(display("failed to write the header/footer to the webserver config file"))]
30+
WriteHeaderFooter { source: std::io::Error },
31+
}
32+
33+
/// Renders the `webserver_config.py` contents: operator defaults (derived from the
34+
/// resolved authentication/authorization config) with the user's `config_overrides`
35+
/// applied last, wrapped by the optional `FILE_HEADER`/`FILE_FOOTER` Python blocks.
36+
pub fn build(
37+
authentication_config: &AirflowClientAuthenticationDetailsResolved,
38+
authorization_config: &AirflowAuthorizationResolved,
39+
product_version: &str,
40+
config_file_overrides: &BTreeMap<String, String>,
41+
) -> Result<String, Error> {
42+
let mut config: BTreeMap<String, String> = BTreeMap::new();
43+
44+
// this will call default values from AirflowClientAuthenticationDetails
45+
add_airflow_config(
46+
&mut config,
47+
authentication_config,
48+
authorization_config,
49+
product_version,
50+
)
51+
.context(ConstructConfigSnafu)?;
52+
53+
let mut file_config = config_file_overrides.clone();
54+
55+
// now add any overrides, replacing any defaults
56+
config.append(&mut file_config);
57+
58+
let mut config_file = Vec::new();
59+
60+
// By removing the keys from `config`, we avoid pasting the Python code into a Python variable as well
61+
// (which would be bad)
62+
if let Some(header) = config.remove(CONFIG_OVERRIDE_FILE_HEADER_KEY) {
63+
writeln!(config_file, "{}", header).context(WriteHeaderFooterSnafu)?;
64+
}
65+
66+
let temp_file_footer: Option<String> = config.remove(CONFIG_OVERRIDE_FILE_FOOTER_KEY);
67+
68+
writer::write::<AirflowConfigOptions, _, _>(&mut config_file, config.iter(), PYTHON_IMPORTS)
69+
.context(WriteConfigFileSnafu)?;
70+
71+
if let Some(footer) = temp_file_footer {
72+
writeln!(config_file, "{}", footer).context(WriteHeaderFooterSnafu)?;
73+
}
74+
75+
Ok(String::from_utf8(config_file).expect("the Flask config writer only emits valid UTF-8"))
76+
}

0 commit comments

Comments
 (0)