Allow Templates deletion by the controller only

The admission controller will block the removal of any Template
managed by HMC or from the system namespace if the request is not
made by the HMC controller.

internal/controller/suite_test.go

@@ -29,10 +29,12 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	admissionv1 "k8s.io/api/admissionregistration/v1"
+	authenticationv1 "k8s.io/api/authentication/v1"
 	utilyaml "sigs.k8s.io/cluster-api/util/yaml"
 	ctrl "sigs.k8s.io/controller-runtime"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
 	"sigs.k8s.io/controller-runtime/pkg/webhook"
+	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	helmcontrollerv2 "github.com/fluxcd/helm-controller/api/v2"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
@@ -54,6 +56,8 @@ import (
 const (
 	mutatingWebhookKind   = "MutatingWebhookConfiguration"
 	validatingWebhookKind = "ValidatingWebhookConfiguration"
+
+	hmcServiceAccountName = "hmc-controller-manager"
 )
 
 var (
@@ -62,6 +66,8 @@ var (
 	testEnv   *envtest.Environment
 	ctx       context.Context
 	cancel    context.CancelFunc
+
+	userInfo = authenticationv1.UserInfo{Username: fmt.Sprintf("system:serviceaccount:hmc-system:%s", hmcServiceAccountName)}
 )
 
 func TestControllers(t *testing.T) {
@@ -82,6 +88,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"),
@@ -153,13 +162,19 @@ var _ = BeforeSuite(func() {
 	err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr)
 	Expect(err).NotTo(HaveOccurred())
 
-	err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr)
+	templateValidator := hmcwebhook.TemplateValidator{
+		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() {
@@ -185,6 +200,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) {

internal/controller/template_controller.go

@@ -36,6 +36,7 @@ import (
 
 	hmc "github.com/Mirantis/hmc/api/v1alpha1"
 	"github.com/Mirantis/hmc/internal/helm"
+	"github.com/Mirantis/hmc/internal/templateutil"
 )
 
 const (
@@ -123,14 +124,7 @@ func (r *ProviderTemplateReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	return r.ReconcileTemplate(ctx, providerTemplate)
 }
 
-// Template is the interface defining a list of methods to interact with templates
-type Template interface {
-	client.Object
-	GetSpec() *hmc.TemplateSpecCommon
-	GetStatus() *hmc.TemplateStatusCommon
-}
-
-func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template Template) (ctrl.Result, error) {
+func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template templateutil.Template) (ctrl.Result, error) {
 	l := log.FromContext(ctx)
 
 	spec := template.GetSpec()
@@ -149,7 +143,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template Tem
 			l.Error(err, "invalid helm chart reference")
 			return ctrl.Result{}, err
 		}
-		if template.GetNamespace() == r.SystemNamespace || !templateManagedByHMC(template) {
+		if template.GetNamespace() == r.SystemNamespace || !templateutil.IsManagedByHMC(template) {
 			err := r.reconcileDefaultHelmRepository(ctx, template.GetNamespace())
 			if err != nil {
 				l.Error(err, "Failed to reconcile default HelmRepository", "namespace", template.GetNamespace())
@@ -222,11 +216,7 @@ func (r *TemplateReconciler) ReconcileTemplate(ctx context.Context, template Tem
 	return ctrl.Result{}, r.updateStatus(ctx, template, "")
 }
 
-func templateManagedByHMC(template Template) bool {
-	return template.GetLabels()[hmc.HMCManagedLabelKey] == hmc.HMCManagedLabelValue
-}
-
-func parseChartMetadata(template Template, inChart *chart.Chart) error {
+func parseChartMetadata(template templateutil.Template, inChart *chart.Chart) error {
 	if inChart.Metadata == nil {
 		return fmt.Errorf("chart metadata is empty")
 	}
@@ -261,7 +251,7 @@ func parseChartMetadata(template Template, inChart *chart.Chart) error {
 	return nil
 }
 
-func (r *TemplateReconciler) updateStatus(ctx context.Context, template Template, validationError string) error {
+func (r *TemplateReconciler) updateStatus(ctx context.Context, template templateutil.Template, validationError string) error {
 	status := template.GetStatus()
 	status.ObservedGeneration = template.GetGeneration()
 	status.ValidationError = validationError
@@ -312,7 +302,7 @@ func (r *TemplateReconciler) reconcileDefaultHelmRepository(ctx context.Context,
 	return nil
 }
 
-func (r *TemplateReconciler) reconcileHelmChart(ctx context.Context, template Template) (*sourcev1.HelmChart, error) {
+func (r *TemplateReconciler) reconcileHelmChart(ctx context.Context, template templateutil.Template) (*sourcev1.HelmChart, error) {
 	spec := template.GetSpec()
 	namespace := template.GetNamespace()
 	if namespace == "" {

internal/controller/template_controller_test.go

@@ -170,7 +170,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())
 		})
 		It("should successfully reconcile the resource", func() {

internal/templateutil/interface.go

@@ -0,0 +1,36 @@
+// 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 templateutil
+
+import (
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	hmc "github.com/Mirantis/hmc/api/v1alpha1"
+)
+
+// Template is the interface defining a list of methods to interact with templates
+type Template interface {
+	client.Object
+	GetSpec() *hmc.TemplateSpecCommon
+	GetStatus() *hmc.TemplateStatusCommon
+}
+
+func IsManagedByHMC(template Template) bool {
+	labels := template.GetLabels()
+	if labels == nil {
+		return false
+	}
+	return labels[hmc.HMCManagedLabelKey] == hmc.HMCManagedLabelValue
+}

internal/webhook/template_webhook.go

@@ -18,6 +18,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"os"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/fields"
@@ -28,13 +29,21 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
 
 	"github.com/Mirantis/hmc/api/v1alpha1"
+	"github.com/Mirantis/hmc/internal/templateutil"
 )
 
-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()
@@ -66,14 +75,21 @@ 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))
 	}
+	deletionAllowed, 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 !deletionAllowed {
+		return nil, errTemplateDeletionForbidden
+	}
 
 	managedClusters := &v1alpha1.ManagedClusterList{}
 	listOptions := client.ListOptions{
 		FieldSelector: fields.SelectorFromSet(fields.Set{v1alpha1.TemplateKey: template.Name}),
 		Limit:         1,
 		Namespace:     template.Namespace,
 	}
-	err := v.Client.List(ctx, managedClusters, &listOptions)
+	err = v.Client.List(ctx, managedClusters, &listOptions)
 	if err != nil {
 		return nil, err
 	}
@@ -91,15 +107,15 @@ func (*ClusterTemplateValidator) Default(context.Context, runtime.Object) error
 }
 
 type ServiceTemplateValidator struct {
-	client.Client
+	TemplateValidator
 }
 
-func (in *ServiceTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	in.Client = mgr.GetClient()
+func (v *ServiceTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	v.Client = mgr.GetClient()
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ServiceTemplate{}).
-		WithValidator(in).
-		WithDefaulter(in).
+		WithValidator(v).
+		WithDefaulter(v).
 		Complete()
 }
 
@@ -119,7 +135,18 @@ func (*ServiceTemplateValidator) ValidateUpdate(_ context.Context, _ runtime.Obj
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (*ServiceTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+func (v *ServiceTemplateValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	template, ok := obj.(*v1alpha1.ServiceTemplate)
+	if !ok {
+		return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected ServiceTemplate but got a %T", obj))
+	}
+	deletionAllowed, 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 !deletionAllowed {
+		return nil, errTemplateDeletionForbidden
+	}
 	return nil, nil
 }
 
@@ -129,15 +156,15 @@ func (*ServiceTemplateValidator) Default(_ context.Context, _ runtime.Object) er
 }
 
 type ProviderTemplateValidator struct {
-	client.Client
+	TemplateValidator
 }
 
-func (in *ProviderTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
-	in.Client = mgr.GetClient()
+func (v *ProviderTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
+	v.Client = mgr.GetClient()
 	return ctrl.NewWebhookManagedBy(mgr).
 		For(&v1alpha1.ProviderTemplate{}).
-		WithValidator(in).
-		WithDefaulter(in).
+		WithValidator(v).
+		WithDefaulter(v).
 		Complete()
 }
 
@@ -157,11 +184,45 @@ func (*ProviderTemplateValidator) ValidateUpdate(_ context.Context, _ runtime.Ob
 }
 
 // ValidateDelete implements webhook.Validator so a webhook will be registered for the type.
-func (*ProviderTemplateValidator) ValidateDelete(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+func (v *ProviderTemplateValidator) ValidateDelete(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	template, ok := obj.(*v1alpha1.ProviderTemplate)
+	if !ok {
+		return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected ProviderTemplate but got a %T", obj))
+	}
+	deletionAllowed, 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 nil, errTemplateDeletionForbidden
+	}
 	return nil, nil
 }
 
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (*ProviderTemplateValidator) Default(_ context.Context, _ runtime.Object) error {
 	return nil
 }
+
+func (v TemplateValidator) isTemplateDeletionAllowed(ctx context.Context, template templateutil.Template) (bool, error) {
+	req, err := admission.RequestFromContext(ctx)
+	if err != nil {
+		return false, err
+	}
+	if v.InjectUserInfo != nil {
+		v.InjectUserInfo(&req)
+	}
+	// Allow all templates' deletion for the HMC controller
+	if serviceAccountIsEqual(req, os.Getenv(ServiceAccountEnvName)) {
+		return true, nil
+	}
+	// Cluster-scoped ProviderTemplates and Templates from the system namespace are not allowed to be deleted
+	if template.GetNamespace() == "" || template.GetNamespace() == v.SystemNamespace {
+		return false, nil
+	}
+	// Allow template deletion if template is not managed by the TemplateManagement
+	if !templateutil.IsManagedByHMC(template) {
+		return true, nil
+	}
+	return false, nil
+}