diff --git a/api/v1alpha1/common.go b/api/v1alpha1/common.go index ddd02be9a..ddf27cd83 100644 --- a/api/v1alpha1/common.go +++ b/api/v1alpha1/common.go @@ -79,6 +79,10 @@ func SetupIndexers(ctx context.Context, mgr ctrl.Manager) error { return err } + if err := SetupMultiClusterServiceServicesIndexer(ctx, mgr); err != nil { + return err + } + if err := SetupClusterTemplateChainIndexer(ctx, mgr); err != nil { return err } @@ -131,10 +135,10 @@ func ExtractReleaseTemplates(rawObj client.Object) []string { const ServicesTemplateKey = ".spec.services[].Template" func SetupManagedClusterServicesIndexer(ctx context.Context, mgr ctrl.Manager) error { - return mgr.GetFieldIndexer().IndexField(ctx, &ManagedCluster{}, ServicesTemplateKey, ExtractServiceTemplateName) + return mgr.GetFieldIndexer().IndexField(ctx, &ManagedCluster{}, ServicesTemplateKey, ExtractServiceTemplateFromManagedCluster) } -func ExtractServiceTemplateName(rawObj client.Object) []string { +func ExtractServiceTemplateFromManagedCluster(rawObj client.Object) []string { cluster, ok := rawObj.(*ManagedCluster) if !ok { return nil @@ -148,6 +152,24 @@ func ExtractServiceTemplateName(rawObj client.Object) []string { return templates } +func SetupMultiClusterServiceServicesIndexer(ctx context.Context, mgr ctrl.Manager) error { + return mgr.GetFieldIndexer().IndexField(ctx, &MultiClusterService{}, ServicesTemplateKey, ExtractServiceTemplateFromMultiClusterService) +} + +func ExtractServiceTemplateFromMultiClusterService(rawObj client.Object) []string { + cluster, ok := rawObj.(*MultiClusterService) + if !ok { + return nil + } + + templates := []string{} + for _, s := range cluster.Spec.Services { + templates = append(templates, s.Template) + } + + return templates +} + const SupportedTemplateKey = ".spec.supportedTemplates[].Name" func SetupClusterTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error { diff --git a/cmd/main.go b/cmd/main.go index acf62c376..cc7eb777a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -331,6 +331,10 @@ func setupWebhooks(mgr ctrl.Manager, currentNamespace string) error { setupLog.Error(err, "unable to create webhook", "webhook", "ManagedCluster") return err } + if err := (&hmcwebhook.MultiClusterServiceValidator{}).SetupWebhookWithManager(mgr); err != nil { + setupLog.Error(err, "unable to create webhook", "webhook", "MultiClusterService") + return err + } if err := (&hmcwebhook.ManagementValidator{}).SetupWebhookWithManager(mgr); err != nil { setupLog.Error(err, "unable to create webhook", "webhook", "Management") return err diff --git a/internal/controller/managedcluster_controller_test.go b/internal/controller/managedcluster_controller_test.go index 2182968fe..ac37d1901 100644 --- a/internal/controller/managedcluster_controller_test.go +++ b/internal/controller/managedcluster_controller_test.go @@ -27,6 +27,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" hmc "github.com/Mirantis/hmc/api/v1alpha1" @@ -38,8 +39,9 @@ var _ = Describe("ManagedCluster Controller", func() { managedClusterName = "test-managed-cluster" managedClusterNamespace = "test" - templateName = "test-template" - credentialName = "test-credential" + templateName = "test-template" + svcTemplateName = "test-svc-template" + credentialName = "test-credential" ) ctx := context.Background() @@ -50,6 +52,7 @@ var _ = Describe("ManagedCluster Controller", func() { } managedCluster := &hmc.ManagedCluster{} template := &hmc.ClusterTemplate{} + svcTemplate := &hmc.ServiceTemplate{} management := &hmc.Management{} credential := &hmc.Credential{} namespace := &corev1.Namespace{} @@ -66,7 +69,7 @@ var _ = Describe("ManagedCluster Controller", func() { Expect(k8sClient.Create(ctx, namespace)).To(Succeed()) } - By("creating the custom resource for the Kind Template") + By("creating the custom resource for the Kind ClusterTemplate") err = k8sClient.Get(ctx, typeNamespacedName, template) if err != nil && errors.IsNotFound(err) { template = &hmc.ClusterTemplate{ @@ -99,6 +102,35 @@ var _ = Describe("ManagedCluster Controller", func() { Expect(k8sClient.Status().Update(ctx, template)).To(Succeed()) } + By("creating the custom resource for the Kind ServiceTemplate") + err = k8sClient.Get(ctx, client.ObjectKey{Namespace: managedClusterNamespace, Name: svcTemplateName}, svcTemplate) + if err != nil && errors.IsNotFound(err) { + svcTemplate = &hmc.ServiceTemplate{ + ObjectMeta: metav1.ObjectMeta{ + Name: svcTemplateName, + Namespace: managedClusterNamespace, + }, + Spec: hmc.ServiceTemplateSpec{ + Helm: hmc.HelmSpec{ + ChartRef: &hcv2.CrossNamespaceSourceReference{ + Kind: "HelmChart", + Name: "ref-test", + Namespace: "default", + }, + }, + }, + } + Expect(k8sClient.Create(ctx, svcTemplate)).To(Succeed()) + svcTemplate.Status = hmc.ServiceTemplateStatus{ + TemplateStatusCommon: hmc.TemplateStatusCommon{ + TemplateValidationStatus: hmc.TemplateValidationStatus{ + Valid: true, + }, + }, + } + Expect(k8sClient.Status().Update(ctx, svcTemplate)).To(Succeed()) + } + By("creating the custom resource for the Kind Management") err = k8sClient.Get(ctx, typeNamespacedName, management) if err != nil && errors.IsNotFound(err) { @@ -148,6 +180,12 @@ var _ = Describe("ManagedCluster Controller", func() { Spec: hmc.ManagedClusterSpec{ Template: templateName, Credential: credentialName, + Services: []hmc.ServiceSpec{ + { + Template: svcTemplateName, + Name: "test-svc-name", + }, + }, }, } Expect(k8sClient.Create(ctx, managedCluster)).To(Succeed()) diff --git a/internal/controller/multiclusterservice_controller_test.go b/internal/controller/multiclusterservice_controller_test.go index 245f5b55e..7502bda85 100644 --- a/internal/controller/multiclusterservice_controller_test.go +++ b/internal/controller/multiclusterservice_controller_test.go @@ -150,6 +150,19 @@ var _ = Describe("MultiClusterService Controller", func() { } Expect(k8sClient.Create(ctx, serviceTemplate)).To(Succeed()) + By("reconciling ServiceTemplate used by MultiClusterService") + templateReconciler := TemplateReconciler{ + Client: k8sClient, + downloadHelmChartFunc: fakeDownloadHelmChartFunc, + } + serviceTemplateReconciler := &ServiceTemplateReconciler{TemplateReconciler: templateReconciler} + _, err = serviceTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: serviceTemplateRef}) + Expect(err).NotTo(HaveOccurred()) + + By("having the valid service template status") + Expect(k8sClient.Get(ctx, serviceTemplateRef, serviceTemplate)).To(Succeed()) + Expect(serviceTemplate.Status.Valid && serviceTemplate.Status.ValidationError == "").To(BeTrue()) + By("creating MultiClusterService") err = k8sClient.Get(ctx, multiClusterServiceRef, multiClusterService) if err != nil && apierrors.IsNotFound(err) { @@ -204,19 +217,10 @@ var _ = Describe("MultiClusterService Controller", func() { }) It("should successfully reconcile the resource", func() { - By("reconciling ServiceTemplate used by MultiClusterService") - templateReconciler := TemplateReconciler{ - Client: k8sClient, - downloadHelmChartFunc: fakeDownloadHelmChartFunc, - } - serviceTemplateReconciler := &ServiceTemplateReconciler{TemplateReconciler: templateReconciler} - _, err := serviceTemplateReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: serviceTemplateRef}) - Expect(err).NotTo(HaveOccurred()) - By("reconciling MultiClusterService") multiClusterServiceReconciler := &MultiClusterServiceReconciler{Client: k8sClient} - _, err = multiClusterServiceReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: multiClusterServiceRef}) + _, err := multiClusterServiceReconciler.Reconcile(ctx, reconcile.Request{NamespacedName: multiClusterServiceRef}) Expect(err).NotTo(HaveOccurred()) Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, clusterProfileRef, clusterProfile).ShouldNot(HaveOccurred()) diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go index 12e321f03..f9fbabc82 100644 --- a/internal/controller/suite_test.go +++ b/internal/controller/suite_test.go @@ -144,6 +144,9 @@ var _ = BeforeSuite(func() { err = (&hmcwebhook.ManagedClusterValidator{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) + err = (&hmcwebhook.MultiClusterServiceValidator{}).SetupWebhookWithManager(mgr) + Expect(err).NotTo(HaveOccurred()) + err = (&hmcwebhook.ManagementValidator{}).SetupWebhookWithManager(mgr) Expect(err).NotTo(HaveOccurred()) diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index 5ad95f986..bf5ecc51c 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -67,7 +67,7 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } - if err := isTemplateValid(template); err != nil { + if err := isTemplateValid(template.GetCommonStatus()); err != nil { return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } @@ -79,6 +79,18 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } + // validate services + for _, svc := range managedCluster.Spec.Services { + tpl, err := getServiceTemplate(ctx, v.Client, managedCluster.Namespace, svc.Template) + if err != nil { + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) + } + + if err := isTemplateValid(tpl.GetCommonStatus()); err != nil { + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) + } + } + return nil, nil } @@ -106,7 +118,7 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne return admission.Warnings{msg}, errClusterUpgradeForbidden } - if err := isTemplateValid(template); err != nil { + if err := isTemplateValid(template.GetCommonStatus()); err != nil { return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } @@ -119,6 +131,18 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) } + // validate services + for _, svc := range newManagedCluster.Spec.Services { + tpl, err := getServiceTemplate(ctx, v.Client, newManagedCluster.Namespace, svc.Template) + if err != nil { + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) + } + + if err := isTemplateValid(tpl.GetCommonStatus()); err != nil { + return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err) + } + } + return nil, nil } @@ -185,7 +209,7 @@ func (v *ManagedClusterValidator) Default(ctx context.Context, obj runtime.Objec return fmt.Errorf("could not get template for the managedcluster: %w", err) } - if err := isTemplateValid(template); err != nil { + if err := isTemplateValid(template.GetCommonStatus()); err != nil { return fmt.Errorf("template is invalid: %w", err) } @@ -216,9 +240,9 @@ func (v *ManagedClusterValidator) getManagedClusterCredential(ctx context.Contex return cred, nil } -func isTemplateValid(template *hmcv1alpha1.ClusterTemplate) error { - if !template.Status.Valid { - return fmt.Errorf("the template is not valid: %s", template.Status.ValidationError) +func isTemplateValid(status *hmcv1alpha1.TemplateStatusCommon) error { + if !status.Valid { + return fmt.Errorf("the template is not valid: %s", status.ValidationError) } return nil diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 4d8cc84a5..87a1ed148 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -97,6 +97,32 @@ func TestManagedClusterValidateCreate(t *testing.T) { }, err: fmt.Sprintf("the ManagedCluster is invalid: clustertemplates.hmc.mirantis.com \"%s\" not found", testTemplateName), }, + { + name: "should fail if the ServiceTemplates are not found in same namespace", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace("othernamespace"), + ), + }, + err: fmt.Sprintf("the ManagedCluster is invalid: servicetemplates.hmc.mirantis.com \"%s\" not found", testSvcTemplate1Name), + }, { name: "should fail if the cluster template was found but is invalid (some validation error)", managedCluster: managedcluster.NewManagedCluster( @@ -116,11 +142,41 @@ func TestManagedClusterValidateCreate(t *testing.T) { }, err: "the ManagedCluster is invalid: the template is not valid: validation error example", }, + { + name: "should fail if the service templates were found but are invalid (some validation error)", + managedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + ), + }, + err: "the ManagedCluster is invalid: the template is not valid: validation error example", + }, { name: "should succeed", managedCluster: managedcluster.NewManagedCluster( managedcluster.WithClusterTemplate(testTemplateName), managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), ), existingObjects: []runtime.Object{ mgmt, @@ -134,6 +190,10 @@ func TestManagedClusterValidateCreate(t *testing.T) { }), template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), }, }, { @@ -394,6 +454,148 @@ func TestManagedClusterValidateUpdate(t *testing.T) { ), }, }, + { + name: "should succeed if serviceTemplates are added", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"bar"}`), + managedcluster.WithCredential(testCredentialName), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"a":"b"}`), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "should succeed if serviceTemplates are removed", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"bar"}`), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"a":"b"}`), + managedcluster.WithCredential(testCredentialName), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "should fail if serviceTemplates are not in the same namespace", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"bar"}`), + managedcluster.WithCredential(testCredentialName), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"a":"b"}`), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace("othernamespace"), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: fmt.Sprintf("the ManagedCluster is invalid: servicetemplates.hmc.mirantis.com \"%s\" not found", testSvcTemplate1Name), + }, + { + name: "should fail if the ServiceTemplates were found but are invalid", + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"bar"}`), + managedcluster.WithCredential(testCredentialName), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithClusterTemplate(testTemplateName), + managedcluster.WithConfig(`{"a":"b"}`), + managedcluster.WithCredential(testCredentialName), + managedcluster.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + mgmt, + cred, + template.NewClusterTemplate( + template.WithName(testTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + template.WithProvidersStatus(v1alpha1.Providers{ + "infrastructure-aws", + "control-plane-k0smotron", + "bootstrap-k0smotron", + }), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + ), + }, + err: "the ManagedCluster is invalid: the template is not valid: validation error example", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/webhook/multiclusterservice_webhook.go b/internal/webhook/multiclusterservice_webhook.go new file mode 100644 index 000000000..aa6a85016 --- /dev/null +++ b/internal/webhook/multiclusterservice_webhook.go @@ -0,0 +1,112 @@ +// 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 ( + "context" + "fmt" + + apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/runtime" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" + + "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" +) + +type MultiClusterServiceValidator struct { + client.Client +} + +const invalidMultiClusterServiceMsg = "the MultiClusterService is invalid" + +// SetupWebhookWithManager will setup the manager to manage the webhooks +func (v *MultiClusterServiceValidator) SetupWebhookWithManager(mgr ctrl.Manager) error { + v.Client = mgr.GetClient() + return ctrl.NewWebhookManagedBy(mgr). + For(&v1alpha1.MultiClusterService{}). + WithValidator(v). + WithDefaulter(v). + Complete() +} + +var ( + _ webhook.CustomValidator = &MultiClusterServiceValidator{} + _ webhook.CustomDefaulter = &MultiClusterServiceValidator{} +) + +// Default implements webhook.Defaulter so a webhook will be registered for the type. +func (*MultiClusterServiceValidator) Default(_ context.Context, _ runtime.Object) error { + return nil +} + +// ValidateCreate implements webhook.Validator so a webhook will be registered for the type. +func (v *MultiClusterServiceValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) { + mcs, ok := obj.(*v1alpha1.MultiClusterService) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected MultiClusterService but got a %T", obj)) + } + + for _, svc := range mcs.Spec.Services { + tpl, err := getServiceTemplate(ctx, v.Client, utils.DefaultSystemNamespace, svc.Template) + if err != nil { + return nil, fmt.Errorf("%s: %w", invalidMultiClusterServiceMsg, err) + } + + if err := isTemplateValid(tpl.GetCommonStatus()); err != nil { + return nil, fmt.Errorf("%s: %w", invalidMultiClusterServiceMsg, err) + } + } + + return nil, nil +} + +// ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. +func (v *MultiClusterServiceValidator) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) { + _, ok := oldObj.(*v1alpha1.MultiClusterService) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected MultiClusterService but got a %T", oldObj)) + } + mcs, ok := newObj.(*v1alpha1.MultiClusterService) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected MultiClusterService but got a %T", newObj)) + } + + for _, svc := range mcs.Spec.Services { + tpl, err := getServiceTemplate(ctx, v.Client, utils.DefaultSystemNamespace, svc.Template) + if err != nil { + return nil, fmt.Errorf("%s: %w", invalidMultiClusterServiceMsg, err) + } + + if err := isTemplateValid(tpl.GetCommonStatus()); err != nil { + return nil, fmt.Errorf("%s: %w", invalidMultiClusterServiceMsg, err) + } + } + + return nil, nil +} + +// ValidateDelete implements webhook.Validator so a webhook will be registered for the type. +func (*MultiClusterServiceValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) { + return nil, nil +} + +func getServiceTemplate(ctx context.Context, c client.Client, templateNamespace, templateName string) (tpl *v1alpha1.ServiceTemplate, err error) { + tpl = new(v1alpha1.ServiceTemplate) + return tpl, c.Get(ctx, client.ObjectKey{Namespace: templateNamespace, Name: templateName}, tpl) +} diff --git a/internal/webhook/multiclusterservice_webhook_test.go b/internal/webhook/multiclusterservice_webhook_test.go new file mode 100644 index 000000000..600b83f3e --- /dev/null +++ b/internal/webhook/multiclusterservice_webhook_test.go @@ -0,0 +1,264 @@ +// 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 ( + "context" + "fmt" + "testing" + + . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/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/internal/utils" + "github.com/Mirantis/hmc/test/objects/multiclusterservice" + "github.com/Mirantis/hmc/test/objects/template" + "github.com/Mirantis/hmc/test/scheme" +) + +const ( + testMCSName = "testmcs" + testSvcTemplate1Name = "test-servicetemplate-1" + testSvcTemplate2Name = "test-servicetemplate-2" +) + +func TestMultiClusterServiceValidateCreate(t *testing.T) { + ctx := admission.NewContextWithRequest(context.Background(), admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Create, + }, + }) + + tests := []struct { + name string + mcs *v1alpha1.MultiClusterService + existingObjects []runtime.Object + err string + warnings admission.Warnings + }{ + { + name: "should fail if the ServiceTemplates are not found in hmc-system namespace", + mcs: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace("othernamespace"), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: fmt.Sprintf("the MultiClusterService is invalid: servicetemplates.hmc.mirantis.com \"%s\" not found", testSvcTemplate1Name), + }, + { + name: "should fail if the ServiceTemplates were found but are invalid", + mcs: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + ), + }, + err: "the MultiClusterService is invalid: the template is not valid: validation error example", + }, + { + name: "should succeed", + mcs: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "should succeed with multiple serviceTemplates", + mcs: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + multiclusterservice.WithServiceTemplate(testSvcTemplate2Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate2Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "should succeed without any serviceTemplates", + mcs: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + ), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() + validator := &MultiClusterServiceValidator{Client: c} + warn, err := validator.ValidateCreate(ctx, tt.mcs) + 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 TestMultiClusterServiceValidateUpdate(t *testing.T) { + ctx := admission.NewContextWithRequest(context.Background(), admission.Request{ + AdmissionRequest: admissionv1.AdmissionRequest{ + Operation: admissionv1.Update, + }, + }) + + oldMCS := multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + ) + + tests := []struct { + name string + newMCS *v1alpha1.MultiClusterService + existingObjects []runtime.Object + err string + warnings admission.Warnings + }{ + { + name: "should fail if the ServiceTemplates are not found in hmc-system namespace", + newMCS: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + multiclusterservice.WithServiceTemplate(testSvcTemplate2Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate2Name), + template.WithNamespace("othernamespace"), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + err: fmt.Sprintf("the MultiClusterService is invalid: servicetemplates.hmc.mirantis.com \"%s\" not found", testSvcTemplate2Name), + }, + { + name: "should fail if the ServiceTemplates were found but are invalid", + newMCS: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(testMCSName), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + multiclusterservice.WithServiceTemplate(testSvcTemplate2Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate2Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{ + Valid: false, + ValidationError: "validation error example", + }), + ), + }, + err: "the MultiClusterService is invalid: the template is not valid: validation error example", + }, + { + name: "should succeed if another template is added", + newMCS: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(oldMCS.Name), + multiclusterservice.WithServiceTemplate(testSvcTemplate1Name), + multiclusterservice.WithServiceTemplate(testSvcTemplate2Name), + ), + existingObjects: []runtime.Object{ + template.NewServiceTemplate( + template.WithName(testSvcTemplate1Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + template.NewServiceTemplate( + template.WithName(testSvcTemplate2Name), + template.WithNamespace(utils.DefaultSystemNamespace), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, + { + name: "should succeed if all templates removed", + newMCS: multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName(oldMCS.Name), + ), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + + c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build() + validator := &MultiClusterServiceValidator{Client: c} + warn, err := validator.ValidateUpdate(ctx, oldMCS, tt.newMCS) + 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()) + } + }) + } +} diff --git a/internal/webhook/template_webhook.go b/internal/webhook/template_webhook.go index b2a0dacd8..6666c2575 100644 --- a/internal/webhook/template_webhook.go +++ b/internal/webhook/template_webhook.go @@ -27,6 +27,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" ) type ClusterTemplateValidator struct { @@ -133,6 +134,20 @@ func (v *ServiceTemplateValidator) ValidateDelete(ctx context.Context, obj runti return admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden } + // MultiClusterServices can only refer to serviceTemplates in hmc-system namespace. + if tmpl.Namespace == utils.DefaultSystemNamespace { + multiSvcClusters := &v1alpha1.MultiClusterServiceList{} + if err := v.Client.List(ctx, multiSvcClusters, + client.MatchingFields{v1alpha1.ServicesTemplateKey: tmpl.Name}, + client.Limit(1)); err != nil { + return nil, err + } + + if len(multiSvcClusters.Items) > 0 { + return admission.Warnings{"The ServiceTemplate object can't be removed if MultiClusterService objects referencing it still exist"}, errTemplateDeletionForbidden + } + } + return nil, nil } diff --git a/internal/webhook/template_webhook_test.go b/internal/webhook/template_webhook_test.go index 44938a4cc..962a97560 100644 --- a/internal/webhook/template_webhook_test.go +++ b/internal/webhook/template_webhook_test.go @@ -24,7 +24,9 @@ import ( "sigs.k8s.io/controller-runtime/pkg/webhook/admission" "github.com/Mirantis/hmc/api/v1alpha1" + "github.com/Mirantis/hmc/internal/utils" "github.com/Mirantis/hmc/test/objects/managedcluster" + "github.com/Mirantis/hmc/test/objects/multiclusterservice" "github.com/Mirantis/hmc/test/objects/template" "github.com/Mirantis/hmc/test/scheme" ) @@ -136,6 +138,18 @@ func TestServiceTemplateValidateDelete(t *testing.T) { template: tmpl, existingObjects: []runtime.Object{managedcluster.NewManagedCluster()}, }, + { + title: "should fail if a MultiClusterService is referencing serviceTemplate in hmc-system namespace", + template: template.NewServiceTemplate(template.WithNamespace(utils.DefaultSystemNamespace), template.WithName(tmpl.Name)), + existingObjects: []runtime.Object{ + multiclusterservice.NewMultiClusterService( + multiclusterservice.WithName("mymulticlusterservice"), + multiclusterservice.WithServiceTemplate(tmpl.Name), + ), + }, + warnings: admission.Warnings{"The ServiceTemplate object can't be removed if MultiClusterService objects referencing it still exist"}, + err: errTemplateDeletionForbidden.Error(), + }, } for _, tt := range tests { @@ -146,7 +160,8 @@ 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.ExtractServiceTemplateFromManagedCluster). + WithIndex(&v1alpha1.MultiClusterService{}, v1alpha1.ServicesTemplateKey, v1alpha1.ExtractServiceTemplateFromMultiClusterService). Build() validator := &ServiceTemplateValidator{Client: c} warn, err := validator.ValidateDelete(ctx, tt.template) diff --git a/templates/provider/hmc/templates/webhooks.yaml b/templates/provider/hmc/templates/webhooks.yaml index b9faed015..1c1237864 100644 --- a/templates/provider/hmc/templates/webhooks.yaml +++ b/templates/provider/hmc/templates/webhooks.yaml @@ -28,6 +28,28 @@ webhooks: resources: - managedclusters sideEffects: None + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: {{ include "hmc.webhook.serviceName" . }} + namespace: {{ include "hmc.webhook.serviceNamespace" . }} + path: /mutate-hmc-mirantis-com-v1alpha1-multiclusterservice + failurePolicy: Fail + matchPolicy: Equivalent + name: mutation.multiclusterservice.hmc.mirantis.com + rules: + - apiGroups: + - hmc.mirantis.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - multiclusterservices + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 @@ -81,6 +103,28 @@ webhooks: resources: - managedclusters sideEffects: None + - admissionReviewVersions: + - v1 + - v1beta1 + clientConfig: + service: + name: {{ include "hmc.webhook.serviceName" . }} + namespace: {{ include "hmc.webhook.serviceNamespace" . }} + path: /validate-hmc-mirantis-com-v1alpha1-multiclusterservice + failurePolicy: Fail + matchPolicy: Equivalent + name: validation.multiclusterservice.hmc.mirantis.com + rules: + - apiGroups: + - hmc.mirantis.com + apiVersions: + - v1alpha1 + operations: + - CREATE + - UPDATE + resources: + - multiclusterservices + sideEffects: None - admissionReviewVersions: - v1 - v1beta1 diff --git a/test/objects/multiclusterservice/multiclusterservice.go b/test/objects/multiclusterservice/multiclusterservice.go new file mode 100644 index 000000000..cc6aec977 --- /dev/null +++ b/test/objects/multiclusterservice/multiclusterservice.go @@ -0,0 +1,54 @@ +// 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 multiclusterservice + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/Mirantis/hmc/api/v1alpha1" +) + +const ( + DefaultName = "multiclusterservice" +) + +type Opt func(multiClusterService *v1alpha1.MultiClusterService) + +func NewMultiClusterService(opts ...Opt) *v1alpha1.MultiClusterService { + p := &v1alpha1.MultiClusterService{ + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultName, + }, + } + + for _, opt := range opts { + opt(p) + } + return p +} + +func WithName(name string) Opt { + return func(p *v1alpha1.MultiClusterService) { + p.Name = name + } +} + +func WithServiceTemplate(templateName string) Opt { + return func(p *v1alpha1.MultiClusterService) { + p.Spec.Services = append(p.Spec.Services, v1alpha1.ServiceSpec{ + Template: templateName, + }) + } +}