aquasecurity / trivy

Find vulnerabilities, misconfigurations, secrets, SBOM in containers, Kubernetes, code repositories, clouds and more
https://aquasecurity.github.io/trivy
Apache License 2.0
22.14k stars 2.18k forks source link

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

Closed knqyf263 closed 2 weeks ago

knqyf263 commented 2 weeks ago

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

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 commented 2 weeks ago

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

nikpivkin commented 2 weeks ago

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 commented 2 weeks ago

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?

simar7 commented 2 weeks ago

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 commented 2 weeks ago

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.

ckcr4lyf commented 1 week ago
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 commented 1 week ago
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? https://github.com/aquasecurity/trivy/discussions/7061

ckcr4lyf commented 1 week ago

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 commented 1 week ago

@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. https://github.com/aquasecurity/trivy/pull/7072