fix(bundler): stop dry-run previewing a stale cached chart archive

[lockwobr]

Summary

Gate the cached-chart reuse branch in install-upstream-helm.sh.tmpl on DRY_RUN_FLAG, so a dry run previews the chart the next real run will fetch instead of an archive left behind by an earlier deploy.

Motivation / Context

apply-crds.sh pulls the pinned chart once and leaves it at .aicr-chart.tgz, and install.sh installs from that exact file so both phases are bound to one artifact. That part is deliberate: re-resolving CHART/VERSION would be a second fetch that a mutable tag does not promise returns the same bytes.

But apply-crds.sh is skipped under --dry-run, because a dry run must not touch the cluster:

if [[ -z "${DRY_RUN_FLAG:-}" ]]; then
  bash ./apply-crds.sh
fi

while the reuse branch below it was unconditional:

if [[ -f "${SCRIPT_DIR}/.aicr-chart.tgz" ]]; then

So on a dry run no pull happens, and any archive on disk is by definition left over from a previous real deploy. If the pin has moved since, the preview shows the wrong chart. Worse than the ticket describes: the reuse branch also clears CHART_VERSION_ARGS and REPO, so the previewed command silently loses --version too. A dry run is the one operation whose entire purpose is to tell you what is about to happen.

Fixes: #2842
Related: #2853 (touches the same template; expect a golden conflict on whichever lands second)

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Refactoring (no functional changes)
• Build/CI/tooling

Component(s) Affected

• CLI (cmd/aicr, pkg/cli)
• API server (cmd/aicrd, pkg/server)
• Recipe engine / data (pkg/recipe)
• Bundlers (pkg/bundler, pkg/component/*)
• Collectors / snapshotter (pkg/collector, pkg/snapshotter)
• Validator (pkg/validator)
• Core libraries (pkg/errors, pkg/k8s)
• Docs/examples (docs/, examples/)
• Other: ____________

Implementation Notes

One condition, matching the one that already gates the pull, so the two halves of the handoff agree about when an archive exists.

install-local-helm.sh.tmpl was checked and has no equivalent branch: it installs from the vendored chart directory and never consults .aicr-chart.tgz, so the upstream template is the only one affected.

Note this was not closed by the private-mktemp -d rework that landed in #2725. That hardened which archive the pull selects; this is about the install trusting a file the current invocation never produced.

Testing

go test -race ./pkg/bundler/deployer/localformat/
go test ./pkg/bundler/ -run TestStockRenderParityGolden
golangci-lint run -c .golangci.yaml ./pkg/bundler/deployer/localformat/...

All pass; linter reports 0 issues.

The regression renders a bundle, plants a .aicr-chart.tgz, runs install.sh with DRY_RUN_FLAG=--dry-run under a stub helm that records argv, and asserts the upgrade resolved the pinned coordinates. It asserts positively on both the chart ref and --version, not merely on the absence of the archive path, because dropping --version is the second symptom. It also fails if the upgrade call was never reached, so it cannot pass vacuously.

Mutation-verified. Reverting the guard reproduces the defect exactly:

--- FAIL: TestInstallScript_DryRunIgnoresStaleChartArchive
    dry-run install used the stale cached archive instead of resolving the pinned
    chart coordinates: upgrade --install k8s-aibom .../001-k8s-aibom/.aicr-chart.tgz
    --namespace k8s-aibom-system --create-namespace -f values.yaml
    -f cluster-values.yaml --dry-run

Note the absent --version in that output: both symptoms in one line.

Risk Assessment

• Low — Isolated change, well-tested, easy to revert
• Medium — Touches multiple components or has broader impact
• High — Breaking change, affects critical paths, or complex rollout

One shell condition. Real installs are unaffected, since DRY_RUN_FLAG is empty there and the branch behaves exactly as before. The only behavior that changes is the dry run, which now resolves CHART/VERSION the way the subsequent real run will.

Rollout notes: N/A.

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)

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: NVIDIA/aicr/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0cd6279b-2640-44c7-997b-ead892bff743

📥 Commits

Reviewing files that changed from the base of the PR and between 3e8ffd7 and 7b1c149.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: NVIDIA/aicr/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 3716917b-a2b3-4567-a9da-ed072bf1f114

📥 Commits

Reviewing files that changed from the base of the PR and between dd718ea and 3e8ffd7.

📒 Files selected for processing (1)

• pkg/bundler/testdata/stock_render_golden.yaml

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.

---

📝 Walkthrough

Walkthrough

The Helm template and generated install scripts now reuse .aicr-chart.tgz only during non-dry-run installations. Dry runs retain the configured chart reference and version. A regression test verifies this behavior with a stale archive. The stock-render golden manifest contains updated artifact digests.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: coffeepac

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main fix: preventing dry runs from using a stale cached chart archive.
Description check	✅ Passed	The description directly explains the defect, the implementation, the regression test, and the expected behavior for dry runs and real installs.
Linked Issues check	✅ Passed	For #2842, install-upstream-helm.sh.tmpl now uses .aicr-chart.tgz only when DRY_RUN_FLAG is unset and the archive exists. Dry runs therefore resolve the pinned chart coordinates and preserve the…
Out of Scope Changes check	✅ Passed	The rendered install.sh fixtures and regenerated pkg/bundler/testdata/stock_render_golden.yaml reflect the template change. The new test supports #2842. No unrelated change is demonstrated.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

---

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

[github-actions]

Recipe evidence check

No leaf overlays affected by this PR.

This gate is warning-only and never blocks merge.

[github-actions]

Coverage Report ✅

Metric	Value
Coverage	84.9%
Threshold	83%
Status	Pass

Coverage Badge

![Coverage](https://img.shields.io/badge/coverage-84.9%25-brightgreen)

No Go source files changed in this PR.

[mchmarny]

Approve: no findings against 982d560. Required checks and focused dry-run/install tests pass at the reviewed SHA.