Check ManagedCluster template validness only in case it was changed

Closes #244
Mirantis · Sep 10, 2024 · 8f2caaa · 8f2caaa
1 parent 5a0cc4a
commit 8f2caaa
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 8 deletions.
diff --git a/internal/webhook/managedcluster_webhook.go b/internal/webhook/managedcluster_webhook.go
@@ -77,18 +77,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
@@ -275,6 +275,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 {