refactor(datastore): using webhooks for secrets instead of finalizers

api/v1alpha1/datastore_secret_webhook.go

@@ -0,0 +1,57 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha1
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/go-logr/logr"
+	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/fields"
+	"k8s.io/apimachinery/pkg/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+//+kubebuilder:webhook:path=/validate--v1-secret,mutating=false,failurePolicy=ignore,sideEffects=None,groups="",resources=secrets,verbs=delete,versions=v1,name=vdatastoresecrets.kb.io,admissionReviewVersions=v1
+
+type dataStoreSecretValidator struct {
+	log    logr.Logger
+	client client.Client
+}
+
+func (d *dataStoreSecretValidator) ValidateCreate(context.Context, runtime.Object) error {
+	return nil
+}
+
+func (d *dataStoreSecretValidator) ValidateUpdate(context.Context, runtime.Object, runtime.Object) error {
+	return nil
+}
+
+func (d *dataStoreSecretValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	secret := obj.(*corev1.Secret) //nolint:forcetypeassert
+
+	dsList := &DataStoreList{}
+
+	if err := d.client.List(ctx, dsList, client.MatchingFieldsSelector{Selector: fields.OneTermEqualSelector(DatastoreUsedSecretNamespacedNameKey, fmt.Sprintf("%s/%s", secret.GetNamespace(), secret.GetName()))}); err != nil {
+		return err
+	}
+
+	if len(dsList.Items) > 0 {
+		var res []string
+
+		for _, ds := range dsList.Items {
+			res = append(res, ds.GetName())
+		}
+
+		return fmt.Errorf("the Secret is used by the following kamajiv1alpha1.DataStores and cannot be deleted (%s)", strings.Join(res, ", "))
+	}
+
+	return nil
+}
+
+func (d *dataStoreSecretValidator) Default(context.Context, runtime.Object) error {
+	return nil
+}

api/v1alpha1/datastore_webhook.go

@@ -10,6 +10,7 @@ import (
 	"github.com/go-logr/logr"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -17,18 +18,27 @@ import (
 )
 
 //+kubebuilder:webhook:path=/mutate-kamaji-clastix-io-v1alpha1-datastore,mutating=true,failurePolicy=fail,sideEffects=None,groups=kamaji.clastix.io,resources=datastores,verbs=create;update,versions=v1alpha1,name=mdatastore.kb.io,admissionReviewVersions=v1
-//+kubebuilder:webhook:path=/validate-kamaji-clastix-io-v1alpha1-datastore,mutating=false,failurePolicy=fail,sideEffects=None,groups=kamaji.clastix.io,resources=datastores,verbs=create;update,versions=v1alpha1,name=vdatastore.kb.io,admissionReviewVersions=v1
+//+kubebuilder:webhook:path=/validate-kamaji-clastix-io-v1alpha1-datastore,mutating=false,failurePolicy=fail,sideEffects=None,groups=kamaji.clastix.io,resources=datastores,verbs=create;update;delete,versions=v1alpha1,name=vdatastore.kb.io,admissionReviewVersions=v1
 
 func (in *DataStore) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	validator := &dataStoreValidator{
+	secretValidator := &dataStoreSecretValidator{
+		log:    mgr.GetLogger().WithName("datastore-secret-webhook"),
+		client: mgr.GetClient(),
+	}
+
+	if err := ctrl.NewWebhookManagedBy(mgr).For(&corev1.Secret{}).WithValidator(secretValidator).Complete(); err != nil {
+		return err
+	}
+
+	dsValidator := &dataStoreValidator{
 		log:    mgr.GetLogger().WithName("datastore-webhook"),
 		client: mgr.GetClient(),
 	}
 
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(in).
-		WithValidator(validator).
-		WithDefaulter(validator).
+		WithValidator(dsValidator).
+		WithDefaulter(dsValidator).
 		Complete()
 }
 
@@ -74,7 +84,22 @@ func (d *dataStoreValidator) ValidateUpdate(ctx context.Context, oldObj, newObj
 	return nil
 }
 
-func (d *dataStoreValidator) ValidateDelete(context.Context, runtime.Object) error {
+func (d *dataStoreValidator) ValidateDelete(ctx context.Context, obj runtime.Object) error {
+	ds, ok := obj.(*DataStore)
+	if !ok {
+		return fmt.Errorf("expected *kamajiv1alpha1.DataStore")
+	}
+
+	tcpList := &TenantControlPlaneList{}
+
+	if err := d.client.List(ctx, tcpList, client.MatchingFieldsSelector{Selector: fields.OneTermEqualSelector(TenantControlPlaneUsedDataStoreKey, ds.GetName())}); err != nil {
+		return err
+	}
+
+	if len(tcpList.Items) > 0 {
+		return fmt.Errorf("the DataStore is used by multiple TenantControlPlanes and cannot be removed")
+	}
+
 	return nil
 }
 

api/v1alpha1/indexer_datastore_usedsecret.go

@@ -0,0 +1,68 @@
+// Copyright 2022 Clastix Labs
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha1
+
+import (
+	"context"
+	"fmt"
+
+	controllerruntime "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+)
+
+const (
+	DatastoreUsedSecretNamespacedNameKey = "secretRef"
+)
+
+type DatastoreUsedSecret struct{}
+
+func (d *DatastoreUsedSecret) SetupWithManager(ctx context.Context, mgr controllerruntime.Manager) error {
+	return mgr.GetFieldIndexer().IndexField(ctx, d.Object(), d.Field(), d.ExtractValue())
+}
+
+func (d *DatastoreUsedSecret) Object() client.Object {
+	return &DataStore{}
+}
+
+func (d *DatastoreUsedSecret) Field() string {
+	return DatastoreUsedSecretNamespacedNameKey
+}
+
+func (d *DatastoreUsedSecret) ExtractValue() client.IndexerFunc {
+	return func(object client.Object) (res []string) {
+		ds := object.(*DataStore) //nolint:forcetypeassert
+
+		if ds.Spec.BasicAuth != nil {
+			if ds.Spec.BasicAuth.Username.SecretRef != nil {
+				res = append(res, d.namespacedName(*ds.Spec.BasicAuth.Username.SecretRef))
+			}
+
+			if ds.Spec.BasicAuth.Password.SecretRef != nil {
+				res = append(res, d.namespacedName(*ds.Spec.BasicAuth.Password.SecretRef))
+			}
+		}
+
+		if ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef != nil {
+			res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.Certificate.SecretRef))
+		}
+
+		if ds.Spec.TLSConfig.CertificateAuthority.PrivateKey != nil && ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef != nil {
+			res = append(res, d.namespacedName(*ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.SecretRef))
+		}
+
+		if ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef != nil {
+			res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.Certificate.SecretRef))
+		}
+
+		if ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef != nil {
+			res = append(res, d.namespacedName(*ds.Spec.TLSConfig.ClientCertificate.PrivateKey.SecretRef))
+		}
+
+		return res
+	}
+}
+
+func (d *DatastoreUsedSecret) namespacedName(ref SecretReference) string {
+	return fmt.Sprintf("%s/%s", ref.Namespace, ref.Name)
+}

api/v1alpha1/indexer_tenantcontrolplane_useddatastore.go

@@ -1,15 +1,13 @@
 // Copyright 2022 Clastix Labs
 // SPDX-License-Identifier: Apache-2.0
 
-package indexers
+package v1alpha1
 
 import (
 	"context"
 
 	controllerruntime "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-
-	kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
 )
 
 const (
@@ -19,7 +17,7 @@ const (
 type TenantControlPlaneStatusDataStore struct{}
 
 func (t *TenantControlPlaneStatusDataStore) Object() client.Object {
-	return &kamajiv1alpha1.TenantControlPlane{}
+	return &TenantControlPlane{}
 }
 
 func (t *TenantControlPlaneStatusDataStore) Field() string {
@@ -28,8 +26,7 @@ func (t *TenantControlPlaneStatusDataStore) Field() string {
 
 func (t *TenantControlPlaneStatusDataStore) ExtractValue() client.IndexerFunc {
 	return func(object client.Object) []string {
-		//nolint:forcetypeassert
-		tcp := object.(*kamajiv1alpha1.TenantControlPlane)
+		tcp := object.(*TenantControlPlane) //nolint:forcetypeassert
 
 		return []string{tcp.Status.Storage.DataStoreName}
 	}

cmd/manager/cmd.go

@@ -22,7 +22,6 @@ import (
 	cmdutils "github.com/clastix/kamaji/cmd/utils"
 	"github.com/clastix/kamaji/controllers"
 	"github.com/clastix/kamaji/controllers/soot"
-	"github.com/clastix/kamaji/indexers"
 	"github.com/clastix/kamaji/internal"
 	datastoreutils "github.com/clastix/kamaji/internal/datastore/utils"
 	"github.com/clastix/kamaji/internal/webhook"
@@ -131,7 +130,13 @@ func NewCmd(scheme *runtime.Scheme) *cobra.Command {
 				return err
 			}
 
-			if err = (&indexers.TenantControlPlaneStatusDataStore{}).SetupWithManager(ctx, mgr); err != nil {
+			if err = (&kamajiv1alpha1.DatastoreUsedSecret{}).SetupWithManager(ctx, mgr); err != nil {
+				setupLog.Error(err, "unable to create indexer", "indexer", "DatastoreUsedSecret")
+
+				return err
+			}
+
+			if err = (&kamajiv1alpha1.TenantControlPlaneStatusDataStore{}).SetupWithManager(ctx, mgr); err != nil {
 				setupLog.Error(err, "unable to create indexer", "indexer", "TenantControlPlaneStatusDataStore")
 
 				return err

controllers/datastore_controller.go

@@ -5,7 +5,6 @@ package controllers
 
 import (
 	"context"
-	"fmt"
 
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/fields"
@@ -15,7 +14,6 @@ import (
 	controllerruntime "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -24,11 +22,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/source"
 
 	kamajiv1alpha1 "github.com/clastix/kamaji/api/v1alpha1"
-	"github.com/clastix/kamaji/indexers"
-)
-
-const (
-	dataStoreFinalizer = "finalizer.kamaji.clastix.io/datastore"
 )
 
 type DataStore struct {
@@ -41,7 +34,6 @@ type DataStore struct {
 
 //+kubebuilder:rbac:groups=kamaji.clastix.io,resources=datastores,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=kamaji.clastix.io,resources=datastores/status,verbs=get;update;patch
-//+kubebuilder:rbac:groups=kamaji.clastix.io,resources=datastores/finalizers,verbs=update
 
 func (r *DataStore) Reconcile(ctx context.Context, request reconcile.Request) (reconcile.Result, error) {
 	log := log.FromContext(ctx)
@@ -56,66 +48,11 @@ func (r *DataStore) Reconcile(ctx context.Context, request reconcile.Request) (r
 
 		return reconcile.Result{}, err
 	}
-	// Managing the finalizer, required to don't drop a DataSource if this is still used by a Tenant Control Plane.
-	switch {
-	case ds.DeletionTimestamp != nil && controllerutil.ContainsFinalizer(ds, dataStoreFinalizer):
-		log.Info("marked for deletion, checking conditions")
-
-		if len(ds.Status.UsedBy) == 0 {
-			log.Info("resource is no more used by any Tenant Control Plane")
-
-			controllerutil.RemoveFinalizer(ds, dataStoreFinalizer)
-
-			return reconcile.Result{}, r.client.Update(ctx, ds)
-		}
-
-		log.Info("DataStore is still used by some Tenant Control Planes, cannot be removed")
-	case ds.DeletionTimestamp == nil && !controllerutil.ContainsFinalizer(ds, dataStoreFinalizer):
-		log.Info("the resource is missing the required finalizer, adding it")
-
-		controllerutil.AddFinalizer(ds, dataStoreFinalizer)
-
-		return reconcile.Result{}, r.client.Update(ctx, ds)
-	}
-	// A Data Source can trigger several Tenant Control Planes and requires a minimum validation:
-	// we have to ensure the data provided by the Data Source is valid and referencing an existing Secret object.
-	if _, err := ds.Spec.TLSConfig.CertificateAuthority.Certificate.GetContent(ctx, r.client); err != nil {
-		log.Error(err, "invalid Certificate Authority data")
-
-		return reconcile.Result{}, err
-	}
-
-	if ds.Spec.Driver == kamajiv1alpha1.EtcdDriver {
-		if ds.Spec.TLSConfig.CertificateAuthority.PrivateKey == nil {
-			err := fmt.Errorf("a valid private key is required for the etcd driver")
-
-			log.Error(err, "missing Certificate Authority private key data")
-
-			return reconcile.Result{}, err
-		}
-		if _, err := ds.Spec.TLSConfig.CertificateAuthority.PrivateKey.GetContent(ctx, r.client); err != nil {
-			log.Error(err, "invalid Certificate Authority private key data")
-
-			return reconcile.Result{}, err
-		}
-	}
-
-	if _, err := ds.Spec.TLSConfig.ClientCertificate.Certificate.GetContent(ctx, r.client); err != nil {
-		log.Error(err, "invalid Client Certificate data")
-
-		return reconcile.Result{}, err
-	}
-
-	if _, err := ds.Spec.TLSConfig.ClientCertificate.PrivateKey.GetContent(ctx, r.client); err != nil {
-		log.Error(err, "invalid Client Certificate private key data")
-
-		return reconcile.Result{}, err
-	}
 
 	tcpList := kamajiv1alpha1.TenantControlPlaneList{}
 
 	if err := r.client.List(ctx, &tcpList, client.MatchingFieldsSelector{
-		Selector: fields.OneTermEqualSelector(indexers.TenantControlPlaneUsedDataStoreKey, ds.GetName()),
+		Selector: fields.OneTermEqualSelector(kamajiv1alpha1.TenantControlPlaneUsedDataStoreKey, ds.GetName()),
 	}); err != nil {
 		log.Error(err, "cannot retrieve list of the Tenant Control Plane using the following instance")