[tiraboschi]

HCO jsonpatch annotations (*.kubevirt.io/jsonpatch) are critical user controls.
Protect them from deletion or overwrite regressions.

[rlobillo]

Thank you for adding this test — it's great to see the user-managed HCO jsonpatch annotations getting regression coverage. The core promise of the PR is well covered: a regression that replaces the SSA apply with a full Update, or overwrites the annotation values, would fail this test.

If you're open to a small suggestion, I think two cheap assertions could make it a bit harder to defeat accidentally:

1. Assert that the first reconciliation actually applied

The return value of ReconcileAsset is currently discarded (_, err = ...). If a future regression made the asset soft-skip silently (an empty render, an activation condition gone wrong, or a drift detector that always reports "no drift"), the user annotations would survive trivially because nothing touched them, and the test would stay green while no longer protecting anything. Capturing the result closes that hole:

By("reconciling the HCO golden configuration twice")
applied, err := patcher.ReconcileAsset(ctx, hcoAsset, renderCtx)
Expect(err).NotTo(HaveOccurred())
Expect(applied).To(BeTrue(),
    "first reconciliation must adopt the pre-existing HCO via SSA; "+
        "a silent skip would make the annotations survive trivially")
_, err = patcher.ReconcileAsset(ctx, hcoAsset, renderCtx)
Expect(err).NotTo(HaveOccurred())

This pattern is already used in test/asset_failure_test.go (the descheduler namespace-skip case), so it would be consistent with the suite.

2. Verify the golden markers landed on the reconciled object

Asserting that platform.kubevirt.io/version is present on the live object is a second, independent proof that the SSA apply really ran against the user's object:

Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(hco), reconciled)).To(Succeed())
Expect(reconciled.GetAnnotations()).To(HaveKeyWithValue("platform.kubevirt.io/version", "1.0.0"),
    "golden markers must be present, proving SSA ran on the user's object")
for key, value := range hcoJSONPatches {
    Expect(reconciled.GetAnnotations()).To(HaveKeyWithValue(key, value))
}

Both are entirely optional hardening — the test is correct and valuable as is. Feel free to take them, adapt them, or ignore them. Thanks again for putting this together!

[rlobillo]

/lgtm
/approve

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rlobillo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• test/OWNERS [rlobillo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment