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

Remove the VulnerabilityMetadataProvider #2456

Open
kzantow opened this issue Feb 18, 2025 · 0 comments
Open

Remove the VulnerabilityMetadataProvider #2456

kzantow opened this issue Feb 18, 2025 · 0 comments
Milestone

Comments

@kzantow
Copy link
Contributor

kzantow commented Feb 18, 2025

In order to minimize changes for v6, there was not much changed between Vulnerability and VulnerabilityMetadata: FindVulnerabilities returns Vulnerability objects, and these are later used to fetch VulnerabilityMetadata. I believe the reason for this split was to help performance in v5 databases, where a list of vulnerabilities was returned prior to any filtering, including version filtering. This can be an expensive and unnecessary process to hydrate these with data from the database, since many queries were returning hundreds of records only to have them filtered out due to version constraints that don't match and hydrating all the data for these vulnerabilities including the metadata can be expensive, especially when there are thousands of packages returning hundreds of vulnerabilities each. However, v6 optimizes fetching in some different ways, and I believe it should simply fill out the VulnerabilityMetadata on returned Vulnerabilities as the last step before returning them, since this data is almost always used later -- it's the only way to get a severity, for example, which I think is used by every output format. We could both simplify the VulnerabilityProvider interface and make the code more robust since it doesn't have to look up metadata later, potentially returning nothing -- a process that requires metadata would simply be able to access this on the vulnerability directly. I believe this change could also allow us to remove the Internal field on the vulnerability Reference object, since we wouldn't need to do further lookups in the database.

@kzantow kzantow added this to the Grype 1.0 milestone Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

1 participant