From d967b7d8ad0179aeefc599e99cb3ca25526ebcdb Mon Sep 17 00:00:00 2001 From: Ekaterina Kazakova Date: Thu, 19 Sep 2024 12:22:14 +0400 Subject: [PATCH] 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 --- api/v1alpha1/indexers.go | 6 +- cmd/main.go | 10 +- internal/controller/suite_test.go | 10 +- .../controller/template_controller_test.go | 2 +- internal/webhook/template_webhook.go | 184 ++++++++++++-- internal/webhook/template_webhook_test.go | 232 ++++++++++++++++-- .../provider/hmc/templates/webhooks.yaml | 21 ++ test/objects/templatechain/templatechain.go | 10 +- 8 files changed, 428 insertions(+), 47 deletions(-) diff --git a/api/v1alpha1/indexers.go b/api/v1alpha1/indexers.go index 684dfce46..a77557d20 100644 --- 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 index 0feb222fb..10e58e9e0 100644 --- 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 index 16e694103..166921d3a 100644 --- 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 index ceddb6201..7db52ba75 100644 --- 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 index e6dfe9e47..a396556c2 100644 --- 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,16 @@ 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 + if templateManagedByHMC(template) { + return admission.Warnings{"The Template is managed by the TemplateManagement and " + v.templateChainKind}, errTemplateDeletionForbidden } - if len(managedClusters.Items) > 0 { - return admission.Warnings{"The ClusterTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden + 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 inUse { + return warnings, errTemplateDeletionForbidden } return nil, nil @@ -87,12 +98,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 +134,16 @@ 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 + if templateManagedByHMC(tmpl) { + return admission.Warnings{"The Template is managed by the TemplateManagement and " + v.templateChainKind}, errTemplateDeletionForbidden } - if len(managedClusters.Items) > 0 { - return admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden + 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 inUse { + return warnings, errTemplateDeletionForbidden } // MultiClusterServices can only refer to serviceTemplates in system namespace. @@ -157,11 +169,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,7 +198,19 @@ 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 } @@ -193,3 +218,118 @@ func (*ProviderTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Ob 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 +} + +func templateManagedByHMC(template client.Object) bool { + return template.GetLabels()[v1alpha1.HMCManagedLabelKey] == v1alpha1.HMCManagedLabelValue +} diff --git a/internal/webhook/template_webhook_test.go b/internal/webhook/template_webhook_test.go index bdc034dd2..0d4fe402e 100644 --- a/internal/webhook/template_webhook_test.go +++ b/internal/webhook/template_webhook_test.go @@ -16,6 +16,7 @@ package webhook import ( "context" + "fmt" "testing" . "github.com/onsi/gomega" @@ -25,26 +26,165 @@ import ( "github.com/Mirantis/hmc/api/v1alpha1" "github.com/Mirantis/hmc/test/objects/managedcluster" + "github.com/Mirantis/hmc/test/objects/management" "github.com/Mirantis/hmc/test/objects/multiclusterservice" + "github.com/Mirantis/hmc/test/objects/release" "github.com/Mirantis/hmc/test/objects/template" + tc "github.com/Mirantis/hmc/test/objects/templatechain" + tm "github.com/Mirantis/hmc/test/objects/templatemanagement" "github.com/Mirantis/hmc/test/scheme" ) +const ( + namespace = "test-ns" + tmName = "test-tm" +) + +func TestProviderTemplateValidateDelete(t *testing.T) { + ctx := context.Background() + + providerName := "myprovider" + tmplName := "mytemplate" + tmpl := template.NewProviderTemplate(template.WithName(tmplName)) + + releaseName := "hmc-0-0-3" + + tests := []struct { + title string + template *v1alpha1.ProviderTemplate + existingObjects []runtime.Object + warnings admission.Warnings + err string + }{ + { + title: "should fail if the core ProviderTemplate is removed. Template name is defined in the Management spec", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement(management.WithRelease(releaseName), management.WithCoreComponents(&v1alpha1.Core{ + HMC: v1alpha1.Component{ + Template: tmplName, + }, + })), + release.New(release.WithName(releaseName)), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it's in use", tmplName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should fail if the core ProviderTemplate is removed. Template name is defined in the Release object", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement(management.WithRelease(releaseName), management.WithCoreComponents(&v1alpha1.Core{HMC: v1alpha1.Component{}})), + release.New(release.WithName(releaseName), release.WithHMCTemplateName(tmplName)), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it's in use", tmplName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should fail if the provider is enabled in Management spec. Template name is defined in Management spec", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithRelease(releaseName), + management.WithCoreComponents(&v1alpha1.Core{}), + management.WithProviders(v1alpha1.Provider{ + Name: "myprovider", + Component: v1alpha1.Component{ + Template: tmplName, + }, + }, + ), + ), + release.New(release.WithName(releaseName)), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it's in use", tmplName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should fail if the provider is enabled in Management spec. Template name is defined in the Release object", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithRelease(releaseName), + management.WithCoreComponents(&v1alpha1.Core{}), + management.WithProviders(v1alpha1.Provider{Name: providerName}), + ), + release.New(release.WithName(releaseName), + release.WithProviders(v1alpha1.NamedProviderTemplate{Name: providerName, CoreProviderTemplate: v1alpha1.CoreProviderTemplate{Template: tmplName}}), + ), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it's in use", tmplName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should succeed if the provider is not enabled in Management spec", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithRelease(releaseName), + management.WithCoreComponents(&v1alpha1.Core{}), + management.WithProviders(v1alpha1.Provider{ + Name: "cluster-api-provider-aws", + Component: v1alpha1.Component{ + Template: "cluster-api-provider-aws-0-0-2", + }, + }, + )), + release.New(release.WithName(releaseName)), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.title, func(t *testing.T) { + g := NewWithT(t) + + c := fake. + NewClientBuilder(). + WithScheme(scheme.Scheme). + WithRuntimeObjects(tt.existingObjects...). + WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.ManagedClusterServiceTemplatesIndexKey, v1alpha1.ExtractServiceTemplateNamesFromManagedCluster). + WithIndex(&v1alpha1.ServiceTemplateChain{}, v1alpha1.TemplateChainSupportedTemplatesIndexKey, v1alpha1.ExtractSupportedTemplatesNamesFromTemplateChain). + Build() + + validator := &ProviderTemplateValidator{ + TemplateValidator{ + Client: c, + SystemNamespace: testSystemNamespace, + templateKind: v1alpha1.ProviderTemplateKind, + }, + } + + warn, err := validator.ValidateDelete(ctx, tt.template) + if tt.err != "" { + g.Expect(err).To(MatchError(tt.err)) + } else { + g.Expect(err).To(Succeed()) + } + + if len(tt.warnings) > 0 { + g.Expect(warn).To(Equal(tt.warnings)) + } else { + g.Expect(warn).To(BeEmpty()) + } + }) + } +} + func TestClusterTemplateValidateDelete(t *testing.T) { ctx := context.Background() - namespace := "test" + tpl := template.NewClusterTemplate(template.WithName("testTemplateFail"), template.WithNamespace(namespace)) - tplTest := template.NewClusterTemplate(template.WithName("testTemplate"), template.WithNamespace(namespace)) tests := []struct { - name string + title string template *v1alpha1.ClusterTemplate existingObjects []runtime.Object err string warnings admission.Warnings }{ { - name: "should fail if ManagedCluster objects exist in the same namespace", + title: "should fail if ManagedCluster object referencing the template exists in the same namespace", template: tpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster( managedcluster.WithNamespace(namespace), @@ -54,7 +194,27 @@ func TestClusterTemplateValidateDelete(t *testing.T) { err: "template deletion is forbidden", }, { - name: "should succeed if some ManagedCluster from another namespace references the template", + title: "should fail if one or more ClusterTemplateChain object references the template", + template: tpl, + existingObjects: []runtime.Object{tc.NewClusterTemplateChain(tc.WithNamespace(tpl.Namespace), tc.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + { + Name: tpl.Name, + }, + }), + )}, + warnings: admission.Warnings{"The ClusterTemplate object can't be removed if ClusterTemplateChain object referencing it exists"}, + err: "template deletion is forbidden", + }, + { + title: "should fail if the template is managed by HMC", + template: template.NewClusterTemplate(template.ManagedByHMC()), + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, + warnings: admission.Warnings{"The Template is managed by the TemplateManagement and ClusterTemplateChain"}, + err: "template deletion is forbidden", + }, + { + title: "should succeed if some ManagedCluster from another namespace references the template with the same name", template: tpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster( managedcluster.WithNamespace("new"), @@ -62,27 +222,36 @@ func TestClusterTemplateValidateDelete(t *testing.T) { )}, }, { - name: "should be OK because of a different cluster", + title: "should succeed if the template is not managed by HMC", template: tpl, - existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, }, { - name: "should succeed", - template: template.NewClusterTemplate(), - existingObjects: []runtime.Object{managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(tplTest.Name))}, + title: "should succeed because no ManagedCluster or ClusterTemplateChain references the template", + template: tpl, + existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, }, } for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + t.Run(tt.title, func(t *testing.T) { g := NewWithT(t) c := fake.NewClientBuilder(). WithScheme(scheme.Scheme). WithRuntimeObjects(tt.existingObjects...). WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.ManagedClusterTemplateIndexKey, v1alpha1.ExtractTemplateNameFromManagedCluster). + WithIndex(&v1alpha1.ClusterTemplateChain{}, v1alpha1.TemplateChainSupportedTemplatesIndexKey, v1alpha1.ExtractSupportedTemplatesNamesFromTemplateChain). Build() - validator := &ClusterTemplateValidator{Client: c} + validator := &ClusterTemplateValidator{ + TemplateValidator: TemplateValidator{ + Client: c, + SystemNamespace: testSystemNamespace, + templateKind: v1alpha1.ClusterTemplateKind, + templateChainKind: v1alpha1.ClusterTemplateChainKind, + }, + } + warn, err := validator.ValidateDelete(ctx, tt.template) if tt.err != "" { g.Expect(err).To(MatchError(tt.err)) @@ -122,6 +291,26 @@ func TestServiceTemplateValidateDelete(t *testing.T) { warnings: admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, err: errTemplateDeletionForbidden.Error(), }, + { + title: "should fail if one or more ServiceTemplateChain object references the template", + template: tmpl, + existingObjects: []runtime.Object{tc.NewServiceTemplateChain(tc.WithNamespace(tmpl.Namespace), tc.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + { + Name: tmpl.Name, + }, + }), + )}, + warnings: admission.Warnings{"The ServiceTemplate object can't be removed if ServiceTemplateChain object referencing it exists"}, + err: "template deletion is forbidden", + }, + { + title: "should fail if the template is managed by HMC", + template: template.NewServiceTemplate(template.ManagedByHMC()), + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, + warnings: admission.Warnings{"The Template is managed by the TemplateManagement and ServiceTemplateChain"}, + err: "template deletion is forbidden", + }, { title: "should succeed if managedCluster referencing ServiceTemplate is another namespace", template: tmpl, @@ -133,7 +322,12 @@ func TestServiceTemplateValidateDelete(t *testing.T) { }, }, { - title: "should be OK because of a different cluster", + title: "should succeed if the template is not managed by HMC", + template: tmpl, + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, + }, + { + title: "should succeed because no cluster references the template", template: tmpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, }, @@ -161,8 +355,18 @@ func TestServiceTemplateValidateDelete(t *testing.T) { WithRuntimeObjects(tt.existingObjects...). WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.ManagedClusterServiceTemplatesIndexKey, v1alpha1.ExtractServiceTemplateNamesFromManagedCluster). WithIndex(&v1alpha1.MultiClusterService{}, v1alpha1.MultiClusterServiceTemplatesIndexKey, v1alpha1.ExtractServiceTemplateNamesFromMultiClusterService). + WithIndex(&v1alpha1.ServiceTemplateChain{}, v1alpha1.TemplateChainSupportedTemplatesIndexKey, v1alpha1.ExtractSupportedTemplatesNamesFromTemplateChain). Build() - validator := &ServiceTemplateValidator{Client: c, SystemNamespace: testSystemNamespace} + + validator := &ServiceTemplateValidator{ + TemplateValidator{ + Client: c, + SystemNamespace: testSystemNamespace, + templateKind: v1alpha1.ServiceTemplateKind, + templateChainKind: v1alpha1.ServiceTemplateChainKind, + }, + } + warn, err := validator.ValidateDelete(ctx, tt.template) if tt.err != "" { g.Expect(err).To(MatchError(tt.err)) diff --git a/templates/provider/hmc/templates/webhooks.yaml b/templates/provider/hmc/templates/webhooks.yaml index 851fcf8ae..2e4f8b712 100644 --- a/templates/provider/hmc/templates/webhooks.yaml +++ b/templates/provider/hmc/templates/webhooks.yaml @@ -168,6 +168,27 @@ webhooks: resources: - servicetemplates sideEffects: None + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: {{ include "hmc.webhook.serviceName" . }} + namespace: {{ include "hmc.webhook.serviceNamespace" . }} + path: /validate-hmc-mirantis-com-v1alpha1-providertemplate + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.providertemplate.hmc.mirantis.com + rules: + - apiGroups: + - hmc.mirantis.com + apiVersions: + - v1alpha1 + operations: + - DELETE + resources: + - providertemplates + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/test/objects/templatechain/templatechain.go b/test/objects/templatechain/templatechain.go index ac3296008..4876e865b 100644 --- a/test/objects/templatechain/templatechain.go +++ b/test/objects/templatechain/templatechain.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package templatemanagement +package templatechain import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -34,6 +34,10 @@ type Opt func(tc *TemplateChain) func NewClusterTemplateChain(opts ...Opt) *v1alpha1.ClusterTemplateChain { tc := NewTemplateChain(opts...) return &v1alpha1.ClusterTemplateChain{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ClusterTemplateChainKind, + }, ObjectMeta: tc.ObjectMeta, Spec: tc.Spec, } @@ -42,6 +46,10 @@ func NewClusterTemplateChain(opts ...Opt) *v1alpha1.ClusterTemplateChain { func NewServiceTemplateChain(opts ...Opt) *v1alpha1.ServiceTemplateChain { tc := NewTemplateChain(opts...) return &v1alpha1.ServiceTemplateChain{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ServiceTemplateChainKind, + }, ObjectMeta: tc.ObjectMeta, Spec: tc.Spec, }