From 0bfccca8dba88917cc68a4f514a26761a14b6f2e Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:12:03 +0200 Subject: [PATCH 01/10] Route containerd ns mirror requests to configured registries containerd's hosts.toml mirrors append ?ns= to every request. The container handler ignored it, so a single _default mirror entry could not serve more than one registry. ns is a closed-world lookup key and is never dialed. Docker Hub aliases and the host of upstream.oci_default select the default route, hosts of upstream.oci entries select their named upstream. Unknown hosts return NAME_UNKNOWN so containerd falls back to its next host. Registry URLs with a path are not indexed, because ns names the registry at the root of a host. Host collisions log a warning and resolve deterministically. With ns the path is the verbatim upstream repository, the reserved upstream/ prefix is rejected, and repository prefix routes such as Homebrew's do not apply. Cache names match the unprefixed and upstream/{name}/ routes, so all routes share cached blobs, manifests and tag lists. ns is no longer forwarded on tag-list requests, and the pagination Link is rewritten per request instead of at store time so clients on different routes get links for their own route. Refs #303 --- internal/handler/container.go | 165 ++++++++++- internal/handler/container_ns_test.go | 411 ++++++++++++++++++++++++++ internal/handler/container_tags.go | 45 ++- 3 files changed, 591 insertions(+), 30 deletions(-) create mode 100644 internal/handler/container_ns_test.go diff --git a/internal/handler/container.go b/internal/handler/container.go index 5a47d795..6fd57f54 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -4,8 +4,12 @@ import ( "encoding/json" "errors" "fmt" + "maps" + "net" "net/http" + "net/url" "regexp" + "slices" "strings" ) @@ -15,8 +19,17 @@ const ( manifestMatchCount = 3 // full match + name + reference tagsListMatchCount = 2 // full match + name registrySelectorParts = 3 // upstream + name + repository + + // namespaceQueryParam is the query parameter containerd appends to mirror + // requests to name the registry the image reference points at. + namespaceQueryParam = "ns" + // defaultNamespaceRoute marks namespace hosts served by the default registry. + defaultNamespaceRoute = "" ) +// dockerHubNamespaces are the registry hosts clients use for Docker Hub. +var dockerHubNamespaces = []string{"docker.io", "index.docker.io", "registry-1.docker.io"} //nolint:gochecknoglobals // fixed alias list + // ContainerHandler handles OCI/Docker container registry protocol requests. // It implements the OCI Distribution Spec for pulling images. // Reference: https://github.com/opencontainers/distribution-spec/blob/main/spec.md @@ -26,6 +39,9 @@ type ContainerHandler struct { proxyURL string namedRegistries map[string]string registries []containerRegistry + // namespaces maps normalized registry hosts from containerd's ns query + // parameter to a named upstream, or to defaultNamespaceRoute. + namespaces map[string]string } type containerRegistry struct { @@ -38,9 +54,27 @@ type containerRegistry struct { // upstream/{name}/, leaving unprefixed requests compatible with the Docker Hub // mirror behavior. func NewContainerHandler(proxy *Proxy, proxyURL string, namedRegistries ...map[string]string) *ContainerHandler { + return newContainerHandler(proxy, proxyURL, dockerHubRegistry, namedRegistries...) +} + +// NewContainerHandlerWithRegistry creates a container handler with a custom +// default registry and optional named registries. +func NewContainerHandlerWithRegistry( + proxy *Proxy, + proxyURL, registryURL string, + namedRegistries ...map[string]string, +) *ContainerHandler { + return newContainerHandler(proxy, proxyURL, configuredUpstreamURL(registryURL, dockerHubRegistry), namedRegistries...) +} + +func newContainerHandler( + proxy *Proxy, + proxyURL, registryURL string, + namedRegistries ...map[string]string, +) *ContainerHandler { h := &ContainerHandler{ proxy: proxy, - registryURL: dockerHubRegistry, + registryURL: registryURL, proxyURL: strings.TrimSuffix(proxyURL, "/"), } if len(namedRegistries) > 0 { @@ -49,19 +83,81 @@ func NewContainerHandler(proxy *Proxy, proxyURL string, namedRegistries ...map[s h.namedRegistries[name] = strings.TrimSuffix(registryURL, "/") } } + // The index needs the final default registry URL, so build it last. + h.buildNamespaceIndex() return h } -// NewContainerHandlerWithRegistry creates a container handler with a custom -// default registry and optional named registries. -func NewContainerHandlerWithRegistry( - proxy *Proxy, - proxyURL, registryURL string, - namedRegistries ...map[string]string, -) *ContainerHandler { - h := NewContainerHandler(proxy, proxyURL, namedRegistries...) - h.registryURL = configuredUpstreamURL(registryURL, dockerHubRegistry) - return h +// buildNamespaceIndex maps the registry hosts containerd may send in the ns +// query parameter to the configured routes. The index is closed-world: a host +// is only ever looked up, never dialed. Docker Hub aliases and the default +// registry's host select the default route; hosts of upstream.oci entries +// select their named upstream. Registry URLs with a path are skipped because +// ns names the registry at the root of that host, not a repository mounted +// below it. On collisions the default route wins, then the alphabetically +// first upstream name. +func (h *ContainerHandler) buildNamespaceIndex() { + h.namespaces = make(map[string]string, len(dockerHubNamespaces)+1+len(h.namedRegistries)) + for _, host := range dockerHubNamespaces { + h.namespaces[host] = defaultNamespaceRoute + } + if host, ok := namespaceHostForURL(h.registryURL); ok { + h.namespaces[host] = defaultNamespaceRoute + } else { + h.warn("default OCI registry is not reachable through the ns query parameter: URL has a path", + "url", h.registryURL) + } + for _, name := range slices.Sorted(maps.Keys(h.namedRegistries)) { + host, ok := namespaceHostForURL(h.namedRegistries[name]) + if !ok { + h.warn("OCI upstream is not reachable through the ns query parameter: URL has a path", + "upstream", name, "url", h.namedRegistries[name]) + continue + } + if owner, exists := h.namespaces[host]; exists { + if owner == defaultNamespaceRoute { + owner = "default registry" + } + h.warn("OCI upstream shares its registry host with another route; ns requests use the other route", + "upstream", name, "host", host, "route", owner) + continue + } + h.namespaces[host] = name + } +} + +// namespaceHostForURL returns the ns lookup key for a registry URL. It reports +// false for URLs that are not a bare registry root. +func namespaceHostForURL(registryURL string) (string, bool) { + parsed, err := url.Parse(registryURL) + if err != nil || parsed.Host == "" || (parsed.Path != "" && parsed.Path != "/") { + return "", false + } + return registryHostKey(parsed.Host), true +} + +// registryHostKey normalizes a registry host[:port] for ns lookups. Hosts are +// case-insensitive and ports 80 and 443 are dropped because ns carries no +// scheme. The same function normalizes both configured URLs and ns values. +func registryHostKey(hostport string) string { + host, port, err := net.SplitHostPort(hostport) + if err != nil { + host, port = strings.TrimSuffix(strings.TrimPrefix(hostport, "["), "]"), "" + } + host = strings.ToLower(host) + if port != "" && port != "80" && port != "443" { + return net.JoinHostPort(host, port) + } + if strings.Contains(host, ":") { + return "[" + host + "]" + } + return host +} + +func (h *ContainerHandler) warn(msg string, args ...any) { + if h.proxy != nil && h.proxy.Logger != nil { + h.proxy.Logger.Warn(msg, args...) + } } // RegisterRegistry routes a repository and its descendants to a specific OCI @@ -144,7 +240,7 @@ func (h *ContainerHandler) handleBlobDownload(w http.ResponseWriter, r *http.Req return } - registryURL, upstreamName, cacheName, ok := h.registryForName(name) + registryURL, upstreamName, cacheName, ok := h.registryForRequest(r, name) if !ok { h.containerError(w, http.StatusNotFound, "NAME_UNKNOWN", "unknown upstream registry") return @@ -225,7 +321,7 @@ func (h *ContainerHandler) handleManifest(w http.ResponseWriter, r *http.Request return } - registryURL, upstreamName, _, ok := h.registryForName(name) + registryURL, upstreamName, _, ok := h.registryForRequest(r, name) if !ok { h.containerError(w, http.StatusNotFound, "NAME_UNKNOWN", "unknown upstream registry") return @@ -248,7 +344,7 @@ func (h *ContainerHandler) handleTagsList(w http.ResponseWriter, r *http.Request return } - registryURL, upstreamName, _, ok := h.registryForName(name) + registryURL, upstreamName, _, ok := h.registryForRequest(r, name) if !ok { h.containerError(w, http.StatusNotFound, "NAME_UNKNOWN", "unknown upstream registry") return @@ -286,6 +382,47 @@ func (h *ContainerHandler) proxyBlobHead(w http.ResponseWriter, r *http.Request, }) } +// registryForRequest resolves the repository name of a request. containerd +// mirror requests name the target registry in the ns query parameter; without +// it the name is routed by registryForName. +func (h *ContainerHandler) registryForRequest(r *http.Request, name string) (registryURL, upstreamName, cacheName string, ok bool) { + namespaces := r.URL.Query()[namespaceQueryParam] + switch { + case len(namespaces) == 0 || (len(namespaces) == 1 && namespaces[0] == ""): + return h.registryForName(name) + case len(namespaces) > 1: + return "", "", "", false + } + return h.registryForNamespace(namespaces[0], name) +} + +// registryForNamespace resolves a repository name verbatim against the registry +// named by ns. The reserved upstream/ prefix is rejected so ns requests cannot +// address cache entries of another route. Cache names match the unprefixed and +// upstream/{name}/ routes, so all routes to one registry share blobs. +func (h *ContainerHandler) registryForNamespace(namespace, name string) (registryURL, upstreamName, cacheName string, ok bool) { + if strings.HasPrefix(name, "upstream/") { + return "", "", "", false + } + route, ok := h.namespaces[registryHostKey(namespace)] + if !ok { + return "", "", "", false + } + if route == defaultNamespaceRoute { + // ns explicitly names the default registry, so repository prefix + // routes such as Homebrew's do not apply. + if h.registryURL == "" { + return "", "", "", false + } + return h.registryURL, name, name, true + } + registryURL = h.namedRegistries[route] + if registryURL == "" { + return "", "", "", false + } + return registryURL, name, "upstream/" + route + "/" + name, true +} + // registryForName resolves a client-visible OCI repository name to an upstream // registry and its repository name. Named upstreams use upstream/{name}/ as a // reserved prefix. Other names are matched against registered repository diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go new file mode 100644 index 00000000..fa699478 --- /dev/null +++ b/internal/handler/container_ns_test.go @@ -0,0 +1,411 @@ +package handler + +import ( + "bytes" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync" + "testing" + "time" + + "github.com/git-pkgs/registries/fetch" +) + +const ( + nsTestBlob = "layer bytes" + nsTestProxyURL = "http://proxy.example.test" +) + +// nsTestRegistry is a fake OCI registry serving one repository below an +// optional path prefix. It records every request URI it receives. +type nsTestRegistry struct { + *httptest.Server + repository string + + mu sync.Mutex + requests []string +} + +func newNSTestRegistry(t *testing.T, repository, pathPrefix string) *nsTestRegistry { + t.Helper() + registry := &nsTestRegistry{repository: repository} + manifest := nsTestManifest() + blobDigest := "sha256:" + sha256Hex(nsTestBlob) + manifestDigest := "sha256:" + sha256Hex(manifest) + base := pathPrefix + "/v2/" + repository + registry.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + registry.mu.Lock() + registry.requests = append(registry.requests, r.URL.RequestURI()) + registry.mu.Unlock() + + switch r.URL.Path { + case base + "/manifests/latest", base + "/manifests/" + manifestDigest: + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Docker-Content-Digest", manifestDigest) + _, _ = io.WriteString(w, manifest) + case base + "/blobs/" + blobDigest: + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = io.WriteString(w, nsTestBlob) + case base + "/tags/list": + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("last") == "" { + w.Header().Set("Link", `<`+base+`/tags/list?last=1.0&n=1>; rel="next"`) + _, _ = io.WriteString(w, `{"name":"`+repository+`","tags":["1.0"]}`) + return + } + _, _ = io.WriteString(w, `{"name":"`+repository+`","tags":["2.0"]}`) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(registry.Close) + return registry +} + +func nsTestManifest() string { + return `{"schemaVersion":2,"layers":[{"digest":"sha256:` + sha256Hex(nsTestBlob) + `"}]}` +} + +func (r *nsTestRegistry) host() string { + return strings.TrimPrefix(r.URL, "http://") +} + +func (r *nsTestRegistry) requestCount() int { + r.mu.Lock() + defer r.mu.Unlock() + return len(r.requests) +} + +func (r *nsTestRegistry) lastRequest() string { + r.mu.Lock() + defer r.mu.Unlock() + if len(r.requests) == 0 { + return "" + } + return r.requests[len(r.requests)-1] +} + +// newNSTestHandler builds a container handler the way the server does, with a +// real fetcher so blob downloads reach the fake registries. Warnings logged +// while building the handler are written to the returned buffer. +func newNSTestHandler(t *testing.T, defaultURL string, named map[string]string) (http.Handler, *ContainerHandler, *bytes.Buffer) { + t.Helper() + proxy, _, _, _ := setupTestProxy(t) + logs := &bytes.Buffer{} + proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) + client := &http.Client{} + proxy.HTTPClient = client + proxy.MetadataTTL = time.Hour + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(client), fetch.WithMaxRetries(0)) + proxy.Fetcher = fetcher + t.Cleanup(func() { _ = fetcher.Close() }) + h := NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, defaultURL, named) + return http.StripPrefix("/v2", h.Routes()), h, logs +} + +func serveNS(routes http.Handler, target string) *httptest.ResponseRecorder { + response := httptest.NewRecorder() + routes.ServeHTTP(response, httptest.NewRequest(http.MethodGet, target, nil)) + return response +} + +func assertNameUnknown(t *testing.T, response *httptest.ResponseRecorder) { + t.Helper() + if response.Code != http.StatusNotFound { + t.Fatalf("status = %d, want 404: %s", response.Code, response.Body.String()) + } + if !strings.Contains(response.Body.String(), `"NAME_UNKNOWN"`) { + t.Errorf("body = %s, want NAME_UNKNOWN error", response.Body.String()) + } +} + +func TestContainerHandler_NamespaceSelectsDefaultRegistry(t *testing.T) { + registry := newNSTestRegistry(t, "library/nginx", "") + // The default registry is a custom oci_default, so its host is only + // resolvable when the index is built from the final registry URL. + routes, _, _ := newNSTestHandler(t, registry.URL, nil) + + for _, namespace := range []string{"docker.io", "index.docker.io", "registry-1.docker.io", registry.host()} { + t.Run(namespace, func(t *testing.T) { + response := serveNS(routes, "/v2/library/nginx/manifests/latest?ns="+namespace) + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", response.Code, response.Body.String()) + } + if got, want := registry.lastRequest(), "/v2/library/nginx/manifests/latest"; got != want { + t.Errorf("upstream request = %q, want %q", got, want) + } + }) + } +} + +func TestContainerHandler_NamespaceSelectsNamedRegistry(t *testing.T) { + fallback := newNSTestRegistry(t, "owner/app", "") + named := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, fallback.URL, map[string]string{"ghcr": named.URL}) + digest := "sha256:" + sha256Hex(nsTestBlob) + + manifest := serveNS(routes, "/v2/owner/app/manifests/latest?ns="+named.host()) + if manifest.Code != http.StatusOK { + t.Fatalf("manifest status = %d, want 200: %s", manifest.Code, manifest.Body.String()) + } + blob := serveNS(routes, "/v2/owner/app/blobs/"+digest+"?ns="+named.host()) + if blob.Code != http.StatusOK { + t.Fatalf("blob status = %d, want 200: %s", blob.Code, blob.Body.String()) + } + if got := blob.Body.String(); got != nsTestBlob { + t.Errorf("blob body = %q, want %q", got, nsTestBlob) + } + if got := named.requestCount(); got != 2 { + t.Errorf("named registry requests = %d, want 2", got) + } + if got := fallback.requestCount(); got != 0 { + t.Errorf("default registry requests = %d, want 0", got) + } +} + +func TestContainerHandler_NamespaceRejectsUnresolvableRequests(t *testing.T) { + registry := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, registry.URL, map[string]string{"ghcr": registry.URL}) + digest := "sha256:" + sha256Hex(nsTestBlob) + + tests := map[string]string{ + "unknown host manifest": "/v2/owner/app/manifests/latest?ns=quay.io", + "unknown host blob": "/v2/owner/app/blobs/" + digest + "?ns=quay.io", + "unknown host tags": "/v2/owner/app/tags/list?ns=quay.io", + "multiple ns values": "/v2/owner/app/manifests/latest?ns=docker.io&ns=" + registry.host(), + "reserved prefix (default)": "/v2/upstream/ghcr/owner/app/manifests/latest?ns=docker.io", + "reserved prefix (named)": "/v2/upstream/ghcr/owner/app/blobs/" + digest + "?ns=" + registry.host(), + } + for name, target := range tests { + t.Run(name, func(t *testing.T) { + assertNameUnknown(t, serveNS(routes, target)) + }) + } + if got := registry.requestCount(); got != 0 { + t.Errorf("upstream requests = %d, want 0", got) + } +} + +func TestContainerHandler_NamespaceSharesCacheWithOtherRoutes(t *testing.T) { + digest := "sha256:" + sha256Hex(nsTestBlob) + manifestDigest := "sha256:" + sha256Hex(nsTestManifest()) + + t.Run("named registry", func(t *testing.T) { + registry := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, "", map[string]string{"ghcr": registry.URL}) + + for _, target := range []string{ + "/v2/upstream/ghcr/owner/app/blobs/" + digest, + "/v2/upstream/ghcr/owner/app/manifests/" + manifestDigest, + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("warm %s status = %d: %s", target, response.Code, response.Body.String()) + } + } + warmed := registry.requestCount() + for _, target := range []string{ + "/v2/owner/app/blobs/" + digest + "?ns=" + registry.host(), + "/v2/owner/app/manifests/" + manifestDigest + "?ns=" + registry.host(), + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("ns %s status = %d: %s", target, response.Code, response.Body.String()) + } + } + if got := registry.requestCount(); got != warmed { + t.Errorf("upstream requests after ns pulls = %d, want %d (cache hits)", got, warmed) + } + }) + + t.Run("default registry", func(t *testing.T) { + registry := newNSTestRegistry(t, "library/nginx", "") + routes, _, _ := newNSTestHandler(t, registry.URL, nil) + + for _, target := range []string{ + "/v2/library/nginx/blobs/" + digest + "?ns=docker.io", + "/v2/library/nginx/manifests/" + manifestDigest + "?ns=docker.io", + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("warm %s status = %d: %s", target, response.Code, response.Body.String()) + } + } + warmed := registry.requestCount() + for _, target := range []string{ + "/v2/library/nginx/blobs/" + digest, + "/v2/library/nginx/manifests/" + manifestDigest, + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("unprefixed %s status = %d: %s", target, response.Code, response.Body.String()) + } + } + if got := registry.requestCount(); got != warmed { + t.Errorf("upstream requests after unprefixed pulls = %d, want %d (cache hits)", got, warmed) + } + }) +} + +func TestContainerHandler_NamespaceTagsList(t *testing.T) { + registry := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, "", map[string]string{"ghcr": registry.URL}) + + // The prefix route fills the shared cache entry first. + prefixed := serveNS(routes, "/v2/upstream/ghcr/owner/app/tags/list?n=1") + if prefixed.Code != http.StatusOK { + t.Fatalf("prefixed status = %d: %s", prefixed.Code, prefixed.Body.String()) + } + const wantPrefixedLink = `<` + nsTestProxyURL + `/v2/upstream/ghcr/owner/app/tags/list?last=1.0&n=1>; rel="next"` + if got := prefixed.Header().Get("Link"); got != wantPrefixedLink { + t.Errorf("prefixed Link = %q, want %q", got, wantPrefixedLink) + } + + namespaced := serveNS(routes, "/v2/owner/app/tags/list?n=1&ns="+registry.host()) + if namespaced.Code != http.StatusOK { + t.Fatalf("ns status = %d: %s", namespaced.Code, namespaced.Body.String()) + } + if got := registry.requestCount(); got != 1 { + t.Errorf("upstream requests = %d, want 1 (ns request served from the shared cache)", got) + } + wantNSLink := `<` + nsTestProxyURL + `/v2/owner/app/tags/list?last=1.0&n=1&ns=` + url.QueryEscape(registry.host()) + `>; rel="next"` + link := namespaced.Header().Get("Link") + if link != wantNSLink { + t.Fatalf("ns Link = %q, want %q", link, wantNSLink) + } + + next := serveNS(routes, strings.TrimPrefix(strings.SplitN(link, ">", 2)[0], "<")) + if next.Code != http.StatusOK { + t.Fatalf("next page status = %d: %s", next.Code, next.Body.String()) + } + if got, want := next.Body.String(), `{"name":"owner/app","tags":["2.0"]}`; got != want { + t.Errorf("next page body = %q, want %q", got, want) + } + if got, want := registry.lastRequest(), "/v2/owner/app/tags/list?last=1.0&n=1"; got != want { + t.Errorf("upstream request = %q, want %q (ns must not be forwarded)", got, want) + } +} + +func TestContainerHandler_NamespaceDefaultBypassesRepositoryPrefixRoutes(t *testing.T) { + hub := newNSTestRegistry(t, "homebrew/core/jq", "") + brew := newNSTestRegistry(t, "homebrew/core/jq", "") + routes, h, _ := newNSTestHandler(t, hub.URL, nil) + RegisterHomebrewArtifacts(h, brew.URL) + + if response := serveNS(routes, "/v2/homebrew/core/jq/manifests/latest?ns=docker.io"); response.Code != http.StatusOK { + t.Fatalf("ns status = %d: %s", response.Code, response.Body.String()) + } + if hub.requestCount() != 1 || brew.requestCount() != 0 { + t.Errorf("ns request reached hub=%d brew=%d, want hub=1 brew=0", hub.requestCount(), brew.requestCount()) + } + + if response := serveNS(routes, "/v2/homebrew/core/jq/manifests/latest"); response.Code != http.StatusOK { + t.Fatalf("unprefixed status = %d: %s", response.Code, response.Body.String()) + } + if hub.requestCount() != 1 || brew.requestCount() != 1 { + t.Errorf("unprefixed request reached hub=%d brew=%d, want hub=1 brew=1", hub.requestCount(), brew.requestCount()) + } +} + +func TestContainerHandler_NamespaceMatchesConfiguredHosts(t *testing.T) { + // Each pair is a configured registry URL and the ns value containerd + // sends for an image reference on that registry. + tests := []struct { + configured string + namespace string + }{ + {"https://GHCR.io", "ghcr.io"}, + {"https://ghcr.io/", "GHCR.IO"}, + {"http://[fd00::1]:5000", "[fd00::1]:5000"}, + {"https://[fd00::1]", "[fd00::1]"}, + {"https://reg.example:80", "reg.example:80"}, + {"https://reg.example:443", "reg.example"}, + {"https://reg.example", "reg.example:443"}, + {"http://reg.example:5000", "reg.example:5000"}, + } + for _, tt := range tests { + t.Run(tt.configured+" "+tt.namespace, func(t *testing.T) { + h := NewContainerHandlerWithRegistry(nil, nsTestProxyURL, "", map[string]string{"lab": tt.configured}) + registryURL, upstreamName, cacheName, ok := h.registryForNamespace(tt.namespace, "owner/app") + if !ok { + t.Fatalf("ns %q did not resolve for upstream %q", tt.namespace, tt.configured) + } + if registryURL != strings.TrimSuffix(tt.configured, "/") || upstreamName != "owner/app" || cacheName != "upstream/lab/owner/app" { + t.Errorf("route = (%q, %q, %q), want (%q, owner/app, upstream/lab/owner/app)", + registryURL, upstreamName, cacheName, tt.configured) + } + }) + } + + if _, _, _, ok := (NewContainerHandlerWithRegistry(nil, nsTestProxyURL, "", map[string]string{"lab": "http://reg.example:5000"})). + registryForNamespace("reg.example:5001", "owner/app"); ok { + t.Error("ns with a different port resolved, want no match") + } +} + +func TestContainerHandler_NamespaceHostCollisions(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + logs := &bytes.Buffer{} + proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) + h := NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, "https://mirror.example", map[string]string{ + "zeta": "https://shared.example", + "alpha": "https://Shared.example:443", + "mirror": "https://mirror.example", + "hub": "https://registry-1.docker.io", + }) + + tests := map[string]string{ + "shared.example": "https://Shared.example:443", + "mirror.example": "https://mirror.example", + "registry-1.docker.io": "https://mirror.example", + } + for namespace, want := range tests { + registryURL, _, cacheName, ok := h.registryForNamespace(namespace, "owner/app") + if !ok || registryURL != want { + t.Errorf("ns %q resolved to (%q, %v), want %q", namespace, registryURL, ok, want) + } + if namespace == "shared.example" && cacheName != "upstream/alpha/owner/app" { + t.Errorf("ns %q cache name = %q, want upstream/alpha/owner/app", namespace, cacheName) + } + } + for _, upstream := range []string{"upstream=zeta", "upstream=mirror", "upstream=hub"} { + if !strings.Contains(logs.String(), upstream) { + t.Errorf("missing collision warning for %s in logs:\n%s", upstream, logs.String()) + } + } +} + +func TestContainerHandler_NamespaceSkipsRegistryURLsWithPath(t *testing.T) { + registry := newNSTestRegistry(t, "owner/app", "/artifactory/api/docker/remote") + routes, _, logs := newNSTestHandler(t, "", map[string]string{ + "art": registry.URL + "/artifactory/api/docker/remote", + }) + + assertNameUnknown(t, serveNS(routes, "/v2/owner/app/manifests/latest?ns="+registry.host())) + if got := registry.requestCount(); got != 0 { + t.Errorf("upstream requests via ns = %d, want 0", got) + } + if !strings.Contains(logs.String(), "upstream=art") { + t.Errorf("missing warning for path-prefixed upstream in logs:\n%s", logs.String()) + } + + if response := serveNS(routes, "/v2/upstream/art/owner/app/manifests/latest"); response.Code != http.StatusOK { + t.Fatalf("prefix route status = %d, want 200: %s", response.Code, response.Body.String()) + } + + t.Run("default registry", func(t *testing.T) { + mirror := newNSTestRegistry(t, "library/nginx", "/hub") + routes, _, logs := newNSTestHandler(t, mirror.URL+"/hub", nil) + + assertNameUnknown(t, serveNS(routes, "/v2/library/nginx/manifests/latest?ns="+mirror.host())) + if got := mirror.requestCount(); got != 0 { + t.Errorf("upstream requests via ns host = %d, want 0", got) + } + if response := serveNS(routes, "/v2/library/nginx/manifests/latest?ns=docker.io"); response.Code != http.StatusOK { + t.Fatalf("ns=docker.io status = %d, want 200: %s", response.Code, response.Body.String()) + } + if !strings.Contains(logs.String(), "default OCI registry") { + t.Errorf("missing warning for path-prefixed default registry in logs:\n%s", logs.String()) + } + }) +} diff --git a/internal/handler/container_tags.go b/internal/handler/container_tags.go index e9803ed1..1750d8a1 100644 --- a/internal/handler/container_tags.go +++ b/internal/handler/container_tags.go @@ -27,20 +27,24 @@ type cachedContainerTags struct { } func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, registryURL, name string) { - cacheKey := h.containerTagsCacheKey(registryURL, name, r.URL.Query()) + // ns only selects the registry; it is neither forwarded upstream nor part + // of the cache identity, so all routes to one registry share tag lists. + query := r.URL.Query() + query.Del(namespaceQueryParam) + cacheKey := h.containerTagsCacheKey(registryURL, name, query) cached, err := h.loadContainerTags(r.Context(), cacheKey) if err != nil { h.proxy.Logger.Warn("failed to read cached container tag list", "error", err) cached = nil } if cached != nil && h.containerTagsFresh(cached) { - writeContainerTags(w, cached, false) + h.writeContainerTags(w, r, registryURL, cached, false) return } upstreamURL := fmt.Sprintf("%s/v2/%s/tags/list", registryURL, name) - if query := r.URL.Query().Encode(); query != "" { - upstreamURL += "?" + query + if encoded := query.Encode(); encoded != "" { + upstreamURL += "?" + encoded } req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) if err != nil { @@ -54,7 +58,7 @@ func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, resp, err := h.proxy.HTTPClient.Do(req) if err != nil { - h.serveStaleTagsOrError(w, cached, err) + h.serveStaleTagsOrError(w, r, registryURL, cached, err) return } defer func() { _ = resp.Body.Close() }() @@ -64,12 +68,12 @@ func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, if err := h.storeContainerTags(r.Context(), cacheKey, cached); err != nil { h.proxy.Logger.Warn("failed to refresh cached container tag list", "error", err) } - writeContainerTags(w, cached, false) + h.writeContainerTags(w, r, registryURL, cached, false) return } if resp.StatusCode != http.StatusOK { if cached != nil && shouldServeStaleManifest(resp.StatusCode) { - writeContainerTags(w, cached, true) + h.writeContainerTags(w, r, registryURL, cached, true) return } h.proxy.relayResponse(w, r, resp, copyContainerTagsHeaders) @@ -78,14 +82,16 @@ func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, body, err := h.proxy.ReadMetadata(resp.Body) if err != nil { - h.serveStaleTagsOrError(w, cached, fmt.Errorf("reading tag list: %w", err)) + h.serveStaleTagsOrError(w, r, registryURL, cached, fmt.Errorf("reading tag list: %w", err)) return } + // The upstream Link is stored verbatim and rewritten per request because + // clients on different routes share this cache entry. tags := &cachedContainerTags{ body: body, contentType: resp.Header.Get(headerContentType), etag: resp.Header.Get(headerETag), - link: h.rewriteContainerTagsLink(strings.Join(resp.Header.Values("Link"), ", "), registryURL, r.URL.Path), + link: strings.Join(resp.Header.Values("Link"), ", "), size: int64(len(body)), fetchedAt: time.Now(), } @@ -95,13 +101,13 @@ func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, if err := h.storeContainerTags(r.Context(), cacheKey, tags); err != nil { h.proxy.Logger.Warn("failed to cache container tag list", "error", err) } - writeContainerTags(w, tags, false) + h.writeContainerTags(w, r, registryURL, tags, false) } -func (h *ContainerHandler) serveStaleTagsOrError(w http.ResponseWriter, cached *cachedContainerTags, err error) { +func (h *ContainerHandler) serveStaleTagsOrError(w http.ResponseWriter, r *http.Request, registryURL string, cached *cachedContainerTags, err error) { if cached != nil { h.proxy.Logger.Warn("upstream tag list fetch failed, serving stale cache", "error", err) - writeContainerTags(w, cached, true) + h.writeContainerTags(w, r, registryURL, cached, true) return } h.proxy.Logger.Error("failed to fetch container tag list", "error", err) @@ -165,14 +171,15 @@ func (h *ContainerHandler) storeContainerTags(ctx context.Context, cacheKey stri return nil } -func writeContainerTags(w http.ResponseWriter, tags *cachedContainerTags, stale bool) { +func (h *ContainerHandler) writeContainerTags(w http.ResponseWriter, r *http.Request, registryURL string, tags *cachedContainerTags, stale bool) { w.Header().Set(headerContentType, tags.contentType) w.Header().Set(headerContentLength, strconv.FormatInt(tags.size, 10)) if tags.etag != "" { w.Header().Set(headerETag, tags.etag) } - if tags.link != "" { - w.Header().Set("Link", tags.link) + link := h.rewriteContainerTagsLink(tags.link, registryURL, r.URL.Path, r.URL.Query().Get(namespaceQueryParam)) + if link != "" { + w.Header().Set("Link", link) } if stale { w.Header().Set("Warning", containerStaleWarning) @@ -189,7 +196,7 @@ func copyContainerTagsHeaders(destination, source http.Header) { } } -func (h *ContainerHandler) rewriteContainerTagsLink(link, registryURL, requestPath string) string { +func (h *ContainerHandler) rewriteContainerTagsLink(link, registryURL, requestPath, namespace string) string { if link == "" { return "" } @@ -221,6 +228,12 @@ func (h *ContainerHandler) rewriteContainerTagsLink(link, registryURL, requestPa linkURL.User = proxyURL.User linkURL.Path = strings.TrimSuffix(proxyURL.Path, "/") + "/v2" + requestPath linkURL.RawPath = "" + if namespace != "" { + // Keep follow-up pages on the ns route the client is using. + query := linkURL.Query() + query.Set(namespaceQueryParam, namespace) + linkURL.RawQuery = query.Encode() + } return "<" + linkURL.String() + ">" }) } From 96ada2447c18f39768b84d4f916863cda3b7f7f5 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:12:24 +0200 Subject: [PATCH 02/10] Document containerd ns mirroring Refs #303 --- README.md | 17 +++++++++++++++++ docs/configuration.md | 13 +++++++++++++ 2 files changed, 30 insertions(+) diff --git a/README.md b/README.md index 70de9d79..1b61c1ce 100644 --- a/README.md +++ b/README.md @@ -447,6 +447,23 @@ Or pull images directly: docker pull localhost:8080/library/nginx:latest ``` +#### containerd (Kubernetes, k3s, nerdctl) + +containerd mirrors send the original registry host in an `ns` query +parameter, so one mirror entry can serve Docker Hub and every registry +configured in `upstream.oci`. Create `/etc/containerd/certs.d/_default/hosts.toml`: + +```toml +[host."http://proxy.example.com:8080"] + capabilities = ["pull", "resolve"] +``` + +`docker.io` and the host of `upstream.oci_default` use the default registry; +the host of each `upstream.oci` URL uses that named registry. Pulls for any +other registry get `404 NAME_UNKNOWN`, and containerd falls back to the +registry itself. k3s achieves the same with `mirrors: {"*": {endpoint: +["http://proxy.example.com:8080"]}}` in `/etc/rancher/k3s/registries.yaml`. + ### Helm Configure each HTTP chart repository with a name, then add the matching proxy diff --git a/docs/configuration.md b/docs/configuration.md index ed5ad404..a52c807e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -290,6 +290,19 @@ mise section in the README for the client-side `url_replacements`. while `upstream.oci` selects named registries through the `upstream/{name}/` repository prefix. For example, `oci://proxy.example.com/upstream/ghcr/owner/chart` uses the `ghcr` registry with `owner/chart` as its repository. + +containerd mirror requests carry the original registry host in an `ns` query +parameter (see the containerd section in the README). The proxy only looks the +host up and never connects to it: `docker.io`, `index.docker.io`, +`registry-1.docker.io` and the host of `upstream.oci_default` select the default +registry, and the host of each `upstream.oci` URL selects that named registry. +Hosts are compared case-insensitively, and ports 80 and 443 are ignored. An +unknown host returns `404 NAME_UNKNOWN`, so containerd falls back to its next +host. Registry URLs with a path (for example an Artifactory repository path) +are not reachable through `ns`, only through `upstream/{name}/`. When two +entries share a host, the proxy logs a warning at startup; the default registry +wins, otherwise the alphabetically first name. Pulls through `ns`, +`upstream/{name}/` and unprefixed requests share the same cache entries. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. From 669869353d0fc1f16155c11b749126edbe73135f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:36:32 +0200 Subject: [PATCH 03/10] Version the tag-list cache identity for the raw-link format Tag-list rows now store the upstream Link verbatim and rewrite it per request. Rows written by earlier versions hold a Link already rewritten for one route. Under the unchanged key an older binary running next to this one (rolling update on shared Postgres, or a rollback) would serve the raw Link unrewritten, sending clients to the wrong route for the next page. A format marker in the key keeps the two apart at the cost of one cache miss per tag list after the upgrade. Adversarial review finding F1: tag-list cache rows hold raw upstream Links under unchanged keys, so an older binary serves them unrewritten. --- internal/handler/container_ns_test.go | 37 +++++++++++++++++++++++++++ internal/handler/container_tags.go | 11 ++++++-- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index fa699478..4cc477e7 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -2,6 +2,9 @@ package handler import ( "bytes" + "context" + "crypto/sha256" + "encoding/hex" "io" "log/slog" "net/http" @@ -409,3 +412,37 @@ func TestContainerHandler_NamespaceSkipsRegistryURLsWithPath(t *testing.T) { } }) } + +func TestContainerHandler_TagsListIgnoresLegacyCacheRows(t *testing.T) { + registry := newNSTestRegistry(t, "owner/app", "") + routes, h, _ := newNSTestHandler(t, "", map[string]string{"ghcr": registry.URL}) + + // Rows written before the raw-link format hold a Link already rewritten + // for the route that filled them. Seed one under the legacy identity. + query := url.Values{"n": {"1"}} + legacySum := sha256.Sum256([]byte(registry.URL + "\x00owner/app\x00" + query.Encode())) + legacyKey := hex.EncodeToString(legacySum[:]) + if legacyKey == h.containerTagsCacheKey(registry.URL, "owner/app", query) { + t.Fatal("legacy and current tag-list cache keys are equal") + } + legacy := &cachedContainerTags{ + body: []byte(`{"name":"owner/app","tags":["legacy"]}`), + contentType: contentTypeJSON, + link: `<` + nsTestProxyURL + `/v2/upstream/ghcr/owner/app/tags/list?last=legacy&n=1>; rel="next"`, + fetchedAt: time.Now(), + } + if err := h.storeContainerTags(context.Background(), legacyKey, legacy); err != nil { + t.Fatalf("store legacy tag list: %v", err) + } + + response := serveNS(routes, "/v2/owner/app/tags/list?n=1&ns="+registry.host()) + if response.Code != http.StatusOK { + t.Fatalf("status = %d: %s", response.Code, response.Body.String()) + } + if got, want := response.Body.String(), `{"name":"owner/app","tags":["1.0"]}`; got != want { + t.Errorf("body = %q, want %q (legacy row must not be served)", got, want) + } + if got := registry.requestCount(); got != 1 { + t.Errorf("upstream requests = %d, want 1 (legacy row must not be served)", got) + } +} diff --git a/internal/handler/container_tags.go b/internal/handler/container_tags.go index 1750d8a1..a0ac9c18 100644 --- a/internal/handler/container_tags.go +++ b/internal/handler/container_tags.go @@ -13,7 +13,14 @@ import ( "time" ) -const containerTagsCacheEcosystem = "oci-tags" +const ( + containerTagsCacheEcosystem = "oci-tags" + // containerTagsCacheFormat versions the tag-list cache identity. Rows + // written before it hold a Link already rewritten for the route that + // filled them, while this format stores the upstream Link verbatim. The + // two must not share rows: an older binary would serve a raw Link as is. + containerTagsCacheFormat = "raw-link" +) var containerLinkTargetPattern = regexp.MustCompile(`<([^>]*)>`) @@ -115,7 +122,7 @@ func (h *ContainerHandler) serveStaleTagsOrError(w http.ResponseWriter, r *http. } func (h *ContainerHandler) containerTagsCacheKey(registryURL, name string, query url.Values) string { - identity := registryURL + "\x00" + name + "\x00" + query.Encode() + identity := containerTagsCacheFormat + "\x00" + registryURL + "\x00" + name + "\x00" + query.Encode() sum := sha256.Sum256([]byte(identity)) return hex.EncodeToString(sum[:]) } From 5f62462bae8242edb4f3219ac8ab4d1edbed029b Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:36:32 +0200 Subject: [PATCH 04/10] Qualify the README claim about ns mirror coverage Registries whose URL has a path and hosts shared by two entries are not reachable through ns; point to the configuration guide for both rules. Adversarial review finding F3: README says ns covers every upstream.oci registry. --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1b61c1ce..80828e9f 100644 --- a/README.md +++ b/README.md @@ -450,8 +450,9 @@ docker pull localhost:8080/library/nginx:latest #### containerd (Kubernetes, k3s, nerdctl) containerd mirrors send the original registry host in an `ns` query -parameter, so one mirror entry can serve Docker Hub and every registry -configured in `upstream.oci`. Create `/etc/containerd/certs.d/_default/hosts.toml`: +parameter, so one mirror entry can serve Docker Hub and the registries +configured in `upstream.oci` whose URL has no path. Create +`/etc/containerd/certs.d/_default/hosts.toml`: ```toml [host."http://proxy.example.com:8080"] @@ -461,7 +462,8 @@ configured in `upstream.oci`. Create `/etc/containerd/certs.d/_default/hosts.tom `docker.io` and the host of `upstream.oci_default` use the default registry; the host of each `upstream.oci` URL uses that named registry. Pulls for any other registry get `404 NAME_UNKNOWN`, and containerd falls back to the -registry itself. k3s achieves the same with `mirrors: {"*": {endpoint: +registry itself. See [docs/configuration.md](docs/configuration.md) for how +hosts are matched and which entry wins when two share a host. k3s achieves the same with `mirrors: {"*": {endpoint: ["http://proxy.example.com:8080"]}}` in `/etc/rancher/k3s/registries.yaml`. ### Helm From c121a4ee36a6f0262929b8b3dfeb5fd46300a801 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:37:20 +0200 Subject: [PATCH 05/10] Redact registry URLs in ns index warnings Configured registry URLs may carry userinfo credentials, which the new startup warnings wrote to the log verbatim. Log them with the password masked, and say precisely what a path-prefixed default registry means: its host is not indexed, while the Docker Hub aliases still select it. Adversarial review finding F2: startup warnings print registry URLs unredacted and the default-registry message is misleading. --- internal/handler/container.go | 15 ++++++++++++--- internal/handler/container_ns_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/internal/handler/container.go b/internal/handler/container.go index 6fd57f54..7ba3aae7 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -104,14 +104,14 @@ func (h *ContainerHandler) buildNamespaceIndex() { if host, ok := namespaceHostForURL(h.registryURL); ok { h.namespaces[host] = defaultNamespaceRoute } else { - h.warn("default OCI registry is not reachable through the ns query parameter: URL has a path", - "url", h.registryURL) + h.warn("host of the default OCI registry is not indexed for ns lookups: URL has a path; Docker Hub aliases still select it", + "url", redactedURL(h.registryURL)) } for _, name := range slices.Sorted(maps.Keys(h.namedRegistries)) { host, ok := namespaceHostForURL(h.namedRegistries[name]) if !ok { h.warn("OCI upstream is not reachable through the ns query parameter: URL has a path", - "upstream", name, "url", h.namedRegistries[name]) + "upstream", name, "url", redactedURL(h.namedRegistries[name])) continue } if owner, exists := h.namespaces[host]; exists { @@ -154,6 +154,15 @@ func registryHostKey(hostport string) string { return host } +// redactedURL returns a registry URL for logging with any password masked. +func redactedURL(raw string) string { + parsed, err := url.Parse(raw) + if err != nil { + return "" + } + return parsed.Redacted() +} + func (h *ContainerHandler) warn(msg string, args ...any) { if h.proxy != nil && h.proxy.Logger != nil { h.proxy.Logger.Warn(msg, args...) diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index 4cc477e7..d0ec82bf 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -446,3 +446,27 @@ func TestContainerHandler_TagsListIgnoresLegacyCacheRows(t *testing.T) { t.Errorf("upstream requests = %d, want 1 (legacy row must not be served)", got) } } + +func TestContainerHandler_NamespaceWarningsRedactCredentials(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + logs := &bytes.Buffer{} + proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) + NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, "https://svc:s3cret@mirror.example/hub", map[string]string{ + "art": "https://bot:hunter2@art.example/artifactory/api/docker/remote", + }) + + for _, secret := range []string{"s3cret", "hunter2"} { + if strings.Contains(logs.String(), secret) { + t.Errorf("logs contain credential %q:\n%s", secret, logs.String()) + } + } + for _, want := range []string{ + "svc:xxxxx@mirror.example/hub", + "bot:xxxxx@art.example/artifactory", + "Docker Hub aliases still select it", + } { + if !strings.Contains(logs.String(), want) { + t.Errorf("logs missing %q:\n%s", want, logs.String()) + } + } +} From 24fa62f51f6ca6ba7e5922bd1e13598f7a5ab63b Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 21:06:49 +0200 Subject: [PATCH 06/10] Keep non-default ports apart in the ns index Dropping 80 and 443 regardless of scheme turned https://host:80 and https://host into the same key, so a request for one of them could land on the other registry. Only the default port of the URL's scheme is optional now. A configured host is indexed with and without that port, because image references spell it either way; any other port has to match exactly, and the ns value is used as containerd sends it, apart from case and IPv6 brackets. Since a URL now yields two keys that can belong to different routes, the collision warning lists the owner per key. The handler test runs two registries on one host that differ only in the port. Tests can't bind 80 or 443, so a dialer maps those addresses to the fake servers. --- docs/configuration.md | 5 +- internal/handler/container.go | 107 ++++++++++++++++++-------- internal/handler/container_ns_test.go | 104 +++++++++++++++++++++++-- 3 files changed, 176 insertions(+), 40 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index a52c807e..4767db2d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -296,7 +296,10 @@ parameter (see the containerd section in the README). The proxy only looks the host up and never connects to it: `docker.io`, `index.docker.io`, `registry-1.docker.io` and the host of `upstream.oci_default` select the default registry, and the host of each `upstream.oci` URL selects that named registry. -Hosts are compared case-insensitively, and ports 80 and 443 are ignored. An +Hosts are compared case-insensitively. The scheme's default port (443 for +`https`, 80 for `http`) may be spelled out or left out in the image reference; +any other port must match exactly, so `https://registry.example:80` and +`https://registry.example` are two different registries. An unknown host returns `404 NAME_UNKNOWN`, so containerd falls back to its next host. Registry URLs with a path (for example an Artifactory repository path) are not reachable through `ns`, only through `upstream/{name}/`. When two diff --git a/internal/handler/container.go b/internal/handler/container.go index 7ba3aae7..7f35600e 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -27,8 +27,11 @@ const ( defaultNamespaceRoute = "" ) -// dockerHubNamespaces are the registry hosts clients use for Docker Hub. -var dockerHubNamespaces = []string{"docker.io", "index.docker.io", "registry-1.docker.io"} //nolint:gochecknoglobals // fixed alias list +// dockerHubNamespaces are the registry URLs clients use for Docker Hub. +var dockerHubNamespaces = []string{"https://docker.io", "https://index.docker.io", "https://registry-1.docker.io"} //nolint:gochecknoglobals // fixed alias list + +// schemeDefaultPorts are the ports an image reference may leave out. +var schemeDefaultPorts = map[string]string{"https": "443", "http": "80"} //nolint:gochecknoglobals // fixed table // ContainerHandler handles OCI/Docker container registry protocol requests. // It implements the OCI Distribution Spec for pulling images. @@ -97,61 +100,101 @@ func newContainerHandler( // below it. On collisions the default route wins, then the alphabetically // first upstream name. func (h *ContainerHandler) buildNamespaceIndex() { - h.namespaces = make(map[string]string, len(dockerHubNamespaces)+1+len(h.namedRegistries)) - for _, host := range dockerHubNamespaces { - h.namespaces[host] = defaultNamespaceRoute + h.namespaces = make(map[string]string) + for _, registryURL := range dockerHubNamespaces { + h.indexNamespace(defaultNamespaceRoute, registryURL) } - if host, ok := namespaceHostForURL(h.registryURL); ok { - h.namespaces[host] = defaultNamespaceRoute - } else { + if !h.indexNamespace(defaultNamespaceRoute, h.registryURL) { h.warn("host of the default OCI registry is not indexed for ns lookups: URL has a path; Docker Hub aliases still select it", "url", redactedURL(h.registryURL)) } for _, name := range slices.Sorted(maps.Keys(h.namedRegistries)) { - host, ok := namespaceHostForURL(h.namedRegistries[name]) - if !ok { + if !h.indexNamespace(name, h.namedRegistries[name]) { h.warn("OCI upstream is not reachable through the ns query parameter: URL has a path", "upstream", name, "url", redactedURL(h.namedRegistries[name])) - continue } - if owner, exists := h.namespaces[host]; exists { + } +} + +// indexNamespace maps the ns lookup keys of registryURL to route. Keys that +// another route already owns stay with that route and are reported once. It +// reports false when the URL is not a bare registry root. +func (h *ContainerHandler) indexNamespace(route, registryURL string) bool { + keys, ok := namespaceKeysForURL(registryURL) + if !ok { + return false + } + var taken []string + for _, key := range keys { + owner, exists := h.namespaces[key] + switch { + case !exists: + h.namespaces[key] = route + case owner != route: if owner == defaultNamespaceRoute { owner = "default registry" } - h.warn("OCI upstream shares its registry host with another route; ns requests use the other route", - "upstream", name, "host", host, "route", owner) - continue + taken = append(taken, key+"="+owner) } - h.namespaces[host] = name } + if len(taken) > 0 { + h.warn("OCI upstream shares a registry host with another route; ns requests for it use the other route", + "upstream", route, "hosts", strings.Join(taken, ", ")) + } + return true } -// namespaceHostForURL returns the ns lookup key for a registry URL. It reports -// false for URLs that are not a bare registry root. -func namespaceHostForURL(registryURL string) (string, bool) { +// namespaceKeysForURL returns the ns lookup keys of a registry URL. The +// scheme-default port is optional in image references, so such a URL is +// indexed both without and with the port. Any other port is kept as is, which +// keeps https://host:80 and https://host apart. It reports false for URLs +// that are not a bare registry root. +func namespaceKeysForURL(registryURL string) ([]string, bool) { parsed, err := url.Parse(registryURL) if err != nil || parsed.Host == "" || (parsed.Path != "" && parsed.Path != "/") { - return "", false + return nil, false + } + host, port := splitNamespaceHost(parsed.Host) + defaultPort := schemeDefaultPorts[strings.ToLower(parsed.Scheme)] + switch { + case port != "" && port != defaultPort: + return []string{namespaceKey(host, port)}, true + case defaultPort == "": + return []string{namespaceKey(host, "")}, true + default: + return []string{namespaceKey(host, ""), namespaceKey(host, defaultPort)}, true } - return registryHostKey(parsed.Host), true } -// registryHostKey normalizes a registry host[:port] for ns lookups. Hosts are -// case-insensitive and ports 80 and 443 are dropped because ns carries no -// scheme. The same function normalizes both configured URLs and ns values. -func registryHostKey(hostport string) string { +// namespaceKeyForRequest normalizes the ns value of a request. containerd +// sends the registry host of the image reference, with or without a port, so +// the value is matched as sent apart from case and IPv6 bracket form. +func namespaceKeyForRequest(namespace string) string { + host, port := splitNamespaceHost(namespace) + return namespaceKey(host, port) +} + +// splitNamespaceHost splits host[:port], accepting bracketed and bare IPv6 +// hosts without a port. +func splitNamespaceHost(hostport string) (host, port string) { host, port, err := net.SplitHostPort(hostport) if err != nil { - host, port = strings.TrimSuffix(strings.TrimPrefix(hostport, "["), "]"), "" + return strings.TrimSuffix(strings.TrimPrefix(hostport, "["), "]"), "" } + return host, port +} + +// namespaceKey builds a lookup key from a host and an optional port: the host +// lowercased, IPv6 literals in brackets. +func namespaceKey(host, port string) string { host = strings.ToLower(host) - if port != "" && port != "80" && port != "443" { - return net.JoinHostPort(host, port) - } if strings.Contains(host, ":") { - return "[" + host + "]" + host = "[" + host + "]" + } + if port == "" { + return host } - return host + return host + ":" + port } // redactedURL returns a registry URL for logging with any password masked. @@ -413,7 +456,7 @@ func (h *ContainerHandler) registryForNamespace(namespace, name string) (registr if strings.HasPrefix(name, "upstream/") { return "", "", "", false } - route, ok := h.namespaces[registryHostKey(namespace)] + route, ok := h.namespaces[namespaceKeyForRequest(namespace)] if !ok { return "", "", "", false } diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index d0ec82bf..9b8e1d85 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -5,8 +5,10 @@ import ( "context" "crypto/sha256" "encoding/hex" + "fmt" "io" "log/slog" + "net" "net/http" "net/http/httptest" "net/url" @@ -311,9 +313,10 @@ func TestContainerHandler_NamespaceDefaultBypassesRepositoryPrefixRoutes(t *test } func TestContainerHandler_NamespaceMatchesConfiguredHosts(t *testing.T) { - // Each pair is a configured registry URL and the ns value containerd - // sends for an image reference on that registry. - tests := []struct { + // Each pair is a configured registry URL and an ns value containerd sends + // for an image reference on that registry. The scheme-default port may be + // spelled out or left out on either side; any other port must match. + matches := []struct { configured string namespace string }{ @@ -321,12 +324,15 @@ func TestContainerHandler_NamespaceMatchesConfiguredHosts(t *testing.T) { {"https://ghcr.io/", "GHCR.IO"}, {"http://[fd00::1]:5000", "[fd00::1]:5000"}, {"https://[fd00::1]", "[fd00::1]"}, + {"https://[fd00::1]", "[fd00::1]:443"}, {"https://reg.example:80", "reg.example:80"}, {"https://reg.example:443", "reg.example"}, {"https://reg.example", "reg.example:443"}, + {"http://reg.example:80", "reg.example"}, + {"http://reg.example", "reg.example:80"}, {"http://reg.example:5000", "reg.example:5000"}, } - for _, tt := range tests { + for _, tt := range matches { t.Run(tt.configured+" "+tt.namespace, func(t *testing.T) { h := NewContainerHandlerWithRegistry(nil, nsTestProxyURL, "", map[string]string{"lab": tt.configured}) registryURL, upstreamName, cacheName, ok := h.registryForNamespace(tt.namespace, "owner/app") @@ -340,9 +346,69 @@ func TestContainerHandler_NamespaceMatchesConfiguredHosts(t *testing.T) { }) } - if _, _, _, ok := (NewContainerHandlerWithRegistry(nil, nsTestProxyURL, "", map[string]string{"lab": "http://reg.example:5000"})). - registryForNamespace("reg.example:5001", "owner/app"); ok { - t.Error("ns with a different port resolved, want no match") + mismatches := []struct { + configured string + namespace string + }{ + {"http://reg.example:5000", "reg.example:5001"}, + {"https://reg.example:80", "reg.example"}, + {"https://reg.example", "reg.example:80"}, + {"http://reg.example:443", "reg.example"}, + } + for _, tt := range mismatches { + t.Run("mismatch "+tt.configured+" "+tt.namespace, func(t *testing.T) { + h := NewContainerHandlerWithRegistry(nil, nsTestProxyURL, "", map[string]string{"lab": tt.configured}) + if _, _, _, ok := h.registryForNamespace(tt.namespace, "owner/app"); ok { + t.Errorf("ns %q resolved for upstream %q, want no match", tt.namespace, tt.configured) + } + }) + } +} + +func TestContainerHandler_NamespaceKeepsNonDefaultPorts(t *testing.T) { + // Two registries on one host that differ only in the port, one of them on + // the scheme-default port. Tests cannot listen on ports 80 or 443, so the + // dialer maps those addresses to the fake registries. + onDefaultPort := newNSTestRegistry(t, "owner/app", "") + onOtherPort := newNSTestRegistry(t, "owner/app", "") + routes, h, _ := newNSTestHandler(t, "", map[string]string{ + "std": "http://registry.example", + "alt": "http://registry.example:443", + }) + endpoints := map[string]string{ + "registry.example:80": onDefaultPort.Listener.Addr().String(), + "registry.example:443": onOtherPort.Listener.Addr().String(), + } + transport := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + target, ok := endpoints[addr] + if !ok { + return nil, fmt.Errorf("unexpected dial to %s", addr) + } + return (&net.Dialer{}).DialContext(ctx, network, target) + }} + t.Cleanup(transport.CloseIdleConnections) + h.proxy.HTTPClient = &http.Client{Transport: transport} + + // Every request below misses the cache: a different registry or a + // different endpoint than the request before it. + tests := []struct { + target string + wantDefault int + wantOther int + }{ + {"/v2/owner/app/manifests/latest?ns=registry.example:80", 1, 0}, + {"/v2/owner/app/manifests/latest?ns=registry.example:443", 1, 1}, + {"/v2/owner/app/tags/list?ns=registry.example", 2, 1}, + } + for _, tt := range tests { + response := serveNS(routes, tt.target) + if response.Code != http.StatusOK { + t.Fatalf("%s status = %d: %s", tt.target, response.Code, response.Body.String()) + } + if onDefaultPort.requestCount() != tt.wantDefault || onOtherPort.requestCount() != tt.wantOther { + t.Errorf("after %s: requests default-port=%d other-port=%d, want %d/%d", + tt.target, onDefaultPort.requestCount(), onOtherPort.requestCount(), tt.wantDefault, tt.wantOther) + } } } @@ -378,6 +444,30 @@ func TestContainerHandler_NamespaceHostCollisions(t *testing.T) { } } +func TestContainerHandler_NamespaceCollisionWarningNamesEachOwner(t *testing.T) { + // r's two keys end up with two different owners; the warning has to + // name both, or an admin fixing one entry misses the other. + proxy, _, _, _ := setupTestProxy(t) + logs := &bytes.Buffer{} + proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) + h := NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, "", map[string]string{ + "p": "http://h.example", + "q": "http://h.example:443", + "r": "https://h.example", + }) + + for namespace, want := range map[string]string{"h.example": "p", "h.example:80": "p", "h.example:443": "q"} { + if got := h.namespaces[namespace]; got != want { + t.Errorf("index[%q] = %q, want %q", namespace, got, want) + } + } + for _, want := range []string{"upstream=r", "h.example=p", "h.example:443=q"} { + if !strings.Contains(logs.String(), want) { + t.Errorf("logs missing %q:\n%s", want, logs.String()) + } + } +} + func TestContainerHandler_NamespaceSkipsRegistryURLsWithPath(t *testing.T) { registry := newNSTestRegistry(t, "owner/app", "/artifactory/api/docker/remote") routes, _, logs := newNSTestHandler(t, "", map[string]string{ From fde5ffc33ac50b1833ae9d9a84304e79b49c25f2 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 21:06:49 +0200 Subject: [PATCH 07/10] Allow upstream/{name}/ together with ns when they agree containerd sends ns on every request to a mirror host, override_path or not. Per-registry mirrors pointing at /v2/upstream/{name} therefore arrive as upstream/{name}/...?ns=, and refusing every prefixed name under ns broke them, including the only way to mirror an upstream whose URL has a path. Such requests are now handled like the prefix route without ns, as long as ns names that upstream's own host under the same port rules as the index. Any other ns on a prefixed name is still NAME_UNKNOWN, so ns can't be used to create cache entries under another registry's name. --- docs/configuration.md | 4 ++ internal/handler/container.go | 42 +++++++++++++--- internal/handler/container_ns_test.go | 72 ++++++++++++++++++++++++--- 3 files changed, 102 insertions(+), 16 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 4767db2d..044e3b0b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -306,6 +306,10 @@ are not reachable through `ns`, only through `upstream/{name}/`. When two entries share a host, the proxy logs a warning at startup; the default registry wins, otherwise the alphabetically first name. Pulls through `ns`, `upstream/{name}/` and unprefixed requests share the same cache entries. +Requests that combine the `upstream/{name}/` prefix with `ns`, as per-registry +containerd mirrors with `override_path = true` send them, are accepted when +`ns` names that upstream's host, also for upstreams whose URL has a path, and +rejected otherwise. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. diff --git a/internal/handler/container.go b/internal/handler/container.go index 7f35600e..62adaa47 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -148,21 +148,27 @@ func (h *ContainerHandler) indexNamespace(route, registryURL string) bool { // scheme-default port is optional in image references, so such a URL is // indexed both without and with the port. Any other port is kept as is, which // keeps https://host:80 and https://host apart. It reports false for URLs -// that are not a bare registry root. +// that are not a bare registry root; namespaceKeysForHost serves those. func namespaceKeysForURL(registryURL string) ([]string, bool) { parsed, err := url.Parse(registryURL) if err != nil || parsed.Host == "" || (parsed.Path != "" && parsed.Path != "/") { return nil, false } + return namespaceKeysForHost(parsed), true +} + +// namespaceKeysForHost returns the lookup keys of a URL's host, ignoring its +// path. +func namespaceKeysForHost(parsed *url.URL) []string { host, port := splitNamespaceHost(parsed.Host) defaultPort := schemeDefaultPorts[strings.ToLower(parsed.Scheme)] switch { case port != "" && port != defaultPort: - return []string{namespaceKey(host, port)}, true + return []string{namespaceKey(host, port)} case defaultPort == "": - return []string{namespaceKey(host, "")}, true + return []string{namespaceKey(host, "")} default: - return []string{namespaceKey(host, ""), namespaceKey(host, defaultPort)}, true + return []string{namespaceKey(host, ""), namespaceKey(host, defaultPort)} } } @@ -449,12 +455,19 @@ func (h *ContainerHandler) registryForRequest(r *http.Request, name string) (reg } // registryForNamespace resolves a repository name verbatim against the registry -// named by ns. The reserved upstream/ prefix is rejected so ns requests cannot -// address cache entries of another route. Cache names match the unprefixed and -// upstream/{name}/ routes, so all routes to one registry share blobs. +// named by ns. Cache names match the unprefixed and upstream/{name}/ routes, +// so all routes to one registry share blobs. +// +// Per-registry containerd mirrors with override_path address the reserved +// upstream/{name}/ prefix and still send ns. Such requests are served like +// the prefix route without ns, but only when ns names that upstream's own +// host, so an ns request can never mint cache entries of another registry. func (h *ContainerHandler) registryForNamespace(namespace, name string) (registryURL, upstreamName, cacheName string, ok bool) { if strings.HasPrefix(name, "upstream/") { - return "", "", "", false + if !h.namespaceNamesPrefixUpstream(namespace, name) { + return "", "", "", false + } + return h.registryForName(name) } route, ok := h.namespaces[namespaceKeyForRequest(namespace)] if !ok { @@ -475,6 +488,19 @@ func (h *ContainerHandler) registryForNamespace(namespace, name string) (registr return registryURL, name, "upstream/" + route + "/" + name, true } +// namespaceNamesPrefixUpstream reports whether ns names the host of the +// upstream an upstream/{name}/ repository selects. The URL's path is +// irrelevant here because the prefix already picks the upstream. +func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) bool { + rest, _ := strings.CutPrefix(name, "upstream/") + upstream, _, _ := strings.Cut(rest, "/") + parsed, err := url.Parse(h.namedRegistries[upstream]) + if err != nil || parsed.Host == "" { + return false + } + return slices.Contains(namespaceKeysForHost(parsed), namespaceKeyForRequest(namespace)) +} + // registryForName resolves a client-visible OCI repository name to an upstream // registry and its repository name. Named upstreams use upstream/{name}/ as a // reserved prefix. Other names are matched against registered repository diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index 9b8e1d85..0142339f 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -174,27 +174,76 @@ func TestContainerHandler_NamespaceSelectsNamedRegistry(t *testing.T) { func TestContainerHandler_NamespaceRejectsUnresolvableRequests(t *testing.T) { registry := newNSTestRegistry(t, "owner/app", "") - routes, _, _ := newNSTestHandler(t, registry.URL, map[string]string{"ghcr": registry.URL}) + other := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, registry.URL, map[string]string{"ghcr": registry.URL, "quay": other.URL}) digest := "sha256:" + sha256Hex(nsTestBlob) tests := map[string]string{ - "unknown host manifest": "/v2/owner/app/manifests/latest?ns=quay.io", - "unknown host blob": "/v2/owner/app/blobs/" + digest + "?ns=quay.io", - "unknown host tags": "/v2/owner/app/tags/list?ns=quay.io", - "multiple ns values": "/v2/owner/app/manifests/latest?ns=docker.io&ns=" + registry.host(), - "reserved prefix (default)": "/v2/upstream/ghcr/owner/app/manifests/latest?ns=docker.io", - "reserved prefix (named)": "/v2/upstream/ghcr/owner/app/blobs/" + digest + "?ns=" + registry.host(), + "unknown host manifest": "/v2/owner/app/manifests/latest?ns=quay.io", + "unknown host blob": "/v2/owner/app/blobs/" + digest + "?ns=quay.io", + "unknown host tags": "/v2/owner/app/tags/list?ns=quay.io", + "multiple ns values": "/v2/owner/app/manifests/latest?ns=docker.io&ns=" + registry.host(), + "prefix route with default ns": "/v2/upstream/ghcr/owner/app/manifests/latest?ns=docker.io", + "prefix route with other upstream": "/v2/upstream/ghcr/owner/app/blobs/" + digest + "?ns=" + other.host(), + "prefix route with unknown upstream": "/v2/upstream/nope/owner/app/manifests/latest?ns=" + registry.host(), + "prefix route without repository": "/v2/upstream/ghcr/manifests/latest?ns=" + registry.host(), } for name, target := range tests { t.Run(name, func(t *testing.T) { assertNameUnknown(t, serveNS(routes, target)) }) } - if got := registry.requestCount(); got != 0 { + if got := registry.requestCount() + other.requestCount(); got != 0 { t.Errorf("upstream requests = %d, want 0", got) } } +func TestContainerHandler_NamespaceAcceptsPrefixRouteForOwnHost(t *testing.T) { + // Per-registry containerd mirrors with override_path address the + // upstream/{name}/ prefix and still send ns. They must keep working and + // share the prefix route's cache entries. + registry := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, "", map[string]string{"ghcr": registry.URL}) + digest := "sha256:" + sha256Hex(nsTestBlob) + + for _, target := range []string{ + "/v2/upstream/ghcr/owner/app/manifests/latest?ns=" + registry.host(), + "/v2/upstream/ghcr/owner/app/blobs/" + digest + "?ns=" + registry.host(), + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("%s status = %d: %s", target, response.Code, response.Body.String()) + } + } + if got, want := registry.lastRequest(), "/v2/owner/app/blobs/"+digest; got != want { + t.Errorf("upstream request = %q, want %q", got, want) + } + warmed := registry.requestCount() + + for _, target := range []string{ + "/v2/upstream/ghcr/owner/app/manifests/latest", + "/v2/upstream/ghcr/owner/app/blobs/" + digest, + } { + if response := serveNS(routes, target); response.Code != http.StatusOK { + t.Fatalf("%s status = %d: %s", target, response.Code, response.Body.String()) + } + } + if got := registry.requestCount(); got != warmed { + t.Errorf("upstream requests after prefix pulls without ns = %d, want %d (cache hits)", got, warmed) + } + + tags := serveNS(routes, "/v2/upstream/ghcr/owner/app/tags/list?n=1&ns="+registry.host()) + if tags.Code != http.StatusOK { + t.Fatalf("tags status = %d: %s", tags.Code, tags.Body.String()) + } + wantLink := `<` + nsTestProxyURL + `/v2/upstream/ghcr/owner/app/tags/list?last=1.0&n=1&ns=` + url.QueryEscape(registry.host()) + `>; rel="next"` + if got := tags.Header().Get("Link"); got != wantLink { + t.Errorf("Link = %q, want %q", got, wantLink) + } + if got, want := registry.lastRequest(), "/v2/owner/app/tags/list?n=1"; got != want { + t.Errorf("upstream tags request = %q, want %q (ns must not be forwarded)", got, want) + } +} + func TestContainerHandler_NamespaceSharesCacheWithOtherRoutes(t *testing.T) { digest := "sha256:" + sha256Hex(nsTestBlob) manifestDigest := "sha256:" + sha256Hex(nsTestManifest()) @@ -482,6 +531,13 @@ func TestContainerHandler_NamespaceSkipsRegistryURLsWithPath(t *testing.T) { t.Errorf("missing warning for path-prefixed upstream in logs:\n%s", logs.String()) } + // Per-registry mirrors with override_path reach it with ns attached. + if response := serveNS(routes, "/v2/upstream/art/owner/app/manifests/latest?ns="+registry.host()); response.Code != http.StatusOK { + t.Fatalf("prefix route with ns status = %d, want 200: %s", response.Code, response.Body.String()) + } + if got := registry.requestCount(); got != 1 { + t.Errorf("upstream requests via prefix route with ns = %d, want 1", got) + } if response := serveNS(routes, "/v2/upstream/art/owner/app/manifests/latest"); response.Code != http.StatusOK { t.Fatalf("prefix route status = %d, want 200: %s", response.Code, response.Body.String()) } From 9fef87677207e62e48c08a6ba719990cae24984d Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 21:06:49 +0200 Subject: [PATCH 08/10] README: containerd config_path and how to check it A _default/hosts.toml does nothing while CRI has no hosts directory configured, so add the config.toml snippet for containerd 2.x and 1.x. To check the setup, look at containerd config dump and pull with crictl, then find the request in the proxy log; ctr --hosts-dir reads the directory on its own and would succeed even without config_path. Also mention that existing per-registry override_path entries keep working and that k3s generates the directory itself. --- README.md | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 80828e9f..3392c323 100644 --- a/README.md +++ b/README.md @@ -451,20 +451,55 @@ docker pull localhost:8080/library/nginx:latest containerd mirrors send the original registry host in an `ns` query parameter, so one mirror entry can serve Docker Hub and the registries -configured in `upstream.oci` whose URL has no path. Create -`/etc/containerd/certs.d/_default/hosts.toml`: +configured in `upstream.oci` whose URL has no path. Point containerd's CRI +plugin at a hosts directory in `/etc/containerd/config.toml` and restart +containerd. For containerd 2.x: + +```toml +version = 3 + +[plugins."io.containerd.cri.v1.images".registry] + config_path = "/etc/containerd/certs.d" +``` + +For containerd 1.x: + +```toml +version = 2 + +[plugins."io.containerd.grpc.v1.cri".registry] + config_path = "/etc/containerd/certs.d" +``` + +Then create `/etc/containerd/certs.d/_default/hosts.toml`: ```toml [host."http://proxy.example.com:8080"] capabilities = ["pull", "resolve"] ``` +To check that CRI picked up the directory, confirm the setting containerd +runs with and pull through CRI (on k3s use `k3s crictl`): + +```bash +containerd config dump | grep config_path +crictl pull docker.io/library/nginx:latest +``` + +The proxy logs a `container manifest request` line for the pull and, when +`access_log.path` is set, an access-log entry. `ctr images pull --hosts-dir` +is no substitute for this check: it reads the directory itself and succeeds +even while CRI still has no `config_path`. + `docker.io` and the host of `upstream.oci_default` use the default registry; the host of each `upstream.oci` URL uses that named registry. Pulls for any other registry get `404 NAME_UNKNOWN`, and containerd falls back to the -registry itself. See [docs/configuration.md](docs/configuration.md) for how -hosts are matched and which entry wins when two share a host. k3s achieves the same with `mirrors: {"*": {endpoint: -["http://proxy.example.com:8080"]}}` in `/etc/rancher/k3s/registries.yaml`. +registry itself. Existing per-registry `hosts.toml` files that point at +`/v2/upstream/{name}` with `override_path = true` keep working. See [docs/configuration.md](docs/configuration.md) for how +hosts are matched and which entry wins when two share a host. k3s generates +the hosts directory itself from `/etc/rancher/k3s/registries.yaml`; `mirrors: +{"*": {endpoint: ["http://proxy.example.com:8080"]}}` produces the same +`_default` entry. ### Helm From 2969771573061b6590b185eec4e217fe1fc71fd3 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 5 Oct 2026 23:20:20 +0200 Subject: [PATCH 09/10] Let the prefix route take ns for aliases and mirrored registries With upstream.oci.hub pointing at registry-1.docker.io and a docker.io hosts.toml mirror on /v2/upstream/hub, containerd sends ns=docker.io, and the prefix check only accepted the upstream's own host. The same happens whenever the upstream is a mirror of the registry the nodes pull from, say an Artifactory remote for ghcr.io: ns carries ghcr.io, the upstream host is something else, and the pull got a 404. The prefix already decides where the content comes from and which cache entries are used, so ns can't redirect anything there. The check now only refuses an ns that contradicts the prefix, meaning a host the proxy knows that belongs to another route. The Docker Hub aliases count as one host, and a host the proxy doesn't know at all is accepted, since a client can only arrive at the prefix through its own mirror entry. --- README.md | 3 +- docs/configuration.md | 8 ++-- internal/handler/container.go | 40 +++++++++++++++--- internal/handler/container_ns_test.go | 61 +++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 3392c323..dab4ca53 100644 --- a/README.md +++ b/README.md @@ -495,7 +495,8 @@ even while CRI still has no `config_path`. the host of each `upstream.oci` URL uses that named registry. Pulls for any other registry get `404 NAME_UNKNOWN`, and containerd falls back to the registry itself. Existing per-registry `hosts.toml` files that point at -`/v2/upstream/{name}` with `override_path = true` keep working. See [docs/configuration.md](docs/configuration.md) for how +`/v2/upstream/{name}` with `override_path = true` keep working, also when +that upstream is a mirror of the registry the nodes pull from. See [docs/configuration.md](docs/configuration.md) for how hosts are matched and which entry wins when two share a host. k3s generates the hosts directory itself from `/etc/rancher/k3s/registries.yaml`; `mirrors: {"*": {endpoint: ["http://proxy.example.com:8080"]}}` produces the same diff --git a/docs/configuration.md b/docs/configuration.md index 044e3b0b..6d0f62f3 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -307,9 +307,11 @@ entries share a host, the proxy logs a warning at startup; the default registry wins, otherwise the alphabetically first name. Pulls through `ns`, `upstream/{name}/` and unprefixed requests share the same cache entries. Requests that combine the `upstream/{name}/` prefix with `ns`, as per-registry -containerd mirrors with `override_path = true` send them, are accepted when -`ns` names that upstream's host, also for upstreams whose URL has a path, and -rejected otherwise. +containerd mirrors with `override_path = true` send them, are routed by the +prefix. They are refused only when `ns` names a host that belongs to another +configured route. The Docker Hub aliases count as one host, and a host the +proxy does not know is accepted, for example `ghcr.io` when the upstream is a +mirror of it, because only a mirror entry on the client leads there. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. diff --git a/internal/handler/container.go b/internal/handler/container.go index 62adaa47..93e9b8d5 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -33,6 +33,22 @@ var dockerHubNamespaces = []string{"https://docker.io", "https://index.docker.io // schemeDefaultPorts are the ports an image reference may leave out. var schemeDefaultPorts = map[string]string{"https": "443", "http": "80"} //nolint:gochecknoglobals // fixed table +// dockerHubNamespaceKeys are the ns lookup keys of the Docker Hub aliases. +var dockerHubNamespaceKeys = dockerHubKeys() //nolint:gochecknoglobals // fixed table + +func dockerHubKeys() []string { + var keys []string + for _, registryURL := range dockerHubNamespaces { + hostKeys, _ := namespaceKeysForURL(registryURL) + keys = append(keys, hostKeys...) + } + return keys +} + +func isDockerHubKey(key string) bool { + return slices.Contains(dockerHubNamespaceKeys, key) +} + // ContainerHandler handles OCI/Docker container registry protocol requests. // It implements the OCI Distribution Spec for pulling images. // Reference: https://github.com/opencontainers/distribution-spec/blob/main/spec.md @@ -460,8 +476,8 @@ func (h *ContainerHandler) registryForRequest(r *http.Request, name string) (reg // // Per-registry containerd mirrors with override_path address the reserved // upstream/{name}/ prefix and still send ns. Such requests are served like -// the prefix route without ns, but only when ns names that upstream's own -// host, so an ns request can never mint cache entries of another registry. +// the prefix route without ns unless ns contradicts the prefix, see +// namespaceNamesPrefixUpstream. func (h *ContainerHandler) registryForNamespace(namespace, name string) (registryURL, upstreamName, cacheName string, ok bool) { if strings.HasPrefix(name, "upstream/") { if !h.namespaceNamesPrefixUpstream(namespace, name) { @@ -488,9 +504,15 @@ func (h *ContainerHandler) registryForNamespace(namespace, name string) (registr return registryURL, name, "upstream/" + route + "/" + name, true } -// namespaceNamesPrefixUpstream reports whether ns names the host of the -// upstream an upstream/{name}/ repository selects. The URL's path is -// irrelevant here because the prefix already picks the upstream. +// namespaceNamesPrefixUpstream decides whether an upstream/{name}/ request +// may carry the given ns. The prefix already picks the upstream and the +// cache entries, so ns cannot change where content comes from; the check +// only refuses an ns that contradicts the prefix. It passes for the +// upstream's own host (the Docker Hub aliases count as one host, and the +// URL's path is irrelevant) and for a host this proxy does not know, which +// is a per-registry mirror entry for a registry the upstream mirrors, say an +// Artifactory remote for ghcr.io. It fails for a host that belongs to another +// route. func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) bool { rest, _ := strings.CutPrefix(name, "upstream/") upstream, _, _ := strings.Cut(rest, "/") @@ -498,7 +520,13 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) if err != nil || parsed.Host == "" { return false } - return slices.Contains(namespaceKeysForHost(parsed), namespaceKeyForRequest(namespace)) + key := namespaceKeyForRequest(namespace) + hosts := namespaceKeysForHost(parsed) + if slices.Contains(hosts, key) || (isDockerHubKey(key) && slices.ContainsFunc(hosts, isDockerHubKey)) { + return true + } + _, known := h.namespaces[key] + return !known } // registryForName resolves a client-visible OCI repository name to an upstream diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index 0142339f..28aa0912 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -244,6 +244,67 @@ func TestContainerHandler_NamespaceAcceptsPrefixRouteForOwnHost(t *testing.T) { } } +func TestContainerHandler_NamespacePrefixRouteAcceptsDockerHubAliases(t *testing.T) { + // upstream.oci.hub points at Docker Hub and a docker.io hosts.toml mirror + // with override_path addresses /v2/upstream/hub, so containerd sends + // ns=docker.io. The dialer stands in for registry-1.docker.io. + hub := newNSTestRegistry(t, "library/nginx", "") + other := newNSTestRegistry(t, "library/nginx", "") + routes, h, _ := newNSTestHandler(t, "", map[string]string{ + "hub": "http://registry-1.docker.io", + "quay": other.URL, + }) + transport := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if addr != "registry-1.docker.io:80" { + return nil, fmt.Errorf("unexpected dial to %s", addr) + } + return (&net.Dialer{}).DialContext(ctx, network, hub.Listener.Addr().String()) + }} + t.Cleanup(transport.CloseIdleConnections) + h.proxy.HTTPClient = &http.Client{Transport: transport} + + for _, query := range []string{"?ns=docker.io", "?ns=index.docker.io", "?ns=registry-1.docker.io", "?ns=docker.io:443", ""} { + response := serveNS(routes, "/v2/upstream/hub/library/nginx/manifests/latest"+query) + if response.Code != http.StatusOK { + t.Fatalf("%q status = %d: %s", query, response.Code, response.Body.String()) + } + } + if got := hub.requestCount(); got != 1 { + t.Errorf("hub requests = %d, want 1 (the rest are cache hits)", got) + } + + // A known host of another route still contradicts the prefix. + assertNameUnknown(t, serveNS(routes, "/v2/upstream/hub/library/nginx/manifests/latest?ns="+other.host())) + if got := other.requestCount(); got != 0 { + t.Errorf("other registry requests = %d, want 0", got) + } +} + +func TestContainerHandler_NamespacePrefixRouteTrustsUnknownHosts(t *testing.T) { + // The upstream is a mirror of ghcr.io, nodes keep pulling ghcr.io/... and + // their ghcr.io hosts.toml points at /v2/upstream/ghcr, so ns says + // ghcr.io while the upstream host is something else. Only a mirror entry + // of the client can lead here, so the prefix wins. + tests := map[string]string{ + "plain mirror host": "", + "artifactory remote": "/artifactory/api/docker/ghcr-remote", + } + for name, pathPrefix := range tests { + t.Run(name, func(t *testing.T) { + mirror := newNSTestRegistry(t, "owner/app", pathPrefix) + routes, _, _ := newNSTestHandler(t, "", map[string]string{"ghcr": mirror.URL + pathPrefix}) + + response := serveNS(routes, "/v2/upstream/ghcr/owner/app/manifests/latest?ns=ghcr.io") + if response.Code != http.StatusOK { + t.Fatalf("status = %d: %s", response.Code, response.Body.String()) + } + if got, want := mirror.lastRequest(), pathPrefix+"/v2/owner/app/manifests/latest"; got != want { + t.Errorf("upstream request = %q, want %q", got, want) + } + }) + } +} + func TestContainerHandler_NamespaceSharesCacheWithOtherRoutes(t *testing.T) { digest := "sha256:" + sha256Hex(nsTestBlob) manifestDigest := "sha256:" + sha256Hex(nsTestManifest()) From 314b27bef46ce0f69a1c4fa1e1f49b2baa4a54e4 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Tue, 6 Oct 2026 00:41:28 +0200 Subject: [PATCH 10/10] Take docker.io on every prefix route A Docker Hub mirror configured as a named upstream (mirror.gcr.io, an Artifactory remote) with a docker.io hosts.toml pointing at /v2/upstream/hub got a 404 again: docker.io is always in the index for the default route, so the check saw a known host of another route. That setup worked before this branch. Docker Hub repository names have exactly two path components, so docker.io/upstream/... can never be a real image, and accepting the aliases on every prefix route shadows nothing. While here: the collision warning claimed ns requests go to the other route, which is only true without the prefix, and the README said pulls for unknown registries always get a 404, which isn't the case for paths under upstream/{name}/. --- README.md | 3 ++- docs/configuration.md | 8 +++++--- internal/handler/container.go | 19 ++++++++++--------- internal/handler/container_ns_test.go | 18 +++++++++++++++++- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index dab4ca53..2afb43d8 100644 --- a/README.md +++ b/README.md @@ -494,7 +494,8 @@ even while CRI still has no `config_path`. `docker.io` and the host of `upstream.oci_default` use the default registry; the host of each `upstream.oci` URL uses that named registry. Pulls for any other registry get `404 NAME_UNKNOWN`, and containerd falls back to the -registry itself. Existing per-registry `hosts.toml` files that point at +registry itself; only an image path that itself starts with `upstream/{name}/` +always selects that upstream. Existing per-registry `hosts.toml` files that point at `/v2/upstream/{name}` with `override_path = true` keep working, also when that upstream is a mirror of the registry the nodes pull from. See [docs/configuration.md](docs/configuration.md) for how hosts are matched and which entry wins when two share a host. k3s generates diff --git a/docs/configuration.md b/docs/configuration.md index 6d0f62f3..ac7e8ee4 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -309,9 +309,11 @@ wins, otherwise the alphabetically first name. Pulls through `ns`, Requests that combine the `upstream/{name}/` prefix with `ns`, as per-registry containerd mirrors with `override_path = true` send them, are routed by the prefix. They are refused only when `ns` names a host that belongs to another -configured route. The Docker Hub aliases count as one host, and a host the -proxy does not know is accepted, for example `ghcr.io` when the upstream is a -mirror of it, because only a mirror entry on the client leads there. +configured route. Docker Hub (`docker.io` and its aliases) is always accepted +there, because Docker Hub repository names have two path components and can +never start with `upstream/`, so a Docker Hub mirror behind any prefix stays +reachable. A host the proxy does not know is accepted as well, for example +`ghcr.io` when the upstream is a mirror of it. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. diff --git a/internal/handler/container.go b/internal/handler/container.go index 93e9b8d5..1c653772 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -154,7 +154,7 @@ func (h *ContainerHandler) indexNamespace(route, registryURL string) bool { } } if len(taken) > 0 { - h.warn("OCI upstream shares a registry host with another route; ns requests for it use the other route", + h.warn("OCI upstream shares a registry host with another route; unprefixed ns requests for it go to the other route", "upstream", route, "hosts", strings.Join(taken, ", ")) } return true @@ -507,12 +507,14 @@ func (h *ContainerHandler) registryForNamespace(namespace, name string) (registr // namespaceNamesPrefixUpstream decides whether an upstream/{name}/ request // may carry the given ns. The prefix already picks the upstream and the // cache entries, so ns cannot change where content comes from; the check -// only refuses an ns that contradicts the prefix. It passes for the -// upstream's own host (the Docker Hub aliases count as one host, and the -// URL's path is irrelevant) and for a host this proxy does not know, which -// is a per-registry mirror entry for a registry the upstream mirrors, say an -// Artifactory remote for ghcr.io. It fails for a host that belongs to another -// route. +// only refuses an ns that contradicts the prefix, meaning a host this proxy +// knows that belongs to another route. It passes for the upstream's own +// host (the URL's path is irrelevant), for a host this proxy does not know, +// which is a per-registry mirror entry for a registry the upstream mirrors, +// say an Artifactory remote for ghcr.io, and always for Docker Hub: its +// repository names have two path components, so docker.io/upstream/... is +// never a real image and a Docker Hub mirror behind any prefix stays +// reachable. func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) bool { rest, _ := strings.CutPrefix(name, "upstream/") upstream, _, _ := strings.Cut(rest, "/") @@ -521,8 +523,7 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) return false } key := namespaceKeyForRequest(namespace) - hosts := namespaceKeysForHost(parsed) - if slices.Contains(hosts, key) || (isDockerHubKey(key) && slices.ContainsFunc(hosts, isDockerHubKey)) { + if isDockerHubKey(key) || slices.Contains(namespaceKeysForHost(parsed), key) { return true } _, known := h.namespaces[key] diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index 28aa0912..69471706 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -183,7 +183,7 @@ func TestContainerHandler_NamespaceRejectsUnresolvableRequests(t *testing.T) { "unknown host blob": "/v2/owner/app/blobs/" + digest + "?ns=quay.io", "unknown host tags": "/v2/owner/app/tags/list?ns=quay.io", "multiple ns values": "/v2/owner/app/manifests/latest?ns=docker.io&ns=" + registry.host(), - "prefix route with default ns": "/v2/upstream/ghcr/owner/app/manifests/latest?ns=docker.io", + "prefix route with default host": "/v2/upstream/quay/owner/app/manifests/latest?ns=" + registry.host(), "prefix route with other upstream": "/v2/upstream/ghcr/owner/app/blobs/" + digest + "?ns=" + other.host(), "prefix route with unknown upstream": "/v2/upstream/nope/owner/app/manifests/latest?ns=" + registry.host(), "prefix route without repository": "/v2/upstream/ghcr/manifests/latest?ns=" + registry.host(), @@ -278,6 +278,22 @@ func TestContainerHandler_NamespacePrefixRouteAcceptsDockerHubAliases(t *testing if got := other.requestCount(); got != 0 { t.Errorf("other registry requests = %d, want 0", got) } + + t.Run("Docker Hub mirror as named upstream", func(t *testing.T) { + // The upstream is a Docker Hub mirror on some other host (mirror.gcr.io, + // an Artifactory remote); nodes still pull docker.io/... through + // /v2/upstream/hub, so ns says docker.io. + mirror := newNSTestRegistry(t, "library/nginx", "") + routes, _, _ := newNSTestHandler(t, "", map[string]string{"hub": mirror.URL}) + + response := serveNS(routes, "/v2/upstream/hub/library/nginx/manifests/latest?ns=docker.io") + if response.Code != http.StatusOK { + t.Fatalf("status = %d: %s", response.Code, response.Body.String()) + } + if got, want := mirror.lastRequest(), "/v2/library/nginx/manifests/latest"; got != want { + t.Errorf("upstream request = %q, want %q", got, want) + } + }) } func TestContainerHandler_NamespacePrefixRouteTrustsUnknownHosts(t *testing.T) {