Improve Templates deletion validation

The following rules are validated: 1. ClusterTemplate or ServiceTemplate can't be removed if it is in use by ManagedCluster 2. ClusterTemplate or ServiceTemplate can't be removed if the request was triggered by user and the template is managed by the TemplateManagement 3. ProviderTemplate can't be removed if it's a core provider or enabled in Management spec.providers
Mirantis · Nov 12, 2024 · 220b985 · 220b985
1 parent 55bf573
commit 220b985
Show file tree

Hide file tree

Showing 14 changed files with 562 additions and 54 deletions.
diff --git a/api/v1alpha1/indexers.go b/api/v1alpha1/indexers.go
@@ -122,14 +122,14 @@ func extractReleaseTemplates(rawObj client.Object) []string {
 const TemplateChainSupportedTemplatesIndexKey = ".spec.supportedTemplates[].Name"
 
 func setupClusterTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error {
-	return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, extractSupportedTemplatesNames)
+	return mgr.GetFieldIndexer().IndexField(ctx, &ClusterTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, ExtractSupportedTemplatesNamesFromTemplateChain)
 }
 
 func setupServiceTemplateChainIndexer(ctx context.Context, mgr ctrl.Manager) error {
-	return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, extractSupportedTemplatesNames)
+	return mgr.GetFieldIndexer().IndexField(ctx, &ServiceTemplateChain{}, TemplateChainSupportedTemplatesIndexKey, ExtractSupportedTemplatesNamesFromTemplateChain)
 }
 
