Skip to content
This repository was archived by the owner on Jul 4, 2026. It is now read-only.

Commit f3ceb9f

Browse files
committed
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.
1 parent 67439ee commit f3ceb9f

3 files changed

Lines changed: 10 additions & 27 deletions

File tree

server/dl_routes_test.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@ import (
88
"github.com/gorilla/mux"
99
)
1010

11-
// TestDLRouteResolution exercises the gorilla/mux route table to confirm
12-
// that /dl/ URLs split into (name, ref) the way handleArtifactDownload
13-
// expects. The handler-side fallback for the legacy multi-segment-name
14-
// form is integration-tested against a live registry; this test only
15-
// validates the URL parsing, which is the part with routing ambiguity.
11+
// TestDLRouteResolution verifies /dl/ URLs split into (name, ref) the way
12+
// handleArtifactDownload expects. The handler-side legacy fallback is
13+
// integration-tested separately.
1614
func TestDLRouteResolution(t *testing.T) {
1715
tests := []struct {
1816
path string
@@ -64,8 +62,7 @@ func resolveDLRoute(t *testing.T, path string) (name, ref string, matched bool)
6462
return name, ref, matched
6563
}
6664

67-
// TestDLPublicPathPostMigration ensures the new /dl/{name}/{ref} URLs
68-
// still bypass auth.
65+
// TestDLPublicPathPostMigration ensures /dl/{name}/{ref} bypasses auth.
6966
func TestDLPublicPathPostMigration(t *testing.T) {
7067
paths := []string{
7168
"/dl/win11/latest",

server/download.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,9 @@ func (d *registryDownloader) GetBlob(ctx context.Context, _, digest string) (io.
4545
}
4646

4747
// handleArtifactDownload streams a cloud image or snapshot. Auth-exempt.
48-
//
49-
// Canonical URL: /dl/{name}/{ref} — ref is a tag ("latest", "22h2-20260510")
50-
// or a digest reference ("sha256:..."). The route also matches /dl/{name}
51-
// (no ref); in that case ref defaults to "latest". Both routes funnel here.
52-
//
53-
// Backward-compat fallback: if the (name, ref) lookup 404s, retry as
54-
// (name+"/"+ref, "latest"). That covers the pre-2026-05 2-segment form
55-
// `/dl/simular/win11`, which the new route would split as name=simular,
56-
// ref=win11 — the fallback re-joins to name=simular/win11 + implicit latest.
48+
// On 404 retries (name+"/"+ref, "latest") to keep the pre-2026-05
49+
// /dl/{name-with-slash} form working under the new /dl/{name}/{ref} route,
50+
// flagging the response with a Deprecation header so callers migrate.
5751
func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request) {
5852
name := urlVar(r, "name")
5953
ref := urlVar(r, "ref")
@@ -73,11 +67,9 @@ func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request)
7367
return
7468
}
7569
if useName != name || useRef != ref {
76-
// Surface that the legacy form resolved via fallback so callers can
77-
// migrate. The Deprecation header follows the IETF draft convention.
7870
w.Header().Set("Deprecation", "true")
7971
w.Header().Set("Link", `</dl/`+useName+`/`+useRef+`>; rel="successor-version"`)
80-
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)
72+
logger.Warnf(r.Context(), "legacy /dl/ form %s:%s resolved via fallback to %s:%s", name, ref, useName, useRef)
8173
}
8274

8375
m, err := manifest.Parse(raw)
@@ -99,9 +91,8 @@ func (s *Server) handleArtifactDownload(w http.ResponseWriter, r *http.Request)
9991
}
10092
}
10193

102-
// fetchManifestWithLegacyFallback tries (name, ref) first; on 404 retries
103-
// (name+"/"+ref, "latest"). Returns the (name, ref) pair that resolved so
104-
// the caller can flag deprecation. Non-404 errors short-circuit immediately.
94+
// fetchManifestWithLegacyFallback returns the resolved (name, ref) so the
95+
// caller can detect when the fallback path fired.
10596
func (s *Server) fetchManifestWithLegacyFallback(r *http.Request, name, ref string) ([]byte, string, string, error) {
10697
raw, err := s.loadManifestRaw(r, name, ref)
10798
if err == nil {

server/server.go

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,6 @@ func (s *Server) setupRoutes(ctx context.Context) {
142142
api.HandleFunc("/repositories/{name:.+}/tags", s.apiListTags).Methods(http.MethodGet)
143143
api.HandleFunc("/repositories/{name:.+}", s.apiGetRepository).Methods(http.MethodGet)
144144

145-
// /dl/{name}/{ref} is the canonical form — ref is a tag (e.g. "latest",
146-
// "22h2-20260510") or a digest reference ("sha256:..."). The single-arg
147-
// /dl/{name} route is kept as a backward-compat shim that handler-side
148-
// retries name+"/"+ref as the repo with implicit latest, covering the
149-
// pre-2026-05 form `/dl/simular/win11` (2 segments, name=simular/win11).
150145
s.router.HandleFunc("/dl/{name:.+}/{ref}", s.handleArtifactDownload).Methods(http.MethodGet)
151146
s.router.HandleFunc("/dl/{name:.+}", s.handleArtifactDownload).Methods(http.MethodGet)
152147

0 commit comments

Comments
 (0)