diff --git a/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook.go b/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook.go index 237d8f5..d68d09d 100644 --- a/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook.go +++ b/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook.go @@ -18,11 +18,13 @@ package v1beta2 import ( "context" + "fmt" "reflect" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/util/validation/field" + "sigs.k8s.io/cluster-api/util/topology" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" @@ -88,18 +90,26 @@ func (v *CloudscaleMachineTemplateCustomValidator) ValidateCreate(_ context.Cont } // ValidateUpdate implements webhook.CustomValidator so a webhook will be registered for the type CloudscaleMachineTemplate. -func (v *CloudscaleMachineTemplateCustomValidator) ValidateUpdate(_ context.Context, oldObj, newObj *infrastructurev1beta2.CloudscaleMachineTemplate) (admission.Warnings, error) { +func (v *CloudscaleMachineTemplateCustomValidator) ValidateUpdate(ctx context.Context, oldObj, newObj *infrastructurev1beta2.CloudscaleMachineTemplate) (admission.Warnings, error) { cloudscalemachinetemplatelog.Info("Validation for CloudscaleMachineTemplate upon update", "name", newObj.GetName()) - // MachineTemplate spec is fully immutable (CAPI convention). - if !reflect.DeepEqual(newObj.Spec.Template.Spec, oldObj.Spec.Template.Spec) { - var allErrs = make(field.ErrorList, 0, 1) - allErrs = append(allErrs, field.Forbidden( - field.NewPath("spec", "template", "spec"), - "field is immutable")) - return nil, apierrors.NewInvalid( - schema.GroupKind{Group: infrastructurev1beta2.SchemeGroupVersion.Group, Kind: "CloudscaleMachineTemplate"}, - newObj.Name, allErrs) + req, err := admission.RequestFromContext(ctx) + if err != nil { + return nil, apierrors.NewBadRequest( + fmt.Sprintf("expected an admission.Request inside context: %v", err)) + } + + if !topology.IsDryRunRequest(req, newObj) { + // MachineTemplate spec is fully immutable (CAPI convention). + if !reflect.DeepEqual(newObj.Spec.Template.Spec, oldObj.Spec.Template.Spec) { + var allErrs = make(field.ErrorList, 0, 1) + allErrs = append(allErrs, field.Forbidden( + field.NewPath("spec", "template", "spec"), + "field is immutable")) + return nil, apierrors.NewInvalid( + schema.GroupKind{Group: infrastructurev1beta2.SchemeGroupVersion.Group, Kind: "CloudscaleMachineTemplate"}, + newObj.Name, allErrs) + } } return nil, nil diff --git a/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook_test.go b/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook_test.go index b092c10..45545ef 100644 --- a/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook_test.go +++ b/internal/webhook/v1beta2/cloudscalemachinetemplate_webhook_test.go @@ -20,6 +20,7 @@ import ( "testing" . "github.com/onsi/gomega" + clusterv1 "sigs.k8s.io/cluster-api/api/core/v1beta2" infrastructurev1beta2 "github.com/cloudscale-ch/cluster-api-provider-cloudscale/api/v1beta2" "github.com/cloudscale-ch/cluster-api-provider-cloudscale/internal/testutils" @@ -116,7 +117,7 @@ func TestMachineTemplateValidateUpdate_FlavorChange(t *testing.T) { validator := CloudscaleMachineTemplateCustomValidator{FlavorInfo: testutils.NewTestFlavorInfo()} obj.Spec.Template.Spec.Flavor = "flex-16-8" - _, err := validator.ValidateUpdate(ctx, oldObj, obj) + _, err := validator.ValidateUpdate(newAdmissionContext(false), oldObj, obj) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("spec.template.spec")) } @@ -127,7 +128,7 @@ func TestMachineTemplateValidateUpdate_ImageChange(t *testing.T) { validator := CloudscaleMachineTemplateCustomValidator{FlavorInfo: testutils.NewTestFlavorInfo()} obj.Spec.Template.Spec.Image = "ubuntu-22.04" - _, err := validator.ValidateUpdate(ctx, oldObj, obj) + _, err := validator.ValidateUpdate(newAdmissionContext(false), oldObj, obj) g.Expect(err).To(HaveOccurred()) g.Expect(err.Error()).To(ContainSubstring("spec.template.spec")) } @@ -144,3 +145,54 @@ func TestMachineTemplateValidateDelete_AlwaysSucceeds(t *testing.T) { _, err := validator.ValidateDelete(ctx, obj) g.Expect(err).NotTo(HaveOccurred()) } + +// ============================================================================ +// Tests for ClusterClass dry-run support +// ============================================================================ + +func TestMachineTemplateValidateUpdate_DryRunWithChangedSpec(t *testing.T) { + g := NewWithT(t) + obj, oldObj := newMachineTemplateWebhookTestObjects() + validator := CloudscaleMachineTemplateCustomValidator{FlavorInfo: testutils.NewTestFlavorInfo()} + + // Simulate a topology-driven template rotation: the spec changed. + obj.Spec.Template.Spec.Flavor = "flex-16-8" + + // Annotate the new object with the topology dry-run marker. + if obj.Annotations == nil { + obj.Annotations = map[string]string{} + } + obj.Annotations[clusterv1.TopologyDryRunAnnotation] = "" + + _, err := validator.ValidateUpdate(newAdmissionContext(true), oldObj, obj) + g.Expect(err).NotTo(HaveOccurred()) +} + +func TestMachineTemplateValidateUpdate_DryRunWithoutAnnotationStillDenied(t *testing.T) { + g := NewWithT(t) + obj, oldObj := newMachineTemplateWebhookTestObjects() + validator := CloudscaleMachineTemplateCustomValidator{FlavorInfo: testutils.NewTestFlavorInfo()} + obj.Spec.Template.Spec.Flavor = "flex-16-8" + + // No TopologyDryRunAnnotation on the object. + _, err := validator.ValidateUpdate(newAdmissionContext(true), oldObj, obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("spec.template.spec")) +} + +func TestMachineTemplateValidateUpdate_AnnotationWithoutDryRunStillDenied(t *testing.T) { + g := NewWithT(t) + obj, oldObj := newMachineTemplateWebhookTestObjects() + validator := CloudscaleMachineTemplateCustomValidator{FlavorInfo: testutils.NewTestFlavorInfo()} + obj.Spec.Template.Spec.Flavor = "flex-16-8" + + if obj.Annotations == nil { + obj.Annotations = map[string]string{} + } + obj.Annotations[clusterv1.TopologyDryRunAnnotation] = "" + + // DryRun is false. + _, err := validator.ValidateUpdate(newAdmissionContext(false), oldObj, obj) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(ContainSubstring("spec.template.spec")) +} diff --git a/internal/webhook/v1beta2/webhook_suite_test.go b/internal/webhook/v1beta2/webhook_suite_test.go index 45da5f3..67655e1 100644 --- a/internal/webhook/v1beta2/webhook_suite_test.go +++ b/internal/webhook/v1beta2/webhook_suite_test.go @@ -33,6 +33,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/envtest" metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" "sigs.k8s.io/controller-runtime/pkg/webhook" + "sigs.k8s.io/controller-runtime/pkg/webhook/admission" infrastructurev1beta2 "github.com/cloudscale-ch/cluster-api-provider-cloudscale/api/v1beta2" "github.com/cloudscale-ch/cluster-api-provider-cloudscale/internal/testenv" @@ -143,3 +144,13 @@ func TestMain(m *testing.M) { _ = testEnv.Stop() os.Exit(code) } + +// newAdmissionContext returns a context containing an admission.Request with the given DryRun value. +// Use this helper when unit-testing ValidateUpdate directly, because the webhook now expects +// an admission.Request inside the context. +func newAdmissionContext(dryRun bool) context.Context { + req := admission.Request{ + DryRun: &dryRun, + } + return admission.NewContextWithRequest(ctx, req) +}