ci(deploy-camunda): native --extra-values on matrix run with per-scenario overrides

[Ian-wang-liyang]

Which problem does the PR fix?

Closes #6312.

Two gaps remained after #6375 shipped the --extra-values CLI flag:

1. Upgrade flow never forwarded the file in CI — the workflow had a NOTE saying it was intentionally omitted, pending feat(deploy-camunda): native --extra-values on matrix run, with per-scenario overrides #6312.
2. Per-scenario overrides were not implemented — scenarios couldn't declare their own extra values files.

What's in this PR?

Gap B — wire upgrade flow (test-integration-runner.yaml)

The CLI already applied --extra-values to Step 2 only (runner_upgrade.go nils Step 1's copy), so the run-built image tag correctly lands on the upgraded chart without contaminating the previous-version install. The workflow just needed wiring: add EXTRA_VALUES to the upgrade step env, tee to /tmp/extra-values-file.yaml, build EXTRA_VALUES_ARGS, inject into the matrix run invocation — mirroring the existing install-flow pattern above it.

Gap A — per-scenario extra-values

Scenarios can now declare their own values files:

# charts/camunda-platform-8.10/test/ci/registry/scenarios/my-scenario.yaml
extra-values:
  - values/extra/image-override.yaml

appendScenarioExtraValues resolves relative paths against chart-full-setup and appends them after the global --extra-values. Precedence (last wins in Helm's -f merge):

chart defaults < global --extra-values < per-scenario extra-values < scenario layers

The field threads through: registryScenario → CIScenario → Entry → DeploymentFlags.ExtraValues. The Step-1 nil in runner_upgrade.go already clears the full merged slice, so both global and per-scenario files apply to Step 2 only in upgrade flows.

registry_validator.go catches dangling relative paths at load time (parity with checkFeature/checkDep). Absolute paths pass through — they're runtime-supplied.

Tests

Four new tests covering all three propagation hops:

• TestLoadRegistryCarriesExtraValues — YAML → CIScenario
• TestGenerate_PropagatesExtraValues — CIScenario → Entry via Generate
• TestAppendScenarioExtraValues — precedence order + path resolution
• TestRegistryValidatorRejectsMissingExtraValues — validator rejects dangling relative, passes absolute

Also debloated three --help flag descriptions in cmd/matrix.go.

Checklist

Before opening the PR:

• In the repo's root dir, run make go.update-golden-only.
• There is no other open pull request for the same update/change.
• Tests for charts are added (if needed).
• In-repo documentation are updated (if needed).

After opening the PR:

• Did you sign our CLA (Contributor License Agreement)? It will show once you open the PR.
• Did all checks/tests pass in the PR?

[Copilot]

Pull request overview

This PR completes native --extra-values support for deploy-camunda matrix run by wiring workflow upgrade flows to pass the extra values file and by adding per-scenario extra-values declarations that are propagated from the scenario registry through to runtime flags.

Changes:

• Add per-scenario extra-values to the registry schema and propagate it through CIScenario → Entry → flags.Deployment.ExtraValues, with defined precedence (global first, per-scenario next).
• Update the integration test upgrade workflow to forward workflow extra-values via the native --extra-values flag (Step 2 only for two-step upgrades).
• Add unit tests covering loader propagation, Generate propagation, precedence/path resolution, and validator behavior for missing extra-values.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scripts/deploy-camunda/matrix/runner_upgrade.go	Clarifies and enforces that upgrade Step 1 ignores all extra-values (global + per-scenario).
scripts/deploy-camunda/matrix/runner_execute.go	Appends scenario-declared extra-values after global extra-values when constructing DeploymentFlags.
scripts/deploy-camunda/matrix/registry.go	Extends scenario registry parsing/plumbing to include extra-values.
scripts/deploy-camunda/matrix/registry_validator.go	Validates scenario extra-values references at load time (relative under chart-full-setup; absolute skipped).
scripts/deploy-camunda/matrix/registry_test.go	Adds unit tests for registry load/generate propagation and validator behavior for extra-values.
scripts/deploy-camunda/matrix/matrix.go	Adds Entry.ExtraValues and copies it from CIScenario during generation.
scripts/deploy-camunda/matrix/matrix_test.go	Adds a unit test pinning precedence/path resolution for per-scenario extra-values.
scripts/deploy-camunda/matrix/config.go	Adds CIScenario.ExtraValues to represent scenario-declared extra-values in config.
scripts/deploy-camunda/cmd/matrix.go	Tweaks --help text for flags including --extra-values.
.github/workflows/test-integration-runner.yaml	For upgrade flows, persists extra-values and forwards them via --extra-values to matrix run.

[eamonnmoloney]

crev review

PR #6429 correctly wires --extra-values into the upgrade CI flow and adds well-guarded per-scenario extra-values support. The implementation is architecturally sound: the Step 1 nil-out in runner_upgrade.go, the checkExtraValues path-traversal guard, and the appendScenarioExtraValues helper are all correct and backed by tests. Two issues warrant attention: (1) the new extra-values scenario YAML field extends a closed enumeration defined in ADR 0093 without a formal ADR amendment — the ADR anticipated this via the #6312 reference but anticipation is not a formal amendment; (2) TestExtraValues_UpgradeStep1Cleared should be extended to include a pre-scenario path in its input slice so the merged-slice ordering invariant (that Step 1 receives nil even when per-scenario paths are present) is explicitly falsifiable.

Specialists run: correctness, api-stability, adr-conformance, test-adequacy, devils-advocate, verifier, escalation-assessor. Devil's-advocate hypotheses: 7 raised, 0 promoted.

_{Hypotheses by stance: adversarial-input=2 author-blind-spot=1 claim-vs-code=1 missing-case=2 scope-discipline=1 · by disposition: covered_by_specialist=1 dropped_low_severity=2 dropped_rubric=3 dropped_ungroundable=1}

---

Escalation: AI review sufficient (score: 0.45, threshold: 0.50). Two P1 findings (upgrade job behavior change now verified as intentional fix, ADR 0093 conformance gap) and external authorship drive moderate risk. Security surface touched via workflow handling potentially sensitive EXTRA_VALUES (tee removal is a security improvement). No NEVER rules violated. Score 0.45 is below threshold; reviewers should focus on ADR 0093 amendment and the test coverage gap for the merged-slice upgrade invariant.

---

Findings on lines outside this PR's diff:

• P2 scripts/deploy-camunda/matrix/matrix_test.go:2079 — TestExtraValues_UpgradeStep1Cleared does not exercise the merged-slice ordering invariant introduced by this PR
  TestExtraValues_UpgradeStep1Cleared constructs baseFlags with a single global extra-values path:

  baseFlags := &config.RuntimeFlags{Deployment: config.DeploymentFlags{ExtraValues: []string{"/tmp/engine-image.yaml"}}}

  Before this PR that was sufficient. After this PR, flags.Deployment.ExtraValues arrives at executeTwoStepUpgrade as a merged slice — global paths followed by resolved per-scenario paths — because appendScenarioExtraValues runs inside executeEntry before the upgrade function is called. The nil-out at runner_upgrade.go:186 correctly zeros the whole merged slice. But because the test's input carries only one global path with no per-scenario entry, it does not confirm that per-scenario paths are absent from Step 1.

  Critically, the test would still pass if appendScenarioExtraValues were accidentally moved to run inside executeTwoStepUpgrade after the nil-out — which would silently inject per-scenario extra-values into Step 1, contaminating the previous-version install baseline that the PR comment explicitly protects.

  Recommendation: Extend TestExtraValues_UpgradeStep1Cleared so baseFlags.Deployment.ExtraValues contains both a global path and a pre-resolved per-scenario path (matching the post-appendScenarioExtraValues production state), then assert step1Flags.Deployment.ExtraValues == nil. No real executeEntry call is needed — the existing isolation approach is fine — but the input must reflect the merged state to make the ordering invariant falsifiable.

[eamonnmoloney]

see comments

[github-actions]

PR Preview Action v1.8.1
🚀 View preview at https://camunda.github.io/camunda-platform-helm/camunda-platform-helm/pr-preview/pr-6429/
Built to branch gh-pages at 2026-06-25 21:40 UTC. Preview will be ready when the GitHub Pages deployment is complete.

[Ian-wang-liyang]

Both findings addressed:

P1 (ADR 0093) — aed7bde adds extra-values to the closed field enumeration in ADR 0093's layout table and adds a validation bullet documenting path-resolution semantics (relative → chart-full-setup, absolute paths are runtime-supplied and not validated at load time).

P2 (TestExtraValues_UpgradeStep1Cleared) — same commit extends the test so baseFlags.Deployment.ExtraValues carries a merged slice (global path + pre-resolved per-scenario path), matching the post-appendScenarioExtraValues production state. The nil-out invariant is now falsifiable: a refactor that accidentally moves appendScenarioExtraValues inside executeTwoStepUpgrade after the nil-out would fail the test.