fix(common): apply registry override to image args - #3885
Open
pujitha24 wants to merge 1 commit into
Open
Conversation
Motivation:
TEKTON_REGISTRY_OVERRIDE is documented as a way to rewrite the
registry for every image the operator deploys, but only
container.Image, Task step images, and StepAction images had a
fallback that applies it when no per-image env var is set. Images
passed as container CLI args (e.g. "-workingdirinit-image
<value>", "-el-image <value>", "-shell-image-win <value>") were
skipped by that fallback, so in an air-gapped install those
container args still pointed at the public upstream registries
(gcr.io, mcr.microsoft.com) even with TEKTON_REGISTRY_OVERRIDE
set, unless the user also set every documented per-image env var
individually. This matches the images the reporter listed as not
being rewritten. No crash or outage results; the effect is
ImagePullBackOff for those specific images in registry-restricted
clusters, and it did already have a documented per-image env var
workaround.
Approach:
replaceContainersArgsImage (pkg/reconciler/common/transformers.go)
only rewrote an arg's image when a matching per-image env var
existed. Add the same registry-override fallback already used by
replaceContainerImages/replaceStepsImages/replaceStepActionImages:
when no per-image override matches an arg whose flag name looks
like an image flag (contains "_image" once normalized, covering
both "-foo-image" and "-foo-image-win" style flags), rewrite just
its registry domain via TEKTON_REGISTRY_OVERRIDE.
Validation:
Added TestDeploymentContainerArgsImagesRegistryOverrideFallback to
pkg/reconciler/common/transformers_test.go, covering both a plain
"-bash-image" arg and the real "-shell-image-win" flag shipped in
the Tekton Pipeline controller manifest. Confirmed (by temporarily
reverting the transformers.go change) that this test fails without
the fix and passes with it.
go build ./...
go vet ./pkg/reconciler/common/...
gofmt -l pkg/reconciler/common/transformers.go pkg/reconciler/common/transformers_test.go
go test ./pkg/reconciler/common/... ./pkg/reconciler/kubernetes/tektonpipeline/... \
./pkg/reconciler/kubernetes/tektontrigger/... ./pkg/reconciler/openshift/tektontrigger/... \
./pkg/reconciler/openshift/common/...
All of the above pass. golangci-lint could not be downloaded in
this sandbox (checksum mismatch fetching the pinned release
binary, unrelated to this change); go vet and gofmt are clean.
Report: tektoncd#3270
```release-note
Fix `TEKTON_REGISTRY_OVERRIDE` not being applied to image
references passed as container args (e.g. entrypoint, nop,
workingdirinit, shell-image-win, and triggers' event-listener
sink images).
```
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
tekton-robot
added
the
release-note
tekton-robot
added
the
size/M
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3885 +/- ##
==========================================
- Coverage 26.13% 26.13% -0.01%
==========================================
Files 465 465
Lines 24933 24937 +4
==========================================
+ Hits 6516 6517 +1
- Misses 17697 17698 +1
- Partials 720 722 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
/approve |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkhelil The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Fixes
TEKTON_REGISTRY_OVERRIDEnot being applied to images passed as container CLI args (e.g.-workingdirinit-image,-el-image,-shell-image-win).TEKTON_REGISTRY_OVERRIDEis documented as a way to rewrite the registry for every image the operator deploys, but onlycontainer.Image, Task step images, and StepAction images had a fallback that applies it when no per-image env var is set. Images passed as container CLI args were skipped by that fallback, so in an air-gapped install those container args still pointed at the public upstream registries (gcr.io, mcr.microsoft.com) even withTEKTON_REGISTRY_OVERRIDEset, unless the user also set every documented per-image env var individually.replaceContainersArgsImage(pkg/reconciler/common/transformers.go) only rewrote an arg's image when a matching per-image env var existed. This adds the same registry-override fallback already used byreplaceContainerImages/replaceStepsImages/replaceStepActionImages: when no per-image override matches an arg whose flag name looks like an image flag (contains_imageonce normalized, covering both-foo-imageand-foo-image-winstyle flags), it rewrites just its registry domain viaTEKTON_REGISTRY_OVERRIDE.Added
TestDeploymentContainerArgsImagesRegistryOverrideFallbacktopkg/reconciler/common/transformers_test.go, covering both a plain-bash-imagearg and the real-shell-image-winflag shipped in the Tekton Pipeline controller manifest. Confirmed (by temporarily reverting thetransformers.gochange) that this test fails without the fix and passes with it.Report: #3270
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make test lintbefore submitting a PRSee the contribution guide for more details.
Release Notes
AI assistance: this change was drafted with Claude Code.
Fixes #3270