From 7ed3c444014b6057cdbbde5692dca98c8580c6a3 Mon Sep 17 00:00:00 2001 From: Dario Tranchitella Date: Tue, 20 Dec 2022 20:28:54 +0100 Subject: [PATCH] refactor(datastore): using webhooks for secrets instead of finalizers --- api/v1alpha1/datastore_secret_webhook.go | 57 ++++++++++++++++ api/v1alpha1/datastore_webhook.go | 35 ++++++++-- api/v1alpha1/indexer_datastore_usedsecret.go | 68 +++++++++++++++++++ ...ndexer_tenantcontrolplane_useddatastore.go | 9 +-- cmd/manager/cmd.go | 9 ++- controllers/datastore_controller.go | 65 +----------------- 6 files changed, 166 insertions(+), 77 deletions(-) create mode 100644 api/v1alpha1/datastore_secret_webhook.go create mode 100644 api/v1alpha1/indexer_datastore_usedsecret.go rename indexers/tcp_useddatastore.go => api/v1alpha1/indexer_tenantcontrolplane_useddatastore.go (81%) diff --git a/api/v1alpha1/datastore_secret_webhook.go b/api/v1alpha1/datastore_secret_webhook.go new file mode 100644 index 0000000..da05919 --- /dev/null +++ b/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 +} diff --git a/api/v1alpha1/datastore_webhook.go b/api/v1alpha1/datastore_webhook.go index 1622dc7..835da52 100644 --- a/api/v1alpha1/datastore_webhook.go +++ b/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 } diff --git a/api/v1alpha1/indexer_datastore_usedsecret.go b/api/v1alpha1/indexer_datastore_usedsecret.go new file mode 100644 index 0000000..1e481f1 --- /dev/null +++ b/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) +} diff --git a/indexers/tcp_useddatastore.go b/api/v1alpha1/indexer_tenantcontrolplane_useddatastore.go similarity index 81% rename from indexers/tcp_useddatastore.go rename to api/v1alpha1/indexer_tenantcontrolplane_useddatastore.go index 73a26ac..916931e 100644 --- a/indexers/tcp_useddatastore.go +++ b/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} } diff --git a/cmd/manager/cmd.go b/cmd/manager/cmd.go index 2c7a40d..057c035 100644 --- a/cmd/manager/cmd.go +++ b/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 diff --git a/controllers/datastore_controller.go b/controllers/datastore_controller.go index 2079da8..603f246 100644 --- a/controllers/datastore_controller.go +++ b/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")