From adabd049eb9b83b8c0e62465d8bc72cd283f20df Mon Sep 17 00:00:00 2001 From: mmatur Date: Mon, 26 Oct 2026 10:24:48 +0100 Subject: [PATCH 1/5] feat: serve plugins with dependencies at the commit pinned by the Go module proxy When a tag moved after publication, download the GitHub zipball at the commit proxy.golang.org pinned for that version instead of the tag. Part of traefik/hub-issues#3124, closes traefik/infra#12142. --- cmd/serve/command.go | 8 +++ cmd/serve/config.go | 2 + cmd/serve/serve.go | 2 +- go.mod | 2 +- pkg/handlers/blacklist_test.go | 10 +-- pkg/handlers/handlers.go | 24 +++++--- pkg/handlers/handlers_test.go | 10 +-- pkg/handlers/module.go | 11 +++- pkg/handlers/pinned.go | 108 +++++++++++++++++++++++++++++++++ pkg/handlers/pinned_test.go | 93 ++++++++++++++++++++++++++++ 10 files changed, 247 insertions(+), 23 deletions(-) create mode 100644 pkg/handlers/pinned.go create mode 100644 pkg/handlers/pinned_test.go diff --git a/cmd/serve/command.go b/cmd/serve/command.go index 041473c..2f4f14c 100644 --- a/cmd/serve/command.go +++ b/cmd/serve/command.go @@ -14,6 +14,7 @@ const ( flagGoProxyURL = "go-proxy-url" flagGoProxyUsername = "go-proxy-username" flagGoProxyPassword = "go-proxy-password" + flagPinProxyURL = "pin-proxy-url" flagTracingAddress = "tracing-address" flagTracingInsecure = "tracing-insecure" @@ -71,6 +72,7 @@ func buildConfig(cliCtx *cli.Context) Config { Username: cliCtx.String(flagGoProxyUsername), Password: cliCtx.String(flagGoProxyPassword), }, + PinProxyURL: cliCtx.String(flagPinProxyURL), } } @@ -94,6 +96,12 @@ func goProxyFlags() []cli.Flag { EnvVars: []string{strcase.ToSNAKE(flagGoProxyPassword)}, Required: true, }, + &cli.StringFlag{ + Name: flagPinProxyURL, + Usage: "Go module proxy pinning the commit each version was published from", + EnvVars: []string{strcase.ToSNAKE(flagPinProxyURL)}, + Value: "https://proxy.golang.org", + }, } } diff --git a/cmd/serve/config.go b/cmd/serve/config.go index 652e7d4..f87a76a 100644 --- a/cmd/serve/config.go +++ b/cmd/serve/config.go @@ -13,6 +13,8 @@ type Config struct { MongoDB mongodb.Config Tracing tracer.Config GoProxy GoProxy + + PinProxyURL string } // GoProxy holds the go-proxy configuration. diff --git a/cmd/serve/serve.go b/cmd/serve/serve.go index 2eb8ac0..cd29bf9 100644 --- a/cmd/serve/serve.go +++ b/cmd/serve/serve.go @@ -46,7 +46,7 @@ func run(ctx context.Context, cfg Config) error { ghClient = newGitHubClient(context.Background(), cfg.GitHubToken) } - handler := handlers.New(store, gpClient, ghClient) + handler := handlers.New(store, gpClient, ghClient, cfg.PinProxyURL) healthChecker := healthcheck.Client{DB: store} diff --git a/go.mod b/go.mod index e81169d..8aea896 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.46.0 go.opentelemetry.io/otel/sdk v1.46.0 go.opentelemetry.io/otel/trace v1.46.0 + golang.org/x/mod v0.40.0 golang.org/x/oauth2 v0.36.0 ) @@ -50,7 +51,6 @@ require ( go.opentelemetry.io/proto/otlp v1.11.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/crypto v0.55.0 // indirect - golang.org/x/mod v0.40.0 // indirect golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect diff --git a/pkg/handlers/blacklist_test.go b/pkg/handlers/blacklist_test.go index b18373f..fa22726 100644 --- a/pkg/handlers/blacklist_test.go +++ b/pkg/handlers/blacklist_test.go @@ -28,7 +28,7 @@ func TestHandlers_ListBlacklist(t *testing.T) { rw := httptest.NewRecorder() req := httptest.NewRequest(http.MethodGet, "/internal/blacklist", http.NoBody) - New(testDB, nil, nil).ListBlacklist(rw, req) + New(testDB, nil, nil, "").ListBlacklist(rw, req) require.Equal(t, http.StatusOK, rw.Code) assert.JSONEq(t, `[ @@ -52,7 +52,7 @@ func TestHandlers_AddToBlacklist(t *testing.T) { body := `{"repository":"deas/teectl","reason":"Not a plugin","author":"alice"}` req := httptest.NewRequest(http.MethodPost, "/internal/blacklist", strings.NewReader(body)) - New(testDB, nil, nil).AddToBlacklist(rw, req) + New(testDB, nil, nil, "").AddToBlacklist(rw, req) require.Equal(t, http.StatusOK, rw.Code) assert.Equal(t, "deas/teectl", got.Repository) @@ -73,7 +73,7 @@ func TestHandlers_AddToBlacklist_invalidRepository(t *testing.T) { body := `{"repository":"` + repository + `"}` req := httptest.NewRequest(http.MethodPost, "/internal/blacklist", strings.NewReader(body)) - New(testDB, nil, nil).AddToBlacklist(rw, req) + New(testDB, nil, nil, "").AddToBlacklist(rw, req) assert.Equalf(t, http.StatusBadRequest, rw.Code, "repository %q", repository) } @@ -94,7 +94,7 @@ func TestHandlers_DeleteFromBlacklist(t *testing.T) { req.SetPathValue("owner", "deas") req.SetPathValue("repo", "teectl") - New(testDB, nil, nil).DeleteFromBlacklist(rw, req) + New(testDB, nil, nil, "").DeleteFromBlacklist(rw, req) require.Equal(t, http.StatusNoContent, rw.Code) assert.Equal(t, "deas/teectl", deleted) @@ -112,7 +112,7 @@ func TestHandlers_DeleteFromBlacklist_notFound(t *testing.T) { req.SetPathValue("owner", "deas") req.SetPathValue("repo", "teectl") - New(testDB, nil, nil).DeleteFromBlacklist(rw, req) + New(testDB, nil, nil, "").DeleteFromBlacklist(rw, req) assert.Equal(t, http.StatusNoContent, rw.Code) } diff --git a/pkg/handlers/handlers.go b/pkg/handlers/handlers.go index 1730d01..c98f8a5 100644 --- a/pkg/handlers/handlers.go +++ b/pkg/handlers/handlers.go @@ -9,6 +9,7 @@ import ( "net/url" "regexp" "strconv" + "time" "github.com/google/go-github/v74/github" "github.com/ldez/grignotin/goproxy" @@ -46,19 +47,24 @@ type Storer interface { // Handlers a set of handlers. type Handlers struct { - store Storer - goProxy *goproxy.Client - gh *github.Client - tracer trace.Tracer + store Storer + goProxy *goproxy.Client + gh *github.Client + pinProxyURL string + httpClient *http.Client + tracer trace.Tracer } // New creates all HTTP handlers. -func New(store Storer, goProxy *goproxy.Client, gh *github.Client) Handlers { +// pinProxyURL is the Go module proxy whose version info pins the commit each version was published from. +func New(store Storer, goProxy *goproxy.Client, gh *github.Client, pinProxyURL string) Handlers { return Handlers{ - store: store, - goProxy: goProxy, - gh: gh, - tracer: otel.GetTracerProvider().Tracer("handler"), + store: store, + goProxy: goProxy, + gh: gh, + pinProxyURL: pinProxyURL, + httpClient: &http.Client{Timeout: 10 * time.Second}, + tracer: otel.GetTracerProvider().Tracer("handler"), } } diff --git a/pkg/handlers/handlers_test.go b/pkg/handlers/handlers_test.go index 267a682..5c6a5e4 100644 --- a/pkg/handlers/handlers_test.go +++ b/pkg/handlers/handlers_test.go @@ -59,7 +59,7 @@ func TestHandlers_List(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/", http.NoBody) - New(testDB, nil, nil).List(rw, req) + New(testDB, nil, nil, "").List(rw, req) assert.Equal(t, http.StatusOK, rw.Code) assert.Equal(t, "next", rw.Header().Get(nextPageHeader)) @@ -98,7 +98,7 @@ func TestHandlers_List_GetByName(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/?name=Demo%20Plugin", http.NoBody) - New(testDB, nil, nil).getByName(rw, req) + New(testDB, nil, nil, "").getByName(rw, req) assert.Equal(t, http.StatusOK, rw.Code) @@ -140,7 +140,7 @@ func TestHandlers_List_GetByName_hidden(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/?name=Demo%20Plugin", http.NoBody) - New(testDB, nil, nil).getByName(rw, req) + New(testDB, nil, nil, "").getByName(rw, req) assert.Equal(t, http.StatusNotFound, rw.Code) @@ -148,7 +148,7 @@ func TestHandlers_List_GetByName_hidden(t *testing.T) { req = httptest.NewRequest(http.MethodGet, "/?name=Demo%20Plugin&filterHidden=true", http.NoBody) - New(testDB, nil, nil).getByName(rw, req) + New(testDB, nil, nil, "").getByName(rw, req) assert.Equal(t, http.StatusOK, rw.Code) @@ -186,7 +186,7 @@ func TestHandlers_List_SearchByName(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/?query=demo", http.NoBody) - New(testDB, nil, nil).getByName(rw, req) + New(testDB, nil, nil, "").getByName(rw, req) assert.Equal(t, http.StatusOK, rw.Code) diff --git a/pkg/handlers/module.go b/pkg/handlers/module.go index 5baf056..73c8b8b 100644 --- a/pkg/handlers/module.go +++ b/pkg/handlers/module.go @@ -483,11 +483,18 @@ func (h Handlers) getArchiveLinkRequest(ctx context.Context, moduleName, version ctx, span := h.tracer.Start(ctx, "handler_getArchiveLinkRequest") defer span.End() - opts := &github.RepositoryContentGetOptions{Ref: version} - owner, repoName := path.Split(strings.TrimPrefix(moduleName, "github.com/")) owner = strings.TrimSuffix(owner, "/") + ref, err := h.archiveRef(ctx, owner, repoName, moduleName, version) + if err != nil { + span.RecordError(err) + + return nil, fmt.Errorf("failed to get archive ref: %w", err) + } + + opts := &github.RepositoryContentGetOptions{Ref: ref} + link, _, err := h.gh.Repositories.GetArchiveLink(ctx, owner, repoName, github.Zipball, opts, 3) if err != nil { span.RecordError(err) diff --git a/pkg/handlers/pinned.go b/pkg/handlers/pinned.go new file mode 100644 index 0000000..603cde8 --- /dev/null +++ b/pkg/handlers/pinned.go @@ -0,0 +1,108 @@ +package handlers + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "strings" + + "github.com/rs/zerolog/log" + "golang.org/x/mod/module" +) + +// errNotPinned the Go module proxy does not know which commit the version was published from. +var errNotPinned = errors.New("no pinned commit") + +// archiveRef returns the ref to download the GitHub zipball of moduleName@version from. +// +// It is the tag while the tag still points at the commit pinned by the Go module proxy at publication, +// and the pinned commit once the tag moved or was deleted. +// Requesting the tag keeps the archive bytes: the zipball of an annotated tag is named after the tag object, not the commit. +func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, version string) (string, error) { + pinned, err := h.pinnedCommit(ctx, moduleName, version) + if errors.Is(err, errNotPinned) { + return version, nil + } + + if err != nil { + return "", err + } + + // A conditional request answered 304 is not counted in the GitHub rate limit. + sha, resp, err := h.gh.Repositories.GetCommitSHA1(ctx, owner, repoName, version, pinned) + + switch { + case resp != nil && resp.StatusCode == http.StatusNotModified: + return version, nil + case err == nil && sha == pinned: + return version, nil + case err == nil, resp != nil && (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity): + log.Info().Str("module_name", moduleName).Str("module_version", version). + Str("tag_commit", sha).Str("pinned_commit", pinned). + Msg("Tag moved after publication, serving the pinned commit") + + return pinned, nil + default: + return "", fmt.Errorf("resolve tag %s: %w", version, err) + } +} + +// pinnedCommit returns the commit the Go module proxy pinned when moduleName@version was published. +func (h Handlers) pinnedCommit(ctx context.Context, moduleName, version string) (string, error) { + escModule, err := module.EscapePath(moduleName) + if err != nil { + return "", err + } + + escVersion, err := module.EscapeVersion(version) + if err != nil { + return "", err + } + + endpoint := fmt.Sprintf("%s/%s/@v/%s.info", strings.TrimSuffix(h.pinProxyURL, "/"), escModule, escVersion) + + //nolint:gosec // G704: the host is the configured Go module proxy. + req, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, http.NoBody) + if err != nil { + return "", err + } + + resp, err := h.httpClient.Do(req) //nolint:gosec // G704: same request. + if err != nil { + return "", fmt.Errorf("get %s: %w", endpoint, err) + } + + defer func() { _ = resp.Body.Close() }() + + switch resp.StatusCode { + case http.StatusOK: + case http.StatusNotFound, http.StatusGone: + return "", errNotPinned + default: + return "", fmt.Errorf("get %s: status %d", endpoint, resp.StatusCode) + } + + body, err := io.ReadAll(resp.Body) + if err != nil { + return "", err + } + + var info struct { + Origin *struct { + Hash string `json:"Hash"` //nolint:tagliatelle // Go module proxy protocol. + } `json:"Origin"` //nolint:tagliatelle // Go module proxy protocol. + } + + if err = json.Unmarshal(body, &info); err != nil { + return "", fmt.Errorf("decode %s: %w", endpoint, err) + } + + if info.Origin == nil || info.Origin.Hash == "" { + return "", errNotPinned + } + + return info.Origin.Hash, nil +} diff --git a/pkg/handlers/pinned_test.go b/pkg/handlers/pinned_test.go new file mode 100644 index 0000000..85d7026 --- /dev/null +++ b/pkg/handlers/pinned_test.go @@ -0,0 +1,93 @@ +package handlers + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/google/go-github/v74/github" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestArchiveRef(t *testing.T) { + proxy := http.NewServeMux() + proxy.HandleFunc("/github.com/o/unknown/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusNotFound) + }) + proxy.HandleFunc("/github.com/o/noorigin/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprint(rw, `{"Version":"v1.0.0"}`) + }) + proxy.HandleFunc("/github.com/o/proxydown/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusBadGateway) + }) + + for _, repo := range []string{"stable", "moved", "deleted", "ghdown"} { + proxy.HandleFunc("/github.com/o/"+repo+"/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprint(rw, `{"Version":"v1.0.0","Origin":{"VCS":"git","Hash":"pinned"}}`) + }) + } + + proxySrv := httptest.NewServer(proxy) + t.Cleanup(proxySrv.Close) + + // Answers like GitHub: 304 when the tag still points at the If-None-Match commit. + commit := func(sha string) http.HandlerFunc { + return func(rw http.ResponseWriter, req *http.Request) { + if req.Header.Get("If-None-Match") == `"`+sha+`"` { + rw.WriteHeader(http.StatusNotModified) + return + } + + _, _ = fmt.Fprint(rw, sha) + } + } + + gh := http.NewServeMux() + gh.HandleFunc("/repos/o/stable/commits/v1.0.0", commit("pinned")) + gh.HandleFunc("/repos/o/moved/commits/v1.0.0", commit("other")) + gh.HandleFunc("/repos/o/deleted/commits/v1.0.0", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusUnprocessableEntity) + }) + gh.HandleFunc("/repos/o/ghdown/commits/v1.0.0", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusServiceUnavailable) + }) + + ghSrv := httptest.NewServer(gh) + t.Cleanup(ghSrv.Close) + + ghClient := github.NewClient(nil) + ghClient.BaseURL, _ = url.Parse(ghSrv.URL + "/") + + h := New(nil, nil, ghClient, proxySrv.URL) + + testCases := []struct { + repo string + ref string + wantErr bool + }{ + {repo: "unknown", ref: "v1.0.0"}, + {repo: "noorigin", ref: "v1.0.0"}, + {repo: "stable", ref: "v1.0.0"}, + {repo: "moved", ref: "pinned"}, + {repo: "deleted", ref: "pinned"}, + {repo: "ghdown", wantErr: true}, + {repo: "proxydown", wantErr: true}, + } + + for _, test := range testCases { + t.Run(test.repo, func(t *testing.T) { + ref, err := h.archiveRef(t.Context(), "o", test.repo, "github.com/o/"+test.repo, "v1.0.0") + + if test.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, test.ref, ref) + }) + } +} From 46da4c8468695d5780cf8c734271d93aa10bbcbb Mon Sep 17 00:00:00 2001 From: Michael Date: Mon, 26 Oct 2026 12:11:49 +0100 Subject: [PATCH 2/5] Update pkg/handlers/pinned.go Co-authored-by: aikido-pr-checks[bot] <169896070+aikido-pr-checks[bot]@users.noreply.github.com> --- pkg/handlers/pinned.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/handlers/pinned.go b/pkg/handlers/pinned.go index 603cde8..51574ef 100644 --- a/pkg/handlers/pinned.go +++ b/pkg/handlers/pinned.go @@ -39,7 +39,9 @@ func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, v return version, nil case err == nil && sha == pinned: return version, nil - case err == nil, resp != nil && (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity): + case err == nil: + fallthrough + case resp != nil && (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity): log.Info().Str("module_name", moduleName).Str("module_version", version). Str("tag_commit", sha).Str("pinned_commit", pinned). Msg("Tag moved after publication, serving the pinned commit") From ac8eadf5077e165d7a784df95da5da09c9b1e1e2 Mon Sep 17 00:00:00 2001 From: mmatur Date: Mon, 26 Oct 2026 12:16:48 +0100 Subject: [PATCH 3/5] fix: name the archive ref cases instead of falling through --- pkg/handlers/pinned.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/handlers/pinned.go b/pkg/handlers/pinned.go index 51574ef..f5e29a6 100644 --- a/pkg/handlers/pinned.go +++ b/pkg/handlers/pinned.go @@ -34,14 +34,14 @@ func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, v // A conditional request answered 304 is not counted in the GitHub rate limit. sha, resp, err := h.gh.Repositories.GetCommitSHA1(ctx, owner, repoName, version, pinned) + unchanged := (resp != nil && resp.StatusCode == http.StatusNotModified) || (err == nil && sha == pinned) + moved := err == nil + deleted := resp != nil && (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity) + switch { - case resp != nil && resp.StatusCode == http.StatusNotModified: - return version, nil - case err == nil && sha == pinned: + case unchanged: return version, nil - case err == nil: - fallthrough - case resp != nil && (resp.StatusCode == http.StatusNotFound || resp.StatusCode == http.StatusUnprocessableEntity): + case moved, deleted: log.Info().Str("module_name", moduleName).Str("module_version", version). Str("tag_commit", sha).Str("pinned_commit", pinned). Msg("Tag moved after publication, serving the pinned commit") From 39bba2da01c426866271e050d7aa3cc968ff6e32 Mon Sep 17 00:00:00 2001 From: mmatur Date: Mon, 26 Oct 2026 15:07:44 +0100 Subject: [PATCH 4/5] fix: serve the tag when the Go module proxy does not answer --- cmd/serve/command.go | 2 +- pkg/handlers/pinned.go | 19 +++++++++++++++++-- pkg/handlers/pinned_test.go | 12 +++++++++++- 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/cmd/serve/command.go b/cmd/serve/command.go index 2f4f14c..1f2081c 100644 --- a/cmd/serve/command.go +++ b/cmd/serve/command.go @@ -98,7 +98,7 @@ func goProxyFlags() []cli.Flag { }, &cli.StringFlag{ Name: flagPinProxyURL, - Usage: "Go module proxy pinning the commit each version was published from", + Usage: "Go module proxy pinning the commit each version was published from, empty to serve tags", EnvVars: []string{strcase.ToSNAKE(flagPinProxyURL)}, Value: "https://proxy.golang.org", }, diff --git a/pkg/handlers/pinned.go b/pkg/handlers/pinned.go index f5e29a6..7d1d174 100644 --- a/pkg/handlers/pinned.go +++ b/pkg/handlers/pinned.go @@ -21,14 +21,23 @@ var errNotPinned = errors.New("no pinned commit") // It is the tag while the tag still points at the commit pinned by the Go module proxy at publication, // and the pinned commit once the tag moved or was deleted. // Requesting the tag keeps the archive bytes: the zipball of an annotated tag is named after the tag object, not the commit. +// The pinned commit of a moved annotated tag cannot give those bytes back, its hash is realigned instead. +// Without an answer from the Go module proxy, it falls back to the tag, as before pinning. func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, version string) (string, error) { + if h.pinProxyURL == "" { + return version, nil + } + pinned, err := h.pinnedCommit(ctx, moduleName, version) if errors.Is(err, errNotPinned) { return version, nil } if err != nil { - return "", err + log.Warn().Err(err).Str("module_name", moduleName).Str("module_version", version). + Msg("Failed to get the pinned commit, serving the tag") + + return version, nil } // A conditional request answered 304 is not counted in the GitHub rate limit. @@ -41,11 +50,17 @@ func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, v switch { case unchanged: return version, nil - case moved, deleted: + case moved: log.Info().Str("module_name", moduleName).Str("module_version", version). Str("tag_commit", sha).Str("pinned_commit", pinned). Msg("Tag moved after publication, serving the pinned commit") + return pinned, nil + case deleted: + log.Info().Str("module_name", moduleName).Str("module_version", version). + Str("pinned_commit", pinned).Int("status", resp.StatusCode). + Msg("Tag not found, serving the pinned commit") + return pinned, nil default: return "", fmt.Errorf("resolve tag %s: %w", version, err) diff --git a/pkg/handlers/pinned_test.go b/pkg/handlers/pinned_test.go index 85d7026..39f9dfa 100644 --- a/pkg/handlers/pinned_test.go +++ b/pkg/handlers/pinned_test.go @@ -74,7 +74,8 @@ func TestArchiveRef(t *testing.T) { {repo: "moved", ref: "pinned"}, {repo: "deleted", ref: "pinned"}, {repo: "ghdown", wantErr: true}, - {repo: "proxydown", wantErr: true}, + {repo: "proxydown", ref: "v1.0.0"}, + {repo: "Bad!Path", ref: "v1.0.0"}, } for _, test := range testCases { @@ -91,3 +92,12 @@ func TestArchiveRef(t *testing.T) { }) } } + +func TestArchiveRef_noPinProxy(t *testing.T) { + h := New(nil, nil, github.NewClient(nil), "") + + ref, err := h.archiveRef(t.Context(), "o", "moved", "github.com/o/moved", "v1.0.0") + require.NoError(t, err) + + assert.Equal(t, "v1.0.0", ref) +} From 801d327ec4cf75a4ed456d7cccb7ba49c0514bf3 Mon Sep 17 00:00:00 2001 From: mmatur Date: Mon, 26 Oct 2026 16:07:28 +0100 Subject: [PATCH 5/5] fix: cache the pinned commit, document --pin-proxy-url --- pkg/handlers/handlers.go | 8 ++++++-- pkg/handlers/pinned.go | 22 +++++++++++++++++++++- pkg/handlers/pinned_test.go | 31 ++++++++++++++++++++++++++++++- readme.md | 1 + 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/pkg/handlers/handlers.go b/pkg/handlers/handlers.go index c98f8a5..214fe9a 100644 --- a/pkg/handlers/handlers.go +++ b/pkg/handlers/handlers.go @@ -9,6 +9,7 @@ import ( "net/url" "regexp" "strconv" + "sync" "time" "github.com/google/go-github/v74/github" @@ -51,8 +52,10 @@ type Handlers struct { goProxy *goproxy.Client gh *github.Client pinProxyURL string - httpClient *http.Client - tracer trace.Tracer + // pins caches the pinned commit by module@version: the proxy never changes it. + pins *sync.Map + httpClient *http.Client + tracer trace.Tracer } // New creates all HTTP handlers. @@ -63,6 +66,7 @@ func New(store Storer, goProxy *goproxy.Client, gh *github.Client, pinProxyURL s goProxy: goProxy, gh: gh, pinProxyURL: pinProxyURL, + pins: &sync.Map{}, httpClient: &http.Client{Timeout: 10 * time.Second}, tracer: otel.GetTracerProvider().Tracer("handler"), } diff --git a/pkg/handlers/pinned.go b/pkg/handlers/pinned.go index 7d1d174..06ec53b 100644 --- a/pkg/handlers/pinned.go +++ b/pkg/handlers/pinned.go @@ -21,7 +21,8 @@ var errNotPinned = errors.New("no pinned commit") // It is the tag while the tag still points at the commit pinned by the Go module proxy at publication, // and the pinned commit once the tag moved or was deleted. // Requesting the tag keeps the archive bytes: the zipball of an annotated tag is named after the tag object, not the commit. -// The pinned commit of a moved annotated tag cannot give those bytes back, its hash is realigned instead. +// The pinned commit of a moved annotated tag cannot give those bytes back, nor match a hash recorded after the move: +// those hashes are realigned with `piceus realign --only`, reviewed one by one. // Without an answer from the Go module proxy, it falls back to the tag, as before pinning. func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, version string) (string, error) { if h.pinProxyURL == "" { @@ -69,6 +70,25 @@ func (h Handlers) archiveRef(ctx context.Context, owner, repoName, moduleName, v // pinnedCommit returns the commit the Go module proxy pinned when moduleName@version was published. func (h Handlers) pinnedCommit(ctx context.Context, moduleName, version string) (string, error) { + key := moduleName + "@" + version + + if cached, ok := h.pins.Load(key); ok { + if pinned, ok := cached.(string); ok { + return pinned, nil + } + } + + pinned, err := h.fetchPinnedCommit(ctx, moduleName, version) + if err != nil { + return "", err + } + + h.pins.Store(key, pinned) + + return pinned, nil +} + +func (h Handlers) fetchPinnedCommit(ctx context.Context, moduleName, version string) (string, error) { escModule, err := module.EscapePath(moduleName) if err != nil { return "", err diff --git a/pkg/handlers/pinned_test.go b/pkg/handlers/pinned_test.go index 39f9dfa..40fa321 100644 --- a/pkg/handlers/pinned_test.go +++ b/pkg/handlers/pinned_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "sync/atomic" "testing" "github.com/google/go-github/v74/github" @@ -23,8 +24,11 @@ func TestArchiveRef(t *testing.T) { proxy.HandleFunc("/github.com/o/proxydown/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { rw.WriteHeader(http.StatusBadGateway) }) + proxy.HandleFunc("/github.com/o/proxygone/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusGone) + }) - for _, repo := range []string{"stable", "moved", "deleted", "ghdown"} { + for _, repo := range []string{"stable", "moved", "deleted", "notfound", "ghdown"} { proxy.HandleFunc("/github.com/o/"+repo+"/@v/v1.0.0.info", func(rw http.ResponseWriter, _ *http.Request) { _, _ = fmt.Fprint(rw, `{"Version":"v1.0.0","Origin":{"VCS":"git","Hash":"pinned"}}`) }) @@ -51,6 +55,9 @@ func TestArchiveRef(t *testing.T) { gh.HandleFunc("/repos/o/deleted/commits/v1.0.0", func(rw http.ResponseWriter, _ *http.Request) { rw.WriteHeader(http.StatusUnprocessableEntity) }) + gh.HandleFunc("/repos/o/notfound/commits/v1.0.0", func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(http.StatusNotFound) + }) gh.HandleFunc("/repos/o/ghdown/commits/v1.0.0", func(rw http.ResponseWriter, _ *http.Request) { rw.WriteHeader(http.StatusServiceUnavailable) }) @@ -73,8 +80,10 @@ func TestArchiveRef(t *testing.T) { {repo: "stable", ref: "v1.0.0"}, {repo: "moved", ref: "pinned"}, {repo: "deleted", ref: "pinned"}, + {repo: "notfound", ref: "pinned"}, {repo: "ghdown", wantErr: true}, {repo: "proxydown", ref: "v1.0.0"}, + {repo: "proxygone", ref: "v1.0.0"}, {repo: "Bad!Path", ref: "v1.0.0"}, } @@ -93,6 +102,26 @@ func TestArchiveRef(t *testing.T) { } } +func TestPinnedCommit_cached(t *testing.T) { + var calls atomic.Int32 + + proxySrv := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, _ *http.Request) { + calls.Add(1) + _, _ = fmt.Fprint(rw, `{"Version":"v1.0.0","Origin":{"VCS":"git","Hash":"pinned"}}`) + })) + t.Cleanup(proxySrv.Close) + + h := New(nil, nil, nil, proxySrv.URL) + + for range 2 { + pinned, err := h.pinnedCommit(t.Context(), "github.com/o/r", "v1.0.0") + require.NoError(t, err) + assert.Equal(t, "pinned", pinned) + } + + assert.Equal(t, int32(1), calls.Load()) +} + func TestArchiveRef_noPinProxy(t *testing.T) { h := New(nil, nil, github.NewClient(nil), "") diff --git a/readme.md b/readme.md index bc76501..ba8055a 100644 --- a/readme.md +++ b/readme.md @@ -16,6 +16,7 @@ OPTIONS: --go-proxy-url value Go Proxy URL [$GO_PROXY_URL] --go-proxy-username value Go Proxy Username [$GO_PROXY_USERNAME] --go-proxy-password value Go Proxy Password [$GO_PROXY_PASSWORD] + --pin-proxy-url value Go module proxy pinning the commit each version was published from, empty to serve tags (default: "https://proxy.golang.org") [$PIN_PROXY_URL] --tracing-address value Address to send traces (default: "jaeger.jaeger.svc.cluster.local:4318") [$TRACING_ADDRESS] --tracing-insecure use HTTP instead of HTTPS (default: true) [$TRACING_INSECURE] --tracing-username value Username to connect to Jaeger (default: "jaeger") [$TRACING_USERNAME]