docs: Add/improve various (doc) comments

Techassi · Techassi · commit 41f864946214 · 2025-09-23T17:11:49.000+02:00
diff --git a/crates/stackable-operator/src/crd/maintainer.rs b/crates/stackable-operator/src/crd/maintainer.rs
@@ -50,6 +50,33 @@ pub struct CustomResourceDefinitionMaintainer {
 impl CustomResourceDefinitionMaintainer {
     /// Creates and returns a new [`CustomResourceDefinitionMaintainer`] which manages one or more
     /// custom resource definitions.
+    ///
+    /// ## Parameters
+    ///
+    /// This function expects four parameters:
+    ///
+    /// - `client`: A [`Client`] to interact with the Kubernetes API server. It continuously patches
+    ///   the CRDs when the TLS certificate is rotated.
+    /// - `certificate_rx`: A [`mpsc::Receiver`] to receive newly generated TLS certificates. The
+    ///   certificate data sent through the channel is used to set the caBundle in the conversion
+    ///   section of the CRD.
+    /// - `definitions`: An iterator of [`CustomResourceDefinition`]s which should be maintained
+    ///   by this maintainer. If the iterator is empty, the maintainer returns early without doing
+    ///   any work. As such, a polling mechanism which waits for all futures should be used to
+    ///   prevent premature termination of the operator.
+    /// - `options`: Provides [`CustomResourceDefinitionMaintainerOptions`] to customize various
+    ///   parts of the maintainer. In the future, this will be converted to a builder, to enable a
+    ///   cleaner API interface.
+    ///
+    /// ## Return Values
+    ///
+    /// This function returns a 2-tuple (pair) of values:
+    ///
+    /// - The [`CustomResourceDefinitionMaintainer`] itself. This is used to run the maintainer.
+    ///   See [`CustomResourceDefinitionMaintainer::run`] for more details.
+    /// - The [`oneshot::Receiver`] which will be used to send out a message once the initial
+    ///   CRD reconciliation ran. This signal can be used to trigger the deployment of custom
+    ///   resources defined by the maintained CRDs.
     pub fn new(
         client: Client,
         certificate_rx: mpsc::Receiver<Certificate>,
@@ -70,26 +97,36 @@ impl CustomResourceDefinitionMaintainer {
         (maintainer, initial_reconcile_rx)
     }
 
+    /// Runs the [`CustomResourceDefinitionMaintainer`] asynchronously.
+    ///
+    /// This needs to be polled in parallel with other parts of an operator, like controllers or
+    /// webhook servers. If it is disabled, the returned future immediately resolves to
+    /// [`std::task::Poll::Ready`] and thus doesn't consume any resources.
     pub async fn run(mut self) -> Result<(), Error> {
         let CustomResourceDefinitionMaintainerOptions {
             operator_service_name,
             operator_namespace,
             field_manager,
-            https_port,
+            webhook_https_port: https_port,
             disabled,
         } = self.options;
 
-        // If the maintainer is disabled, immediately return without doing any work.
-        if disabled {
+        // If the maintainer is disabled or there are no custom resource definitions, immediately
+        // return without doing any work.
+        if disabled || self.definitions.is_empty() {
             return Ok(());
         }
 
+        // This get's polled by the async runtime on a regular basis (or when woken up). Once we
+        // receive a message containing the newly generated TLS certificate for the conversion
+        // webhook, we need to update the caBundle in the CRD.
         while let Some(certificate) = self.certificate_rx.recv().await {
             tracing::info!(
                 k8s.crd.names = ?self.definitions.iter().map(CustomResourceDefinition::name_any).collect::<Vec<_>>(),
                 "reconciling custom resource definitions"
             );
 
+            // The caBundle needs to be provided as a base64-encoded PEM envelope.
             let ca_bundle = certificate
                 .to_pem(LineEnding::LF)
                 .context(EncodeCertificateAuthorityAsPemSnafu)?;
@@ -109,9 +146,12 @@ impl CustomResourceDefinitionMaintainer {
                 crd.spec.conversion = Some(CustomResourceConversion {
                     strategy: "Webhook".to_owned(),
                     webhook: Some(WebhookConversion {
-                        // conversionReviewVersions indicates what ConversionReview versions are understood/preferred by the webhook.
-                        // The first version in the list understood by the API server is sent to the webhook.
-                        // The webhook must respond with a ConversionReview object in the same version it received.
+                        // conversionReviewVersions indicates what ConversionReview versions are
+                        // supported by the webhook. The first version in the list understood by the
+                        // API server is sent to the webhook. The webhook must respond with a
+                        // ConversionReview object in the same version it received. We only support
+                        // the stable v1 ConversionReview to keep the implementation as simple as
+                        // possible.
                         conversion_review_versions: vec!["v1".to_owned()],
                         client_config: Some(WebhookClientConfig {
                             service: Some(ServiceReference {
@@ -120,12 +160,15 @@ impl CustomResourceDefinitionMaintainer {
                                 path: Some(format!("/convert/{crd_name}")),
                                 port: Some(https_port.into()),
                             }),
+                            // Here, ByteString takes care of encoding the provided content as
+                            // base64.
                             ca_bundle: Some(ByteString(ca_bundle.as_bytes().to_vec())),
                             url: None,
                         }),
                     }),
                 });
 
+                // Deploy the updated CRDs using a server-side apply.
                 let patch = Patch::Apply(&crd);
                 let patch_params = PatchParams::apply(&field_manager);
                 crd_api
@@ -134,12 +177,15 @@ impl CustomResourceDefinitionMaintainer {
                     .with_context(|_| PatchCrdSnafu { crd_name })?;
             }
 
-            // Once all CRDs are reconciled, send a heartbeat for consumers to be notified that
-            // custom resources of these kinds can bow be deployed.
+            // After the reconciliation of the CRDs, the initial reconcile heartbeat is sent out
+            // via the oneshot channel. This channel can only be used exactly once. The sender's
+            // send method consumes self, and as such, the sender is wrapped in an Option to be
+            // able to call take to consume the inner value.
             if let Some(initial_reconcile_tx) = self.initial_reconcile_tx.take() {
-                initial_reconcile_tx
-                    .send(())
-                    .ignore_context(SendInitialReconcileHeartbeatSnafu)?
+                match initial_reconcile_tx.send(()) {
+                    Ok(_) => {}
+                    Err(_) => return SendInitialReconcileHeartbeatSnafu.fail(),
+                }
             }
         }
 
@@ -148,30 +194,20 @@ impl CustomResourceDefinitionMaintainer {
 }
 
 // TODO (@Techassi): Make this a builder instead
+/// This contains required options to customize a [`CustomResourceDefinitionMaintainer`].
 pub struct CustomResourceDefinitionMaintainerOptions {
+    /// The service name used by the operator/conversion webhook.
     operator_service_name: String,
+
+    /// The namespace the operator/conversion webhook runs in.
     operator_namespace: String,
+
+    /// The name of the field manager used for the server-side apply.
     field_manager: String,
-    https_port: u16,
-    disabled: bool,
-}
 
-trait ResultContextExt<T> {
-    fn ignore_context<C, E2>(self, context: C) -> Result<T, E2>
-    where
-        C: snafu::IntoError<E2, Source = snafu::NoneError>,
-        E2: std::error::Error + snafu::ErrorCompat;
-}
+    /// The HTTPS port the conversion webhook listens on.
+    webhook_https_port: u16,
 
-impl<T, E> ResultContextExt<T> for Result<T, E> {
-    fn ignore_context<C, E2>(self, context: C) -> Result<T, E2>
-    where
-        C: snafu::IntoError<E2, Source = snafu::NoneError>,
-        E2: std::error::Error + snafu::ErrorCompat,
-    {
-        match self {
-            Ok(v) => Ok(v),
-            Err(_) => Err(context.into_error(snafu::NoneError)),
-        }
-    }
+    /// Indicates if the maintainer should be disabled.
+    disabled: bool,
 }
diff --git a/crates/stackable-webhook/src/options.rs b/crates/stackable-webhook/src/options.rs
@@ -1,4 +1,5 @@
-//! Contains available options to configure the [WebhookServer][crate::WebhookServer].
+//! Contains available options to configure the [WebhookServer].
+
 use std::{
     net::{IpAddr, SocketAddr},
     path::PathBuf,
diff --git a/crates/stackable-webhook/src/servers/conversion.rs b/crates/stackable-webhook/src/servers/conversion.rs
@@ -74,76 +74,59 @@ impl ConversionWebhookServer {
     /// [`WebhookServer::DEFAULT_SOCKET_ADDRESS`].
     pub const DEFAULT_SOCKET_ADDRESS: SocketAddr = WebhookServer::DEFAULT_SOCKET_ADDRESS;
 
-    /// Creates a new conversion webhook server, which expects POST requests being made to the
-    /// `/convert/{CRD_NAME}` endpoint.
+    /// Creates and returns a new [`ConversionWebhookServer`], which expects POST requests being
+    /// made to the `/convert/{CRD_NAME}` endpoint.
     ///
-    /// You need to provide a few things for every CRD passed in via the `crds_and_handlers` argument:
+    /// ## Parameters
     ///
-    /// 1. The CRD
-    /// 2. A conversion function to convert between CRD versions. Typically you would use the
-    ///    the auto-generated `try_convert` function on CRD spec definition structs for this.
-    /// 3. A [`kube::Client`] used to create/update the CRDs.
+    /// This function expects the following parameters:
     ///
-    /// The [`ConversionWebhookServer`] takes care of reconciling the CRDs into the Kubernetes
-    /// cluster and takes care of adding itself as conversion webhook. This includes TLS
-    /// certificates and CA bundles.
+    /// - `crds_and_handlers`: An iterator over a 2-tuple (pair) mapping a [`CustomResourceDefinition`]
+    ///   to a handler function. In most cases, the generated `CustomResource::try_merge` function
+    ///   should be used. It provides the expected `fn(ConversionReview) -> ConversionReview`
+    ///   signature.
+    /// - `options`: Provides [`ConversionWebhookOptions`] to customize various parts of the
+    ///   webhook server, eg. the socket address used to listen on.
     ///
-    /// # Example
+    /// ## Return Values
     ///
-    /// ```no_run
-    /// use clap::Parser;
-    /// use stackable_webhook::{
-    ///     servers::{ConversionWebhookServer, ConversionWebhookOptions},
-    ///     constants::CONVERSION_WEBHOOK_HTTPS_PORT,
-    ///     WebhookOptions
-    /// };
-    /// use stackable_operator::{
-    ///     kube::Client,
-    ///     crd::s3::{S3Connection, S3ConnectionVersion},
-    ///     cli::{RunArguments, MaintenanceOptions},
-    /// };
+    /// This function returns a [`Result`] which contains a 2-tuple (pair) of values for the [`Ok`]
+    /// variant:
+    ///
+    /// - The [`ConversionWebhookServer`] itself. This is used to run the server. See
+    ///   [`ConversionWebhookServer::run`] for more details.
+    /// - The [`mpsc::Receiver`] which will be used to send out messages containing the newly
+    ///   generated TLS certificate. This channel is used by the CRD maintainer to trigger a
+    ///   reconcile of the CRDs it maintains.
     ///
-    /// # async fn test() {
-    /// // Things that should already be in you operator:
-    /// const OPERATOR_NAME: &str = "product-operator";
-    /// let client = Client::try_default().await.expect("failed to create Kubernetes client");
-    /// let RunArguments {
-    ///     operator_environment,
-    ///     maintenance: MaintenanceOptions {
-    ///         disable_crd_maintenance,
-    ///         ..
-    ///     },
-    ///     ..
-    /// } = RunArguments::parse();
+    /// ## Example
+    ///
+    /// ```
+    /// use stackable_webhook::{ConversionWebhookServer, ConversionWebhookOptions};
+    /// use stackable_operator::crd::s3::{S3Connection, S3ConnectionVersion};
     ///
-    ///  let crds_and_handlers = [
+    /// # #[tokio::test]
+    /// # async fn main() {
+    /// let crds_and_handlers = vec![
     ///     (
     ///         S3Connection::merged_crd(S3ConnectionVersion::V1Alpha1)
-    ///             .expect("failed to merge S3Connection CRD"),
-    ///         S3Connection::try_convert as fn(_) -> _,
-    ///     ),
+    ///             .expect("the S3Connection CRD must be merged"),
+    ///         S3Connection::try_convert,
+    ///     )
     /// ];
     ///
     /// let options = ConversionWebhookOptions {
-    ///     socket_addr: format!("0.0.0.0:{CONVERSION_WEBHOOK_HTTPS_PORT}")
-    ///         .parse()
-    ///         .expect("static address is always valid"),
-    ///     namespace: operator_environment.operator_namespace,
-    ///     service_name: operator_environment.operator_service_name,
-    ///     maintain_crds: !disable_crd_maintenance,
-    ///     field_manager: OPERATOR_NAME.to_owned(),
+    ///     socket_addr: ConversionWebhookServer::DEFAULT_SOCKET_ADDRESS,
+    ///     namespace: "stackable-operators".to_owned(),
+    ///     service_name: "product-operator".to_owned(),
     /// };
     ///
-    /// // Construct the conversion webhook server
-    /// let conversion_webhook = ConversionWebhookServer::new(
-    ///     crds_and_handlers,
-    ///     options,
-    ///     client,
-    /// )
-    /// .await
-    /// .expect("failed to create ConversionWebhookServer");
+    /// let (conversion_webhook_server, _certificate_rx) =
+    ///         ConversionWebhookServer::new(crds_and_handlers, options)
+    ///             .await
+    ///             .unwrap();
     ///
-    /// conversion_webhook.run().await.expect("failed to run ConversionWebhookServer");
+    /// conversion_webhook_server.run().await.unwrap();
     /// # }
     /// ```
     #[instrument(name = "create_conversion_webhook_server", skip(crds_and_handlers))]
