internal/scan: move request to worker
scan.Request was specific to vulncheck, so move it to vulncheck_scan.go
in internal/worker.
Change-Id: I7e7f2bb4d9fb933368cd332b4211f89aeea78122
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/467296
Run-TryBot: Jonathan Amsterdam <jba@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Julie Qiu <julieqiu@google.com>
diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go
index 5b91c65..a7cbf48 100644
--- a/internal/queue/queue_test.go
+++ b/internal/queue/queue_test.go
@@ -9,7 +9,6 @@
"github.com/google/go-cmp/cmp"
"golang.org/x/pkgsite-metrics/internal/config"
- "golang.org/x/pkgsite-metrics/internal/scan"
taskspb "google.golang.org/genproto/googleapis/cloud/tasks/v2"
"google.golang.org/protobuf/testing/protocmp"
"google.golang.org/protobuf/types/known/durationpb"
@@ -79,16 +78,10 @@
Namespace: "test",
TaskNameSuffix: "suf",
}
- sreq := &scan.Request{
- ModuleURLPath: scan.ModuleURLPath{
- Module: "mod",
- Version: "v1.2.3",
- },
- RequestParams: scan.RequestParams{
- ImportedBy: 0,
- Mode: "test",
- Insecure: true,
- },
+ sreq := &testTask{
+ name: "name",
+ path: "mod@v1.2.3",
+ params: "importedby=0&mode=test&insecure=true",
}
got, err := gcp.newTaskRequest(sreq, opts)
if err != nil {
diff --git a/internal/scan/parse.go b/internal/scan/parse.go
index 8b27e73..d0e29f8 100644
--- a/internal/scan/parse.go
+++ b/internal/scan/parse.go
@@ -7,7 +7,6 @@
import (
"bufio"
- "errors"
"fmt"
"net/http"
"os"
@@ -19,65 +18,6 @@
"golang.org/x/pkgsite-metrics/internal/version"
)
-// Request contains information passed
-// to a scan endpoint.
-type Request struct {
- ModuleURLPath
- RequestParams
-}
-
-// RequestParams has query parameters for a scan request.
-type RequestParams struct {
- ImportedBy int
- Mode string
- Insecure bool
-}
-
-// These methods implement queue.Task.
-func (r *Request) Name() string { return r.Module + "@" + r.Version }
-
-func (r *Request) Path() string {
- p := r.Module + "@" + r.Version
- if r.Suffix != "" {
- p += "/" + r.Suffix
- }
- return p
-}
-
-func (r *Request) Params() string {
- return FormatParams(r.RequestParams)
-}
-
-// ParseRequest parses an http request r for an endpoint
-// scanPrefix and produces a corresponding ScanRequest.
-//
-// The module and version should have one of the following three forms:
-// - <module>/@v/<version>
-// - <module>@<version>
-// - <module>/@latest
-//
-// (These are the same forms that the module proxy accepts.)
-func ParseRequest(r *http.Request, scanPrefix string) (_ *Request, err error) {
- defer derrors.Wrap(&err, "ParseRequest(%s)", scanPrefix)
-
- mp, err := ParseModuleURLPath(strings.TrimPrefix(r.URL.Path, scanPrefix))
- if err != nil {
- return nil, err
- }
-
- rp := RequestParams{ImportedBy: -1} // use -1 to detect missing param (explicit 0 is OK)
- if err := ParseParams(r, &rp); err != nil {
- return nil, err
- }
- if rp.ImportedBy < 0 {
- return nil, errors.New(`missing or negative "importedby" query param`)
- }
- return &Request{
- ModuleURLPath: mp,
- RequestParams: rp,
- }, nil
-}
-
func ParseRequiredIntParam(r *http.Request, name string) (int, error) {
value := r.FormValue(name)
if value == "" {
diff --git a/internal/scan/parse_test.go b/internal/scan/parse_test.go
index f25967a..253f61c 100644
--- a/internal/scan/parse_test.go
+++ b/internal/scan/parse_test.go
@@ -6,7 +6,6 @@
import (
"net/http"
- "net/url"
"reflect"
"strings"
"testing"
@@ -15,113 +14,78 @@
"golang.org/x/pkgsite-metrics/internal/version"
)
-func TestParseScanRequest(t *testing.T) {
+func TestParseModuleURLPath(t *testing.T) {
for _, test := range []struct {
- name string
- url string
- want Request
+ path string
+ want ModuleURLPath
}{
{
- name: "ValidScanURL",
- url: "https://worker.com/scan/module/@v/v1.0.0?importedby=50",
- want: Request{
- ModuleURLPath{Module: "module", Version: "v1.0.0"},
- RequestParams{ImportedBy: 50, Mode: ""},
+ "/module/@v/v1.0.0",
+ ModuleURLPath{Module: "module", Version: "v1.0.0"},
+ },
+ {
+ "/module/@v/v1.0.0-abcdefgh/suffix",
+ ModuleURLPath{
+ Module: "module",
+ Version: "v1.0.0-abcdefgh",
+ Suffix: "suffix",
},
},
{
- name: "ValidImportsOnlyScanURL",
- url: "https://worker.com/scan/module/@v/v1.0.0-abcdefgh?importedby=100&mode=mode1",
- want: Request{
- ModuleURLPath{Module: "module", Version: "v1.0.0-abcdefgh"},
- RequestParams{ImportedBy: 100, Mode: "mode1"},
- },
- },
- {
- name: "Module@Version",
- url: "https://worker.com/scan/module@v1.2.3?importedby=1",
- want: Request{
- ModuleURLPath{Module: "module", Version: "v1.2.3"},
- RequestParams{ImportedBy: 1, Mode: ""},
- },
- },
- {
- name: "Module@Version suffix",
- url: "https://worker.com/scan/module@v1.2.3/path/to/dir?importedby=1",
- want: Request{
- ModuleURLPath{Module: "module", Version: "v1.2.3", Suffix: "path/to/dir"},
- RequestParams{ImportedBy: 1, Mode: ""},
+ "/module@v1.2.3/a/b/c",
+ ModuleURLPath{
+ Module: "module",
+ Version: "v1.2.3",
+ Suffix: "a/b/c",
},
},
} {
- t.Run(test.name, func(t *testing.T) {
- u, err := url.Parse(test.url)
- if err != nil {
- t.Errorf("url.Parse(%q): %v", test.url, err)
- }
- r := &http.Request{URL: u}
- got, err := ParseRequest(r, "/scan")
- if err != nil {
- t.Fatal(err)
- }
- if g, w := *got, test.want; g != w {
- t.Errorf("\ngot %+v\nwant %+v", g, w)
- }
- })
+ got, err := ParseModuleURLPath(test.path)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if g, w := got, test.want; !cmp.Equal(g, w) {
+ t.Errorf("%s:\ngot %+v\nwant %+v", test.path, g, w)
+ }
}
}
-func TestParseScanRequestError(t *testing.T) {
+func TestModuleURLPAthError(t *testing.T) {
for _, test := range []struct {
name string
- url string
+ path string
want string
}{
{
name: "InvalidScanURL",
- url: "/",
+ path: "/",
want: `invalid path "/": missing '@'`,
},
{
name: "InvalidScanURLNoModule",
- url: "/@v/version",
+ path: "/@v/version",
want: `invalid path "/@v/version": missing module`,
},
{
name: "InvalidScanURLNoVersion",
- url: "/module/@v/",
+ path: "/module/@v/",
want: `invalid path "/module/@v/": missing version`,
},
{
name: "NoVersion",
- url: "/module@",
+ path: "/module@",
want: `invalid path "/module@": missing version`,
},
{
name: "NoVersionSuffix",
- url: "/module@/suffix",
+ path: "/module@/suffix",
want: `invalid path "/module@/suffix": missing version`,
},
- {
- name: "MissingImportedBy",
- url: "/module/@v/v1.0.0",
- want: `missing or negative "importedby" query param`,
- },
- {
- name: "BadImportedBy",
- url: "/module@v1?importedby=1a",
- want: `param importedby: strconv.Atoi: parsing "1a": invalid syntax`,
- },
} {
t.Run(test.name, func(t *testing.T) {
- u, err := url.Parse(test.url)
- if err != nil {
- t.Errorf("url.Parse(%q): %v", test.url, err)
- }
- r := &http.Request{URL: u}
- if _, err := ParseRequest(r, "/scan"); err != nil {
- if got := err.Error(); !strings.Contains(got, test.want) {
- t.Fatalf("\ngot %s\nwant %s", got, test.want)
+ if _, err := ParseModuleURLPath(test.path); err != nil {
+ if got := err.Error(); !strings.HasSuffix(got, test.want) {
+ t.Fatalf("\ngot %s\nwant suffix %s", got, test.want)
}
} else {
t.Fatalf("error = nil; want = (%v)", test.want)
diff --git a/internal/worker/enqueue.go b/internal/worker/enqueue.go
index 804fab7..814ee9e 100644
--- a/internal/worker/enqueue.go
+++ b/internal/worker/enqueue.go
@@ -6,7 +6,6 @@
import (
"context"
- "errors"
"sync"
"golang.org/x/pkgsite-metrics/internal/config"
@@ -37,8 +36,8 @@
return pkgsitedb.ModuleSpecs(ctx, db, minImportedByCount)
}
-func enqueueModules(ctx context.Context, sreqs []*scan.Request, q queue.Queue, opts *queue.Options) (err error) {
- defer derrors.Wrap(&err, "enqueueModules")
+func enqueueTasks(ctx context.Context, tasks []queue.Task, q queue.Queue, opts *queue.Options) (err error) {
+ defer derrors.Wrap(&err, "enqueueTasks")
// Enqueue concurrently, because sequentially takes a while.
const concurrentEnqueues = 10
@@ -48,14 +47,8 @@
)
sem := make(chan struct{}, concurrentEnqueues)
- for _, sreq := range sreqs {
+ for _, sreq := range tasks {
log.Infof(ctx, "enqueuing: %s?%s", sreq.Path(), sreq.Params())
- if sreq.Module == "std" {
- continue // ignore the standard library
- }
- if sreq.Mode == "" {
- return errors.New("ScanRequest.Mode cannot be empty")
- }
sreq := sreq
sem <- struct{}{}
go func() {
@@ -79,12 +72,18 @@
return nil
}
-func moduleSpecsToScanRequests(modspecs []scan.ModuleSpec, mode string) []*scan.Request {
- var sreqs []*scan.Request
+func moduleSpecsToScanRequests(modspecs []scan.ModuleSpec, mode string) []*vulncheckRequest {
+ var sreqs []*vulncheckRequest
for _, ms := range modspecs {
- sreqs = append(sreqs, &scan.Request{
- ModuleURLPath: scan.ModuleURLPath{Module: ms.Path, Version: ms.Version},
- RequestParams: scan.RequestParams{ImportedBy: ms.ImportedBy, Mode: mode},
+ sreqs = append(sreqs, &vulncheckRequest{
+ scan.ModuleURLPath{
+ Module: ms.Path,
+ Version: ms.Version,
+ },
+ vulncheckRequestParams{
+ ImportedBy: ms.ImportedBy,
+ Mode: mode,
+ },
})
}
return sreqs
diff --git a/internal/worker/vulncheck_enqueue.go b/internal/worker/vulncheck_enqueue.go
index 778b252..b3ddb54 100644
--- a/internal/worker/vulncheck_enqueue.go
+++ b/internal/worker/vulncheck_enqueue.go
@@ -18,39 +18,48 @@
"google.golang.org/api/iterator"
)
+// query params for vulncheck/enqueue
+type vulncheckEnqueueParams struct {
+ Suffix string // appended to task queue IDs to generate unique tasks
+ Mode string // type of analysis to run
+ Min int // minimum import-by count for a module to be included
+ File string // path to file containing modules; if missing, use DB
+}
+
// handleEnqueue enqueues multiple modules for a single vulncheck mode.
-// Query params:
-// - suffix: appended to task queue IDs to generate unique tasks
-// - mode: type of analysis to run; see [modes]
-// - file: path to file containing modules; if missing, use DB
-// - min: minimum import-by count for a module to be included
func (h *VulncheckServer) handleEnqueue(w http.ResponseWriter, r *http.Request) error {
+ params := &vulncheckEnqueueParams{Min: defaultMinImportedByCount}
+ if err := scan.ParseParams(r, ¶ms); err != nil {
+ return err
+ }
ctx := r.Context()
- suffix := r.FormValue("suffix")
- mode, err := vulncheckMode(scan.ParseMode(r))
+ mode, err := vulncheckMode(params.Mode)
if err != nil {
return err
}
- var sreqs []*scan.Request
+ var reqs []*vulncheckRequest
if mode == ModeBinary {
var err error
- sreqs, err = readBinaries(ctx, h.cfg.BinaryBucket)
+ reqs, err = readBinaries(ctx, h.cfg.BinaryBucket)
if err != nil {
return err
}
} else {
- minImpCount, err := scan.ParseOptionalIntParam(r, "min", defaultMinImportedByCount)
+ modspecs, err := readModules(ctx, h.cfg, params.File, params.Min)
if err != nil {
return err
}
- modspecs, err := readModules(ctx, h.cfg, r.FormValue("file"), minImpCount)
- if err != nil {
- return err
- }
- sreqs = moduleSpecsToScanRequests(modspecs, mode)
+ reqs = moduleSpecsToScanRequests(modspecs, mode)
}
- return enqueueModules(ctx, sreqs, h.queue, &queue.Options{Namespace: "vulncheck", TaskNameSuffix: suffix})
+ var sreqs []queue.Task
+ for _, req := range reqs {
+ if req.Module != "std" { // ignore the standard library
+ sreqs = append(sreqs, req)
+ }
+ }
+ return enqueueTasks(ctx, sreqs, h.queue,
+ &queue.Options{Namespace: "vulncheck", TaskNameSuffix: params.Suffix})
}
func vulncheckMode(mode string) (string, error) {
@@ -71,28 +80,34 @@
// - file: path to file containing modules; if missing, use DB
// - min: minimum import-by count for a module to be included
func (h *VulncheckServer) handleEnqueueAll(w http.ResponseWriter, r *http.Request) error {
+ params := &vulncheckEnqueueParams{Min: defaultMinImportedByCount}
+ if err := scan.ParseParams(r, ¶ms); err != nil {
+ return err
+ }
+
ctx := r.Context()
- suffix := r.FormValue("suffix")
- minImpCount, err := scan.ParseOptionalIntParam(r, "min", defaultMinImportedByCount)
+ modspecs, err := readModules(ctx, h.cfg, params.File, params.Min)
if err != nil {
return err
}
- modspecs, err := readModules(ctx, h.cfg, r.FormValue("file"), minImpCount)
- if err != nil {
- return err
- }
- opts := &queue.Options{Namespace: "vulncheck", TaskNameSuffix: suffix}
+ opts := &queue.Options{Namespace: "vulncheck", TaskNameSuffix: params.Suffix}
for mode := range modes {
- var sreqs []*scan.Request
+ var reqs []*vulncheckRequest
if mode == ModeBinary {
- sreqs, err = readBinaries(ctx, h.cfg.BinaryBucket)
+ reqs, err = readBinaries(ctx, h.cfg.BinaryBucket)
if err != nil {
return err
}
} else {
- sreqs = moduleSpecsToScanRequests(modspecs, mode)
+ reqs = moduleSpecsToScanRequests(modspecs, mode)
}
- if err := enqueueModules(ctx, sreqs, h.queue, opts); err != nil {
+ var tasks []queue.Task
+ for _, req := range reqs {
+ if req.Module != "std" { // ignore the standard library
+ tasks = append(tasks, req)
+ }
+ }
+ if err := enqueueTasks(ctx, tasks, h.queue, opts); err != nil {
return err
}
}
@@ -102,7 +117,7 @@
// binaryDir is the directory in the GCS bucket that contains binaries that should be scanned.
const binaryDir = "binaries"
-func readBinaries(ctx context.Context, bucketName string) (sreqs []*scan.Request, err error) {
+func readBinaries(ctx context.Context, bucketName string) (reqs []*vulncheckRequest, err error) {
defer derrors.Wrap(&err, "readBinaries(%q)", bucketName)
if bucketName == "" {
log.Infof(ctx, "binary bucket not configured; not enqueuing binaries")
@@ -125,10 +140,10 @@
if err != nil {
return nil, err
}
- sreqs = append(sreqs, &scan.Request{
- ModuleURLPath: mp,
- RequestParams: scan.RequestParams{Mode: ModeBinary},
+ reqs = append(reqs, &vulncheckRequest{
+ ModuleURLPath: mp,
+ vulncheckRequestParams: vulncheckRequestParams{Mode: ModeBinary},
})
}
- return sreqs, nil
+ return reqs, nil
}
diff --git a/internal/worker/vulncheck_enqueue_test.go b/internal/worker/vulncheck_enqueue_test.go
index f178601..931e9df 100644
--- a/internal/worker/vulncheck_enqueue_test.go
+++ b/internal/worker/vulncheck_enqueue_test.go
@@ -22,13 +22,13 @@
if err != nil {
t.Fatal(err)
}
- want := &scan.Request{
- ModuleURLPath: scan.ModuleURLPath{
+ want := &vulncheckRequest{
+ scan.ModuleURLPath{
Module: "golang.org/x/pkgsite",
Version: "v0.0.0-20221004150836-873fb37c2479",
Suffix: "cmd/worker",
},
- RequestParams: scan.RequestParams{Mode: ModeBinary},
+ vulncheckRequestParams{Mode: ModeBinary},
}
found := false
for _, sr := range sreqs {
diff --git a/internal/worker/vulncheck_scan.go b/internal/worker/vulncheck_scan.go
index 200bb5c..e4aca86 100644
--- a/internal/worker/vulncheck_scan.go
+++ b/internal/worker/vulncheck_scan.go
@@ -84,7 +84,7 @@
}()
ctx := r.Context()
- sreq, err := scan.ParseRequest(r, "/vulncheck/scan")
+ sreq, err := parseVulncheckRequest(r, "/vulncheck/scan")
if err != nil {
return fmt.Errorf("%w: %v", derrors.InvalidArgument, err)
}
@@ -184,7 +184,7 @@
return s.err
}
-func (s *scanner) ScanModule(ctx context.Context, sreq *scan.Request) {
+func (s *scanner) ScanModule(ctx context.Context, sreq *vulncheckRequest) {
if sreq.Module == "std" {
return // ignore the standard library
}
@@ -716,7 +716,7 @@
// This runs a scan but returns the resulting JSON instead of writing it to BigQuery.
func (s *Server) handleTestVulncheckSandbox(w http.ResponseWriter, r *http.Request) error {
ctx := r.Context()
- sreq, err := scan.ParseRequest(r, "/test-vulncheck-sandbox")
+ sreq, err := parseVulncheckRequest(r, "/test-vulncheck-sandbox")
if err != nil {
return fmt.Errorf("%w: %v", derrors.InvalidArgument, err)
}
@@ -729,3 +729,61 @@
_, err = w.Write(out)
return err
}
+
+// vulncheckRequest contains information passed
+// to a scan endpoint.
+type vulncheckRequest struct {
+ scan.ModuleURLPath
+ vulncheckRequestParams
+}
+
+// vulncheckRequestParams has query parameters for a vulncheck scan request.
+type vulncheckRequestParams struct {
+ ImportedBy int
+ Mode string
+ Insecure bool
+ // TODO: support optional parameters?
+}
+
+// These methods implement queue.Task.
+func (r *vulncheckRequest) Name() string { return r.Module + "@" + r.Version }
+
+func (r *vulncheckRequest) Path() string {
+ p := r.Module + "@" + r.Version
+ if r.Suffix != "" {
+ p += "/" + r.Suffix
+ }
+ return p
+}
+
+func (r *vulncheckRequest) Params() string {
+ return scan.FormatParams(r.vulncheckRequestParams)
+}
+
+// parseVulncheckRequest parses an http request r for an endpoint
+// prefix and produces a corresponding ScanRequest.
+//
+// The module and version should have one of the following three forms:
+// - <module>/@v/<version>
+// - <module>@<version>
+// - <module>/@latest
+//
+// (These are the same forms that the module proxy accepts.)
+func parseVulncheckRequest(r *http.Request, prefix string) (*vulncheckRequest, error) {
+ mp, err := scan.ParseModuleURLPath(strings.TrimPrefix(r.URL.Path, prefix))
+ if err != nil {
+ return nil, err
+ }
+
+ rp := vulncheckRequestParams{ImportedBy: -1}
+ if err := scan.ParseParams(r, &rp); err != nil {
+ return nil, err
+ }
+ if rp.ImportedBy < 0 {
+ return nil, errors.New(`missing or negative "importedby" query param`)
+ }
+ return &vulncheckRequest{
+ ModuleURLPath: mp,
+ vulncheckRequestParams: rp,
+ }, nil
+}
