Skip to content

Commit

Permalink
internal/frontend, internal/vulns: isolate references to x/vuln repo
Browse files Browse the repository at this point in the history 
No-op refactor to move all code that depends on x/vuln to the
internal/vuln (renamed from internal/vulns) package. This will allow
us to more easily remove the dependency, as a part of the migration to
the v1 database schema.

For golang/go#58928

Change-Id: Ic8ac2377832d8e4a2a6afbb42729a7e10553665c
Reviewed-on: https://go-review.googlesource.com/c/pkgsite/+/474255
Reviewed-by: Julie Qiu <julieqiu@google.com>
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Run-TryBot: Tatiana Bradley <tatianabradley@google.com>
TryBot-Result: kokoro <noreply+kokoro@google.com>
  • Loading branch information
@tatianab
tatianab committed Mar 8, 2023
1 parent a0d43ae commit 0817681
Show file tree
Hide file tree
Showing 15 changed files with 176 additions and 119 deletions.
8 changes: 3 additions & 5 deletions cmd/frontend/main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import (
"golang.org/x/pkgsite/internal/proxy"
"golang.org/x/pkgsite/internal/queue"
"golang.org/x/pkgsite/internal/source"
vulnc "golang.org/x/vuln/client"
"golang.org/x/pkgsite/internal/vuln"
)

