From 0bfccca8dba88917cc68a4f514a26761a14b6f2e Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sun, 4 Oct 2026 00:12:03 +0200 Subject: [PATCH 01/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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/22] 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) { From 901dd5cfc68bbe136c823fe5cb55a00c69b6e356 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:44:56 +0200 Subject: [PATCH 11/22] Only take a foreign ns on a prefix route when it is configured A containerd _default mirror sends the same request for a pull of unconfigured.example/upstream/mirror/owner/app as a ghcr.io hosts.toml with override_path does for a mirror upstream. Accepting any host the proxy didn't know therefore let such a pull be answered with owner/app from the mirror upstream, a different image, instead of a 404 that lets containerd go to the registry the image actually names. The prefix route now accepts ns only for the upstream's own host, for Docker Hub, and for hosts listed in the new upstream.oci_mirrors setting. That setting is how an Artifactory remote of ghcr.io keeps working: upstream: oci_mirrors: ghcr: ["ghcr.io"] The listed hosts also select the upstream for unprefixed ns requests, which makes a mirror with a path in its URL reachable through the _default entry as well. Hosts from configured URLs still win over mirror entries; the proxy warns about such overlaps at startup. --- README.md | 15 +++-- config.example.yaml | 6 ++ docs/configuration.md | 26 ++++++++- internal/config/config.go | 27 +++++++++ internal/config/config_test.go | 39 +++++++++++++ internal/handler/container.go | 54 +++++++++++++----- internal/handler/container_ns_test.go | 82 +++++++++++++++++++++++---- internal/server/server.go | 1 + 8 files changed, 214 insertions(+), 36 deletions(-) diff --git a/README.md b/README.md index 2afb43d8..89ef81b4 100644 --- a/README.md +++ b/README.md @@ -492,12 +492,15 @@ 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; 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 +the host of each `upstream.oci` URL uses that named registry, and so do the +hosts listed for it in `upstream.oci_mirrors`. 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 for that upstream's own host and for +Docker Hub. If the upstream is a mirror of the registry the nodes pull from, +say an Artifactory remote of `ghcr.io`, list that registry in +`upstream.oci_mirrors` (`ghcr: ["ghcr.io"]`); otherwise those pulls get the +404 as well. 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/config.example.yaml b/config.example.yaml index 44a3bfc3..a8339322 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -229,6 +229,12 @@ upstream: # oci: # ghcr: "https://ghcr.io" + # Registries a named OCI upstream mirrors, e.g. when ghcr above points at + # an Artifactory remote of ghcr.io. containerd mirror requests whose ns + # parameter names one of these hosts are served by that upstream. + # oci_mirrors: + # ghcr: ["ghcr.io"] + # Named Alpine APK repositories (used by /apk/{name}/). # Defaults to {"alpine": "https://dl-cdn.alpinelinux.org/alpine"} when empty; # configuring any entry replaces that default. diff --git a/docs/configuration.md b/docs/configuration.md index ac7e8ee4..87e66ab9 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -302,7 +302,8 @@ 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 +are not reachable through their own host in `ns`, only through +`upstream/{name}/` or through `upstream.oci_mirrors`. 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. @@ -312,8 +313,27 @@ prefix. They are refused only when `ns` names a host that belongs to another 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. +reachable. Any other host gets `404 NAME_UNKNOWN`: a containerd `_default` +mirror sends the same request for an image such as +`unconfigured.example/upstream/ghcr/owner/app`, and that pull must not be +answered from the `ghcr` upstream. + +When a named upstream mirrors another registry, for example an Artifactory +remote of `ghcr.io`, say so in `upstream.oci_mirrors`: + +```yaml +upstream: + oci: + ghcr: "https://artifactory.example.com/artifactory/api/docker/ghcr-remote" + oci_mirrors: + ghcr: ["ghcr.io"] +``` + +Each entry lists bare registry hosts as containerd sends them, optionally with +a port. Those hosts select the upstream for unprefixed `ns` requests and are +accepted as `ns` on its `upstream/{name}/` prefix. A host that is also the host +of a configured registry URL stays with that registry for unprefixed requests; +the proxy logs a warning at startup. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. diff --git a/internal/config/config.go b/internal/config/config.go index 05ed6d2f..0583c696 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -630,6 +630,12 @@ type UpstreamConfig struct { // oci://proxy.example.com/upstream/ghcr/owner/chart. OCI map[string]string `json:"oci" yaml:"oci"` + // OCIMirrors lists, per upstream.oci name, the registry hosts that + // upstream mirrors, for example {"ghcr": ["ghcr.io"]} for an Artifactory + // remote of ghcr.io. containerd requests whose ns query parameter names + // one of these hosts are served by that upstream. + OCIMirrors map[string][]string `json:"oci_mirrors" yaml:"oci_mirrors"` + // Generic maps names to plain HTTP upstream base URLs, served at // /generic/{name}/. The remaining request path and query string are // appended to the upstream URL. GitHub release asset paths @@ -687,6 +693,9 @@ func (u *UpstreamConfig) Validate() error { if err := validateNamedUpstreams("upstream.oci", u.OCI); err != nil { return err } + if err := validateOCIMirrors(u.OCIMirrors, u.OCI); err != nil { + return err + } if err := validateNamedUpstreams("upstream.generic", u.Generic); err != nil { return err } @@ -708,6 +717,24 @@ func (u *UpstreamConfig) Validate() error { // paths, which a repository of the same name would shadow. var debianReservedRepositoryNames = []string{"pool", "dists"} +// validateOCIMirrors checks that every upstream.oci_mirrors entry belongs to +// an upstream.oci name and lists bare registry hosts, the form containerd +// sends in the ns query parameter. +func validateOCIMirrors(mirrors map[string][]string, upstreams map[string]string) error { + for name, hosts := range mirrors { + if _, ok := upstreams[name]; !ok { + return fmt.Errorf("invalid upstream.oci_mirrors name %q: no upstream.oci entry of that name", name) + } + for _, host := range hosts { + parsed, err := url.Parse("https://" + host) + if host == "" || err != nil || parsed.Host != host || parsed.User != nil { + return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: must be a registry host such as ghcr.io or registry.example:5000", name, host) + } + } + } + return nil +} + func validateNamedUpstreams(field string, upstreams map[string]string) error { for name, upstreamURL := range upstreams { if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\\`) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 20069704..3c08b20b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1336,6 +1336,45 @@ func TestValidateNamedUpstreams(t *testing.T) { }, wantErr: true, }, + { + name: "OCI mirrors for a configured upstream", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://artifactory.example/api/docker/ghcr-remote"} + cfg.Upstream.OCIMirrors = map[string][]string{"ghcr": {"ghcr.io", "registry.example:5000"}} + }, + }, + { + name: "OCI mirrors for an unknown upstream", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://ghcr.io"} + cfg.Upstream.OCIMirrors = map[string][]string{"quay": {"quay.io"}} + }, + wantErr: true, + }, + { + name: "OCI mirror given as URL", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://mirror.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"ghcr": {"https://ghcr.io"}} + }, + wantErr: true, + }, + { + name: "OCI mirror with a path", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://mirror.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"ghcr": {"ghcr.io/owner"}} + }, + wantErr: true, + }, + { + name: "OCI mirror host is empty", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://mirror.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"ghcr": {""}} + }, + wantErr: true, + }, { name: "OCI upstream URL is not absolute", modify: func(cfg *Config) { diff --git a/internal/handler/container.go b/internal/handler/container.go index 1c653772..f8717ef2 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -61,6 +61,9 @@ type ContainerHandler struct { // namespaces maps normalized registry hosts from containerd's ns query // parameter to a named upstream, or to defaultNamespaceRoute. namespaces map[string]string + // mirrorKeys holds, per named upstream, the ns lookup keys of the + // registries it is configured to mirror. + mirrorKeys map[string][]string } type containerRegistry struct { @@ -132,6 +135,31 @@ func (h *ContainerHandler) buildNamespaceIndex() { } } +// SetMirroredRegistries records which registry hosts each named upstream +// mirrors, for example "ghcr" -> ["ghcr.io"] for an Artifactory remote of +// ghcr.io. Those hosts select the upstream in ns lookups and are accepted as +// ns on its upstream/{name}/ prefix. They rank below every host taken from a +// configured URL, so they are indexed after the constructor's index. +func (h *ContainerHandler) SetMirroredRegistries(mirrors map[string][]string) { + h.mirrorKeys = make(map[string][]string, len(mirrors)) + for _, name := range slices.Sorted(maps.Keys(mirrors)) { + if _, ok := h.namedRegistries[name]; !ok { + h.warn("OCI mirror entry has no upstream of that name", "upstream", name) + continue + } + for _, host := range mirrors[name] { + registryURL := "https://" + host + keys, ok := namespaceKeysForURL(registryURL) + if !ok { + h.warn("OCI mirror host is not a registry host", "upstream", name, "host", host) + continue + } + h.mirrorKeys[name] = append(h.mirrorKeys[name], keys...) + h.indexNamespace(name, registryURL) + } + } +} + // 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. @@ -505,16 +533,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, 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. +// may carry the given ns. A per-registry mirror with override_path and a +// _default wildcard pull of /upstream/{name}/... send the same request, +// so ns has to name a registry the upstream is known to serve: its own host +// (the URL's path is irrelevant), a host listed for it in upstream.oci_mirrors, +// or Docker Hub. Docker Hub 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. Any other ns gets a 404 and containerd falls +// back to the registry the image actually names. func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) bool { rest, _ := strings.CutPrefix(name, "upstream/") upstream, _, _ := strings.Cut(rest, "/") @@ -523,11 +549,9 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) return false } key := namespaceKeyForRequest(namespace) - if isDockerHubKey(key) || slices.Contains(namespaceKeysForHost(parsed), key) { - return true - } - _, known := h.namespaces[key] - return !known + return isDockerHubKey(key) || + slices.Contains(namespaceKeysForHost(parsed), key) || + slices.Contains(h.mirrorKeys[upstream], key) } // 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 69471706..4813ba2c 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -296,11 +296,26 @@ func TestContainerHandler_NamespacePrefixRouteAcceptsDockerHubAliases(t *testing }) } -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. +func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) { + // A _default wildcard mirror turns a pull of + // unconfigured.example/upstream/mirror/owner/app into exactly this + // request. Serving it would hand out owner/app from the mirror upstream + // instead of letting containerd fall back to unconfigured.example. + mirror := newNSTestRegistry(t, "owner/app", "") + routes, _, _ := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) + + for _, namespace := range []string{"unconfigured.example", "ghcr.io"} { + assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns="+namespace)) + } + if got := mirror.requestCount(); got != 0 { + t.Errorf("upstream requests = %d, want 0", got) + } +} + +func TestContainerHandler_NamespaceMirroredRegistries(t *testing.T) { + // The upstream mirrors ghcr.io, and upstream.oci_mirrors says so. Nodes + // keep pulling ghcr.io/..., either through a ghcr.io hosts.toml pointing + // at /v2/upstream/ghcr or through the _default wildcard mirror. tests := map[string]string{ "plain mirror host": "", "artifactory remote": "/artifactory/api/docker/ghcr-remote", @@ -308,19 +323,62 @@ func TestContainerHandler_NamespacePrefixRouteTrustsUnknownHosts(t *testing.T) { 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()) + routes, h, _ := newNSTestHandler(t, "", map[string]string{"ghcr": mirror.URL + pathPrefix}) + h.SetMirroredRegistries(map[string][]string{"ghcr": {"ghcr.io"}}) + + for _, target := range []string{ + "/v2/upstream/ghcr/owner/app/manifests/latest?ns=ghcr.io", + "/v2/upstream/ghcr/owner/app/manifests/latest?ns=GHCR.io:443", + "/v2/owner/app/manifests/latest?ns=ghcr.io", + } { + before := mirror.requestCount() + response := serveNS(routes, target) + if response.Code != http.StatusOK { + t.Fatalf("GET %s status = %d: %s", target, response.Code, response.Body.String()) + } + // The manifest is cached after the first request, so only + // the first one may reach the upstream. + if before == 0 { + if got, want := mirror.lastRequest(), pathPrefix+"/v2/owner/app/manifests/latest"; got != want { + t.Errorf("upstream request = %q, want %q", got, want) + } + } } - if got, want := mirror.lastRequest(), pathPrefix+"/v2/owner/app/manifests/latest"; got != want { - t.Errorf("upstream request = %q, want %q", got, want) + if got := mirror.requestCount(); got != 1 { + t.Errorf("upstream requests = %d, want 1 (all routes share the cache)", got) } + + assertNameUnknown(t, serveNS(routes, "/v2/upstream/ghcr/owner/app/manifests/latest?ns=quay.io")) }) } } +func TestContainerHandler_NamespaceMirrorHostsRankBelowConfiguredURLs(t *testing.T) { + proxy, _, _, _ := setupTestProxy(t) + logs := &bytes.Buffer{} + proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) + h := NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, "", map[string]string{ + "ghcr": "https://ghcr.io", + "art": "https://artifactory.example/api/docker/ghcr-remote", + }) + h.SetMirroredRegistries(map[string][]string{"art": {"ghcr.io"}, "missing": {"quay.io"}}) + + if got := h.namespaces["ghcr.io"]; got != "ghcr" { + t.Errorf("index[ghcr.io] = %q, want ghcr (configured URL wins over a mirror entry)", got) + } + if !h.namespaceNamesPrefixUpstream("ghcr.io", "upstream/art/owner/app") { + t.Error("ns ghcr.io refused on upstream/art/, want accepted as its mirrored registry") + } + if _, ok := h.namespaces["quay.io"]; ok { + t.Error("index has quay.io from a mirror entry without upstream") + } + for _, want := range []string{"ghcr.io=ghcr", "upstream=missing"} { + if !strings.Contains(logs.String(), want) { + t.Errorf("logs missing %q:\n%s", want, logs.String()) + } + } +} + func TestContainerHandler_NamespaceSharesCacheWithOtherRoutes(t *testing.T) { digest := "sha256:" + sha256Hex(nsTestBlob) manifestDigest := "sha256:" + sha256Hex(nsTestManifest()) diff --git a/internal/server/server.go b/internal/server/server.go index 24839091..e890cd8f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -429,6 +429,7 @@ func (s *Server) mountProtocolHandlers(r chi.Router, proxy *handler.Proxy) { s.cfg.Upstream.OCIDefault, s.cfg.Upstream.OCI, ) + containerHandler.SetMirroredRegistries(s.cfg.Upstream.OCIMirrors) handler.RegisterHomebrewArtifacts(containerHandler, s.cfg.Upstream.HomebrewArtifact) helmHandler := handler.NewHelmHandlerWithOCIRegistries( proxy, s.cfg.BaseURL, s.cfg.Upstream.Helm, s.cfg.Upstream.OCIDefault, s.cfg.Upstream.OCI) From 70ddc694bbfb59b042bd52ec9c7a5a447046a65a Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:49:14 +0200 Subject: [PATCH 12/22] Fix the docs sentence on which ns a prefix route takes configuration.md still said a prefix request with ns is only refused when ns names another route's host. Since the last commit it is the other way round: only the upstream's own host, its oci_mirrors entries and Docker Hub get through. --- docs/configuration.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 87e66ab9..01838b4b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -309,11 +309,11 @@ 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 routed by the -prefix. They are refused only when `ns` names a host that belongs to another -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. Any other host gets `404 NAME_UNKNOWN`: a containerd `_default` +prefix. They are accepted only when `ns` is the upstream's own host, a host +listed for it in `upstream.oci_mirrors`, or Docker Hub. 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. Any other host gets `404 NAME_UNKNOWN`: a containerd `_default` mirror sends the same request for an image such as `unconfigured.example/upstream/ghcr/owner/app`, and that pull must not be answered from the `ghcr` upstream. From fd25cd84065ed971a1eba7e3b79685cf7a6758ef Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:49:20 +0200 Subject: [PATCH 13/22] Note what a host accepted on a prefix route gives up Listing ghcr.io for an upstream (or the upstream's own host) still has the ambiguity from the wildcard case: ghcr.io/upstream/ghcr/owner/app lands on that upstream. That is the price of an explicit entry and fine in practice, but it should be written down. --- docs/configuration.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/configuration.md b/docs/configuration.md index 01838b4b..f6f43cf6 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -334,6 +334,12 @@ a port. Those hosts select the upstream for unprefixed `ns` requests and are accepted as `ns` on its `upstream/{name}/` prefix. A host that is also the host of a configured registry URL stays with that registry for unprefixed requests; the proxy logs a warning at startup. + +A host that is accepted on a prefix route gives up image paths of the form +`/upstream/{name}/...` under a `_default` mirror: with `ghcr: ["ghcr.io"]`, +a pull of `ghcr.io/upstream/ghcr/owner/app` is answered with `owner/app` from +the `ghcr` upstream, because containerd sends the same request for it as a +per-registry mirror does. The same applies to the upstream's own host. When the proxy uses plain HTTP (for example `localhost:8080`), pass `--plain-http` to Helm OCI commands. From b3af6f23ad59887266281cc8f3e8c64e712417e2 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:49:37 +0200 Subject: [PATCH 14/22] Test that oci_mirrors entries stay with their own upstream Two gaps in the coverage of the last change: nothing failed if a host listed for one upstream was also accepted on another upstream's prefix, and nothing checked that the server actually hands upstream.oci_mirrors to the container handler. The handler test now has a second upstream that must refuse ghcr.io, and a small server test pulls through a mirror entry on both the prefix and the unprefixed route. --- internal/handler/container_ns_test.go | 8 +++-- internal/server/oci_mirrors_test.go | 45 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 internal/server/oci_mirrors_test.go diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index 4813ba2c..fbf8fd50 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -358,8 +358,9 @@ func TestContainerHandler_NamespaceMirrorHostsRankBelowConfiguredURLs(t *testing logs := &bytes.Buffer{} proxy.Logger = slog.New(slog.NewTextHandler(logs, nil)) h := NewContainerHandlerWithRegistry(proxy, nsTestProxyURL, "", map[string]string{ - "ghcr": "https://ghcr.io", - "art": "https://artifactory.example/api/docker/ghcr-remote", + "ghcr": "https://ghcr.io", + "art": "https://artifactory.example/api/docker/ghcr-remote", + "other": "https://other.example", }) h.SetMirroredRegistries(map[string][]string{"art": {"ghcr.io"}, "missing": {"quay.io"}}) @@ -369,6 +370,9 @@ func TestContainerHandler_NamespaceMirrorHostsRankBelowConfiguredURLs(t *testing if !h.namespaceNamesPrefixUpstream("ghcr.io", "upstream/art/owner/app") { t.Error("ns ghcr.io refused on upstream/art/, want accepted as its mirrored registry") } + if h.namespaceNamesPrefixUpstream("ghcr.io", "upstream/other/owner/app") { + t.Error("ns ghcr.io accepted on upstream/other/, want refused: only art lists it") + } if _, ok := h.namespaces["quay.io"]; ok { t.Error("index has quay.io from a mirror entry without upstream") } diff --git a/internal/server/oci_mirrors_test.go b/internal/server/oci_mirrors_test.go new file mode 100644 index 00000000..5668dd49 --- /dev/null +++ b/internal/server/oci_mirrors_test.go @@ -0,0 +1,45 @@ +package server + +import ( + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/handler" + "github.com/go-chi/chi/v5" +) + +func TestOCIMirrorsReachContainerHandler(t *testing.T) { + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/art/v2/owner/app/manifests/latest" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + _, _ = io.WriteString(w, `{"schemaVersion":2}`) + })) + defer upstream.Close() + + cfg := config.Default() + cfg.BaseURL = "https://proxy.example" + cfg.Upstream.OCI = map[string]string{"art": upstream.URL + "/art"} + cfg.Upstream.OCIMirrors = map[string][]string{"art": {"ghcr.io"}} + p := &handler.Proxy{HTTPClient: upstream.Client(), Logger: slog.New(slog.NewTextHandler(io.Discard, nil))} + s := &Server{cfg: cfg} + r := chi.NewRouter() + s.mountProtocolHandlers(r, p) + + for _, target := range []string{ + "/v2/upstream/art/owner/app/manifests/latest?ns=ghcr.io", + "/v2/owner/app/manifests/latest?ns=ghcr.io", + } { + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, target, nil)) + if w.Code != http.StatusOK { + t.Errorf("GET %s status = %d, want 200 through upstream.oci_mirrors: %s", target, w.Code, w.Body.String()) + } + } +} From 68e93a528bc4f00b7e7ea3dedcc445bf4fad585c Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:49:47 +0200 Subject: [PATCH 15/22] Log when a prefix route refuses an ns Setups with a ghcr.io hosts.toml pointing at a mirror upstream worked before this branch and now need an oci_mirrors entry. Without one, containerd gets a 404, quietly pulls from ghcr.io directly, and nobody notices the proxy is being skipped. The refusal is now logged as a warning that names the upstream and the ns and points at upstream.oci_mirrors. --- internal/handler/container.go | 11 +++++++++-- internal/handler/container_ns_test.go | 8 +++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/internal/handler/container.go b/internal/handler/container.go index f8717ef2..0b9183e2 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -549,9 +549,16 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) return false } key := namespaceKeyForRequest(namespace) - return isDockerHubKey(key) || + if isDockerHubKey(key) || slices.Contains(namespaceKeysForHost(parsed), key) || - slices.Contains(h.mirrorKeys[upstream], key) + slices.Contains(h.mirrorKeys[upstream], key) { + return true + } + // containerd falls back to the registry itself on this 404 without + // reporting anything, so this line is all an operator gets to see. + h.warn("refusing ns on OCI upstream prefix; if the upstream mirrors this registry, list it in upstream.oci_mirrors", + "upstream", upstream, "ns", namespace) + return false } // 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 fbf8fd50..8b42419a 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -302,7 +302,7 @@ func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) // request. Serving it would hand out owner/app from the mirror upstream // instead of letting containerd fall back to unconfigured.example. mirror := newNSTestRegistry(t, "owner/app", "") - routes, _, _ := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) + routes, _, logs := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) for _, namespace := range []string{"unconfigured.example", "ghcr.io"} { assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns="+namespace)) @@ -310,6 +310,12 @@ func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) if got := mirror.requestCount(); got != 0 { t.Errorf("upstream requests = %d, want 0", got) } + // containerd swallows the 404, so the refusal has to show up in the log. + for _, want := range []string{"upstream.oci_mirrors", "upstream=mirror", "ns=ghcr.io"} { + if !strings.Contains(logs.String(), want) { + t.Errorf("logs missing %q:\n%s", want, logs.String()) + } + } } func TestContainerHandler_NamespaceMirroredRegistries(t *testing.T) { From 2aac55137a1fc3e82dc0f5fedcc62d2da93ce954 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:55:54 +0200 Subject: [PATCH 16/22] Drop a check in validateOCIMirrors that could never fire user@ghcr.io parses with ghcr.io as its host, so the parsed.Host != host comparison already rejects it and the extra parsed.User test was dead. A test case for a host with credentials keeps that path covered. --- internal/config/config.go | 2 +- internal/config/config_test.go | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 0583c696..0eb7614d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -727,7 +727,7 @@ func validateOCIMirrors(mirrors map[string][]string, upstreams map[string]string } for _, host := range hosts { parsed, err := url.Parse("https://" + host) - if host == "" || err != nil || parsed.Host != host || parsed.User != nil { + if host == "" || err != nil || parsed.Host != host { return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: must be a registry host such as ghcr.io or registry.example:5000", name, host) } } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3c08b20b..5f73d95e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1367,6 +1367,14 @@ func TestValidateNamedUpstreams(t *testing.T) { }, wantErr: true, }, + { + name: "OCI mirror host with credentials", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"ghcr": "https://mirror.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"ghcr": {"user@ghcr.io"}} + }, + wantErr: true, + }, { name: "OCI mirror host is empty", modify: func(cfg *Config) { From ea69214351688a8638c1a988631df1d8fff78c6b Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:56:08 +0200 Subject: [PATCH 17/22] Reject an oci_mirrors host that is listed for two upstreams With ghcr.io under both art and nexus, both prefix routes took ns=ghcr.io while unprefixed requests went to whichever name sorted first, with nothing but a startup warning. Config validation now refuses the second entry, comparing hosts without case and without the default :443. --- docs/configuration.md | 2 +- internal/config/config.go | 19 ++++++++++++++++--- internal/config/config_test.go | 8 ++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index f6f43cf6..20c4c1dd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -330,7 +330,7 @@ upstream: ``` Each entry lists bare registry hosts as containerd sends them, optionally with -a port. Those hosts select the upstream for unprefixed `ns` requests and are +a port. A host can be listed for one upstream only. Those hosts select the upstream for unprefixed `ns` requests and are accepted as `ns` on its `upstream/{name}/` prefix. A host that is also the host of a configured registry URL stays with that registry for unprefixed requests; the proxy logs a warning at startup. diff --git a/internal/config/config.go b/internal/config/config.go index 0eb7614d..7ac9ee90 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -719,17 +719,30 @@ var debianReservedRepositoryNames = []string{"pool", "dists"} // validateOCIMirrors checks that every upstream.oci_mirrors entry belongs to // an upstream.oci name and lists bare registry hosts, the form containerd -// sends in the ns query parameter. +// sends in the ns query parameter. A host may belong to one upstream only; +// otherwise both prefixes would take it while unprefixed requests silently +// went to one of them. func validateOCIMirrors(mirrors map[string][]string, upstreams map[string]string) error { - for name, hosts := range mirrors { + names := make([]string, 0, len(mirrors)) + for name := range mirrors { + names = append(names, name) + } + sort.Strings(names) + owners := make(map[string]string) + for _, name := range names { if _, ok := upstreams[name]; !ok { return fmt.Errorf("invalid upstream.oci_mirrors name %q: no upstream.oci entry of that name", name) } - for _, host := range hosts { + for _, host := range mirrors[name] { parsed, err := url.Parse("https://" + host) if host == "" || err != nil || parsed.Host != host { return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: must be a registry host such as ghcr.io or registry.example:5000", name, host) } + key := strings.TrimSuffix(strings.ToLower(host), ":443") + if owner, taken := owners[key]; taken && owner != name { + return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: already listed for %s", name, host, owner) + } + owners[key] = name } } return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 5f73d95e..4425c48a 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1375,6 +1375,14 @@ func TestValidateNamedUpstreams(t *testing.T) { }, wantErr: true, }, + { + name: "OCI mirror host listed for two upstreams", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"art": "https://art.example", "nexus": "https://nexus.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"art": {"ghcr.io"}, "nexus": {"GHCR.io:443"}} + }, + wantErr: true, + }, { name: "OCI mirror host is empty", modify: func(cfg *Config) { From e3591743f1068088cf41b7d1686698f0124e27f1 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 21:56:36 +0200 Subject: [PATCH 18/22] Log each refused ns once per upstream and refuse overlong values The refusal warning fired on every request, and a node with a missing oci_mirrors entry asks for the manifest and each layer, so the same line piled up. Each upstream/ns pair is now logged once, for up to 1024 pairs; after that every refusal is logged again. ns comes straight from the client and can be as long as the request line allows, so remembering it as is would let anyone pin a lot of memory with a handful of requests. No registry host is longer than 259 characters (a 253-character name plus a port), so anything longer is refused right away, before it is logged or remembered. --- internal/handler/container.go | 42 +++++++++++++++++++++++++-- internal/handler/container_ns_test.go | 18 +++++++++++- 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/internal/handler/container.go b/internal/handler/container.go index 0b9183e2..40f554c0 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -11,6 +11,7 @@ import ( "regexp" "slices" "strings" + "sync" ) const ( @@ -25,6 +26,13 @@ const ( namespaceQueryParam = "ns" // defaultNamespaceRoute marks namespace hosts served by the default registry. defaultNamespaceRoute = "" + // maxReportedNamespaceRefusals bounds the upstream/ns pairs remembered + // for logging refusals once; past it every refusal is logged again. + maxReportedNamespaceRefusals = 1024 + // maxNamespaceLength is the longest ns a registry host can produce: a + // 253-character DNS name plus ":65535". Longer values are refused before + // they are logged or remembered. + maxNamespaceLength = 259 ) // dockerHubNamespaces are the registry URLs clients use for Docker Hub. @@ -64,6 +72,10 @@ type ContainerHandler struct { // mirrorKeys holds, per named upstream, the ns lookup keys of the // registries it is configured to mirror. mirrorKeys map[string][]string + + refusalsMu sync.Mutex + // refusals remembers upstream/ns pairs whose refusal was already logged. + refusals map[string]struct{} } type containerRegistry struct { @@ -507,6 +519,9 @@ func (h *ContainerHandler) registryForRequest(r *http.Request, name string) (reg // 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 len(namespace) > maxNamespaceLength { + return "", "", "", false + } if strings.HasPrefix(name, "upstream/") { if !h.namespaceNamesPrefixUpstream(namespace, name) { return "", "", "", false @@ -554,11 +569,32 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) slices.Contains(h.mirrorKeys[upstream], key) { return true } - // containerd falls back to the registry itself on this 404 without - // reporting anything, so this line is all an operator gets to see. + h.reportNamespaceRefusal(upstream, key, namespace) + return false +} + +// reportNamespaceRefusal logs a refused ns on a prefix route. containerd +// falls back to the registry itself on that 404 without reporting anything, +// so this line is all an operator gets to see. Each upstream/ns pair is +// logged once, since a misconfigured node repeats the same request for every +// layer; ns comes from the client, so the set is bounded and overlong values +// never get here (see maxNamespaceLength). +func (h *ContainerHandler) reportNamespaceRefusal(upstream, key, namespace string) { + pair := upstream + "\x00" + key + h.refusalsMu.Lock() + _, seen := h.refusals[pair] + if !seen && len(h.refusals) < maxReportedNamespaceRefusals { + if h.refusals == nil { + h.refusals = make(map[string]struct{}) + } + h.refusals[pair] = struct{}{} + } + h.refusalsMu.Unlock() + if seen { + return + } h.warn("refusing ns on OCI upstream prefix; if the upstream mirrors this registry, list it in upstream.oci_mirrors", "upstream", upstream, "ns", namespace) - return false } // 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 8b42419a..b6fdebc4 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -302,7 +302,7 @@ func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) // request. Serving it would hand out owner/app from the mirror upstream // instead of letting containerd fall back to unconfigured.example. mirror := newNSTestRegistry(t, "owner/app", "") - routes, _, logs := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) + routes, h, logs := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) for _, namespace := range []string{"unconfigured.example", "ghcr.io"} { assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns="+namespace)) @@ -316,6 +316,22 @@ func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) t.Errorf("logs missing %q:\n%s", want, logs.String()) } } + + // A node repeats the refused request for every pull; one line per + // upstream/ns pair is enough. An ns longer than any registry host is + // refused before it is logged or remembered. + assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns=GHCR.io")) + long := strings.Repeat("a", 300) + ".example" + assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns="+long)) + if got := strings.Count(logs.String(), "upstream.oci_mirrors"); got != 2 { + t.Errorf("refusal warnings = %d, want 2 (one per upstream/ns pair):\n%s", got, logs.String()) + } + if strings.Contains(logs.String(), "aaaa") { + t.Errorf("logs mention the overlong ns, want it refused silently:\n%s", logs.String()) + } + if got := len(h.refusals); got != 2 { + t.Errorf("remembered refusals = %d, want 2", got) + } } func TestContainerHandler_NamespaceMirroredRegistries(t *testing.T) { From 7df9738571e0405ca3b69c95954478221de701de Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 22:02:53 +0200 Subject: [PATCH 19/22] Compare oci_mirrors hosts the way the handler does The duplicate check lowercased the host and trimmed :443, while the handler splits host and port, brackets IPv6 literals and drops an empty port. So ghcr.io and ghcr.io:, or [::1] and [::1]:, passed as different hosts in two upstreams and then collided in the handler. The check now builds the same key the handler uses. --- internal/config/config.go | 22 +++++++++++++++++++++- internal/config/config_test.go | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 7ac9ee90..7b6c8bd3 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -63,6 +63,7 @@ import ( "encoding/base64" "encoding/json" "fmt" + "net" "net/url" "os" "path/filepath" @@ -738,7 +739,7 @@ func validateOCIMirrors(mirrors map[string][]string, upstreams map[string]string if host == "" || err != nil || parsed.Host != host { return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: must be a registry host such as ghcr.io or registry.example:5000", name, host) } - key := strings.TrimSuffix(strings.ToLower(host), ":443") + key := ociMirrorHostKey(host) if owner, taken := owners[key]; taken && owner != name { return fmt.Errorf("invalid upstream.oci_mirrors.%s host %q: already listed for %s", name, host, owner) } @@ -748,6 +749,25 @@ func validateOCIMirrors(mirrors map[string][]string, upstreams map[string]string return nil } +// ociMirrorHostKey normalizes an upstream.oci_mirrors host the way the +// container handler builds its ns lookup keys: lowercase, IPv6 literals in +// brackets, an empty or default https port dropped. Two entries with the same +// key would select the same route. +func ociMirrorHostKey(host string) string { + name, port, err := net.SplitHostPort(host) + if err != nil { + name, port = strings.TrimSuffix(strings.TrimPrefix(host, "["), "]"), "" + } + name = strings.ToLower(name) + if strings.Contains(name, ":") { + name = "[" + name + "]" + } + if port == "" || port == "443" { + return name + } + return name + ":" + port +} + func validateNamedUpstreams(field string, upstreams map[string]string) error { for name, upstreamURL := range upstreams { if name == "" || name == "." || name == ".." || strings.ContainsAny(name, `/\\`) { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4425c48a..3b7136c0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -1383,6 +1383,29 @@ func TestValidateNamedUpstreams(t *testing.T) { }, wantErr: true, }, + { + name: "OCI mirror host listed for two upstreams with an empty port", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"art": "https://art.example", "nexus": "https://nexus.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"art": {"ghcr.io"}, "nexus": {"ghcr.io:"}} + }, + wantErr: true, + }, + { + name: "OCI mirror IPv6 host listed for two upstreams", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"art": "https://art.example", "nexus": "https://nexus.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"art": {"[::1]"}, "nexus": {"[::1]:"}} + }, + wantErr: true, + }, + { + name: "OCI mirror hosts that differ only in a non-default port", + modify: func(cfg *Config) { + cfg.Upstream.OCI = map[string]string{"art": "https://art.example", "nexus": "https://nexus.example"} + cfg.Upstream.OCIMirrors = map[string][]string{"art": {"registry.example"}, "nexus": {"registry.example:5000"}} + }, + }, { name: "OCI mirror host is empty", modify: func(cfg *Config) { From 30660c9477a72130c5f1a6280b4d273f32b9dc42 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 22:03:00 +0200 Subject: [PATCH 20/22] Rewrap the oci_mirrors paragraphs in the configuration docs Two sentences had been dropped into the middle of wrapped lines, and the prefix-route paragraph named Docker Hub twice in a row. Same content, shorter lines, one mention. --- docs/configuration.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 20c4c1dd..b8a570f2 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -310,10 +310,10 @@ 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 accepted only when `ns` is the upstream's own host, a host -listed for it in `upstream.oci_mirrors`, or Docker Hub. 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. Any other host gets `404 NAME_UNKNOWN`: a containerd `_default` +listed for it in `upstream.oci_mirrors`, or Docker Hub (`docker.io` and its +aliases). Docker Hub repository names have two path components and can never +start with `upstream/`, so a Docker Hub mirror behind any prefix stays +reachable. Any other host gets `404 NAME_UNKNOWN`: a containerd `_default` mirror sends the same request for an image such as `unconfigured.example/upstream/ghcr/owner/app`, and that pull must not be answered from the `ghcr` upstream. @@ -330,10 +330,11 @@ upstream: ``` Each entry lists bare registry hosts as containerd sends them, optionally with -a port. A host can be listed for one upstream only. Those hosts select the upstream for unprefixed `ns` requests and are -accepted as `ns` on its `upstream/{name}/` prefix. A host that is also the host -of a configured registry URL stays with that registry for unprefixed requests; -the proxy logs a warning at startup. +a port. A host can be listed for one upstream only. Those hosts select the +upstream for unprefixed `ns` requests and are accepted as `ns` on its +`upstream/{name}/` prefix. A host that is also the host of a configured +registry URL stays with that registry for unprefixed requests; the proxy logs a +warning at startup. A host that is accepted on a prefix route gives up image paths of the form `/upstream/{name}/...` under a `_default` mirror: with `ghcr: ["ghcr.io"]`, From b1708e015c74a66e0dad5f73f25d3a03758d27fc Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 22:03:14 +0200 Subject: [PATCH 21/22] Stop logging ns refusals once the set is full Past 1024 different upstream/ns pairs every refusal was logged again, so anyone could fill the set with made-up hosts and then flood the log at warning level. When the set is full the proxy now says so once and keeps quiet after that. A real misconfiguration shows up long before that, with the first pull. --- internal/handler/container.go | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/internal/handler/container.go b/internal/handler/container.go index 40f554c0..546e55f6 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -27,7 +27,7 @@ const ( // defaultNamespaceRoute marks namespace hosts served by the default registry. defaultNamespaceRoute = "" // maxReportedNamespaceRefusals bounds the upstream/ns pairs remembered - // for logging refusals once; past it every refusal is logged again. + // for logging refusals once; past it further refusals are not logged. maxReportedNamespaceRefusals = 1024 // maxNamespaceLength is the longest ns a registry host can produce: a // 253-character DNS name plus ":65535". Longer values are refused before @@ -76,6 +76,9 @@ type ContainerHandler struct { refusalsMu sync.Mutex // refusals remembers upstream/ns pairs whose refusal was already logged. refusals map[string]struct{} + // refusalsFull is set once the refusals set reached its limit and that + // was logged. + refusalsFull bool } type containerRegistry struct { @@ -577,22 +580,31 @@ func (h *ContainerHandler) namespaceNamesPrefixUpstream(namespace, name string) // falls back to the registry itself on that 404 without reporting anything, // so this line is all an operator gets to see. Each upstream/ns pair is // logged once, since a misconfigured node repeats the same request for every -// layer; ns comes from the client, so the set is bounded and overlong values +// layer. ns comes from the client, so the set is bounded: once it is full, +// that is logged one time and later refusals stay quiet. Overlong values // never get here (see maxNamespaceLength). func (h *ContainerHandler) reportNamespaceRefusal(upstream, key, namespace string) { pair := upstream + "\x00" + key h.refusalsMu.Lock() - _, seen := h.refusals[pair] - if !seen && len(h.refusals) < maxReportedNamespaceRefusals { - if h.refusals == nil { - h.refusals = make(map[string]struct{}) - } - h.refusals[pair] = struct{}{} + if _, seen := h.refusals[pair]; seen { + h.refusalsMu.Unlock() + return } - h.refusalsMu.Unlock() - if seen { + if len(h.refusals) >= maxReportedNamespaceRefusals { + announce := !h.refusalsFull + h.refusalsFull = true + h.refusalsMu.Unlock() + if announce { + h.warn("too many different ns values refused on OCI upstream prefixes; further refusals are not logged", + "limit", maxReportedNamespaceRefusals) + } return } + if h.refusals == nil { + h.refusals = make(map[string]struct{}) + } + h.refusals[pair] = struct{}{} + h.refusalsMu.Unlock() h.warn("refusing ns on OCI upstream prefix; if the upstream mirrors this registry, list it in upstream.oci_mirrors", "upstream", upstream, "ns", namespace) } From bc37ce069247f1aa45dc33ea6a6306843abc2daa Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Fri, 9 Oct 2026 22:03:24 +0200 Subject: [PATCH 22/22] Test the limit on remembered ns refusals Nothing covered what happens once 1024 pairs are remembered. The new test fills the set, then sends a known pair and two new ones: the proxy must log the limit notice exactly once, log none of the refusals and keep the set at its size. --- internal/handler/container_ns_test.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/handler/container_ns_test.go b/internal/handler/container_ns_test.go index b6fdebc4..e4a1e121 100644 --- a/internal/handler/container_ns_test.go +++ b/internal/handler/container_ns_test.go @@ -334,6 +334,28 @@ func TestContainerHandler_NamespacePrefixRouteRejectsUnknownHosts(t *testing.T) } } +func TestContainerHandler_NamespaceRefusalLogIsBounded(t *testing.T) { + mirror := newNSTestRegistry(t, "owner/app", "") + routes, h, logs := newNSTestHandler(t, "", map[string]string{"mirror": mirror.URL}) + h.refusals = make(map[string]struct{}, maxReportedNamespaceRefusals) + for i := range maxReportedNamespaceRefusals { + h.refusals[fmt.Sprintf("mirror\x00filler-%d.example", i)] = struct{}{} + } + + for _, namespace := range []string{"filler-0.example", "first.example", "second.example"} { + assertNameUnknown(t, serveNS(routes, "/v2/upstream/mirror/owner/app/manifests/latest?ns="+namespace)) + } + if got := strings.Count(logs.String(), "further refusals are not logged"); got != 1 { + t.Errorf("limit notices = %d, want 1:\n%s", got, logs.String()) + } + if strings.Contains(logs.String(), "refusing ns") { + t.Errorf("refusal logged past the limit:\n%s", logs.String()) + } + if got := len(h.refusals); got != maxReportedNamespaceRefusals { + t.Errorf("remembered refusals = %d, want %d", got, maxReportedNamespaceRefusals) + } +} + func TestContainerHandler_NamespaceMirroredRegistries(t *testing.T) { // The upstream mirrors ghcr.io, and upstream.oci_mirrors says so. Nodes // keep pulling ghcr.io/..., either through a ghcr.io hosts.toml pointing