internal,proxy, all: replace proxy.DefaultClient with NewDefaultClient()

Instead of exposing a global public variable, require that users of the proxy package instantiate their own proxy client. This makes the package harder to misuse, and makes it clearer where we are using a real client vs. a test client. Change-Id: I2f0f5895065e6efec1d50b3ac34cb04847e3b002 Reviewed-on: https://go-review.googlesource.com/c/vulndb/+/524457 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
golang · Sep 11, 2023 · 44ab8d2 · 44ab8d2
1 parent a435f71
commit 44ab8d2
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 30 deletions.
diff --git a/all_test.go b/all_test.go
@@ -77,7 +77,7 @@ func TestLintReports(t *testing.T) {
 			return r.LintOffline()
 		}
 	} else {
-		pc := proxy.DefaultClient
+		pc := proxy.NewDefaultClient()
 		lint = func(r *report.Report) []string {
 			return r.Lint(pc)
 		}

diff --git a/cmd/issue/main.go b/cmd/issue/main.go
@@ -53,11 +53,12 @@ func main() {
 	}
 	c := issues.NewClient(ctx, &issues.Config{Owner: owner, Repo: repoName, Token: *githubToken})
 	ghsaClient := ghsa.NewClient(ctx, *githubToken)
+	pc := proxy.NewDefaultClient()
 	switch cmd {
 	case "triage":
-		err = createIssueToTriage(ctx, c, ghsaClient, filename)
+		err = createIssueToTriage(ctx, c, ghsaClient, pc, filename)
 	case "excluded":
-		err = createExcluded(ctx, c, ghsaClient, filename)
+		err = createExcluded(ctx, c, ghsaClient, pc, filename)
 	default:
 		err = fmt.Errorf("unsupported command: %q", cmd)
 	}
@@ -66,33 +67,33 @@ func main() {
 	}
 }
 
