aio-libs / aio-libs/aiodocker

Export Docker image reference parsing, and use it to derive the registry for auth headers

Offen
#1,055 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
Vorherrschende Sprache
Python
Sterne
537
Forks
118
PR-Merge-Kennzahlen
Keine gemergten PRs in 30 T.

Beschreibung

### Is your feature request related to a problem?

`DockerImages.pull()` and `DockerImages.push()` derive the registry host for the `X-Registry-Auth` header by splitting the reference on the first slash:

```python
# aiodocker/images.py:162 (pull), :244 (push)
registry, has_registry_host, _ = image.partition("/")
if not has_registry_host:
raise ValueError("Image should have registry host when auth information is provided")
headers["X-Registry-Auth"] = compose_auth_header(auth, registry)
```

The part before the first slash is only the registry host when it looks like a domain. For a Docker Hub reference such as `homeassistant/home-assistant` it is the namespace, so `serveraddress` becomes `homeassistant`. Docker's containerd image store rejects that as a host mismatch, and the `ValueError` also fires for a valid single-name reference like `nginx`.

In Home Assistant Supervisor it surfaced as [401 Unauthorized on registry-1.docker.io when installing official add-ons](https://github.com/home-assistant/supervisor/issues/6668), because `serveraddress` was the namespace rather than a registry host. We fixed it downstream in [home-assistant/supervisor#6677](https://github.com/home-assistant/supervisor/pull/6677) by prefixing Docker Hub images with `docker.io/` purely so that `partition("/")` yields something the daemon accepts. That workaround then has to be careful not to double the prefix when the reference already carries a domain, which is a bug we subsequently hit.

### Describe the solution you'd like

Docker's own rule for this lives in [distribution/reference](https://github.com/distribution/reference). `splitDockerDomain()` cuts at the first slash and then tests the candidate:

```go
switch {
case maybeDomain == localhost: // reserved namespace
case maybeDomain == legacyDefaultDomain: // index.docker.io -> docker.io
case strings.ContainsAny(maybeDomain, ".:"):// dot or colon -> domain (ports, IPv4, IPv6)
case strings.ToLower(maybeDomain) != maybeDomain: // uppercase can't be a path component
default: // not a domain -> Docker Hub
}
```

Two parts to the request:

1. **Use domain detection for the auth header.** Deriving `serveraddress` this way makes `pull()`/`push()` correct for Docker Hub references with and without a domain, for a bare `nginx`, and for registries with a port or an IPv6 host. It also removes the reason for callers to rewrite the reference first, and would let us revert the workaround in supervisor#6677.

2. **Export the parsing helpers**, so callers that need to reason about a reference do not each reimplement it:
- `split_docker_domain(ref) -> (domain, remainder)`
- domain validation, equivalent to `DomainRegexp` from [reference/regexp.go](https://github.com/distribution/reference/blob/main/regexp.go)
- splitting name from tag/digest — the tag separator is the last colon with no slash after it, since `TagRegexp` disallows a slash. This generalizes the existing private `_has_embedded_tag_or_digest()` at `images.py:26`, which already relies on the same observation.
- the related constants: `docker.io`, the legacy `index.docker.io`, and the `library/` prefix for official images

We have all of the above implemented against `reference`'s behavior in Supervisor and would be happy to contribute it upstream instead of keeping it local.

### Describe alternatives you've considered

Keeping the helpers in each project, which is the status quo, and continuing to pre-qualify references before calling `pull()`. That works but every caller has to rediscover the same rules, and the pre-qualification is the kind of workaround that invites bugs — we hit one where a reference already carrying `index.docker.io/` ended up pulled as `docker.io/index.docker.io/...`.

### Additional context

Observed with aiodocker 0.27.0. Out of scope on purpose: mapping `docker.io` to the `registry-1.docker.io` API host is a registry-client concern rather than a daemon-client one, so we are not proposing it here.

Beitragsleitfaden

Beitragsleitfaden öffnen

Bewertung

Dieses Issue wurde noch nicht bewertet.

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.