Skip to content

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Jul 31, 2025

relates to;

Rewrite to remove the github.com/docker/docker/registry dependency, which will not be included in the upcoming "api" and "client" modules, and will not be a public package in the module used for the daemon itself.

1. don't call "/info" API endpoint to get default registry

The IndexServerAddress in the /info endpoint was added 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 through docker/docker PR 21100.

However, the ElectAuthServer was left in place, quoting from that PR;

make the client check which default registry the daemon uses is still
more correct than leaving it up to the client, even if it won't technically
matter after this PR. There may be some backward compatibility scenarios
where ElectAuthServer [sic] is still helpful.

That comment was 10 Years ago, and the CLI stopped using this information, as the default registry is not configurable, so in practice was a static value. (see docker/cli@b4ca1c7).

2. replace ParseRepositoryInfo and GetAuthConfigKey with local implementation

The ParseRepositoryInfo function was originally implemented for use by the daemon itself. It returns a RepositoryInfo struct that holds information about the repository and the registry the repository can be found in.

As it was written for use by the daemon, it also was designed to be used in combination with the daemon's configuration (such as mirrors, and insecure registries). If no daemon configuration is present, which would be the case when used in a CLI, it uses fallback logic as used in the daemon to detect if the registry is running on a localhost / loopback address, because such addresses are allowed to be "insecure" by default; this includes resolving the IP-address of the host (if it's not an IP-address).

Unfortunately, these functions (and related types) were reused in the CLI and many other places, which resulted in those types to be deeply ingrained in interfaces and (external) code.

For compose; it was only used to get the "auth-config key" to use for looking up auth information from the credentials store, which still needs special handling for the "default" (docker hub) domain, which unlike other image references doesn't use the hostname included in the image reference for the actual registry (and key for storing auth).

For those that want to follow along;

First, note that GetAuthConfig only requires a registry.IndexInfo, so not the whole RepositoryInfo struct;
https://github.com/moby/moby/blob/v28.3.3/registry/types.go#L8-L24

From the registry.IndexInfo it only uses the IsOfficial and Name fields; https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L390-L395

But to get the IndexInfo, ParseRepositoryInfo is needed, which first takes the image reference's "domain name" (e.g. docker.io); https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L421

This gets "normalized" for some cases where the info.IndexServerAddress was incorrectly assumed to be the canonical domain for Docker Hub registry, and which does happen to also be accessible as a "v2" registry. https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L334-L341

After normalizing, it checks if it's a docker hub address ("docker.io" after normalizing); Docker Hub is always required to use a secure connection, so no detection happens, and the Official field is set to indicate it's Docker Hub (this code path was already simplified as historically it would try to find daemon configuration (or otherwise use a default) for Mirror configuration;
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L420-L443

For non-Docker Hub registries, it also sets the name, and attempts to detect if the registry is allowed to be "insecure"; https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L435-L442

Which (as mentioned) involves parsing the address and, if needed, resolving the hostname
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L445-L481

As Insecure is not used for looking up the auth-config key, all of the above can be reduced to;

  • Is the hostname obtained from the image reference "docker.io" (after normalizing)?
  • If so, use the special https://index.docker.io/v1/ as auth-config key (another horrible remnant)
  • Otherwise use the hostname obtained from the image reference as-is

What I did

Related issue

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

@thaJeztah thaJeztah requested a review from a team as a code owner July 31, 2025 10:38
@thaJeztah thaJeztah requested review from ndeloof and glours July 31, 2025 10:38
Rewrite to remove the `github.com/docker/docker/registry` dependency,
which will not be included in the upcoming "api" and "client" modules,
and will not be a public package in the module used for the daemon itself.

1. don't call "/info" API endpoint to get default registry

The `IndexServerAddress` in the `/info` endpoint was added 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 through docker/docker PR 21100.

However, the `ElectAuthServer` was left in place, quoting from that PR;

> make the client check which default registry the daemon uses is still
> more correct than leaving it up to the client, even if it won't technically
> matter after this PR. There may be some backward compatibility scenarios
> where `ElectAuthServer` [sic] is still helpful.

That comment was 10 Years ago, and the CLI stopped using this information,
as the default registry is not configurable, so in practice was a static
value. (see docker/cli@b4ca1c7).

2. replace `ParseRepositoryInfo` and `GetAuthConfigKey` with local impl

The `ParseRepositoryInfo` function was originally implemented for use by
the daemon itself. It returns a `RepositoryInfo` struct that holds information
about the repository and the registry the repository can be found in.

As it was written for use by the daemon, it also was designed to be used
in combination with the daemon's configuration (such as mirrors, and
insecure registries). If no daemon configuration is present, which would
be the case when used in a CLI, it uses fallback logic as used in the daemon
to detect if the registry is running on a localhost / loopback address,
because such addresses are allowed to be "insecure" by default; this includes
resolving the IP-address of the host (if it's not an IP-address).

Unfortunately, these functions (and related types) were reused in the
CLI and many other places, which resulted in those types to be deeply
ingrained in interfaces and (external) code.

For compose; it was only used to get the "auth-config key" to use for
looking up auth information from the credentials store, which still
needs special handling for the "default" (docker hub) domain, which
unlike other image references doesn't use the hostname included in
the image reference for the actual registry (and key for storing
auth).

For those that want to follow along;

First, note that `GetAuthConfig` only requires a `registry.IndexInfo`, so not
the whole `RepositoryInfo` struct;
https://github.com/moby/moby/blob/v28.3.3/registry/types.go#L8-L24

From the `registry.IndexInfo` it only uses the `IsOfficial` and `Name` fields;
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L390-L395

But to get the `IndexInfo`, `ParseRepositoryInfo` is needed, which first
takes the image reference's "domain name" (e.g. `docker.io`);
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L421

This gets "normalized" for some cases where the `info.IndexServerAddress`
was incorrectly assumed to be the canonical domain for Docker Hub registry,
and which _does_ happen to also be accessible as a "v2" registry.
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L334-L341

After normalizing, it checks if it's a docker hub address ("docker.io"
after normalizing); Docker Hub is always required to use a secure
connection, so no detection happens, and the `Official` field is set
to indicate it's Docker Hub (this code path was already simplified
as historically it would try to find daemon configuration (or otherwise
use a default) for Mirror configuration;
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L420-L443

