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

Read indexServerAddress from Docker's /info #5347

Merged
merged 5 commits into from
May 14, 2022

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented May 3, 2022

Related to #5263
Fixes #5121

@bsideup bsideup added this to the next milestone May 3, 2022
@bsideup bsideup requested review from rnorth and kiview as code owners May 3, 2022 14:51
@gesellix
Copy link
Contributor

gesellix commented May 3, 2022

You might be interested in this one: docker/cli#2819

I myself didn't have the time to adopt that change in my docker-client implementation - fetching the daemon's index server address still seems to work.

@bsideup
Copy link
Member Author

bsideup commented May 3, 2022

@gesellix interesting, many thanks for sharing!

@kiview
Copy link
Member

kiview commented May 10, 2022

@bsideup
Copy link
Member Author

bsideup commented May 10, 2022

FTR it looks like they hardcoded it to avoid calling /info (because it is considered expensive) but we call it anyways (to get the OS info and fail fast on Windows containers)

@bsideup bsideup requested a review from a team May 10, 2022 14:21
@gesellix
Copy link
Contributor

gesellix commented May 10, 2022

Isn't the new approach just using a hardcoded value

Yes, that's my impression, too, and from reading the details I would assume that calling the /info endpoint was necessary in the early stages of Windows support, but has been obsolete for quite a while.

I wonder, though, if using the /info endpoint resolves the linked issues on testcontainer's side... using the daemon's indexServerAddress shouldn't hurt, but I would be surprised if the issues are resolved.

Maybe there's a little detail missing when handling the default index domains. As far as I know a client should handle both index.docker.io and docker.io as "default domain", meaning that the client should authenticate in both cases with the credentials for https://index.docker.io/v1/.

edit: I didn't find the issue before, but this is why I mentioned the "default domain": #4912 (comment) (which is related to #4474).

@thaJeztah
Copy link

👋 sorry for commenting on old PRs, but I was working on some changes in this area, and saw this PR linked to our repo.

FTR it looks like they hardcoded it to avoid calling /info (because it is considered expensive) but we call it anyways (to get the OS info and fail fast on Windows containers)

@bsideup The /info endpoint can indeed be "expensive" (depending on what's running on the daemon). If all you need is information about the daemon's platform, you could consider using the HEAD /_ping (or GET if HEAD is not an option) endpoint.

That endpoint should return some minimum amount of information in the Headers (API version supported, platform (windows / linux));

curl --unix-socket /var/run/docker.sock -I 'http://foo/_ping'
HTTP/1.1 200 OK
Api-Version: 1.43
Builder-Version: 2
Cache-Control: no-cache, no-store, must-revalidate
Content-Type: text/plain; charset=utf-8
Date: Tue, 11 Apr 2023 21:13:45 GMT
Docker-Experimental: false
Ostype: linux
Pragma: no-cache
Server: Docker/22.06.0-beta.0-926-g914b02ebaf.m (linux)
Swarm: active/manager

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

Successfully merging this pull request may close these issues.

Cannot pull images when logged in to Docker Desktop (Status 500: unauthorized: incorrect username or password)
5 participants