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

Don't use "info.IndexServerAddress" for authentication #10458

Merged
merged 1 commit into from
Apr 12, 2023

Conversation

thaJeztah
Copy link
Member

The IndexServerAddress field was as part of the initial Windows implementation of the engine. For legal reasons, Microsoft Windows (and thus Docker images based on Windows) were not allowed to be distributed through non-Microsoft infrastructure. As a temporary solution, a dedicated "registry-win-tp3.docker.io" registry was created to serve Windows images.

Using separate registries was not an ideal solution, and a more permanent solution was created by introducing "foreign image layers" in the distribution spec, after which the "registry-win-tp3.docker.io" ceased to exist, and removed from the engine.

This replaces the code that calls out to the "/info" endpoint to use the GetAuthConfigKey() function instead.

Related PR in docker/cli:
docker/cli@b4ca1c7

What I did

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

The IndexServerAddress field was  as part of the initial Windows implementation
of the engine. For legal reasons, Microsoft Windows (and thus Docker images
based on Windows) were not allowed to be distributed through non-Microsoft
infrastructure. As a temporary solution, a dedicated "registry-win-tp3.docker.io"
registry was created to serve Windows images.

Using separate registries was not an ideal solution, and a more permanent
solution was created by introducing "foreign image layers" in the distribution
spec, after which the "registry-win-tp3.docker.io" ceased to exist, and
removed from the engine.

This replaces the code that calls out to the "/info" endpoint to use the
GetAuthConfigKey() function instead.

Related PR in docker/cli:
docker/cli@b4ca1c7

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov
Copy link

codecov bot commented Apr 11, 2023

Codecov Report

Patch coverage: 71.42% and project coverage change: -7.32 ⚠️

Comparison is base (a4af5e2) 59.76% compared to head (1892be8) 52.44%.

Additional details and impacted files
@@            Coverage Diff             @@
##               v2   #10458      +/-   ##
==========================================
- Coverage   59.76%   52.44%   -7.32%     
==========================================
  Files         105      105              
  Lines        9074     9053      -21     
==========================================
- Hits         5423     4748     -675     
- Misses       3080     3791     +711     
+ Partials      571      514      -57     
Impacted Files Coverage Δ
pkg/compose/compose.go 67.10% <0.00%> (+1.72%) ⬆️
pkg/compose/pull.go 76.72% <100.00%> (+2.75%) ⬆️

... and 25 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thaJeztah
Copy link
Member Author

LOL, I don't understand CodeCov's reports. Looks like my PR "improves" coverage for the files I touched, but it considers "indirect" changes to reduce coverage 🤷
Screenshot 2023-04-11 at 23 25 13

@thaJeztah thaJeztah requested a review from milas April 11, 2023 21:27
@milas
Copy link
Contributor

milas commented Apr 12, 2023

@thaJeztah The codecov setup is in a slightly broken state right now, we've been working to get E2E coverage etc properly incorporated. You can safely ignore it for the moment, sorry about that!

@milas milas merged commit af6f0ff into docker:v2 Apr 12, 2023
@thaJeztah thaJeztah deleted the simplify_auth branch April 12, 2023 17:10
@thaJeztah
Copy link
Member Author

Yeah, CodeCov (in my experience) often is a bit hit-and-miss. Sometimes useful, sometimes completely off 😞

Thanks!

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.

3 participants