-func extractSupportedTemplatesNames(rawObj client.Object) []string {
+func ExtractSupportedTemplatesNamesFromTemplateChain(rawObj client.Object) []string {
 	chainSpec := TemplateChainSpec{}
 	switch chain := rawObj.(type) {
 	case *ClusterTemplateChain:

diff --git a/api/v1alpha1/management_types.go b/api/v1alpha1/management_types.go
@@ -75,6 +75,35 @@ func (in *Component) HelmValues() (values map[string]any, err error) {
 	return values, err
 }
 
+// Templates returns the templates for all enabled components.
+// If a template is not specified in Management, it retrieves the default from the Release object.
+func (in *Management) Templates(release *Release) []string {
+	templates := make([]string, 0, len(in.Spec.Providers)+2)
+	if core := in.Spec.Core; core != nil {
+		if core.HMC.Template != "" {
+			templates = append(templates, core.HMC.Template)
+		} else {
+			templates = append(templates, release.Spec.HMC.Template)
+		}
+		if core.CAPI.Template != "" {
+			templates = append(templates, core.CAPI.Template)
+		} else {
+			templates = append(templates, release.Spec.CAPI.Template)
+		}
+	} else {
+		templates = append(templates, release.Spec.HMC.Template, release.Spec.CAPI.Template)
+	}
+
+	for _, p := range in.Spec.Providers {
+		if p.Template != "" {
+			templates = append(templates, p.Template)
+		} else {
+			templates = append(templates, release.ProviderTemplate(p.Name))
+		}
+	}
+	return templates
+}
+
 func GetDefaultProviders() []Provider {
 	return []Provider{
 		{Name: ProviderK0smotronName},

diff --git a/cmd/main.go b/cmd/main.go
@@ -352,15 +352,19 @@ func setupWebhooks(mgr ctrl.Manager, currentNamespace string) error {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplateChain")
 		return err
 	}
-	if err := (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil {
+
+	templateValidator := hmcwebhook.TemplateValidator{
+		SystemNamespace: currentNamespace,
+	}
+	if err := (&hmcwebhook.ClusterTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ClusterTemplate")
 		return err
 	}
-	if err := (&hmcwebhook.ServiceTemplateValidator{SystemNamespace: currentNamespace}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&hmcwebhook.ServiceTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ServiceTemplate")
 		return err
 	}
-	if err := (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr); err != nil {
+	if err := (&hmcwebhook.ProviderTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr); err != nil {
 		setupLog.Error(err, "unable to create webhook", "webhook", "ProviderTemplate")
 		return err
 	}

diff --git a/go.mod b/go.mod
@@ -24,6 +24,7 @@ require (
 	k8s.io/api v0.31.2
 	k8s.io/apiextensions-apiserver v0.31.2
 	k8s.io/apimachinery v0.31.2
+	k8s.io/apiserver v0.31.2
 	k8s.io/client-go v0.31.2
 	k8s.io/utils v0.0.0-20240921022957-49e7df575cb6
 	sigs.k8s.io/cluster-api v1.8.5
@@ -186,7 +187,6 @@ require (
 	gopkg.in/evanphx/json-patch.v4 v4.12.0 // indirect
 	gopkg.in/inf.v0 v0.9.1 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect
-	k8s.io/apiserver v0.31.2 // indirect
 	k8s.io/cli-runtime v0.31.2 // indirect
 	k8s.io/cluster-bootstrap v0.31.1 // indirect
 	k8s.io/component-base v0.31.2 // indirect

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -32,6 +32,7 @@ import (
 	. "github.com/onsi/gomega"
 	sveltosv1beta1 "github.com/projectsveltos/addon-controller/api/v1beta1"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
+	authenticationv1 "k8s.io/api/authentication/v1"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/client-go/rest"
@@ -43,6 +44,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	hmcmirantiscomv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
 	hmcwebhook "github.com/Mirantis/hmc/internal/webhook"
@@ -65,6 +67,10 @@ var (
 	testEnv       *envtest.Environment
 	ctx           context.Context
 	cancel        context.CancelFunc
+
+	hmcServiceAccountName = "hmc-controller-manager"
+
+	userInfo = authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:%s:%s", testSystemNamespace, hmcServiceAccountName)}
 )
 
 func TestControllers(t *testing.T) {
@@ -85,6 +91,9 @@ var _ = BeforeSuite(func() {
 	)
 	Expect(err).NotTo(HaveOccurred())
 
+	err = os.Setenv(hmcwebhook.ServiceAccountEnvName, hmcServiceAccountName)
+	Expect(err).To(Succeed())
+
 	testEnv = &envtest.Environment{
 		CRDDirectoryPaths: []string{
 			filepath.Join("..", "..", "templates", "provider", "hmc", "templates", "crds"),
@@ -167,13 +176,20 @@ var _ = BeforeSuite(func() {
 	err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr)
+	templateValidator := hmcwebhook.TemplateValidator{
+		SystemNamespace: testSystemNamespace,
+		InjectUserInfo: func(req *admission.Request) {
+			req.UserInfo = userInfo
+		},
+	}
+
+	err = (&hmcwebhook.ClusterTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ServiceTemplateValidator{SystemNamespace: testSystemNamespace}).SetupWebhookWithManager(mgr)
+	err = (&hmcwebhook.ServiceTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ProviderTemplateValidator{}).SetupWebhookWithManager(mgr)
+	err = (&hmcwebhook.ProviderTemplateValidator{TemplateValidator: templateValidator}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
 	go func() {
@@ -199,6 +215,9 @@ var _ = AfterSuite(func() {
 	cancel()
 	err := testEnv.Stop()
 	Expect(err).NotTo(HaveOccurred())
+
+	err = os.Unsetenv(hmcwebhook.ServiceAccountEnvName)
+	Expect(err).To(Succeed())
 })
 
 func loadWebhooks(path string) ([]*admissionv1.ValidatingWebhookConfiguration, []*admissionv1.MutatingWebhookConfiguration, error) {

diff --git a/internal/controller/template_controller_test.go b/internal/controller/template_controller_test.go
@@ -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
@@ -231,7 +231,7 @@ var _ = Describe("Template Chain Controller", func() {
 
 				Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, clusterTemplateChainResource).Should(HaveOccurred())
 			}
-			for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct2"]} {
+			for _, template := range []*hmcmirantiscomv1alpha1.ClusterTemplate{ctTemplates["test"], ctTemplates["ct0"], ctTemplates["ct1"], ctTemplates["ct2"]} {
 				Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed())
 			}
 
@@ -251,7 +251,7 @@ var _ = Describe("Template Chain Controller", func() {
 
 				Eventually(k8sClient.Get, 1*time.Minute, 5*time.Second).WithArguments(ctx, chain, serviceTemplateChainResource).Should(HaveOccurred())
 			}
-			for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"], stTemplates["st2"]} {
+			for _, template := range []*hmcmirantiscomv1alpha1.ServiceTemplate{stTemplates["test"], stTemplates["st0"], stTemplates["st1"], stTemplates["st2"]} {
 				Expect(crclient.IgnoreNotFound(k8sClient.Delete(ctx, template))).To(Succeed())
 			}