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.

Sign up for GitHub

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

fix: enable http timeout #1777

Merged
merged 4 commits into from
Apr 4, 2024
Merged

fix: enable http timeout #1777

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f0d6ed3
fix: enable http timeout
willmurphyscode Apr 1, 2024
09449fa
separate timeouts for listing and database download
willmurphyscode Apr 2, 2024
7a21a6d
assign timeout after checking err is nil
willmurphyscode Apr 2, 2024
c2109ef
add unit test for timeout behavior
willmurphyscode Apr 2, 2024
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
28 changes: 20 additions & 8 deletions cmd/grype/cli/options/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,33 @@ import (
)

type Database struct {
Dir string `yaml:"cache-dir" json:"cache-dir" mapstructure:"cache-dir"`
UpdateURL string `yaml:"update-url" json:"update-url" mapstructure:"update-url"`
CACert string `yaml:"ca-cert" json:"ca-cert" mapstructure:"ca-cert"`
AutoUpdate bool `yaml:"auto-update" json:"auto-update" mapstructure:"auto-update"`
ValidateByHashOnStart bool `yaml:"validate-by-hash-on-start" json:"validate-by-hash-on-start" mapstructure:"validate-by-hash-on-start"`
ValidateAge bool `yaml:"validate-age" json:"validate-age" mapstructure:"validate-age"`
MaxAllowedBuiltAge time.Duration `yaml:"max-allowed-built-age" json:"max-allowed-built-age" mapstructure:"max-allowed-built-age"`
Dir string `yaml:"cache-dir" json:"cache-dir" mapstructure:"cache-dir"`
UpdateURL string `yaml:"update-url" json:"update-url" mapstructure:"update-url"`
CACert string `yaml:"ca-cert" json:"ca-cert" mapstructure:"ca-cert"`
AutoUpdate bool `yaml:"auto-update" json:"auto-update" mapstructure:"auto-update"`
ValidateByHashOnStart bool `yaml:"validate-by-hash-on-start" json:"validate-by-hash-on-start" mapstructure:"validate-by-hash-on-start"`
ValidateAge bool `yaml:"validate-age" json:"validate-age" mapstructure:"validate-age"`
MaxAllowedBuiltAge time.Duration `yaml:"max-allowed-built-age" json:"max-allowed-built-age" mapstructure:"max-allowed-built-age"`
UpdateAvailableTimeout time.Duration `yaml:"update-available-timeout" json:"update-available-timeout" mapstructure:"update-available-timeout"`
UpdateDownloadTimeout time.Duration `yaml:"update-download-timeout" json:"update-download-timeout" mapstructure:"update-download-timeout"`
}

const (
defaultMaxDBAge time.Duration = time.Hour * 24 * 5
defaultUpdateAvailableTimeout = time.Second * 30
defaultUpdateDownloadTimeout = time.Second * 120
)

func DefaultDatabase(id clio.Identification) Database {
return Database{
Dir: path.Join(xdg.CacheHome, id.Name, "db"),
UpdateURL: internal.DBUpdateURL,
AutoUpdate: true,
ValidateAge: true,
// After this period (5 days) the db data is considered stale
MaxAllowedBuiltAge: time.Hour * 24 * 5,
MaxAllowedBuiltAge: defaultMaxDBAge,
UpdateAvailableTimeout: defaultUpdateAvailableTimeout,
UpdateDownloadTimeout: defaultUpdateDownloadTimeout,
}
}

Expand All @@ -40,5 +50,7 @@ func (cfg Database) ToCuratorConfig() db.Config {
ValidateByHashOnGet: cfg.ValidateByHashOnStart,
ValidateAge: cfg.ValidateAge,
MaxAllowedBuiltAge: cfg.MaxAllowedBuiltAge,
ListingFileTimeout: cfg.UpdateAvailableTimeout,
UpdateTimeout: cfg.UpdateDownloadTimeout,
}
}
22 changes: 17 additions & 5 deletions grype/db/curator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,14 @@ type Config struct {
ValidateByHashOnGet bool
ValidateAge bool
MaxAllowedBuiltAge time.Duration
ListingFileTimeout time.Duration
UpdateTimeout time.Duration
}

