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

Updaters: failed update stores 'fingerprint' (etag), a retry doesn't happen unless source changes #959

Open
frostmar opened this issue Jun 5, 2023 · 2 comments

Comments

@frostmar
Copy link
Contributor

frostmar commented Jun 5, 2023

I think there's an issue in how updaters are run, that can lead to missing vulnerabilities:

When an error happens while storing vulnerabilities, vuln table rows aren't created. Future update runs don't create those vulns unless the source changes, because 'fingerprint' logic skips the parse/store from vuln sources that haven't changed. This means vulnerabilities could be missing for long periods. Some vuln sources (eg historic alpine releases) are rarely or never updated, so missing vulns could be permanent.

Details

driveUpdater() sequences the fetch/parse/store of vulns from each source:

func (m *Manager) driveUpdater(ctx context.Context, u driver.Updater) (err error) {

the 'fingerprint' (eg etag) of the last source state seen is fetched from the latest update_operation row

opmap, err := m.store.GetUpdateOperations(ctx, uoKind, name)
if err != nil {
return
}
if s := opmap[name]; len(s) > 0 {
prevFP = s[0].Fingerprint
}

... however that update_operation may not have been successful. The fingerprint looks to be stored in the update_operation whether or not errors were encountered.

Example

In some of our environments, we had fewer vuln table entries than expected for updaters such as alpine-community-v3.4-updater, alpine-community-v3.5-updater, alpine-community-v3.9-updater.

The update_status table rows for those updaters had last_error of failed to update: failed to commit transaction: conn closed or failed to begin transaction: write failed: write tcp

Regular updates were started for those updaters, but did not create the missing vulns because the source fingerprint had not changed.

Hacking the 'fingerprint' by editing the last update_operation row caused the updaters to run a full fetch/parse/store, and missing vulns were created.

@frostmar
Copy link
Contributor Author

frostmar commented Jun 5, 2023

Perhaps the update_operation row created should only contain a fingerprint if the update was completely successful?

@crozzy
Copy link
Contributor

crozzy commented Jun 5, 2023

I think this is a legitimate concern especially now we're using the latest_vuln view to query for vulnerabilities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants