test: Enable hot reloading in password files (#868)

* test: Enable hot reloading in password files

* doc: Explain the annotation restarter.stackable.tech/ignore in the security documentation

* chore: Update changelog

* feat: Add annotations to the StatefulSet to ignore hot-reloadable Secrets

* chore: Avoid an unnecessary clone of a data structure

Author: siegfriedweber

CHANGELOG.md

@@ -20,7 +20,9 @@ All notable changes to this project will be documented in this file.
   This caused problems, as they have been cached by Kubernetes, so re-creations of the mentioned Secrets (e.g. by deleting and re-creating the stacklet)
   could cause Trino Pods to have different shared secrets, causing workers failing to join the coordinator.
   This fix places the secrets in mutable Kubernetes Secrets going forward and migrates existing immutable Secrets to mutable by re-creating them ([#876]).
+- Re-enable hot-reloading of password file Secrets ([#868]).
 
+[#868]: https://github.com/stackabletech/trino-operator/pull/868
 [#869]: https://github.com/stackabletech/trino-operator/pull/869
 [#876]: https://github.com/stackabletech/trino-operator/pull/876
 [#878]: https://github.com/stackabletech/trino-operator/pull/878

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

@@ -7,7 +7,7 @@
 //! - volume and volume mounts
 //! - extra containers and commands
 //!
-use std::collections::{BTreeMap, HashMap};
+use std::collections::{BTreeMap, BTreeSet, HashMap};
 
 use snafu::{OptionExt, ResultExt, Snafu};
 use stackable_operator::{
@@ -99,6 +99,8 @@ pub struct TrinoAuthenticationConfig {
     volume_mounts: HashMap<TrinoRole, BTreeMap<crate::crd::Container, Vec<VolumeMount>>>,
     /// Additional side car container for the provided role
     sidecar_containers: HashMap<TrinoRole, Vec<Container>>,
+    /// Secrets which can be hot-reloaded and should be excluded from the restart controller
+    hot_reloaded_secrets: BTreeSet<String>,
 }
 
 impl TrinoAuthenticationConfig {
@@ -368,6 +370,16 @@ impl TrinoAuthenticationConfig {
             .unwrap_or_default()
     }
 
+    /// Retrieve all Secrets which can be hot-reloaded
+    pub fn hot_reloaded_secrets(&self) -> &BTreeSet<String> {
+        &self.hot_reloaded_secrets
+    }
+
+    /// Add a Secret which can be hot-reloaded
+    pub fn add_hot_reloaded_secret(&mut self, secret_name: String) {
+        self.hot_reloaded_secrets.insert(secret_name);
+    }
+
     /// This is a helper to easily extend/merge this struct
     fn extend(&mut self, other: Self) {
         for (role, data) in other.config_properties {
@@ -419,6 +431,8 @@ impl TrinoAuthenticationConfig {
                 .or_default()
                 .extend(data)
         }
+
+        self.hot_reloaded_secrets.extend(other.hot_reloaded_secrets);
     }
 }
 

rust/operator-binary/src/authentication/password/file.rs

@@ -65,10 +65,15 @@ impl FileAuthenticator {
         config_data
     }
 
+    /// Return the name of the Secret providing the usernames and passwords
+    pub fn secret_name(&self) -> String {
+        self.file.user_credentials_secret.name.clone()
+    }
+
     /// Build the volume for the user secret
     pub fn secret_volume(&self) -> Volume {
         VolumeBuilder::new(self.secret_volume_name())
-            .with_secret(&self.file.user_credentials_secret.name, false)
+            .with_secret(self.secret_name(), false)
             .build()
     }
 

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

@@ -119,6 +119,9 @@ impl TrinoPasswordAuthentication {
                         Container::Trino,
                         FileAuthenticator::password_db_volume_mount(),
                     );
+
+                    password_authentication_config
+                        .add_hot_reloaded_secret(file_authenticator.secret_name());
                 }
                 TrinoPasswordAuthenticator::Ldap(ldap_authenticator) => {
                     let config_file_name = ldap_authenticator.config_file_name();

rust/operator-binary/src/controller.rs

@@ -51,7 +51,7 @@ use stackable_operator::{
         core::{DeserializeGuard, error_boundary},
         runtime::{controller::Action, reflector::ObjectRef},
     },
-    kvp::{Annotation, Labels, ObjectLabels},
+    kvp::{Annotation, Annotations, Labels, ObjectLabels},
     logging::controller::ReconcilerError,
     memory::{BinaryMultiple, MemoryQuantity},
     product_config_utils::{
@@ -1381,6 +1381,21 @@ fn build_rolegroup_statefulset(
     pod_template.merge_from(role.config.pod_overrides.clone());
     pod_template.merge_from(rolegroup.config.pod_overrides.clone());
 
+    let ignore_secret_annotations = trino_authentication_config
+        .hot_reloaded_secrets()
+        .iter()
+        .enumerate()
+        .map(|(i, secret_name)| {
+            (
+                format!("restarter.stackable.tech/ignore-secret.{i}"),
+                secret_name,
+            )
+        })
+        .collect::<BTreeMap<_, _>>();
+
+    let annotations =
+        Annotations::try_from(ignore_secret_annotations).context(AnnotationBuildSnafu)?;
+
     Ok(StatefulSet {
         metadata: ObjectMetaBuilder::new()
             .name_and_namespace(trino)
@@ -1395,6 +1410,7 @@ fn build_rolegroup_statefulset(
             ))
             .context(MetadataBuildSnafu)?
             .with_label(RESTART_CONTROLLER_ENABLED_LABEL.to_owned())
+            .with_annotations(annotations)
             .build(),
         spec: Some(StatefulSetSpec {
             pod_management_policy: Some("Parallel".to_string()),

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

@@ -5,4 +5,6 @@ metadata:
   name: create-ldap-user
 commands:
   # We need to replace $NAMESPACE (by KUTTL) in the create-authentication-classes.yaml(.j2)
-  - script: eval "echo \"$(cat create-authentication-classes.yaml)\"" | kubectl apply -f -
+  - script: >
+      envsubst '$NAMESPACE' < create-authentication-classes.yaml |
+      kubectl apply --filename=-

tests/templates/kuttl/authentication/30-hot-reloading-add-user.yaml

@@ -3,4 +3,6 @@ apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
   # We need to replace $NAMESPACE (by KUTTL) in the add_user.yaml(.j2)
-  - script: eval "echo \"$(cat add_user.yaml)\"" | kubectl replace -f -
+  - script: >
+      envsubst '$NAMESPACE' < add_user.yaml |
+      kubectl replace --filename=-

tests/templates/kuttl/authentication/32-hot-reloading-remove-user.yaml

@@ -3,4 +3,6 @@ apiVersion: kuttl.dev/v1beta1
 kind: TestStep
 commands:
   # We need to replace $NAMESPACE (by KUTTL) in the remove_user.yaml(.j2)
-  - script: eval "echo \"$(cat remove_user.yaml)\"" | kubectl replace -f -
+  - script: >
+      envsubst '$NAMESPACE' < remove_user.yaml |
+      kubectl replace --filename=-

tests/templates/kuttl/authentication/33-assert.yaml

@@ -5,4 +5,12 @@ timeout: 600
 commands:
   # We use the check-active-workers script for the login. Since we do want to wait until we cannot log in anymore
   # we flip the return value in the end.
-  - script: kubectl exec -n $NAMESPACE trino-test-helper-0 -- python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -c trino-coordinator-default-headless.$NAMESPACE.svc.cluster.local -w 1; if [ $? -eq 0 ]; then exit 1; fi
+  - script: |
+      set +e
+      kubectl exec -n $NAMESPACE trino-test-helper-0 -- \
+        python /tmp/check-active-workers.py -u hot_reloaded -p hot_reloaded -c trino-coordinator-default-headless.$NAMESPACE.svc.cluster.local -w 1
+      if [ $? -eq 1 ]; then
+        exit 0
+      else
+        exit 1
+      fi