Skip to content

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

Merged
milas merged 1 commit into
docker:v2from
thaJeztah:simplify_auth
Apr 12, 2023
Merged

Don't use "info.IndexServerAddress" for authentication#10458
milas merged 1 commit into
docker:v2from
thaJeztah:simplify_auth

Conversation

@thaJeztah

Copy link
Copy Markdown
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

codecovBot commented Apr 11, 2023

Copy link
Copy Markdown

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 FilesCoverage Δ
pkg/compose/compose.go67.10% <0.00%> (+1.72%)⬆️
pkg/compose/pull.go76.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
Copy Markdown
MemberAuthor

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 milasApril 11, 2023 21:27
@milas

Copy link
Copy Markdown
Contributor

@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:v2Apr 12, 2023
@thaJeztah
thaJeztah deleted the simplify_auth branch April 12, 2023 17:10
@thaJeztah

Copy link
Copy Markdown
MemberAuthor

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

Thanks!

Sign up for freeto 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

@thaJeztah@milas@ndeloof