Watch
1
0
Fork
You've already forked pkg-proxy
1
mirror of https://github.com/git-pkgs/proxy.git synced 2026-11-09 15:49:16 -05:00
pkg-proxy/internal/server/oci_mirrors_test.go

45 lines
1.4 KiB
Go
Raw Permalink Normal View History

Route containerd ns mirror requests to configured registries (#405) * Route containerd ns mirror requests to configured registries containerd's hosts.toml mirrors append ?ns=<registry-host> 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 * Document containerd ns mirroring Refs #303 * 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. * 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. * 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. * 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. * 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=<registry>, 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. * 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. * 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. * 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}/. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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.
2026-10-10 08:35:50 +02:00
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())
}
}
}