var (
Expand Down Expand Up @@ -105,11 +105,9 @@ func main() {
}

rc := cmdconfig.ReportingClient(ctx, cfg)
vc, err := vulnc.NewClient([]string{cfg.VulnDB}, vulnc.Options{
HTTPCache: newVulndbCache(),
})
vc, err := vuln.NewClient(cfg.VulnDB)
if err != nil {
log.Fatalf(ctx, "vulndbc.NewClient: %v", err)
log.Fatalf(ctx, "vuln.NewClient: %v", err)
}
staticSource := template.TrustedSourceFromFlag(flag.Lookup("static").Value)
server, err := frontend.NewServer(frontend.ServerConfig{
Expand Down
25 changes: 12 additions & 13 deletions internal/frontend/search.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,8 @@ import (
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
"golang.org/x/pkgsite/internal/version"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
"golang.org/x/text/message"
vulnc "golang.org/x/vuln/client"
)

// serveSearch applies database data to the search template. Handles endpoint
Expand Down Expand Up @@ -59,7 +58,7 @@ type searchAction struct {
page interface{ setBasePage(basePage) }
}

func determineSearchAction(r *http.Request, ds internal.DataSource, vulnClient vulnc.Client) (*searchAction, error) {
func determineSearchAction(r *http.Request, ds internal.DataSource, vulnClient *vuln.Client) (*searchAction, error) {
if r.Method != http.MethodGet && r.Method != http.MethodHead {
return nil, &serverError{status: http.StatusMethodNotAllowed}
}
Expand Down Expand Up @@ -130,9 +129,9 @@ func determineSearchAction(r *http.Request, ds internal.DataSource, vulnClient v
if len(filters) > 0 {
symbol = filters[0]
}
var getVulnEntries vulns.VulnEntriesFunc
var getVulnEntries vuln.VulnEntriesFunc
if vulnClient != nil {
getVulnEntries = vulnClient.GetByModule
getVulnEntries = vulnClient.ByModule
}
page, err := fetchSearchPage(ctx, db, cq, symbol, pageParams, mode == searchModeSymbol, getVulnEntries)
if err != nil {
Expand Down Expand Up @@ -226,7 +225,7 @@ type SearchResult struct {
SymbolGOOS string
SymbolGOARCH string
SymbolLink string
Vulns []vulns.Vuln
Vulns []vuln.Vuln
}

type subResult struct {
Expand All @@ -237,7 +236,7 @@ type subResult struct {
// fetchSearchPage fetches data matching the search query from the database and
// returns a SearchPage.
func fetchSearchPage(ctx context.Context, db *postgres.DB, cq, symbol string,
pageParams paginationParams, searchSymbols bool, getVulnEntries vulns.VulnEntriesFunc) (*SearchPage, error) {
pageParams paginationParams, searchSymbols bool, getVulnEntries vuln.VulnEntriesFunc) (*SearchPage, error) {
maxResultCount := maxSearchOffset + pageParams.limit

// Pageless search: always start from the beginning.
Expand Down Expand Up @@ -371,7 +370,7 @@ func searchRequestRedirectPath(ctx context.Context, ds internal.DataSource, quer
return fmt.Sprintf("/%s", requestedPath)
}

func searchVulnModule(ctx context.Context, mode, cq string, client vulnc.Client) (_ *searchAction, err error) {
func searchVulnModule(ctx context.Context, mode, cq string, client *vuln.Client) (_ *searchAction, err error) {
if mode != searchModeVuln {
return nil, nil
}
Expand Down Expand Up @@ -401,13 +400,13 @@ EntryLoop:
}, nil
}

func searchVulnAlias(ctx context.Context, mode, cq string, vulnClient vulnc.Client) (_ *searchAction, err error) {
func searchVulnAlias(ctx context.Context, mode, cq string, vulnClient *vuln.Client) (_ *searchAction, err error) {
defer derrors.Wrap(&err, "searchVulnAlias(%q, %q)", mode, cq)

if mode != searchModeVuln || !isVulnAlias(cq) {
return nil, nil
}
aliasEntries, err := vulnClient.GetByAlias(ctx, cq)
aliasEntries, err := vulnClient.ByAlias(ctx, cq)
if err != nil {
return nil, err
}
Expand All @@ -429,7 +428,7 @@ func searchVulnAlias(ctx context.Context, mode, cq string, vulnClient vulnc.Clie
}
}

// Regexps that match aliases for Go vulns.
// Regexps that match aliases for Go vuln.
var (
cveRegexp = regexp.MustCompile("^CVE-[0-9]{4}-[0-9]+$")
ghsaRegexp = regexp.MustCompile("^GHSA-.{4}-.{4}-.{4}$")
Expand Down Expand Up @@ -608,7 +607,7 @@ func elapsedTime(date time.Time) string {

// addVulns adds vulnerability information to search results by consulting the
// vulnerability database.
func addVulns(ctx context.Context, rs []*SearchResult, getVulnEntries vulns.VulnEntriesFunc) {
func addVulns(ctx context.Context, rs []*SearchResult, getVulnEntries vuln.VulnEntriesFunc) {
// Get all vulns concurrently.
var wg sync.WaitGroup
// TODO(golang/go#48223): throttle concurrency?
Expand All @@ -617,7 +616,7 @@ func addVulns(ctx context.Context, rs []*SearchResult, getVulnEntries vulns.Vuln
wg.Add(1)
go func() {
defer wg.Done()
r.Vulns = vulns.VulnsForPackage(ctx, r.ModulePath, r.Version, r.PackagePath, getVulnEntries)
r.Vulns = vuln.VulnsForPackage(ctx, r.ModulePath, r.Version, r.PackagePath, getVulnEntries)
}()
}
wg.Wait()
Expand Down
10 changes: 5 additions & 5 deletions internal/frontend/search_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"golang.org/x/pkgsite/internal/licenses"
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/testing/sample"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
"golang.org/x/text/language"
"golang.org/x/text/message"
"golang.org/x/vuln/osv"
Expand All @@ -39,7 +39,7 @@ func TestDetermineSearchAction(t *testing.T) {
for _, v := range modules {
postgres.MustInsertModule(ctx, t, testDB, v)
}
vc := newVulndbTestClient(testEntries)
vc := vuln.NewTestClient(testEntries)
for _, test := range []struct {
name string
method string
Expand Down Expand Up @@ -385,7 +385,7 @@ func TestFetchSearchPage(t *testing.T) {
DisplayVersion: moduleFoo.Version,
Licenses: []string{"MIT"},
CommitTime: elapsedTime(moduleFoo.CommitTime),
Vulns: []vulns.Vuln{{ID: "test", Details: "vuln"}},
Vulns: []vuln.Vuln{{ID: "test", Details: "vuln"}},
},
},
},
Expand Down Expand Up @@ -552,7 +552,7 @@ func TestSearchRequestRedirectPath(t *testing.T) {
}

func TestSearchVulnAlias(t *testing.T) {
vc := newVulndbTestClient(testEntries)
vc := vuln.NewTestClient(testEntries)
for _, test := range []struct {
name string
mode string
Expand Down Expand Up @@ -624,7 +624,7 @@ func TestSearchVulnAlias(t *testing.T) {
}

func TestSearchVulnModulePath(t *testing.T) {
vc := newVulndbTestClient(testEntries)
vc := vuln.NewTestClient(testEntries)
for _, test := range []struct {
name string
mode string
Expand Down
6 changes: 3 additions & 3 deletions internal/frontend/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ import (
"golang.org/x/pkgsite/internal/queue"
"golang.org/x/pkgsite/internal/static"
"golang.org/x/pkgsite/internal/version"
"golang.org/x/pkgsite/internal/vuln"
"golang.org/x/text/cases"
"golang.org/x/text/language"
vulnc "golang.org/x/vuln/client"
)

// Server can be installed to serve the go discovery frontend.
Expand All @@ -59,7 +59,7 @@ type Server struct {
serveStats bool
reportingClient *errorreporting.Client
fileMux *http.ServeMux
vulnClient vulnc.Client
vulnClient *vuln.Client
versionID string
instanceID string

Expand All @@ -81,7 +81,7 @@ type ServerConfig struct {
DevMode bool
StaticPath string // used only for dynamic loading in dev mode
ReportingClient *errorreporting.Client
VulndbClient vulnc.Client
VulndbClient *vuln.Client
}

// NewServer creates a new Server for the given database and template directory.
Expand Down
4 changes: 2 additions & 2 deletions internal/frontend/tabs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (

"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
)

// TabSettings defines tab-specific metadata.
Expand Down Expand Up @@ -78,7 +78,7 @@ func init() {
// handler.
func fetchDetailsForUnit(ctx context.Context, r *http.Request, tab string, ds internal.DataSource, um *internal.UnitMeta,
requestedVersion string, bc internal.BuildContext,
getVulnEntries vulns.VulnEntriesFunc) (_ any, err error) {
getVulnEntries vuln.VulnEntriesFunc) (_ any, err error) {
defer derrors.Wrap(&err, "fetchDetailsForUnit(r, %q, ds, um=%q,%q,%q)", tab, um.Path, um.ModulePath, um.Version)
switch tab {
case tabMain:
Expand Down
10 changes: 5 additions & 5 deletions internal/frontend/unit.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
"golang.org/x/pkgsite/internal/middleware"
"golang.org/x/pkgsite/internal/stdlib"
"golang.org/x/pkgsite/internal/version"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
)

// UnitPage contains data needed to render the unit template.
Expand Down Expand Up @@ -92,7 +92,7 @@ type UnitPage struct {
Details any

// Vulns holds vulnerability information.
Vulns []vulns.Vuln
Vulns []vuln.Vuln

// DepsDevURL holds the full URL to this module version on deps.dev.
DepsDevURL string
Expand Down Expand Up @@ -135,9 +135,9 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht
// It's also okay to provide just one (e.g. GOOS=windows), which will select
// the first doc with that value, ignoring the other one.
bc := internal.BuildContext{GOOS: r.FormValue("GOOS"), GOARCH: r.FormValue("GOARCH")}
var getVulnEntries vulns.VulnEntriesFunc
var getVulnEntries vuln.VulnEntriesFunc
if s.vulnClient != nil {
getVulnEntries = s.vulnClient.GetByModule
getVulnEntries = s.vulnClient.ByModule
}
d, err := fetchDetailsForUnit(ctx, r, tab, ds, um, info.requestedVersion, bc, getVulnEntries)
if err != nil {
Expand Down Expand Up @@ -241,7 +241,7 @@ func (s *Server) serveUnitPage(ctx context.Context, w http.ResponseWriter, r *ht

// Get vulnerability information.
if s.vulnClient != nil {
page.Vulns = vulns.VulnsForPackage(ctx, um.ModulePath, um.Version, um.Path, s.vulnClient.GetByModule)
page.Vulns = vuln.VulnsForPackage(ctx, um.ModulePath, um.Version, um.Path, s.vulnClient.ByModule)
}
s.servePage(ctx, w, tabSettings.TemplateName, page)
return nil
Expand Down
10 changes: 5 additions & 5 deletions internal/frontend/versions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
"golang.org/x/pkgsite/internal/postgres"
"golang.org/x/pkgsite/internal/stdlib"
"golang.org/x/pkgsite/internal/version"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
)

// VersionsDetails contains the hierarchy of version summary information used
Expand Down Expand Up @@ -82,10 +82,10 @@ type VersionSummary struct {
RetractionRationale string
IsMinor bool
Symbols [][]*Symbol
Vulns []vulns.Vuln
Vulns []vuln.Vuln
}

func fetchVersionsDetails(ctx context.Context, ds internal.DataSource, um *internal.UnitMeta, getVulnEntries vulns.VulnEntriesFunc) (*VersionsDetails, error) {
func fetchVersionsDetails(ctx context.Context, ds internal.DataSource, um *internal.UnitMeta, getVulnEntries vuln.VulnEntriesFunc) (*VersionsDetails, error) {
db, ok := ds.(*postgres.DB)
if !ok {
// The proxydatasource does not support the imported by page.
Expand Down Expand Up @@ -146,7 +146,7 @@ func buildVersionDetails(ctx context.Context, currentModulePath, packagePath str
modInfos []*internal.ModuleInfo,
sh *internal.SymbolHistory,
linkify func(v *internal.ModuleInfo) string,
getVulnEntries vulns.VulnEntriesFunc,
getVulnEntries vuln.VulnEntriesFunc,
) *VersionsDetails {
// lists organizes versions by VersionListKey.
lists := make(map[VersionListKey]*VersionList)
Expand Down Expand Up @@ -201,7 +201,7 @@ func buildVersionDetails(ctx context.Context, currentModulePath, packagePath str
if mi.ModulePath == stdlib.ModulePath {
pkg = packagePath
}
vs.Vulns = vulns.VulnsForPackage(ctx, mi.ModulePath, mi.Version, pkg, getVulnEntries)
vs.Vulns = vuln.VulnsForPackage(ctx, mi.ModulePath, mi.Version, pkg, getVulnEntries)
vl := lists[key]
if vl == nil {
seenLists = append(seenLists, key)
Expand Down
4 changes: 2 additions & 2 deletions internal/frontend/versions_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
"golang.org/x/pkgsite/internal/stdlib"
"golang.org/x/pkgsite/internal/testing/sample"
"golang.org/x/pkgsite/internal/version"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
"golang.org/x/vuln/osv"
)

Expand Down Expand Up @@ -150,7 +150,7 @@ func TestFetchPackageVersionsDetails(t *testing.T) {
ThisModule: []*VersionList{
func() *VersionList {
vl := makeList(v1Path, modulePath1, "v1", []string{"v1.3.0", "v1.2.3", "v1.2.1"}, false)
vl.Versions[2].Vulns = []vulns.Vuln{{
vl.Versions[2].Vulns = []vuln.Vuln{{
Details: vulnEntry.Details,
}}
return vl
Expand Down
19 changes: 9 additions & 10 deletions internal/frontend/vulns.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@ import (

"golang.org/x/pkgsite/internal"
"golang.org/x/pkgsite/internal/derrors"
"golang.org/x/pkgsite/internal/vulns"
"golang.org/x/pkgsite/internal/vuln"
"golang.org/x/sync/errgroup"
vulnc "golang.org/x/vuln/client"
"golang.org/x/vuln/osv"
)

Expand All @@ -34,7 +33,7 @@ type VulnListPage struct {
type VulnPage struct {
basePage
Entry OSVEntry
AffectedPackages []*vulns.AffectedPackage
AffectedPackages []*vuln.AffectedPackage
AliasLinks []link
AdvisoryLinks []link
}
Expand Down Expand Up @@ -105,8 +104,8 @@ func (s *Server) serveVuln(w http.ResponseWriter, r *http.Request, _ internal.Da
return nil
}

func newVulnPage(ctx context.Context, client vulnc.Client, id string) (*VulnPage, error) {
entry, err := client.GetByID(ctx, id)
func newVulnPage(ctx context.Context, client *vuln.Client, id string) (*VulnPage, error) {
entry, err := client.ByID(ctx, id)
if err != nil {
return nil, derrors.VulnDBError
}
Expand All @@ -115,13 +114,13 @@ func newVulnPage(ctx context.Context, client vulnc.Client, id string) (*VulnPage
}
return &VulnPage{
Entry: OSVEntry{entry},
AffectedPackages: vulns.AffectedPackages(entry),
AffectedPackages: vuln.AffectedPackages(entry),
AliasLinks: aliasLinks(entry),
AdvisoryLinks: advisoryLinks(entry),
}, nil
}

func newVulnListPage(ctx context.Context, client vulnc.Client) (*VulnListPage, error) {
func newVulnListPage(ctx context.Context, client *vuln.Client) (*VulnListPage, error) {
entries, err := vulnList(ctx, client)
if err != nil {
return nil, err
Expand All @@ -131,10 +130,10 @@ func newVulnListPage(ctx context.Context, client vulnc.Client) (*VulnListPage, e
return &VulnListPage{Entries: entries}, nil
}

func vulnList(ctx context.Context, client vulnc.Client) ([]OSVEntry, error) {
func vulnList(ctx context.Context, client *vuln.Client) ([]OSVEntry, error) {
const concurrency = 4

ids, err := client.ListIDs(ctx)
ids, err := client.IDs(ctx)
if err != nil {
return nil, derrors.VulnDBError
}
Expand All @@ -148,7 +147,7 @@ func vulnList(ctx context.Context, client vulnc.Client) ([]OSVEntry, error) {
sem <- struct{}{}
g.Go(func() error {
defer func() { <-sem }()
e, err := client.GetByID(ctx, id)
e, err := client.ByID(ctx, id)
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 0817681

Please sign in to comment.