Skip to content

ci: prevent image overrides for OCI chart deploys#6258

Merged
bkenez merged 2 commits into
mainfrom
ci/prevent-oci-image-overrides
May 29, 2026
Merged

ci: prevent image overrides for OCI chart deploys#6258
bkenez merged 2 commits into
mainfrom
ci/prevent-oci-image-overrides

Conversation

@bkenez

@bkenez bkenez commented May 28, 2026

Copy link
Copy Markdown
Member

Summary

  • Add an OCI immutability guard to deploy-camunda matrix run.
  • When --chart-ref is set, deploy the chart artifact with its baked-in image versions by default.
  • Suppress all image-version override sources in OCI mode:
    • base-image-tags.yaml
    • values-digest.yaml
    • values-latest.yaml
    • values-enterprise.yaml
    • *_IMAGE_TAG keys from env files
  • Add --force-image-overrides as an explicit escape hatch for advanced debugging.

Root Cause

helmChartVersion is intended to test the immutable OCI chart artifact as released, but the SM workflow path could still override image versions from local git checkout state.

This happened through multiple independent paths:

  • image-tags: true in ci-test-config.yaml forced base-image-tags.yaml
  • VALUES_CONFIG always carried E2E_TESTS_*_IMAGE_TAG values
  • ChartRootOverlays could still add values-digest.yaml or values-latest.yaml

This regressed the OCI artifact validation flow seen in:

Related

Validation

  • go test ./matrix ./cmd from scripts/deploy-camunda
  • Dry-run with --chart-ref ... --chart-version 13.10.0-rc shows no image override layers
  • Dry-run with --force-image-overrides restores image override behavior intentionally

Copilot AI review requested due to automatic review settings May 28, 2026 15:10
@bkenez bkenez requested a review from a team as a code owner May 28, 2026 15:11
@bkenez bkenez requested review from hisImminence and removed request for a team May 28, 2026 15:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an OCI chart immutability guard to deploy-camunda matrix run, ensuring published chart artifacts are deployed with their baked-in image versions unless explicitly overridden.

Changes:

  • Adds --force-image-overrides and threads it through matrix run options and workflows.
  • Suppresses image-tag layers, chart-root image overlays, and *_IMAGE_TAG env-file values when --chart-ref is used.
  • Adds unit coverage for OCI immutability behavior and env-file sanitization.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
scripts/deploy-camunda/matrix/runner.go Implements OCI immutability mode, image-tag suppression, overlay resolution, and env-file sanitization.
scripts/deploy-camunda/matrix/matrix_test.go Adds tests covering image override suppression and sanitized env-file output.
scripts/deploy-camunda/cmd/matrix.go Adds the --force-image-overrides CLI flag and passes it into matrix run options.
.github/workflows/test-integration-template.yaml Adds the reusable workflow input for forcing image overrides.
.github/workflows/test-integration-runner.yaml Wires the new input into install/upgrade jobs and filters image-tag values in OCI mode.

@hisImminence

Copy link
Copy Markdown
Contributor

Code review findings from reading the branch directly. Overall the approach is correct and well-structured — a few things worth addressing before merge.


Critical

values-enterprise.yaml comment is misleading. The comment in resolveChartRootOverlays says enterprise "changes registry/repo, not tags" — but values-enterprise.yaml does set image tags (Bitnami sub-chart images: Keycloak, PostgreSQL, etc.). The suppression in OCI mode is correct behaviour, but the comment will mislead the next person who reads it. Suggest: // OCI artifacts bake all image versions; skip overlays that would override them (includes enterprise sub-chart image pins).


Important

--force-image-overrides can't fully restore env-file image tags. The workflow jq filter strips *_IMAGE_TAG keys from VALUES_CONFIG before writing the env-file. sanitizeEnvFileForOCIImmutability then strips them again at the Go layer. An operator passing --force-image-overrides gets the Go-layer overlays back, but the env-file image tags were already discarded upstream — they can't be restored by the flag. The flag description "force image overrides even in OCI immutability mode" implies full restoration, which isn't true. Either document this gap explicitly or add a note in the flag help text.

defer cleanupEnvFile() is placed after the if err != nil check — non-idiomatic Go and a latent leak risk:

tmpEnvFile, cleanupEnvFile, err := sanitizeEnvFileForOCIImmutability(opts.EnvFile, opts)
if err != nil {
    return err
}
defer cleanupEnvFile()   // ← should be here, before the error check above

If a future refactor adds an early return between the assignment and the current defer line, the temp file leaks. Move defer cleanupEnvFile() to immediately after the err != nil guard.

Missing test coverage:

  • ForceImageOverrides: true + ImageTags: false — verify the digest overlay is restored
  • All-image-tag-only env-file passed to sanitizeEnvFileForOCIImmutability — result should be empty path / no temp file
  • OCI mode with no env-file (EnvFile == "") — verify it short-circuits cleanly

Minor

effectiveImageTags(entry Entry, opts RunOptions) only reads entry.ImageTags and opts.ForceImageOverrides — taking full structs is fine for now but a signature like effectiveImageTags(imageTags bool, forceOverride bool) would make it easier to test in isolation.

The OCI immutability warning log only fires in executeEntry, not in resolveChartRootOverlaysQuiet (the dry-run path). A dry-run call won't surface the suppression — minor, but could confuse operators debugging with --dry-run.

@hisImminence hisImminence left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After addressing the comments lgtm! Thanks!

bkenez and others added 2 commits May 29, 2026 08:36
Add an OCI immutability guard to deploy-camunda matrix run so chart refs deploy with their baked-in image versions by default.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@bkenez bkenez force-pushed the ci/prevent-oci-image-overrides branch from 0e7010d to 5a80fab Compare May 29, 2026 06:36
@bkenez bkenez added this pull request to the merge queue May 29, 2026
Merged via the queue into main with commit 347642d May 29, 2026
214 checks passed
@bkenez bkenez deleted the ci/prevent-oci-image-overrides branch May 29, 2026 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants