From dcfb9a820df0e692b59f3f1c21a0bc6bcb6866e0 Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Sat, 10 Oct 2026 22:55:09 +0200 Subject: [PATCH 1/2] Enable storage retention for nuget The handler stores NuGet ids lowercased, but purl keeps nuget names as written, so package rules get lowercased before they're matched. Case in a rule doesn't matter. The mirror stores ids as given, so a mixed-case PURL ends up as a copy clients never get. Until that's fixed, the docs ask for lowercase ids in mirror lists. Refs #306 --- docs/configuration.md | 2 +- internal/handler/nuget.go | 19 ++ internal/handler/nuget_test.go | 27 +++ internal/server/retention_nuget_test.go | 250 ++++++++++++++++++++++++ 4 files changed, 297 insertions(+), 1 deletion(-) create mode 100644 internal/server/retention_nuget_test.go diff --git a/docs/configuration.md b/docs/configuration.md index 41ba4d0d..9766f305 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -102,7 +102,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `julia` | no | - | | `maven` | yes | `pkg:maven/com.google.guava/guava`, group and artifact as published, case included. Gradle build cache entries are not covered; they use `gradle.build_cache.max_age`. | | `npm` | yes | `pkg:npm/lodash`, `pkg:npm/@babel/core`; the name has to match exactly, case included, so `pkg:npm/jsonstream` doesn't cover `JSONStream`. | -| `nuget` | no | - | +| `nuget` | yes | `pkg:nuget/Newtonsoft.Json`; case doesn't matter. Give `proxy mirror` lowercase ids (`pkg:nuget/newtonsoft.json@13.0.3`): a mixed-case id is stored as a separate copy that clients are never served, so mirroring it doesn't keep their copy from aging out. | | `oci` | yes | `pkg:oci/library/nginx`, the repository name of the pull. A named upstream keeps its prefix, also when containerd reaches it through `ns`: `ghcr.io/owner/app` pulled that way is `pkg:oci/upstream/ghcr/owner/app`. Homebrew bottles use Homebrew's image name, with `@` written as `/` and `+` as `x` (`openssl@3` is `pkg:oci/homebrew/core/openssl/3`). Helm charts pulled over OCI are stored like images. Only blobs are evicted; manifests stay in the metadata cache. | | `pub` | no | - | | `pypi` | yes | `pkg:pypi/requests`; case and single `_`/`-` don't matter. Dots and repeated separators are kept as written, so a project like `zope.interface` can be stored as `zope.interface` and `zope-interface`; add a rule for each. | diff --git a/internal/handler/nuget.go b/internal/handler/nuget.go index 612f32ee..1a3d0a11 100644 --- a/internal/handler/nuget.go +++ b/internal/handler/nuget.go @@ -6,6 +6,9 @@ import ( "fmt" "net/http" "strings" + + "github.com/git-pkgs/proxy/internal/retention" + "github.com/git-pkgs/purl" ) const ( @@ -13,6 +16,22 @@ const ( nugetSearchUpstream = "https://azuresearch-usnc.nuget.org" ) +// Packages are stored under pkg:nuget/{id} with the id lowercased, since +// NuGet ids are case-insensitive. purl keeps nuget names as written, so +// package rules are lowercased the same way before the default mapping. +func init() { + retention.Default.Register(retention.Spec{Key: "nuget", CanonicalPackage: nugetRetentionKey}) +} + +// nugetRetentionKey maps a package rule to the stored package PURL, so +// pkg:nuget/Newtonsoft.Json matches the stored pkg:nuget/newtonsoft.json. +func nugetRetentionKey(p *purl.PURL) (string, bool) { + if p == nil { + return "", false + } + return retention.DefaultCanonical("nuget")(purl.New(p.Type, p.Namespace, strings.ToLower(p.Name), "", nil)) +} + // NuGetHandler handles NuGet V3 API protocol requests. type NuGetHandler struct { proxy *Proxy diff --git a/internal/handler/nuget_test.go b/internal/handler/nuget_test.go index 710b8eae..5ae77dcf 100644 --- a/internal/handler/nuget_test.go +++ b/internal/handler/nuget_test.go @@ -11,6 +11,7 @@ import ( "time" "github.com/git-pkgs/cooldown" + "github.com/git-pkgs/purl" ) func nugetTestProxy() *Proxy { @@ -1063,3 +1064,29 @@ func TestNuGetHandleRegistrationWithoutCooldown(t *testing.T) { t.Fatalf("status = %d, want %d", w.Code, http.StatusOK) } } + +func TestNuGetRetentionKey(t *testing.T) { + tests := []struct { + key string + want string + ok bool + }{ + {"pkg:nuget/Newtonsoft.Json", "pkg:nuget/newtonsoft.json", true}, + {"pkg:nuget/serilog", "pkg:nuget/serilog", true}, + {"pkg:nuget/serilog/serilog", "", false}, + {"pkg:npm/serilog", "", false}, + } + for _, tt := range tests { + p, err := purl.Parse(tt.key) + if err != nil { + t.Fatalf("Parse(%s): %v", tt.key, err) + } + got, ok := nugetRetentionKey(p) + if ok != tt.ok || (ok && got != tt.want) { + t.Errorf("nugetRetentionKey(%s) = %q, %v, want %q, %v", tt.key, got, ok, tt.want, tt.ok) + } + } + if _, ok := nugetRetentionKey(nil); ok { + t.Error("nugetRetentionKey(nil) accepted") + } +} diff --git a/internal/server/retention_nuget_test.go b/internal/server/retention_nuget_test.go new file mode 100644 index 00000000..44d45bbf --- /dev/null +++ b/internal/server/retention_nuget_test.go @@ -0,0 +1,250 @@ +package server + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/database" + "github.com/git-pkgs/proxy/internal/handler" + "github.com/git-pkgs/proxy/internal/mirror" + "github.com/git-pkgs/proxy/internal/retention" + "github.com/git-pkgs/registries/fetch" +) + +const ( + nugetJSONPath = "/v3-flatcontainer/newtonsoft.json/13.0.3/newtonsoft.json.13.0.3.nupkg" + nugetSerilogPath = "/v3-flatcontainer/serilog/3.1.1/serilog.3.1.1.nupkg" +) + +// nugetRetentionUpstream serves packages the way the NuGet flat container +// does, counting downloads. +func nugetRetentionUpstream(t *testing.T) (*httptest.Server, *atomic.Int64) { + t.Helper() + var downloads atomic.Int64 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case nugetJSONPath, nugetSerilogPath: + downloads.Add(1) + _, _ = io.WriteString(w, "nupkg bytes") + default: + http.NotFound(w, r) + } + })) + t.Cleanup(srv.Close) + return srv, &downloads +} + +func newNuGetRetentionProxy(t *testing.T, upstream *httptest.Server) (*httptest.Server, *database.DB) { + t.Helper() + db, store := setupEvictionTest(t) + // The upstream is a plain-HTTP loopback server; the default fetcher + // refuses loopback, a stock client reaches it. + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + proxy.HTTPClient = upstream.Client() + h := handler.NewNuGetHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL) + srv := httptest.NewServer(h.Routes()) + t.Cleanup(srv.Close) + return srv, db +} + +func nugetGet(t *testing.T, url string) { + t.Helper() + resp, err := http.Get(url) + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + body, _ := io.ReadAll(resp.Body) + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("GET %s = %d %s, want 200", url, resp.StatusCode, body) + } +} + +// nugetArtifact returns the record of the package file named filename. +func nugetArtifact(t *testing.T, db *database.DB, filename string) database.Artifact { + t.Helper() + var art database.Artifact + if err := db.Get(&art, db.Rebind(`SELECT * FROM artifacts WHERE filename = ?`), filename); err != nil { + t.Fatalf("reading artifact %s: %v", filename, err) + } + return art +} + +func nugetBackdate(t *testing.T, db *database.DB, filename string, fetched, accessed time.Time) { + t.Helper() + query := db.Rebind(`UPDATE artifacts SET fetched_at = ?, last_accessed_at = ? WHERE filename = ?`) + if _, err := db.Exec(query, fetched, accessed, filename); err != nil { + t.Fatalf("backdating %s: %v", filename, err) + } +} + +func TestNuGetRegistersForRetention(t *testing.T) { + if _, ok := retention.Default.Lookup("nuget"); !ok { + t.Fatal("nuget is not registered for retention") + } + if key, ok := retention.Default.KeyForDBEcosystem("nuget"); !ok || key != "nuget" { + t.Errorf("KeyForDBEcosystem(nuget) = %q, %v, want nuget, true", key, ok) + } +} + +// A package served from the cache stays while it is used, is evicted once +// its rule runs out, comes back from upstream on the next download, and +// stays after that even with the evicted copy's old access time on the +// record. +func TestNuGetRetentionEvictsAndRefetches(t *testing.T) { + upstream, downloads := nugetRetentionUpstream(t) + srv, db := newNuGetRetentionProxy(t, upstream) + const nupkg = "serilog.3.1.1.nupkg" + + nugetGet(t, srv.URL+nugetSerilogPath) + nugetGet(t, srv.URL+nugetSerilogPath) + if got := downloads.Load(); got != 1 { + t.Fatalf("upstream downloads = %d, want 1", got) + } + if !nugetArtifact(t, db, nupkg).LastAccessedAt.Valid { + t.Fatal("cache hit did not record an access") + } + + rules := retention.Rules{Ecosystems: map[string]time.Duration{"nuget": retentionDay}} + now := time.Now() + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if !nugetArtifact(t, db, nupkg).StoragePath.Valid { + t.Fatal("package used today was evicted") + } + + old := now.Add(-2 * retentionDay) + nugetBackdate(t, db, nupkg, old, old) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, now) + if nugetArtifact(t, db, nupkg).StoragePath.Valid { + t.Fatal("package unused for two days kept under a one-day rule") + } + + nugetGet(t, srv.URL+nugetSerilogPath) + if got := downloads.Load(); got != 2 { + t.Errorf("upstream downloads = %d, want 2 after the eviction", got) + } + art := nugetArtifact(t, db, nupkg) + nugetBackdate(t, db, nupkg, art.FetchedAt.Time, now.Add(-30*retentionDay)) + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + if !nugetArtifact(t, db, nupkg).StoragePath.Valid { + t.Error("refetched package evicted again by the next sweep") + } +} + +// A key with a namespace parses as a nuget PURL but can never match a +// stored package, so it is refused at validation. A plain key is accepted. +func TestNuGetRetentionRefusesNamespacedKeys(t *testing.T) { + cfg := config.Default() + cfg.Storage.Retention.Packages = map[string]string{"pkg:nuget/Serilog": "0"} + if _, err := cfg.RetentionRules(retention.Default); err != nil { + t.Fatalf("RetentionRules(pkg:nuget/Serilog): %v", err) + } + cfg.Storage.Retention.Packages = map[string]string{"pkg:nuget/serilog/serilog": "0"} + _, err := cfg.RetentionRules(retention.Default) + if err == nil || !strings.Contains(err.Error(), "does not match how nuget packages are stored") { + t.Errorf("RetentionRules(pkg:nuget/serilog/serilog) error = %v, want the key refused", err) + } +} + +// The handler stores package ids lowercased, and a package rule reaches the +// package whatever case its id is written in. +func TestNuGetRetentionPackageRules(t *testing.T) { + upstream, _ := nugetRetentionUpstream(t) + srv, db := newNuGetRetentionProxy(t, upstream) + const newtonsoft, serilog = "newtonsoft.json.13.0.3.nupkg", "serilog.3.1.1.nupkg" + + // The id in the URL is upper case here; nuget clients lowercase it, but + // the handler stores it lowercased either way. + nugetGet(t, srv.URL+"/v3-flatcontainer/Newtonsoft.Json/13.0.3/newtonsoft.json.13.0.3.nupkg") + var stored string + art := nugetArtifact(t, db, newtonsoft) + if err := db.Get(&stored, db.Rebind(`SELECT package_purl FROM versions WHERE purl = ?`), art.VersionPURL); err != nil { + t.Fatalf("reading stored package PURL: %v", err) + } + if stored != "pkg:nuget/newtonsoft.json" { + t.Errorf("stored package PURL = %q, want pkg:nuget/newtonsoft.json", stored) + } + + for _, key := range []string{"pkg:nuget/Newtonsoft.Json", "pkg:nuget/newtonsoft.json"} { + nugetGet(t, srv.URL+nugetJSONPath) + nugetGet(t, srv.URL+nugetSerilogPath) + old := time.Now().Add(-30 * retentionDay) + nugetBackdate(t, db, newtonsoft, old, old) + nugetBackdate(t, db, serilog, old, old) + + cfg := config.Default() + cfg.Storage.Retention.Ecosystems = map[string]string{"nuget": "1d"} + cfg.Storage.Retention.Packages = map[string]string{key: "0"} + rules, err := cfg.RetentionRules(retention.Default) + if err != nil { + t.Fatalf("RetentionRules(%s): %v", key, err) + } + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + + if !nugetArtifact(t, db, newtonsoft).StoragePath.Valid { + t.Errorf("with %s: Newtonsoft.Json evicted despite its never-evict rule", key) + } + if nugetArtifact(t, db, serilog).StoragePath.Valid { + t.Errorf("with %s: Serilog kept past the nuget rule", key) + } + } +} + +// nugetRewriteTransport sends every request to the test upstream, so the +// mirror command's fixed api.nuget.org URLs reach it. +type nugetRewriteTransport struct { + target *url.URL + base http.RoundTripper +} + +func (r nugetRewriteTransport) RoundTrip(req *http.Request) (*http.Response, error) { + req = req.Clone(req.Context()) + req.URL.Scheme = r.target.Scheme + req.URL.Host = r.target.Host + req.Host = r.target.Host + return r.base.RoundTrip(req) +} + +// A package preloaded with the mirror command falls under the nuget rule. +func TestNuGetRetentionCoversMirroredPackages(t *testing.T) { + upstream, downloads := nugetRetentionUpstream(t) + target, err := url.Parse(upstream.URL) + if err != nil { + t.Fatal(err) + } + db, store := setupEvictionTest(t) + client := &http.Client{Transport: nugetRewriteTransport{target: target, base: upstream.Client().Transport}} + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(client), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + + m := mirror.New(proxy, db, store, discardLogger(), 1) + progress, err := m.Run(context.Background(), &mirror.PURLSource{PURLs: []string{"pkg:nuget/serilog@3.1.1"}}) + if err != nil || progress.Failed != 0 || downloads.Load() != 1 { + t.Fatalf("mirror run = %+v, %v, downloads %d; want one successful download", progress, err, downloads.Load()) + } + + const nupkg = "serilog.3.1.1.nupkg" + if nugetArtifact(t, db, nupkg).LastAccessedAt.Valid { + t.Fatal("mirrored package has an access recorded before anyone downloaded it") + } + query := db.Rebind(`UPDATE artifacts SET fetched_at = ? WHERE filename = ?`) + if _, err := db.Exec(query, time.Now().Add(-30*retentionDay), nupkg); err != nil { + t.Fatalf("backdating %s: %v", nupkg, err) + } + rules := retention.Rules{Ecosystems: map[string]time.Duration{"nuget": retentionDay}} + sweepRetention(context.Background(), db, discardLogger(), retention.Default, rules, time.Now()) + if nugetArtifact(t, db, nupkg).StoragePath.Valid { + t.Error("never-served mirrored package kept past the nuget rule") + } +} From 5d7855ffc831bd30d6f37b4f8937c6c161eea87f Mon Sep 17 00:00:00 2001 From: Christian Heim Date: Mon, 12 Oct 2026 19:56:38 +0200 Subject: [PATCH 2/2] Store mirrored NuGet ids lowercased The mirror passed ids through as written, so pkg:nuget/Newtonsoft.Json@13.0.3 was stored under a copy no client is served while the handler kept serving its lowercased one. GetOrFetchArtifact now lowercases nuget names, and the docs no longer ask for lowercase ids in mirror lists. Refs #306 --- docs/configuration.md | 2 +- internal/handler/handler.go | 6 ++++ internal/server/retention_nuget_test.go | 37 +++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/docs/configuration.md b/docs/configuration.md index 9766f305..5f39da5b 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -102,7 +102,7 @@ A package rule wins over an ecosystem rule, which wins over `default`. Package r | `julia` | no | - | | `maven` | yes | `pkg:maven/com.google.guava/guava`, group and artifact as published, case included. Gradle build cache entries are not covered; they use `gradle.build_cache.max_age`. | | `npm` | yes | `pkg:npm/lodash`, `pkg:npm/@babel/core`; the name has to match exactly, case included, so `pkg:npm/jsonstream` doesn't cover `JSONStream`. | -| `nuget` | yes | `pkg:nuget/Newtonsoft.Json`; case doesn't matter. Give `proxy mirror` lowercase ids (`pkg:nuget/newtonsoft.json@13.0.3`): a mixed-case id is stored as a separate copy that clients are never served, so mirroring it doesn't keep their copy from aging out. | +| `nuget` | yes | `pkg:nuget/Newtonsoft.Json`; case doesn't matter. | | `oci` | yes | `pkg:oci/library/nginx`, the repository name of the pull. A named upstream keeps its prefix, also when containerd reaches it through `ns`: `ghcr.io/owner/app` pulled that way is `pkg:oci/upstream/ghcr/owner/app`. Homebrew bottles use Homebrew's image name, with `@` written as `/` and `+` as `x` (`openssl@3` is `pkg:oci/homebrew/core/openssl/3`). Helm charts pulled over OCI are stored like images. Only blobs are evicted; manifests stay in the metadata cache. | | `pub` | no | - | | `pypi` | yes | `pkg:pypi/requests`; case and single `_`/`-` don't matter. Dots and repeated separators are kept as written, so a project like `zope.interface` can be stored as `zope.interface` and `zope-interface`; add a rule for each. | diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 036e2a5f..73c4d5dd 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -244,6 +244,12 @@ type CacheResult struct { // GetOrFetchArtifact retrieves an artifact from cache or fetches from upstream. func (p *Proxy) GetOrFetchArtifact(ctx context.Context, ecosystem, name, version, filename string) (*CacheResult, error) { + // NuGet ids are case-insensitive and the NuGet handler stores them + // lowercased, so a mirrored Newtonsoft.Json must land on the same record + // clients are served. + if ecosystem == "nuget" { + name = strings.ToLower(name) + } pkgPURL, versionPURL, err := packagePURLStrings(ecosystem, name, version) if err != nil { return nil, err diff --git a/internal/server/retention_nuget_test.go b/internal/server/retention_nuget_test.go index 44d45bbf..db7daa89 100644 --- a/internal/server/retention_nuget_test.go +++ b/internal/server/retention_nuget_test.go @@ -248,3 +248,40 @@ func TestNuGetRetentionCoversMirroredPackages(t *testing.T) { t.Error("never-served mirrored package kept past the nuget rule") } } + +// Mirroring a mixed-case id fills the record clients are served, so their +// first download comes from the cache. +func TestNuGetMirrorStoresIDsLowercased(t *testing.T) { + upstream, downloads := nugetRetentionUpstream(t) + target, err := url.Parse(upstream.URL) + if err != nil { + t.Fatal(err) + } + db, store := setupEvictionTest(t) + client := &http.Client{Transport: nugetRewriteTransport{target: target, base: upstream.Client().Transport}} + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(client), fetch.WithMaxRetries(0)) + t.Cleanup(func() { _ = fetcher.Close() }) + proxy := handler.NewProxy(db, store, fetcher, fetch.NewResolver(), discardLogger()) + proxy.HTTPClient = upstream.Client() + + m := mirror.New(proxy, db, store, discardLogger(), 1) + progress, err := m.Run(context.Background(), &mirror.PURLSource{PURLs: []string{"pkg:nuget/Serilog@3.1.1"}}) + if err != nil || progress.Failed != 0 || downloads.Load() != 1 { + t.Fatalf("mirror run = %+v, %v, downloads %d; want one successful download", progress, err, downloads.Load()) + } + + h := handler.NewNuGetHandlerWithUpstreams(proxy, "http://proxy.test", upstream.URL, upstream.URL) + srv := httptest.NewServer(h.Routes()) + t.Cleanup(srv.Close) + nugetGet(t, srv.URL+nugetSerilogPath) + if got := downloads.Load(); got != 1 { + t.Errorf("upstream downloads = %d, want 1: the client was not served the mirrored copy", got) + } + var n int + if err := db.Get(&n, `SELECT COUNT(*) FROM artifacts`); err != nil { + t.Fatal(err) + } + if n != 1 { + t.Errorf("artifacts = %d, want 1", n) + } +}