Improve Templates deletion validation

The following rules are validated: 1. ClusterTemplate or ServiceTemplate can't be removed if it is in use by ManagedCluster or TemplateChain 2. ClusterTemplate or ServiceTemplate can't be removed if the template is managed by the TemplateManagement 3. ProviderTemplate can't be removed if it's a core provider or enabled in Management spec.providers
k0rdent · Dec 3, 2024 · d2c3317 · d2c3317
1 parent 5ddea6f
commit d2c3317
Show file tree

Hide file tree

Showing 8 changed files with 404 additions and 49 deletions.
diff --git a/api/v1alpha1/indexers.go b/api/v1alpha1/indexers.go
@@ -142,14 +142,14 @@ func extractReleaseTemplates(rawObj client.Object) []string {
 const TemplateChainSupportedTemplatesIndexKey = ".spec.supportedTemplates[].Name"
 
 func setupClusterTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error {
-	return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, extractSupportedTemplatesNames)
+	return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, ExtractSupportedTemplatesNamesFromTemplateChain)
 }
 
 func setupServiceTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error {
-	return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, extractSupportedTemplatesNames)
+	return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, ExtractSupportedTemplatesNamesFromTemplateChain)
 }
 
-func extractSupportedTemplatesNames(rawObj client.Object) []string {
+func ExtractSupportedTemplatesNamesFromTemplateChain(rawObj client.Object) []string {
 	chainSpec := TemplateChainSpec{}
 	switch chain := rawObj.(type) {
 	case *ClusterTemplateChain:

diff --git a/cmd/main.go b/cmd/main.go
@@ -352,15 +352,19 @@ func setupWebhooks(mgr ctrl.Manager, currentNamespace string) error {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplateChain")
 		return err
 	}
-	if err := (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil {
+
+	templateValidator := hmcwebhook.TemplateValidator{
+		SystemNamespace: currentNamespace,
+	}
+	if err := (&hmcwebhook.ClusterTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplate")
 		return err
 	}
-	if err := (&hmcwebhook.ServiceTemplateValidator{SystemNamespace: currentNamespace}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&hmcwebhook.ServiceTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplate")
 		return err
 	}
-	if err := (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&hmcwebhook.ProviderTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ProviderTemplate")
 		return err
 	}

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -169,13 +169,17 @@ var _ = BeforeSuite(func() {
 	err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr)
+	templateValidator := hmcwebhook.TemplateValidator{
+		SystemNamespace: testSystemNamespace,
+	}
+
+	err = (&hmcwebhook.ClusterTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ServiceTemplateValidator{SystemNamespace: testSystemNamespace}).SetupWebhookWithManager(mgr)
+	err = (&hmcwebhook.ServiceTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr)
+	err = (&hmcwebhook.ProviderTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
 	go func() {

diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go
@@ -171,7 +171,7 @@ var _ = Describe("Template Controller", func() {
 			err = k8sClient.Get(ctx, typeNamespacedName, providerTemplateResource)
 			Expect(err).NotTo(HaveOccurred())
 
-			By("Cleanup the specific resource instance ClusterTemplate")
+			By("Cleanup the specific resource instance ProviderTemplate")
 			Expect(k8sClient.Delete(ctx, providerTemplateResource)).To(Succeed())
 		})
 

diff --git a/internal/webhook/template_webhook.go b/internal/webhook/template_webhook.go
@@ -18,9 +18,11 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"slices"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
@@ -29,14 +31,23 @@ import (
 	"github.com/Mirantis/hmc/api/v1alpha1"
 )
 
-type ClusterTemplateValidator struct {
+var errTemplateDeletionForbidden = errors.New("template deletion is forbidden")
+
+type TemplateValidator struct {
 	client.Client
+	SystemNamespace   string
+	templateKind      string
+	templateChainKind string
 }
 
-var errTemplateDeletionForbidden = errors.New("template deletion is forbidden")
+type ClusterTemplateValidator struct {
+	TemplateValidator
+}
 
 func (v *ClusterTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	v.Client = mgr.GetClient()
+	v.templateKind = v1alpha1.ClusterTemplateKind
+	v.templateChainKind = v1alpha1.ClusterTemplateChainKind
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ClusterTemplate{}).
 		WithValidator(v).
@@ -66,16 +77,12 @@ func (v *ClusterTemplateValidator) ValidateDelete(ctx context.Context, obj runti
 		return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected ClusterTemplate but got a %T", obj))
 	}
 
-	managedClusters := &v1alpha1.ManagedClusterList{}
-	if err := v.Client.List(ctx, managedClusters,
-		client.InNamespace(template.Namespace),
-		client.MatchingFields{v1alpha1.ManagedClusterTemplateIndexKey: template.Name},
-		client.Limit(1)); err != nil {
-		return nil, err
+	inUse, warnings, err := v.templateIsInUse(ctx, template)
+	if err != nil {
+		return nil, fmt.Errorf("failed to check if the ClusterTemplate %s/%s is in use: %w", template.Namespace, template.Name, err)
 	}
-
-	if len(managedClusters.Items) > 0 {
-		return admission.Warnings{"The ClusterTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden
+	if inUse {
+		return warnings, errTemplateDeletionForbidden
 	}
 
 	return nil, nil
@@ -87,12 +94,13 @@ func (*ClusterTemplateValidator) Default(context.Context, runtime.Object) error
 }
 
 type ServiceTemplateValidator struct {
-	client.Client
-	SystemNamespace string
+	TemplateValidator
 }
 
 func (v *ServiceTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	v.Client = mgr.GetClient()
+	v.templateKind = v1alpha1.ServiceTemplateKind
+	v.templateChainKind = v1alpha1.ServiceTemplateChainKind
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ServiceTemplate{}).
 		WithValidator(v).
@@ -122,16 +130,12 @@ func (v *ServiceTemplateValidator) ValidateDelete(ctx context.Context, obj runti
 		return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected ServiceTemplate but got a %T", obj))
 	}
 
-	managedClusters := &v1alpha1.ManagedClusterList{}
-	if err := v.Client.List(ctx, managedClusters,
-		client.InNamespace(tmpl.Namespace),
-		client.MatchingFields{v1alpha1.ManagedClusterServiceTemplatesIndexKey: tmpl.Name},
-		client.Limit(1)); err != nil {
-		return nil, err
+	inUse, warnings, err := v.templateIsInUse(ctx, tmpl)
+	if err != nil {
+		return nil, fmt.Errorf("failed to check if the ServiceTemplate %s/%s is in use: %w", tmpl.Namespace, tmpl.Name, err)
 	}
-
-	if len(managedClusters.Items) > 0 {
-		return admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden
+	if inUse {
+		return warnings, errTemplateDeletionForbidden
 	}
 
 	// MultiClusterServices can only refer to serviceTemplates in system namespace.
@@ -157,11 +161,12 @@ func (*ServiceTemplateValidator) Default(_ context.Context, _ runtime.Object) er
 }
 
 type ProviderTemplateValidator struct {
-	client.Client
+	TemplateValidator
 }
 
 func (v *ProviderTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
 	v.Client = mgr.GetClient()
+	v.templateKind = v1alpha1.ProviderTemplateKind
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ProviderTemplate{}).
 		WithValidator(v).
@@ -185,11 +190,134 @@ func (*ProviderTemplateValidator) ValidateUpdate(_ context.Context, _, _ runtime
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (*ProviderTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+func (v *ProviderTemplateValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	template, ok := obj.(*v1alpha1.ProviderTemplate)
+	if !ok {
+		return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected ProviderTemplate but got a %T", obj))
+	}
+
+	inUse, err := v.providerTemplateIsInUse(ctx, template.Name)
+	if err != nil {
+		return nil, fmt.Errorf("failed to check if the ProviderTemplate %s is in use: %w", template.Name, err)
+	}
+	if inUse {
+		return admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it's in use", template.GetName())}, errTemplateDeletionForbidden
+	}
 	return nil, nil
 }
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (*ProviderTemplateValidator) Default(_ context.Context, _ runtime.Object) error {
 	return nil
 }
+
+func (v TemplateValidator) templateIsInUse(ctx context.Context, template client.Object) (bool, admission.Warnings, error) {
+	inUseByCluster, err := v.templateIsInUseByCluster(ctx, template)
+	if err != nil {
+		return false, nil, err
+	}
+	if inUseByCluster {
+		return true, admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, nil
+	}
+	inUseByChain, err := v.templateIsInUseByTemplateChain(ctx, template)
+	if err != nil {
+		return false, nil, err
+	}
+	if inUseByChain {
+		return true, admission.Warnings{fmt.Sprintf("The %s object can't be removed if %s object referencing it exists", v.templateKind, v.templateChainKind)}, nil
+	}
+	return false, nil, nil
+}
+
+func (v TemplateValidator) templateIsInUseByCluster(ctx context.Context, template client.Object) (bool, error) {
+	var key string
+
+	switch v.templateKind {
+	case v1alpha1.ClusterTemplateKind:
+		key = v1alpha1.ManagedClusterTemplateIndexKey
+	case v1alpha1.ServiceTemplateKind:
+		key = v1alpha1.ManagedClusterServiceTemplatesIndexKey
+	default:
+		return false, fmt.Errorf("invalid Template kind %s. Supported values are: %s and %s", v.templateKind, v1alpha1.ClusterTemplateKind, v1alpha1.ServiceTemplateKind)
+	}
+
+	managedClusters := &v1alpha1.ManagedClusterList{}
+	if err := v.Client.List(ctx, managedClusters,
+		client.InNamespace(template.GetNamespace()),
+		client.MatchingFields{key: template.GetName()},
+		client.Limit(1)); err != nil {
+		return false, err
+	}
+	if len(managedClusters.Items) > 0 {
+		return true, nil
+	}
+	return false, nil
+}
+
+func (v TemplateValidator) templateIsInUseByTemplateChain(ctx context.Context, template client.Object) (bool, error) {
+	listOpts := []client.ListOption{
+		client.InNamespace(template.GetNamespace()),
+		client.MatchingFields{v1alpha1.TemplateChainSupportedTemplatesIndexKey: template.GetName()},
+		client.Limit(1),
+	}
+	if v.templateChainKind == v1alpha1.ClusterTemplateChainKind {
+		chainList := &v1alpha1.ClusterTemplateChainList{}
+		if err := v.Client.List(ctx, chainList, listOpts...); err != nil {
+			return false, err
+		}
+		if len(chainList.Items) > 0 {
+			return true, nil
+		}
+	}
+	if v.templateChainKind == v1alpha1.ServiceTemplateChainKind {
+		chainList := &v1alpha1.ServiceTemplateChainList{}
+		if err := v.Client.List(ctx, chainList, listOpts...); err != nil {
+			return false, err
+		}
+		if len(chainList.Items) > 0 {
+			return true, nil
+		}
+	}
+	return false, nil
+}
+
+func (v TemplateValidator) providerTemplateIsInUse(ctx context.Context, templateName string) (bool, error) {
+	mgmt := &v1alpha1.Management{}
+	err := v.Get(ctx, types.NamespacedName{Name: v1alpha1.ManagementName}, mgmt)
+	if err != nil {
+		if !apierrors.IsNotFound(err) {
+			return false, err
+		}
+		return false, nil
+	}
+
+	release := &v1alpha1.Release{}
+	err = v.Get(ctx, types.NamespacedName{Name: mgmt.Spec.Release}, release)
+	if err != nil {
+		return false, err
+	}
+
+	selectTemplate := func(primary, fallback string) string {
+		if primary != "" {
+			return primary
+		}
+		return fallback
+	}
+
+	templatesInUse := make([]string, 0, len(mgmt.Spec.Providers)+2)
+	if core := mgmt.Spec.Core; core != nil {
+		templatesInUse = append(templatesInUse,
+			selectTemplate(core.HMC.Template, release.Spec.HMC.Template),
+			selectTemplate(core.CAPI.Template, release.Spec.CAPI.Template),
+		)
+	} else {
+		templatesInUse = append(templatesInUse, release.Spec.HMC.Template, release.Spec.CAPI.Template)
+	}
+	for _, provider := range mgmt.Spec.Providers {
+		templatesInUse = append(templatesInUse, selectTemplate(provider.Template, release.ProviderTemplate(provider.Name)))
+	}
+	if slices.Contains(templatesInUse, templateName) {
+		return true, nil
+	}
+	return false, nil
+}