Add support for registry paths in auth.json #1278
Conversation
vrothberg
left a comment
vrothberg
left a comment
There was a problem hiding this comment.
I think we should walk the path. Let's assume we pull quay.io/repo/image:tag, I'd like the code to be able to find the following credentials in order:
- quay.io/repo/image
- quay.io/repo (to separate credentials for different users)
- quay.io (also to remain backwards compat)
saschagrunert
changed the title
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
We’ll also need the write path (login/logout). Is that expected to be a separate PR? (That’s fine if so, this PR is big enough already — just to make sure this is on the radar.)
Note to self: Only skimmed the tests for now.
Yes, let's follow-up on this one in another PR. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
WIP, to shorten the iteration cycle.
Overall looks good, I still need to read the tests.
docker/docker_client.go
Outdated
| v1Res := &V1Results{} | ||
|
|
||
| // Get credentials from authfile for the underlying hostname | ||
| // nolint: staticcheck |
There was a problem hiding this comment.
Is there a way to silence that specific warning and not everything from that linter?
There was a problem hiding this comment.
Yes, by adding a custom rules go .golangci.yml. Changed to use that instead of the comment.
There was a problem hiding this comment.
Ouch, that‘s worse than silencing the checker, and the ideal fix doesn’t work: golangci/golangci-lint#741
I have ended up with
//lint:ignore SA1019 We can't use GetCredentialsForRef because we want to search the whole registry.
auth, err := config.GetCredentials(sys, registry) //nolint:staticcheck // https://github.com/golangci/golangci-lint/issues/741so that both a direct invocation of staticcheck and use via golangci-lint don’t complain.
There was a problem hiding this comment.
Fixed, thank you for your suggestion.
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
A full review now.
Still checking for URLs even if the caller submits a ref (because the primary caller is going to always submit one) is the only non-trivial concern, the rest is just nits and small cleanups.
This patch adds support for `host[:port]/ns/…repo` to auth.json while keeping the backwards compatible behavior. Signed-off-by: Sascha Grunert <sgrunert@redhat.com>
3 participants
This patch adds support for
host[:port]/ns/…repoto auth.json while keeping the backwards compatible behavior.Refers to #1276