-func createIssueToTriage(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, filename string) (err error) {
+func createIssueToTriage(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, pc *proxy.Client, filename string) (err error) {
 	aliases, err := parseAliases(filename)
 	if err != nil {
 		return err
 	}
 	for _, alias := range aliases {
-		if err := constructIssue(ctx, c, ghsaClient, alias, []string{"NeedsTriage"}); err != nil {
+		if err := constructIssue(ctx, c, ghsaClient, pc, alias, []string{"NeedsTriage"}); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func createExcluded(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, filename string) (err error) {
+func createExcluded(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, pc *proxy.Client, filename string) (err error) {
 	records, err := parseExcluded(filename)
 	if err != nil {
 		return err
 	}
 	for _, r := range records {
-		if err := constructIssue(ctx, c, ghsaClient, r.identifier, []string{fmt.Sprintf("excluded: %s", r.category)}); err != nil {
+		if err := constructIssue(ctx, c, ghsaClient, pc, r.identifier, []string{fmt.Sprintf("excluded: %s", r.category)}); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func constructIssue(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, alias string, labels []string) (err error) {
+func constructIssue(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Client, pc *proxy.Client, alias string, labels []string) (err error) {
 	var ghsas []*ghsa.SecurityAdvisory
 	if strings.HasPrefix(alias, "GHSA") {
 		sa, err := ghsaClient.FetchGHSA(ctx, alias)
@@ -132,7 +133,6 @@ func constructIssue(ctx context.Context, c *issues.Client, ghsaClient *ghsa.Clie
 	if err != nil {
 		return err
 	}
-	pc := proxy.DefaultClient
 	for _, sa := range ghsas {
 		for _, id := range sa.Identifiers {
 			ids = append(ids, id.Value)

diff --git a/cmd/vulnreport/main.go b/cmd/vulnreport/main.go
@@ -153,11 +153,11 @@ func main() {
 	}
 
 	ghsaClient := ghsa.NewClient(ctx, *githubToken)
-	pc := proxy.DefaultClient
+	pc := proxy.NewDefaultClient()
 	var cmdFunc func(context.Context, string) error
 	switch cmd {
 	case "lint":
-		cmdFunc = lint
+		cmdFunc = func(ctx context.Context, name string) error { return lint(ctx, name, pc) }
 	case "commit":
 		cmdFunc = func(ctx context.Context, name string) error { return commit(ctx, name, ghsaClient, pc, *force) }
 	case "cve":
@@ -321,7 +321,7 @@ func setupCreate(ctx context.Context, args []string) ([]int, *createCfg, error)
 	return githubIDs, &createCfg{
 		issuesClient:    issues.NewClient(ctx, &issues.Config{Owner: owner, Repo: repoName, Token: *githubToken}),
 		ghsaClient:      ghsa.NewClient(ctx, *githubToken),
-		proxyClient:     proxy.DefaultClient,
+		proxyClient:     proxy.NewDefaultClient(),
 		existingByFile:  existingByFile,
 		existingByIssue: existingByIssue,
 		allowClosed:     *closedOk,
@@ -716,11 +716,11 @@ func addReferenceTODOs(r *report.Report) {
 	}
 }
 
-func lint(_ context.Context, filename string) (err error) {
+func lint(_ context.Context, filename string, pc *proxy.Client) (err error) {
 	defer derrors.Wrap(&err, "lint(%q)", filename)
 	infolog.Printf("lint %s\n", filename)
 
-	_, err = report.ReadAndLint(filename, proxy.DefaultClient)
+	_, err = report.ReadAndLint(filename, pc)
 	return err
 }
 

diff --git a/cmd/worker/main.go b/cmd/worker/main.go
@@ -263,7 +263,7 @@ func createIssuesCommand(ctx context.Context) error {
 	if err != nil {
 		return err
 	}
-	pc := proxy.DefaultClient
+	pc := proxy.NewDefaultClient()
 	return worker.CreateIssues(ctx, cfg.Store, client, pc, allReports, *limit)
 }
 

diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go
@@ -21,12 +21,6 @@ import (
 	"golang.org/x/vulndb/internal/version"
 )
 
-// TODO(https://go.dev/issues/60275): Replace this with a function that
-// returns a new instance of a default client.
-var DefaultClient *Client
-
-const ProxyURL = "https://proxy.golang.org"
-
 // Client is a client for reading from the proxy.
 //
 // It uses a simple in-memory cache that does not expire,
@@ -41,14 +35,6 @@ type Client struct {
 	errLog *errLog // for testing
 }
 
-func init() {
-	proxyURL := ProxyURL
-	if proxy, ok := os.LookupEnv("GOPROXY"); ok {
-		proxyURL = proxy
-	}
-	DefaultClient = NewClient(http.DefaultClient, proxyURL)
-}
-
 func NewClient(c *http.Client, url string) *Client {
 	return &Client{
 		Client: c,
@@ -58,6 +44,16 @@ func NewClient(c *http.Client, url string) *Client {
 	}
 }
 
+const ProxyURL = "https://proxy.golang.org"
+
+func NewDefaultClient() *Client {
+	proxyURL := ProxyURL
+	if proxy, ok := os.LookupEnv("GOPROXY"); ok {
+		proxyURL = proxy
+	}
+	return NewClient(http.DefaultClient, proxyURL)
+}
+
 func (c *Client) lookup(urlSuffix string) ([]byte, error) {
 	url := fmt.Sprintf("%s/%s", c.url, urlSuffix)
 	if b, found := c.cache.get(urlSuffix); found {

diff --git a/internal/worker/server.go b/internal/worker/server.go
@@ -99,7 +99,7 @@ func NewServer(ctx context.Context, cfg Config) (_ *Server, err error) {
 		log.Infof(ctx, "issue creation disabled")
 	}
 
-	s.proxyClient = proxy.DefaultClient
+	s.proxyClient = proxy.NewDefaultClient()
 
 	s.indexTemplate, err = parseTemplate(staticPath, template.TrustedSourceFromConstant("index.tmpl"))
 	if err != nil {