diff --git a/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go index 90b801606..992b25ea9 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/go.mod b/go.mod index e0c917937..f87458e7a 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.4 @@ -178,7 +179,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/component-base v0.31.2 // indirect k8s.io/klog/v2 v2.130.1 // indirect diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 12e321f03..41bfdebea 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/kubernetes/scheme" "k8s.io/client-go/rest" utilyaml "sigs.k8s.io/cluster-api/util/yaml" @@ -42,8 +43,10 @@ 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" + "github.com/Mirantis/hmc/internal/utils" hmcwebhook "github.com/Mirantis/hmc/internal/webhook" ) @@ -53,6 +56,8 @@ import ( const ( mutatingWebhookKind = "MutatingWebhookConfiguration" validatingWebhookKind = "ValidatingWebhookConfiguration" + + hmcServiceAccountName = "hmc-controller-manager" ) var ( @@ -62,6 +67,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) { @@ -82,6 +89,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 +166,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 +205,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 373046f72..dce5da61c 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 b2a0dacd8..09fa2d282 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,11 +32,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 +75,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 +92,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 +121,17 @@ func (*ServiceTemplateValidator) ValidateUpdate(_ context.Context, _, _ runtime. // 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 +143,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 +171,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: %v", template.Name, err) + } + if !deletionAllowed { + return warnings, errTemplateDeletionForbidden + } return nil, nil } @@ -178,3 +190,135 @@ 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{"The Template is managed by the TemplateManagement and " + 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 { + 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: + 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..5c218c4d0 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 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.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 3d2b0e550..2b89367d0 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, }