diff --git a/cmd/serve/command.go b/cmd/serve/command.go index 041473c..1f2081c 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, empty to serve tags", + 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..214fe9a 100644 --- a/pkg/handlers/handlers.go +++ b/pkg/handlers/handlers.go @@ -9,6 +9,8 @@ import ( "net/url" "regexp" "strconv" + "sync" + "time" "github.com/google/go-github/v74/github" "github.com/ldez/grignotin/goproxy" @@ -46,19 +48,27 @@ 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 + // 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. -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, + pins: &sync.Map{}, + 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..06ec53b --- /dev/null +++ b/pkg/handlers/pinned.go @@ -0,0 +1,145 @@ +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. +// 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 == "" { + return version, nil + } + + pinned, err := h.pinnedCommit(ctx, moduleName, version) + if errors.Is(err, errNotPinned) { + return version, nil + } + + if err != nil { + 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. + 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 unchanged: + return version, nil + 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) + } +} + +// 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 + } + + 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..40fa321 --- /dev/null +++ b/pkg/handlers/pinned_test.go @@ -0,0 +1,132 @@ +package handlers + +import ( + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "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) + }) + 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", "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"}}`) + }) + } + + 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/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) + }) + + 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: "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"}, + } + + 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) + }) + } +} + +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), "") + + 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) +} 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]