For non-Docker Hub registries, it also sets the name, and attempts
to detect if the registry is allowed to be "insecure";
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L435-L442

Which (as mentioned) involves parsing the address and, if needed, resolving
the hostname
https://github.com/moby/moby/blob/v28.3.3/registry/config.go#L445-L481

As `Insecure` is not used for looking up the auth-config key, all of the
above can be reduced to;

- Is the hostname obtained from the image reference "docker.io" (after normalizing)?
- If so, use the special `https://index.docker.io/v1/` as auth-config key (another horrible remnant)
- Otherwise use the hostname obtained from the image reference as-is

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the simplify_authconfig_key branch from c662cbb to d2f4ef6 Compare July 31, 2025 10:40
@thaJeztah thaJeztah requested a review from Benehiko July 31, 2025 12:22

// GetAuthConfigKey special-cases using the full index address of the official
// index as the AuthConfig key, and uses the (host)name[:port] for private indexes.
func GetAuthConfigKey(reposName reference.Named) string {
Copy link
Member

@mdelapenya mdelapenya Jul 31, 2025

Choose a reason for hiding this comment

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

I would see a lot of value having this as a shared component that can be used by Testcontainers, i.e.

I think the Go SDK has something similar to this here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, much to do in this area but don't want to expose yet until some other bits are sorted (it's why I put it in "internal" for now).

Copy link
Contributor

@glours glours left a comment

Choose a reason for hiding this comment

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

LGTM

@glours glours merged commit fc66da0 into docker:main Aug 5, 2025
26 checks passed
@thaJeztah thaJeztah deleted the simplify_authconfig_key branch August 5, 2025 08:35
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Aug 11, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [docker/compose](https://github.com/docker/compose) | patch | `v2.39.1` -> `v2.39.2` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>docker/compose (docker/compose)</summary>

### [`v2.39.2`](https://github.com/docker/compose/releases/tag/v2.39.2)

[Compare Source](docker/compose@v2.39.1...v2.39.2)

#### What's Changed

##### 🐛 Fixes

- Fix (regression): compose build render build output with tty support by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#13107
- Add missing `_MODEL` suffix to model variable pass to dependent services of a model by [@&#8203;glours](https://github.com/glours) in docker/compose#13109
- Apply `BUILDKIT_PROGRESS` value when building with bake by [@&#8203;glours](https://github.com/glours) in docker/compose#13110
- Define `pull` and `no_cache` from either service or flags values when building with bake by [@&#8203;glours](https://github.com/glours) in docker/compose#13133
- Only monitor attached services on `up` command by [@&#8203;glours](https://github.com/glours) in docker/compose#13114

##### 🔧  Internal

- Add Streams Comment by [@&#8203;suwakei](https://github.com/suwakei) in docker/compose#13103
- Add test of `json.go` by [@&#8203;suwakei](https://github.com/suwakei) in docker/compose#13106
- Refactoring of redundant condition checks by [@&#8203;suwakei](https://github.com/suwakei) in docker/compose#13104
- Eliminated magic string by [@&#8203;suwakei](https://github.com/suwakei) in docker/compose#13105
- Use log API for containers we didn't attached to by [@&#8203;ndeloof](https://github.com/ndeloof) in docker/compose#13111
- Use `cli-plugins/metadata` package by [@&#8203;thaJeztah](https://github.com/thaJeztah) in docker/compose#13130
- `pkg/compose`: simplify getting auth-config key by [@&#8203;thaJeztah](https://github.com/thaJeztah) in docker/compose#13120
- Add go as a prerequisite in build instructions by [@&#8203;mattrunyon](https://github.com/mattrunyon) in docker/compose#13131

##### ⚙️ Dependencies

- Build(deps): bump github.com/docker/cli from `28.3.2+incompatible` to `28.3.3+incompatible` by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in docker/compose#13116
- Build(deps): bump github.com/docker/docker from `28.3.2+incompatible` to `28.3.3+incompatible` by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in docker/compose#13115
- Build(deps): bump github.com/containerd/containerd/v2 from `2.1.3` to `2.1.4` by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in docker/compose#13119
- Build(deps): bump github.com/docker/go-connections from `0.5.0` to `0.6.0` by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in docker/compose#13137
- Build(deps): bump golang.org/x/sys from `0.34.0` to `0.35.0` by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in docker/compose#13138
- Bump golang to `1.23.12` by [@&#8203;austinvazquez](https://github.com/austinvazquez) in docker/compose#13142

#### New Contributors

- [@&#8203;mattrunyon](https://github.com/mattrunyon) made their first contribution in docker/compose#13131
- [@&#8203;austinvazquez](https://github.com/austinvazquez) made their first contribution in docker/compose#13142

**Full Changelog**: docker/compose@v2.39.1...v2.39.2

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS41OC4yIiwidXBkYXRlZEluVmVyIjoiNDEuNTguMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
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