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

vex: account for package module when parsing VEX #1416

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

crozzy
Copy link
Contributor

@crozzy crozzy commented Sep 18, 2024

Previously, we'd made the decision to ignore the package module and rely on the repo and package for matching but now that the VEX data has the package module data and it helps filter the results on the DB side, it seems best to add the package module information and keep the matcher constraints as it is. Without this we'd need change the RHEL matcher to remove the PackageModule constraint.

@crozzy crozzy requested a review from a team as a code owner September 18, 2024 18:52
@crozzy crozzy requested review from hdonnay and removed request for a team September 18, 2024 18:52
@crozzy crozzy marked this pull request as draft September 18, 2024 18:53
@crozzy crozzy force-pushed the account-for-module-vex branch 3 times, most recently from 6aac3ad to e4b64e8 Compare September 18, 2024 19:07
Copy link

codecov bot commented Sep 18, 2024

Codecov Report

Attention: Patch coverage is 70.31250% with 19 lines in your changes missing coverage. Please review.

Project coverage is 55.43%. Comparing base (7088f7b) to head (a3c2044).

Files with missing lines Patch % Lines
rhel/vex/parser.go 70.31% 15 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1416      +/-   ##
==========================================
+ Coverage   55.41%   55.43%   +0.01%     
==========================================
  Files         282      282              
  Lines       17890    17934      +44     
==========================================
+ Hits         9914     9941      +27     
- Misses       6934     6948      +14     
- Partials     1042     1045       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@crozzy crozzy force-pushed the account-for-module-vex branch 2 times, most recently from 5e7886d to 033b09c Compare September 19, 2024 15:34
@crozzy crozzy marked this pull request as ready for review September 19, 2024 19:50
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser_test.go Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
@crozzy crozzy force-pushed the account-for-module-vex branch 2 times, most recently from 3e77536 to b34a500 Compare September 20, 2024 22:26
}
if version, _, found := strings.Cut(purl.Version, ":"); found {
modName = purl.Name + ":" + version
} else {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RTann adding this clause as some of the PURLs don't conform to a standard format and will be potentially updated:
Conventional: pkg:rpmmod/redhat/postgresql@13:8080020231114105206:63b34585
Unconventional: pkg:rpmmod/redhat/postgresql:15/postgresql
Possible update for unconventional format: pkg:rpmmod/redhat/postgresql@15

This is being looked at by RH prodsec.

Copy link
Contributor

Choose a reason for hiding this comment

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

Based on https://go.dev/play/p/PamAwGzH8r0 I think you can just do

version, _, _ := strings.Cut(purl.Version, ":")
return purl.Name + ":" + version, nil

Copy link
Contributor

Choose a reason for hiding this comment

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

This is being looked at by RH prodsec.

Is this blocked, then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure there is a point in editing it until we get a reply about whether this is:

  1. A bug and will be fixed
  2. A bug and won't be immediately fixed
  3. Not a but and won't be changed

If we don't get that answer soon I think we just account for the conventional way and possibly modify in the future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been a couple of days with no reply, going to change to just account for conventional rpmmod PURLs

@crozzy crozzy requested a review from RTann September 20, 2024 22:30
@crozzy crozzy force-pushed the account-for-module-vex branch 3 times, most recently from 59434dd to a3c2044 Compare September 23, 2024 19:12
}
if version, _, found := strings.Cut(purl.Version, ":"); found {
modName = purl.Name + ":" + version
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Based on https://go.dev/play/p/PamAwGzH8r0 I think you can just do

version, _, _ := strings.Cut(purl.Version, ":")
return purl.Name + ":" + version, nil

rhel/vex/parser.go Outdated Show resolved Hide resolved
}
if version, _, found := strings.Cut(purl.Version, ":"); found {
modName = purl.Name + ":" + version
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is being looked at by RH prodsec.

Is this blocked, then?

rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Outdated Show resolved Hide resolved
rhel/vex/parser.go Show resolved Hide resolved
@crozzy crozzy force-pushed the account-for-module-vex branch 2 times, most recently from dc451f0 to 1114dae Compare October 1, 2024 17:05
@crozzy crozzy requested a review from RTann October 1, 2024 17:31
RTann
RTann previously approved these changes Oct 4, 2024
Copy link
Contributor

@RTann RTann left a comment

Choose a reason for hiding this comment

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

one minor nit otherwise LGTM

rhel/vex/parser.go Outdated Show resolved Hide resolved
RTann
RTann previously approved these changes Oct 4, 2024
rhel/vex/parser.go Outdated Show resolved Hide resolved
Previously, we'd made the decision to ignore the package module and rely
on the repo and package for matching but now that the VEX data has the
package module data and it helps filter the results on the DB side, it
seems best to add the package module information and keep the matcher
constraints as it is.

Signed-off-by: crozzy <[email protected]>
@crozzy
Copy link
Contributor Author

crozzy commented Oct 4, 2024

/fast-forward

@github-actions github-actions bot merged commit 54959d6 into quay:main Oct 4, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants