From ee7c734498e9f18fe891f0f3de2d4b8a053fc4a7 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. ProviderTemplates can't be removed if it's a core provider or enabled in Management spec.providers --- api/v1alpha1/common.go | 38 ++- api/v1alpha1/providertemplate_types.go | 5 + go.mod | 2 +- internal/controller/suite_test.go | 29 +- .../controller/template_controller_test.go | 2 +- .../templatechain_controller_test.go | 26 +- internal/webhook/template_webhook.go | 214 +++++++++++-- internal/webhook/template_webhook_test.go | 284 ++++++++++++++++-- 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/template/template.go | 12 + test/objects/templatechain/templatechain.go | 10 +- 14 files changed, 619 insertions(+), 64 deletions(-) create mode 100644 internal/webhook/userinfo.go diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index 31400432f..54c7f0ef6 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -71,7 +71,14 @@ func SetupIndexers(ctx context.Context, mgr ctrl.Manager) error { return err } - return SetupManagedClusterServicesIndexer(ctx, mgr) + if err := SetupManagedClusterServicesIndexer(ctx, mgr); err != nil { + return err + } + + if err := SetupClusterTemplateChainIndexer(ctx, mgr); err != nil { + return err + } + return SetupServiceTemplateChainIndexer(ctx, mgr) } const TemplateKey = ".spec.template" @@ -121,3 +128,32 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { return templates } + +const SupportedTemplateKey = ".spec.supportedTemplates[].Name" + +func SetupClusterTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error { + return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, SupportedTemplateKey, ExtractSupportedTemplatesNames) +} + +func SetupServiceTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error { + return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, SupportedTemplateKey, ExtractSupportedTemplatesNames) +} + +func ExtractSupportedTemplatesNames(rawObj client.Object) []string { + chainSpec := TemplateChainSpec{} + switch chain := rawObj.(type) { + case *ClusterTemplateChain: + chainSpec = chain.Spec + case *ServiceTemplateChain: + chainSpec = chain.Spec + default: + return nil + } + + supportedTemplates := make([]string, 0, len(chainSpec.SupportedTemplates)) + for _, t := range chainSpec.SupportedTemplates { + supportedTemplates = append(supportedTemplates, t.Name) + } + + return supportedTemplates +} diff --git a/api/v1alpha1/providertemplate_types.go b/api/v1alpha1/providertemplate_types.go index ad102bfcf..39f9ea0aa 100644 --- a/api/v1alpha1/providertemplate_types.go +++ b/api/v1alpha1/providertemplate_types.go @@ -20,6 +20,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // Denotes the providertemplate resource Kind. + ProviderTemplateKind = "ProviderTemplate" +) + // ProviderTemplateSpec defines the desired state of ProviderTemplate type ProviderTemplateSpec struct { Helm HelmSpec `json:"helm,omitempty"` diff --git a/go.mod b/go.mod index 08794106b..a603601b6 100644 --- a/go.mod +++ b/go.mod @@ -24,6 +24,7 @@ require ( k8s.io/api v0.31.1 k8s.io/apiextensions-apiserver v0.31.1 k8s.io/apimachinery v0.31.1 + k8s.io/apiserver v0.31.1 k8s.io/client-go v0.31.1 k8s.io/utils v0.0.0-20240921022957-49e7df575cb6 sigs.k8s.io/cluster-api v1.8.4 @@ -175,7 +176,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.1 // indirect k8s.io/cli-runtime v0.31.1 // indirect k8s.io/component-base v0.31.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 50f8d66f3..857d7e735 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -29,10 +29,12 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" admissionv1 "k8s.io/api/admissionregistration/v1" + authenticationv1 "k8s.io/api/authentication/v1" utilyaml "sigs.k8s.io/cluster-api/util/yaml" ctrl "sigs.k8s.io/controller-runtime" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2" sourcev1 "github.com/fluxcd/source-controller/api/v1" @@ -43,9 +45,11 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/log/zap" + sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" + hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" hmcwebhook "github.com/Mirantis/hmc/internal/webhook" - sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1" // +kubebuilder:scaffold:imports ) @@ -55,6 +59,8 @@ import ( const ( mutatingWebhookKind = "MutatingWebhookConfiguration" validatingWebhookKind = "ValidatingWebhookConfiguration" + + hmcServiceAccountName = "hmc-controller-manager" ) var ( @@ -63,6 +69,8 @@ var ( testEnv *envtest.Environment ctx context.Context cancel context.CancelFunc + + userInfo = authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:%s:%s", utils.DefaultSystemNamespace, hmcServiceAccountName)} ) func TestControllers(t *testing.T) { @@ -83,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"), @@ -156,13 +167,20 @@ var _ = BeforeSuite(func() { err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr) + templateValidator := hmcwebhook.TemplateValidator{ + SystemNamespace: utils.DefaultSystemNamespace, + InjectUserInfo: func(req *admission.Request) { + req.UserInfo = userInfo + }, + } + + err = (&hmcwebhook.ClusterTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) - err = (&hmcwebhook.ServiceTemplateValidator{}).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() { @@ -188,6 +206,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 dc1c38991..4eb94fb37 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 a4813fd4c..e1381cabf 100644 --- a/internal/controller/templatechain_controller_test.go +++ b/internal/controller/templatechain_controller_test.go @@ -213,19 +213,6 @@ var _ = Describe("Template Chain Controller", func() { }) AfterEach(func() { - for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ - ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct1"], ctTemplates["ct2"], - } { - err := k8sClient.Delete(ctx, template) - Expect(crclient.IgnoreNotFound(err)).To(Succeed()) - } - for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{ - stTemplates["test"], stTemplates["st0"], stTemplates["st1"], stTemplates["st2"], - } { - err := k8sClient.Delete(ctx, template) - Expect(crclient.IgnoreNotFound(err)).To(Succeed()) - } - for _, chain := range ctChainNames { clusterTemplateChainResource := &hmcmirantiscomv1alpha1.ClusterTemplateChain{} err := k8sClient.Get(ctx, chain, clusterTemplateChainResource) @@ -243,6 +230,19 @@ var _ = Describe("Template Chain Controller", func() { Expect(k8sClient.Delete(ctx, serviceTemplateChainResource)).To(Succeed()) } + for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ + ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct1"], ctTemplates["ct2"], + } { + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{ + stTemplates["test"], stTemplates["st0"], stTemplates["st1"], stTemplates["st2"], + } { + err := k8sClient.Delete(ctx, template) + Expect(crclient.IgnoreNotFound(err)).To(Succeed()) + } + By("Cleanup the namespace") err := k8sClient.Get(ctx, types.NamespacedName{Name: namespace.Name}, namespace) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/webhook/template_webhook.go b/internal/webhook/template_webhook.go index a57343ef7..c3efc678c 100644 --- a/internal/webhook/template_webhook.go +++ b/internal/webhook/template_webhook.go @@ -18,9 +18,11 @@ import ( "context" "errors" "fmt" + "os" 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,11 +31,18 @@ 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 + InjectUserInfo func(*admission.Request) } -var errTemplateDeletionForbidden = errors.New("template deletion is forbidden") +type ClusterTemplateValidator struct { + TemplateValidator +} func (v *ClusterTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { v.Client = mgr.GetClient() @@ -65,17 +74,12 @@ func (v *ClusterTemplateValidator) ValidateDelete(ctx context.Context, obj runti if !ok { 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.TemplateKey: 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: %v", 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,7 +91,7 @@ func (*ClusterTemplateValidator) Default(context.Context, runtime.Object) error } type ServiceTemplateValidator struct { - client.Client + TemplateValidator } func (v *ServiceTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -116,21 +120,17 @@ func (*ServiceTemplateValidator) ValidateUpdate(_ context.Context, _ runtime.Obj // ValidateDelete implements webhook.Validator so a webhook will be registered for the type. func (v *ServiceTemplateValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { - tmpl, ok := obj.(*v1alpha1.ServiceTemplate) + template, ok := obj.(*v1alpha1.ServiceTemplate) if !ok { 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.ServicesTemplateKey: tmpl.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 ServiceTemplate %s/%s is allowed to be deleted: %v", template.Namespace, template.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 } return nil, nil @@ -142,7 +142,7 @@ func (*ServiceTemplateValidator) Default(_ context.Context, _ runtime.Object) er } type ProviderTemplateValidator struct { - client.Client + TemplateValidator } func (v *ProviderTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { @@ -170,7 +170,18 @@ func (*ProviderTemplateValidator) ValidateUpdate(_ context.Context, _ runtime.Ob } // 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: %v", template.Name, err) + } + if !deletionAllowed { + return warnings, errTemplateDeletionForbidden + } return nil, nil } @@ -178,3 +189,156 @@ 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 + } + + // Forbid template deletion if the template is managed by the TemplateManagement + if !triggeredByController && templateManagedByHMC(template) { + return false, admission.Warnings{fmt.Sprintf("The Template is managed by the TemplateManagement and %s", getTemplateChainKind(template))}, nil + } + + kind := template.GetObjectKind().GroupVersionKind().Kind + switch kind { + 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 { + 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 mgmt.Spec.Core == nil { + return false, nil, fmt.Errorf("spec.core in Management object is undefined") + } + capiTemplate := mgmt.Spec.Core.CAPI.Template + if capiTemplate == "" { + capiTemplate = release.Spec.CAPI.Template + } + hmcTemplate := mgmt.Spec.Core.HMC.Template + if hmcTemplate == "" { + hmcTemplate = release.Spec.HMC.Template + } + if template.GetName() == capiTemplate || template.GetName() == hmcTemplate { + return false, admission.Warnings{fmt.Sprintf("The ProviderTemplate %s can't be removed, as core components are mandatory", template.GetName())}, nil + } + + for _, provider := range mgmt.Spec.Providers { + templateName := provider.Template + if templateName == "" { + templateName = release.ProviderTemplate(provider.Name) + } + if templateName == "" { + return false, nil, fmt.Errorf("failed to get template name for the %s provider", provider.Name) + } + if templateName == template.GetName() { + return false, admission.Warnings{fmt.Sprintf("The ProviderTemplate %s can't be removed until it is enabled in the Management spec.providers", template.GetName())}, nil + } + } + return true, nil, nil + case v1alpha1.ClusterTemplateKind, v1alpha1.ServiceTemplateKind: + 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", kind)}, 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", kind, getTemplateChainKind(template))}, 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 template.GetObjectKind().GroupVersionKind().Kind { + case v1alpha1.ClusterTemplateKind: + key = v1alpha1.TemplateKey + case v1alpha1.ServiceTemplateKind: + key = v1alpha1.ServicesTemplateKey + default: + return false, fmt.Errorf("invalid Template kind. Supported values are: %s and %s", 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.SupportedTemplateKey: template.GetName()}, + client.Limit(1), + } + templateChainKind := getTemplateChainKind(template) + if 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 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 +} + +func getTemplateChainKind(template client.Object) string { + kind := template.GetObjectKind().GroupVersionKind().Kind + if kind == v1alpha1.ClusterTemplateKind { + return v1alpha1.ClusterTemplateChainKind + } + if kind == v1alpha1.ServiceTemplateKind { + return v1alpha1.ServiceTemplateChainKind + } + return "" +} diff --git a/internal/webhook/template_webhook_test.go b/internal/webhook/template_webhook_test.go index 44938a4cc..54cab2150 100644 --- a/internal/webhook/template_webhook_test.go +++ b/internal/webhook/template_webhook_test.go @@ -16,34 +16,195 @@ 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/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" + systemNamespace = "hmc" + 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, as core components are mandatory", 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, as core components are mandatory", 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 until it is enabled in the Management spec.providers", 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 until it is enabled in the Management spec.providers", 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.ServicesTemplateKey, v1alpha1.ExtractServiceTemplateName). + WithIndex(&v1alpha1.ServiceTemplateChain{}, v1alpha1.SupportedTemplateKey, v1alpha1.ExtractSupportedTemplatesNames). + Build() + + validator := &ProviderTemplateValidator{ + TemplateValidator{ + Client: c, + SystemNamespace: systemNamespace, + }, + } + + 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), @@ -53,7 +214,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"), @@ -61,28 +242,48 @@ 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", systemNamespace, 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(tt.existingObjects[0], v1alpha1.TemplateKey, v1alpha1.ExtractTemplateName). + WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.TemplateKey, v1alpha1.ExtractTemplateName). + WithIndex(&v1alpha1.ClusterTemplateChain{}, v1alpha1.SupportedTemplateKey, v1alpha1.ExtractSupportedTemplatesNames). Build() - validator := &ClusterTemplateValidator{Client: c} - warn, err := validator.ValidateDelete(ctx, tt.template) + validator := &ClusterTemplateValidator{ + TemplateValidator: TemplateValidator{ + Client: c, + SystemNamespace: systemNamespace, + }, + } + + 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 { @@ -106,6 +307,7 @@ func TestServiceTemplateValidateDelete(t *testing.T) { title string template *v1alpha1.ServiceTemplate existingObjects []runtime.Object + userInfo authenticationv1.UserInfo warnings admission.Warnings err string }{ @@ -121,6 +323,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, @@ -132,7 +354,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", systemNamespace, 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()}, }, @@ -146,10 +379,25 @@ func TestServiceTemplateValidateDelete(t *testing.T) { NewClientBuilder(). WithScheme(scheme.Scheme). WithRuntimeObjects(tt.existingObjects...). - WithIndex(tt.existingObjects[0], v1alpha1.ServicesTemplateKey, v1alpha1.ExtractServiceTemplateName). + WithIndex(&v1alpha1.ManagedCluster{}, v1alpha1.ServicesTemplateKey, v1alpha1.ExtractServiceTemplateName). + WithIndex(&v1alpha1.ServiceTemplateChain{}, v1alpha1.SupportedTemplateKey, v1alpha1.ExtractSupportedTemplatesNames). Build() - validator := &ServiceTemplateValidator{Client: c} - warn, err := validator.ValidateDelete(ctx, tt.template) + + validator := &ServiceTemplateValidator{ + TemplateValidator{ + Client: c, + SystemNamespace: systemNamespace, + }, + } + + 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 b9faed015..1b7cd58f4 100644 --- a/templates/provider/hmc/templates/webhooks.yaml +++ b/templates/provider/hmc/templates/webhooks.yaml @@ -146,6 +146,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/template/template.go b/test/objects/template/template.go index 073be7207..9557f49c0 100644 --- a/test/objects/template/template.go +++ b/test/objects/template/template.go @@ -41,6 +41,10 @@ type ( func NewClusterTemplate(opts ...Opt) *v1alpha1.ClusterTemplate { t := &v1alpha1.ClusterTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ClusterTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, Namespace: DefaultNamespace, @@ -56,6 +60,10 @@ func NewClusterTemplate(opts ...Opt) *v1alpha1.ClusterTemplate { func NewServiceTemplate(opts ...Opt) *v1alpha1.ServiceTemplate { t := &v1alpha1.ServiceTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ServiceTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, Namespace: DefaultNamespace, @@ -71,6 +79,10 @@ func NewServiceTemplate(opts ...Opt) *v1alpha1.ServiceTemplate { func NewProviderTemplate(opts ...Opt) *v1alpha1.ProviderTemplate { t := &v1alpha1.ProviderTemplate{ + TypeMeta: metav1.TypeMeta{ + APIVersion: v1alpha1.GroupVersion.String(), + Kind: v1alpha1.ProviderTemplateKind, + }, ObjectMeta: metav1.ObjectMeta{ Name: DefaultName, }, 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, }