feat(cli): Add available version checking #8553
Conversation
There was a problem hiding this comment.
@owenrumney left comments.
Take a look, please.
Also I think update is not a very good name for this logic.
Maybe something like notification?
UPD:
and I would also think about how we can inform users about the ---no-notises flag
pkg/update/check.go
Outdated
| req.Header.Set("-x-trivy-identifier", uniqueIdentifier()) | ||
| req.Header.Set("-x-trivy-command", strings.Join(args, " ")) | ||
| req.Header.Set("-x-trivy-os", runtime.GOOS) | ||
| req.Header.Set("-x-trivy-arch", runtime.GOARCH) |
There was a problem hiding this comment.
We take information about PC components.
Do we need it?
OS and architecture are not a big problem.
But I am worried that users will ask why we need information about network interfaces, mac address, etc.
There was a problem hiding this comment.
We're not taking the nic or mac details and forwarding them on, we're using them to generate a unique identifier for the user - it's a one way hash purely as an ID
There was a problem hiding this comment.
i didn't see hash - identifier is MacBook-Pro-Dmitrij.local-xx:xx:xx:xx:xx:db-macbook-pro-dmitrij.local
There was a problem hiding this comment.
we're using them to generate a unique identifier for the user
why does API need this ID?
There was a problem hiding this comment.
i didn't see hash -
identifier is MacBook-Pro-Dmitrij.local-xx:xx:xx:xx:xx:db-macbook-pro-dmitrij.local
do you mean you saw this in the TestGetMachineIdentifier test? I'm not sure where that line is output.
There was a problem hiding this comment.
no, i ran trivy image alpine and saw this code here(in debugger):
https://github.com/aquasecurity/trivy/blob/be383c37f224936951e89fd0c799eb8272794464/pkg/notification/identifier.go#L29-L28
- Update the preRun and postRun to check and print the latest version
- Run in a go routine so as not to interfere or slow the normal flow
- Provide new `--no-notices` flag to prevent notice checking, same if
they `-q/--quiet` flag is used
- Add tests for the identifier and the check logic
owenrumney
force-pushed
the
feat/add-update-checking
branch
from
38dc33e to
4a269dd
Compare
- move the package from update to notifications - use a version-check prefix on the debug logs - move the `--no-notices` flag to the scan flags - update the tests to include a flag test - regenerate the docs
owenrumney
force-pushed
the
feat/add-update-checking
branch
from
4a269dd to
be383c3
Compare
| @@ -135,6 +137,13 @@ func NewRunner(ctx context.Context, cliOptions flag.Options, opts ...RunnerOptio | |||
| m.Register() | |||
| r.module = m | |||
|
|
|||
| // Make a silent attempt to check for updates in the background | |||
There was a problem hiding this comment.
@DmitriyLewen - I'm not sure if this is the optimal place to hook into Scan runs to trigger the best efforts check for notices. Happy to move if somewhere more appropriate can be suggested
There was a problem hiding this comment.
I think your previous place was better.
You run the check before rootCommand (PersistentPreRunE) and show notifications after rootCommand (PersistentPostRun).
Both commands are on the same level
There was a problem hiding this comment.
I'm trying to sasify you suggestion that it only run on scan commands. If its in the root command there will need to be some inspection of the called sub command to ensure its a scan.
As it's a best efforts request, I think I should go back to it being run as part of the preRun and all commands get it so --no-notices goes back to being a global flag. wdyt?
There was a problem hiding this comment.
I looked at the current implementation again. Let's leave it like this. Given that we only need notifications when scanning, this would be the optimal place.
Thanks for clarifying this nuance.
Description
Adds a background check to
https://api.trivy.cloud/checkto see if there is new version or any relevant notices available.The check will be suppressed if the user uses the
--no-noticesor--quietenvvars or flags. The docs have been updated with the new notices flagExample output
Although the image shows dummy versions, the api has been updated to reflect the correct information and has no announcements at this time... just the latest version (0.60.0)
Related issues
Checklist