From 6c75adf276bae83fb8412f0ee4009a3980d0259a Mon Sep 17 00:00:00 2001 From: Sebastian Bernauer Date: Mon, 20 Nov 2023 15:53:21 +0100 Subject: [PATCH] update to new operator-rs --- Cargo.lock | 4 +- rust/crd/src/authentication.rs | 14 ++++++- .../operator-binary/src/authentication/mod.rs | 24 ++++++------ .../src/authentication/oidc/mod.rs | 39 ++++++------------- 4 files changed, 38 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c137740b..bd2b4865 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2040,7 +2040,7 @@ checksum = "6980e8d7511241f8acf4aebddbb1ff938df5eebe98691418c4468d0b72a96a67" [[package]] name = "stackable-operator" version = "0.56.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=feat/sso-auth-classes#0fce5c7ab3f83bde49b7db9c724deaf31adcd28b" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=feat/sso-auth-classes#1e326ea34ba5b1ef08e18d85060ca19a9f773912" dependencies = [ "chrono", "clap", @@ -2076,7 +2076,7 @@ dependencies = [ [[package]] name = "stackable-operator-derive" version = "0.56.0" -source = "git+https://github.com/stackabletech//operator-rs.git?branch=feat/sso-auth-classes#0fce5c7ab3f83bde49b7db9c724deaf31adcd28b" +source = "git+https://github.com/stackabletech//operator-rs.git?branch=feat/sso-auth-classes#1e326ea34ba5b1ef08e18d85060ca19a9f773912" dependencies = [ "darling 0.20.3", "proc-macro2", diff --git a/rust/crd/src/authentication.rs b/rust/crd/src/authentication.rs index f9f0caef..ac1ab075 100644 --- a/rust/crd/src/authentication.rs +++ b/rust/crd/src/authentication.rs @@ -2,6 +2,7 @@ use snafu::{ResultExt, Snafu}; use stackable_operator::{ client::Client, commons::authentication::{oidc, AuthenticationClass, ClientAuthenticationDetails}, + kube::ResourceExt, }; #[derive(Snafu, Debug)] @@ -10,13 +11,18 @@ pub enum Error { AuthenticationClassRetrieval { source: stackable_operator::error::Error, }, + + #[snafu(display("Invalid OIDC configuration"))] + OidcConfiguration { + source: stackable_operator::error::Error, + }, } type Result = std::result::Result; pub struct ResolvedAuthenticationClassRef { pub authentication_class: AuthenticationClass, - pub oidc: Option, + pub oidc: oidc::ClientAuthenticationOptions, } /// Retrieve all provided `AuthenticationClass` references. @@ -31,10 +37,14 @@ pub async fn resolve_authentication_classes( .resolve_class(client) .await .context(AuthenticationClassRetrievalSnafu)?; + let auth_class_name = resolved_auth_class.name_any(); resolved_auth_classes.push(ResolvedAuthenticationClassRef { authentication_class: resolved_auth_class, - oidc: client_authentication_detail.oidc.clone(), + oidc: client_authentication_detail + .oidc_or_error(auth_class_name) + .context(OidcConfigurationSnafu)? + .clone(), }); } diff --git a/rust/operator-binary/src/authentication/mod.rs b/rust/operator-binary/src/authentication/mod.rs index b3c0fc09..5180f164 100644 --- a/rust/operator-binary/src/authentication/mod.rs +++ b/rust/operator-binary/src/authentication/mod.rs @@ -494,14 +494,8 @@ impl TryFrom> for TrinoAuthenticationTypes { oidc_authenticators.push(OidcAuthenticator::new( auth_class_name, provider, - resolved_auth_class - .oidc - .as_ref() - .map(|o| o.client_credentials_secret_ref.clone()), - resolved_auth_class - .oidc - .map(|o| o.extra_scopes) - .unwrap_or_default(), + resolved_auth_class.oidc.client_credentials_secret_ref, + resolved_auth_class.oidc.extra_scopes, )); TrinoAuthenticationTypes::insert_auth_type_order( @@ -582,7 +576,10 @@ mod tests { ), }, }, - oidc: None, + oidc: oidc::ClientAuthenticationOptions { + client_credentials_secret_ref: "my-oidc-secret".to_string(), + extra_scopes: Vec::new(), + }, } } @@ -629,7 +626,10 @@ mod tests { deserializer, ) .unwrap(), - oidc: None, + oidc: oidc::ClientAuthenticationOptions { + client_credentials_secret_ref: "my-oidc-secret".to_string(), + extra_scopes: Vec::new(), + }, } } @@ -655,10 +655,10 @@ mod tests { deserializer, ) .unwrap(), - oidc: Some(oidc::ClientAuthenticationOptions { + oidc: oidc::ClientAuthenticationOptions { client_credentials_secret_ref: "my-oidc-secret".to_string(), extra_scopes: Vec::new(), - }), + }, } } diff --git a/rust/operator-binary/src/authentication/oidc/mod.rs b/rust/operator-binary/src/authentication/oidc/mod.rs index e9552ca9..a7671e18 100644 --- a/rust/operator-binary/src/authentication/oidc/mod.rs +++ b/rust/operator-binary/src/authentication/oidc/mod.rs @@ -3,7 +3,7 @@ use crate::authentication::TrinoAuthenticationConfig; use crate::command; -use snafu::{OptionExt, ResultExt, Snafu}; +use snafu::{ResultExt, Snafu}; use stackable_operator::commons::authentication::oidc::{ self, CLIENT_ID_SECRET_KEY, CLIENT_SECRET_SECRET_KEY, }; @@ -56,7 +56,7 @@ pub struct TrinoOidcAuthentication { pub struct OidcAuthenticator { name: String, oidc: oidc::AuthenticationProvider, - secret: Option, + client_credentials_secret: String, extra_scopes: Vec, } @@ -64,13 +64,13 @@ impl OidcAuthenticator { pub fn new( name: String, provider: oidc::AuthenticationProvider, - secret_ref: Option, + client_credentials_secret: String, extra_scopes: Vec, ) -> Self { Self { name, oidc: provider, - secret: secret_ref, + client_credentials_secret, extra_scopes, } } @@ -87,15 +87,6 @@ impl TrinoOidcAuthentication { // Check for single OAuth2 AuthenticationClass and error out if multiple were provided let authenticator = self.get_single_oauth2_authentication_class()?; - // We require a secret with client credentials - let secret_name = - authenticator - .secret - .as_deref() - .context(MissingOauth2CredentialSecretSnafu { - auth_class_name: authenticator.name.clone(), - })?; - let issuer = authenticator .oidc .endpoint_url() @@ -117,13 +108,15 @@ impl TrinoOidcAuthentication { ); let (client_id_env, client_secret_env) = - oidc::AuthenticationProvider::client_credentials_env_names(secret_name); + oidc::AuthenticationProvider::client_credentials_env_names( + &authenticator.client_credentials_secret, + ); oauth2_authentication_config.add_env_vars( TrinoRole::Coordinator, stackable_trino_crd::Container::Trino, oidc::AuthenticationProvider::client_credentials_env_var_mounts( - secret_name.to_string(), + authenticator.client_credentials_secret, ), ); @@ -217,7 +210,7 @@ mod tests { fn setup_test_authenticator( auth_class_name: &str, - credential_secret: Option, + credential_secret: String, ) -> OidcAuthenticator { let input = format!( r#" @@ -242,26 +235,18 @@ mod tests { #[test] fn test_oidc_authentication_limit_one_error() { let oidc_authentication = TrinoOidcAuthentication::new(vec![ - setup_test_authenticator(AUTH_CLASS_NAME_1, None), - setup_test_authenticator(AUTH_CLASS_NAME_2, None), + setup_test_authenticator(AUTH_CLASS_NAME_1, AUTH_CLASS_CREDENTIAL_SECRET.to_string()), + setup_test_authenticator(AUTH_CLASS_NAME_2, AUTH_CLASS_CREDENTIAL_SECRET.to_string()), ]); assert!(oidc_authentication.oauth2_authentication_config().is_err()) } - #[test] - fn test_oidc_authentication_missing_secret_error() { - let oidc_authentication = - TrinoOidcAuthentication::new(vec![setup_test_authenticator(AUTH_CLASS_NAME_1, None)]); - - assert!(oidc_authentication.oauth2_authentication_config().is_err()); - } - #[test] fn test_oidc_authentication_settings() { let oidc_authentication = TrinoOidcAuthentication::new(vec![setup_test_authenticator( AUTH_CLASS_NAME_1, - Some(AUTH_CLASS_CREDENTIAL_SECRET.to_string()), + AUTH_CLASS_CREDENTIAL_SECRET.to_string(), )]); let trino_oidc_auth = oidc_authentication.oauth2_authentication_config().unwrap();