From 3ef2c5facb08ca89677ea582169d40b1114dcc6c Mon Sep 17 00:00:00 2001 From: Teddy Andrieux Date: Tue, 4 Nov 2025 15:38:38 +0000 Subject: [PATCH] chore: Add finalizer to handle CRL Distribution Point cleanup Signed-off-by: Teddy Andrieux --- internal/controller/managedcrl_controller.go | 77 +++++++++++++++++++ .../integration/managedcrl_controller_test.go | 29 +++++++ 2 files changed, 106 insertions(+) diff --git a/internal/controller/managedcrl_controller.go b/internal/controller/managedcrl_controller.go index fa26a65..c91720d 100644 --- a/internal/controller/managedcrl_controller.go +++ b/internal/controller/managedcrl_controller.go @@ -29,6 +29,7 @@ import ( cmv1 "github.com/cert-manager/cert-manager/pkg/apis/certmanager/v1" cmmetav1 "github.com/cert-manager/cert-manager/pkg/apis/meta/v1" + "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" @@ -54,6 +55,9 @@ const ( // secretCRLKey is the key in the Secret data where the CRL is stored. secretCRLKey = "ca.crl" + // finalizerName is the name of the finalizer used to clean up resources. + finalizerName = "crl-operator.scality.com/finalizer" + // Common labels labelManagedByName = "app.kubernetes.io/managed-by" labelManagedByValue = "crl-operator" @@ -120,6 +124,32 @@ func (r *ManagedCRLReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, client.IgnoreNotFound(err) } + if instance.DeletionTimestamp.IsZero() { + // Add finalizer if not present + if !controllerutil.ContainsFinalizer(instance, finalizerName) { + logger.WithValues("finalizer", finalizerName).Info("adding finalizer") + controllerutil.AddFinalizer(instance, finalizerName) + if err := r.Update(ctx, instance); err != nil { + return ctrl.Result{}, err + } + } + } else { + // The object is being deleted + if controllerutil.ContainsFinalizer(instance, finalizerName) { + finLogger := logger.WithValues("finalizer", finalizerName) + if err := r.handleFinalization(finLogger, ctx, instance); err != nil { + return ctrl.Result{}, err + } + + // Remove finalizer + finLogger.Info("removing finalizer") + controllerutil.RemoveFinalizer(instance, finalizerName) + if err := r.Update(ctx, instance); err != nil { + return ctrl.Result{}, err + } + } + return ctrl.Result{}, nil + } // Apply defaults instance.WithDefaults() if err := instance.Validate(); err != nil { @@ -756,6 +786,53 @@ func (r *ManagedCRLReconciler) stdMutate(obj metav1.Object, instance *crloperato return nil } +// handleFinalization handles the deletion of a ManagedCRL resource by cleaning up +// the CRL distribution points from the issuer and removing the finalizer. +func (r *ManagedCRLReconciler) handleFinalization(logger logr.Logger, ctx context.Context, instance *crloperatorv1alpha1.ManagedCRL) error { + logger.Info("handling ManagedCRL deletion") + + issuer, err := r.getIssuer(ctx, instance.Namespace, instance.Spec.IssuerRef) + if err != nil { + if apierrors.IsNotFound(err) { + // Issuer no longer exists, nothing to clean up + logger.Info("issuer not found, skipping cleanup") + } else { + return fmt.Errorf("failed to get issuer: %w", err) + } + } + + var originalIssuer client.Object + switch issuer := issuer.(type) { + case *cmv1.Issuer: + if issuer.Spec.CA == nil || len(issuer.Spec.CA.CRLDistributionPoints) == 0 { + // Nothing to do + break + } + originalIssuer = issuer.DeepCopy() + issuer.Spec.CA.CRLDistributionPoints = nil + case *cmv1.ClusterIssuer: + if issuer.Spec.CA == nil || len(issuer.Spec.CA.CRLDistributionPoints) == 0 { + // Nothing to do + break + } + originalIssuer = issuer.DeepCopy() + issuer.Spec.CA.CRLDistributionPoints = nil + default: + logger.Error(errors.New("unsupported issuer kind for cleaning up CRL distribution points"), "unsupported issuer kind") + // Nothing to do + } + + if originalIssuer != nil { + if err := r.Patch(ctx, issuer, client.MergeFrom(originalIssuer)); err != nil { + return fmt.Errorf("failed to patch issuer: %w", err) + } + logger.Info("removed CRL distribution points from issuer during deletion") + } + + logger.Info("ManagedCRL deletion handled successfully") + return nil +} + // SetupWithManager sets up the controller with the Manager. func (r *ManagedCRLReconciler) SetupWithManager(mgr ctrl.Manager) error { mapIssuerToCRL := func(ctx context.Context, obj client.Object) []ctrl.Request { diff --git a/test/integration/managedcrl_controller_test.go b/test/integration/managedcrl_controller_test.go index 29039a2..8dbb383 100644 --- a/test/integration/managedcrl_controller_test.go +++ b/test/integration/managedcrl_controller_test.go @@ -336,6 +336,33 @@ var _ = Describe("ManagedCRL Controller", func() { k8sClient.Get(ctx, typeNamespacedName, &crloperatorv1alpha1.ManagedCRL{}), ) }, 10*time.Second, time.Second).Should(BeTrue()) + + By("checking the CRL Distribution Points have been removed from the Issuer/ClusterIssuer") + switch tc.spec.IssuerRef.Kind { + case "Issuer": + issuer := &cmv1.Issuer{} + Expect(k8sClient.Get( + ctx, + types.NamespacedName{ + Name: tc.spec.IssuerRef.Name, + Namespace: typeNamespacedName.Namespace, + }, + issuer, + )).To(Succeed()) + Expect(issuer.Spec.CA.CRLDistributionPoints).To(BeEmpty()) + case "ClusterIssuer": + clusterIssuer := &cmv1.ClusterIssuer{} + Expect(k8sClient.Get( + ctx, + types.NamespacedName{ + Name: tc.spec.IssuerRef.Name, + }, + clusterIssuer, + )).To(Succeed()) + Expect(clusterIssuer.Spec.CA.CRLDistributionPoints).To(BeEmpty()) + default: + Fail("unexpected IssuerRef.Kind") + } }) }, toTableEntry(testCases)) }) @@ -390,6 +417,8 @@ func checkSecret(mcrlRef types.NamespacedName) { }, 10*time.Second, time.Second).Should(BeTrue()) retrieved.WithDefaults() + Expect(retrieved.ObjectMeta.Finalizers).To(ContainElement("crl-operator.scality.com/finalizer")) + expectedSecretNs := mcrlRef.Namespace if retrieved.Spec.IssuerRef.Kind == "ClusterIssuer" { expectedSecretNs = certManagerNamespace