Validate Management creation/update with Release

Signed-off-by: Andrei Pavlov <[email protected]>
k0rdent · Dec 2, 2024 · 81b8ce0 · 81b8ce0
1 parent d49e006
commit 81b8ce0
Show file tree

Hide file tree

Showing 3 changed files with 112 additions and 3 deletions.
diff --git a/internal/webhook/management_webhook.go b/internal/webhook/management_webhook.go
@@ -53,7 +53,17 @@ var (
 )
 
 // ValidateCreate implements webhook.Validator so a webhook will be registered for the type.
-func (*ManagementValidator) ValidateCreate(_ context.Context, _ runtime.Object) (admission.Warnings, error) {
+func (v *ManagementValidator) ValidateCreate(ctx context.Context, obj runtime.Object) (admission.Warnings, error) {
+	mgmt, ok := obj.(*hmcv1alpha1.Management)
+	if !ok {
+		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected Management but got a %T", obj))
+	}
+	if err := validateRelease(ctx, v.Client, mgmt.Spec.Release); err != nil {
+		return nil,
+			apierrors.NewInvalid(mgmt.GroupVersionKind().GroupKind(), mgmt.Name, field.ErrorList{
+				field.Forbidden(field.NewPath("spec", "release"), err.Error()),
+			})
+	}
 	return nil, nil
 }
 
@@ -71,6 +81,15 @@ func (v *ManagementValidator) ValidateUpdate(ctx context.Context, oldObj, newObj
 		return nil, apierrors.NewBadRequest(fmt.Sprintf("expected Management but got a %T", oldObj))
 	}
 
+	if oldMgmt.Spec.Release != newMgmt.Spec.Release {
+		if err := validateRelease(ctx, v.Client, newMgmt.Spec.Release); err != nil {
+			return nil,
+				apierrors.NewInvalid(newMgmt.GroupVersionKind().GroupKind(), newMgmt.Name, field.ErrorList{
+					field.Forbidden(field.NewPath("spec", "release"), err.Error()),
+				})
+		}
+	}
+
 	if err := checkComponentsRemoval(ctx, v.Client, oldMgmt, newMgmt); err != nil {
 		return admission.Warnings{"Some of the providers cannot be removed"},
 			apierrors.NewInvalid(newMgmt.GroupVersionKind().GroupKind(), newMgmt.Name, field.ErrorList{
@@ -234,6 +253,17 @@ func (v *ManagementValidator) ValidateDelete(ctx context.Context, _ runtime.Obje
 	return nil, nil
 }
 
+func validateRelease(ctx context.Context, cl client.Client, releaseName string) error {
+	release := &hmcv1alpha1.Release{}
+	if err := cl.Get(ctx, client.ObjectKey{Name: releaseName}, release); err != nil {
+		return err
+	}
+	if !release.Status.Ready {
+		return fmt.Errorf("release \"%s\" status is not ready", releaseName)
+	}
+	return nil
+}
+
 // Default implements webhook.Defaulter so a webhook will be registered for the type.
 func (*ManagementValidator) Default(_ context.Context, _ runtime.Object) error {
 	return nil

diff --git a/internal/webhook/management_webhook_test.go b/internal/webhook/management_webhook_test.go
@@ -33,6 +33,62 @@ import (
 	"github.com/Mirantis/hmc/test/scheme"
 )
 
+func TestManagementValidateCreate(t *testing.T) {
+	g := NewWithT(t)
+
+	ctx := admission.NewContextWithRequest(context.Background(), admission.Request{AdmissionRequest: admissionv1.AdmissionRequest{Operation: admissionv1.Create}})
+
+	tests := []struct {
+		name            string
+		management      *v1alpha1.Management
+		existingObjects []runtime.Object
+		err             string
+		warnings        admission.Warnings
+	}{
+		{
+			name: "release is not ready, should fail",
+			management: management.NewManagement(
+				management.WithRelease(release.DefaultName),
+			),
+			existingObjects: []runtime.Object{
+				release.New(
+					release.WithName(release.DefaultName),
+					release.WithReadyStatus(false),
+				),
+			},
+			err: fmt.Sprintf(`Management "%s" is invalid: spec.release: Forbidden: release "%s" status is not ready`, management.DefaultName, release.DefaultName),
+		},
+		{
+			name: "should succeed",
+			management: management.NewManagement(
+				management.WithRelease(release.DefaultName),
+			),
+			existingObjects: []runtime.Object{
+				release.New(
+					release.WithName(release.DefaultName),
+				),
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(_ *testing.T) {
+			c := fake.NewClientBuilder().WithScheme(scheme.Scheme).WithRuntimeObjects(tt.existingObjects...).Build()
+			validator := &ManagementValidator{Client: c}
+
+			warn, err := validator.ValidateCreate(ctx, tt.management)
+			if tt.err != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err).To(MatchError(tt.err))
+			} else {
+				g.Expect(err).To(Succeed())
+			}
+
+			g.Expect(warn).To(Equal(tt.warnings))
+		})
+	}
+}
+
 func TestManagementValidateUpdate(t *testing.T) {
 	g := NewWithT(t)
 
@@ -78,8 +134,7 @@ func TestManagementValidateUpdate(t *testing.T) {
 				management.WithProviders(),
 				management.WithRelease(release.DefaultName),
 			),
-			warnings: admission.Warnings{"Some of the providers cannot be removed"},
-			err:      fmt.Sprintf(`Management "%s" is invalid: spec.providers: Forbidden: failed to get Release %s: releases.hmc.mirantis.com "%s" not found`, management.DefaultName, release.DefaultName, release.DefaultName),
+			err: fmt.Sprintf(`Management "%s" is invalid: spec.release: Forbidden: releases.hmc.mirantis.com "%s" not found`, management.DefaultName, release.DefaultName),
 		},
 		{
 			name: "removed provider does not have related providertemplate, should fail",
@@ -293,6 +348,21 @@ func TestManagementValidateUpdate(t *testing.T) {
 				),
 			},
 		},
+		{
+			name: "release is not ready, should fail",
+			oldMgmt: management.NewManagement(
+				management.WithRelease("old-release"),
+			),
+			management: management.NewManagement(
+				management.WithRelease(release.DefaultName),
+			),
+			existingObjects: []runtime.Object{
+				release.New(
+					release.WithReadyStatus(false),
+				),
+			},
+			err: fmt.Sprintf(`Management "%s" is invalid: spec.release: Forbidden: release "%s" status is not ready`, management.DefaultName, release.DefaultName),
+		},
 	}
 
 	for _, tt := range tests {

diff --git a/test/objects/release/release.go b/test/objects/release/release.go
@@ -42,6 +42,9 @@ func New(opts ...Opt) *v1alpha1.Release {
 				Template: DefaultCAPITemplateName,
 			},
 		},
+		Status: v1alpha1.ReleaseStatus{
+			Ready: true,
+		},
 	}
 
 	for _, opt := range opts {
@@ -68,3 +71,9 @@ func WithCAPITemplateName(v string) Opt {
 		r.Spec.CAPI.Template = v
 	}
 }
+
+func WithReadyStatus(ready bool) Opt {
+	return func(r *v1alpha1.Release) {
+		r.Status.Ready = ready
+	}
+}