From 220b9851c2e49d92407d59c9d651e06c371dd511 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 2. ClusterTemplate or ServiceTemplate can't be removed if the request was triggered by user and 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 +- api/v1alpha1/management_types.go | 29 ++ cmd/main.go | 10 +- go.mod | 2 +- internal/controller/suite_test.go | 25 +- .../controller/template_controller_test.go | 2 +- .../templatechain_controller_test.go | 4 +- internal/webhook/template_webhook.go | 185 ++++++++++-- internal/webhook/template_webhook_test.go | 282 +++++++++++++++++- internal/webhook/userinfo.go | 32 ++ .../provider/hmc/templates/deployment.yaml | 2 + .../provider/hmc/templates/webhooks.yaml | 21 ++ test/objects/release/release.go | 6 + test/objects/templatechain/templatechain.go | 10 +- 14 files changed, 562 insertions(+), 54 deletions(-) create mode 100644 internal/webhook/userinfo.go diff --git a/api/v1alpha1/indexers.go b/api/v1alpha1/indexers.go index 0229fcecb..27a38f352 100644 --- a/api/v1alpha1/indexers.go +++ b/api/v1alpha1/indexers.go @@ -122,14 +122,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/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go index 40844a3ed..d15a750a2 100644 --- a/api/v1alpha1/management_types.go +++ b/api/v1alpha1/management_types.go @@ -75,6 +75,35 @@ func (in *Component) HelmValues() (values map[string]any, err error) { return values, err } +// Templates returns the templates for all enabled components. +// If a template is not specified in Management, it retrieves the default from the Release object. +func (in *Management) Templates(release *Release) []string { + templates := make([]string, 0, len(in.Spec.Providers)+2) + if core := in.Spec.Core; core != nil { + if core.HMC.Template != "" { + templates = append(templates, core.HMC.Template) + } else { + templates = append(templates, release.Spec.HMC.Template) + } + if core.CAPI.Template != "" { + templates = append(templates, core.CAPI.Template) + } else { + templates = append(templates, release.Spec.CAPI.Template) + } + } else { + templates = append(templates, release.Spec.HMC.Template, release.Spec.CAPI.Template) + } + + for _, p := range in.Spec.Providers { + if p.Template != "" { + templates = append(templates, p.Template) + } else { + templates = append(templates, release.ProviderTemplate(p.Name)) + } + } + return templates +} + func GetDefaultProviders() []Provider { return []Provider{ {Name: ProviderK0smotronName}, diff --git a/cmd/main.go b/cmd/main.go index 5ee887fb9..9fce5b36f 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/go.mod b/go.mod index e913ba3bb..f9543e033 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( k8s.io/api v0.31.2 k8s.io/apiextensions-apiserver v0.31.2 k8s.io/apimachinery v0.31.2 + k8s.io/apiserver v0.31.2 k8s.io/client-go v0.31.2 k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 sigs.k8s.io/cluster-api v1.8.5 @@ -186,7 +187,6 @@ require ( gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect - k8s.io/apiserver v0.31.2 // indirect k8s.io/cli-runtime v0.31.2 // indirect k8s.io/cluster-bootstrap v0.31.1 // indirect k8s.io/component-base v0.31.2 // indirect diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index e805f260c..cb41e3595 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -32,6 +32,7 @@ import ( . "github.com/onsi/gomega" sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" admissionv1 "k8s.io/api/admissionregistration/v1" + authenticationv1 "k8s.io/api/authentication/v1" "k8s.io/client-go/dynamic" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/rest" @@ -43,6 +44,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" hmcwebhook "github.com/Mirantis/hmc/internal/webhook" @@ -65,6 +67,10 @@ var ( testEnv *envtest.Environment ctx context.Context cancel context.CancelFunc + + hmcServiceAccountName = "hmc-controller-manager" + + userInfo = authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:%s:%s", testSystemNamespace, hmcServiceAccountName)} ) func TestControllers(t *testing.T) { @@ -85,6 +91,9 @@ var _ = BeforeSuite(func() { ) Expect(err).NotTo(HaveOccurred()) + err = os.Setenv(hmcwebhook.ServiceAccountEnvName, hmcServiceAccountName) + Expect(err).To(Succeed()) + testEnv = &envtest.Environment{ CRDDirectoryPaths: []string{ filepath.Join("..", "..", "templates", "provider", "hmc", "templates", "crds"), @@ -167,13 +176,20 @@ var _ = BeforeSuite(func() { err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr) + templateValidator := hmcwebhook.TemplateValidator{ + SystemNamespace: testSystemNamespace, + InjectUserInfo: func(req *admission.Request) { + req.UserInfo = userInfo + }, + } + + 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() { @@ -199,6 +215,9 @@ var _ = AfterSuite(func() { cancel() err := testEnv.Stop() Expect(err).NotTo(HaveOccurred()) + + err = os.Unsetenv(hmcwebhook.ServiceAccountEnvName) + Expect(err).To(Succeed()) }) func loadWebhooks(path string) ([]*admissionv1.ValidatingWebhookConfiguration, []*admissionv1.MutatingWebhookConfiguration, error) { 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/controller/templatechain_controller_test.go b/internal/controller/templatechain_controller_test.go index 4457644d4..5cd32da4f 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -231,7 +231,7 @@ var _ = Describe("Template Chain Controller", func() { Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, clusterTemplateChainResource).Should(HaveOccurred()) } - for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct2"]} { + for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct1"], ctTemplates["ct2"]} { Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed()) } @@ -251,7 +251,7 @@ var _ = Describe("Template Chain Controller", func() { Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, serviceTemplateChainResource).Should(HaveOccurred()) } - for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"], stTemplates["st2"]} { + for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"], stTemplates["st1"], stTemplates["st2"]} { Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed()) } diff --git a/internal/webhook/template_webhook.go b/internal/webhook/template_webhook.go index e6dfe9e47..4a87f2ef3 100644 --- a/internal/webhook/template_webhook.go +++ b/internal/webhook/template_webhook.go @@ -18,9 +18,12 @@ import ( "context" "errors" "fmt" + "os" + "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 +32,24 @@ import ( "github.com/Mirantis/hmc/api/v1alpha1" ) -type ClusterTemplateValidator struct { +var errTemplateDeletionForbidden = errors.New("template deletion is forbidden") + +type TemplateValidator struct { client.Client + InjectUserInfo func(*admission.Request) + 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 +79,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 + deletionAllowed, warnings, err := v.isTemplateDeletionAllowed(ctx, template) + if err != nil { + return nil, fmt.Errorf("failed to check if the ClusterTemplate %s/%s is allowed to be deleted: %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 !deletionAllowed { + return warnings, errTemplateDeletionForbidden } return nil, nil @@ -87,12 +96,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 +132,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 + deletionAllowed, warnings, err := v.isTemplateDeletionAllowed(ctx, tmpl) + if err != nil { + return nil, fmt.Errorf("failed to check if the ServiceTemplate %s/%s is allowed to be deleted: %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 !deletionAllowed { + return warnings, errTemplateDeletionForbidden } // MultiClusterServices can only refer to serviceTemplates in system namespace. @@ -157,11 +163,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 +192,18 @@ 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)) + } + deletionAllowed, warnings, err := v.isTemplateDeletionAllowed(ctx, template) + if err != nil { + return nil, fmt.Errorf("failed to check if the ProviderTemplate %s is allowed to be deleted: %w", template.Name, err) + } + if !deletionAllowed { + return warnings, errTemplateDeletionForbidden + } return nil, nil } @@ -193,3 +211,122 @@ func (*ProviderTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Ob func (*ProviderTemplateValidator) Default(_ context.Context, _ runtime.Object) error { return nil } + +func (v TemplateValidator) isTemplateDeletionAllowed(ctx context.Context, template client.Object) (bool, admission.Warnings, error) { + req, err := admission.RequestFromContext(ctx) + if err != nil { + return false, nil, err + } + if v.InjectUserInfo != nil { + v.InjectUserInfo(&req) + } + + triggeredByController := false + if serviceAccountIsEqual(req, v.SystemNamespace, os.Getenv(ServiceAccountEnvName)) { + triggeredByController = true + } + + switch v.templateKind { + case v1alpha1.ProviderTemplateKind: + if triggeredByController { + return true, nil, nil + } + mgmt := &v1alpha1.Management{} + err := v.Get(ctx, types.NamespacedName{Name: v1alpha1.ManagementName}, mgmt) + if err != nil { + if apierrors.IsNotFound(err) { + return true, nil, nil + } + return false, nil, err + } + release := &v1alpha1.Release{} + err = v.Get(ctx, types.NamespacedName{Name: mgmt.Spec.Release}, release) + if err != nil { + return false, nil, err + } + + if slices.Contains(mgmt.Templates(release), template.GetName()) { + return false, admission.Warnings{fmt.Sprintf("The ProviderTemplate %s can't be removed while it is enabled in the Management spec", template.GetName())}, nil + } + return true, nil, nil + case v1alpha1.ClusterTemplateKind, v1alpha1.ServiceTemplateKind: + // Forbid template deletion if the template is managed by the TemplateManagement + if !triggeredByController && templateManagedByHMC(template) { + return false, admission.Warnings{"The Template is managed by the TemplateManagement and " + v.templateChainKind}, nil + } + + inUseByCluster, err := v.templateInUseByCluster(ctx, template) + if err != nil { + return false, nil, err + } + if inUseByCluster { + return false, admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, nil + } + inUseByChain, err := v.templateInUseByTemplateChain(ctx, template) + if err != nil { + return false, nil, err + } + if inUseByChain && !triggeredByController { + return false, admission.Warnings{fmt.Sprintf("The %s object can't be removed if %s object referencing it exists", v.templateKind, v.templateChainKind)}, nil + } + default: + return false, nil, fmt.Errorf("invalid Template kind. Supported values are: %s, %s and %s", v1alpha1.ProviderTemplateKind, v1alpha1.ClusterTemplateKind, v1alpha1.ServiceTemplateKind) + } + return true, nil, nil +} + +func (v TemplateValidator) templateInUseByCluster(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) templateInUseByTemplateChain(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 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..a8faa4355 100644 --- a/internal/webhook/template_webhook_test.go +++ b/internal/webhook/template_webhook_test.go @@ -16,35 +16,194 @@ package webhook import ( "context" + "fmt" "testing" . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" + authenticationv1 "k8s.io/api/authentication/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" + tm "github.com/Mirantis/hmc/test/objects/templatemanagement" "github.com/Mirantis/hmc/test/scheme" ) +const ( + namespace = "test-ns" + tmName = "test-tm" + hmcServiceAccountName = "hmc-controller-manager" +) + +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 + userInfo authenticationv1.UserInfo + 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 can't be removed while it is enabled in the Management spec", 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 can't be removed while it is enabled in the Management spec", 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 can't be removed while it is enabled in the Management spec", 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 can't be removed while it is enabled in the Management spec", 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, + }, + } + + t.Setenv(ServiceAccountEnvName, hmcServiceAccountName) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: tt.userInfo, + }, + } + warn, err := validator.ValidateDelete(admission.NewContextWithRequest(ctx, req), 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 + userInfo authenticationv1.UserInfo 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 +213,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 and the user triggered the deletion", + 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", template: tpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster( managedcluster.WithNamespace("new"), @@ -62,28 +241,50 @@ func TestClusterTemplateValidateDelete(t *testing.T) { )}, }, { - name: "should be OK because of a different cluster", + title: "should succeed if the template is managed by HMC and the controller triggered the deletion", + template: template.NewClusterTemplate(template.ManagedByHMC()), + userInfo: authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:%s:%s", testSystemNamespace, hmcServiceAccountName)}, + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, + }, + { + 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 cluster 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} - warn, err := validator.ValidateDelete(ctx, tt.template) + validator := &ClusterTemplateValidator{ + TemplateValidator: TemplateValidator{ + Client: c, + SystemNamespace: testSystemNamespace, + templateKind: v1alpha1.ClusterTemplateKind, + templateChainKind: v1alpha1.ClusterTemplateChainKind, + }, + } + + t.Setenv(ServiceAccountEnvName, hmcServiceAccountName) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: tt.userInfo, + }, + } + warn, err := validator.ValidateDelete(admission.NewContextWithRequest(ctx, req), tt.template) if tt.err != "" { g.Expect(err).To(MatchError(tt.err)) } else { @@ -107,6 +308,7 @@ func TestServiceTemplateValidateDelete(t *testing.T) { title string template *v1alpha1.ServiceTemplate existingObjects []runtime.Object + userInfo authenticationv1.UserInfo warnings admission.Warnings err string }{ @@ -122,6 +324,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 and the user triggered the deletion", + 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 +355,18 @@ func TestServiceTemplateValidateDelete(t *testing.T) { }, }, { - title: "should be OK because of a different cluster", + title: "should succeed if the template is managed by HMC and the controller triggered the deletion", + template: template.NewServiceTemplate(template.ManagedByHMC()), + userInfo: authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:%s:%s", testSystemNamespace, hmcServiceAccountName)}, + existingObjects: []runtime.Object{tm.NewTemplateManagement(tm.WithName(tmName))}, + }, + { + 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,9 +394,26 @@ 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} - warn, err := validator.ValidateDelete(ctx, tt.template) + + validator := &ServiceTemplateValidator{ + TemplateValidator{ + Client: c, + SystemNamespace: testSystemNamespace, + templateKind: v1alpha1.ServiceTemplateKind, + templateChainKind: v1alpha1.ServiceTemplateChainKind, + }, + } + + t.Setenv(ServiceAccountEnvName, hmcServiceAccountName) + + req := admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + UserInfo: tt.userInfo, + }, + } + warn, err := validator.ValidateDelete(admission.NewContextWithRequest(ctx, req), tt.template) if tt.err != "" { g.Expect(err).To(MatchError(tt.err)) } else { diff --git a/internal/webhook/userinfo.go b/internal/webhook/userinfo.go new file mode 100644 index 000000000..2ca3f7e4a --- /dev/null +++ b/internal/webhook/userinfo.go @@ -0,0 +1,32 @@ +// Copyright 2024 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package webhook + +import ( + "k8s.io/apiserver/pkg/authentication/serviceaccount" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" +) + +const ( + ServiceAccountEnvName = "SERVICE_ACCOUNT" +) + +func serviceAccountIsEqual(req admission.Request, namespace, name string) bool { + saNamespace, saName, err := serviceaccount.SplitUsername(req.UserInfo.Username) + if err != nil { + return false + } + return namespace == saNamespace && name == saName +} diff --git a/templates/provider/hmc/templates/deployment.yaml b/templates/provider/hmc/templates/deployment.yaml index 2c3c3c542..a42be8183 100644 --- a/templates/provider/hmc/templates/deployment.yaml +++ b/templates/provider/hmc/templates/deployment.yaml @@ -39,6 +39,8 @@ spec: env: - name: KUBERNETES_CLUSTER_DOMAIN value: {{ quote .Values.kubernetesClusterDomain }} + - name: SERVICE_ACCOUNT + value: {{ include "hmc.fullname" . }}-controller-manager image: {{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }} imagePullPolicy: {{ .Values.image.pullPolicy }} diff --git a/templates/provider/hmc/templates/webhooks.yaml b/templates/provider/hmc/templates/webhooks.yaml index b0e41200e..86302aab9 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/release/release.go b/test/objects/release/release.go index ab92e5b4e..bef914af1 100644 --- a/test/objects/release/release.go +++ b/test/objects/release/release.go @@ -68,3 +68,9 @@ func WithCAPITemplateName(v string) Opt { r.Spec.CAPI.Template = v } } + +func WithProviders(providers []v1alpha1.NamedProviderTemplate) Opt { + return func(r *v1alpha1.Release) { + r.Spec.Providers = providers + } +} 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, }