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

feat: add custom maven comparator #1571

Merged
merged 22 commits into from
Oct 27, 2023

Conversation

spiffcs
Copy link
Contributor

@spiffcs spiffcs commented Oct 24, 2023

Summary

Fix for #1526.

This PR takes the recommendation from #1526 and adapts the go-mvn-version to be used as a custom comparator for matching against packages that have the JavaPkg type. Packages of type JavaPkg will no longer use the stock matcher.

The specific case mentioned in the issue has been included in both new test harnesses.

Here is a screenshot of the issues test case working as intended with the versions for jenkins-core being correctly compared. The FP is eliminated:
Screenshot 2023-10-25 at 1 50 09 PM

Related Labeled PR for updating the quality gate:

Local Sample showing FP improvements with this change:
Screenshot 2023-10-25 at 10 00 34 PM

Todo

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs changed the title feat: add outline for new Java comparator feat: add correct java comparator Oct 24, 2023
@spiffcs spiffcs changed the title feat: add correct java comparator feat: add custom java comparator Oct 24, 2023
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs linked an issue Oct 24, 2023 that may be closed by this pull request
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs marked this pull request as ready for review October 25, 2023 03:26
…ions

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
grype/version/format.go Outdated Show resolved Hide resolved
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs force-pushed the 1526-incorrect-version-comparisons-maven branch from 46ae774 to 4d54c0a Compare October 25, 2023 15:12
@spiffcs spiffcs changed the title feat: add custom java comparator feat: add custom maven comparator Oct 25, 2023
@spiffcs
Copy link
Contributor Author

spiffcs commented Oct 25, 2023

Putting this back in draft - I think it needs another look as I'm still getting the faulty match mentioned in the issue. While we're testing correctly against the comparator being used as intended in the unit tests, something is not hooked up correctly. The constraint is still behaving as a fuzzy constraint.

@spiffcs spiffcs marked this pull request as draft October 25, 2023 15:44
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs marked this pull request as ready for review October 25, 2023 17:43
@spiffcs
Copy link
Contributor Author

spiffcs commented Oct 25, 2023

ad906d4 updates the fuzzyComparitor to trust the type of version object it accepts as an argument. Trust in this case means that it will attempt to create a temporary comparator based on the verObj type to see if Satisfied is true before falling through to the current path.

This has a minor downside. If the package being examined is not actually of the assumed type(from the verObj) , then the satisfied logic for the newly generated comparator might give an incorrect true result. If it allowed to progress the fuzzy matcher may have returned a correct false result. This all depends on the specific ecosystem's version specification.

The trade off here is that we can now allow new comparators to be tried by trusting the versionObj knows what it's looking for without an update to the version_format field in grype-db. A fuzzyConstraint is generated when this field is not filled.

The best case here is that version_format is already populated and we're using the right comparitor before having any knowledge of the verObj. This change attempts to handle the middle ground a bit better so we don't always need an update to the DB.

@spiffcs
Copy link
Contributor Author

spiffcs commented Oct 26, 2023

Blocked on anchore/yardstick#171

The following is being returned by the quality gate locally:

Failed quality gate
   - current indeterminate matches % is greater than 10%: 
   - current=10.22% image=docker.io/cloudbees/cloudbees-core-oc@sha256:9cd85ee84e401dc27e3a8268aae67b594a651b2f4c7fc056ca14c7b0a0a6b82d

Exploring the labeled data for this image shows that a majority of the Labeled data that is categorized as Unclear contains DISPUTED in the seciton. The above PR removes Unclear from the considered set of unlabeled data.

Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs added the bug Something isn't working label Oct 26, 2023
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
spiffcs and others added 7 commits October 27, 2023 11:51
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Alex Goodman <wagoodman@users.noreply.github.com>
Signed-off-by: Christopher Phillips <christopher.phillips@anchore.com>
@spiffcs spiffcs merged commit 401d67c into main Oct 27, 2023
9 checks passed
@spiffcs spiffcs deleted the 1526-incorrect-version-comparisons-maven branch October 27, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect version comparisons for maven packages
2 participants