Skip to content

Commit

Permalink
fix: TLS CA cert path used in REQUESTS_CA_BUNDLE
Browse files Browse the repository at this point in the history
  • Loading branch information
@siegfriedweber
siegfriedweber committed Nov 4, 2024
1 parent 6529f52 commit 7249f91
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 29 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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], [#529])

### Changed

Expand All @@ -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
[#529]: https://github.com/stackabletech/airflow-operator/pull/529

## [24.7.0] - 2024-07-24

Expand Down
158 changes: 138 additions & 20 deletions rust/crd/src/authentication.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@ use serde::{Deserialize, Serialize};
use snafu::{ensure, ResultExt, Snafu};
use stackable_operator::{
client::Client,
commons::authentication::{
ldap,
oidc::{self, IdentityProviderHint},
AuthenticationClass, AuthenticationClassProvider, ClientAuthenticationDetails,
commons::{
authentication::{
ldap,
oidc::{self, IdentityProviderHint},
AuthenticationClass, AuthenticationClassProvider, ClientAuthenticationDetails,
},
tls_verification::TlsClientDetails,
},
schemars::{self, JsonSchema},
};
Expand Down Expand Up @@ -78,6 +81,9 @@ pub enum Error {
supported: String,
auth_class_name: String,
},

#[snafu(display("Currently only one CA certificate is supported."))]
MultipleCaCertsNotSupported,
}

type Result<T, E = Error> = std::result::Result<T, E>;
Expand Down Expand Up @@ -130,17 +136,7 @@ pub struct AirflowClientAuthenticationDetailsResolved {
pub user_registration: bool,
pub user_registration_role: String,
pub sync_roles_at: FlaskRolesSyncMoment,
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum AirflowAuthenticationClassResolved {
Ldap {
provider: ldap::AuthenticationProvider,
},
Oidc {
provider: oidc::AuthenticationProvider,
oidc: oidc::ClientAuthenticationOptions<()>,
},
pub tls_ca_cert_mount_path: Option<String>,
}

impl AirflowClientAuthenticationDetailsResolved {
Expand Down Expand Up @@ -254,12 +250,24 @@ impl AirflowClientAuthenticationDetailsResolved {
None => sync_roles_at = Some(entry.sync_roles_at.to_owned()),
}
}

let mut tls_ca_cert_mount_paths = resolved_auth_classes
.iter()
.filter_map(AirflowAuthenticationClassResolved::tls_ca_cert_mount_path)
.collect::<BTreeSet<_>>();
let tls_ca_cert_mount_path = tls_ca_cert_mount_paths.pop_first();
ensure!(
tls_ca_cert_mount_paths.is_empty(),
MultipleCaCertsNotSupportedSnafu
);

Ok(AirflowClientAuthenticationDetailsResolved {
authentication_classes_resolved: resolved_auth_classes,
user_registration: user_registration.unwrap_or_else(default_user_registration),
user_registration_role: user_registration_role
.unwrap_or_else(default_user_registration_role),
sync_roles_at: sync_roles_at.unwrap_or_else(FlaskRolesSyncMoment::default),
tls_ca_cert_mount_path,
})
}

Expand Down Expand Up @@ -314,6 +322,35 @@ impl AirflowClientAuthenticationDetailsResolved {
}
}

#[derive(Clone, Debug, Eq, PartialEq)]
pub enum AirflowAuthenticationClassResolved {
Ldap {
provider: ldap::AuthenticationProvider,
},
Oidc {
provider: oidc::AuthenticationProvider,
oidc: oidc::ClientAuthenticationOptions<()>,
},
}

impl AirflowAuthenticationClassResolved {
pub fn tls_ca_cert_mount_path(&self) -> Option<String> {
self.tls_client_details().tls_ca_cert_mount_path()
}

pub fn tls_client_details(&self) -> &TlsClientDetails {
match self {
AirflowAuthenticationClassResolved::Ldap {
provider: ldap::AuthenticationProvider { tls, .. },
} => tls,
AirflowAuthenticationClassResolved::Oidc {
provider: oidc::AuthenticationProvider { tls, .. },
..
} => tls,
}
}
}

#[cfg(test)]
mod tests {
use std::pin::Pin;
Expand All @@ -336,7 +373,8 @@ mod tests {
authentication_classes_resolved: Vec::default(),
user_registration: default_user_registration(),
user_registration_role: default_user_registration_role(),
sync_roles_at: FlaskRolesSyncMoment::default()
sync_roles_at: FlaskRolesSyncMoment::default(),
tls_ca_cert_mount_path: None,
},
auth_details_resolved
);
Expand All @@ -362,18 +400,32 @@ mod tests {
provider:
ldap:
hostname: my.ldap.server
tls:
verification:
server:
caCert:
secretClass: tls
"},
)
.await;

assert_eq!(
AirflowClientAuthenticationDetailsResolved {
authentication_classes_resolved: vec![AirflowAuthenticationClassResolved::Ldap {
provider: serde_yaml::from_str("hostname: my.ldap.server").unwrap()
provider: serde_yaml::from_str(indoc! {"
hostname: my.ldap.server
tls:
verification:
server:
caCert:
secretClass: tls
"})
.unwrap()
}],
user_registration: false,
user_registration_role: "Gamma".into(),
sync_roles_at: FlaskRolesSyncMoment::Login
sync_roles_at: FlaskRolesSyncMoment::Login,
tls_ca_cert_mount_path: Some("/stackable/secrets/tls/ca.crt".into()),
},
auth_details_resolved
);
Expand Down Expand Up @@ -437,6 +489,11 @@ mod tests {
- openid
- email
- profile
tls:
verification:
server:
caCert:
secretClass: tls
"},
)
.await;
Expand Down Expand Up @@ -471,7 +528,13 @@ mod tests {
HostName::try_from("second.oidc.server".to_string()).unwrap(),
None,
"/realms/test".into(),
TlsClientDetails { tls: None },
TlsClientDetails {
tls: Some(Tls {
verification: TlsVerification::Server(TlsServerVerification {
ca_cert: CaCert::SecretClass("tls".into())
})
})
},
"preferred_username".into(),
vec!["openid".into(), "email".into(), "profile".into()],
None
Expand All @@ -485,7 +548,8 @@ mod tests {
],
user_registration: false,
user_registration_role: "Gamma".into(),
sync_roles_at: FlaskRolesSyncMoment::Login
sync_roles_at: FlaskRolesSyncMoment::Login,
tls_ca_cert_mount_path: Some("/stackable/secrets/tls/ca.crt".into()),
},
auth_details_resolved
);
Expand Down Expand Up @@ -829,6 +893,60 @@ mod tests {
);
}

#[tokio::test]
async fn reject_different_tls_ca_certs() {
let error_message = test_resolve_and_expect_error(
indoc! {"
- authenticationClass: oidc1
oidc:
clientCredentialsSecret: airflow-oidc-client1
- authenticationClass: oidc2
oidc:
clientCredentialsSecret: airflow-oidc-client2
"},
indoc! {"
---
apiVersion: authentication.stackable.tech/v1alpha1
kind: AuthenticationClass
metadata:
name: oidc1
spec:
provider:
oidc:
hostname: first.oidc.server
principalClaim: preferred_username
scopes: []
tls:
verification:
server:
caCert:
secretClass: tls1
---
apiVersion: authentication.stackable.tech/v1alpha1
kind: AuthenticationClass
metadata:
name: oidc2
spec:
provider:
oidc:
hostname: second.oidc.server
principalClaim: preferred_username
scopes: []
tls:
verification:
server:
caCert:
secretClass: tls2
"},
)
.await;

assert_eq!(
"Currently only one CA certificate is supported.",
error_message
);
}

/// Call `AirflowClientAuthenticationDetailsResolved::resolve` with
/// the given lists of `AirflowClientAuthenticationDetails` and
/// `AuthenticationClass`es and return the
Expand Down
3 changes: 3 additions & 0 deletions rust/operator-binary/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ mod tests {
user_registration: true,
user_registration_role: "User".to_string(),
sync_roles_at: FlaskRolesSyncMoment::Registration,
tls_ca_cert_mount_path: None,
};

let mut result = BTreeMap::new();
Expand Down Expand Up @@ -314,6 +315,7 @@ mod tests {
user_registration: true,
user_registration_role: "Admin".to_string(),
sync_roles_at: FlaskRolesSyncMoment::Registration,
tls_ca_cert_mount_path: Some("/stackable/secrets/openldap-tls/ca.crt".to_string()),
};

let mut result = BTreeMap::new();
Expand Down Expand Up @@ -394,6 +396,7 @@ mod tests {
user_registration: default_user_registration(),
user_registration_role: "Admin".to_string(),
sync_roles_at: default_sync_roles_at(),
tls_ca_cert_mount_path: Some("keycloak-ca-cert".to_string()),
};

let mut result = BTreeMap::new();
Expand Down
18 changes: 10 additions & 8 deletions rust/operator-binary/src/env_vars.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,14 +200,16 @@ 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()
},
);
if let Some(tls_ca_cert_mount_path) = &auth_config.tls_ca_cert_mount_path {
env.insert(
"REQUESTS_CA_BUNDLE".into(),
EnvVar {
name: "REQUESTS_CA_BUNDLE".to_string(),
value: Some(tls_ca_cert_mount_path.to_owned()),
..Default::default()
},
);
}
}
_ => {}
}
Expand Down

0 comments on commit 7249f91

Please sign in to comment.