feat(recipe): adopt the ADR-015 gpuStack profile on AKS

[yuanchen8911]

Summary

Adopt the ADR-015 gpuStack configuration profile on the AKS family — azure-managed (default, the AKS "Driver only" preinstall) and operator-managed (--gpu-driver none pools) — together with the snapshot projection that qualifies it: aicr snapshot --aks-gpu-pools <file> projects an operator-supplied az aks nodepool list -o json dump into the K8s.aks-gpu-pools.gpu-driver reading (ADR-015 DD3). The bundle-time four-flag override tuple is replaced by generation-time selection and ownership locking.

Motivation / Context

Rollout PR 2 of the staged plan on the implementation umbrella #1761, per ADR-015: recipe configuration profiles (Accepted 2026-07-21; the AKS pool-mode signal is Deferred Decision 3). AKS is the first embedded adopter because its recipe already defaults to the four-path azure-managed tuple, and the alternative mode required flipping all four paths together by hand at bundle time — the exact unqualified-hybrid risk the profile core exists to close (#1757).

The projection and the adopter were briefly staged as two PRs (#1968 → this one); they are recombined here because the halves are inseparable in practice — the reading is inert without the declaration, the declaration's snapshot-qualified path is broken without the reading, the docs cross-reference each other, and review findings repeatedly crossed the PR boundary. #1968 is closed with its review history intact; its final content is contained in this PR verbatim.

Fixes: N/A
Related: #1761 (implementation umbrella), #1933 (rollout PR 1, merged), #1968 (closed; projection half recombined here), #1757

Type of Change

• New feature
• Breaking change — existing /v1/recipe and /v1/query clients using service=aks criteria are rejected after this merges (documented /v2 cut-over in docs/user/api-reference.md), and pre-existing AKS snapshots without the pool reading fail snapshot-qualified resolution closed. Other families and criteria-only AKS generation are unaffected. Upgrade hazard for external --data catalogs: a pre-existing (pre-conversion, v1alpha2) external overlays/aks.yaml wholesale-replaces the embedded declaring overlay, so upgrading AICR with such a catalog silently keeps the AKS family unprofiled — no error, no selectedProfile, no pool constraint. Documented with the operator migration step in docs/integrator/data-extension.md and pinned by a regression test; load-time shadow detection is a candidate follow-up.

Component(s) Affected

• Recipes / overlays (recipes/)
• Recipe resolution (pkg/recipe, pkg/client/v1)
• CLI (cmd/aicr, pkg/cli)
• Collectors / snapshotter (pkg/collector, pkg/snapshotter)
• Bundler validations (pkg/bundler/validations)
• API server (behavioral only — no pkg/server code changes in this PR; the /v1 AKS rejection is the merged profile core's behavior, triggered by this PR's adoption)
• Docs/examples (docs/)

Implementation Notes

The operator flow, end to end

Three steps; the pool dump is consumed only at step 2 — the snapshot carries the reading from then on (recipe takes the snapshot, bundle takes the recipe):

1. Dump the pools (the ownership mode lives in the Azure control plane, not in any K8s object): az aks nodepool list -g <rg> --cluster-name <cluster> -o json > pools.json
2. Snapshot with the projection: aicr snapshot --aks-gpu-pools pools.json -o snapshot.yaml — works in agent Job and local mode; the file never enters the cluster; a bad file fails before any cluster work.
3. Generate and bundle with the value the pools call for: aicr recipe --service aks … --snapshot snapshot.yaml (azure-managed default) or --profile gpuStack=operator-managed; then aicr bundle -r recipe.yaml.

Selection and verification are independent axes (explicit-intent per ADR-015; the ADR's motivating-example wording is clarified accordingly in this PR): --profile — or its absence — decides the selected value (the declaration default, azure-managed, when omitted; never derived from the snapshot), and --snapshot — or its absence — decides whether that selection is verified now or later:

Invocation	Selected value	Pool-mode check
no --profile, no --snapshot	default (azure-managed)	none possible — constraint recorded, enforced at validate readiness
--profile …=operator-managed, no --snapshot	operator-managed	deferred to validate the same way
no --profile, --snapshot	default (azure-managed)	checked at generation: pools must read Install
--profile …=operator-managed, --snapshot	operator-managed	checked at generation: pools must read None

With a snapshot present the check is never skipped — a missing reading fails closed for either selection; deliberately unverified generation is the criteria-only path (drop --snapshot). Every pools × selection combination is deterministic and was confirmed live on both clusters at this head:

Pools read	Default (azure-managed)	--profile gpuStack=operator-managed
Install (aicr-test6)	✅ resolves, bundle renders driver.enabled=false / nvidia-container-runtime / root /	❌ fails closed: constraint expects None
None (aicr-test5)	❌ fails closed: constraint expects Install	✅ resolves, bundle renders true / nvidia / /run/nvidia/driver
Mixed / Managed	❌ fails closed naming the observed state	❌ fails closed
no reading (captured without --aks-gpu-pools)	❌ fails closed: reading unavailable — recapture with the pool dump	❌ same

The same flow is documented for operators in docs/integrator/aks-gpu-setup.md ("End-to-end flow").

Profile adoption

Declaration (recipes/overlays/aks.yaml, now aicr.run/v1alpha3): gpuStack declares two values over gpu-operator (driver.enabled, toolkit.enabled, operator.runtimeClass) and nvidia-dra-driver-gpu (nvidiaDriverRoot), plus the synthetic enabled presence paths. azure-managed is the default and is value-identical to what the family has always shipped; operator-managed flips all four paths together. (Value names were finalized as azure-managed/operator-managed — naming ownership of the declared driver+toolkit layer — replacing the earlier driver-only/operator, which inverted perspective and misstated the Azure preinstall; the qualifying constraint keeps Azure's official Install/None vocabulary.) Each value records its distinguishing constraint (K8s.aks-gpu-pools.gpu-driver: Install / None), so snapshot-qualified resolution verifies the actual pool mode and fails closed otherwise — Managed, Mixed, unknown values, and a missing reading match neither constraint.

Ownership lock: per-path --set overrides of the owned paths that diverge from the selected value are rejected at bundle time (identical values pass); legacy pre-profile recipes without metadata.selectedProfile keep the old tuple behavior.

Driver-state interplay (three cases, all documented in component-catalog.md and pinned by tests):

• Mismatched/missing pool reading → resolution fails closed at profile-constraint evaluation, naming the observed state.
• Install pools whose sampled node has no loaded driver (failed AKS install, mid-reimage) → the constraint passes (pool mode is the ownership contract, not live state); resolution records metadata.gpuDriverState: absent and the bundle-time CheckDriverOwnershipCoherence gate blocks with the reworded AKS remedy (repair pools + recapture, or recreate with --gpu-driver none + recapture + --profile gpuStack=operator-managed). The remedy twins (pkg/client/v1/gpu_driver_state.go, pkg/bundler/validations/checks.go) branch on metadata.selectedProfile: profiled artifacts get the recapture + --profile wording, legacy pre-profile artifacts keep the four-flag tuple wording (the lock does not apply to them). The values-aks.yaml header is updated; the legacy auto-override is subordinated on owned paths.
• Non-profiled families and legacy AKS artifacts → today's warn-record-gate flow, unchanged.

Snapshot projection (ADR-015 DD3)

Projection semantics (pkg/collector/k8s/aksgpupools.go): NVIDIA GPU pools are identified by VM-size family (NC/ND/NV) minus AMD accelerators (NG family; MI300X/MI325X ND sizes, which AKS requires creating with --gpu-driver none; Radeon Pro V620/V710 NV sizes) — without the exclusion, a supported NVIDIA-Install + AMD cluster would falsely project Mixed. Absent/null gpuProfile follows the provider's documented Install default; gpuProfile.nvidia with managementMode: Managed (or unknown/empty mode) projects Managed, while Unmanaged follows the driver field (a supported azure-managed configuration); disagreement projects Mixed; no GPU pools omits the reading. gpu-pool-count and a sorted gpu-pools roster accompany the reading for diagnostics. Known limitation (documented in code): detection is prefix+marker based; the AgentPool object carries no vendor field.

Orchestration-layer design: the projection is pure file processing and never enters the cluster. The Job-mode merge is performed on generic maps, not through the controller's typed Snapshot struct, so a version-skewed (newer) agent image's unknown fields survive the merge and the ConfigMap rewrite. Local mode projects before any collector runs; agent Job mode projects controller-side before deploying — a bad file fails in milliseconds with zero cluster mutations — then merges the subtype into the returned snapshot and rewrites the Job's result ConfigMap (Cleanup deletes Job+RBAC but never that ConfigMap, so without the rewrite a stale projection-less snapshot would persist, including when the ConfigMap is the requested output). aicr validate accepts the same flag for its live-capture path, with --aks-gpu-pools in its duplicate-flag guard, structured errors preserved (PropagateOrWrap), and a warning when the flag is passed alongside --snapshot (where it is ignored).

Fail-loud contract: every read/decode failure — including a top-level JSON null, which json.Unmarshal silently accepts into a slice — is an error that fails the run, never a degraded measurement subject to the snapshotter's degrade-to-warning collector policy. The read is descriptor-first and context-aware: open with O_NOFOLLOW|O_NONBLOCK (no symlinks; a substituted FIFO cannot block the open), regular-file and 1 MiB size checks on the opened descriptor (pkg/defaults cap), then a chunked read with cancellation checks under FileReadTimeout — the verifier's established bounded-read pattern.

Evidence-pipeline integration details (review-hardened):

• TestGrid coordinate is deliberately unsuffixed — per ADR-015, the publisher's digest-bound build ID already partitions per value, so profiled runs stay on the family's canonical tab and Recipe Health/presence links remain valid. The corroboration projection is where the profile segment is path-forming (meta.json records it; the corroborate criteria inversion strips it, so it is never misread as a phantom platform).
• Profile value names are catalog-validated case-insensitively unique — lowercase path segments would otherwise collapse Operator-Managed/operator-managed onto one evidence directory.
• The repo evidence gate protects suffixed dirs at BASE too, per value — deleting the sole pointer of a profiled recipe surfaces the "evidence pointer removed" de-protection row instead of silently demoting the recipe to "no evidence yet", and removing one profile value's last pointer warns naming that dir even while the sibling value still holds evidence (both smoke-tested in isolated fixture repos; a pipefail/SIGPIPE hazard in the new helper was found and fixed during that testing).
• Local TestGrid publishes derive a bounded content digest from the bundle's canonical manifest.json (per-file digests of the whole bundle, so even same-recipe/different-results bundles diverge); read errors propagate, and the "local" placeholder survives only under --dry-run — test-pinned. Two bundles sharing a second-resolution timestamp cannot collide on the deliberately unsuffixed TestGrid coordinate.
• Profiled pointers are verifier-checked: pointer.profile requires the recipe name to carry the lowercase -<name>-<value> segment (test-pinned), so a hand-written pointer cannot collapse two values into one evidence directory; the ADR-007 example shows the consistent form.
• Presence live-paths report the suffixed routes, and the committed presence manifest withholds the AKS entries (with the rationale inline), so docs/user/recipe-health.md — regenerated in this PR — shows the AKS rows as an honest pending rather than linking soon-to-be-historical unsuffixed routes; profile-aware Health links are the recorded follow-up.
• Selection-collision quarantine in corroboration aggregation: runs whose exact selections merely collide on the lossy lowercase segment (case variants, ambiguous - joins like gpu-stack=operator vs gpu=stack-operator) can no longer merge — every run's selection must re-derive its own segment at intake (inconsistent metadata is skipped), and a same-route/different-selection conflict quarantines the whole coordinate order-independently instead of crowning a first-writer. Test-pinned (collision, mismatched-derivation, and the existing distinct-values rows).
• Per-surface lock matrix documented (verified against the code first): bundle+mirror accept identical statics/reject divergent; --dynamic rejects on intersection; argocd-helm install-time values reject any owned-key presence even when identical; component presence is not changeable by reselection (fragments cannot assign enabled). The shadow regression test now exercises the real --data layered provider (with anti-vacuous source assertions), evidence-identity language distinguishes the three identities (overlay name / criteria coordinate / evidence slug), data-flow documents parse-before/attach-after, and the RQ1/health docs read in present tense.
• External-catalog upgrade hazard documented and pinned: a pre-conversion v1alpha2 external overlays/aks.yaml wholesale-replaces the embedded declaring overlay, silently keeping the family unprofiled on upgrade — the operator migration step is in data-extension.md, TestAKSLegacyExternalShadowStaysUnprofiled pins the behavior as intentional, and load-time shadow detection is the recorded follow-up. The override-lock docs now state the three distinct cases (owned value paths: identical-accepted/divergent-rejected from any static source; synthetic enabled: scalar-only, typed sources always rejected; --dynamic: rejected on intersection).
• Pointer/predicate identity is content-bound: after the manifest inventory verifies the bundle bytes, checkRecipeIdentity derives the recipe name, exact profile selection, and canonical digest from the verified recipe.yaml and requires the pointer's recipe/profile and the predicate's name/digest to match exactly — closing the name-collision spoof (a recipe named …-ubuntu-training no longer "satisfies" a fabricated profile: ubuntu=training); suffix checks remain as fast-fail pre-checks only. Test-pinned including the exact spoof, digest mismatch, and predicate-name mismatch; legacy criteria-less recipes keep verifying (name equality enforced only when derivable — the profile and digest bindings always run).
• The dashboard's Copy-CLI / Copy-config reproduce the exact selection: the exact-case name=value selection rides through meta.json (profileSelection), aggregation, and the browser model (the lowercase segment is lossy by design and never reversed); both generators emit --profile / spec.recipe.profile — test-pinned end to end.
• The route contract is stated once, in ADR-012: CoordinateFor is the shared criteria-only base coordinate; the Golden Path corroboration route appends the profile segment to the tab, TestGrid keeps the unsuffixed base and partitions per value via its digest-bound build ID — both user guides and the publisher comments now say exactly this. Recipe Health documents that profiled families are structurally graded at the declaration default, and pending means "no committed linkable presence" (not "no live evidence").
• Renderer routing is fully profile-aware: hydration carries the segment and every hash construction (route map, overview, grid, time-series) goes through one routeSegs helper; sidebar/breadcrumb labels show the suffixed tab so two values are distinguishable. Case-insensitive value-name uniqueness is stated in the integrator guide and the ADR's declaration invariants, matching the enforced rule.
• The corroboration dashboard keeps distinct routes per value — meta.json records the profile segment; the criteria inversion strips it (fail-closed skip on a tab/profile mismatch) and the renderer appends it to the tab route, so azure-managed and operator-managed never overwrite each other's lookup entry while criteria facets stay profile-blind. The full plumbing (Synthesize writes it → corroborate strips/routes it) is test-pinned.
• ADR-015, ADR-007, and the contributor guide are aligned with the staged rollout (v1 predicate now; the descriptor-bound predicate type is GKE-stage work per Implement ADR-015: recipe-declared configuration profiles #1761).

CUJ chainsaw fixtures actually pass now: beyond migrating to the profiled shape, the componentRefs/deploymentOrder assertions were re-synced to the emitted order — chainsaw list asserts are per-index, and the old alphabetized lists failed against freshly generated recipes even on main (pre-existing; both fixtures now verified rc=0 with chainsaw assert against fresh output).

Public SDK surface: the pkg/client/v1 facade AgentConfig carries AKSGPUPoolsPath (translated in toInternalAgentConfig, pinned by an SDK-level test) so Client.CollectSnapshot supports the documented collect-then-resolve workflow. This deliberately differs from ClusterConfigPath/DiscoverNetwork (optional in-pod enrichments, off the facade): this is controller-side input that AKS profile-qualified resolution requires.

Generalization path (GKE and beyond): per-provider projectors with namespaced subtypes (gke-gpu-pools beside aks-gpu-pools), the shared bounded reader (providerpools.go), and additive sibling flags — documented for contributors in docs/contributor/collector.md ("Provider Node-Pool Projections").

API and version surface

Every resolved AKS recipe is now aicr.run/v1alpha3; pre-flip artifacts keep resolving as before. /v1/recipe and /v1/query on AKS criteria reject once this merges — the /v2 cut-over is documented in docs/user/api-reference.md (with a real gpuStack example). metadata.selectedProfile records gpuStack, the selected value, and the declaration-wide ownedPaths.

Test moves. Fixtures that declared a gpuStack profile on AKS criteria (profile-core integration, server profile-endpoint, facade profile test) move to EKS: composition-wide uniqueness now collides with the embedded declaration. profile_aks_test.go qualifies both embedded values, the ownership surface, recorded constraints, and leaf inheritance. gpuHardwareSnapshotPools decouples pool mode from sampled driver state. TestReadingShapeMatchesProfileContract pins the reading contract shared by the declaration and the projector.

Undraft checklist — complete

All pre-undraft items are done; the only follow-up is post-merge by design (AKS evidence regeneration, item 2).

• Subordination test for the legacy gpuDriverState auto-override on owned paths — pinned at the sharp case against the real embedded declaration: gpuStack=operator-managed (fragment driver.enabled=true) + loaded-driver snapshot; the injector must skip the owned path and the fragment value survives (TestResolveRecipeFromSnapshot_GPUDriverAutoDetect, aks operator subordination row). Also observed live on aicr-test5.
• Per-profile evidence: code support ships in this PR (an earlier "post-merge only" disposition was corrected by review — Implement ADR-015: recipe-declared configuration profiles #1761 assigns it to this rollout stage, and the Azure UAT's ingest job would otherwise reject AKS bundles). Delivered: the "deferred to the profile adoption rollout" rejections in dashboard synthesis (pkg/evidence/project) and TestGrid publication are removed; a shared ProfileSegment helper joins the profile value into the evidence path name (RecipeNameFor) and the corroboration Tab so the two values never overwrite each other (the TestGrid coordinate deliberately stays unsuffixed — its digest-bound build ID partitions per value); pointers record their selection (profile: name=value); aicr evidence digest --profile computes selection-correct digests (rejected on hydrated-result inputs); and the repo evidence gate recomputes each pointer against its recorded selection, including suffixed-dir → overlay mapping. Committed pre-profile AKS pointers become historical (designed semantics); their regeneration against the merged digest remains the standard post-merge publish flow.
• Qualification runs for both values on live AKS clusters (aicr-test5 = operator-managed pools, aicr-test6 = azure-managed pools) — see Testing below.
• KWOK/e2e drift: none — KWOK profiles are EKS-only and e2e is Kind-based (verified in review round 2); e2e runs in the make qualify gate below. (The Azure UAT lane is already migrated in this PR: uat-azure.yaml dumps the pool modes after cluster connect and exports AICR_AKS_GPU_POOLS_PATH, which every subsequent snapshot/validate step — prep, install gate, conformance, CUJ chainsaw — picks up. A spec.snapshot.aksGpuPools AICRConfig field is a possible follow-up for config-file parity; the env var is the supported path today.)
• make qualify full gate — green (rc=0) at every review round's head, most recently a5b1a9652 (round 20), 20547800d (round 19), and a35c5dccc (round 18 + the value-name finalization) — all green on the first run with no tests/releasepolicy: 10s per-script deadline flakes under full-suite load on dev machines #1974 flake rerun. The head 66542dacf (final-review wording fixes, comment-only, plus rebases over two non-overlapping validator commits) was also green on the first run; a907d81b6 (first human-review round) was green on the first run as well; the current head a1df9946f (re-review nitpick + rebase onto 018dd55) is green on the first run as well. Roughly half the runs needed one rerun for the tracked tests/releasepolicy: 10s per-script deadline flakes under full-suite load on dev machines #1974 flake (tests/releasepolicy 10s script-deadline margin; passes in isolation every time; not this PR — its diff intersects nothing the package executes). CI runs the same gate on CI runners and has been green throughout.

Testing

make qualify   # full gate green (rc=0) at 3f3d3d93b (round 16; one #1974 flake rerun) — round-17 head pending its own run

Two dedicated backward-compat/ADR-contract tests beyond the coverage described below:

• TestAKSDefaultKeepsPreProfileEffectiveValues — the no-behavior-change guarantee for the default (azure-managed) path: criteria-only resolution against the embedded catalog vs the same catalog with the declaration stripped → identical component sets, byte-identical effective values for every component, and a constraint delta of exactly K8s.aks-gpu-pools.gpu-driver=Install.
• TestClassifyIgnoredAKSGPUPools — the ignored-flag note's provenance matrix: explicit CLI presence (both flag forms) always warns, ambient env demotes to debug, prefix false-positives excluded.
• The subordination row in TestResolveRecipeFromSnapshot_GPUDriverAutoDetect — gpuStack=operator-managed + loaded-driver snapshot: the legacy auto-override must skip the profile-owned driver.enabled and the fragment's true survives (also observed live on aicr-test5).

Projection: table-driven tests cover all-Install, all-None, absent/null gpuProfile → Install, Mixed, Managed, unknown-value preservation, VM-family + AMD filtering (MI300X/V710 excluded, AMD-only omits the reading), no-GPU-pools omission; fail-loud tests cover missing file, wrong JSON shape, top-level null, malformed JSON, size cap, non-regular files. Orchestration: local-mode attach + fail-before-collectors, Job-mode merge round-trip, ConfigMap-rewrite guards, SDK translation. Profile: both embedded values qualified end to end (ownership surface, constraints, leaf inheritance), lock rejection of diverging --set, legacy-tuple acceptance on pre-profile recipes, and the three driver-state cases above.

Live qualification (2026-07-30, both values, real AKS clusters, binary built from this branch):

Check	aicr-test6 (pools Install → azure-managed)	aicr-test5 (pools None → operator-managed)
Job-mode aicr snapshot --aks-gpu-pools (stock v0.18.0 agent image — the projection is controller-side, so the in-pod image needs no new code)	✅ gpu-driver: Install, CPU/system pools filtered	✅ gpu-driver: None
Result ConfigMap rewritten with the reading	✅ verified in-cluster	✅
Matching profile value resolves from snapshot	✅ azure-managed selected, selectedProfile recorded	✅ operator-managed selected; the legacy auto-override's subordination on owned paths fired live (driver observed loaded, mutation skipped with advisory log)
Mismatched value fails closed	✅ gpuStack=operator-managed rejected: constraint "K8s.aks-gpu-pools.gpu-driver" failed	✅ default azure-managed rejected likewise
Bundle renders the value's four ownership paths	✅ driver.enabled=false, toolkit.enabled=false, runtimeClass=nvidia-container-runtime, nvidiaDriverRoot=/	✅ true/true/nvidia//run/nvidia/driver
Ownership lock	✅ diverging --set gpuoperator:driver.enabled=true rejected naming the owned path; identical value accepted	—
Validate readiness	✅ expected=Install actual=Install passes; crossed artifacts (test6 recipe × test5 snapshot) fail closed: expected Install, got None	✅
Fail-fast negatives (missing file, top-level JSON null)	✅ instant INVALID_REQUEST, zero cluster mutations	—

Blast-radius proof (non-AKS families): all 60 non-AKS leaf recipes in the embedded catalog (EKS/GKE/OKE/OCP/Kind/LKE/BCM/metal3 × every accelerator/OS/intent/platform) generated with this branch's binary and a clean-origin/main binary are byte-identical, 60/60 (real outputs verified non-vacuous: every file carries componentRefs, ~5.2 MB total).

Final live e2e re-run at a5b1a9652 (the reviewed head; the subsequent delta to 66542dacf is comment-only): fresh az pool dumps → Job-mode snapshots with the controller-side projection (gpu-driver: Install on aicr-test6, None on aicr-test5) → the full resolution matrix (default passes on test6 / fails closed on test5; --profile gpuStack=operator-managed fails closed on test6 / passes on test5 with the legacy auto-override subordinated live — driver.enabled=true and /run/nvidia/driver survive a loaded-driver snapshot) → criteria-only generation records both values' constraints unevaluated → bundles render the right four-path tuple per value → ownership lock rejects divergent --set and accepts identical on both clusters → aicr validate readiness fails closed in both mismatch directions with the observed pool state named → deployment-phase validation passes 4/4 on aicr-test6; on aicr-test5 it passes 3/4 with expected-resources correctly detecting pre-existing stack drift (that cluster's 2026-07-09 deployment never installed nodewright-customizations; the check times out waiting for the Skyhook CR — environmental, demonstrated by the same binary+image passing 4/4 on test6, and the check doing exactly its job). Per-profile evidence digest: default == explicit azure-managed, operator-managed distinct, hydrated-result input rejects --profile.

Earlier full live e2e re-run at 0f471c929 (the evidence-fixed tree): fresh az dumps → Job-mode snapshots → all four resolution-matrix cells → bundles with rendered-value verification → ownership lock → validate readiness both directions — all green on both clusters, plus the new evidence surfaces: evidence digest default == explicit azure-managed, operator-managed digest distinct, hydrated-result input rejects --profile.

Independent adversarial review: twenty rounds across two reviewers (read-only worktrees, two-question mechanism/reachability protocol, scoped to the ADR-015 implementation). The arc: rounds 1–3 fixed the Azure UAT lane break, legacy-remedy regression, version-skew merge loss, and follow-ons; rounds 4–9 drove the docs-contract sweep, deterministic marshal, breaking-change metadata, and the case-uniqueness rule; rounds 10–14 hardened the evidence pipeline (profile path segments, per-value gate protection, dashboard routing, exact-selection copy generators, content digests); rounds 15–17 closed the security-grade identity binding (pointer/predicate claims derived from manifest-verified recipe bytes, with the name-collision spoof test-pinned), the external-catalog upgrade hazard (documented + regression-pinned), and the selection-collision quarantine in corroboration aggregation; round 18 fixed the profiled evidence-refresh flow (gate output, publishing guide, and signing-workflow header now capture --aks-gpu-pools and hydrate the recipe with the pointer's recorded --profile selection — the raw-overlay flow could only regenerate default-value evidence), split the per-surface lock matrix's bundle/mirror rows (mirror exposes only scalar --set; config-file deployment.set is not applied by mirror list), corrected the argocd-helm claim to presence-based rejection, gave the SDK example separate resolve/snapshot/validate contexts, and cleaned the remaining RQ1, ADR-012 identity, and meta.json-provenance doc residue; round 19 completed the refresh flow with the target leaf's --intent/--platform (the snapshot fingerprint deliberately reports any for author-selected criteria, so the earlier commands could hydrate a different leaf), made the fallback validate the hydrated recipe explicitly, added WithValidationTimeout(0) to the SDK example (the facade's default 75-minute cap would otherwise override the two-hour context), rewrote the pool-file reader descriptor-first with context threading (see below), and swept the last rename/matrix/identity residue off public surfaces (api-reference, automation, the selection table, the checks.go remedy twin, the peermem manifest, recipe-development.md's mirror row, and the TestGrid coordinate comment); round 20 found no reachable code or workflow defect — the exit criterion (a round with no valid, blocking finding) — and its three comment/metadata cleanups are folded into the final rebase. A final pre-undraft self-review (two independent adversarial passes over the full range — code correctness and documentation contracts, both applying the mechanism/reachability protocol) found no blocking findings; its minor wording items (lock-rule precision in aks-gpu-setup.md, retiring the old value name from sibling-file comments, a stale values-merge comment, a missing profile-lock bullet in the CLI reference's override rules) are folded into the final head, and its follow-up-grade observations (profile-owned short-circuit skips two advisory warns in gpu_driver_state.go; RecipeNameFor empty-name guard comment; ADR-015 lock-mechanics wording predating this PR; an air-gap-mirror.md profile caveat) are noted here rather than churned in. First human review (njhensley): APPROVED, with one minor and four nitpicks — all verified and addressed at a907d81b6: the internal ConfigMap rewrite is now fail-loud only when cm:// is the delivery vehicle (file/stdout/SDK runs warn and deliver the merged snapshot instead of discarding a successful capture), the UAT pool-dump step gained the client-connect retry shape, and two coverage gaps got direct tests (TestRawSnapshotDocRoundTrip, TestDriverAbsentRemedyBranches); the dedup observation was agreed unreachable in supported flows and left as-is (replied inline). His re-review re-approved with one follow-on nitpick — the new best-effort branch itself was untested — closed at a1df9946f by extracting the delivery contract into rewriteMergedSnapshotConfigMap and pinning both directions in TestRewriteMergedSnapshotConfigMapDeliveryContract. Deliberately declined with recorded rationale: softening the ConfigMap-rewrite fail-loud, and nvidia: {} as Unmanaged (Azure documents null/explicit-Managed shapes; fail-closed retained — independently confirmed by review). The pool-reader Lstat→Open TOCTOU, declined earlier as a self-race, was ultimately implemented in round 19 on stronger grounds: the reader took no context (violating the project's I/O rule — a swapped-in FIFO could stall a snapshot with no cancellation), so it now opens descriptor-first (O_NOFOLLOW|O_NONBLOCK), validates the opened descriptor, and streams with cancellation checks under FileReadTimeout, following the verifier's established pattern.

Risk Assessment

• Medium — Core functionality changes for the AKS family, thoroughly tested; other families byte-identical

Rollout notes: No behavior change for non-AKS families (byte-identical, proven by the 60/60 catalog sweep below). On AKS the migration is unconditional at generation — recipes flip to aicr.run/v1alpha3 with the recorded profile regardless of flags (--aks-gpu-pools gates only the snapshot reading); /v1 AKS clients must move to /v2 (documented cut-over).

Checklist

• Tests pass locally (make test with -race)
• Linter passes (make lint)
• I did not skip/disable tests to make CI green
• I added/updated tests for new functionality
• I updated docs if user-facing behavior changed
• Changes follow existing patterns in the codebase
• Commits are cryptographically signed (git commit -S) — GPG signing info

[github-actions]

Recipe evidence check

Protected recipes

Recipes with committed evidence (recipes/evidence/<slug>/<source>/<digest>.yaml) that this PR affects: 3

Recipe	Source	Pointer	Verify	Digest match
h100-aks-ubuntu-inference-dynamo	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-b7d3b1c672568329cae994ed4c831af5e569b23209fb81e789d2e2288b44100d	✅ passed	⚠️ stale (b0081437bf6d… vs current 4cdb994dd022…)
h100-aks-ubuntu-inference-dynamo	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-ca96cea68b11cd3b5f0dbad677d40365287fce8e0a5412b32861888d335c5bdc	✅ passed	⚠️ stale (35e1d989567a… vs current 4cdb994dd022…)
h100-aks-ubuntu-inference-dynamo	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-edc042d2e32d58bde9bb0e7cfdaa14568a13c144fdf0869958a4d582f3fc8cfc	✅ passed	⚠️ stale (ea8757f630ce… vs current 4cdb994dd022…)
h100-aks-ubuntu-inference-dynamo	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-f8d2a0188274d179f37dfe39a257aeaa3fbb97273162586853e0986bfa5d3c05	✅ passed	⚠️ stale (8e88ca57dea5… vs current 4cdb994dd022…)
h100-aks-ubuntu-training-kubeflow	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-7bfed65fb09c14c6e6cbe87a68e0810a7d24178e0e83d1691c020556c92dbbd8	✅ passed	⚠️ stale (7726976735b7… vs current bab15e101107…)
h100-aks-ubuntu-training-kubeflow	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-7e7c4680bab4c44bb68fab53fc85a7f8d8065ca6b796458a2bc7cb4f4a49bfa9	✅ passed	⚠️ stale (748b0a7f5852… vs current bab15e101107…)
h100-aks-ubuntu-training-kubeflow	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-dc1670c23bbe6711a6ffd86a49160b06d992c8ff84e8f3303facc54dd7aecb61	✅ passed	⚠️ stale (fac7033fea5c… vs current bab15e101107…)
h100-aks-ubuntu-training	5bf9e82f0e90a11528ac85f4bcb866c8	sha256-c51d0f2dd75b9f397ddc9713150159553f4a8d15982095ea52a28872d7eef479	✅ passed	⚠️ stale (0f210b23045c… vs current 1c4e27d658d3…)

Other affected recipes without evidence yet: 7

These recipes are affected by this PR but carry no committed evidence pointer, so there is
nothing to verify. This is expected — evidence is hardware-gated and added over time.

• a100-aks-training
• a100-aks-ubuntu-training-kubeflow
• a100-aks-ubuntu-training
• h100-aks-inference
• h100-aks-training
• h100-aks-ubuntu-inference
• h100-aks-ubuntu-training-slurm

How to refresh evidence

Run on a cluster matching the recipe's criteria:

aicr snapshot -o snapshot.yaml
# Profiled families (AKS gpuStack): capture the pool projection and
# hydrate the recipe with the pointer's recorded 'profile:' selection
# first — validating the raw overlay resolves only the declaration
# default, and 'aicr validate' has no --profile flag:
#   az aks nodepool list -g <rg> --cluster-name <cluster> -o json > pools.json
#   aicr snapshot --aks-gpu-pools pools.json -o snapshot.yaml
#   aicr recipe -s snapshot.yaml --intent <intent> [--platform <platform>] \
#     --profile <name>=<value> -o recipe.yaml
# State the target leaf's intent/platform explicitly (the snapshot
# fingerprint supplies service/accelerator/OS but intent and platform
# default to 'any') and pass -r recipe.yaml below instead of the raw
# overlay.
aicr validate \
  -r recipes/overlays/<slug>.yaml \
  -s snapshot.yaml \
  --emit-attestation ./out \
  --push ghcr.io/<your-fork>/aicr-evidence
# Copy to the per-source path printed in the emit 'copyTo' hint:
#   recipes/evidence/<slug>/<source>/<bundle-digest>.yaml

This gate is warning-only and never blocks merge. See ADR-007 for the trust model.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds the AKS gpuStack profile with driver-only and operator modes, component overrides, and pool-driver constraints. Extends recipe, catalog, metadata, snapshot, overlay, and HTTP tests for profile selection, fail-closed resolution, v1 compatibility, and v2 validation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related issues

• Implement ADR-015: recipe-declared configuration profiles #1761: Covers the ADR-015 profile mechanism and AKS adopter implemented by this change.

Possibly related PRs

• NVIDIA/aicr#1933: Introduces the profile-core behavior exercised by the AKS profile and endpoint tests.

Suggested reviewers: njhensley

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: adopting the ADR-015 gpuStack profile for AKS recipes.
Description check	✅ Passed	The description directly explains the AKS gpuStack profile adoption, snapshot qualification, ownership locking, breaking changes, testing, and rollout impact.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 9

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/recipe/metadata.go (1)

807-822: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

Legacy string/scalar forms still accept an empty name.

The object forms now reject name: "", but "" (JSON string) and an empty !!str YAML scalar still produce an ExcludedOverlay with an empty Name. Consider applying the same non-empty check on the scalar/string branches so all four decode paths agree.

♻️ Proposed change (JSON path)

 	var name string
 	if err := json.Unmarshal(data, &name); err == nil {
+		if name == "" {
+			return errors.New(errors.ErrCodeInvalidRequest,
+				"excluded overlay requires a non-empty name")
+		}
 		e.Name = name

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/recipe/metadata.go` around lines 807 - 822, Update the scalar/string
decoding branches for ExcludedOverlay to reject empty names, matching the
existing raw.Name validation for object forms. In the JSON path around
json.Unmarshal into name, validate name before assigning e.Name; apply the
equivalent non-empty check to the YAML scalar branch, while preserving
successful decoding for non-empty values.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/integrator/recipe-development.md`:
- Around line 425-428: Update docs/integrator/recipe-development.md lines
425-428 to identify AKS as the current first adopter through gpuStack rather
than a planned rollout. Update docs/design/015-recipe-configuration-profiles.md
lines 1261-1266 to remove or qualify the “no adopter” wording so it applies only
to the earlier core phase and reflects the delivered AKS adoption.

In `@pkg/bundler/deployer/argocdhelm/argocdhelm.go`:
- Around line 545-553: Update inspectProfileLockTemplate to detect a missing
outputPath using errors.Is with fs.ErrNotExist instead of os.IsNotExist, adding
the io/fs import as needed. Preserve the existing nil return for missing files
and wrapped internal error handling for all other open failures.

In `@pkg/recipe/profile_resolution.go`:
- Around line 182-228: Update the constraint-processing loop in the profile
resolution function to accumulate validated value constraints in a local slice
rather than appending directly to mergedSpec.Constraints. Preserve collision and
evaluation error returns, then append the local constraints to
mergedSpec.Constraints and sort only after the entire loop completes
successfully.

In `@pkg/recipe/profile.go`:
- Around line 580-610: Update the ownership validation loop over
selected.OwnedPaths to iterate components in lexicographically sorted key order,
using the established sorted-key pattern, while preserving all existing path and
component validation behavior.

In `@pkg/serializer/reader_test.go`:
- Around line 1621-1631: Add a non-strict YAML trailing-document test case
alongside the existing format cases in the relevant test table, mirroring the
legacy non-strict JSON case with multi-document YAML input and expected
acceptance. Preserve the existing strict YAML rejection and JSON cases.

In `@pkg/serializer/reader.go`:
- Around line 618-619: In readConfigMapDataWithKubeconfigContext, replace
defaults.ConfigMapWriteTimeout with a new read-specific
defaults.ConfigMapReadTimeout constant when creating the timeout context. Define
the new constant alongside the existing ConfigMapWriteTimeout value, preserving
the current timeout setup and cancellation behavior.

In `@pkg/server/bundle_handler.go`:
- Around line 103-132: In decodeBundleRecipe, resolve and validate the v2 body
format with v2BodyFormat(contentType) before calling io.ReadAll(input). Return
the format error immediately for unsupported media types, then read and decode
the body using the validated format while preserving the existing error
propagation.

In `@pkg/server/openapi_sync_test.go`:
- Around line 288-530: Split TestOpenAPIV2BundleContract into focused t.Run
subtests for the request/response contract, BundleRecipeV2Request,
ProfileRecipeResponse, RecipeResponseBase, LegacyRecipeResponse, and
VersionlessLegacyRecipeResponse. Keep shared spec loading and schema lookup
setup in the parent test, but move each independent assertion group into its
corresponding subtest so failures remain isolated and clearly named.

In `@pkg/server/recipe_handler_test.go`:
- Around line 688-695: Update the “recipe YAML profile without content type
preserves JSON default” case in the recipe handler tests to assert only the
stable “[INVALID_REQUEST] failed to parse JSON body” prefix, rather than the
verbatim encoding/json decoder message; keep the existing invalid-request
expectation and test setup unchanged.

---

Outside diff comments:
In `@pkg/recipe/metadata.go`:
- Around line 807-822: Update the scalar/string decoding branches for
ExcludedOverlay to reject empty names, matching the existing raw.Name validation
for object forms. In the JSON path around json.Unmarshal into name, validate
name before assigning e.Name; apply the equivalent non-empty check to the YAML
scalar branch, while preserving successful decoding for non-empty values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: e3ec5c6c-6182-4d7e-a238-5c304c103877

📥 Commits

Reviewing files that changed from the base of the PR and between 8eee3c5 and 5405138.

📒 Files selected for processing (81)

• api/aicr/v1/server.yaml
• docs/contributor/api-server.md
• docs/contributor/recipe.md
• docs/design/015-recipe-configuration-profiles.md
• docs/integrator/data-flow.md
• docs/integrator/go-library.md
• docs/integrator/recipe-development.md
• docs/user/api-reference.md
• docs/user/cli-config.md
• docs/user/cli-reference.md
• pkg/bundler/bundler.go
• pkg/bundler/bundler_test.go
• pkg/bundler/config/config.go
• pkg/bundler/deployer/argocdhelm/argocdhelm.go
• pkg/bundler/deployer/argocdhelm/argocdhelm_test.go
• pkg/bundler/handler.go
• pkg/bundler/handler_test.go
• pkg/bundler/validations/checks.go
• pkg/bundler/validations/checks_test.go
• pkg/cli/consts.go
• pkg/cli/mirror.go
• pkg/cli/query.go
• pkg/cli/query_test.go
• pkg/cli/recipe.go
• pkg/cli/recipe_test.go
• pkg/client/v1/aicr.go
• pkg/client/v1/aicr_internal_test.go
• pkg/client/v1/aicr_test.go
• pkg/client/v1/bundle.go
• pkg/client/v1/gpu_driver_state.go
• pkg/client/v1/gpu_driver_state_test.go
• pkg/client/v1/stability_test.go
• pkg/client/v1/types.go
• pkg/component/overrides.go
• pkg/component/overrides_test.go
• pkg/config/accessors.go
• pkg/config/accessors_test.go
• pkg/config/config.go
• pkg/config/config_test.go
• pkg/config/validate.go
• pkg/evidence/project/synthesize.go
• pkg/evidence/project/synthesize_test.go
• pkg/mirror/discover.go
• pkg/mirror/discover_test.go
• pkg/recipe/builder.go
• pkg/recipe/catalog.go
• pkg/recipe/criteria.go
• pkg/recipe/criteria_test.go
• pkg/recipe/decode.go
• pkg/recipe/loader.go
• pkg/recipe/loader_provider_test.go
• pkg/recipe/loader_test.go
• pkg/recipe/metadata.go
• pkg/recipe/metadata_store.go
• pkg/recipe/metadata_store_test.go
• pkg/recipe/profile.go
• pkg/recipe/profile_aks_test.go
• pkg/recipe/profile_integration_test.go
• pkg/recipe/profile_resolution.go
• pkg/recipe/profile_test.go
• pkg/recipe/query.go
• pkg/recipe/query_request.go
• pkg/recipe/query_test.go
• pkg/recipe/testdata/profile-overlay/overlays/h100-eks-ubuntu-training-kubeflow.yaml
• pkg/recipe/testdata/profile-overlay/registry.yaml
• pkg/recipe/yaml_test.go
• pkg/serializer/reader.go
• pkg/serializer/reader_test.go
• pkg/server/bundle_handler.go
• pkg/server/bundle_handler_test.go
• pkg/server/consts.go
• pkg/server/doc.go
• pkg/server/openapi_sync_test.go
• pkg/server/recipe_handler.go
• pkg/server/recipe_handler_test.go
• pkg/server/serve.go
• pkg/server/serve_test.go
• pkg/server/server.go
• recipes/overlays/aks.yaml
• tools/testgrid-publish/bundle.go
• tools/testgrid-publish/bundle_test.go

[coderabbitai]

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

docs/design/015-recipe-configuration-profiles.md (1)

147-171: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove the obsolete “PR 2” constraint caveats.

The example says symmetric constraints are deferred, but this PR already makes both AKS profile values constrain K8s.aks-gpu-pools.gpu-driver. Document the current Install/None constraints here rather than leaving the profile contract incomplete.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/design/015-recipe-configuration-profiles.md` around lines 147 - 171,
Update the gpuProfile values documentation in the recipe configuration profiles
example to remove both obsolete “PR 2” caveats and explicitly document the
current constraints for driver-only/Install and operator/None against
K8s.aks-gpu-pools.gpu-driver, preserving the existing component overrides.

♻️ Duplicate comments (1)

docs/integrator/recipe-development.md (1)

425-428: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Align profile rollout documentation with AKS adoption.

These passages still describe a future/no-adopter rollout, while this PR adopts gpuStack for AKS. Update them consistently, while retaining that snapshot-qualified AKS resolution remains fail-closed until the pool-projection collector lands.

• docs/integrator/recipe-development.md#L425-L428: identify AKS as the current first adopter.
• docs/integrator/recipe-development.md#L487-L491: describe the gpuProfile.driver projection as planned and document current fail-closed behavior.
• docs/user/api-reference.md#L407-L418: remove the claim that no embedded recipe declares a profile and update the v2 example.
• docs/user/api-reference.md#L463-L464: remove the “after its profile adopter lands” wording or qualify it as a future adopter.
• docs/user/cli-reference.md#L336-L345: replace the no-adopter/external-overlay rollout description with the current AKS adoption status.

Based on the PR objectives, AKS is the first gpuStack adopter in this rollout.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/integrator/recipe-development.md` around lines 425 - 428, Align the
profile rollout documentation with AKS as the first current gpuStack adopter:
update docs/integrator/recipe-development.md lines 425-428 to identify AKS, and
lines 487-491 to mark gpuProfile.driver projection as planned while documenting
fail-closed snapshot-qualified AKS resolution; update docs/user/api-reference.md
lines 407-418 to remove the no-adopter claim and revise the v2 example, and
lines 463-464 to remove or qualify the future-adopter wording; update
docs/user/cli-reference.md lines 336-345 to describe current AKS adoption
instead of a no-adopter/external-overlay rollout.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@api/aicr/v1/server.yaml`:
- Around line 1932-1994: The criteria enums are duplicated across the v1 inline
parameters, shared components, Criteria, and CriteriaV2; consolidate them into
single reusable definitions. Update the v1 path parameters to reference the
corresponding components/parameters entries, move their detailed descriptions
into those shared definitions, and define CriteriaV2 from Criteria using allOf
with additionalProperties: false so all versions share the same enum sources.

In `@pkg/config/config.go`:
- Line 108: Document the Profile field with a Go doc comment stating that it
expects the strict name=value form accepted by recipe.ParseProfileSelection,
with names and values limited to letters, digits, dots, underscores, and
hyphens. Place the comment directly above Profile, matching the documentation
style of CriteriaStrict.

In `@pkg/config/validate.go`:
- Around line 108-111: The profile validation in the recipe validation flow must
preserve the field context when ParseProfileSelection returns an existing coded
error. Update the handling around ParseProfileSelection to explicitly attach or
prepend “invalid spec.recipe.profile” while retaining the original error code
and detail, rather than relying on PropagateOrWrap’s fallback message.

In `@pkg/recipe/profile_aks_test.go`:
- Around line 97-132: Strengthen the assertions in the profile test around
operator override extraction and constraint matching: require successful type
assertions for the driver, toolkit, and operator maps before checking their
fields, so missing overrides fail explicitly. In the constraint loop, count
every K8s.aks-gpu-pools.gpu-driver match, retain the matched value for
comparison, and require exactly one match before validating it against
tt.wantConstraint.

In `@pkg/recipe/profile_integration_test.go`:
- Around line 189-193: Update the comparison around
first.Metadata.SelectedProfile and second.Metadata.SelectedProfile to validate
both SelectedProfile values are non-nil before accessing OwnedPaths; report a
clear test failure if either selection is missing, then preserve the existing
owned-path equality assertion.

In `@pkg/server/openapi_sync_test.go`:
- Around line 342-347: Guard the LegacyBundleRecipeV2Request allOf access by
asserting that legacyBranchAllOf has exactly two entries before indexing [1],
and report a named contract failure through the test assertion. Keep the
existing legacyOverlay required-field validation unchanged after the length
check.

In `@pkg/server/recipe_handler.go`:
- Around line 143-159: Extract the duplicated bounded POST-body reading and
size-limit response logic from the current handler and parseQueryPOSTBody into a
shared helper such as readBoundedPOSTBody, preserving MaxBytesReader setup,
drain/close cleanup, MaxBytesError logging, and the 413 response with
keyLimitBytes. Update both callers to use the helper and retain their existing
handling for non-size-limit read errors.
- Around line 616-631: Review the v2 body validation flow around
validateV2EnvelopeProfile and decodeStrictV2Envelope to confirm whether the
second full-payload parse via bodyHasTopLevelProfile is necessary. If the
decoded envelope can expose omitted versus explicit null profile states, carry
that distinction through a Profile *json.RawMessage or *yaml.Node field and
remove the redundant parse; otherwise document or preserve the intentional
bounded double-parse behavior.

---

Outside diff comments:
In `@docs/design/015-recipe-configuration-profiles.md`:
- Around line 147-171: Update the gpuProfile values documentation in the recipe
configuration profiles example to remove both obsolete “PR 2” caveats and
explicitly document the current constraints for driver-only/Install and
operator/None against K8s.aks-gpu-pools.gpu-driver, preserving the existing
component overrides.

---

Duplicate comments:
In `@docs/integrator/recipe-development.md`:
- Around line 425-428: Align the profile rollout documentation with AKS as the
first current gpuStack adopter: update docs/integrator/recipe-development.md
lines 425-428 to identify AKS, and lines 487-491 to mark gpuProfile.driver
projection as planned while documenting fail-closed snapshot-qualified AKS
resolution; update docs/user/api-reference.md lines 407-418 to remove the
no-adopter claim and revise the v2 example, and lines 463-464 to remove or
qualify the future-adopter wording; update docs/user/cli-reference.md lines
336-345 to describe current AKS adoption instead of a
no-adopter/external-overlay rollout.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: c228e937-e084-48a5-a3e1-0dcf796fb56d

📥 Commits

Reviewing files that changed from the base of the PR and between 5405138 and de3f7fb.

📒 Files selected for processing (81)

• api/aicr/v1/server.yaml
• docs/contributor/api-server.md
• docs/contributor/recipe.md
• docs/design/015-recipe-configuration-profiles.md
• docs/integrator/data-flow.md
• docs/integrator/go-library.md
• docs/integrator/recipe-development.md
• docs/user/api-reference.md
• docs/user/cli-config.md
• docs/user/cli-reference.md
• pkg/bundler/bundler.go
• pkg/bundler/bundler_test.go
• pkg/bundler/config/config.go
• pkg/bundler/deployer/argocdhelm/argocdhelm.go
• pkg/bundler/deployer/argocdhelm/argocdhelm_test.go
• pkg/bundler/handler.go
• pkg/bundler/handler_test.go
• pkg/bundler/validations/checks.go
• pkg/bundler/validations/checks_test.go
• pkg/cli/consts.go
• pkg/cli/mirror.go
• pkg/cli/query.go
• pkg/cli/query_test.go
• pkg/cli/recipe.go
• pkg/cli/recipe_test.go
• pkg/client/v1/aicr.go
• pkg/client/v1/aicr_internal_test.go
• pkg/client/v1/aicr_test.go
• pkg/client/v1/bundle.go
• pkg/client/v1/gpu_driver_state.go
• pkg/client/v1/gpu_driver_state_test.go
• pkg/client/v1/stability_test.go
• pkg/client/v1/types.go
• pkg/component/overrides.go
• pkg/component/overrides_test.go
• pkg/config/accessors.go
• pkg/config/accessors_test.go
• pkg/config/config.go
• pkg/config/config_test.go
• pkg/config/validate.go
• pkg/evidence/project/synthesize.go
• pkg/evidence/project/synthesize_test.go
• pkg/mirror/discover.go
• pkg/mirror/discover_test.go
• pkg/recipe/builder.go
• pkg/recipe/catalog.go
• pkg/recipe/criteria.go
• pkg/recipe/criteria_test.go
• pkg/recipe/decode.go
• pkg/recipe/loader.go
• pkg/recipe/loader_provider_test.go
• pkg/recipe/loader_test.go
• pkg/recipe/metadata.go
• pkg/recipe/metadata_store.go
• pkg/recipe/metadata_store_test.go
• pkg/recipe/profile.go
• pkg/recipe/profile_aks_test.go
• pkg/recipe/profile_integration_test.go
• pkg/recipe/profile_resolution.go
• pkg/recipe/profile_test.go
• pkg/recipe/query.go
• pkg/recipe/query_request.go
• pkg/recipe/query_test.go
• pkg/recipe/testdata/profile-overlay/overlays/h100-eks-ubuntu-training-kubeflow.yaml
• pkg/recipe/testdata/profile-overlay/registry.yaml
• pkg/recipe/yaml_test.go
• pkg/serializer/reader.go
• pkg/serializer/reader_test.go
• pkg/server/bundle_handler.go
• pkg/server/bundle_handler_test.go
• pkg/server/consts.go
• pkg/server/doc.go
• pkg/server/openapi_sync_test.go
• pkg/server/recipe_handler.go
• pkg/server/recipe_handler_test.go
• pkg/server/serve.go
• pkg/server/serve_test.go
• pkg/server/server.go
• recipes/overlays/aks.yaml
• tools/testgrid-publish/bundle.go
• tools/testgrid-publish/bundle_test.go

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

pkg/client/v1/aicr_test.go (1)

872-878: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Unchecked nested assertion panics instead of failing readably.

If the profile fragment stops applying, values["driver"] is nil and Line 876 panics rather than reporting the mismatch.

🛡️ Proposed fix

-	if enabled := values["driver"].(map[string]any)["enabled"]; enabled != true {
-		t.Fatalf("driver.enabled = %v, want true", enabled)
+	driver, ok := values["driver"].(map[string]any)
+	if !ok {
+		t.Fatalf("gpu-operator values.driver = %#v, want map", values["driver"])
+	}
+	if enabled := driver["enabled"]; enabled != true {
+		t.Fatalf("driver.enabled = %v, want true", enabled)
 	}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@pkg/client/v1/aicr_test.go` around lines 872 - 878, Update the assertions in
the test around GetValuesForComponentWithContext to avoid unchecked nested type
assertions on values["driver"]. Safely validate that the driver entry exists and
has the expected map shape before checking enabled, and fail the test with a
readable mismatch instead of allowing a panic.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@pkg/client/v1/aicr_test.go`:
- Around line 872-878: Update the assertions in the test around
GetValuesForComponentWithContext to avoid unchecked nested type assertions on
values["driver"]. Safely validate that the driver entry exists and has the
expected map shape before checking enabled, and fail the test with a readable
mismatch instead of allowing a panic.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 33e848e7-94b4-4bd8-b6ba-4d9fc0cdda4d

📥 Commits

Reviewing files that changed from the base of the PR and between de3f7fb and 99bf520.

📒 Files selected for processing (9)

• docs/integrator/recipe-development.md
• pkg/client/v1/aicr_test.go
• pkg/recipe/loader_provider_test.go
• pkg/recipe/profile_aks_test.go
• pkg/recipe/profile_integration_test.go
• pkg/recipe/testdata/profile-overlay/overlays/h100-eks-ubuntu-training-kubeflow.yaml
• pkg/recipe/yaml_test.go
• pkg/server/recipe_handler_test.go
• recipes/overlays/aks.yaml

[njhensley]

Review — ADR-015 gpuStack profile on AKS

Method: five independent persona reviewers (Correctness, Security/fail-closed, Domain & Architecture, Operability/CI-DX, Test-coverage) followed by an adversarial senior meta-reviewer that independently re-derived every finding from the resolved code. Reviewed at 66542dac; re-verified byte-identical to the current head after a rebase onto main (the newer commits are already-merged #1973 base code, not part of this PR's diff).

Overall assessment

This is an unusually well-defended change. The projection, the bounded pool-file reader, the evidence identity binding, and the ownership lock are all hardened correctly, and the new logic is thoroughly test-pinned. No blocker or major defect survived adjudication. The one issue every reviewer converged on — a ConfigMap-rewrite failure path — was retiered from Major to Minor once the writer's Server-Side-Apply (create-or-update) semantics were confirmed: it's a narrow, transient-only robustness issue, not a deterministic bug.

CI at the reviewed commit: green across Tier 1 (all deployers × aks/aks-training/aks-inference), GPU nvkind H100/L40G, Mirror E2E, pointer-contract, CodeRabbit, and Fern.

Recommendation: Approve with comments — the 🟡 is worth addressing (or consciously accepting); the rest are optional cleanups.

Findings (detail inline)

• 🟡 Minor — pkg/snapshotter/agent.go:321: internal ConfigMap-rewrite failure aborts snapshot delivery for file/stdout output.
• 🔵 Nitpick — pkg/snapshotter/agent.go:735: rawSnapshotDoc helpers 0%-covered.
• 🔵 Nitpick — pkg/client/v1/gpu_driver_state.go:73: driverAbsentRemedy twin, 4/6 branches untested.
• 🔵 Nitpick — pkg/snapshotter/snapshot.go:367: attach/mergeAKSGPUPools append without dedup (defensive).
• 🔵 Nitpick — .github/workflows/uat-azure.yaml:526: pool-dump step lacks the az retry the client-connect step has.
• 🔵 Nitpick — stale "driver-only" terminology survives in comments/test names on pre-existing files this PR doesn't modify (pkg/recipe/aks_driver_profile_tuple_test.go:22, toolkit_hardening_gate_test.go:55/70, pkg/client/v1/aicr.go:754). Functional values are azure-managed/operator-managed throughout; cosmetic only, safe to defer.

Examined and cleared (confirmed non-issues)

• Projection semantics (aksgpupools.go): AMD markers short-circuit before NC/ND/NV prefix match; null gpuProfile→Install; managementMode: Managed; Unmanaged→driver field; Mixed on disagreement; no-GPU omission; top-level JSON null rejected.
• Bounded reader (providerpools.go): descriptor-first O_NOFOLLOW|O_NONBLOCK, IsRegular on the opened fd (no TOCTOU), dual size cap, ctx.Done() between chunks under FileReadTimeout.
• Evidence identity binding: the name-collision spoof is genuinely closed — name/profile/digest derived from manifest-verified recipe bytes after CheckInventory; suffix checks are fast-fail only.
• Ownership lock: runs on final componentValues post---set; --dynamic intersection rejected even when identical; argocd-helm install-time values covered — not bypassable.
• azure-managed = pre-PR default (byte-identical effective values); the breaking change is the added override-lock surface + /v1→/v2 cut-over, both documented.
• Selection-collision quarantine is order-independent; the profile segment is stripped before criteria inversion (no phantom platform).
• No GHA template injection in the dump step; env-var propagation wired on both snapshot and validate flags; recipe-evidence-check.sh SIGPIPE fix present.

Tier tally

🔴 Blocker 0 · 🟠 Major 0 · 🟡 Minor 1 · 🔵 Nitpick 5

[njhensley]

🟡 Minor — ConfigMap-rewrite failure aborts snapshot delivery for file/stdout output

When --aks-gpu-pools is set, deployAndWaitForResult merges the projection then hard-returns on any rewriteSnapshotConfigMap error (this line) before control reaches the file/stdout write switch in measureWithAgent (agent.go:677-698) or the return in DeployAndGetSnapshot. agentOutput is the internal cm://<ns>/aicr-snapshot URI for every case except an explicit cm:// output request, so for -o file, stdout, and every aicr validate live-capture (which consumes the returned struct, not that ConfigMap), a failed Apply to that internal ConfigMap discards an already-captured-and-merged snapshot. The rewrite is hygiene on a ConfigMap that Cleanup orphans anyway (per the comment just above).

Blast radius: The new UAT-Azure lane exports AICR_AKS_GPU_POOLS_PATH job-wide, so every aicr validate there rewrites an internal ConfigMap it never consumes — one transient apiserver throttle/409 reds an otherwise-green burn-in. For end users, aicr snapshot --aks-gpu-pools pools.json -o snapshot.yaml produces no file despite a successful capture. Kept at Minor (not Major): the writer uses Server-Side Apply (create-or-update) with the same identity that just succeeded reading the ConfigMap one step earlier, so the failure is transient and retryable, not deterministic.

Fix: Make the internal-ConfigMap rewrite best-effort (slog.Warn + continue) when the ConfigMap is not the user's requested deliverable; keep it fatal only when the user explicitly requested cm:// output (thread finalOutput/an isDeliverable flag into deployAndWaitForResult).

[yuanchen8911]

Fixed in a907d81. The rewrite now fails the run only when the ConfigMap is the delivery vehicle (explicit cm:// output, where a later consumer reads the ConfigMap). For file/stdout output and SDK callers, a rewrite failure logs a loud warning and the run delivers the returned snapshot, which already carries the merged reading; the warning names the orphaned pre-merge ConfigMap. deployAndWaitForResult takes an explicit deliverViaConfigMap flag from both call sites.

[njhensley]

🔵 Nitpick — rawSnapshotDoc.MarshalYAML/GetKind/GetMetadata are 0%-covered

These three methods are only reachable through the live ConfigMap-rewrite path; TestRewriteSnapshotConfigMapRejectsBadInput exercises only the two early-return guards before the struct is constructed. They populate the rewritten ConfigMap's kind/metadata labels — the version-skew preservation this whole path protects — so a wrong key or type-assertion would silently write empty labels, uncaught.

Blast radius: Low: unexported methods on an unexported type (no coverage-gate block). A regression in label preservation would only surface downstream.

Fix: Add a cluster-free table test that builds a rawSnapshotDoc{doc: {...}} and asserts GetKind/GetMetadata/MarshalYAML round-trip.

[yuanchen8911]

Added TestRawSnapshotDocRoundTrip in a907d81: cluster-free table covering GetKind/GetMetadata/MarshalYAML round-trip on a representative document, non-string metadata values being skipped (not stringified), and mistyped kind/metadata degrading to empty without panicking.

[njhensley]

🔵 Nitpick — client-side driverAbsentRemedy has 4 of 6 branches untested (twin can silently diverge)

driverAbsentRemedy is a deliberate twin of the bundler copy (pkg/bundler/validations/checks.go:400) and the two are currently byte-identical (verified, 51 lines each). The bundler twin is fully branch-tested; this copy exercises only the AKS-profiled and GKE-COS branches (via TestResolveRecipeFromSnapshot_GPUDriverAutoDetect), leaving legacy-!profiled-AKS, GKE-Ubuntu, GKE-default, and generic untested. The CLAUDE.md 'keep both copies in sync' contract is thus test-enforced on one side only.

Blast radius: Low: output is advisory slog.Warn remedy text, no fail-closed logic. But the twins could drift on the legacy-AKS/GKE-Ubuntu/generic wording without a test catching it.

Fix: Add a small direct table test over {service, os, profiled} asserting a stable substring per branch, mirroring the bundler's existing rows.

[yuanchen8911]

Added TestDriverAbsentRemedyBranches in a907d81: a direct table over all six {service, os, profiled} branches with a stable substring per branch, plus two twin-drift guards - the profiled AKS remedy must differ from the legacy one and must not offer the bundle-time tuple.

[njhensley]

🔵 Nitpick — attach/mergeAKSGPUPools append the subtype without dedup

attachAKSGPUPools (here) and mergeAKSGPUPools (agent.go:371) append the aks-gpu-pools subtype unconditionally. Each runs once per invocation and no collector emits this subtype, so a duplicate only arises from hand-crafted snapshot input — not reachable in the live/agent flow.

Blast radius: Not reachable today; pure defensive observation.

Fix: Optional: use replace-or-append to fully close the hand-crafted-input ambiguity.

[yuanchen8911]

Agreed on reachability: each function runs once per invocation on the freshly captured document, no collector emits this subtype, and user-supplied snapshot files never pass through either path, so a duplicate cannot arise in a supported flow. Leaving the append as-is to keep the merge semantics minimal; replace-or-append can ride along if hand-crafted snapshot mutation ever becomes a supported input.

[njhensley]

🔵 Nitpick — Pool-dump step has no az retry, unlike the client-connect step

'Dump AKS GPU pool modes' runs set -euo pipefail with a single un-retried az aks nodepool list, while the immediately-preceding client-connect step retries 10x. Because this step exports AICR_AKS_GPU_POOLS_PATH consumed by all downstream phases, one transient az throttle reds an otherwise-green burn-in.

Blast radius: Fail-closed (not fail-open), so low severity — a CI flake surface, not a correctness risk.

Fix: Wrap the az call in the same bounded retry loop the client-connect step already uses.

[yuanchen8911]

Fixed in a907d81: the pool dump is now wrapped in the same bounded retry shape as the client-connect step (5 attempts, 30s apart) and fails the step explicitly with an ::error:: annotation after the last attempt.

[njhensley]

Re-review — ADR-015 gpuStack profile on AKS (APPROVE)

Re-review of the fixes pushed since my prior review (which was dismissed on push). The head moved via a squash-rebase; the true content delta is 4 files, all directly addressing the earlier findings. I verified each fix against the resolved code and checked the delta for regressions — none found.

Prior-feedback status

Prior finding	Status	Evidence
🟡 ConfigMap-rewrite aborts file/stdout snapshot delivery (agent.go)	✔️ Addressed	deployAndWaitForResult now takes deliverViaConfigMap; rewrite failure is fatal only when the ConfigMap is the deliverable, else slog.Warn + continue with the merged bytes. measureWithAgent sets it from HasPrefix(finalOutput, cm://); the SDK path passes false. Both (and only) two callers updated.
🔵 rawSnapshotDoc helpers 0%-covered (agent.go)	✔️ Addressed	TestRawSnapshotDocRoundTrip pins GetKind/GetMetadata (incl. non-string-value skip) and MarshalYAML round-trip.
🔵 driverAbsentRemedy twin 4/6 branches untested (gpu_driver_state.go)	✔️ Addressed	TestDriverAbsentRemedyBranches covers all six branches, plus legacy≠profiled distinctness and no-tuple-on-profiled.
🔵 Pool-dump lacks az retry (uat-azure.yaml)	✔️ Addressed	5-attempt bounded retry (30s backoff) mirroring the client-connect step; ::error:: + exit 1 on exhaustion; set -e-safe and injection-free.
🔵 attach/mergeAKSGPUPools append without dedup	◻︎ Consciously declined	Defensive only, not reachable in the live/agent flow — reasonable to leave.
🔵 Stale "driver-only" terminology	◻︎ Consciously declined	Cosmetic, on pre-existing files this PR doesn't modify — reasonable to leave.

Verification of the C1 fix (the only behavior change)

Exactly two callers of deployAndWaitForResult, both updated: the SDK path (false) returns the merged bytes on a rewrite failure instead of discarding the snapshot, and measureWithAgent fails fatally only when the user explicitly requested cm:// output. No other callers; no regression in the retry loop or the new tests.

One residual (follow-up grade, non-blocking — inline)

The new best-effort warn-branch itself is untested. Noted inline; optional.

Net

Four findings fixed correctly, two optional nits consciously declined, zero new defects, no blocker/major. Approving.

🔴 Blocker 0 · 🟠 Major 0 · 🟡 Minor 0 · 🔵 Nitpick 1 (residual, non-blocking)

[njhensley]

🔵 Nitpick — Best-effort ConfigMap-rewrite branch is untested

This new best-effort branch — deliverViaConfigMap == false and rewriteSnapshotConfigMap fails, so we slog.Warn and return the already-merged bytes — is the behavior that resolved the prior C1 finding, but nothing exercises it. TestRewriteSnapshotConfigMapRejectsBadInput still stops at the early guards, and no test drives deployAndWaitForResult with a clientset whose Apply fails.

Blast radius: Low: the branch is a slog.Warn + return, and the fatal counterpart (cm:// deliverable) is the risky direction. But a future refactor could silently make it fatal again — the exact regression this fix closed — without a test catching it.

Fix: Add a snapshotter test with a fake clientset whose ConfigMap Apply returns an error, asserting that with a file/stdout finalOutput the merged snapshot is still returned (warn, not error), and that with a cm:// finalOutput the error propagates. Optional / follow-up grade.

[yuanchen8911]

Pinned in a1df994. The branch is extracted into rewriteMergedSnapshotConfigMap(ctx, uri, kubeconfig, data, deliverViaConfigMap), which encodes the delivery contract in one place, and TestRewriteMergedSnapshotConfigMapDeliveryContract asserts both directions: deliverViaConfigMap=true propagates the rewrite error, false warns and returns nil. Failure is injected via an invalid cm URI (fails inside rewriteSnapshotConfigMap before any cluster access), which exercises the same branch a fake-clientset Apply failure would without the fixture weight — a future refactor that makes the best-effort side fatal again now fails this test.

[njhensley]

Re-review — ADR-015 gpuStack profile on AKS (APPROVE)

Re-review of the fix pushed since my prior approval (dismissed on push). The head moved via a squash-rebase; the true content delta is 2 files (pkg/snapshotter/agent.go, pkg/snapshotter/aksgpupools_test.go), addressing the last open item.

Prior-feedback status — all resolved

Prior finding	Status
🟡 ConfigMap-rewrite aborts file/stdout snapshot delivery	✔️ Addressed (best-effort rewrite; fatal only for cm:// deliverable)
🔵 rawSnapshotDoc helpers 0%-covered	✔️ Addressed (TestRawSnapshotDocRoundTrip)
🔵 driverAbsentRemedy twin 4/6 branches untested	✔️ Addressed (TestDriverAbsentRemedyBranches, all 6 + distinctness)
🔵 Pool-dump lacks az retry	✔️ Addressed (5-attempt bounded retry, set -e-safe, injection-free)
🔵 Best-effort rewrite branch untested (this push)	✔️ Addressed (extracted to rewriteMergedSnapshotConfigMap; TestRewriteMergedSnapshotConfigMapDeliveryContract pins both branches)
🔵 attach/mergeAKSGPUPools append without dedup	◻︎ Consciously declined (defensive, unreachable)
🔵 Stale "driver-only" terminology	◻︎ Consciously declined (cosmetic, pre-existing files)

Verification

The delivery-contract branch is now a named helper (rewriteMergedSnapshotConfigMap, agent.go:722) called cleanly at the single site; behavior is identical to the prior head, the low-level rewriteSnapshotConfigMap is unchanged, and the new test injects a rewrite failure via an invalid URI (failing before any cluster access) to pin both deliverViaConfigMap directions. No regressions in the delta.

Net

All actionable findings resolved; two optional nits consciously declined; zero new defects; no blocker/major. Approving — no inline comments this pass.

🔴 Blocker 0 · 🟠 Major 0 · 🟡 Minor 0 · 🔵 Nitpick 0 (open)