Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve db update check #2020

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cmd/grype/cli/commands/db_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ func runDBCheck(opts options.Database) error {
return err
}

updateAvailable, currentDBMetadata, updateDBEntry, err := dbCurator.IsUpdateAvailable()
currentDBMetadata := dbCurator.GetMetadata()
updateDBEntry, err := dbCurator.GetUpdate(currentDBMetadata)
if err != nil {
return fmt.Errorf("unable to check for vulnerability database update: %+v", err)
}

if !updateAvailable {
if updateDBEntry == nil {
return stderrPrintLnf("No update available")
}

Expand Down
99 changes: 81 additions & 18 deletions grype/db/curator.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"crypto/x509"
"fmt"
"net/http"
"net/url"
"os"
"path"
"strconv"
Expand Down Expand Up @@ -43,6 +44,7 @@ type Config struct {

type Curator struct {
fs afero.Fs
listingClient *http.Client
listingDownloader file.Getter
updateDownloader file.Getter
targetSchema int
Expand Down Expand Up @@ -73,6 +75,7 @@ func NewCurator(cfg Config) (Curator, error) {
return Curator{
fs: fs,
targetSchema: vulnerability.SchemaVersion,
listingClient: listingClient,
listingDownloader: file.NewGetter(listingClient),
updateDownloader: file.NewGetter(dbClient),
dbDir: dbDir,
Expand Down Expand Up @@ -114,6 +117,7 @@ func (c *Curator) Status() Status {

return Status{
Built: metadata.Built,
Updated: metadata.Updated,
SchemaVersion: metadata.Version,
Location: c.dbDir,
Checksum: metadata.Checksum,
Expand Down Expand Up @@ -148,13 +152,14 @@ func (c *Curator) Update() (bool, error) {
defer downloadProgress.SetCompleted()
defer importProgress.SetCompleted()

updateAvailable, metadata, updateEntry, err := c.IsUpdateAvailable()
metadata := c.GetMetadata()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we always download the listing on grype db check and grype db update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, if we reliably know they are already up-to-date, why waste CPU cycles/network?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am pessimistically assuming that something will go wrong, and having a command that means, "no really, I mean it, download that database whether you need to or not" might be helpful.

The goal of this change is to alleviate traffic. Making rare commands less useful isn't going to alleviate traffic.

But I do see your point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this is making commands less useful, they all continue to function exactly as they do: if we downloaded a listing file and got the latest database, we have the latest database, so downloading a listing again and downloading the database again is rather pointless. I think I understand your point, though. I'll see about making this particular function isolated as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think as a user, I would prefer grype db update to be efficient and use this function to eliminate unnecessary downloads if it's already up-to-date. Maybe we add a --force argument or similar to make it download everything anew?

updateEntry, err := c.GetUpdate(metadata)
if err != nil {
// we want to continue if possible even if we can't check for an update
log.Warnf("unable to check for vulnerability database update")
log.Debugf("check for vulnerability update failed: %+v", err)
}
if updateAvailable {
if updateEntry != nil {
log.Infof("downloading new vulnerability DB")
err = c.UpdateTo(updateEntry, downloadProgress, importProgress, stage)
if err != nil {
Expand Down Expand Up @@ -184,35 +189,82 @@ func (c *Curator) Update() (bool, error) {
return false, nil
}

// IsUpdateAvailable indicates if there is a new update available as a boolean, and returns the latest listing information
// available for this schema.
func (c *Curator) IsUpdateAvailable() (bool, *Metadata, *ListingEntry, error) {
// GetMetadata returns the current metadata or nil if unable to find or read metadata
func (c *Curator) GetMetadata() *Metadata {
metadata, err := NewMetadataFromDir(c.fs, c.dbDir)
if err != nil {
log.Debugf("current metadata corrupt: %w", err)
}
return metadata
}

// UpdateMetadataTimestamp updates the metadata file with the current timestamp
func (c *Curator) updateMetadataTimestamp() error {
metadata, err := NewMetadataFromDir(c.fs, c.dbDir)
if err != nil || metadata == nil {
return err
}
// update the check time
metadata.Updated = time.Now()
return metadata.Write(metadataPath(c.dbDir))
}

// GetUpdate returns an available update if one is available or an error if an error occurred while checking
func (c *Curator) GetUpdate(current *Metadata) (*ListingEntry, error) {
log.Debugf("checking for available database updates")

listing, err := c.ListingFromURL()
u, err := url.Parse(c.listingURL)
if err != nil {
return false, nil, nil, err
return nil, fmt.Errorf("invalid URL: %v %v", c.listingURL, err)
}

updateEntry := listing.BestUpdate(c.targetSchema)
if updateEntry == nil {
return false, nil, nil, fmt.Errorf("no db candidates with correct version available (maybe there is an application update available?)")
headers := http.Header{}

s := c.Status()
if s.Err == nil {
// valid status, get the db update time
headers.Add("If-Modified-Since", s.Updated.UTC().Format(http.TimeFormat))
}

req := http.Request{
Method: http.MethodGet,
URL: u,
Header: headers,
}

resp, err := c.listingClient.Do(&req)
if err != nil {
return nil, fmt.Errorf("error attempting to check for update: %w", err)
}
defer func() {
err := resp.Body.Close()
if err != nil {
log.Debug(err)
}
}()

if s.Err == nil && resp.StatusCode == http.StatusNotModified {
return nil, nil
}
log.Debugf("found database update candidate: %s", updateEntry)

// compare created data to current db date
current, err := NewMetadataFromDir(c.fs, c.dbDir)
listing, err := NewListingFromReader(resp.Body)
if err != nil {
return false, nil, nil, fmt.Errorf("current metadata corrupt: %w", err)
return nil, fmt.Errorf("unable to parse db listing: %v", err)
}

if current.IsSupersededBy(updateEntry) {
updateEntry := listing.BestUpdate(c.targetSchema)
if updateEntry == nil {
return nil, fmt.Errorf("no db candidates with correct version available (maybe there is an application update available?)")
}
log.Debugf("found database update candidate: %s", updateEntry)

if current == nil || current.IsSupersededBy(updateEntry) {
log.Debugf("database update available: %s", updateEntry)
return true, current, updateEntry, nil
return updateEntry, nil
}
log.Debugf("no database update available")

return false, nil, nil, nil
return nil, nil
}

// UpdateTo updates the existing DB with the specific other version provided from a listing entry.
Expand Down Expand Up @@ -369,7 +421,18 @@ func (c *Curator) activate(dbDirPath string) error {
}

// activate the new db cache
return file.CopyDir(c.fs, dbDirPath, c.dbDir)
err = file.CopyDir(c.fs, dbDirPath, c.dbDir)
if err != nil {
return err
}

// update the timestamp indicating when this db was downloaded
err = c.updateMetadataTimestamp()
if err != nil {
log.Debugf("unable to update metadata: %v", err)
}

return nil
}

// ListingFromURL loads a Listing from a URL.
Expand Down
2 changes: 1 addition & 1 deletion grype/db/curator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -415,7 +415,7 @@ func TestCuratorTimeoutBehavior(t *testing.T) {
stage := progress.NewAtomicStage("some-stage")

runTheTest := func(success chan struct{}, errs chan error) {
_, _, _, err = curator.IsUpdateAvailable()
_, err = curator.GetUpdate(curator.GetMetadata())
if err == nil {
errs <- errors.New("expected timeout error but got nil")
return
Expand Down
8 changes: 7 additions & 1 deletion grype/db/listing.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package db
import (
"encoding/json"
"fmt"
"io"
"os"
"sort"

Expand Down Expand Up @@ -48,8 +49,13 @@ func NewListingFromFile(fs afero.Fs, path string) (Listing, error) {
}
defer f.Close()

return NewListingFromReader(f)
}

// NewListingFromReader loads a Listing from a given filepath.
func NewListingFromReader(reader io.Reader) (Listing, error) {
var l Listing
err = json.NewDecoder(f).Decode(&l)
err := json.NewDecoder(reader).Decode(&l)
if err != nil {
return Listing{}, fmt.Errorf("unable to parse DB listing: %w", err)
}
Expand Down
14 changes: 13 additions & 1 deletion grype/db/metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@ const MetadataFileName = "metadata.json"
// verify the contents (checksum).
type Metadata struct {
Built time.Time
Updated time.Time
Version int
Checksum string
}

// MetadataJSON is a helper struct for parsing and assembling Metadata objects to and from JSON.
type MetadataJSON struct {
Built string `json:"built"` // RFC 3339
Built string `json:"built"` // RFC 3339
Updated string `json:"updated"` // RFC 3339
Version int `json:"version"`
Checksum string `json:"checksum"`
}
Expand All @@ -37,8 +39,16 @@ func (m MetadataJSON) ToMetadata() (Metadata, error) {
return Metadata{}, fmt.Errorf("cannot convert built time (%s): %+v", m.Built, err)
}

updated, err := time.Parse(time.RFC3339, m.Updated)
if err != nil {
// database build + delay to update when last modified time occurs is ~1 hour,
// so we can use the build time as a reasonable default
updated = build.Add(1 * time.Hour)
}

metadata := Metadata{
Built: build.UTC(),
Updated: updated.UTC(),
Version: m.Version,
Checksum: m.Checksum,
}
Expand Down Expand Up @@ -71,6 +81,7 @@ func NewMetadataFromDir(fs afero.Fs, dir string) (*Metadata, error) {
if err != nil {
return nil, fmt.Errorf("unable to parse DB metadata (%s): %w", metadataFilePath, err)
}

return &m, nil
}

Expand Down Expand Up @@ -120,6 +131,7 @@ func (m Metadata) String() string {
func (m Metadata) Write(toPath string) error {
metadata := MetadataJSON{
Built: m.Built.UTC().Format(time.RFC3339),
Updated: m.Updated.UTC().Format(time.RFC3339),
Version: m.Version,
Checksum: m.Checksum,
}
Expand Down
17 changes: 15 additions & 2 deletions grype/db/metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
)

func TestMetadataParse(t *testing.T) {
timeUTC := time.Date(2020, 06, 15, 14, 02, 36, 0, time.UTC)
timeEDT := time.Date(2020, 06, 15, 18, 02, 36, 0, time.UTC)
tests := []struct {
fixture string
expected *Metadata
Expand All @@ -17,15 +19,26 @@ func TestMetadataParse(t *testing.T) {
{
fixture: "test-fixtures/metadata-gocase",
expected: &Metadata{
Built: time.Date(2020, 06, 15, 14, 02, 36, 0, time.UTC),
Built: timeUTC,
Updated: timeUTC.Add(1 * time.Hour),
Version: 2,
Checksum: "sha256:dcd6a285c839a7c65939e20c251202912f64826be68609dfc6e48df7f853ddc8",
},
},
{
fixture: "test-fixtures/metadata-edt-timezone",
expected: &Metadata{
Built: time.Date(2020, 06, 15, 18, 02, 36, 0, time.UTC),
Built: timeEDT,
Updated: timeEDT,
Version: 2,
Checksum: "sha256:dcd6a285c839a7c65939e20c251202912f64826be68609dfc6e48df7f853ddc8",
},
},
{
fixture: "test-fixtures/metadata-updated",
expected: &Metadata{
Built: timeEDT,
Updated: timeUTC,
Version: 2,
Checksum: "sha256:dcd6a285c839a7c65939e20c251202912f64826be68609dfc6e48df7f853ddc8",
},
Expand Down
1 change: 1 addition & 0 deletions grype/db/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import "time"

type Status struct {
Built time.Time `json:"built"`
Updated time.Time `json:"updated"`
SchemaVersion int `json:"schemaVersion"`
Location string `json:"location"`
Checksum string `json:"checksum"`
Expand Down
7 changes: 7 additions & 0 deletions grype/db/test-fixtures/metadata-updated/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"built": "2020-06-15T14:02:36-04:00",
"updated": "2020-06-15T14:02:36Z",
"last-check": "2020-06-15T14:02:36-04:00",
"version": 2,
"checksum": "sha256:dcd6a285c839a7c65939e20c251202912f64826be68609dfc6e48df7f853ddc8"
}
Loading