Merge remote-tracking branch 'origin/main' into spike/generic-databases

Author: sbernauer

crates/stackable-operator/CHANGELOG.md

@@ -14,6 +14,13 @@ All notable changes to this project will be documented in this file.
 ### Changed
 
 - Implement `Deref` for `kvp::Key` to be more ergonomic to use ([#1182]).
+- BREAKING: Add mandatory `provision_parts` argument to `SecretOperatorVolumeSourceBuilder::new` ([#1165]).
+  It now forces the caller to make an explicit choice if the public parts are sufficient or if private
+  (e.g. a certificate for the Pod) parts are needed as well. This is done to avoid accidentally requesting
+  too much parts. For details see [this issue](https://github.com/stackabletech/issues/issues/547).
+
+  Additionally, `SecretClassVolume::to_volume` and `SecretClassVolume::to_ephemeral_volume_source`
+  also take the same new argument.
 - BREAKING: Change signature of `ContainerBuilder::add_env_vars` from `Vec<EnvVar>` to `IntoIterator<Item = EnvVar>` ([#1163]).
 
 ### Removed
@@ -23,6 +30,7 @@ All notable changes to this project will be documented in this file.
 
 [#1154]: https://github.com/stackabletech/operator-rs/pull/1154
 [#1163]: https://github.com/stackabletech/operator-rs/pull/1163
+[#1165]: https://github.com/stackabletech/operator-rs/pull/1165
 [#1178]: https://github.com/stackabletech/operator-rs/pull/1178
 [#1182]: https://github.com/stackabletech/operator-rs/pull/1182
 

crates/stackable-operator/src/builder/pod/volume.rs

@@ -14,6 +14,7 @@ use tracing::warn;
 
 use crate::{
     builder::meta::ObjectMetaBuilder,
+    commons::secret_class::SecretClassVolumeProvisionParts,
     kvp::{Annotation, AnnotationError, Annotations, LabelError, Labels},
 };
 
@@ -281,17 +282,29 @@ pub struct SecretOperatorVolumeSourceBuilder {
     kerberos_service_names: Vec<String>,
     tls_pkcs12_password: Option<String>,
     auto_tls_cert_lifetime: Option<Duration>,
+    provision_parts: SecretClassVolumeProvisionParts,
 }
 
 impl SecretOperatorVolumeSourceBuilder {
-    pub fn new(secret_class: impl Into<String>) -> Self {
+    /// Creates a builder for a secret-operator volume that uses the specified SecretClass to
+    /// request the specified [`SecretClassVolumeProvisionParts`].
+    ///
+    /// This function forces the caller to make an explicit choice if the public parts are
+    /// sufficient or if private (e.g. a certificate for the Pod) parts are needed as well.
+    /// This is done to avoid accidentally requesting too much parts. For details see
+    /// [this issue](https://github.com/stackabletech/issues/issues/547).
+    pub fn new(
+        secret_class: impl Into<String>,
+        provision_parts: SecretClassVolumeProvisionParts,
+    ) -> Self {
         Self {
             secret_class: secret_class.into(),
             scopes: Vec::new(),
             format: None,
             kerberos_service_names: Vec::new(),
             tls_pkcs12_password: None,
             auto_tls_cert_lifetime: None,
+            provision_parts,
         }
     }
 
@@ -340,8 +353,10 @@ impl SecretOperatorVolumeSourceBuilder {
     pub fn build(&self) -> Result<EphemeralVolumeSource, SecretOperatorVolumeSourceBuilderError> {
         let mut annotations = Annotations::new();
 
+        #[rustfmt::skip]
         annotations
-            .insert(Annotation::secret_class(&self.secret_class).context(ParseAnnotationSnafu)?);
+            .insert(Annotation::secret_class(&self.secret_class).context(ParseAnnotationSnafu)?)
+            .insert(Annotation::secret_provision_parts(&self.provision_parts).context(ParseAnnotationSnafu)?);
 
         if !self.scopes.is_empty() {
             annotations

crates/stackable-operator/src/commons/secret_class.rs

@@ -38,9 +38,10 @@ impl SecretClassVolume {
 
     pub fn to_ephemeral_volume_source(
         &self,
+        provision_parts: SecretClassVolumeProvisionParts,
     ) -> Result<EphemeralVolumeSource, SecretClassVolumeError> {
         let mut secret_operator_volume_builder =
-            SecretOperatorVolumeSourceBuilder::new(&self.secret_class);
+            SecretOperatorVolumeSourceBuilder::new(&self.secret_class, provision_parts);
 
         if let Some(scope) = &self.scope {
             if scope.pod {
@@ -62,8 +63,12 @@ impl SecretClassVolume {
             .context(SecretOperatorVolumeSnafu)
     }
 
-    pub fn to_volume(&self, volume_name: &str) -> Result<Volume, SecretClassVolumeError> {
-        let ephemeral = self.to_ephemeral_volume_source()?;
+    pub fn to_volume(
+        &self,
+        volume_name: &str,
+        provision_parts: SecretClassVolumeProvisionParts,
+    ) -> Result<Volume, SecretClassVolumeError> {
+        let ephemeral = self.to_ephemeral_volume_source(provision_parts)?;
         Ok(VolumeBuilder::new(volume_name).ephemeral(ephemeral).build())
     }
 }
@@ -94,14 +99,33 @@ pub struct SecretClassVolumeScope {
     pub listener_volumes: Vec<String>,
 }
 
+/// What parts of secret material should be provisioned into the requested volume.
+//
+// There intentionally isn't a global [`Default`] impl, as it is secret-operator's concern what it
+// chooses as a default.
+// TODO (@Techassi): This to me is a HUGE indicator this lives in the wrong place. All these secret
+// volume builders/helpers should be defined as part of a secret-operator library to be as close as
+// possible to secret-operator, which is the authoritative source of truth for all of this.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, strum::AsRefStr)]
+#[strum(serialize_all = "kebab-case")]
+pub enum SecretClassVolumeProvisionParts {
+    /// Only provision public parts, such as the CA certificate (either as PEM or truststore) or
+    /// `krb5.conf`.
+    Public,
+
+    /// Provision all parts, which includes all [`Public`](Self::Public) ones as well as additional
+    /// private parts, such as a TLS cert + private key, a keystore or a keytab.
+    PublicPrivate,
+}
+
 #[cfg(test)]
 mod tests {
     use std::collections::BTreeMap;
 
     use super::*;
 
     #[test]
-    fn volume_to_csi_volume_source() {
+    fn volume_to_ephemeral_volume_source() {
         let secret_class_volume_source = SecretClassVolume {
             secret_class: "myclass".to_string(), // pragma: allowlist secret
             scope: Some(SecretClassVolumeScope {
@@ -111,7 +135,8 @@ mod tests {
                 listener_volumes: vec!["mylistener".to_string()],
             }),
         }
-        .to_ephemeral_volume_source()
+        // Let's assume we need some form of private data (e.g. a certificate or S3 credentials)
+        .to_ephemeral_volume_source(SecretClassVolumeProvisionParts::PublicPrivate)
         .unwrap();
 
         let expected_volume_attributes = BTreeMap::from([
@@ -123,6 +148,10 @@ mod tests {
                 "secrets.stackable.tech/scope".to_string(),
                 "pod,service=myservice,listener-volume=mylistener".to_string(),
             ),
+            (
+                "secrets.stackable.tech/provision-parts".to_string(),
+                "public-private".to_string(),
+            ),
         ]);
 
         assert_eq!(

crates/stackable-operator/src/commons/tls_verification.rs

@@ -8,7 +8,9 @@ use crate::{
         self,
         pod::{PodBuilder, container::ContainerBuilder, volume::VolumeMountBuilder},
     },
-    commons::secret_class::{SecretClassVolume, SecretClassVolumeError},
+    commons::secret_class::{
+        SecretClassVolume, SecretClassVolumeError, SecretClassVolumeProvisionParts,
+    },
     constants::secret::SECRET_BASE_PATH,
 };
 
@@ -107,7 +109,8 @@ impl TlsClientDetails {
             let volume_name = format!("{secret_class}-ca-cert");
             let secret_class_volume = SecretClassVolume::new(secret_class.clone(), None);
             let volume = secret_class_volume
-                .to_volume(&volume_name)
+                // We only need the public CA cert
+                .to_volume(&volume_name, SecretClassVolumeProvisionParts::Public)
                 .context(SecretClassVolumeSnafu)?;
 
             volumes.push(volume);

crates/stackable-operator/src/crd/authentication/ldap/v1alpha1_impl.rs

@@ -7,7 +7,10 @@ use crate::{
         self,
         pod::{PodBuilder, container::ContainerBuilder, volume::VolumeMountBuilder},
     },
-    commons::{secret_class::SecretClassVolumeError, tls_verification::TlsClientDetailsError},
+    commons::{
+        secret_class::{SecretClassVolumeError, SecretClassVolumeProvisionParts},
+        tls_verification::TlsClientDetailsError,
+    },
     constants::secret::SECRET_BASE_PATH,
     crd::authentication::ldap::v1alpha1::{AuthenticationProvider, FieldNames},
 };
@@ -94,7 +97,8 @@ impl AuthenticationProvider {
             let secret_class = &bind_credentials.secret_class;
             let volume_name = format!("{secret_class}-bind-credentials");
             let volume = bind_credentials
-                .to_volume(&volume_name)
+                // We need the private LDAP bind credentials
+                .to_volume(&volume_name, SecretClassVolumeProvisionParts::PublicPrivate)
                 .context(BindCredentialsSnafu)?;
 
             volumes.push(volume);
@@ -234,7 +238,10 @@ mod tests {
                     secret_class: "ldap-ca-cert".to_string(),
                     scope: None,
                 }
-                .to_volume("ldap-ca-cert-ca-cert")
+                .to_volume(
+                    "ldap-ca-cert-ca-cert",
+                    SecretClassVolumeProvisionParts::Public
+                )
                 .unwrap()
             ]
         );
@@ -263,13 +270,19 @@ mod tests {
                     secret_class: "openldap-bind-credentials".to_string(),
                     scope: None,
                 }
-                .to_volume("openldap-bind-credentials-bind-credentials")
+                .to_volume(
+                    "openldap-bind-credentials-bind-credentials",
+                    SecretClassVolumeProvisionParts::PublicPrivate
+                )
                 .unwrap(),
                 SecretClassVolume {
                     secret_class: "ldap-ca-cert".to_string(),
                     scope: None,
                 }
-                .to_volume("ldap-ca-cert-ca-cert")
+                .to_volume(
+                    "ldap-ca-cert-ca-cert",
+                    SecretClassVolumeProvisionParts::Public
+                )
                 .unwrap()
             ]
         );

crates/stackable-operator/src/crd/git_sync/v1alpha2_impl.rs

@@ -15,7 +15,7 @@ use crate::{
     commons::{
         self,
         product_image_selection::ResolvedProductImage,
-        secret_class::SecretClassVolume,
+        secret_class::{SecretClassVolume, SecretClassVolumeProvisionParts},
         tls_verification::{CaCert, TlsServerVerification, TlsVerification},
     },
     crd::git_sync::v1alpha2::{Credentials, GitSync},
@@ -276,7 +276,8 @@ impl GitSyncResources {
                 let secret_class_volume = SecretClassVolume::new(secret_class.clone(), None);
                 let volume_name = format!("{CA_CERT_VOLUME_NAME_PREFIX}-{i}");
                 let ca_cert_secret_volume = secret_class_volume
-                    .to_volume(&volume_name)
+                    // We only need the public CA cert
+                    .to_volume(&volume_name, SecretClassVolumeProvisionParts::Public)
                     .context(SecretClassVolumeSnafu)?;
                 resources.git_ca_cert_volumes.push(ca_cert_secret_volume);
             }
@@ -1390,6 +1391,7 @@ name: content-from-git-0
     metadata:
       annotations:
         secrets.stackable.tech/class: git-tls-ca
+        secrets.stackable.tech/provision-parts: public
     spec:
       accessModes:
       - ReadWriteOnce

crates/stackable-operator/src/crd/s3/connection/mod.rs

@@ -174,10 +174,16 @@ mod tests {
                 .unwrap()
                 .annotations
                 .unwrap(),
-            &BTreeMap::from([(
-                "secrets.stackable.tech/class".to_string(),
-                "ionos-s3-credentials".to_string()
-            )]),
+            &BTreeMap::from([
+                (
+                    "secrets.stackable.tech/class".to_string(),
+                    "ionos-s3-credentials".to_string()
+                ),
+                (
+                    "secrets.stackable.tech/provision-parts".to_string(),
+                    "public-private".to_string()
+                )
+            ]),
         );
 
         assert_eq!(mount.name, volume.name);

crates/stackable-operator/src/crd/s3/connection/v1alpha1_impl.rs

@@ -5,7 +5,10 @@ use url::Url;
 use crate::{
     builder::pod::{PodBuilder, container::ContainerBuilder, volume::VolumeMountBuilder},
     client::Client,
-    commons::{secret_class::SecretClassVolumeError, tls_verification::TlsClientDetailsError},
+    commons::{
+        secret_class::{SecretClassVolumeError, SecretClassVolumeProvisionParts},
+        tls_verification::TlsClientDetailsError,
+    },
     constants::secret::SECRET_BASE_PATH,
     crd::s3::{
         connection::ResolvedConnection,
@@ -110,7 +113,8 @@ impl ConnectionSpec {
 
             volumes.push(
                 credentials
-                    .to_volume(&volume_name)
+                    // We need the private S3 credentials
+                    .to_volume(&volume_name, SecretClassVolumeProvisionParts::PublicPrivate)
                     .context(AddS3CredentialVolumesSnafu)?,
             );
             mounts.push(

crates/stackable-operator/src/kvp/annotation/mod.rs

@@ -19,6 +19,7 @@ use delegate::delegate;
 
 use crate::{
     builder::pod::volume::SecretOperatorVolumeScope,
+    commons::secret_class::SecretClassVolumeProvisionParts,
     iter::TryFromIterator,
     kvp::{Key, KeyValuePair, KeyValuePairError, KeyValuePairs, KeyValuePairsError},
 };
@@ -80,6 +81,15 @@ impl Annotation {
         self.0
     }
 
+    /// Constructs a `secrets.stackable.tech/provision-parts` annotation.
+    pub fn secret_provision_parts(
+        provision_parts: &SecretClassVolumeProvisionParts,
+    ) -> Result<Self, AnnotationError> {
+        let kvp =
+            KeyValuePair::try_from(("secrets.stackable.tech/provision-parts", provision_parts))?;
+        Ok(Self(kvp))
+    }
+
     /// Constructs a `secrets.stackable.tech/class` annotation.
     pub fn secret_class(secret_class: &str) -> Result<Self, AnnotationError> {
         let kvp = KeyValuePair::try_from(("secrets.stackable.tech/class", secret_class))?;

crates/stackable-versioned-macros/src/attrs/module.rs

@@ -95,7 +95,11 @@ pub struct ModuleSkipArguments {
 
 /// This struct contains crate overrides to be passed to `#[kube]`.
 #[derive(Clone, Debug, FromMeta)]
+#[darling(and_then = "CrateArguments::dynamic_default")]
 pub struct CrateArguments {
+    #[darling(default = default_kube)]
+    pub kube: Override<Path>,
+
     #[darling(default = default_kube_core)]
     pub kube_core: Override<Path>,
 
@@ -124,6 +128,7 @@ pub struct CrateArguments {
 impl Default for CrateArguments {
     fn default() -> Self {
         Self {
+            kube: default_kube(),
             kube_core: default_kube_core(),
             kube_client: default_kube_client(),
             k8s_openapi: default_k8s_openapi(),
@@ -136,6 +141,30 @@ impl Default for CrateArguments {
     }
 }
 
+impl CrateArguments {
+    fn dynamic_default(mut self) -> Result<Self> {
+        // Adjust the kube_core and kube_client paths automatically if only the kube path is
+        // overridden.
+        if let Override::Explicit(kube_path) = &self.kube {
+            if self.kube_core.is_explicit() || self.kube_client.is_explicit() {
+                return Err(
+                    Error::custom("the `kube` crate override is mutually exclusive with `kube_core` and `kube_client`")
+                        .with_span(&kube_path)
+                );
+            }
+
+            self.kube_core = Override::Explicit(parse_quote! { #kube_path::core });
+            self.kube_client = Override::Explicit(parse_quote! { #kube_path::client });
+        }
+
+        Ok(self)
+    }
+}
+
+fn default_kube() -> Override<Path> {
+    Override::Default(parse_quote! { ::kube })
+}
+
 fn default_kube_core() -> Override<Path> {
     Override::Default(parse_quote! { ::kube::core })
 }
@@ -262,3 +291,12 @@ impl<T> Deref for Override<T> {
         }
     }
 }
+
+impl<T> Override<T> {
+    pub fn is_explicit(&self) -> bool {
+        match self {
+            Override::Default(_) => false,
+            Override::Explicit(_) => true,
+        }
+    }
+}