Skip to content

Commit 36b65a6

Browse files
fix: Adding start up commands for airflow (#530)
* Adding start up commands for airflow * Adding variable to env * Making lints happy * fixing tests * fixing unit tests * Looking for the problem * Revert "Adding start up commands for airflow" This reverts commit 17a6049. * Using python certify to use ca certs * Adding comment to AirflowRole * Apply suggestions from code review Applying Review Co-authored-by: Siegfried Weber <mail@siegfriedweber.net> * Further adapting review * remove reject_different_tls_ca_certs() * Update Cargo.nix * Adding code review --------- Co-authored-by: Siegfried Weber <mail@siegfriedweber.net>
1 parent b694188 commit 36b65a6

File tree

6 files changed

+61
-24
lines changed

6 files changed

+61
-24
lines changed

CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
Use the env var `KUBERNETES_CLUSTER_DOMAIN` or the operator Helm chart property `kubernetesClusterDomain` to set a non-default cluster domain ([#518]).
1010
- Support for `2.9.3` ([#494]).
1111
- Experimental Support for `2.10.2` ([#512]).
12-
- Add support for OpenID Connect ([#524])
12+
- Add support for OpenID Connect ([#524], [#530])
1313

1414
### Changed
1515

@@ -32,6 +32,7 @@
3232
[#518]: https://github.com/stackabletech/airflow-operator/pull/518
3333
[#520]: https://github.com/stackabletech/airflow-operator/pull/520
3434
[#524]: https://github.com/stackabletech/airflow-operator/pull/524
35+
[#530]: https://github.com/stackabletech/airflow-operator/pull/530
3536

3637
## [24.7.0] - 2024-07-24
3738

Cargo.lock

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

Cargo.nix

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

rust/crd/src/lib.rs

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1-
use std::collections::BTreeMap;
1+
use std::collections::{BTreeMap, BTreeSet};
22

3+
use authentication::{
4+
AirflowAuthenticationClassResolved, AirflowClientAuthenticationDetailsResolved,
5+
};
36
use git_sync::GitSync;
47
use product_config::flask_app_config_writer::{FlaskAppConfigOptions, PythonType};
58
use serde::{Deserialize, Serialize};
@@ -322,7 +325,12 @@ impl AirflowRole {
322325
/// components to have the same image/configuration (e.g. DAG folder location), even if not all
323326
/// configuration settings are used everywhere. For this reason we ensure that the webserver
324327
/// config file is in the Airflow home directory on all pods.
325-
pub fn get_commands(&self) -> Vec<String> {
328+
/// Only the webserver needs to know about authentication CA's which is added via python's certify
329+
/// if authentication is enabled.
330+
pub fn get_commands(
331+
&self,
332+
auth_config: &AirflowClientAuthenticationDetailsResolved,
333+
) -> Vec<String> {
326334
let mut command = vec![
327335
format!("cp -RL {CONFIG_PATH}/{AIRFLOW_CONFIG_FILENAME} {AIRFLOW_HOME}/{AIRFLOW_CONFIG_FILENAME}"),
328336
// graceful shutdown part
@@ -331,10 +339,15 @@ impl AirflowRole {
331339
];
332340

333341
match &self {
334-
AirflowRole::Webserver => command.extend(vec![
335-
"prepare_signal_handlers".to_string(),
336-
"airflow webserver &".to_string(),
337-
]),
342+
AirflowRole::Webserver => {
343+
// Getting auth commands for AuthClass
344+
command.extend(Self::authentication_start_commands(auth_config));
345+
command.extend(vec![
346+
"prepare_signal_handlers".to_string(),
347+
"airflow webserver &".to_string(),
348+
]);
349+
}
350+
338351
AirflowRole::Scheduler => command.extend(vec![
339352
// Database initialization is limited to the scheduler, see https://github.com/stackabletech/airflow-operator/issues/259
340353
"airflow db init".to_string(),
@@ -364,6 +377,38 @@ impl AirflowRole {
364377
command
365378
}
366379

380+
fn authentication_start_commands(
381+
auth_config: &AirflowClientAuthenticationDetailsResolved,
382+
) -> Vec<String> {
383+
let mut commands = Vec::new();
384+
385+
let mut tls_client_credentials = BTreeSet::new();
386+
387+
for auth_class_resolved in &auth_config.authentication_classes_resolved {
388+
match auth_class_resolved {
389+
AirflowAuthenticationClassResolved::Oidc { provider, .. } => {
390+
tls_client_credentials.insert(&provider.tls);
391+
392+
// WebPKI will be handled implicitly
393+
}
394+
AirflowAuthenticationClassResolved::Ldap { .. } => {}
395+
}
396+
}
397+
398+
for tls in tls_client_credentials {
399+
commands.push(tls.tls_ca_cert_mount_path().map(|tls_ca_cert_mount_path| {
400+
Self::add_cert_to_python_certifi_command(&tls_ca_cert_mount_path)
401+
}));
402+
}
403+
404+
commands.iter().flatten().cloned().collect::<Vec<_>>()
405+
}
406+
407+
// Adding certificate to the mount path for airflow startup commands
408+
fn add_cert_to_python_certifi_command(cert_file: &str) -> String {
409+
format!("cat {cert_file} >> \"$(python -c 'import certifi; print(certifi.where())')\"")
410+
}
411+
367412
/// Will be used to expose service ports and - by extension - which roles should be
368413
/// created as services.
369414
pub fn get_http_port(&self) -> Option<u16> {

rust/operator-binary/src/airflow_controller.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -883,7 +883,7 @@ fn build_server_rolegroup_statefulset(
883883
.context(GracefulShutdownSnafu)?;
884884

885885
let mut airflow_container_args = Vec::new();
886-
airflow_container_args.extend(airflow_role.get_commands());
886+
airflow_container_args.extend(airflow_role.get_commands(authentication_config));
887887

888888
airflow_container
889889
.image_from_product_image(resolved_product_image)

rust/operator-binary/src/env_vars.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -200,14 +200,6 @@ pub fn build_airflow_statefulset_envs(
200200
AirflowRole::Webserver => {
201201
let auth_vars = authentication_env_vars(auth_config);
202202
env.extend(auth_vars.into_iter().map(|var| (var.name.to_owned(), var)));
203-
env.insert(
204-
"REQUESTS_CA_BUNDLE".into(),
205-
EnvVar {
206-
name: "REQUESTS_CA_BUNDLE".to_string(),
207-
value: Some("/stackable/secrets/tls/ca.crt".to_string()),
208-
..Default::default()
209-
},
210-
);
211203
}
212204
_ => {}
213205
}

0 commit comments

Comments
 (0)