Validation for MultiClusterService and some additional testing

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 {

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

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())

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())

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())
 

internal/webhook/managedcluster_webhook.go

@@ -67,8 +67,8 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim
 		return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err)
 	}
 
-	if err := isTemplateValid(template); err != nil {
-		return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err)
+	if err := isTemplateValid(template.GetCommonStatus()); err != nil {
+		return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err)
 	}
 
 	if err := validateK8sCompatibility(ctx, v.Client, template, managedCluster); err != nil {
@@ -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: %v", invalidManagedClusterMsg, err)
+		}
+
+		if err := isTemplateValid(tpl.GetCommonStatus()); err != nil {
+			return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err)
+		}
+	}
+
 	return nil, nil
 }
 
@@ -106,8 +118,8 @@ func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj, ne
 			return admission.Warnings{msg}, errClusterUpgradeForbidden
 		}
 
-		if err := isTemplateValid(template); err != nil {
-			return nil, fmt.Errorf("%s: %w", invalidManagedClusterMsg, err)
+		if err := isTemplateValid(template.GetCommonStatus()); err != nil {
+			return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err)
 		}
 
 		if err := validateK8sCompatibility(ctx, v.Client, template, newManagedCluster); err != nil {
@@ -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: %v", invalidManagedClusterMsg, err)
+		}
+
+		if err := isTemplateValid(tpl.GetCommonStatus()); err != nil {
+			return nil, fmt.Errorf("%s: %v", invalidManagedClusterMsg, err)
+		}
+	}
+
 	return nil, nil
 }
 
@@ -185,8 +209,8 @@ 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 {
-		return fmt.Errorf("template is invalid: %w", err)
+	if err := isTemplateValid(template.GetCommonStatus()); err != nil {
+		return fmt.Errorf("template is invalid: %v", err)
 	}
 
 	if template.Status.Config == nil {
@@ -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