feat(recipe): per-field union merge for validation phase checks

[yuanchen8911]

Summary

RecipeMetadataSpec.Merge in pkg/recipe/metadata.go:459-482 treats each validation phase (readiness, deployment, performance, conformance) as a per-phase pointer replace. If a leaf overlay defines deployment:, the leaf's block fully overwrites the inherited one — including checks: and constraints:. This is the only list-shaped field in the spec with replace semantics; every other field unions.

Proposal: switch validation.{phase}.checks (and likely .constraints) to the same union-by-name semantics already used at the top level. The merge stays explicit about precedence; leaves only need to declare additions and per-name overrides, not repeat the inherited list.

Background

This footgun surfaced while closing #969. The fix added a baseline deployment.checks list to the 6 service-root overlays (aks, eks, gke-cos, kind, lke, oke) so descendants inherit it. The 9 existing leaf overlays that already define deployment: happen to repeat the full 4-check list verbatim, so they coexist with the inherited one by accident — but the replace semantics mean any future leaf that adds a deployment: block (to add a constraint, or one extra check) silently drops every inherited check unless the author remembers to re-state them.

The 6 service-root overlays were annotated with a YAML comment about this (see PR closing #969), but a comment is a weak guard. The merge semantics themselves should make accidental drops impossible.

Current semantics by field (for reference)

Field	Semantics
constraints (metadata.go:420-434)	Union by name; same-name overwrites.
componentRefs (metadata.go:437-457)	Union by name; same-name uses per-field merge via mergeComponentRef (overlay-wins-if-non-empty).
mixins (metadata.go:488-498)	Dedupe and append, preserving order.
validation.{phase} (metadata.go:459-482)	Per-phase pointer replace — outlier.

Proposed semantics

For each phase pointer that is non-nil on the overlay:

• If the destination's phase is nil → clone the overlay phase (current behavior, unchanged).
• If both are non-nil → merge field-by-field:
  • Checks: deduplicated union, preserving order (overlay's additions appended after inherited entries).
  • Constraints: union by Name, overlay wins on same-name (mirroring the top-level constraint merge).
  • NodeSelection: unclear — proposal is overlay-wins-if-non-nil (full struct replace) since the struct is small and per-field semantics aren't obvious. Open for discussion.

This makes the validation merge a direct analogue of the existing top-level constraint and component merges.

Risks / migration

• No existing overlay relies on replace semantics for checks today (all 9 existing deployment: leaves re-state the full inherited list verbatim). Switching to union merge produces identical resolved output for them.
• The TestOverlayValidationPhaseFloor floor gate keeps working — it checks phase presence, not list contents.
• New test coverage required: a few cases in pkg/recipe/metadata_test.go exercising the union/dedupe behavior, plus a constraint same-name override case.

Done when

• RecipeMetadataSpec.Merge unions validation.{phase}.checks and validation.{phase}.constraints per the proposal.
• The replace-semantics warning comments in recipes/overlays/{aks,eks,gke-cos,kind,lke,oke}.yaml are removed.
• New unit tests cover union, dedupe, and per-name override.
• Documentation in docs/contributor/data.md or the equivalent recipe-development page describes the merge semantics for validation phases alongside constraints and components.

Related

• Surfaces from the fix for Close deployment-phase validation coverage gaps for accelerator-bound GPU recipes #969.

[yuanchen8911]

Surfaced and called out in #1001 — that PR added YAML comments to the 6 service-root overlays explaining the per-phase replace semantics that this issue proposes to fix. The follow-up here is to remove those comments by changing the merge behavior so the warning is no longer needed.