Skip to content
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

fix: Docker auth config lookup portability #480

Merged
merged 1 commit into from
Feb 8, 2023
Merged

Conversation

AaronFriel
Copy link
Contributor

On various platforms, the Docker credentials store may return keys with URLs with a scheme ("https://") prefixed on all entries, or hostnames only. An exception to this is the legacy Docker index server, which appears as "https://index.docker.io/v1/".

The push auth config is now selected by first resolving a valid ("non-legacy") entry, which will be either a hostname aka registry name, or the "https://index.docker.io/v1/" value. This value is then passed to GetAuthConfig, using the Docker library's existing lookup fallback behavior to handle URLs.

Two additional legacy names are handled, as discovered by reading the ORAS' project's implementation with the same goal. See:
https://github.com/oras-project/oras-go/blob/v1.2.2/pkg/auth/docker/resolver.go#L79-L86

@github-actions
Copy link

github-actions bot commented Feb 3, 2023

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

return "", nil, err
}

pushAuthConfig = types.AuthConfig(cliPushAuthConfig)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The concrete types that implement the interface providing GetAuthConfig perform the https:// fallback lookup, so we trust the library's behavior here.

@github-actions
Copy link

github-actions bot commented Feb 3, 2023

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to see the comment on L220 updated but LGTM otherwise

provider/image.go Outdated Show resolved Hide resolved
On various platforms, the Docker credentials store may return keys with URLs
with a scheme ("https://") prefixed on all entries, or hostnames only. An
exception to this is the legacy Docker index server, which appears as
"https://index.docker.io/v1/".

The push auth config is now selected by first resolving a valid ("non-legacy")
entry, which will be either a hostname aka registry name, or the
"https://index.docker.io/v1/" value. This value is then passed to
`GetAuthConfig`, using the Docker library's existing lookup fallback behavior to
handle URLs.

Two additional legacy names are handled, as discovered by reading the ORAS'
project's implementation with the same goal. See:
https://github.com/oras-project/oras-go/blob/v1.2.2/pkg/auth/docker/resolver.go#L79-L86
@github-actions
Copy link

github-actions bot commented Feb 8, 2023

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants