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

Jump to bottom

Fail when grype cant check for db update #1247

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fail when grype cant check for db update #1247

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion grype/db/curator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,9 @@ func (c *Curator) Update() (bool, error) {

updateAvailable, metadata, updateEntry, err := c.IsUpdateAvailable()
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)
return false, fmt.Errorf("unable to update vulnerability database: %w", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we may want to only return an error if there is no database to use after this check

Copy link
Contributor Author

@shanedell shanedell Apr 20, 2023 •
edited

Choose a reason for hiding this comment

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

But wouldn't you still want return an error if unable to update the database? For example if I had an old outdated database locally and tried to update it but it failed to update, say from a dns error like in this example. If the error is not thrown the command would return that No vulnerability database update available which would be misleading. So this could also cause the user to believe their outdated database is up to date as the returned value of No vulnerability database update available, at least to me, makes me think I have the latest database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kzantow Or would something like this work?

stage.Current = "no update available"
updateAvailable, metadata, updateEntry, err := c.IsUpdateAvailable()

if err != nil {
    log.Warnf("unable to check for vulnerability database update")
    log.Debugf("check for vulnerability update failed: %+v", err)
    stage.Current = fmt.Sprintf("unable to update vulnerability database: %w", err)
}

...

if stage.Current != "no update available" {
    return false, errors.New(state.Current)
} else {
    return false, nil
}

?

}
if updateAvailable {
log.Infof("downloading new vulnerability DB")
Expand Down