fix: Construction of OIDC endpoint when rootPath has a trailing slash (#673)

* fix: Calculation of OIDC endpoint when rootPath has a trailing slash

* Update test

* Use op-rs 0.82.0

* nixfiles

* Update tests and docs

* changelog

* Rename oidc -> client_auth_options

Author: sbernauer

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]).
 
 ### Removed
 
@@ -36,6 +37,7 @@ All notable changes to this project will be documented in this file.
 [#646]: https://github.com/stackabletech/trino-operator/pull/646
 [#655]: https://github.com/stackabletech/trino-operator/pull/655
 [#657]: https://github.com/stackabletech/trino-operator/pull/657
+[#673]: https://github.com/stackabletech/trino-operator/pull/673
 
 ## [24.7.0] - 2024-07-24
 

Cargo.lock

@@ -24,7 +24,7 @@ serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
 serde_yaml = "0.9"
 snafu = "0.8"
-stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.80.0" }
+stackable-operator = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-operator-0.82.0" }
 product-config = { git = "https://github.com/stackabletech/product-config.git", tag = "0.7.0" }
 strum = { version = "0.26", features = ["derive"] }
 tokio = { version = "1.40", features = ["full"] }

Cargo.nix

@@ -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

Cargo.toml

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

crate-hashes.json

@@ -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)

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

@@ -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: (),

examples/simple-trino-oauth2.yaml

@@ -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)