fix(storage): drop imagePullSecrets from OCI auth - #1883
Open
pujitha24 wants to merge 1 commit into
Open
Conversation
Motivation: When a TaskRun/PipelineRun's ServiceAccount has both imagePullSecrets and mounted secrets targeting the same OCI registry, the vendored go-containerregistry keychain consolidates both sets and returns the first match, which is always the read-only imagePullSecrets credential. The OCI storage backend (pkg/chains/storage/oci/legacy.go, still the active auth path wired in pkg/chains/storage/storage.go despite its "Deprecated" doc-comment on the Backend type) only ever pushes signed attestations, so resolving a read-only credential makes the push fail with an auth error even though a push-capable credential is available in secrets. This addresses the ServiceAccount-level case described in the issue; the PodTemplate-level case was already fixed in a prior, separate change. Approach: - Bump the vendored github.com/google/go-containerregistry pkg/authn/kubernetes module to the commit that merged upstream's IgnorePullSecrets option (google/go-containerregistry#2315), which lets the keychain skip both the ServiceAccount's imagePullSecrets and any explicit pull secrets. - Set IgnorePullSecrets: true when building the k8schain options used by the OCI backend, since this keychain is only ever used to push, never to pull. Validation: - go build ./... passes. - make test-unit passes for all packages. - make golangci-lint PKG=./pkg/chains/storage/oci/... reports 0 issues. - Added TestK8schainOptions_IgnoresImagePullSecrets, confirmed to fail if IgnorePullSecrets: true is removed and to pass with the fix, asserting the OCI backend's k8schain options are built with IgnorePullSecrets set. - Not run: no live cluster is available in this environment, so the fix was not reproduced end-to-end against a real registry. Correctness instead relies on the upstream library's own test coverage for IgnorePullSecrets plus the added unit test confirming this backend threads the option through. Report: tektoncd#1336 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Contributor
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
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
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
When a TaskRun/PipelineRun's ServiceAccount has both
imagePullSecretsand mounted
secretstargeting the same OCI registry, the vendoredgo-containerregistry keychain consolidates both sets and returns the
first match, which is always the read-only
imagePullSecretscredential. The OCI storage backend (
pkg/chains/storage/oci/legacy.go,still the active auth path wired in
pkg/chains/storage/storage.godespite its "Deprecated" doc-comment on the
Backendtype) only everpushes signed attestations, so resolving a read-only credential makes
the push fail with an auth error even though a push-capable credential
is available in
secrets.This addresses the ServiceAccount-level case described in the issue;
the PodTemplate-level case was already fixed in a prior, separate
change.
Approach
github.com/google/go-containerregistrypkg/authn/kubernetesmodule to the commit that merged upstream'sIgnorePullSecretsoption(feat(kubernetes): allow ignoring pull secrets google/go-containerregistry#2315), which
lets the keychain skip both the ServiceAccount's
imagePullSecretsand any explicit pull secrets.
IgnorePullSecrets: truewhen building the k8schain options usedby the OCI backend, since this keychain is only ever used to push,
never to pull.
Validation
go build ./...passes.make test-unitpasses for all packages.make golangci-lint PKG=./pkg/chains/storage/oci/...reports 0 issues.TestK8schainOptions_IgnoresImagePullSecrets, confirmed to failif
IgnorePullSecrets: trueis removed and to pass with the fix,asserting the OCI backend's k8schain options are built with
IgnorePullSecretsset.was not reproduced end-to-end against a real registry. Correctness
instead relies on the upstream library's own test coverage for
IgnorePullSecretsplus the added unit test confirming this backendthreads the option through.
Submitter Checklist
As the author of this PR, please check off the items in this checklist:
functionality, content, code)
Release Notes
AI assistance: this change was drafted with Claude Code.
Fixes #1336