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

Fix cataloger selection to be more specific #1582

Merged
merged 1 commit into from
Feb 17, 2023

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Feb 17, 2023

Today the cataloger selection allows for partial name matches, so input such as go will select all go catalogers:

  • go-mod-file-cataloger
  • go-module-binary-cataloger

Unfortunately the current implementation takes a simplistic approach (a simple substring check) which causes additional false selections across ecosystems, which would most likely be unintentional. For instance, the same go search would additionally select:

  • rust-cargo-lock-cataloger
  • cargo-auditable-binary-cataloger

This PR applies a workaround to make this behavior a little better before #1383 lands in a release (which fixes this behavior entirely).

The new behavior restricts the substring check to only allow for full words, separated by -. This still allows for the current selection behavior today but fixes all of the (incorrect) cross selections I could find.

Closes #1573

@wagoodman wagoodman added the bug Something isn't working label Feb 17, 2023
@wagoodman wagoodman self-assigned this Feb 17, 2023
@wagoodman wagoodman requested a review from a team February 17, 2023 14:49
@github-actions
Copy link

github-actions bot commented Feb 17, 2023

Benchmark Test Results

Benchmark results from the latest changes vs base branch
goos: linux
goarch: amd64
pkg: github.com/anchore/syft/test/integration
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
                                                          │ ./.tmp/benchmark-9af296a.txt │
                                                          │            sec/op            │
ImagePackageCatalogers/alpmdb-cataloger-2                                   11.71m ± 25%
ImagePackageCatalogers/ruby-gemspec-cataloger-2                             845.4µ ±  5%
ImagePackageCatalogers/python-package-cataloger-2                           2.972m ±  0%
ImagePackageCatalogers/php-composer-installed-cataloger-2                   636.7µ ±  2%
ImagePackageCatalogers/javascript-package-cataloger-2                       347.3µ ±  3%
ImagePackageCatalogers/dpkgdb-cataloger-2                                   467.5µ ±  3%
ImagePackageCatalogers/rpm-db-cataloger-2                                   434.9µ ±  1%
ImagePackageCatalogers/java-cataloger-2                                     10.44m ±  2%
ImagePackageCatalogers/graalvm-native-image-cataloger-2                     7.587µ ±  2%
ImagePackageCatalogers/apkdb-cataloger-2                                    452.7µ ±  0%
ImagePackageCatalogers/go-module-binary-cataloger-2                         18.01µ ±  1%
ImagePackageCatalogers/dotnet-deps-cataloger-2                              947.4µ ±  2%
ImagePackageCatalogers/portage-cataloger-2                                  276.0µ ±  1%
ImagePackageCatalogers/sbom-cataloger-2                                     102.4µ ±  1%
ImagePackageCatalogers/binary-cataloger-2                                   136.9µ ±  0%
geomean                                                                     429.6µ

                                                          │ ./.tmp/benchmark-9af296a.txt │
                                                          │             B/op             │
ImagePackageCatalogers/alpmdb-cataloger-2                                   5.061Mi ± 0%
ImagePackageCatalogers/ruby-gemspec-cataloger-2                             141.8Ki ± 0%
ImagePackageCatalogers/python-package-cataloger-2                           947.3Ki ± 0%
ImagePackageCatalogers/php-composer-installed-cataloger-2                   155.9Ki ± 0%
ImagePackageCatalogers/javascript-package-cataloger-2                       95.89Ki ± 0%
ImagePackageCatalogers/dpkgdb-cataloger-2                                   144.6Ki ± 0%
ImagePackageCatalogers/rpm-db-cataloger-2                                   170.8Ki ± 0%
ImagePackageCatalogers/java-cataloger-2                                     2.724Mi ± 0%
ImagePackageCatalogers/graalvm-native-image-cataloger-2                     1.523Ki ± 0%
ImagePackageCatalogers/apkdb-cataloger-2                                    123.1Ki ± 0%
ImagePackageCatalogers/go-module-binary-cataloger-2                         3.102Ki ± 0%
ImagePackageCatalogers/dotnet-deps-cataloger-2                              313.9Ki ± 0%
ImagePackageCatalogers/portage-cataloger-2                                  75.52Ki ± 0%
ImagePackageCatalogers/sbom-cataloger-2                                     13.06Ki ± 0%
ImagePackageCatalogers/binary-cataloger-2                                   20.56Ki ± 0%
geomean                                                                     106.7Ki

                                                          │ ./.tmp/benchmark-9af296a.txt │
                                                          │          allocs/op           │
ImagePackageCatalogers/alpmdb-cataloger-2                                    86.71k ± 0%
ImagePackageCatalogers/ruby-gemspec-cataloger-2                              2.159k ± 0%
ImagePackageCatalogers/python-package-cataloger-2                            15.49k ± 0%
ImagePackageCatalogers/php-composer-installed-cataloger-2                    3.457k ± 0%
ImagePackageCatalogers/javascript-package-cataloger-2                        1.253k ± 0%
ImagePackageCatalogers/dpkgdb-cataloger-2                                    2.646k ± 0%
ImagePackageCatalogers/rpm-db-cataloger-2                                    3.759k ± 0%
ImagePackageCatalogers/java-cataloger-2                                      38.27k ± 0%
ImagePackageCatalogers/graalvm-native-image-cataloger-2                       40.00 ± 0%
ImagePackageCatalogers/apkdb-cataloger-2                                     3.252k ± 0%
ImagePackageCatalogers/go-module-binary-cataloger-2                           101.0 ± 0%
ImagePackageCatalogers/dotnet-deps-cataloger-2                               5.010k ± 0%
ImagePackageCatalogers/portage-cataloger-2                                   1.487k ± 0%
ImagePackageCatalogers/sbom-cataloger-2                                       392.0 ± 0%
ImagePackageCatalogers/binary-cataloger-2                                     609.0 ± 0%
geomean                                                                      2.170k

@wagoodman wagoodman enabled auto-merge (squash) February 17, 2023 14:58
@spiffcs
Copy link
Contributor

spiffcs commented Feb 17, 2023

@wagoodman code looks good - there is a weird error in CI whee the the AT won't pass

Signed-off-by: Alex Goodman <alex.goodman@anchore.com>
@wagoodman wagoodman force-pushed the workaround-cataloger-selection branch from 4047734 to 93eee4d Compare February 17, 2023 15:30
@wagoodman
Copy link
Contributor Author

CI issue workaround applied, see #1584

@wagoodman wagoodman merged commit f671609 into main Feb 17, 2023
@wagoodman wagoodman deleted the workaround-cataloger-selection branch February 17, 2023 15:35
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.

Cataloger filtering cross matches wrong catalogers
2 participants