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

Minor cataloger improvements for the CLI #1831

Closed

Conversation

jsquyres
Copy link
Contributor

  1. Add ALL (synonym for all), ALL:IMAGE, and ALL:DIRECTORY catalog selectors (somewhat in response to Why are different catalogers disabled (by default) in different scenarios? #1776)
  2. Add some more INFO and DEBUG log messages about which catalogers were selected.
  3. Add a sanity check: if 0 catalogers are selected, emit an error.
    • Also make the Syft CLI fail faster if 0 catalogers are selected

See the individual commits and their corresponding commit messages for details.

A few minor improvements to the handling of how catalogers are
selected:

- Add --catalogers keyword "ALL" (which is a synonym for "all")
- Add --catalogers keyword "ALL:IMAGE" which selects the default set
  of catalogers used with images
- Add --catalogers keyword "ALL:DIRECTORY" which selects the default
  set of catalogers used with directories
- Add INFO logging messages that indicate which category of catalogers
  were selected
- Add a sanity check: if 0 catalogers are selected, emit an error
- Add DEBUG logging message to show a comma-delimited list of
  the catalogers that were finally selected (suitable for use with the
  --catalogers CLI option)

Also updated the README.md to document the `ALL*` keywords and clarify
a few points about cataloger selection.

Signed-off-by: Jeff Squyres <jeff@squyres.com>
Move the selection/initialization of the catalogers up before the
initialization of the FileResolver.  This allows failing fast if no
catalogers end up getting selected -- potentially before a (slow)
initialization of the FileResolver.

Signed-off-by: Jeff Squyres <jeff@squyres.com>
@kzantow
Copy link
Contributor

kzantow commented May 19, 2023

Hi @jsquyres thank you for this PR -- I'd like to note that there is another upcoming change (which is taking a while to get to completion) that offers some similar, but possibly better options for enabling certain sets of catalogers: #1383 -- would this work for your use case?

@jsquyres
Copy link
Contributor Author

@kzantow Interesting. I'm not deep enough into the zen of Syft to fully grok how #1383 will result in what you're saying, but you seemed to make reference to the same capability in #1776 (comment).

Our use case is that we have entirely scanned images up to this point, but now we need to scan a filesystem. We were a little surprised when scanning an image and a corresponding equivalent filesystem turned up different results (which led to my question in #1776). Reading the docs and digging in to the source code led to better understanding and resulted in this PR. To be clear: by examining the source of Syft v0.79.0 and using the --catalogs CLI option, we were able to get the results to be the same. We can certainly keep doing that, but it's a little tedious to have to keep examining the source code or closely examine all the output from Syft to see exactly which catalogers were used in a given run.

@jsquyres
Copy link
Contributor Author

@kzantow Ping.

@wagoodman
Copy link
Contributor

wagoodman commented Jun 15, 2023

@jsquyres though #1383 is stale, it's quite important to the syft 1.0 work, which I think will be landing within the next 3-ish months. The feature isn't completed in that PR yet, but the API will be accounting for this. There is a follow up task in the PR description to consider #1039 and #1731 . #1383 has implemented a tag-based cataloger selection approach in the API, the next step is to wire it up to the --cataloger CLI option. I'll try to update here with an example usage once the CLI feature in #1383 is implemented.

@jsquyres
Copy link
Contributor Author

@wagoodman @kzantow Using a tag-based cataloger selection sounds fine to me. If I remove the ALL* stuff from this PR, would the other improvements be helpful? (e.g., the docs changes, logging messages, and fail fast)

@kzantow
Copy link
Contributor

kzantow commented Jun 22, 2023

Thanks very much for the contribution @jsquyres. It's really great that you've been improving Syft, but the unfortunate timing of these PRs just so happened to overlap with a couple other analogous but different changes. I don't think there's a lot from this PR that we will be able to use, but we really appreciate the contribution and would encourage you to reach out if there are other things you'd like to work on!

@kzantow kzantow closed this Jun 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants