From de21f264577a010c6df310417b05844172827831 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 | 127 +++++++-- internal/webhook/template_webhook_test.go | 241 ++++++++++++++++-- .../provider/hmc/templates/webhooks.yaml | 21 ++ test/objects/template/template.go | 6 + test/objects/templatechain/templatechain.go | 10 +- 9 files changed, 376 insertions(+), 57 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..ff4184c5f 100644 --- a/internal/webhook/template_webhook.go +++ b/internal/webhook/template_webhook.go @@ -18,9 +18,12 @@ import ( "context" "errors" "fmt" + "slices" + "strings" 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 +32,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 +78,18 @@ 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 { + inUseByCluster, err := v.templateIsInUseByCluster(ctx, template) + if err != nil { return nil, err } + if inUseByCluster { + return admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, errTemplateDeletionForbidden + } - if len(managedClusters.Items) > 0 { - return admission.Warnings{"The ClusterTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden + owners := v.getTemplateChainsOwners(template) + if len(owners) > 0 { + return admission.Warnings{fmt.Sprintf("The %s object can't be removed if it is managed by %s: %s", + v.templateKind, v.templateChainKind, strings.Join(owners, ", "))}, errTemplateDeletionForbidden } return nil, nil @@ -87,12 +101,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 +137,18 @@ 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 + inUseByCluster, err := v.templateIsInUseByCluster(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 inUseByCluster { + return admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, errTemplateDeletionForbidden } - if len(managedClusters.Items) > 0 { - return admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden + owners := v.getTemplateChainsOwners(tmpl) + if len(owners) > 0 { + return admission.Warnings{fmt.Sprintf("The %s object can't be removed if it is managed by %s: %s", + v.templateKind, v.templateChainKind, strings.Join(owners, ", "))}, errTemplateDeletionForbidden } // MultiClusterServices can only refer to serviceTemplates in system namespace. @@ -157,11 +174,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 +203,29 @@ 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)) + } + + mgmt := &v1alpha1.Management{} + err := v.Get(ctx, types.NamespacedName{Name: v1alpha1.ManagementName}, mgmt) + if err != nil { + if !apierrors.IsNotFound(err) { + return nil, err + } + } + if slices.Contains(mgmt.Templates(), template.Name) { + return admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is enabled in the Management spec", + template.GetName())}, errTemplateDeletionForbidden + } + + owners := getReleasesOwners(template) + if len(owners) > 0 { + return admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is part of existing Releases: %s", + template.GetName(), strings.Join(owners, ", "))}, errTemplateDeletionForbidden + } return nil, nil } @@ -193,3 +233,48 @@ func (*ProviderTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Ob func (*ProviderTemplateValidator) Default(_ context.Context, _ runtime.Object) error { return 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) getTemplateChainsOwners(template client.Object) []string { + var owners []string + for _, ownerRef := range template.GetOwnerReferences() { + if ownerRef.Kind == v.templateChainKind { + owners = append(owners, ownerRef.Name) + } + } + return owners +} + +func getReleasesOwners(template client.Object) []string { + var owners []string + for _, ownerRef := range template.GetOwnerReferences() { + if ownerRef.Kind == v1alpha1.ReleaseKind { + owners = append(owners, ownerRef.Name) + } + } + return owners +} diff --git a/internal/webhook/template_webhook_test.go b/internal/webhook/template_webhook_test.go index bdc034dd2..a9bcbd81a 100644 --- a/internal/webhook/template_webhook_test.go +++ b/internal/webhook/template_webhook_test.go @@ -16,73 +16,222 @@ package webhook import ( "context" + "fmt" "testing" . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client/fake" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "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" "github.com/Mirantis/hmc/test/scheme" ) +func TestProviderTemplateValidateDelete(t *testing.T) { + ctx := context.Background() + + const ( + templateName = "mytemplate" + ) + tmpl := template.NewProviderTemplate(template.WithName(templateName)) + + 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 defined in the Management spec is removed", + template: tmpl, + existingObjects: []runtime.Object{ + management.NewManagement(management.WithRelease(releaseName), management.WithCoreComponents(&v1alpha1.Core{ + HMC: v1alpha1.Component{ + Template: templateName, + }, + })), + release.New(release.WithName(releaseName)), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is enabled in the Management spec", templateName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should fail if the template is part of one of the existing Releases", + template: template.NewProviderTemplate( + template.WithName(templateName), + template.WithOwnerReference([]metav1.OwnerReference{ + { + Kind: v1alpha1.ReleaseKind, + Name: "hmc-0-0-3", + }, + { + Kind: v1alpha1.ReleaseKind, + Name: "hmc-0-0-4", + }, + }), + ), + existingObjects: []runtime.Object{ + management.NewManagement(management.WithRelease(releaseName)), + release.New(release.WithName("hmc-0-0-3")), + release.New(release.WithName("hmc-0-0-4")), + }, + warnings: admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is part of existing Releases: hmc-0-0-3, hmc-0-0-4", templateName)}, + err: errTemplateDeletionForbidden.Error(), + }, + { + title: "should succeed if the provider is not enabled in Management spec and is not a part of one of the existing Release", + 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)) + + const ( + templateName = "mytemplate" + templateNamespace = "mynamespace" + ) + + tpl := template.NewClusterTemplate(template.WithName(templateName), template.WithNamespace(templateNamespace)) 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), - managedcluster.WithClusterTemplate(tpl.Name), + managedcluster.WithNamespace(templateNamespace), + managedcluster.WithClusterTemplate(templateName), )}, warnings: admission.Warnings{"The ClusterTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, err: "template deletion is forbidden", }, { - name: "should succeed if some ManagedCluster from another namespace references the template", + title: "should fail if the template is owned by one or more ClusterTemplateChains", + template: template.NewClusterTemplate( + template.WithName(templateName), + template.WithNamespace(templateNamespace), + template.WithOwnerReference([]metav1.OwnerReference{ + { + Kind: v1alpha1.ClusterTemplateChainKind, + Name: "test-chain", + }, + }), + ), + existingObjects: []runtime.Object{ + tc.NewClusterTemplateChain( + tc.WithName("test-chain"), + tc.WithNamespace(templateNamespace), + tc.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + { + Name: templateName, + }, + }), + ), + }, + warnings: admission.Warnings{"The ClusterTemplate object can't be removed if it is managed by ClusterTemplateChain: test-chain"}, + 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"), - managedcluster.WithClusterTemplate(tpl.Name), + managedcluster.WithClusterTemplate(templateName), )}, }, { - name: "should be OK because of a different cluster", + title: "should succeed because no ManagedCluster or ClusterTemplateChain references the template", template: tpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, }, - { - name: "should succeed", - template: template.NewClusterTemplate(), - existingObjects: []runtime.Object{managedcluster.NewManagedCluster(managedcluster.WithClusterTemplate(tplTest.Name))}, - }, } 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)) @@ -101,7 +250,12 @@ func TestClusterTemplateValidateDelete(t *testing.T) { func TestServiceTemplateValidateDelete(t *testing.T) { ctx := context.Background() - tmpl := template.NewServiceTemplate(template.WithNamespace("mynamespace"), template.WithName("mytemplate")) + + const ( + templateName = "mytemplate" + templateNamespace = "mynamespace" + ) + tmpl := template.NewServiceTemplate(template.WithNamespace(templateNamespace), template.WithName(templateName)) tests := []struct { title string @@ -115,35 +269,62 @@ func TestServiceTemplateValidateDelete(t *testing.T) { template: tmpl, existingObjects: []runtime.Object{ managedcluster.NewManagedCluster( - managedcluster.WithNamespace(tmpl.Namespace), - managedcluster.WithServiceTemplate(tmpl.Name), + managedcluster.WithNamespace(templateNamespace), + managedcluster.WithServiceTemplate(templateName), ), }, warnings: admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, err: errTemplateDeletionForbidden.Error(), }, + { + title: "should fail if the template is owned by one or more ServiceTemplateChains", + template: template.NewServiceTemplate( + template.WithName(templateName), + template.WithNamespace(templateNamespace), + template.WithOwnerReference([]metav1.OwnerReference{ + { + Kind: v1alpha1.ServiceTemplateChainKind, + Name: "test-chain", + }, + }), + ), + existingObjects: []runtime.Object{ + tc.NewClusterTemplateChain( + tc.WithName("test-chain"), + tc.WithNamespace(templateNamespace), + tc.WithSupportedTemplates( + []v1alpha1.SupportedTemplate{ + { + Name: templateName, + }, + }), + ), + }, + warnings: admission.Warnings{"The ServiceTemplate object can't be removed if it is managed by ServiceTemplateChain: test-chain"}, + err: "template deletion is forbidden", + }, { title: "should succeed if managedCluster referencing ServiceTemplate is another namespace", template: tmpl, existingObjects: []runtime.Object{ managedcluster.NewManagedCluster( managedcluster.WithNamespace("someothernamespace"), - managedcluster.WithServiceTemplate(tmpl.Name), + managedcluster.WithServiceTemplate(templateName), ), }, }, { - title: "should be OK because of a different cluster", + title: "should succeed because no cluster references the template", template: tmpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, }, { title: "should fail if a MultiClusterService is referencing serviceTemplate in system namespace", - template: template.NewServiceTemplate(template.WithNamespace(testSystemNamespace), template.WithName(tmpl.Name)), + template: template.NewServiceTemplate(template.WithNamespace(testSystemNamespace), template.WithName(templateName)), existingObjects: []runtime.Object{ multiclusterservice.NewMultiClusterService( multiclusterservice.WithName("mymulticlusterservice"), - multiclusterservice.WithServiceTemplate(tmpl.Name), + multiclusterservice.WithServiceTemplate(templateName), ), }, warnings: admission.Warnings{"The ServiceTemplate object can't be removed if MultiClusterService objects referencing it still exist"}, @@ -161,8 +342,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/template/template.go b/test/objects/template/template.go index c26a50668..c49dcf31a 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -113,6 +113,12 @@ func WithLabels(labels map[string]string) Opt { } } +func WithOwnerReference(ownerRef []metav1.OwnerReference) Opt { + return func(t Template) { + t.SetOwnerReferences(ownerRef) + } +} + func ManagedByHMC() Opt { return func(template Template) { labels := template.GetLabels() 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, }