Skip to content

Commit

Permalink
Improve Templates deletion validation
Browse files Browse the repository at this point in the history
The following rules are validated:
1. ClusterTemplate or ServiceTemplate can't be removed if it is in use by
   ManagedCluster or owned by TemplateChain
2. ProviderTemplate can't be removed if it's a core provider or enabled in
   Management spec.providers
  • Loading branch information
eromanova authored and Kshatrix committed Dec 6, 2024
1 parent b35b0da commit c08df7b
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 63 deletions.
10 changes: 7 additions & 3 deletions cmd/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
10 changes: 7 additions & 3 deletions internal/controller/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,13 +169,17 @@ var _ = BeforeSuite(func() {
err = (&hmcwebhook.ServiceTemplateChainValidator{}).SetupWebhookWithManager(mgr)
Expect(err).NotTo(HaveOccurred())

err = (&hmcwebhook.ClusterTemplateValidator{}).SetupWebhookWithManager(mgr)
templateValidator := hmcwebhook.TemplateValidator{
SystemNamespace: testSystemNamespace,
}

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() {
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/template_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})

Expand Down
33 changes: 23 additions & 10 deletions internal/webhook/release_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package webhook

import (
"context"
"errors"
"fmt"
"slices"
"strings"
Expand All @@ -30,6 +31,8 @@ import (
hmcv1alpha1 "github.com/Mirantis/hmc/api/v1alpha1"
)

var errManagementIsNotFound = errors.New("no Management object found")

type ReleaseValidator struct {
client.Client
}
Expand Down Expand Up @@ -61,18 +64,14 @@ func (v *ReleaseValidator) ValidateDelete(ctx context.Context, obj runtime.Objec
if !ok {
return admission.Warnings{"Wrong object"}, apierrors.NewBadRequest(fmt.Sprintf("expected Release but got a %T", obj))
}
mgmtList := &hmcv1alpha1.ManagementList{}
if err := v.List(ctx, mgmtList); err != nil {

mgmt, err := getManagement(ctx, v.Client)
if err != nil {
if errors.Is(err, errManagementIsNotFound) {
return nil, nil
}
return nil, err
}
if len(mgmtList.Items) == 0 {
return nil, nil
}
if len(mgmtList.Items) > 1 {
return nil, fmt.Errorf("expected 1 Management object, got %d", len(mgmtList.Items))
}

mgmt := mgmtList.Items[0]
if mgmt.Spec.Release == release.Name {
return nil, fmt.Errorf("release %s is still in use", release.Name)
}
Expand All @@ -89,3 +88,17 @@ func (v *ReleaseValidator) ValidateDelete(ctx context.Context, obj runtime.Objec
}
return nil, nil
}

func getManagement(ctx context.Context, cl client.Client) (*hmcv1alpha1.Management, error) {
mgmtList := &hmcv1alpha1.ManagementList{}
if err := cl.List(ctx, mgmtList); err != nil {
return nil, err
}
if len(mgmtList.Items) == 0 {
return nil, errManagementIsNotFound
}
if len(mgmtList.Items) > 1 {
return nil, fmt.Errorf("expected 1 Management object, got %d", len(mgmtList.Items))
}
return &mgmtList.Items[0], nil
}
116 changes: 95 additions & 21 deletions internal/webhook/template_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import (
"context"
"errors"
"fmt"
"slices"
"strings"

apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
Expand All @@ -29,14 +31,23 @@ 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
templateKind string
templateChainKind string
}

var errTemplateDeletionForbidden = errors.New("template deletion is forbidden")
type ClusterTemplateValidator struct {
TemplateValidator
}

func (v *ClusterTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
v.Client = mgr.GetClient()
v.templateKind = v1alpha1.ClusterTemplateKind
v.templateChainKind = v1alpha1.ClusterTemplateChainKind
return ctrl.NewWebhookManagedBy(mgr).
For(&v1alpha1.ClusterTemplate{}).
WithValidator(v).
Expand Down Expand Up @@ -66,16 +77,18 @@ func (v *ClusterTemplateValidator) ValidateDelete(ctx context.Context, obj runti
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.ManagedClusterTemplateIndexKey: template.Name},
client.Limit(1)); err != nil {
inUseByCluster, err := v.templateIsInUseByCluster(ctx, template)
if err != nil {
return nil, err
}
if inUseByCluster {
return admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, errTemplateDeletionForbidden
}

if len(managedClusters.Items) > 0 {
return admission.Warnings{"The ClusterTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden
owners := getOwnersWithKind(template, v.templateChainKind)
if len(owners) > 0 {
return admission.Warnings{fmt.Sprintf("The %s object can't be removed if it is managed by %s: %s",
v.templateKind, v.templateChainKind, strings.Join(owners, ", "))}, errTemplateDeletionForbidden
}

return nil, nil
Expand All @@ -87,12 +100,13 @@ func (*ClusterTemplateValidator) Default(context.Context, runtime.Object) error
}

type ServiceTemplateValidator struct {
client.Client
SystemNamespace string
TemplateValidator
}

func (v *ServiceTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
v.Client = mgr.GetClient()
v.templateKind = v1alpha1.ServiceTemplateKind
v.templateChainKind = v1alpha1.ServiceTemplateChainKind
return ctrl.NewWebhookManagedBy(mgr).
For(&v1alpha1.ServiceTemplate{}).
WithValidator(v).
Expand Down Expand Up @@ -122,16 +136,18 @@ func (v *ServiceTemplateValidator) ValidateDelete(ctx context.Context, obj runti
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.ManagedClusterServiceTemplatesIndexKey: tmpl.Name},
client.Limit(1)); err != nil {
return nil, err
inUseByCluster, err := v.templateIsInUseByCluster(ctx, tmpl)
if err != nil {
return nil, fmt.Errorf("failed to check if the ServiceTemplate %s/%s is in use: %w", tmpl.Namespace, tmpl.Name, err)
}
if inUseByCluster {
return admission.Warnings{fmt.Sprintf("The %s object can't be removed if ManagedCluster objects referencing it still exist", v.templateKind)}, errTemplateDeletionForbidden
}

if len(managedClusters.Items) > 0 {
return admission.Warnings{"The ServiceTemplate object can't be removed if ManagedCluster objects referencing it still exist"}, errTemplateDeletionForbidden
owners := getOwnersWithKind(tmpl, v.templateChainKind)
if len(owners) > 0 {
return admission.Warnings{fmt.Sprintf("The %s object can't be removed if it is managed by %s: %s",
v.templateKind, v.templateChainKind, strings.Join(owners, ", "))}, errTemplateDeletionForbidden
}

// MultiClusterServices can only refer to serviceTemplates in system namespace.
Expand All @@ -157,11 +173,12 @@ func (*ServiceTemplateValidator) Default(_ context.Context, _ runtime.Object) er
}

type ProviderTemplateValidator struct {
client.Client
TemplateValidator
}

func (v *ProviderTemplateValidator) SetupWebhookWithManager(mgr ctrl.Manager) error {
v.Client = mgr.GetClient()
v.templateKind = v1alpha1.ProviderTemplateKind
return ctrl.NewWebhookManagedBy(mgr).
For(&v1alpha1.ProviderTemplate{}).
WithValidator(v).
Expand All @@ -185,11 +202,68 @@ 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))
}

owners := getOwnersWithKind(template, v1alpha1.ReleaseKind)
if len(owners) > 0 {
return admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is part of existing Releases: %s",
template.GetName(), strings.Join(owners, ", "))}, errTemplateDeletionForbidden
}

mgmt, err := getManagement(ctx, v.Client)
if err != nil {
if errors.Is(err, errManagementIsNotFound) {
return nil, nil
}
return nil, err
}
if slices.Contains(mgmt.Templates(), template.Name) {
return admission.Warnings{fmt.Sprintf("The ProviderTemplate %s cannot be removed while it is used in the Management spec",
template.GetName())}, 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) templateIsInUseByCluster(ctx context.Context, template client.Object) (bool, error) {
var key string

switch v.templateKind {
case v1alpha1.ClusterTemplateKind:
key = v1alpha1.ManagedClusterTemplateIndexKey
case v1alpha1.ServiceTemplateKind:
key = v1alpha1.ManagedClusterServiceTemplatesIndexKey
default:
return false, fmt.Errorf("invalid Template kind %s. Supported values are: %s and %s", v.templateKind, 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 getOwnersWithKind(template client.Object, kind string) []string {
var owners []string
for _, ownerRef := range template.GetOwnerReferences() {
if ownerRef.Kind == kind {
owners = append(owners, ownerRef.Name)
}
}
return owners
}
Loading

0 comments on commit c08df7b

Please sign in to comment.