Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Templates deletion validation #355

Merged
merged 1 commit into from
Dec 6, 2024
+381 −63
Merged

Improve Templates deletion validation #355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions cmd/main.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading