From 52f1e054d39b65f48f9bd06d19f59ab98af75db7 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 22 Aug 2026 12:25:13 +0530 Subject: [PATCH 1/5] fix(oci): cache tag lists and normalize manifest variants --- docs/architecture.md | 2 +- docs/configuration.md | 2 +- internal/handler/container.go | 25 +--- internal/handler/container_manifest.go | 80 +++++++++- internal/handler/container_tags.go | 196 +++++++++++++++++++++++++ internal/handler/container_test.go | 137 +++++++++++++++++ 6 files changed, 415 insertions(+), 27 deletions(-) create mode 100644 internal/handler/container_tags.go diff --git a/docs/architecture.md b/docs/architecture.md index 6d9bfda..16d8578 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -353,7 +353,7 @@ Eviction can be implemented as: - Fresh data - new versions visible immediately - Metadata is small, upstream fetch is fast - Set `cache_metadata: true` or use the mirror command to enable metadata caching for offline use via the `metadata_cache` table -- OCI manifests are the exception: they are cached automatically so previously fetched images remain pullable when the registry or token service is unavailable +- OCI manifests and tag lists are exceptions: they are cached automatically so previously fetched images remain pullable and tag resolution works when the registry or token service is unavailable **Why stream artifacts?** - Memory efficient - don't load large files into RAM diff --git a/docs/configuration.md b/docs/configuration.md index 3b8b935..c249812 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -292,7 +292,7 @@ Note: Hex cooldown requires disabling registry signature verification since the By default the proxy fetches metadata fresh from upstream on every request. Enable `cache_metadata` to store metadata responses in the database and storage backend for offline fallback. When upstream is unreachable, the proxy serves the last cached copy. ETag-based revalidation avoids re-downloading unchanged metadata. -OCI manifests are always cached because cached image blobs cannot be pulled without their manifests. Digest-addressed manifests are immutable and served directly from cache. Tag-addressed manifests follow `metadata_ttl`, revalidate when stale, and fall back to the last cached response when the registry is unavailable. +OCI manifests and tag lists are always cached because cached image blobs cannot be pulled without their manifests and offline clients may need tag resolution. Digest-addressed manifests are immutable and served directly from cache. Tag-addressed manifests and tag lists follow `metadata_ttl`, revalidate when stale, and fall back to the last cached response when the registry is unavailable. ```yaml cache_metadata: true diff --git a/internal/handler/container.go b/internal/handler/container.go index 74819dd..face4e0 100644 --- a/internal/handler/container.go +++ b/internal/handler/container.go @@ -4,7 +4,6 @@ import ( "encoding/json" "errors" "fmt" - "io" "net/http" "regexp" "strings" @@ -182,7 +181,7 @@ func (h *ContainerHandler) handleManifest(w http.ResponseWriter, r *http.Request h.serveManifest(w, r, registryURL, upstreamName, reference) } -// handleTagsList proxies tag list requests to upstream. +// handleTagsList caches tag list responses for offline OCI pulls. func (h *ContainerHandler) handleTagsList(w http.ResponseWriter, r *http.Request, path string) { if r.Method != http.MethodGet { http.Error(w, "method not allowed", http.StatusMethodNotAllowed) @@ -201,27 +200,7 @@ func (h *ContainerHandler) handleTagsList(w http.ResponseWriter, r *http.Request return } - upstreamURL := fmt.Sprintf("%s/v2/%s/tags/list", registryURL, upstreamName) - if r.URL.RawQuery != "" { - upstreamURL += "?" + r.URL.RawQuery - } - - req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) - if err != nil { - h.containerError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create request") - return - } - - resp, err := h.proxy.HTTPClient.Do(req) - if err != nil { - h.containerError(w, http.StatusBadGateway, "INTERNAL_ERROR", "failed to fetch from upstream") - return - } - defer func() { _ = resp.Body.Close() }() - - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(resp.StatusCode) - _, _ = io.Copy(w, resp.Body) + h.serveTagsList(w, r, registryURL, upstreamName) } // proxyBlobHead handles HEAD requests for blobs. diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index cf058ba..7ef3d1d 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -8,8 +8,10 @@ import ( "encoding/hex" "fmt" "io" + "mime" "net/http" "regexp" + "sort" "strconv" "strings" "time" @@ -35,12 +37,16 @@ type cachedContainerManifest struct { func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, registryURL, name, reference string) { accept := containerManifestAccept(r) - cacheKey := h.containerManifestCacheKey(registryURL, name, reference, accept) + cacheAccept := normalizeContainerManifestAccept(accept) + cacheKey := h.containerManifestCacheKey(registryURL, name, reference, cacheAccept) cached, err := h.loadContainerManifest(r.Context(), cacheKey) if err != nil { h.proxy.Logger.Warn("failed to read cached container manifest", "error", err) cached = nil } + if cached != nil && !containerManifestAccepts(accept, cached.contentType) { + cached = nil + } immutable := manifestDigestReferencePattern.MatchString(reference) if cached != nil && (immutable || h.containerManifestFresh(cached)) { @@ -111,7 +117,7 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, h.proxy.Logger.Warn("failed to cache container manifest", "error", err) } if manifest.contentDigest != reference && manifestDigestReferencePattern.MatchString(manifest.contentDigest) { - digestKey := h.containerManifestCacheKey(registryURL, name, manifest.contentDigest, accept) + digestKey := h.containerManifestCacheKey(registryURL, name, manifest.contentDigest, cacheAccept) if err := h.storeContainerManifest(r.Context(), digestKey, manifest); err != nil { h.proxy.Logger.Warn("failed to cache container manifest by digest", "error", err) } @@ -233,6 +239,76 @@ func containerManifestAccept(r *http.Request) string { }, ", ") } +func normalizeContainerManifestAccept(accept string) string { + mediaTypes := make([]string, 0) + for _, value := range strings.Split(accept, ",") { + value = strings.TrimSpace(value) + if value == "" { + continue + } + mediaType, params, err := mime.ParseMediaType(value) + if err != nil { + mediaTypes = append(mediaTypes, strings.ToLower(value)) + continue + } + paramKeys := make([]string, 0, len(params)) + for key := range params { + paramKeys = append(paramKeys, key) + } + sort.Strings(paramKeys) + canonical := strings.ToLower(mediaType) + for _, key := range paramKeys { + value := params[key] + if strings.EqualFold(key, "q") { + if quality, err := strconv.ParseFloat(value, 64); err == nil { + value = strconv.FormatFloat(quality, 'g', -1, 64) + } + } + canonical += ";" + strings.ToLower(key) + "=" + value + } + mediaTypes = append(mediaTypes, canonical) + } + sort.Strings(mediaTypes) + return strings.Join(mediaTypes, ",") +} + +func containerManifestAccepts(accept, contentType string) bool { + contentType, _, err := mime.ParseMediaType(contentType) + if err != nil { + return false + } + contentType = strings.ToLower(contentType) + contentMajor, contentMinor, found := strings.Cut(contentType, "/") + if !found { + return false + } + + for _, value := range strings.Split(accept, ",") { + mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(value)) + if err != nil || containerAcceptQuality(params) == 0 { + continue + } + mediaType = strings.ToLower(mediaType) + major, minor, found := strings.Cut(mediaType, "/") + if found && (major == "*" || major == contentMajor) && (minor == "*" || minor == contentMinor) { + return true + } + } + return false +} + +func containerAcceptQuality(params map[string]string) float64 { + value, ok := params["q"] + if !ok { + return 1 + } + quality, err := strconv.ParseFloat(value, 64) + if err != nil || quality < 0 || quality > 1 { + return 0 + } + return quality +} + func copyContainerManifestHeaders(destination, source http.Header) { for _, header := range []string{"Content-Type", "Content-Length", "Docker-Content-Digest", "ETag", "WWW-Authenticate"} { if value := source.Get(header); value != "" { diff --git a/internal/handler/container_tags.go b/internal/handler/container_tags.go new file mode 100644 index 0000000..bba8524 --- /dev/null +++ b/internal/handler/container_tags.go @@ -0,0 +1,196 @@ +package handler + +import ( + "bytes" + "context" + "crypto/sha256" + "database/sql" + "encoding/hex" + "fmt" + "io" + "net/http" + "net/url" + "strconv" + "time" + + "github.com/git-pkgs/proxy/internal/database" +) + +const containerTagsCacheEcosystem = "oci-tags" + +type cachedContainerTags struct { + body []byte + contentType string + etag string + size int64 + fetchedAt time.Time +} + +func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, registryURL, name string) { + cacheKey := h.containerTagsCacheKey(registryURL, name, r.URL.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) + return + } + + upstreamURL := fmt.Sprintf("%s/v2/%s/tags/list", registryURL, name) + if query := r.URL.Query().Encode(); query != "" { + upstreamURL += "?" + query + } + req, err := http.NewRequestWithContext(r.Context(), http.MethodGet, upstreamURL, nil) + if err != nil { + h.containerError(w, http.StatusInternalServerError, "INTERNAL_ERROR", "failed to create request") + return + } + req.Header.Set("Accept", "application/json") + if cached != nil && cached.etag != "" { + req.Header.Set("If-None-Match", cached.etag) + } + + resp, err := h.proxy.HTTPClient.Do(req) + if err != nil { + h.serveStaleTagsOrError(w, cached, err) + return + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode == http.StatusNotModified && cached != nil { + cached.fetchedAt = time.Now() + 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) + return + } + if resp.StatusCode != http.StatusOK { + if cached != nil && shouldServeStaleManifest(resp.StatusCode) { + writeContainerTags(w, cached, true) + return + } + copyContainerTagsHeaders(w.Header(), resp.Header) + w.WriteHeader(resp.StatusCode) + _, _ = io.Copy(w, resp.Body) + return + } + + body, err := h.proxy.ReadMetadata(resp.Body) + if err != nil { + h.serveStaleTagsOrError(w, cached, fmt.Errorf("reading tag list: %w", err)) + return + } + tags := &cachedContainerTags{ + body: body, + contentType: resp.Header.Get("Content-Type"), + etag: resp.Header.Get("ETag"), + size: int64(len(body)), + fetchedAt: time.Now(), + } + if tags.contentType == "" { + tags.contentType = contentTypeJSON + } + 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) +} + +func (h *ContainerHandler) serveStaleTagsOrError(w http.ResponseWriter, 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) + return + } + h.proxy.Logger.Error("failed to fetch container tag list", "error", err) + h.containerError(w, http.StatusBadGateway, "INTERNAL_ERROR", "failed to fetch from upstream") +} + +func (h *ContainerHandler) containerTagsCacheKey(registryURL, name string, query url.Values) string { + identity := registryURL + "\x00" + name + "\x00" + query.Encode() + sum := sha256.Sum256([]byte(identity)) + return hex.EncodeToString(sum[:]) +} + +func (h *ContainerHandler) containerTagsFresh(tags *cachedContainerTags) bool { + return h.proxy.MetadataTTL > 0 && !tags.fetchedAt.IsZero() && time.Since(tags.fetchedAt) < h.proxy.MetadataTTL +} + +func (h *ContainerHandler) loadContainerTags(ctx context.Context, cacheKey string) (*cachedContainerTags, error) { + if h.proxy.DB == nil || h.proxy.Storage == nil { + return nil, nil + } + entry, err := h.proxy.DB.GetMetadataCache(containerTagsCacheEcosystem, cacheKey) + if err != nil || entry == nil { + return nil, err + } + reader, err := h.proxy.Storage.Open(ctx, entry.StoragePath) + if err != nil { + return nil, nil + } + defer func() { _ = reader.Close() }() + body, err := h.proxy.ReadMetadata(reader) + if err != nil { + return nil, err + } + + tags := &cachedContainerTags{body: body, contentType: contentTypeJSON, size: int64(len(body))} + if entry.ContentType.Valid { + tags.contentType = entry.ContentType.String + } + if entry.ETag.Valid { + tags.etag = entry.ETag.String + } + if entry.Size.Valid { + tags.size = entry.Size.Int64 + } + if entry.FetchedAt.Valid { + tags.fetchedAt = entry.FetchedAt.Time + } + return tags, nil +} + +func (h *ContainerHandler) storeContainerTags(ctx context.Context, cacheKey string, tags *cachedContainerTags) error { + if h.proxy.DB == nil || h.proxy.Storage == nil { + return nil + } + storagePath := metadataStoragePath(containerTagsCacheEcosystem, cacheKey) + size, _, err := h.proxy.Storage.Store(ctx, storagePath, bytes.NewReader(tags.body)) + if err != nil { + return fmt.Errorf("storing tag list: %w", err) + } + tags.size = size + return h.proxy.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ + Ecosystem: containerTagsCacheEcosystem, + Name: cacheKey, + StoragePath: storagePath, + ETag: sql.NullString{String: tags.etag, Valid: tags.etag != ""}, + ContentType: sql.NullString{String: tags.contentType, Valid: tags.contentType != ""}, + Size: sql.NullInt64{Int64: size, Valid: true}, + FetchedAt: sql.NullTime{Time: tags.fetchedAt, Valid: !tags.fetchedAt.IsZero()}, + }) +} + +func writeContainerTags(w http.ResponseWriter, tags *cachedContainerTags, stale bool) { + w.Header().Set("Content-Type", tags.contentType) + w.Header().Set("Content-Length", strconv.FormatInt(tags.size, 10)) + if tags.etag != "" { + w.Header().Set("ETag", tags.etag) + } + if stale { + w.Header().Set("Warning", containerStaleWarning) + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write(tags.body) +} + +func copyContainerTagsHeaders(destination, source http.Header) { + for _, header := range []string{"Content-Type", "Content-Length", "ETag", "WWW-Authenticate"} { + if value := source.Get(header); value != "" { + destination.Set(header, value) + } + } +} diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 04f00a7..2daadbe 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -134,6 +134,57 @@ func TestContainerHandler_parseTagsListPath(t *testing.T) { } } +func TestContainerHandler_TagsListUsesStaleCacheOnUpstreamFailure(t *testing.T) { + tags := `{"name":"library/nginx","tags":["1.0","latest"]}` + upstreamAvailable := true + upstreamRequests := 0 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamRequests++ + if r.URL.Path != "/v2/library/nginx/tags/list" { + http.NotFound(w, r) + return + } + if !upstreamAvailable { + http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("ETag", `"tags-etag"`) + _, _ = io.WriteString(w, tags) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = 0 + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + + first := httptest.NewRecorder() + h.Routes().ServeHTTP(first, httptest.NewRequest(http.MethodGet, "/library/nginx/tags/list?n=2", nil)) + if first.Code != http.StatusOK { + t.Fatalf("initial status = %d, want 200: %s", first.Code, first.Body.String()) + } + if first.Body.String() != tags { + t.Errorf("initial body = %q, want %q", first.Body.String(), tags) + } + + upstreamAvailable = false + second := httptest.NewRecorder() + h.Routes().ServeHTTP(second, httptest.NewRequest(http.MethodGet, "/library/nginx/tags/list?n=2", nil)) + if second.Code != http.StatusOK { + t.Fatalf("stale status = %d, want 200: %s", second.Code, second.Body.String()) + } + if second.Body.String() != tags { + t.Errorf("stale body = %q, want %q", second.Body.String(), tags) + } + if got := second.Header().Get("Warning"); got != `110 - "Response is Stale"` { + t.Errorf("Warning = %q, want stale warning", got) + } + if upstreamRequests != 2 { + t.Errorf("upstream requests = %d, want 2", upstreamRequests) + } +} + func TestContainerHandler_NamedOCIRegistryServesHelmArtifacts(t *testing.T) { digest := "sha256:abc123def456abc123def456abc123def456abc123def456abc123def456abcd" manifest := `{"schemaVersion":2,"config":{"mediaType":"application/vnd.cncf.helm.config.v1+json"},"layers":[{"mediaType":"application/vnd.cncf.helm.chart.content.v1.tar+gzip","digest":"` + digest + `"}]}` @@ -609,6 +660,92 @@ func TestContainerHandler_ManifestByTag_UsesStaleCacheOnUpstreamFailure(t *testi } } +func TestContainerHandler_ManifestVariantCacheNormalizesCompatibleAccept(t *testing.T) { + digest := "sha256:abababababababababababababababababababababababababababababababab" + manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json"}` + upstreamAvailable := true + upstreamRequests := 0 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamRequests++ + if !upstreamAvailable { + http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json") + w.Header().Set("Docker-Content-Digest", digest) + _, _ = io.WriteString(w, manifest) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = 0 + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + + firstRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + firstRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json,application/vnd.oci.image.manifest.v1+json") + first := httptest.NewRecorder() + h.Routes().ServeHTTP(first, firstRequest) + if first.Code != http.StatusOK { + t.Fatalf("initial status = %d, want 200: %s", first.Code, first.Body.String()) + } + + upstreamAvailable = false + secondRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + secondRequest.Header.Set("Accept", " application/vnd.oci.image.manifest.v1+json , application/vnd.oci.image.index.v1+json ") + second := httptest.NewRecorder() + h.Routes().ServeHTTP(second, secondRequest) + if second.Code != http.StatusOK { + t.Fatalf("stale status = %d, want 200: %s", second.Code, second.Body.String()) + } + if second.Body.String() != manifest { + t.Errorf("stale body = %q, want %q", second.Body.String(), manifest) + } + if got := second.Header().Get("Warning"); got != `110 - "Response is Stale"` { + t.Errorf("Warning = %q, want stale warning", got) + } + if upstreamRequests != 2 { + t.Errorf("upstream requests = %d, want 2", upstreamRequests) + } +} + +func TestContainerHandler_ManifestVariantCacheDoesNotServeUnacceptedContentType(t *testing.T) { + digest := "sha256:cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd" + upstreamAvailable := true + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if !upstreamAvailable { + http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json") + w.Header().Set("Docker-Content-Digest", digest) + _, _ = io.WriteString(w, `{"schemaVersion":2}`) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = 0 + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + + warmRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + warmRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json") + warm := httptest.NewRecorder() + h.Routes().ServeHTTP(warm, warmRequest) + if warm.Code != http.StatusOK { + t.Fatalf("warm status = %d, want 200", warm.Code) + } + + upstreamAvailable = false + offlineRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + offlineRequest.Header.Set("Accept", "application/vnd.oci.image.manifest.v1+json") + offline := httptest.NewRecorder() + h.Routes().ServeHTTP(offline, offlineRequest) + if offline.Code != http.StatusServiceUnavailable { + t.Errorf("offline status = %d, want 503", offline.Code) + } +} + func TestContainerHandler_ManifestByTag_CachesDigestAlias(t *testing.T) { digest := "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json"}` From 1bae32c2ceae3498878d9e051a1dc05c2416e73b Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Sat, 22 Aug 2026 14:46:58 +0530 Subject: [PATCH 2/5] fix(oci): refine manifest cache variants --- internal/handler/container_manifest.go | 19 +++++++++++++------ internal/handler/container_test.go | 4 ++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index 7ef3d1d..d209542 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -44,7 +44,7 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, h.proxy.Logger.Warn("failed to read cached container manifest", "error", err) cached = nil } - if cached != nil && !containerManifestAccepts(accept, cached.contentType) { + if cached != nil && cached.contentType != "" && !containerManifestAccepts(accept, cached.contentType) { cached = nil } @@ -240,7 +240,7 @@ func containerManifestAccept(r *http.Request) string { } func normalizeContainerManifestAccept(accept string) string { - mediaTypes := make([]string, 0) + mediaTypes := make(map[string]struct{}) for _, value := range strings.Split(accept, ",") { value = strings.TrimSpace(value) if value == "" { @@ -248,7 +248,7 @@ func normalizeContainerManifestAccept(accept string) string { } mediaType, params, err := mime.ParseMediaType(value) if err != nil { - mediaTypes = append(mediaTypes, strings.ToLower(value)) + mediaTypes[strings.ToLower(value)] = struct{}{} continue } paramKeys := make([]string, 0, len(params)) @@ -261,15 +261,22 @@ func normalizeContainerManifestAccept(accept string) string { value := params[key] if strings.EqualFold(key, "q") { if quality, err := strconv.ParseFloat(value, 64); err == nil { + if quality == 1 { + continue + } value = strconv.FormatFloat(quality, 'g', -1, 64) } } canonical += ";" + strings.ToLower(key) + "=" + value } - mediaTypes = append(mediaTypes, canonical) + mediaTypes[canonical] = struct{}{} } - sort.Strings(mediaTypes) - return strings.Join(mediaTypes, ",") + canonicalMediaTypes := make([]string, 0, len(mediaTypes)) + for mediaType := range mediaTypes { + canonicalMediaTypes = append(canonicalMediaTypes, mediaType) + } + sort.Strings(canonicalMediaTypes) + return strings.Join(canonicalMediaTypes, ",") } func containerManifestAccepts(accept, contentType string) bool { diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 2daadbe..8336c11 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -671,7 +671,7 @@ func TestContainerHandler_ManifestVariantCacheNormalizesCompatibleAccept(t *test http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) return } - w.Header().Set("Content-Type", "application/vnd.oci.image.index.v1+json") + w.Header().Set("Content-Type", "") w.Header().Set("Docker-Content-Digest", digest) _, _ = io.WriteString(w, manifest) })) @@ -683,7 +683,7 @@ func TestContainerHandler_ManifestVariantCacheNormalizesCompatibleAccept(t *test h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} firstRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) - firstRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json,application/vnd.oci.image.manifest.v1+json") + firstRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json;q=1, application/vnd.oci.image.manifest.v1+json;q=1, application/vnd.oci.image.index.v1+json;q=1") first := httptest.NewRecorder() h.Routes().ServeHTTP(first, firstRequest) if first.Code != http.StatusOK { From 299658af755e5ac9fa734c2989d3bb2088e2a3fd Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Mon, 24 Aug 2026 14:30:24 +0530 Subject: [PATCH 3/5] fix(oci): preserve manifest cache compatibility --- internal/handler/container_manifest.go | 71 ++++++++++++++++++++++---- internal/handler/container_test.go | 60 ++++++++++++++++++++-- 2 files changed, 117 insertions(+), 14 deletions(-) diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index d209542..d54dd72 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -22,6 +22,10 @@ import ( const ( containerManifestCacheEcosystem = "oci-manifest" containerStaleWarning = `110 - "Response is Stale"` + + containerAcceptWildcardSpecificity = iota + containerAcceptTypeWildcardSpecificity + containerAcceptExactSpecificity ) var manifestDigestReferencePattern = regexp.MustCompile(`^[a-z0-9]+:[a-f0-9]+$`) @@ -39,14 +43,7 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, accept := containerManifestAccept(r) cacheAccept := normalizeContainerManifestAccept(accept) cacheKey := h.containerManifestCacheKey(registryURL, name, reference, cacheAccept) - cached, err := h.loadContainerManifest(r.Context(), cacheKey) - if err != nil { - h.proxy.Logger.Warn("failed to read cached container manifest", "error", err) - cached = nil - } - if cached != nil && cached.contentType != "" && !containerManifestAccepts(accept, cached.contentType) { - cached = nil - } + cached := h.loadContainerManifestForAccept(r.Context(), registryURL, name, reference, accept, cacheKey) immutable := manifestDigestReferencePattern.MatchString(reference) if cached != nil && (immutable || h.containerManifestFresh(cached)) { @@ -145,6 +142,37 @@ func (h *ContainerHandler) containerManifestCacheKey(registryURL, name, referenc return hex.EncodeToString(sum[:]) } +func (h *ContainerHandler) loadContainerManifestForAccept(ctx context.Context, registryURL, name, reference, accept, cacheKey string) *cachedContainerManifest { + cached, err := h.loadContainerManifest(ctx, cacheKey) + if err != nil { + h.proxy.Logger.Warn("failed to read cached container manifest", "error", err) + return nil + } + if cached != nil { + if containerManifestCacheCompatible(accept, cached) { + return cached + } + return nil + } + + legacyCacheKey := h.containerManifestCacheKey(registryURL, name, reference, accept) + if legacyCacheKey == cacheKey { + return nil + } + cached, err = h.loadContainerManifest(ctx, legacyCacheKey) + if err != nil { + h.proxy.Logger.Warn("failed to read legacy cached container manifest", "error", err) + return nil + } + if cached == nil || !containerManifestCacheCompatible(accept, cached) { + return nil + } + if err := h.storeContainerManifest(ctx, cacheKey, cached); err != nil { + h.proxy.Logger.Warn("failed to migrate cached container manifest", "error", err) + } + return cached +} + func (h *ContainerHandler) loadContainerManifest(ctx context.Context, cacheKey string) (*cachedContainerManifest, error) { if h.proxy.DB == nil || h.proxy.Storage == nil { return nil, nil @@ -290,18 +318,39 @@ func containerManifestAccepts(accept, contentType string) bool { return false } + bestSpecificity := -1 + bestQuality := 0.0 for _, value := range strings.Split(accept, ",") { mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(value)) - if err != nil || containerAcceptQuality(params) == 0 { + if err != nil { continue } mediaType = strings.ToLower(mediaType) major, minor, found := strings.Cut(mediaType, "/") if found && (major == "*" || major == contentMajor) && (minor == "*" || minor == contentMinor) { - return true + specificity := containerAcceptSpecificity(major, minor) + if specificity > bestSpecificity { + bestSpecificity = specificity + bestQuality = containerAcceptQuality(params) + } } } - return false + return bestQuality > 0 +} + +func containerManifestCacheCompatible(accept string, manifest *cachedContainerManifest) bool { + return manifest.contentType == "" || containerManifestAccepts(accept, manifest.contentType) +} + +func containerAcceptSpecificity(major, minor string) int { + switch { + case major == "*" && minor == "*": + return containerAcceptWildcardSpecificity + case major == "*" || minor == "*": + return containerAcceptTypeWildcardSpecificity + default: + return containerAcceptExactSpecificity + } } func containerAcceptQuality(params map[string]string) float64 { diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 8336c11..7fb0cf7 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -709,7 +709,61 @@ func TestContainerHandler_ManifestVariantCacheNormalizesCompatibleAccept(t *test } } -func TestContainerHandler_ManifestVariantCacheDoesNotServeUnacceptedContentType(t *testing.T) { +func TestContainerHandler_ManifestMigratesLegacyAcceptCacheKey(t *testing.T) { + digest := "sha256:cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd" + manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.index.v1+json"}` + upstreamRequests := 0 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamRequests++ + http.Error(w, "upstream should not be called", http.StatusServiceUnavailable) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = time.Hour + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + accept := "application/vnd.oci.image.index.v1+json;q=1, application/vnd.oci.image.manifest.v1+json;q=1" + cacheAccept := normalizeContainerManifestAccept(accept) + legacyCacheKey := h.containerManifestCacheKey(upstream.URL, "library/nginx", "latest", accept) + cacheKey := h.containerManifestCacheKey(upstream.URL, "library/nginx", "latest", cacheAccept) + if legacyCacheKey == cacheKey { + t.Fatal("legacy and normalized cache keys are equal") + } + + request := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + request.Header.Set("Accept", accept) + legacyManifest := &cachedContainerManifest{ + body: []byte(manifest), + contentType: "application/vnd.oci.image.index.v1+json", + contentDigest: digest, + fetchedAt: time.Now(), + } + if err := h.storeContainerManifest(request.Context(), legacyCacheKey, legacyManifest); err != nil { + t.Fatalf("store legacy manifest: %v", err) + } + + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", response.Code, response.Body.String()) + } + if response.Body.String() != manifest { + t.Errorf("body = %q, want %q", response.Body.String(), manifest) + } + if upstreamRequests != 0 { + t.Errorf("upstream requests = %d, want 0", upstreamRequests) + } + migrated, err := h.loadContainerManifest(request.Context(), cacheKey) + if err != nil { + t.Fatalf("load migrated manifest: %v", err) + } + if migrated == nil { + t.Error("normalized cache entry was not created") + } +} + +func TestContainerHandler_ManifestVariantCacheHonorsSpecificAcceptExclusions(t *testing.T) { digest := "sha256:cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd" upstreamAvailable := true upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -729,7 +783,7 @@ func TestContainerHandler_ManifestVariantCacheDoesNotServeUnacceptedContentType( h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} warmRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) - warmRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json") + warmRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json;q=0, */*;q=1") warm := httptest.NewRecorder() h.Routes().ServeHTTP(warm, warmRequest) if warm.Code != http.StatusOK { @@ -738,7 +792,7 @@ func TestContainerHandler_ManifestVariantCacheDoesNotServeUnacceptedContentType( upstreamAvailable = false offlineRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) - offlineRequest.Header.Set("Accept", "application/vnd.oci.image.manifest.v1+json") + offlineRequest.Header.Set("Accept", "application/vnd.oci.image.index.v1+json;q=0, */*;q=1") offline := httptest.NewRecorder() h.Routes().ServeHTTP(offline, offlineRequest) if offline.Code != http.StatusServiceUnavailable { From 7d061204c2dcb1b079626dbdbfadee28c4994966 Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Tue, 25 Aug 2026 18:24:19 +0530 Subject: [PATCH 4/5] fix(oci): dual-write manifest cache variants --- internal/handler/container_manifest.go | 28 ++++++++++++------- internal/handler/container_test.go | 38 ++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index d54dd72..1002b20 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -71,9 +71,7 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, if resp.StatusCode == http.StatusNotModified && cached != nil { cached.fetchedAt = time.Now() - if err := h.storeContainerManifest(r.Context(), cacheKey, cached); err != nil { - h.proxy.Logger.Warn("failed to refresh cached container manifest", "error", err) - } + h.storeContainerManifestForAccept(r.Context(), registryURL, name, reference, accept, cacheAccept, cached) writeContainerManifest(w, r.Method, cached, false) return } @@ -110,14 +108,9 @@ func (h *ContainerHandler) serveManifest(w http.ResponseWriter, r *http.Request, if manifest.contentDigest == "" { manifest.contentDigest = sha256Digest(body) } - if err := h.storeContainerManifest(r.Context(), cacheKey, manifest); err != nil { - h.proxy.Logger.Warn("failed to cache container manifest", "error", err) - } + h.storeContainerManifestForAccept(r.Context(), registryURL, name, reference, accept, cacheAccept, manifest) if manifest.contentDigest != reference && manifestDigestReferencePattern.MatchString(manifest.contentDigest) { - digestKey := h.containerManifestCacheKey(registryURL, name, manifest.contentDigest, cacheAccept) - if err := h.storeContainerManifest(r.Context(), digestKey, manifest); err != nil { - h.proxy.Logger.Warn("failed to cache container manifest by digest", "error", err) - } + h.storeContainerManifestForAccept(r.Context(), registryURL, name, manifest.contentDigest, accept, cacheAccept, manifest) } writeContainerManifest(w, r.Method, manifest, false) } @@ -173,6 +166,21 @@ func (h *ContainerHandler) loadContainerManifestForAccept(ctx context.Context, r return cached } +func (h *ContainerHandler) storeContainerManifestForAccept(ctx context.Context, registryURL, name, reference, accept, cacheAccept string, manifest *cachedContainerManifest) { + cacheKey := h.containerManifestCacheKey(registryURL, name, reference, cacheAccept) + if err := h.storeContainerManifest(ctx, cacheKey, manifest); err != nil { + h.proxy.Logger.Warn("failed to cache container manifest", "error", err) + } + + legacyCacheKey := h.containerManifestCacheKey(registryURL, name, reference, accept) + if legacyCacheKey == cacheKey { + return + } + if err := h.storeContainerManifest(ctx, legacyCacheKey, manifest); err != nil { + h.proxy.Logger.Warn("failed to cache legacy container manifest", "error", err) + } +} + func (h *ContainerHandler) loadContainerManifest(ctx context.Context, cacheKey string) (*cachedContainerManifest, error) { if h.proxy.DB == nil || h.proxy.Storage == nil { return nil, nil diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 7fb0cf7..672efc2 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -763,6 +763,44 @@ func TestContainerHandler_ManifestMigratesLegacyAcceptCacheKey(t *testing.T) { } } +func TestContainerHandler_ManifestDualWritesLegacyAcceptCacheKeys(t *testing.T) { + digest := "sha256:dededededededededededededededededededededededededededededededede" + manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json"}` + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v2/library/nginx/manifests/latest" { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/vnd.oci.image.manifest.v1+json") + w.Header().Set("Docker-Content-Digest", digest) + _, _ = io.WriteString(w, manifest) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + accept := "application/vnd.oci.image.manifest.v1+json;q=1, application/vnd.oci.image.manifest.v1+json;q=1" + request := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + request.Header.Set("Accept", accept) + response := httptest.NewRecorder() + h.Routes().ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("status = %d, want 200: %s", response.Code, response.Body.String()) + } + + for _, reference := range []string{"latest", digest} { + legacyCacheKey := h.containerManifestCacheKey(upstream.URL, "library/nginx", reference, accept) + cached, err := h.loadContainerManifest(request.Context(), legacyCacheKey) + if err != nil { + t.Fatalf("load legacy %s manifest: %v", reference, err) + } + if cached == nil { + t.Errorf("legacy %s cache entry was not written", reference) + } + } +} + func TestContainerHandler_ManifestVariantCacheHonorsSpecificAcceptExclusions(t *testing.T) { digest := "sha256:cdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcdcd" upstreamAvailable := true From 7633300b93ccb8760226bf633af82dd43aaef8dd Mon Sep 17 00:00:00 2001 From: abhinavgautam01 Date: Thu, 27 Aug 2026 21:38:29 +0530 Subject: [PATCH 5/5] fix(oci): preserve cached pagination links --- internal/database/metadata_cache_test.go | 51 ++++++++++++ internal/database/queries.go | 14 ++-- internal/database/schema.go | 27 +++++- internal/database/types.go | 1 + internal/handler/container_manifest.go | 67 ++++++++------- internal/handler/container_metadata.go | 38 +++++++++ internal/handler/container_tags.go | 62 +++++++++----- internal/handler/container_test.go | 100 +++++++++++++++++++++++ 8 files changed, 302 insertions(+), 58 deletions(-) create mode 100644 internal/handler/container_metadata.go diff --git a/internal/database/metadata_cache_test.go b/internal/database/metadata_cache_test.go index 09dcba3..1cdd1f9 100644 --- a/internal/database/metadata_cache_test.go +++ b/internal/database/metadata_cache_test.go @@ -29,6 +29,7 @@ func TestUpsertAndGetMetadataCache(t *testing.T) { Name: "lodash", StoragePath: "_metadata/npm/lodash/metadata", ETag: sql.NullString{String: `"abc123"`, Valid: true}, + Link: sql.NullString{String: `; rel="next"`, Valid: true}, ContentType: sql.NullString{String: "application/json", Valid: true}, ContentDigest: sql.NullString{ String: "sha256:0123456789abcdef", @@ -63,6 +64,9 @@ func TestUpsertAndGetMetadataCache(t *testing.T) { if !got.ETag.Valid || got.ETag.String != `"abc123"` { t.Errorf("etag = %v, want %q", got.ETag, `"abc123"`) } + if !got.Link.Valid || got.Link.String != `; rel="next"` { + t.Errorf("link = %v, want next link", got.Link) + } if !got.ContentType.Valid || got.ContentType.String != "application/json" { t.Errorf("content_type = %v, want %q", got.ContentType, "application/json") } @@ -158,6 +162,9 @@ func TestUpsertMetadataCacheNullableFields(t *testing.T) { if got.ContentType.Valid { t.Error("expected null content_type") } + if got.Link.Valid { + t.Error("expected null link") + } if got.Size.Valid { t.Error("expected null size") } @@ -229,3 +236,47 @@ func TestMetadataCacheContentDigestMigrationPreservesExistingRows(t *testing.T) t.Errorf("legacy content digest = %q, want NULL", entry.ContentDigest.String) } } + +func TestMetadataCacheLinkMigrationPreservesExistingRows(t *testing.T) { + dbPath := filepath.Join(t.TempDir(), "test.db") + db, err := Create(dbPath) + if err != nil { + t.Fatalf("Create failed: %v", err) + } + defer func() { _ = db.Close() }() + + if _, err := db.Exec("ALTER TABLE metadata_cache DROP COLUMN link"); err != nil { + t.Fatalf("dropping link: %v", err) + } + if _, err := db.Exec("DELETE FROM migrations WHERE name = ?", "007_add_metadata_link"); err != nil { + t.Fatalf("resetting link migration: %v", err) + } + if _, err := db.Exec(` + INSERT INTO metadata_cache (ecosystem, name, storage_path, content_type, size, fetched_at, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?) + `, "oci-tags", "cache-key", "_metadata/oci-tags/cache-key/metadata", "application/json", 2, time.Now(), time.Now(), time.Now()); err != nil { + t.Fatalf("inserting legacy cache row: %v", err) + } + + if err := db.MigrateSchema(); err != nil { + t.Fatalf("MigrateSchema() error = %v", err) + } + hasLink, err := db.HasColumn("metadata_cache", "link") + if err != nil { + t.Fatalf("HasColumn() error = %v", err) + } + if !hasLink { + t.Fatal("metadata_cache.link was not added") + } + + entry, err := db.GetMetadataCache("oci-tags", "cache-key") + if err != nil { + t.Fatalf("GetMetadataCache() error = %v", err) + } + if entry == nil || entry.StoragePath != "_metadata/oci-tags/cache-key/metadata" { + t.Fatalf("existing metadata cache row was not preserved: %#v", entry) + } + if entry.Link.Valid { + t.Errorf("legacy link = %q, want NULL", entry.Link.String) + } +} diff --git a/internal/database/queries.go b/internal/database/queries.go index 9fa5381..ed54ba8 100644 --- a/internal/database/queries.go +++ b/internal/database/queries.go @@ -906,7 +906,7 @@ func (db *DB) CountCachedPackages(ecosystem string) (int64, error) { func (db *DB) GetMetadataCache(ecosystem, name string) (*MetadataCacheEntry, error) { var entry MetadataCacheEntry query := db.Rebind(` - SELECT id, ecosystem, name, storage_path, etag, content_type, + SELECT id, ecosystem, name, storage_path, etag, link, content_type, content_digest, size, last_modified, fetched_at, created_at, updated_at FROM metadata_cache WHERE ecosystem = ? AND name = ? `) @@ -926,12 +926,13 @@ func (db *DB) UpsertMetadataCache(entry *MetadataCacheEntry) error { if db.dialect == DialectPostgres { query = ` - INSERT INTO metadata_cache (ecosystem, name, storage_path, etag, content_type, + INSERT INTO metadata_cache (ecosystem, name, storage_path, etag, link, content_type, content_digest, size, last_modified, fetched_at, created_at, updated_at) - VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12) ON CONFLICT(ecosystem, name) DO UPDATE SET storage_path = EXCLUDED.storage_path, etag = EXCLUDED.etag, + link = EXCLUDED.link, content_type = EXCLUDED.content_type, content_digest = EXCLUDED.content_digest, size = EXCLUDED.size, @@ -941,12 +942,13 @@ func (db *DB) UpsertMetadataCache(entry *MetadataCacheEntry) error { ` } else { query = ` - INSERT INTO metadata_cache (ecosystem, name, storage_path, etag, content_type, + INSERT INTO metadata_cache (ecosystem, name, storage_path, etag, link, content_type, content_digest, size, last_modified, fetched_at, created_at, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(ecosystem, name) DO UPDATE SET storage_path = excluded.storage_path, etag = excluded.etag, + link = excluded.link, content_type = excluded.content_type, content_digest = excluded.content_digest, size = excluded.size, @@ -957,7 +959,7 @@ func (db *DB) UpsertMetadataCache(entry *MetadataCacheEntry) error { } _, err := db.Exec(query, - entry.Ecosystem, entry.Name, entry.StoragePath, entry.ETag, + entry.Ecosystem, entry.Name, entry.StoragePath, entry.ETag, entry.Link, entry.ContentType, entry.ContentDigest, entry.Size, entry.LastModified, entry.FetchedAt, now, now, ) if err != nil { diff --git a/internal/database/schema.go b/internal/database/schema.go index c73877d..a309970 100644 --- a/internal/database/schema.go +++ b/internal/database/schema.go @@ -101,6 +101,7 @@ CREATE TABLE IF NOT EXISTS metadata_cache ( name TEXT NOT NULL, storage_path TEXT NOT NULL, etag TEXT, + link TEXT, content_type TEXT, content_digest TEXT, size INTEGER, @@ -202,6 +203,7 @@ CREATE TABLE IF NOT EXISTS metadata_cache ( name TEXT NOT NULL, storage_path TEXT NOT NULL, etag TEXT, + link TEXT, content_type TEXT, content_digest TEXT, size BIGINT, @@ -362,6 +364,7 @@ var migrations = []migration{ {"004_ensure_vulnerabilities_table", migrateEnsureVulnerabilitiesTable}, {"005_ensure_metadata_cache_table", migrateEnsureMetadataCacheTable}, {"006_add_metadata_content_digest", migrateAddMetadataContentDigest}, + {"007_add_metadata_link", migrateAddMetadataLink}, } // isTableNotFound returns true if the error indicates a missing table. @@ -598,6 +601,20 @@ func migrateAddMetadataContentDigest(db *DB) error { return nil } +func migrateAddMetadataLink(db *DB) error { + hasColumn, err := db.HasColumn("metadata_cache", "link") + if err != nil { + return fmt.Errorf("checking metadata_cache link column: %w", err) + } + if hasColumn { + return nil + } + if _, err := db.Exec("ALTER TABLE metadata_cache ADD COLUMN link TEXT"); err != nil { + return fmt.Errorf("adding metadata_cache link column: %w", err) + } + return nil +} + // EnsureMetadataCacheTable creates the metadata_cache table if it doesn't exist. func (db *DB) EnsureMetadataCacheTable() error { has, err := db.HasTable("metadata_cache") @@ -616,8 +633,9 @@ func (db *DB) EnsureMetadataCacheTable() error { ecosystem TEXT NOT NULL, name TEXT NOT NULL, storage_path TEXT NOT NULL, - etag TEXT, - content_type TEXT, + etag TEXT, + link TEXT, + content_type TEXT, content_digest TEXT, size BIGINT, last_modified TIMESTAMP, @@ -634,8 +652,9 @@ func (db *DB) EnsureMetadataCacheTable() error { ecosystem TEXT NOT NULL, name TEXT NOT NULL, storage_path TEXT NOT NULL, - etag TEXT, - content_type TEXT, + etag TEXT, + link TEXT, + content_type TEXT, content_digest TEXT, size INTEGER, last_modified DATETIME, diff --git a/internal/database/types.go b/internal/database/types.go index 5ddb9f3..5c32187 100644 --- a/internal/database/types.go +++ b/internal/database/types.go @@ -163,6 +163,7 @@ type MetadataCacheEntry struct { Name string `db:"name" json:"name"` StoragePath string `db:"storage_path" json:"storage_path"` ETag sql.NullString `db:"etag" json:"etag,omitempty"` + Link sql.NullString `db:"link" json:"link,omitempty"` ContentType sql.NullString `db:"content_type" json:"content_type,omitempty"` ContentDigest sql.NullString `db:"content_digest" json:"content_digest,omitempty"` Size sql.NullInt64 `db:"size" json:"size,omitempty"` diff --git a/internal/handler/container_manifest.go b/internal/handler/container_manifest.go index 1002b20..b7cdeb5 100644 --- a/internal/handler/container_manifest.go +++ b/internal/handler/container_manifest.go @@ -1,10 +1,8 @@ package handler import ( - "bytes" "context" "crypto/sha256" - "database/sql" "encoding/hex" "fmt" "io" @@ -15,8 +13,6 @@ import ( "strconv" "strings" "time" - - "github.com/git-pkgs/proxy/internal/database" ) const ( @@ -221,25 +217,13 @@ func (h *ContainerHandler) loadContainerManifest(ctx context.Context, cacheKey s } func (h *ContainerHandler) storeContainerManifest(ctx context.Context, cacheKey string, manifest *cachedContainerManifest) error { - if h.proxy.DB == nil || h.proxy.Storage == nil { - return nil - } - storagePath := metadataStoragePath(containerManifestCacheEcosystem, cacheKey) - size, _, err := h.proxy.Storage.Store(ctx, storagePath, bytes.NewReader(manifest.body)) + size, err := h.storeContainerMetadata(ctx, containerManifestCacheEcosystem, cacheKey, manifest.body, + manifest.etag, "", manifest.contentType, manifest.contentDigest, manifest.fetchedAt) if err != nil { return fmt.Errorf("storing manifest: %w", err) } manifest.size = size - return h.proxy.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ - Ecosystem: containerManifestCacheEcosystem, - Name: cacheKey, - StoragePath: storagePath, - ETag: sql.NullString{String: manifest.etag, Valid: manifest.etag != ""}, - ContentType: sql.NullString{String: manifest.contentType, Valid: manifest.contentType != ""}, - ContentDigest: sql.NullString{String: manifest.contentDigest, Valid: manifest.contentDigest != ""}, - Size: sql.NullInt64{Int64: size, Valid: true}, - FetchedAt: sql.NullTime{Time: manifest.fetchedAt, Valid: !manifest.fetchedAt.IsZero()}, - }) + return nil } func writeContainerManifest(w http.ResponseWriter, method string, manifest *cachedContainerManifest, stale bool) { @@ -316,7 +300,7 @@ func normalizeContainerManifestAccept(accept string) string { } func containerManifestAccepts(accept, contentType string) bool { - contentType, _, err := mime.ParseMediaType(contentType) + contentType, contentParams, err := mime.ParseMediaType(contentType) if err != nil { return false } @@ -326,7 +310,8 @@ func containerManifestAccepts(accept, contentType string) bool { return false } - bestSpecificity := -1 + bestMediaTypeSpecificity := -1 + bestParameterSpecificity := 0 bestQuality := 0.0 for _, value := range strings.Split(accept, ",") { mediaType, params, err := mime.ParseMediaType(strings.TrimSpace(value)) @@ -335,10 +320,12 @@ func containerManifestAccepts(accept, contentType string) bool { } mediaType = strings.ToLower(mediaType) major, minor, found := strings.Cut(mediaType, "/") - if found && (major == "*" || major == contentMajor) && (minor == "*" || minor == contentMinor) { - specificity := containerAcceptSpecificity(major, minor) - if specificity > bestSpecificity { - bestSpecificity = specificity + if found && containerAcceptRangeMatches(major, minor, params, contentMajor, contentMinor, contentParams) { + mediaTypeSpecificity, parameterSpecificity := containerAcceptSpecificity(major, minor, params) + if mediaTypeSpecificity > bestMediaTypeSpecificity || + (mediaTypeSpecificity == bestMediaTypeSpecificity && parameterSpecificity > bestParameterSpecificity) { + bestMediaTypeSpecificity = mediaTypeSpecificity + bestParameterSpecificity = parameterSpecificity bestQuality = containerAcceptQuality(params) } } @@ -350,14 +337,36 @@ func containerManifestCacheCompatible(accept string, manifest *cachedContainerMa return manifest.contentType == "" || containerManifestAccepts(accept, manifest.contentType) } -func containerAcceptSpecificity(major, minor string) int { +func containerAcceptRangeMatches(major, minor string, params map[string]string, contentMajor, contentMinor string, contentParams map[string]string) bool { + if (major != "*" && major != contentMajor) || (minor != "*" && minor != contentMinor) { + return false + } + for key, value := range params { + if strings.EqualFold(key, "q") { + continue + } + if contentParams[key] != value { + return false + } + } + return true +} + +func containerAcceptSpecificity(major, minor string, params map[string]string) (int, int) { + parameterSpecificity := 0 + for key := range params { + if !strings.EqualFold(key, "q") { + parameterSpecificity++ + } + } + switch { case major == "*" && minor == "*": - return containerAcceptWildcardSpecificity + return containerAcceptWildcardSpecificity, parameterSpecificity case major == "*" || minor == "*": - return containerAcceptTypeWildcardSpecificity + return containerAcceptTypeWildcardSpecificity, parameterSpecificity default: - return containerAcceptExactSpecificity + return containerAcceptExactSpecificity, parameterSpecificity } } diff --git a/internal/handler/container_metadata.go b/internal/handler/container_metadata.go new file mode 100644 index 0000000..0a9e44b --- /dev/null +++ b/internal/handler/container_metadata.go @@ -0,0 +1,38 @@ +package handler + +import ( + "bytes" + "context" + "database/sql" + "fmt" + "time" + + "github.com/git-pkgs/proxy/internal/database" +) + +func (h *ContainerHandler) storeContainerMetadata(ctx context.Context, ecosystem, cacheKey string, body []byte, etag, link, contentType, contentDigest string, fetchedAt time.Time) (int64, error) { + if h.proxy.DB == nil || h.proxy.Storage == nil { + return int64(len(body)), nil + } + + storagePath := metadataStoragePath(ecosystem, cacheKey) + size, _, err := h.proxy.Storage.Store(ctx, storagePath, bytes.NewReader(body)) + if err != nil { + return 0, fmt.Errorf("storing metadata: %w", err) + } + err = h.proxy.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ + Ecosystem: ecosystem, + Name: cacheKey, + StoragePath: storagePath, + ETag: sql.NullString{String: etag, Valid: etag != ""}, + Link: sql.NullString{String: link, Valid: link != ""}, + ContentType: sql.NullString{String: contentType, Valid: contentType != ""}, + ContentDigest: sql.NullString{String: contentDigest, Valid: contentDigest != ""}, + Size: sql.NullInt64{Int64: size, Valid: true}, + FetchedAt: sql.NullTime{Time: fetchedAt, Valid: !fetchedAt.IsZero()}, + }) + if err != nil { + return 0, err + } + return size, nil +} diff --git a/internal/handler/container_tags.go b/internal/handler/container_tags.go index bba8524..04db92f 100644 --- a/internal/handler/container_tags.go +++ b/internal/handler/container_tags.go @@ -1,27 +1,28 @@ package handler import ( - "bytes" "context" "crypto/sha256" - "database/sql" "encoding/hex" "fmt" "io" "net/http" "net/url" + "regexp" "strconv" + "strings" "time" - - "github.com/git-pkgs/proxy/internal/database" ) const containerTagsCacheEcosystem = "oci-tags" +var containerLinkTargetPattern = regexp.MustCompile(`<([^>]*)>`) + type cachedContainerTags struct { body []byte contentType string etag string + link string size int64 fetchedAt time.Time } @@ -87,6 +88,7 @@ func (h *ContainerHandler) serveTagsList(w http.ResponseWriter, r *http.Request, body: body, contentType: resp.Header.Get("Content-Type"), etag: resp.Header.Get("ETag"), + link: h.rewriteContainerTagsLink(strings.Join(resp.Header.Values("Link"), ", "), registryURL, r.URL.Path), size: int64(len(body)), fetchedAt: time.Now(), } @@ -144,6 +146,9 @@ func (h *ContainerHandler) loadContainerTags(ctx context.Context, cacheKey strin if entry.ETag.Valid { tags.etag = entry.ETag.String } + if entry.Link.Valid { + tags.link = entry.Link.String + } if entry.Size.Valid { tags.size = entry.Size.Int64 } @@ -154,24 +159,13 @@ func (h *ContainerHandler) loadContainerTags(ctx context.Context, cacheKey strin } func (h *ContainerHandler) storeContainerTags(ctx context.Context, cacheKey string, tags *cachedContainerTags) error { - if h.proxy.DB == nil || h.proxy.Storage == nil { - return nil - } - storagePath := metadataStoragePath(containerTagsCacheEcosystem, cacheKey) - size, _, err := h.proxy.Storage.Store(ctx, storagePath, bytes.NewReader(tags.body)) + size, err := h.storeContainerMetadata(ctx, containerTagsCacheEcosystem, cacheKey, tags.body, + tags.etag, tags.link, tags.contentType, "", tags.fetchedAt) if err != nil { return fmt.Errorf("storing tag list: %w", err) } tags.size = size - return h.proxy.DB.UpsertMetadataCache(&database.MetadataCacheEntry{ - Ecosystem: containerTagsCacheEcosystem, - Name: cacheKey, - StoragePath: storagePath, - ETag: sql.NullString{String: tags.etag, Valid: tags.etag != ""}, - ContentType: sql.NullString{String: tags.contentType, Valid: tags.contentType != ""}, - Size: sql.NullInt64{Int64: size, Valid: true}, - FetchedAt: sql.NullTime{Time: tags.fetchedAt, Valid: !tags.fetchedAt.IsZero()}, - }) + return nil } func writeContainerTags(w http.ResponseWriter, tags *cachedContainerTags, stale bool) { @@ -180,6 +174,9 @@ func writeContainerTags(w http.ResponseWriter, tags *cachedContainerTags, stale if tags.etag != "" { w.Header().Set("ETag", tags.etag) } + if tags.link != "" { + w.Header().Set("Link", tags.link) + } if stale { w.Header().Set("Warning", containerStaleWarning) } @@ -188,9 +185,36 @@ func writeContainerTags(w http.ResponseWriter, tags *cachedContainerTags, stale } func copyContainerTagsHeaders(destination, source http.Header) { - for _, header := range []string{"Content-Type", "Content-Length", "ETag", "WWW-Authenticate"} { + for _, header := range []string{"Content-Type", "Content-Length", "ETag", "Link", "WWW-Authenticate"} { if value := source.Get(header); value != "" { destination.Set(header, value) } } } + +func (h *ContainerHandler) rewriteContainerTagsLink(link, registryURL, requestPath string) string { + if link == "" { + return "" + } + upstreamURL, err := url.Parse(registryURL) + if err != nil { + return link + } + proxyURL, err := url.Parse(h.proxyURL) + if err != nil { + return link + } + + return containerLinkTargetPattern.ReplaceAllStringFunc(link, func(target string) string { + linkURL, err := url.Parse(target[1 : len(target)-1]) + if err != nil || !linkURL.IsAbs() || linkURL.Scheme != upstreamURL.Scheme || linkURL.Host != upstreamURL.Host { + return target + } + linkURL.Scheme = proxyURL.Scheme + linkURL.Host = proxyURL.Host + linkURL.User = proxyURL.User + linkURL.Path = strings.TrimSuffix(proxyURL.Path, "/") + "/v2" + requestPath + linkURL.RawPath = "" + return "<" + linkURL.String() + ">" + }) +} diff --git a/internal/handler/container_test.go b/internal/handler/container_test.go index 672efc2..1d1dfce 100644 --- a/internal/handler/container_test.go +++ b/internal/handler/container_test.go @@ -185,6 +185,69 @@ func TestContainerHandler_TagsListUsesStaleCacheOnUpstreamFailure(t *testing.T) } } +func TestContainerHandler_TagsListCachesPaginationLink(t *testing.T) { + tags := `{"name":"library/nginx","tags":["1.0"]}` + upstreamAvailable := true + upstreamRequests := 0 + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + upstreamRequests++ + if !upstreamAvailable { + http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Link", `<`+upstream.URL+`/v2/library/nginx/tags/list?last=1.0&n=2>; rel="next"`) + _, _ = io.WriteString(w, tags) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = time.Hour + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://proxy.example.test"} + wantLink := `; rel="next"` + + warmRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/tags/list?n=2", nil) + warm := httptest.NewRecorder() + h.Routes().ServeHTTP(warm, warmRequest) + if warm.Code != http.StatusOK { + t.Fatalf("warm status = %d, want 200: %s", warm.Code, warm.Body.String()) + } + if got := warm.Header().Get("Link"); got != wantLink { + t.Errorf("warm Link = %q, want %q", got, wantLink) + } + + fresh := httptest.NewRecorder() + h.Routes().ServeHTTP(fresh, httptest.NewRequest(http.MethodGet, "/library/nginx/tags/list?n=2", nil)) + if fresh.Code != http.StatusOK { + t.Fatalf("fresh status = %d, want 200: %s", fresh.Code, fresh.Body.String()) + } + if got := fresh.Header().Get("Link"); got != wantLink { + t.Errorf("fresh Link = %q, want %q", got, wantLink) + } + if upstreamRequests != 1 { + t.Fatalf("upstream requests after fresh cache hit = %d, want 1", upstreamRequests) + } + + proxy.MetadataTTL = 0 + upstreamAvailable = false + stale := httptest.NewRecorder() + h.Routes().ServeHTTP(stale, httptest.NewRequest(http.MethodGet, "/library/nginx/tags/list?n=2", nil)) + if stale.Code != http.StatusOK { + t.Fatalf("stale status = %d, want 200: %s", stale.Code, stale.Body.String()) + } + if got := stale.Header().Get("Link"); got != wantLink { + t.Errorf("stale Link = %q, want %q", got, wantLink) + } + if got := stale.Header().Get("Warning"); got != `110 - "Response is Stale"` { + t.Errorf("stale Warning = %q, want stale warning", got) + } + if upstreamRequests != 2 { + t.Errorf("upstream requests after stale fallback = %d, want 2", upstreamRequests) + } +} + func TestContainerHandler_NamedOCIRegistryServesHelmArtifacts(t *testing.T) { digest := "sha256:abc123def456abc123def456abc123def456abc123def456abc123def456abcd" manifest := `{"schemaVersion":2,"config":{"mediaType":"application/vnd.cncf.helm.config.v1+json"},"layers":[{"mediaType":"application/vnd.cncf.helm.chart.content.v1.tar+gzip","digest":"` + digest + `"}]}` @@ -838,6 +901,43 @@ func TestContainerHandler_ManifestVariantCacheHonorsSpecificAcceptExclusions(t * } } +func TestContainerHandler_ManifestVariantCacheHonorsParameterizedAcceptExclusions(t *testing.T) { + contentType := "application/vnd.oci.image.index.v1+json; charset=utf-8" + upstreamAvailable := true + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + if !upstreamAvailable { + http.Error(w, "upstream unavailable", http.StatusServiceUnavailable) + return + } + w.Header().Set("Content-Type", contentType) + _, _ = io.WriteString(w, `{"schemaVersion":2}`) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + proxy.MetadataTTL = 0 + h := &ContainerHandler{proxy: proxy, registryURL: upstream.URL, proxyURL: "http://localhost:8080"} + accept := "application/vnd.oci.image.index.v1+json;q=1, application/vnd.oci.image.index.v1+json;charset=utf-8;q=0" + + warmRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + warmRequest.Header.Set("Accept", accept) + warm := httptest.NewRecorder() + h.Routes().ServeHTTP(warm, warmRequest) + if warm.Code != http.StatusOK { + t.Fatalf("warm status = %d, want 200", warm.Code) + } + + upstreamAvailable = false + offlineRequest := httptest.NewRequest(http.MethodGet, "/library/nginx/manifests/latest", nil) + offlineRequest.Header.Set("Accept", accept) + offline := httptest.NewRecorder() + h.Routes().ServeHTTP(offline, offlineRequest) + if offline.Code != http.StatusServiceUnavailable { + t.Errorf("offline status = %d, want 503", offline.Code) + } +} + func TestContainerHandler_ManifestByTag_CachesDigestAlias(t *testing.T) { digest := "sha256:cccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc" manifest := `{"schemaVersion":2,"mediaType":"application/vnd.oci.image.manifest.v1+json"}`