diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go index f017521c..11ecd4fa 100644 --- a/internal/webhook/managedcluster_webhook.go +++ b/internal/webhook/managedcluster_webhook.go @@ -75,18 +75,24 @@ func (v *ManagedClusterValidator) ValidateCreate(ctx context.Context, obj runtim } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type. -func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, _ runtime.Object, newObj runtime.Object) (admission.Warnings, error) { +func (v *ManagedClusterValidator) ValidateUpdate(ctx context.Context, oldObj runtime.Object, newObj runtime.Object) (admission.Warnings, error) { + oldManagedCluster, ok := oldObj.(*v1alpha1.ManagedCluster) + if !ok { + return nil, apierrors.NewBadRequest(fmt.Sprintf("expected ManagedCluster but got a %T", oldObj)) + } newManagedCluster, ok := newObj.(*v1alpha1.ManagedCluster) if !ok { return nil, apierrors.NewBadRequest(fmt.Sprintf("expected ManagedCluster but got a %T", newObj)) } - template, err := v.getTemplate(ctx, newManagedCluster.Spec.Template) - if err != nil { - return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) - } - err = v.isTemplateValid(template) - if err != nil { - return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) + if oldManagedCluster.Spec.Template != newManagedCluster.Spec.Template { + template, err := v.getTemplate(ctx, newManagedCluster.Spec.Template) + if err != nil { + return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) + } + err = v.isTemplateValid(template) + if err != nil { + return nil, fmt.Errorf("%s: %v", InvalidManagedClusterErr, err) + } } return nil, nil } diff --git a/internal/webhook/managedcluster_webhook_test.go b/internal/webhook/managedcluster_webhook_test.go index 333212f5..5df0f3e8 100644 --- a/internal/webhook/managedcluster_webhook_test.go +++ b/internal/webhook/managedcluster_webhook_test.go @@ -274,6 +274,63 @@ cluster-api installation failed: cluster-api template is invalid`, }, err: "the ManagedCluster is invalid: the template is not valid: validation error example", }, + { + name: "update - should succeed if another field than the `spec.template` was changed even if the old template is not valid", + operation: admissionv1.Update, + oldManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"foo"}`), + ), + newManagedCluster: managedcluster.NewManagedCluster( + managedcluster.WithTemplate(testTemplateName), + managedcluster.WithConfig(`{"foo":"bar"}`), + ), + existingObjects: []runtime.Object{ + management.NewManagement( + management.WithAvailableProviders(v1alpha1.Providers{ + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + management.WithComponentsStatus(map[string]v1alpha1.ComponentStatus{ + v1alpha1.DefaultCoreHMCTemplate: {Success: false}, + v1alpha1.DefaultCoreCAPITemplate: {Success: true}, + }), + ), + template.NewTemplate( + template.WithName(capzTemplateName), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: false}), + ), + template.NewTemplate( + template.WithName(newTemplateName), + template.WithTypeStatus(v1alpha1.TemplateTypeDeployment), + template.WithProvidersStatus(v1alpha1.Providers{ + InfrastructureProviders: []string{"aws"}, + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: false}), + ), + }, + }, + { + name: "update - should succeed", + operation: admissionv1.Update, + oldManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(testTemplateName)), + newManagedCluster: managedcluster.NewManagedCluster(managedcluster.WithTemplate(newTemplateName)), + existingObjects: []runtime.Object{ + mgmt, + template.NewTemplate( + template.WithName(newTemplateName), + template.WithTypeStatus(v1alpha1.TemplateTypeDeployment), + template.WithProvidersStatus(v1alpha1.Providers{ + InfrastructureProviders: []string{"aws"}, + BootstrapProviders: []string{"k0s"}, + ControlPlaneProviders: []string{"k0s"}, + }), + template.WithValidationStatus(v1alpha1.TemplateValidationStatus{Valid: true}), + ), + }, + }, } for _, tt := range tests {