Merge branch 'main' into chore/pedantic-clippy-lints

Author: sbernauer

crates/stackable-operator/CHANGELOG.md

@@ -6,9 +6,20 @@ All notable changes to this project will be documented in this file.
 
 ### Added
 
-- Implement `Deref` for `kvp::Key` to be more ergonomic to use ([#1182]).
+- Git sync: add support for CAs ([#1154]).
 - Add support for specifying a `clientAuthenticationMethod` for OIDC ([#1178]).
   This was originally done in [#1158] and had been reverted in [#1170].
+- Implement `Deref` for `kvp::Key` to be more ergonomic to use ([#1182]).
+
+### Changed
+
+- 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.
 
 ### Changed
 
@@ -25,6 +36,8 @@ All notable changes to this project will be documented in this file.
 - BREAKING: Remove unused `add_prefix`, `try_add_prefix`, `set_name`, and `try_set_name` associated
   functions from `kvp::Key` to disallow mutable access to inner values ([#1182]).
 
+[#1154]: https://github.com/stackabletech/operator-rs/pull/1154
+[#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
 [#1186]: https://github.com/stackabletech/operator-rs/pull/1186

crates/stackable-operator/crds/DummyCluster.yaml

@@ -152,6 +152,56 @@ spec:
                     description: 'The git repository URL that will be cloned, for example: `https://github.com/stackabletech/airflow-operator` or `ssh://git@github.com:stackable-airflow/dags.git`.'
                     format: uri
                     type: string
+                  tls:
+                    default:
+                      verification:
+                        server:
+                          caCert:
+                            webPki: {}
+                    description: Configure a TLS connection. If not specified it will default to webPki validation.
+                    nullable: true
+                    properties:
+                      verification:
+                        description: The verification method used to verify the certificates of the server and/or the client.
+                        oneOf:
+                        - required:
+                          - none
+                        - required:
+                          - server
+                        properties:
+                          none:
+                            description: Use TLS but don't verify certificates.
+                            type: object
+                          server:
+                            description: Use TLS and a CA certificate to verify the server.
+                            properties:
+                              caCert:
+                                description: CA cert to verify the server.
+                                oneOf:
+                                - required:
+                                  - webPki
+                                - required:
+                                  - secretClass
+                                properties:
+                                  secretClass:
+                                    description: |-
+                                      Name of the [SecretClass](https://docs.stackable.tech/home/nightly/secret-operator/secretclass) which will provide the CA certificate.
+                                      Note that a SecretClass does not need to have a key but can also work with just a CA certificate,
+                                      so if you got provided with a CA cert but don't have access to the key you can still use this method.
+                                    type: string
+                                  webPki:
+                                    description: |-
+                                      Use TLS and the CA certificates trusted by the common web browsers to verify the server.
+                                      This can be useful when you e.g. use public AWS S3 or other public available services.
+                                    type: object
+                                type: object
+                            required:
+                            - caCert
+                            type: object
+                        type: object
+                    required:
+                    - verification
+                    type: object
                   wait:
                     default: 20s
                     description: |-

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},
 };
 
@@ -279,17 +280,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,
         }
     }
 
@@ -338,8 +351,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,
 };
 
@@ -26,6 +28,7 @@ pub enum TlsClientDetailsError {
     },
 }
 
+#[repr(transparent)]
 #[derive(
     Clone, Debug, Deserialize, Eq, Hash, JsonSchema, Ord, PartialEq, PartialOrd, Serialize,
 )]
@@ -35,6 +38,40 @@ pub struct TlsClientDetails {
     pub tls: Option<Tls>,
 }
 
+#[repr(transparent)]
+#[derive(
+    Clone, Debug, Deserialize, Eq, Hash, JsonSchema, Ord, PartialEq, PartialOrd, Serialize,
+)]
+#[serde(rename_all = "camelCase")]
+pub struct TlsClientDetailsWithSecureDefaults {
+    /// Configure a TLS connection. If not specified it will default to webPki validation.
+    #[serde(default = "default_web_pki_tls")]
+    pub tls: Option<Tls>,
+}
+
+impl std::ops::Deref for TlsClientDetailsWithSecureDefaults {
+    type Target = TlsClientDetails;
+
+    fn deref(&self) -> &TlsClientDetails {
+        // SAFETY: both types are `#[repr(transparent)]` over `Option<Tls>`, so they share
+        // the same memory layout and this cast is sound.
+        //
+        // This cannot silently break due to struct changes: `#[repr(transparent)]` requires
+        // exactly one non-zero-sized field, so adding a second real field to either struct
+        // is a compile error. The only scenario that would NOT be caught at compile time is
+        // deliberately removing `#[repr(transparent)]` from one of the two structs.
+        unsafe { &*(self as *const Self as *const TlsClientDetails) }
+    }
+}
+
+fn default_web_pki_tls() -> Option<Tls> {
+    Some(Tls {
+        verification: TlsVerification::Server(TlsServerVerification {
+            ca_cert: CaCert::WebPki {},
+        }),
+    })
+}
+
 impl TlsClientDetails {
     /// This functions adds
     ///
@@ -72,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);
@@ -164,3 +202,51 @@ pub enum CaCert {
     /// so if you got provided with a CA cert but don't have access to the key you can still use this method.
     SecretClass(String),
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::utils::yaml_from_str_singleton_map;
+
+    #[test]
+    fn tls_client_details_with_secure_defaults_deserialization() {
+        // No tls key at all → WebPki default kicks in
+        let parsed: TlsClientDetailsWithSecureDefaults =
+            yaml_from_str_singleton_map("{}").expect("failed to deserialize empty input");
+        assert_eq!(parsed.tls, default_web_pki_tls());
+
+        // Explicit null → opt out of TLS entirely
+        let parsed: TlsClientDetailsWithSecureDefaults =
+            yaml_from_str_singleton_map("tls: null").expect("failed to deserialize tls: null");
+        assert_eq!(parsed.tls, None);
+
+        // Explicit SecretClass value is preserved as-is
+        let parsed: TlsClientDetailsWithSecureDefaults = yaml_from_str_singleton_map(
+            "tls:
+               verification:
+                 server:
+                   caCert:
+                     secretClass: my-ca",
+        )
+        .expect("failed to deserialize secretClass");
+        assert_eq!(
+            parsed.tls,
+            Some(Tls {
+                verification: TlsVerification::Server(TlsServerVerification {
+                    ca_cert: CaCert::SecretClass("my-ca".to_owned()),
+                }),
+            })
+        );
+    }
+
+    #[test]
+    #[allow(clippy::explicit_auto_deref)]
+    fn tls_client_details_with_secure_defaults_deref() {
+        let secure: TlsClientDetailsWithSecureDefaults =
+            yaml_from_str_singleton_map("{}").expect("failed to deserialize");
+
+        // Deref must not panic and must expose the same tls value
+        let tls_client_details: &TlsClientDetails = &*secure;
+        assert_eq!(tls_client_details.tls, secure.tls);
+    }
+}

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()
             ]
         );