diff --git a/docs/deletion.md b/docs/deletion.md deleted file mode 100644 index 8df0b8ce..00000000 --- a/docs/deletion.md +++ /dev/null @@ -1,6 +0,0 @@ -## Deleting a tunnel - -Remember the delete order while deleting the tunnel. -If you delete the secrets before deleting the tunnel, the operator won't be able to clean up the tunnel from Cloudflare, since it no longer has the credentials for the same. - -The correct order is to delete the tunnel, wait for it to actually get deleted (after the finalizer is removed) and then delete the secret. diff --git a/internal/clients/k8s/object.go b/internal/clients/k8s/object.go new file mode 100644 index 00000000..07220b73 --- /dev/null +++ b/internal/clients/k8s/object.go @@ -0,0 +1,88 @@ +// Package k8s provides client abstractions to the kubernetes API +package k8s + +import ( + "context" + "errors" + "fmt" + + "github.com/go-logr/logr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +type ObjectClient struct { + k8sClient client.Client + log *logr.Logger +} + +func NewObjectClient(k8sClient client.Client, log *logr.Logger) (*ObjectClient, error) { + if k8sClient == nil { + return nil, errors.New("k8sClient cannot be nil") + } + if log == nil { + return nil, errors.New("log cannot be nil") + } + return &ObjectClient{ + k8sClient: k8sClient, + log: log, + }, nil +} + +// EnsureFinalizer adds `finalizer` to obj exactly once. +// - It NO-OPs if the finalizer is already there. +// - It uses a strategic-merge Patch so you don’t overwrite concurrent changes. +// - obj must be a pointer that already contains the latest copy from the API server. +func (s *ObjectClient) EnsureFinalizer(ctx context.Context, key client.ObjectKey, obj client.Object, finalizer string) error { + err := s.k8sClient.Get(ctx, key, obj) + if err != nil { + return err + } + // if it already contains the finalizer, there's nothing to do + if controllerutil.ContainsFinalizer(obj, finalizer) { + return nil + } + + //nolint:revive // we know this will serialise, even if the compiler doesn't + base := obj.DeepCopyObject().(client.Object) + controllerutil.AddFinalizer(obj, finalizer) + + s.log. + WithValues("finalizer", finalizer). + WithValues("object", fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())). + WithValues("kind", obj.GetObjectKind().GroupVersionKind().Kind). + Info("creating finalizer") + if err := s.k8sClient.Patch(ctx, obj, client.MergeFrom(base)); err != nil { + return fmt.Errorf("could not add finalizer %q: %w", finalizer, err) + } + return nil +} + +// RemoveFinalizer deletes `finalizer` from obj exactly once. +// - NO-OPs if the finalizer is already gone. +// - Uses a strategic-merge Patch so you never clobber concurrent changes. +// - obj must be a *live* copy fetched from the API server. +func (s *ObjectClient) RemoveFinalizer(ctx context.Context, key client.ObjectKey, obj client.Object, finalizer string) error { + err := s.k8sClient.Get(ctx, key, obj) + if err != nil { + return err + } + // if there is no finalizer, there's nothing to do + if !controllerutil.ContainsFinalizer(obj, finalizer) { + return nil + } + + //nolint:revive // we know this will serialise, even if the compiler doesn't + base := obj.DeepCopyObject().(client.Object) + controllerutil.RemoveFinalizer(obj, finalizer) + + s.log. + WithValues("finalizer", finalizer). + WithValues("object", fmt.Sprintf("%s/%s", obj.GetNamespace(), obj.GetName())). + WithValues("kind", obj.GetObjectKind().GroupVersionKind().Kind). + Info("removing finalizer") + if err := s.k8sClient.Patch(ctx, obj, client.MergeFrom(base)); err != nil { + return fmt.Errorf("could not remove finalizer %q: %w", finalizer, err) + } + return nil +} diff --git a/internal/controller/accesstunnel/controller.go b/internal/controller/accesstunnel/controller.go index c8ff2332..1175d158 100644 --- a/internal/controller/accesstunnel/controller.go +++ b/internal/controller/accesstunnel/controller.go @@ -37,8 +37,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - networkingv1alpha1 "github.com/adyanth/cloudflare-operator/api/v1alpha1" "github.com/go-logr/logr" + + networkingv1alpha1 "github.com/adyanth/cloudflare-operator/api/v1alpha1" ) const CONTAINER_PORT int32 = 8000 diff --git a/internal/controller/adapter.go b/internal/controller/adapter.go index 97e79b45..ea578bd0 100644 --- a/internal/controller/adapter.go +++ b/internal/controller/adapter.go @@ -1,9 +1,10 @@ package controller import ( - networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" + + networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" ) // TunnelAdapter implementation diff --git a/internal/controller/clustertunnel_controller.go b/internal/controller/clustertunnel_controller.go index fed5c74d..56eb11ba 100644 --- a/internal/controller/clustertunnel_controller.go +++ b/internal/controller/clustertunnel_controller.go @@ -20,6 +20,7 @@ import ( "context" "github.com/adyanth/cloudflare-operator/internal/clients/cf" + "github.com/adyanth/cloudflare-operator/internal/clients/k8s" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -30,8 +31,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" "github.com/go-logr/logr" + + networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" ) // ClusterTunnelReconciler reconciles a ClusterTunnel object @@ -145,9 +147,7 @@ func (r *ClusterTunnelReconciler) Reconcile(ctx context.Context, req ctrl.Reques if err := r.Get(ctx, req.NamespacedName, tunnel); err != nil { if apierrors.IsNotFound(err) { // Tunnel object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. - // Return and don't requeue - r.log.Info("Tunnel deleted, nothing to do") + // Owned objects are automatically garbage collected. return ctrl.Result{}, nil } r.log.Error(err, "unable to fetch Tunnel") @@ -158,6 +158,25 @@ func (r *ClusterTunnelReconciler) Reconcile(ctx context.Context, req ctrl.Reques return ctrl.Result{}, err } + objectClient, err := k8s.NewObjectClient(r.Client, &r.log) + if err != nil { + return ctrl.Result{}, err + } + // ensure the secret associated with the tunnel has a finalizer + secret := &corev1.Secret{} + err = objectClient.EnsureFinalizer( + ctx, + client.ObjectKey{ + Namespace: r.GetTunnel().GetNamespace(), + Name: r.GetTunnel().GetSpec().Cloudflare.Secret, + }, + secret, + tunnelFinalizer, + ) + if err != nil { + return ctrl.Result{}, err + } + if res, ok, err := setupTunnel(r); !ok { return res, err } diff --git a/internal/controller/generic_tunnel_reconciler.go b/internal/controller/generic_tunnel_reconciler.go index 67f2beab..6c4b1e2f 100644 --- a/internal/controller/generic_tunnel_reconciler.go +++ b/internal/controller/generic_tunnel_reconciler.go @@ -3,6 +3,7 @@ package controller import ( "errors" "fmt" + "sigs.k8s.io/controller-runtime/pkg/client" "time" "github.com/adyanth/cloudflare-operator/internal/clients/cf" @@ -199,6 +200,27 @@ func cleanupTunnel(r GenericTunnelReconciler) (ctrl.Result, bool, error) { return ctrl.Result{}, false, err } r.GetRecorder().Event(r.GetTunnel().GetObject(), corev1.EventTypeNormal, "FinalizerUnset", "Tunnel Finalizer removed") + + // ensure the secret associated with the tunnel has the finalizer removed, only once the tunnel finalizer can be removed + log := r.GetLog() + objectClient, err := k8s.NewObjectClient(r.GetClient(), &log) + if err != nil { + return ctrl.Result{}, false, err + } + secret := &corev1.Secret{} + err = objectClient.RemoveFinalizer( + r.GetContext(), + client.ObjectKey{ + Namespace: r.GetTunnel().GetNamespace(), + Name: r.GetTunnel().GetSpec().Cloudflare.Secret, + }, + secret, + tunnelFinalizer, + ) + if err != nil { + return ctrl.Result{}, false, err + } + return ctrl.Result{}, true, nil } } diff --git a/internal/controller/tunnel.go b/internal/controller/tunnel.go index 4ee573e6..29ba6756 100644 --- a/internal/controller/tunnel.go +++ b/internal/controller/tunnel.go @@ -1,8 +1,9 @@ package controller import ( - networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" "sigs.k8s.io/controller-runtime/pkg/client" + + networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" ) type Tunnel interface { diff --git a/internal/controller/tunnel_controller.go b/internal/controller/tunnel_controller.go index f1e4325a..8cd27f3e 100644 --- a/internal/controller/tunnel_controller.go +++ b/internal/controller/tunnel_controller.go @@ -20,6 +20,7 @@ import ( "context" "github.com/adyanth/cloudflare-operator/internal/clients/cf" + "github.com/adyanth/cloudflare-operator/internal/clients/k8s" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -28,10 +29,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" - networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" "k8s.io/client-go/tools/record" + + networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" ) // TunnelReconciler reconciles a Tunnel object @@ -140,9 +142,7 @@ func (r *TunnelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if err := r.Get(ctx, req.NamespacedName, tunnel); err != nil { if apierrors.IsNotFound(err) { // Tunnel object not found, could have been deleted after reconcile request. - // Owned objects are automatically garbage collected. For additional cleanup logic use finalizers. - // Return and don't requeue - r.log.Info("Tunnel deleted, nothing to do") + // Owned objects are automatically garbage collected. return ctrl.Result{}, nil } r.log.Error(err, "unable to fetch Tunnel") @@ -153,6 +153,25 @@ func (r *TunnelReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr return ctrl.Result{}, err } + objectClient, err := k8s.NewObjectClient(r.Client, &r.log) + if err != nil { + return ctrl.Result{}, err + } + // ensure the secret associated with the tunnel has a finalizer + secret := &corev1.Secret{} + err = objectClient.EnsureFinalizer( + ctx, + client.ObjectKey{ + Namespace: r.GetTunnel().GetNamespace(), + Name: r.GetTunnel().GetSpec().Cloudflare.Secret, + }, + secret, + tunnelFinalizer, + ) + if err != nil { + return ctrl.Result{}, err + } + if res, ok, err := setupTunnel(r); !ok { return res, err } diff --git a/internal/controller/tunnelbinding_controller.go b/internal/controller/tunnelbinding_controller.go index 82574478..be661366 100644 --- a/internal/controller/tunnelbinding_controller.go +++ b/internal/controller/tunnelbinding_controller.go @@ -26,8 +26,6 @@ import ( "github.com/adyanth/cloudflare-operator/internal/clients/cf" - networkingv1alpha1 "github.com/adyanth/cloudflare-operator/api/v1alpha1" - networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" "github.com/go-logr/logr" yaml "gopkg.in/yaml.v3" corev1 "k8s.io/api/core/v1" @@ -39,6 +37,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ctrllog "sigs.k8s.io/controller-runtime/pkg/log" + networkingv1alpha1 "github.com/adyanth/cloudflare-operator/api/v1alpha1" + networkingv1alpha2 "github.com/adyanth/cloudflare-operator/api/v1alpha2" + appsv1 "k8s.io/api/apps/v1" "k8s.io/client-go/tools/record" )