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

Introduce trivy clean command and remove cache-clearing flags #6992

Closed
knqyf263 opened this issue Jun 21, 2024 · 9 comments · Fixed by #6993
Closed

Introduce trivy clean command and remove cache-clearing flags #6992

knqyf263 opened this issue Jun 21, 2024 · 9 comments · Fixed by #6993
Assignees
Labels
kind/breaking Categorizes issue or PR as related to breaking compatibility.
Milestone

Comments

@knqyf263
Copy link
Collaborator

knqyf263 commented Jun 21, 2024

Background

Trivy has evolved since its initial release (v0.0.1), operating without subcommands (e.g. trivy debian:11) and using CLI flags like --clear-cache and --reset for cache management (e.g., trivy --clear-cache). As the project grew, several changes were added:

  1. Introduction of subcommands: Trivy now supports various operations through subcommands (e.g., trivy image, trivy fs).
  2. Expansion of cached data: Beyond the initial vulnerability database, Trivy now caches Java databases, check bundles, and more.

Then, several problems occurred.

  1. Inconsistent flag availability: Cache-clearing flags like --reset-checks-bundle are not available in subcommands that don't support misconfigurations scanning (e.g., trivy sbom).
  2. Counterintuitive command structure: The --reset flag is implemented under subcommands rather than as a global flag, leading to unintuitive usage like trivy image --reset even when not scanning images.
  3. Confusing behavior: Using these flags causes Trivy to exit without performing a scan, which can be unexpected for users.
  4. Complex internal logic: The current implementation requires exception handling for cases where image names or other arguments are not needed when these flags are specified.

These factors have led to a situation where it's not intuitive which flags are available for which subcommands, and the overall user experience for cache management has become inconsistent and confusing.

Proposal

To simplify the user experience and internal implementation, I'd propose the following changes:

  1. Remove the following flags:

    • --clear-cache
    • --reset
    • --reset-checks-bundle
  2. Introduce a new trivy clean command, inspired by the go clean command.

Examples

Old

$ trivy config --reset-checks-bundle

New

$ trivy clean --checks-bundle

Old

$ trivy image --reset

New

$ trivy clean --all

The new command supports multiple flags, allowing for more flexible cache management, similar to go clean -testcache -modcache.

$ trivy clean --vuln-db --java-db

Benefits

  • More intuitive and consistent user experience across all subcommands
  • Simplified internal logic, making it easier to add new databases in the future
  • Clear separation of concerns: scanning and cache management are now distinct operations

Migration

To assist users in migrating to the new command, we will:

  1. Provide clear error messages when old flags are used, directing users to the new clean command.
$ trivy image --reset
2024-06-21T17:16:58+04:00       ERROR   "--reset" was removed. Use "trivy clean --all" instead.
2024-06-21T17:16:58+04:00       FATAL   Fatal error     flag error: db flag error: unable to parse flag: "--reset" was removed
  1. Announce this breaking change in GitHub Discussions.

This change, while breaking, will lead to a more user-friendly and maintainable Trivy.

@knqyf263
Copy link
Collaborator Author

@aquasecurity/trivy Please let me know if you have any feedback.

@nikpivkin
Copy link
Contributor

nikpivkin commented Jun 21, 2024

Can we also add a flag to clear the Terraform modules cache as well? I'm not entirely sure if it's needed, as the modules are saved in a temporary folder that the system cleans up automatically.

@knqyf263
Copy link
Collaborator Author

I don't mind adding such a flag, but don't we delete the modules after scanning? For example, we clone a remote repository to a system temporary directory, but we'll delete it after scan completes. Are those modules something you want to make permanent in the cache?
If so, should we store them in the Trivy cache directory?

@knqyf263 knqyf263 added the kind/breaking Categorizes issue or PR as related to breaking compatibility. label Jun 21, 2024
@simar7
Copy link
Member

simar7 commented Jun 22, 2024

Are those modules something you want to make permanent in the cache?

We do this so it to help with the scan speeds where we might have a lot of remote modules and the user has not performed a terraform init. If we download once and cache them and re-scan it helps.

If so, should we store them in the Trivy cache directory?

If we move them here we can add them as a cleanup item with trivy clean. Today they are in a system tempdir

@knqyf263
Copy link
Collaborator Author

If we move them here we can add them as a cleanup item with trivy clean. Today they are in a system tempdir

OK. If you move it to the Trivy cache dir, we should add it to trivy clean. Otherwise, we can let OS manage the system directory. Either is fine for me.

@knqyf263 knqyf263 self-assigned this Jun 24, 2024
@knqyf263 knqyf263 added this to the v0.53.0 milestone Jun 24, 2024
@ckcr4lyf
Copy link

ckcr4lyf commented Jul 2, 2024

2. Announce this breaking change in GitHub Discussions.

This change, while breaking, will lead to a more user-friendly and maintainable Trivy.

Was surprised this was not listed as a breaking change in the release notes as well.

@knqyf263
Copy link
Collaborator Author

knqyf263 commented Jul 2, 2024

2. Announce this breaking change in GitHub Discussions.

This change, while breaking, will lead to a more user-friendly and maintainable Trivy.

Was surprised this was not listed as a breaking change in the release notes as well.

It's listed. Do you mean something else?
#7061

@ckcr4lyf
Copy link

ckcr4lyf commented Jul 2, 2024

It's listed. Do you mean something else?

Sorry, I meant the CHANGELOG under Github Releases. https://github.com/aquasecurity/trivy/releases/tag/v0.53.0 , which has a couple of breaking changes but not this one.

While I do appreciate GH Discussion for Q-A style stuff or commenting on a release, I think most people would check the CHANGELOG file, or view it under releases.

Although CHANGELOG.md kinda mentions it, "add clean subcommand" is not as obvious as "Cache Management Flags Removed".

@knqyf263
Copy link
Collaborator Author

knqyf263 commented Jul 2, 2024

@chen-keinan It seems like you forgot to update https://github.com/aquasecurity/trivy/releases/tag/v0.53.0. We usually add a link to the detailed release notes like this.

UPDATE: I opened a PR to document it. #7072

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking Categorizes issue or PR as related to breaking compatibility.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants