[Bug]: ValidateUpdate parameter order bug defeats the dry-run immutability skip from #865 (v1.5.1)

[jproinbox]

What Happened?

The fix in #865 (merged for v1.5.1, closing #864) does not actually work, because OscMachineTemplateWebhook.ValidateUpdate's parameters are swapped relative to what controller-runtime passes in.

Root cause
controller-runtime's Validator interface invokes the hook as (ctx, oldObj, newObj):

// sigs.k8s.io/controller-runtime/pkg/webhook/admission/validator_custom.go
warnings, err = h.validator.ValidateUpdate(ctx, oldObj, obj)

But api/v1beta1/oscmachinetemplate_webhook.go declares:

func (OscMachineTemplateWebhook) ValidateUpdate(ctx context.Context, obj runtime.Object, oldRaw runtime.Object) (admission.Warnings, error) {
        r, ok := obj.(*OscMachineTemplate)
        ...
        old := oldRaw.(*OscMachineTemplate)

Positionally, obj/r here is bound to the old object (controller-runtime's first argument after ctx), and oldRaw/old is bound to the new object. The naming is backwards from what the code assumes throughout the function.

This was harmless before #865, since the only use was a symmetric reflect.DeepEqual(r.Spec.Template.Spec, old.Spec.Template.Spec) — direction doesn't matter for an equality check.

But #865 added:

  if topology.ShouldSkipImmutabilityChecks(req, r) {
      return nil, nil
  }

ShouldSkipImmutabilityChecks checks whether r carries the topology.cluster.x-k8s.io/dry-run annotation. CAPI's topology dry-run only stamps that annotation onto the new/modified object being SSA-applied — the old/current object (as fetched from etcd) never carries it. Since r is actually bound to the old object, the annotation is never found, the skip never triggers, and the immutability check fires on every real spec change — exactly the scenario #865 was meant to fix.

Steps to Reproduce

Given an existing OscMachineTemplate with vmType: tinav5.c8r64p1, submit an update with a different vmType and the dry-run annotation set (mimicking what CAPI's topology controller does):

  apiVersion: infrastructure.cluster.x-k8s.io/v1beta1
  kind: OscMachineTemplate
  metadata:
    name: <existing-template-name>
    namespace: <ns>
    annotations:
      topology.cluster.x-k8s.io/dry-run: ""
    # ... existing labels/ownerReferences unchanged
  spec:
    template:
      spec:
        node:
          vm:
            vmType: tinav5.c8r48p1   # different from the stored value
            ...

  kubectl apply -f test.yaml --dry-run=server

Expected Behavior

Any ClusterClass-driven change to a worker/control-plane OscMachineTemplate (e.g. changing instance type via a ClusterClass variable) still fails topology reconciliation with TopologyReconciled: False, reason: ReconcileFailed, identical to #864, on v1.5.1.

Relevant Output (Logs)

N/A

Environment Details

- cluster-api-provider-outscale: v1.5.1
- cluster-api (core): v1.12.3
- Installed via cluster-api-operator (InfrastructureProvider CR)

Suggested Fix

Rename/reorder the parameters in ValidateUpdate to match the interface contract:

func (OscMachineTemplateWebhook) ValidateUpdate(ctx context.Context, oldObj, newObj runtime.Object) (admission.Warnings, error) {
      r, ok := newObj.(*OscMachineTemplate)
      ...
      old, ok := oldObj.(*OscMachineTemplate)
      ...
      if topology.ShouldSkipImmutabilityChecks(req, r) { // r must be the NEW object
              return nil, nil
      }

[jfbus]

You are right. v1.5.2 will be released to fix this.