Merge branch 'main' into fix/SUP-148

Author: siegfriedweber

CHANGELOG.md

@@ -24,6 +24,7 @@ All notable changes to this project will be documented in this file.
 - Don't ignore envOverrides ([#633]).
 - Don't print credentials to STDOUT during startup. Ideally we should use [config-utils](https://github.com/stackabletech/config-utils), but that's not easy (see [here](https://github.com/stackabletech/trino-operator/tree/fix/secret-printing)) ([#634]).
 - Invalid `TrinoCluster`, `TrinoCatalog` or `AuthenticationClass` objects don't stop the operator from reconciliation ([#657])
+- Fix OIDC endpoint construction in case the `rootPath` does have a trailing slash ([#673]).
 - BREAKING: Use distinct ServiceAccounts for the Stacklets, so that multiple Stacklets can be deployed in one namespace. Existing Stacklets will use the newly created ServiceAccounts after restart ([#672]).
 
 ### Removed
@@ -38,6 +39,7 @@ All notable changes to this project will be documented in this file.
 [#655]: https://github.com/stackabletech/trino-operator/pull/655
 [#657]: https://github.com/stackabletech/trino-operator/pull/657
 [#672]: https://github.com/stackabletech/trino-operator/pull/672
+[#673]: https://github.com/stackabletech/trino-operator/pull/673
 
 ## [24.7.0] - 2024-07-24
 

docs/modules/trino/examples/usage-guide/trino-oidc-auth-snippet.yaml

@@ -21,7 +21,7 @@ spec:
     oidc:
       hostname: keycloak.default.svc.cluster.local
       port: 8080
-      rootPath: /realms/stackable
+      rootPath: /realms/stackable/
       scopes:
       - openid
       principalClaim: preferred_username

examples/simple-trino-oauth2.yaml

@@ -53,7 +53,7 @@ spec:
     oidc:
       hostname: keycloak
       port: 8080
-      rootPath: /realms/stackable
+      rootPath: /realms/stackable/
       scopes: ["openid"]
       principalClaim: preferred_username
 ---

rust/crd/src/authentication.rs

@@ -25,7 +25,7 @@ type Result<T, E = Error> = std::result::Result<T, E>;
 pub struct ResolvedAuthenticationClassRef {
     /// An [AuthenticationClass](DOCS_BASE_URL_PLACEHOLDER/concepts/authentication) to use.
     pub authentication_class: AuthenticationClass,
-    pub oidc: Option<oidc::ClientAuthenticationOptions>,
+    pub client_auth_options: Option<oidc::ClientAuthenticationOptions>,
 }
 
 /// Retrieve all provided AuthenticationClass references.
@@ -43,7 +43,7 @@ pub async fn resolve_authentication_classes(
         let auth_class_name = resolved_auth_class.name_any();
 
         resolved_auth_classes.push(ResolvedAuthenticationClassRef {
-            oidc: match &resolved_auth_class.spec.provider {
+            client_auth_options: match &resolved_auth_class.spec.provider {
                 AuthenticationClassProvider::Oidc(_) => Some(
                     client_authentication_detail
                         .oidc_or_error(&auth_class_name)

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

@@ -508,7 +508,7 @@ impl TryFrom<Vec<ResolvedAuthenticationClassRef>> for TrinoAuthenticationTypes {
                     );
                 }
                 AuthenticationClassProvider::Oidc(provider) => {
-                    let oidc = resolved_auth_class.oidc.context(
+                    let oidc = resolved_auth_class.client_auth_options.context(
                         OidcAuthenticationDetailsNotSpecifiedSnafu {
                             auth_class_name: auth_class_name.clone(),
                         },
@@ -591,7 +591,7 @@ mod tests {
 
         ResolvedAuthenticationClassRef {
             authentication_class: input,
-            oidc: None,
+            client_auth_options: None,
         }
     }
 
@@ -610,7 +610,7 @@ mod tests {
 
         ResolvedAuthenticationClassRef {
             authentication_class: input,
-            oidc: None,
+            client_auth_options: None,
         }
     }
 
@@ -636,7 +636,7 @@ mod tests {
 
         ResolvedAuthenticationClassRef {
             authentication_class: input,
-            oidc: None,
+            client_auth_options: None,
         }
     }
 
@@ -651,7 +651,7 @@ mod tests {
               provider:
                 oidc:
                   hostname: {HOST_NAME}
-                  rootPath: /realms/master
+                  rootPath: /realms/master/
                   scopes: ["openid"]
                   principalClaim: preferred_username
             "#,
@@ -663,7 +663,7 @@ mod tests {
                 deserializer,
             )
             .unwrap(),
-            oidc: Some(ClientAuthenticationOptions {
+            client_auth_options: Some(ClientAuthenticationOptions {
                 client_credentials_secret_ref: "my-oidc-secret".to_string(),
                 extra_scopes: Vec::new(),
                 product_specific_fields: (),

rust/operator-binary/src/authentication/oidc/mod.rs

@@ -210,10 +210,10 @@ mod tests {
     use std::mem;
 
     use super::*;
+    use rstest::rstest;
     use stackable_trino_crd::Container;
 
     const IDP_PORT: u16 = 8080;
-    const IDP_ROOT_PATH: &str = "/realms/master";
     const IDP_SCOPE_1: &str = "openid";
     const IDP_SCOPE_2: &str = "test";
     const AUTH_CLASS_NAME_1: &str = "trino-oidc-auth-1";
@@ -223,12 +223,13 @@ mod tests {
     fn setup_test_authenticator(
         auth_class_name: &str,
         credential_secret: String,
+        root_path: &str,
     ) -> OidcAuthenticator {
         let input = format!(
             r#"
             hostname: keycloak
             port: {IDP_PORT}
-            rootPath: {IDP_ROOT_PATH}
+            rootPath: {root_path}
             scopes:
               - {IDP_SCOPE_1}
             principalClaim: preferred_username
@@ -249,8 +250,16 @@ mod tests {
     #[test]
     fn test_oidc_authentication_limit_one_error() {
         let oidc_authentication = TrinoOidcAuthentication::new(vec![
-            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()),
+            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(),
+                "/",
+            ),
         ]);
 
         let error = oidc_authentication
@@ -264,17 +273,21 @@ mod tests {
         );
     }
 
-    #[test]
-    fn test_oidc_authentication_settings() {
+    #[rstest]
+    #[case("/realms/sdp")]
+    #[case("/realms/sdp/")]
+    #[case("/realms/sdp/////")]
+    fn test_oidc_authentication_settings(#[case] root_path: &str) {
         let oidc_authentication = TrinoOidcAuthentication::new(vec![setup_test_authenticator(
             AUTH_CLASS_NAME_1,
             AUTH_CLASS_CREDENTIAL_SECRET.to_string(),
+            root_path,
         )]);
 
         let trino_oidc_auth = oidc_authentication.oauth2_authentication_config().unwrap();
 
         assert_eq!(
-            Some(&format!("http://keycloak:{IDP_PORT}{IDP_ROOT_PATH}")),
+            Some(&format!("http://keycloak:{IDP_PORT}/realms/sdp")),
             trino_oidc_auth
                 .config_properties
                 .get(&TrinoRole::Coordinator)

tests/templates/kuttl/authentication/create-authentication-classes.yaml.j2

@@ -8,7 +8,7 @@ spec:
     oidc:
       hostname: keycloak.$NAMESPACE.svc.cluster.local
       port: 8443
-      rootPath: /realms/stackable
+      rootPath: /realms/stackable/
       scopes:
         - openid
       principalClaim: preferred_username