-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple secrets of type dockercfg and dockerconfigjson #3659
Support multiple secrets of type dockercfg and dockerconfigjson #3659
Conversation
This changes the credentials initialization to support multiple secrets not just of type basic-auth but also of type dockercfg and dockerconfigjson.
|
Hi @SaschaSchwarze0. Thanks for your PR. I'm waiting for a tektoncd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test /approve Thanks for this! |
The following is the coverage report on the affected files.
|
Received the CLA approval with this statement:
Based on that, I'll close this one and will try to re-open it. |
The following is the coverage report on the affected files.
|
/test pull-tekton-pipeline-integration-tests |
The kind job looks like it's stuck. I'm going to flip the labels to try and bring it back to life. |
Hi @sbwsg, anything else you need from me for this pull request? |
At this point we're waiting for someone else to review & lgtm it on top of my approve. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/meow
/lgtm
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbwsg, vdemeester The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
In the documentation, Tekton explicitly claims to support this use case:
Support is provided through the
Secret
s associated with theTaskRun
sServiceAccount
. The implementation of the credentials code for Docker authentication already supported many credentials provided as basic-authSecret
s, also in combination with credentials defined in exactly oneSecret
of type dockercfg and/or dockerconfigjson. I am lifting the limitations on non basic-auth secrets. The Tekton code already creates Pod containers with multiple-docker-config
arguments, but the consuming code only accepted one and therefore only honored the last item. One may argue whether this is a feature or fixes a bug./kind feature
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
-> Docs linked from above is already good enough = it had not mentioned that only one Secret had been supported.
Reviewer Notes
If API changes are included, additive changes must be approved by at least two OWNERS and backwards incompatible changes must be approved by more than 50% of the OWNERS, and they must first be added in a backwards compatible way.
Release Notes