entrypoint image lookup: change auth order #620
Merged
Conversation
This file contains 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
tekton-robot
added
the
approved
googlebot
added
the
cla: yes
tekton-robot
added
the
size/S
imjasonh
reviewed
Mar 14, 2019
| mkc := authn.NewMultiKeychain(kc, google.Keychain) | ||
| // this will first try to anonymous | ||
| // the fall back to authenticate using the k8schain, | ||
| // then fall back to the google keychain |
There was a problem hiding this comment.
Can you add a comment stating that this will (currently) fail if gcloud isn't available?
| type anonymousKeychain struct{} | ||
|
|
||
| func (a *anonymousKeychain) Resolve(_ name.Registry) (authn.Authenticator, error) { | ||
| return &anonymous{}, nil |
There was a problem hiding this comment.
Does this return your own anonymous implementation since NewMultiKeychain has special logic to detect authn.Anonymous? (ref)
If so, can you document that, so I don't try to make this return authn.Anonymous in a future change?
There was a problem hiding this comment.
It returns our own anonymous implement because NewMultiKeychain has a special logic to detect authn.Anonymous (and will skip it until the end). Good point, I'll had a comment about that too 👍
The google.Keychain will fail hard in case of `gcloud` command missing (which can be the case in minikube and other kubernetes cluster). This means it will fail before trying to contact the registry anonymously — even if the images are publically available. This add an ad-hoc anonymous keychain, and set it as the first keychaien to check. That way, it will first try to get images config anonymously and then try other authentications. Signed-off-by: Vincent Demeester <[email protected]>
vdemeester
force-pushed
the
fix-entrypoint-gcr-public
branch
from
2aaeb30
to
968814d
Compare
|
/hold |
tekton-robot
added
do-not-merge/hold
imjasonh
reviewed
Mar 14, 2019
vdemeester
force-pushed
the
fix-entrypoint-gcr-public
branch
from
38a1e31
to
ce09b2e
Compare
tekton-robot
added
size/S
vdemeester
force-pushed
the
fix-entrypoint-gcr-public
branch
from
ce09b2e
to
b7917e1
Compare
tekton-robot
added
size/XS
vdemeester
force-pushed
the
fix-entrypoint-gcr-public
branch
2 times, most recently
from
1a41bd9
to
bc934d1
Compare
tekton-robot
added
size/S
Signed-off-by: Vincent Demeester <[email protected]>
vdemeester
force-pushed
the
fix-entrypoint-gcr-public
branch
from
bc934d1
to
e16f68b
Compare
|
/test pull-tekton-pipeline-integration-tests |
|
/hold cancel |
tekton-robot
removed
the
do-not-merge/hold
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ImJasonH, 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 |
piyush-garg
pushed a commit
to piyush-garg/pipeline
that referenced
this pull request
Mar 5, 2021
…image remove gcs-fetcher image as removed in upstream
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
The google.Keychain will fail hard in case of
gcloudcommandmissing (which can be the case in minikube and other kubernetes
cluster). This means it will fail before trying to contact the
registry anonymously — even if the images are publically available.
This add an ad-hoc anonymous keychain, and set it as the first
keychaien to check. That way, it will first try to get images config
anonymously and then try other authentications.
Signed-off-by: Vincent Demeester [email protected]
This could be removed if google/go-containerregistry#405 gets merged, but I also think pipeline should try anonymously before the rest.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
[ ] Includes tests (if functionality changed/added)[ ] Includes docs (if user facing)See the contribution guide
for more details.