type Curator struct {
fs afero.Fs
downloader file.Getter
listingDownloader file.Getter
updateDownloader file.Getter
targetSchema int
dbDir string
dbPath string
Expand All @@ -55,15 +58,23 @@ func NewCurator(cfg Config) (Curator, error) {
dbDir := path.Join(cfg.DBRootDir, strconv.Itoa(vulnerability.SchemaVersion))

fs := afero.NewOsFs()
httpClient, err := defaultHTTPClient(fs, cfg.CACert)
listingClient, err := defaultHTTPClient(fs, cfg.CACert)
willmurphyscode marked this conversation as resolved.
Show resolved Hide resolved
listingClient.Timeout = cfg.ListingFileTimeout
if err != nil {
return Curator{}, err
}

dbClient, err := defaultHTTPClient(fs, cfg.CACert)
dbClient.Timeout = cfg.UpdateTimeout
if err != nil {
return Curator{}, err
}

return Curator{
fs: fs,
targetSchema: vulnerability.SchemaVersion,
downloader: file.NewGetter(httpClient),
listingDownloader: file.NewGetter(listingClient),
updateDownloader: file.NewGetter(dbClient),
dbDir: dbDir,
dbPath: path.Join(dbDir, FileName),
listingURL: cfg.ListingURL,
Expand Down Expand Up @@ -283,7 +294,7 @@ func (c *Curator) download(listing *ListingEntry, downloadProgress *progress.Man
url.RawQuery = query.Encode()

// go-getter will automatically extract all files within the archive to the temp dir
err = c.downloader.GetToDir(tempDir, listing.URL.String(), downloadProgress)
err = c.updateDownloader.GetToDir(tempDir, listing.URL.String(), downloadProgress)
if err != nil {
return "", fmt.Errorf("unable to download db: %w", err)
}
Expand Down Expand Up @@ -375,7 +386,7 @@ func (c Curator) ListingFromURL() (Listing, error) {
}()

// download the listing file
err = c.downloader.GetFile(tempFile.Name(), c.listingURL)
err = c.listingDownloader.GetFile(tempFile.Name(), c.listingURL)
if err != nil {
return Listing{}, fmt.Errorf("unable to download listing: %w", err)
}
Expand All @@ -390,6 +401,7 @@ func (c Curator) ListingFromURL() (Listing, error) {

func defaultHTTPClient(fs afero.Fs, caCertPath string) (*http.Client, error) {
httpClient := cleanhttp.DefaultClient()
httpClient.Timeout = 30 * time.Second
if caCertPath != "" {
rootCAs := x509.NewCertPool()

Expand Down
12 changes: 9 additions & 3 deletions grype/db/curator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,14 @@ func newTestCurator(tb testing.TB, fs afero.Fs, getter file.Getter, dbDir, metad

require.NoError(tb, err)

c.downloader = getter
c.listingDownloader = getter
c.updateDownloader = getter
c.fs = fs

return c
}

func Test_defaultHTTPClient(t *testing.T) {
func Test_defaultHTTPClientHasCert(t *testing.T) {
tests := []struct {
name string
hasCert bool
Expand Down Expand Up @@ -105,11 +106,16 @@ func Test_defaultHTTPClient(t *testing.T) {
} else {
assert.Nil(t, httpClient.Transport.(*http.Transport).TLSClientConfig)
}

})
}
}

func Test_defaultHTTPClientTimeout(t *testing.T) {
c, err := defaultHTTPClient(afero.NewMemMapFs(), "")
require.NoError(t, err)
assert.Equal(t, 30*time.Second, c.Timeout)
}

func generateCertFixture(t *testing.T) string {
path := "test-fixtures/tls/server.crt"
if _, err := os.Stat(path); !os.IsNotExist(err) {
Expand Down
Loading