ci: wire useVersionsNumbers to --use-latest for RC release version pinning#6253
ci: wire useVersionsNumbers to --use-latest for RC release version pinning#6253hilalbursalii wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the useVersionsNumbers workflow input so that setting it to true actually deploys pinned RC release versions from values-latest.yaml. Previously the flag was a no-op: the workflow set TEST_IMAGE_TAGS but never passed --use-latest, and even if it had, scenarios with image-tags: true would have blocked the values-latest.yaml overlay.
Changes:
- Adds
effectiveImageTags()helper inrunner.gothat suppresses scenarioimage-tags: truewhen--use-latestis set, wired into dry-run, coverage, and runtime selection paths. - In
test-integration-runner.yaml, both install and upgradematrix runblocks now branch onTEST_IMAGE_TAGS: true → pass--use-latest; false → loadVALUES_CONFIGenv-file as before.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| scripts/deploy-camunda/matrix/runner.go | Adds effectiveImageTags helper and applies it at three call sites to override scenario image-tags when --use-latest is used. |
| .github/workflows/test-integration-runner.yaml | Install and upgrade jobs branch on TEST_IMAGE_TAGS to pass --use-latest (and skip env-file) or keep prior env-file substitution behavior. |
…nning TEST_IMAGE_TAGS was set as an env var but never consumed by deploy-camunda matrix run, making useVersionsNumbers a no-op. Additionally, all qa-* scenarios have image-tags: true hardcoded which blocked --use-latest via the `if !entry.ImageTags` guard in resolveChartRootOverlaysQuiet. Fix the two-part bug: - runner.go: add effectiveImageTags() helper that returns false when opts.UseLatest is true, overriding any scenario-level image-tags: true. Applied at all three BuildDeploymentConfig/SelectionFlags call sites so values-latest.yaml is actually selected when --use-latest is passed. - test-integration-runner.yaml: branch on TEST_IMAGE_TAGS in both the install and upgrade matrix run blocks. When true, pass --use-latest and skip the env-file (values-latest.yaml already has pinned RC versions). When false, load the VALUES_CONFIG env-file for SNAPSHOT tag substitution as before. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-tags: true The previous commit wired effectiveImageTags() into BuildDeploymentConfig and SelectionFlags but missed the ChartRootOverlays closure in executeEntry and the resolveChartRootOverlaysQuiet dry-run helper. Both still used raw entry.ImageTags, so values-latest.yaml was never added to the Helm values stack even when --use-latest was passed — the scenario-level image-tags: true silently blocked it. Apply effectiveImageTags() at those two remaining sites so that UseLatest takes priority end-to-end: base-image-tags.yaml is excluded AND values-latest.yaml is included when --use-latest is set. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e-latest Previous commit had the condition backwards. useVersionsNumbers=false (the default) means "trust the RC" — the HC should use values-latest.yaml with its pinned release versions. useVersionsNumbers=true means "use the explicit version numbers from the workflow inputs" — load the VALUES_CONFIG env-file for SNAPSHOT tag substitution. Flip the TEST_IMAGE_TAGS branch in both install and upgrade matrix run blocks: != "true" → --use-latest, == "true" → env-file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
90521d9 to
40a2e78
Compare
bkenez
left a comment
There was a problem hiding this comment.
🔴 This PR breaks release artifact immutability testing
The core use case for useVersionsNumbers=false + helmChartVersion=13.10.0-rc is:
Deploy the OCI chart exactly as released — no image version overrides from any source — to validate the immutable release artifact.
This PR replaces one wrong override (base-image-tags.yaml → SNAPSHOT tags) with another wrong override (values-latest.yaml → git checkout tags). Both violate the same contract.
Why values-latest.yaml from the git checkout is wrong here
- The OCI chart
13.10.0-rcwas built with specific versions baked into itsvalues.yamlat release time values-latest.yamlonmainis continuously updated (renovate, manual bumps) and will drift from what was baked into the RC- Overlaying git-checkout versions onto an OCI artifact defeats the purpose of testing the release artifact — you'd be testing a Frankenstein of "OCI chart templates + git checkout image versions"
- The git checkout is wholly unnecessary when an OCI tag is specified
What the correct behavior should be
When include-image-tags=false (i.e., useVersionsNumbers=false) AND ChartRef is set (OCI deploy):
- No
base-image-tags.yaml(no SNAPSHOT overrides) ← PR gets this right - No
values-latest.yaml(no git checkout overrides) ← PR gets this wrong - No
values-digest.yaml(no digest pins) - No chart-root overlay at all — the OCI chart's baked-in
values.yamlis the single source of truth
The actual root cause this PR does not address
image-tags: true hardcoded in every qa-* scenario in ci-test-config.yaml (added by #6097). This makes include-image-tags: false a dead input — the scenario config always wins. The proper fix:
- Wire
TEST_IMAGE_TAGSenv var →RunOptions(same pattern asUseQA) so it is a runtime override that trumps scenario config - When
effectiveImageTags=falseANDChartRef != ""→ emptyChartRootOverlays(no overlay files at all) - Remove
image-tags: truefromci-test-config.yamlscenarios — it should be a runtime concern, not a per-scenario hardcode
// Correct overlay selection:
if !effectiveImageTags {
if opts.ChartRef != "" {
// OCI: chart has baked-in versions, no overlay needed
} else if useLatest {
overlays = append(overlays, "latest")
} else {
overlays = append(overlays, "digest")
}
}This preserves the existing nightly SNAPSHOT behavior (runtime TEST_IMAGE_TAGS=true enables image tags) while restoring the ability to cleanly test immutable OCI artifacts (TEST_IMAGE_TAGS=false + helmChartVersion = zero overrides).
|
@eamonnmoloney Since you applied recent changes, could you apply review fixes? |
|
I agree with @bkenez s comment and think we should directly implement the longterm fix: #6258 If I get it right, after the above mentioned PR is merged:
|
Trigger this run, it uses release versions not SNAPSHOTs: https://github.com/camunda/c8-cross-component-e2e-tests/actions/runs/26570390947/job/78275804581 ✅
What went wrong
1.
TEST_IMAGE_TAGSwas a dead env varThe
useVersionsNumbersinput inc8-cross-component-e2e-testsis forwarded asinclude-image-tags→TEST_IMAGE_TAGSintest-integration-runner.yaml. However,TEST_IMAGE_TAGSwas set as a shell environment variable but never consumed bydeploy-camunda matrix run. The flag had zero effect — the HC always deployed with SNAPSHOT builds from the env-file regardless of whatuseVersionsNumberswas set to.2. Scenario-hardcoded
image-tags: trueblockedvalues-latest.yamlAll
qa-*scenarios inci-test-config.yamlhaveimage-tags: truehardcoded. The matrix runner checkedif !entry.ImageTagsbefore selectingvalues-latest.yamlorvalues-digest.yamlas chart-root overlays. Becauseentry.ImageTagswas alwaystrue,--use-latestwas silently ignored even if it had been passed —values-latest.yamlwas never included in the Helm values stack.3. Same
entry.ImageTagsguard was in four placesThe raw
entry.ImageTagswas used at threeBuildDeploymentConfig/SelectionFlagscall sites and inside theChartRootOverlaysclosure inexecuteEntry— the closure being the critical one that actually determines which overlay file (values-latest.yamlvsvalues-digest.yaml) is passed to Helm. All four needed to respectopts.UseLatest.What the fix does
Intended semantics of
useVersionsNumbers:true(default) — use the explicit version numbers supplied as workflow inputs; load them via env-file sobase-image-tags.yamlsubstitution applies SNAPSHOT/custom tagsfalse— trust the RC; usevalues-latest.yaml(pinned release versions, e.g.camunda:8.8.24) and skip the env-filerunner.go— addseffectiveImageTags(entryImageTags, useLatest bool) boolwhich returnsfalsewhenopts.UseLatest=true, overridingimage-tags: truefrom the scenario config. Applied at all fourentry.ImageTagssites (BuildDeploymentConfig×2,SelectionFlags, and theChartRootOverlaysclosure) sovalues-latest.yamlis actually selected andbase-image-tags.yamlis excluded when--use-latestis passed.test-integration-runner.yaml— bothmatrix runblocks (install + upgrade) now branch onTEST_IMAGE_TAGS:TEST_IMAGE_TAGS=false(useVersionsNumbers=false) → passes--use-latest, skips env-file → HC usesvalues-latest.yamlpinned RC versionsTEST_IMAGE_TAGS=true(useVersionsNumbers=true, default) → loadsVALUES_CONFIGenv-file for$E2E_TESTS_*_IMAGE_TAGsubstitution → existing SNAPSHOT behaviour unchangedThis also works when
helmChartVersionpoints to an OCI artifact (e.g.13.10.0-rc): the chart is pulled from OCI butvalues-latest.yamlis still resolved from the local git checkout, and its versions match the OCI chart exactly.Test plan
SM On-Demand 8.8+ InstallwithuseVersionsNumbers: falseandhelmChartVersion: 13.10.0-rc— verify HC pods use pinned versions fromvalues-latest.yaml(e.g.camunda/camunda:8.8.24), not SNAPSHOT tagsuseVersionsNumbers: trueand explicit SNAPSHOT inputs — verify HC pods use the supplied SNAPSHOT tags🤖 Generated with Claude Code