From 01692142a70ed1dc5d3c2ea5a3f37802d1d51a39 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Thu, 22 Oct 2026 20:42:17 +0200 Subject: [PATCH] Cache conda packages per subdir Conda download caching keyed artifacts on channel, name, version and filename only, but the same filename can hold different bytes in each subdir (python_abi-3.11-4_cp311.conda in linux-64 and osx-arm64, for example). A client on one platform could be served another platform's package. The subdir is now part of the cached filename, so each one gets its own entry. Entries cached under the old key are no longer used and are re-fetched on first request. --- internal/handler/conda.go | 5 +- internal/handler/conda_subdir_test.go | 61 +++++++++++++++++++++++++ internal/handler/download_test.go | 4 +- internal/server/retention_conda_test.go | 6 +-- 4 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 internal/handler/conda_subdir_test.go diff --git a/internal/handler/conda.go b/internal/handler/conda.go index aecd01a4..7f235e17 100644 --- a/internal/handler/conda.go +++ b/internal/handler/conda.go @@ -113,8 +113,11 @@ func (h *CondaHandler) handleDownload(w http.ResponseWriter, r *http.Request) { "channel", channel, "arch", arch, "name", name, "version", version) upstreamURL := h.upstreamURL + r.URL.Path + // The same filename can hold different bytes in each subdir, so the + // subdir is part of the cache key. + cacheFilename := arch + "/" + filename - result, err := h.proxy.GetOrFetchArtifactFromURL(r.Context(), "conda", packageName, version, filename, upstreamURL) + result, err := h.proxy.GetOrFetchArtifactFromURL(r.Context(), "conda", packageName, version, cacheFilename, upstreamURL) if err != nil { h.proxy.serveArtifactError(w, err, "failed to fetch package") return diff --git a/internal/handler/conda_subdir_test.go b/internal/handler/conda_subdir_test.go new file mode 100644 index 00000000..1a81e48a --- /dev/null +++ b/internal/handler/conda_subdir_test.go @@ -0,0 +1,61 @@ +package handler + +import ( + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/git-pkgs/registries/fetch" +) + +// The same conda filename can hold different bytes in each subdir, so each +// subdir must get its own cache entry. +func TestCondaDownloadCachesPerSubdir(t *testing.T) { + const filename = "python_abi-3.11-4_cp311.conda" + packages := map[string]string{ + "/conda-forge/linux-64/" + filename: "linux-64 bytes", + "/conda-forge/osx-arm64/" + filename: "osx-arm64 bytes", + } + + var available atomic.Bool + available.Store(true) + var requests atomic.Int32 + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + data, ok := packages[r.URL.Path] + if !available.Load() || !ok { + http.NotFound(w, r) + return + } + requests.Add(1) + _, _ = w.Write([]byte(data)) + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + fetcher := fetch.NewFetcher(fetch.WithHTTPClient(upstream.Client()), fetch.WithMaxRetries(0)) + proxy.Fetcher = fetcher + t.Cleanup(func() { _ = fetcher.Close() }) + routes := NewCondaHandlerWithUpstream(proxy, "http://proxy.local", upstream.URL).Routes() + + get := func(path string) string { + w := httptest.NewRecorder() + routes.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + if w.Code != http.StatusOK { + t.Fatalf("%s: status = %d: %s", path, w.Code, w.Body.String()) + } + return w.Body.String() + } + + for _, round := range []string{"fetch", "cached"} { + for path, want := range packages { + if got := get(path); got != want { + t.Errorf("%s %s: body = %q, want %q", round, path, got, want) + } + } + available.Store(false) + } + if got := requests.Load(); got != 2 { + t.Errorf("upstream requests = %d, want 2 (one per subdir)", got) + } +} diff --git a/internal/handler/download_test.go b/internal/handler/download_test.go index ce447752..51442123 100644 --- a/internal/handler/download_test.go +++ b/internal/handler/download_test.go @@ -488,7 +488,7 @@ func TestPubHandler_CacheMiss(t *testing.T) { func TestCondaHandler_DownloadCacheHit(t *testing.T) { proxy, db, store, _ := setupTestProxy(t) - seedPackageWithPURL(t, db, store, "conda", "main/numpy", "1.24.0", "numpy-1.24.0-py311h64a7726_0.conda", "conda pkg") + seedPackageWithPURL(t, db, store, "conda", "main/numpy", "1.24.0", "linux-64/numpy-1.24.0-py311h64a7726_0.conda", "conda pkg") h := NewCondaHandler(proxy, "http://localhost") srv := httptest.NewServer(h.Routes()) @@ -511,7 +511,7 @@ func TestCondaHandler_DownloadCacheHit(t *testing.T) { func TestCondaHandler_DownloadTarBz2CacheHit(t *testing.T) { proxy, db, store, _ := setupTestProxy(t) - seedPackageWithPURL(t, db, store, "conda", "main/scipy", "1.11.0", "scipy-1.11.0-py311hb2e3ea1_0.tar.bz2", "tar bz2 data") + seedPackageWithPURL(t, db, store, "conda", "main/scipy", "1.11.0", "linux-64/scipy-1.11.0-py311hb2e3ea1_0.tar.bz2", "tar bz2 data") h := NewCondaHandler(proxy, "http://localhost") srv := httptest.NewServer(h.Routes()) diff --git a/internal/server/retention_conda_test.go b/internal/server/retention_conda_test.go index 230ecbb7..d687e22f 100644 --- a/internal/server/retention_conda_test.go +++ b/internal/server/retention_conda_test.go @@ -69,7 +69,7 @@ func condaGet(t *testing.T, url string) { } } -// condaArtifact returns the record of the package file named filename. +// condaArtifact returns the record cached as filename, which is {subdir}/{file}. func condaArtifact(t *testing.T, db *database.DB, filename string) database.Artifact { t.Helper() var art database.Artifact @@ -103,7 +103,7 @@ func TestCondaRegistersForRetention(t *testing.T) { func TestCondaRetentionEvictsAndRefetches(t *testing.T) { upstream, downloads := condaRetentionUpstream(t) srv, db := newCondaRetentionProxy(t, upstream) - const file = "requests-2.31.0-pyhd8ed1ab_0.conda" + const file = "noarch/requests-2.31.0-pyhd8ed1ab_0.conda" condaGet(t, srv.URL+condaRequestsPath) condaGet(t, srv.URL+condaRequestsPath) @@ -162,7 +162,7 @@ func TestCondaRetentionRefusesKeysWithoutChannel(t *testing.T) { func TestCondaRetentionPackageRule(t *testing.T) { upstream, _ := condaRetentionUpstream(t) srv, db := newCondaRetentionProxy(t, upstream) - const forgeNumpy, anacondaNumpy, requests = "numpy-1.24.0-py311h64a7726_0.conda", "numpy-1.24.0-py311h08b1b3b_0.conda", "requests-2.31.0-pyhd8ed1ab_0.conda" + const forgeNumpy, anacondaNumpy, requests = "linux-64/numpy-1.24.0-py311h64a7726_0.conda", "linux-64/numpy-1.24.0-py311h08b1b3b_0.conda", "noarch/requests-2.31.0-pyhd8ed1ab_0.conda" condaGet(t, srv.URL+condaForgeNumpyPath) condaGet(t, srv.URL+condaAnacondaNumpy) condaGet(t, srv.URL+condaRequestsPath)