-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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(cli): rename --vuln-type
flag to --pkg-types
flag
#7104
Conversation
vuln-type
to pkg-types
--vuln-type
flag to --pkg-types
flag
I used trivy/pkg/flag/report_flags.go Lines 109 to 122 in 5bc3fa7
|
I don't think it's a breaking It is always difficult to determine the type of PR, but I think it will be a "refactor" when there is no user impact. In this case, UI will be changed. It's not a new feature or bug fix, but I think "feat" is the closest. |
I thought about this. These are breaking changes for users using Trivy as a library.
hm... okay, i will change type of PR. |
--vuln-type
flag to --pkg-types
flag--vuln-type
flag to --pkg-types
flag
That's a good point. But Trivy is a CLI tool. We could consider whether changing to CLI usage is destructive. Of course, we should ensure the change is minimal for tools importing Trivy, though. Otherwise, adding arguments to an internal function will be a breaking change if it is exported. @naortalmor1 @tamirkiviti13 @tonaim We changed a variable name. I don't think it will have a major impact, but I thought I'd let you know just in case. |
--vuln-type
flag to --pkg-types
flag--vuln-type
flag to --pkg-types
flag
Okay. I got you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not an issue from this PR, but logging doesn't seem to be enabled well. I mean our custom handler is not initialized, and the log format is different.
2024/07/09 10:08:31 WARN '--vuln-type' is deprecated. Use '--pkg-types' instead.
2024-07-09T10:08:31+04:00 INFO Vulnerability scanning is enabled
rpc/scanner/service.proto
Outdated
repeated string scanners = 2; | ||
map<string, Licenses> license_categories = 4; | ||
bool include_dev_deps = 5; | ||
repeated string pkg_types = 6; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to add a new field when renaming it. Protobuf uses a number for marshaling/unmarshaling. When you change the field type, like int to string, you need a new field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it! Updated in aeb4f49
Created #7124 for this. |
// VulnTypeLibrary is a vulnerability type of programming language dependencies | ||
VulnTypeLibrary = VulnType("library") | ||
// PkgTypeLibrary is a package type of programming language dependencies | ||
PkgTypeLibrary = PkgType("library") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to change it to lang
or language
as we lately call them "Language-specific packages". (we used to call them libraries, as you know). But I don't want to break compatibility. Do you think we should open another PR? I think it's a good opportunity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a good opportunity.
Yeah. Now is the best time for this.
I want to change it to lang or language as we lately call them "Language-specific packages"
I think lang
will be enough.
But I don't want to break compatibility. Do you think we should open another PR?
I think it is better to create one more PR for this.
Also we need to create 2 paragraphs in release discussion to attract the attention of users to the fact that we not only renamed flag, but also changed library
to lang
.
Also in the new PR we need to add warning when user uses library
and rename it (like we do with flag aliases)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better to create one more PR for this.
OK. However, we can announce these changes together in a single thread in GitHub Discussions.
e.g. #7010
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's a good idea!
- Fix --vuln-type flag renamed into --pkg-types aquasecurity/trivy#7104; \n- Adopt package relationships aquasecurity/trivy#7237
- Fix --vuln-type flag renamed into --pkg-types aquasecurity/trivy#7104; \n- Adopt package relationships aquasecurity/trivy#7237
* Bump trivy to v0.49.1 * Bump trivy to v0.51.4 - Fix registry version aquasecurity/trivy#6219; - Fix replace zap with slog aquasecurity/trivy#6466; - The fix with slog used a zap to slog bridge (official from zap, but exp). It didn't have a license file, so I hardcoded a commit version that had; - Adopt opts.Align() to validate options object; * Bump trivy to v0.52.2 * Temp change the workflow trigger to test changes * Free up space on runner * Bump trivy to v0.53.0 - Fix go clear cache aquasecurity/trivy#7010 * Bump trivy to v0.54.1 - Fix --vuln-type flag renamed into --pkg-types aquasecurity/trivy#7104; - Adopt package relationships aquasecurity/trivy#7237 * Rollback CI run on target * Clean 'scan cache clean' code and add timeout to it
…#83) * chore(deps): bump github.com/aquasecurity/trivy from 0.53.0 to 0.54.1 Bumps [github.com/aquasecurity/trivy](https://github.com/aquasecurity/trivy) from 0.53.0 to 0.54.1. - [Release notes](https://github.com/aquasecurity/trivy/releases) - [Changelog](https://github.com/aquasecurity/trivy/blob/v0.54.1/CHANGELOG.md) - [Commits](aquasecurity/trivy@v0.53.0...v0.54.1) --- updated-dependencies: - dependency-name: github.com/aquasecurity/trivy dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> * fix: Use new flag for vulnerability type See aquasecurity/trivy#7104 * bump: Trivy version for vulnerability DB updates * fix: Specify types of dependencies to analyze It's only necessary to specifiy these types when running the scanners directly, as we do. When running Trivy via the command line it's not necessary. See aquasecurity/trivy#7237 --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: André Meira <[email protected]>
Description
Rename
--vuln-type
flag to--pkg-types
flag.See #6269
Before:
After:
Related issues
--vuln-type
flag forPackages
#6269Checklist