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

ROX-28098: explicitly set advisory #14284

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

ROX-28098: explicitly set advisory #14284

wants to merge 2 commits into from

Conversation

RTann
Copy link
Contributor

@RTann RTann commented Feb 14, 2025

Description

Central DB's image and CVE data models are going through a metamorphosis now, and this will help. The goal is to show CVEs along with their associated advisory, if applicable, which this PR enables.

Note: this currently only affects Red Hat vulnerability data, and not GHSAs, DSAs, ALAS, etc. The current goal is to just do this for Red Hat data, at this time.

User-facing documentation

  • CHANGELOG update is not needed
  • documentation PR is not needed

Testing and quality

  • the change is production ready: the change is GA or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests

How I validated my change

CI

Copy link

openshift-ci bot commented Feb 14, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@rhacs-bot
Copy link
Contributor

rhacs-bot commented Feb 14, 2025

Images are ready for the commit at a08c71a.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.8.x-22-ga08c71ad43.

@RTann
Copy link
Contributor Author

RTann commented Feb 14, 2025

Postgres tests are failing because we are not writing the advisory to the table. Due to the data model changes, I don't know the best place to put this (CVE, ImageCVE, ImageCVEV2?)

Can someone from @stackrox/core-workflows let me know where to put it?

@RTann RTann marked this pull request as ready for review February 14, 2025 19:07
@RTann RTann requested review from a team as code owners February 14, 2025 19:07
// advisory returns the vulnerability's related advisory.
//
// Only Red Hat advisories (RHSA/RHBA/RHEA) are supported at this time.
func advisory(vuln *claircore.Vulnerability) string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@crozzy can you confirm we can/should fetch the advisory from the links?

Copy link

Choose a reason for hiding this comment

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

There's no API guarantee for the format/contents of the links field, having said that:

  • I don't think there is any other way to tie them together without pulling the VEX CVE file and looking it up yourself.
  • There is no plans to change the format until the Matcher v2 schema changes are updated that should render hacks like this moot.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

works for me. Seems like this is the best way to do this using Claircore's native data

@RTann RTann requested a review from crozzy February 14, 2025 19:09
@dashrews78
Copy link
Contributor

dashrews78 commented Feb 14, 2025

Postgres tests are failing because we are not writing the advisory to the table. Due to the data model changes, I don't know the best place to put this (CVE, ImageCVE, ImageCVEV2?)

Can someone from @stackrox/core-workflows let me know where to put it?

IMO, go to the image store test and add something like what I did in #13682. You can even reference the same ticket if you want and I'll deal with it soon. Minimizes the interference of proto churn and keeps your PR focused.

	for _, comp := range image.GetScan().GetComponents() {
		for _, vuln := range comp.GetVulns() {
			vuln.NvdCvss = 0
			vuln.Epss = nil
		}
		// TODO(ROX-27402) remove this
		comp.Architecture = ""
	}

We are going to have to create a new version of that store and test anyway so there is no real point on spending much thought on it for the scanner side of the changes.

Copy link

openshift-ci bot commented Feb 15, 2025

@RTann: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocp-4-17-scanner-v4-install-tests a08c71a link false /test ocp-4-17-scanner-v4-install-tests
ci/prow/ocp-4-12-scanner-v4-install-tests a08c71a link false /test ocp-4-12-scanner-v4-install-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

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

Successfully merging this pull request may close these issues.

4 participants