From 36b65a60ea1c1f874ba3454b4d732241c5e21e79 Mon Sep 17 00:00:00 2001 From: Maximilian Wittich <56642549+Maleware@users.noreply.github.com> Date: Mon, 11 Nov 2024 12:35:11 +0100 Subject: [PATCH] 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 17a6049d73ce24216795ef8a952b25cac2cab453. * Using python certify to use ca certs * Adding comment to AirflowRole * Apply suggestions from code review Applying Review Co-authored-by: Siegfried Weber * Further adapting review * remove reject_different_tls_ca_certs() * Update Cargo.nix * Adding code review --------- Co-authored-by: Siegfried Weber --- CHANGELOG.md | 3 +- Cargo.lock | 6 +- Cargo.nix | 9 ++- rust/crd/src/lib.rs | 57 +++++++++++++++++-- .../operator-binary/src/airflow_controller.rs | 2 +- rust/operator-binary/src/env_vars.rs | 8 --- 6 files changed, 61 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68f9cbac..d19d70e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ Use the env var `KUBERNETES_CLUSTER_DOMAIN` or the operator Helm chart property `kubernetesClusterDomain` to set a non-default cluster domain ([#518]). - Support for `2.9.3` ([#494]). - Experimental Support for `2.10.2` ([#512]). -- Add support for OpenID Connect ([#524]) +- Add support for OpenID Connect ([#524], [#530]) ### Changed @@ -32,6 +32,7 @@ [#518]: https://github.com/stackabletech/airflow-operator/pull/518 [#520]: https://github.com/stackabletech/airflow-operator/pull/520 [#524]: https://github.com/stackabletech/airflow-operator/pull/524 +[#530]: https://github.com/stackabletech/airflow-operator/pull/530 ## [24.7.0] - 2024-07-24 diff --git a/Cargo.lock b/Cargo.lock index 247ba76a..0efb5c36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -803,9 +803,9 @@ dependencies = [ [[package]] name = "hashbrown" -version = "0.15.0" +version = "0.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e087f84d4f86bf4b218b927129862374b72199ae7d8657835f1e89000eea4fb" +checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" [[package]] name = "headers" @@ -1034,7 +1034,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "707907fe3c25f5424cce2cb7e1cbcafee6bdbe735ca90ef77c29e84591e5b9da" dependencies = [ "equivalent", - "hashbrown 0.15.0", + "hashbrown 0.15.1", ] [[package]] diff --git a/Cargo.nix b/Cargo.nix index afc575fd..22c13476 100644 --- a/Cargo.nix +++ b/Cargo.nix @@ -2284,18 +2284,17 @@ rec { }; resolvedDefaultFeatures = [ "ahash" "allocator-api2" "default" "inline-more" ]; }; - "hashbrown 0.15.0" = rec { + "hashbrown 0.15.1" = rec { crateName = "hashbrown"; - version = "0.15.0"; + version = "0.15.1"; edition = "2021"; - sha256 = "1yx4xq091s7i6mw6bn77k8cp4jrpcac149xr32rg8szqsj27y20y"; + sha256 = "1czsvasi3azv2079fcvbhvpisa16w6fi1mfk8zm2c5wbyqdgr6rs"; authors = [ "Amanieu d'Antras " ]; features = { "alloc" = [ "dep:alloc" ]; "allocator-api2" = [ "dep:allocator-api2" ]; - "borsh" = [ "dep:borsh" ]; "compiler_builtins" = [ "dep:compiler_builtins" ]; "core" = [ "dep:core" ]; "default" = [ "default-hasher" "inline-more" "allocator-api2" "equivalent" "raw-entry" ]; @@ -3062,7 +3061,7 @@ rec { } { name = "hashbrown"; - packageId = "hashbrown 0.15.0"; + packageId = "hashbrown 0.15.1"; usesDefaultFeatures = false; } ]; diff --git a/rust/crd/src/lib.rs b/rust/crd/src/lib.rs index d51e4e6a..7d7be665 100644 --- a/rust/crd/src/lib.rs +++ b/rust/crd/src/lib.rs @@ -1,5 +1,8 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; +use authentication::{ + AirflowAuthenticationClassResolved, AirflowClientAuthenticationDetailsResolved, +}; use git_sync::GitSync; use product_config::flask_app_config_writer::{FlaskAppConfigOptions, PythonType}; use serde::{Deserialize, Serialize}; @@ -322,7 +325,12 @@ impl AirflowRole { /// components to have the same image/configuration (e.g. DAG folder location), even if not all /// configuration settings are used everywhere. For this reason we ensure that the webserver /// config file is in the Airflow home directory on all pods. - pub fn get_commands(&self) -> Vec { + /// Only the webserver needs to know about authentication CA's which is added via python's certify + /// if authentication is enabled. + pub fn get_commands( + &self, + auth_config: &AirflowClientAuthenticationDetailsResolved, + ) -> Vec { let mut command = vec![ format!("cp -RL {CONFIG_PATH}/{AIRFLOW_CONFIG_FILENAME} {AIRFLOW_HOME}/{AIRFLOW_CONFIG_FILENAME}"), // graceful shutdown part @@ -331,10 +339,15 @@ impl AirflowRole { ]; match &self { - AirflowRole::Webserver => command.extend(vec![ - "prepare_signal_handlers".to_string(), - "airflow webserver &".to_string(), - ]), + AirflowRole::Webserver => { + // Getting auth commands for AuthClass + command.extend(Self::authentication_start_commands(auth_config)); + command.extend(vec![ + "prepare_signal_handlers".to_string(), + "airflow webserver &".to_string(), + ]); + } + AirflowRole::Scheduler => command.extend(vec![ // Database initialization is limited to the scheduler, see https://github.com/stackabletech/airflow-operator/issues/259 "airflow db init".to_string(), @@ -364,6 +377,38 @@ impl AirflowRole { command } + fn authentication_start_commands( + auth_config: &AirflowClientAuthenticationDetailsResolved, + ) -> Vec { + let mut commands = Vec::new(); + + let mut tls_client_credentials = BTreeSet::new(); + + for auth_class_resolved in &auth_config.authentication_classes_resolved { + match auth_class_resolved { + AirflowAuthenticationClassResolved::Oidc { provider, .. } => { + tls_client_credentials.insert(&provider.tls); + + // WebPKI will be handled implicitly + } + AirflowAuthenticationClassResolved::Ldap { .. } => {} + } + } + + for tls in tls_client_credentials { + commands.push(tls.tls_ca_cert_mount_path().map(|tls_ca_cert_mount_path| { + Self::add_cert_to_python_certifi_command(&tls_ca_cert_mount_path) + })); + } + + commands.iter().flatten().cloned().collect::>() + } + + // Adding certificate to the mount path for airflow startup commands + fn add_cert_to_python_certifi_command(cert_file: &str) -> String { + format!("cat {cert_file} >> \"$(python -c 'import certifi; print(certifi.where())')\"") + } + /// Will be used to expose service ports and - by extension - which roles should be /// created as services. pub fn get_http_port(&self) -> Option { diff --git a/rust/operator-binary/src/airflow_controller.rs b/rust/operator-binary/src/airflow_controller.rs index 97c96713..c5b9efbb 100644 --- a/rust/operator-binary/src/airflow_controller.rs +++ b/rust/operator-binary/src/airflow_controller.rs @@ -883,7 +883,7 @@ fn build_server_rolegroup_statefulset( .context(GracefulShutdownSnafu)?; let mut airflow_container_args = Vec::new(); - airflow_container_args.extend(airflow_role.get_commands()); + airflow_container_args.extend(airflow_role.get_commands(authentication_config)); airflow_container .image_from_product_image(resolved_product_image) diff --git a/rust/operator-binary/src/env_vars.rs b/rust/operator-binary/src/env_vars.rs index 5d581090..e1e77a5b 100644 --- a/rust/operator-binary/src/env_vars.rs +++ b/rust/operator-binary/src/env_vars.rs @@ -200,14 +200,6 @@ pub fn build_airflow_statefulset_envs( AirflowRole::Webserver => { let auth_vars = authentication_env_vars(auth_config); env.extend(auth_vars.into_iter().map(|var| (var.name.to_owned(), var))); - env.insert( - "REQUESTS_CA_BUNDLE".into(), - EnvVar { - name: "REQUESTS_CA_BUNDLE".to_string(), - value: Some("/stackable/secrets/tls/ca.crt".to_string()), - ..Default::default() - }, - ); } _ => {} }