From 67439ee4b9dad95d80746070fe5f180891bc0200 Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 11 May 2026 11:28:45 +0800 Subject: [PATCH 1/2] feat(dl): tag/digest-aware download endpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Promote /dl/{name}/{ref} as the canonical form. ref is a tag (e.g. "latest", "22h2-20260510") or a digest reference ("sha256:..."), routed via the existing isDigestRef check that the /v2/ manifest endpoint already uses. Backward compat: the legacy /dl/{name} form (latest hardcoded) used to also accept multi-segment URLs like /dl/simular/win11 — under the new two-segment route, those split as name=simular, ref=win11. The handler catches the resulting 404 and retries name+"/"+ref with implicit "latest", then flags the response with a Deprecation header and a successor-version Link so callers know the URL form needs to be updated. Motivation: hot snapshots pin their base image by tag (e.g. simular/win10:22h2-20260510), but the previous /dl/ endpoint hardcoded ManifestJSON(name, "latest"). That made tagged base images unreachable through the cloudimg HTTP-stream channel that cocoon's cloudimg backend relies on, which surfaced as the clone-time "Backing file I/O error" reported in cocoonstack/cocoon#37 #38. The OCI registry on /v2/ already supports tags and digests fine. This patch closes the gap on the cloudimg-stream side without teaching cocoon's HTTP-only cloudimg backend to speak OCI. --- server/dl_routes_test.go | 80 ++++++++++++++++++++++++++++++++++++++++ server/download.go | 51 ++++++++++++++++++++++--- server/server.go | 6 +++ 3 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 server/dl_routes_test.go diff --git a/server/dl_routes_test.go b/server/dl_routes_test.go new file mode 100644 index 0000000..7520c9c --- /dev/null +++ b/server/dl_routes_test.go @@ -0,0 +1,80 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gorilla/mux" +) + +// TestDLRouteResolution exercises the gorilla/mux route table to confirm +// that /dl/ URLs split into (name, ref) the way handleArtifactDownload +// expects. The handler-side fallback for the legacy multi-segment-name +// form is integration-tested against a live registry; this test only +// validates the URL parsing, which is the part with routing ambiguity. +func TestDLRouteResolution(t *testing.T) { + tests := []struct { + path string + wantName string + wantRef string + wantMatched bool + }{ + {"/dl/win11", "win11", "", true}, + {"/dl/win11/latest", "win11", "latest", true}, + {"/dl/win11/sha256:abc", "win11", "sha256:abc", true}, + {"/dl/simular/win11", "simular", "win11", true}, + {"/dl/simular/win11/22h2-20260510", "simular/win11", "22h2-20260510", true}, + {"/dl/simular/win11/sha256:adafd938", "simular/win11", "sha256:adafd938", true}, + {"/dl/", "", "", false}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + gotName, gotRef, matched := resolveDLRoute(t, tt.path) + if matched != tt.wantMatched { + t.Fatalf("matched = %v, want %v", matched, tt.wantMatched) + } + if !matched { + return + } + if gotName != tt.wantName { + t.Errorf("name = %q, want %q", gotName, tt.wantName) + } + if gotRef != tt.wantRef { + t.Errorf("ref = %q, want %q", gotRef, tt.wantRef) + } + }) + } +} + +func resolveDLRoute(t *testing.T, path string) (name, ref string, matched bool) { + t.Helper() + r := mux.NewRouter() + capture := func(w http.ResponseWriter, req *http.Request) { + name = mux.Vars(req)["name"] + ref = mux.Vars(req)["ref"] + matched = true + } + r.HandleFunc("/dl/{name:.+}/{ref}", capture).Methods(http.MethodGet) + r.HandleFunc("/dl/{name:.+}", capture).Methods(http.MethodGet) + + req := httptest.NewRequest(http.MethodGet, path, nil) + r.ServeHTTP(httptest.NewRecorder(), req) + return name, ref, matched +} + +// TestDLPublicPathPostMigration ensures the new /dl/{name}/{ref} URLs +// still bypass auth. +func TestDLPublicPathPostMigration(t *testing.T) { + paths := []string{ + "/dl/win11/latest", + "/dl/simular/win11/22h2-20260510", + "/dl/simular/win11/sha256:adafd938", + } + for _, p := range paths { + if !isPublicPath(p) { + t.Errorf("isPublicPath(%q) = false, want true", p) + } + } +} diff --git a/server/download.go b/server/download.go index 2645d74..d76845c 100644 --- a/server/download.go +++ b/server/download.go @@ -44,34 +44,54 @@ func (d *registryDownloader) GetBlob(ctx context.Context, _, digest string) (io. return body, err } -// handleArtifactDownload streams a cloud image or snapshot by name. Auth-exempt. +// handleArtifactDownload streams a cloud image or snapshot. Auth-exempt. +// +// Canonical URL: /dl/{name}/{ref} — ref is a tag ("latest", "22h2-20260510") +// or a digest reference ("sha256:..."). The route also matches /dl/{name} +// (no ref); in that case ref defaults to "latest". Both routes funnel here. +// +// Backward-compat fallback: if the (name, ref) lookup 404s, retry as +// (name+"/"+ref, "latest"). That covers the pre-2026-05 2-segment form +// `/dl/simular/win11`, which the new route would split as name=simular, +// ref=win11 — the fallback re-joins to name=simular/win11 + implicit latest. func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) { name := urlVar(r, "name") + ref := urlVar(r, "ref") + if ref == "" { + ref = "latest" + } logger := log.WithFunc("server.handleArtifactDownload") - raw, err := s.reg.ManifestJSON(r.Context(), name, "latest") + raw, useName, useRef, err := s.fetchManifestWithLegacyFallback(r, name, ref) if err != nil { if isNotFound(err) { http.Error(w, "artifact not found", http.StatusNotFound) return } - logger.Errorf(r.Context(), err, "fetch manifest %s", name) + logger.Errorf(r.Context(), err, "fetch manifest %s:%s", name, ref) http.Error(w, "internal server error", http.StatusInternalServerError) return } + if useName != name || useRef != ref { + // Surface that the legacy form resolved via fallback so callers can + // migrate. The Deprecation header follows the IETF draft convention. + w.Header().Set("Deprecation", "true") + w.Header().Set("Link", `; rel="successor-version"`) + logger.Warnf(r.Context(), "legacy /dl/ form %s:%s resolved via fallback to %s:%s — caller should migrate to /dl/%s/%s", name, ref, useName, useRef, useName, useRef) + } m, err := manifest.Parse(raw) if err != nil { - logger.Errorf(r.Context(), err, "parse manifest %s", name) + logger.Errorf(r.Context(), err, "parse manifest %s:%s", useName, useRef) http.Error(w, "internal server error", http.StatusInternalServerError) return } switch manifest.ClassifyParsed(m) { case manifest.KindCloudImage: - s.streamCloudImage(w, r, name, m) + s.streamCloudImage(w, r, useName, m) case manifest.KindSnapshot: - s.streamSnapshot(w, r, name, raw, m) + s.streamSnapshot(w, r, useName, raw, m) case manifest.KindContainerImage: http.Error(w, "container image — pull via OCI client (oras / crane / docker)", http.StatusMethodNotAllowed) default: @@ -79,6 +99,25 @@ func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) } } +// fetchManifestWithLegacyFallback tries (name, ref) first; on 404 retries +// (name+"/"+ref, "latest"). Returns the (name, ref) pair that resolved so +// the caller can flag deprecation. Non-404 errors short-circuit immediately. +func (s *Server) fetchManifestWithLegacyFallback(r *http.Request, name, ref string) ([]byte, string, string, error) { + raw, err := s.loadManifestRaw(r, name, ref) + if err == nil { + return raw, name, ref, nil + } + if !isNotFound(err) { + return nil, name, ref, err + } + legacyName := name + "/" + ref + legacy, lerr := s.loadManifestRaw(r, legacyName, "latest") + if lerr != nil { + return nil, name, ref, err + } + return legacy, legacyName, "latest", nil +} + type blobStreamer interface { StreamBlob(ctx context.Context, digest string) (io.ReadCloser, int64, error) } diff --git a/server/server.go b/server/server.go index 091666c..b206036 100644 --- a/server/server.go +++ b/server/server.go @@ -142,6 +142,12 @@ func (s *Server) setupRoutes(ctx context.Context) { api.HandleFunc("/repositories/{name:.+}/tags", s.apiListTags).Methods(http.MethodGet) api.HandleFunc("/repositories/{name:.+}", s.apiGetRepository).Methods(http.MethodGet) + // /dl/{name}/{ref} is the canonical form — ref is a tag (e.g. "latest", + // "22h2-20260510") or a digest reference ("sha256:..."). The single-arg + // /dl/{name} route is kept as a backward-compat shim that handler-side + // retries name+"/"+ref as the repo with implicit latest, covering the + // pre-2026-05 form `/dl/simular/win11` (2 segments, name=simular/win11). + s.router.HandleFunc("/dl/{name:.+}/{ref}", s.handleArtifactDownload).Methods(http.MethodGet) s.router.HandleFunc("/dl/{name:.+}", s.handleArtifactDownload).Methods(http.MethodGet) s.router.HandleFunc("/healthz", func(w http.ResponseWriter, _ *http.Request) { From f3ceb9f544beabeb2e9c0d1327da2bec03d3141e Mon Sep 17 00:00:00 2001 From: CMGS Date: Mon, 11 May 2026 11:34:52 +0800 Subject: [PATCH 2/2] style(dl): tighten download endpoint comments Drop WHAT-style enumerations and inline comments that restated the code. The handler godoc keeps the only non-obvious bit (the legacy fallback WHY); the route registration in server.go and the field-by-field comment on the Deprecation header lose redundant prose. Helper godoc shrinks to one line about the resolved-pair return convention. --- server/dl_routes_test.go | 11 ++++------- server/download.go | 21 ++++++--------------- server/server.go | 5 ----- 3 files changed, 10 insertions(+), 27 deletions(-) diff --git a/server/dl_routes_test.go b/server/dl_routes_test.go index 7520c9c..bdd3f78 100644 --- a/server/dl_routes_test.go +++ b/server/dl_routes_test.go @@ -8,11 +8,9 @@ import ( "github.com/gorilla/mux" ) -// TestDLRouteResolution exercises the gorilla/mux route table to confirm -// that /dl/ URLs split into (name, ref) the way handleArtifactDownload -// expects. The handler-side fallback for the legacy multi-segment-name -// form is integration-tested against a live registry; this test only -// validates the URL parsing, which is the part with routing ambiguity. +// TestDLRouteResolution verifies /dl/ URLs split into (name, ref) the way +// handleArtifactDownload expects. The handler-side legacy fallback is +// integration-tested separately. func TestDLRouteResolution(t *testing.T) { tests := []struct { path string @@ -64,8 +62,7 @@ func resolveDLRoute(t *testing.T, path string) (name, ref string, matched bool) return name, ref, matched } -// TestDLPublicPathPostMigration ensures the new /dl/{name}/{ref} URLs -// still bypass auth. +// TestDLPublicPathPostMigration ensures /dl/{name}/{ref} bypasses auth. func TestDLPublicPathPostMigration(t *testing.T) { paths := []string{ "/dl/win11/latest", diff --git a/server/download.go b/server/download.go index d76845c..7ae0f3a 100644 --- a/server/download.go +++ b/server/download.go @@ -45,15 +45,9 @@ func (d *registryDownloader) GetBlob(ctx context.Context, _, digest string) (io. } // handleArtifactDownload streams a cloud image or snapshot. Auth-exempt. -// -// Canonical URL: /dl/{name}/{ref} — ref is a tag ("latest", "22h2-20260510") -// or a digest reference ("sha256:..."). The route also matches /dl/{name} -// (no ref); in that case ref defaults to "latest". Both routes funnel here. -// -// Backward-compat fallback: if the (name, ref) lookup 404s, retry as -// (name+"/"+ref, "latest"). That covers the pre-2026-05 2-segment form -// `/dl/simular/win11`, which the new route would split as name=simular, -// ref=win11 — the fallback re-joins to name=simular/win11 + implicit latest. +// On 404 retries (name+"/"+ref, "latest") to keep the pre-2026-05 +// /dl/{name-with-slash} form working under the new /dl/{name}/{ref} route, +// flagging the response with a Deprecation header so callers migrate. func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) { name := urlVar(r, "name") ref := urlVar(r, "ref") @@ -73,11 +67,9 @@ func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) return } if useName != name || useRef != ref { - // Surface that the legacy form resolved via fallback so callers can - // migrate. The Deprecation header follows the IETF draft convention. w.Header().Set("Deprecation", "true") w.Header().Set("Link", `; rel="successor-version"`) - logger.Warnf(r.Context(), "legacy /dl/ form %s:%s resolved via fallback to %s:%s — caller should migrate to /dl/%s/%s", name, ref, useName, useRef, useName, useRef) + logger.Warnf(r.Context(), "legacy /dl/ form %s:%s resolved via fallback to %s:%s", name, ref, useName, useRef) } m, err := manifest.Parse(raw) @@ -99,9 +91,8 @@ func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) } } -// fetchManifestWithLegacyFallback tries (name, ref) first; on 404 retries -// (name+"/"+ref, "latest"). Returns the (name, ref) pair that resolved so -// the caller can flag deprecation. Non-404 errors short-circuit immediately. +// fetchManifestWithLegacyFallback returns the resolved (name, ref) so the +// caller can detect when the fallback path fired. func (s *Server) fetchManifestWithLegacyFallback(r *http.Request, name, ref string) ([]byte, string, string, error) { raw, err := s.loadManifestRaw(r, name, ref) if err == nil { diff --git a/server/server.go b/server/server.go index b206036..c734e63 100644 --- a/server/server.go +++ b/server/server.go @@ -142,11 +142,6 @@ func (s *Server) setupRoutes(ctx context.Context) { api.HandleFunc("/repositories/{name:.+}/tags", s.apiListTags).Methods(http.MethodGet) api.HandleFunc("/repositories/{name:.+}", s.apiGetRepository).Methods(http.MethodGet) - // /dl/{name}/{ref} is the canonical form — ref is a tag (e.g. "latest", - // "22h2-20260510") or a digest reference ("sha256:..."). The single-arg - // /dl/{name} route is kept as a backward-compat shim that handler-side - // retries name+"/"+ref as the repo with implicit latest, covering the - // pre-2026-05 form `/dl/simular/win11` (2 segments, name=simular/win11). s.router.HandleFunc("/dl/{name:.+}/{ref}", s.handleArtifactDownload).Methods(http.MethodGet) s.router.HandleFunc("/dl/{name:.+}", s.handleArtifactDownload).Methods(http.MethodGet)