trivy-operator: vulnerabilityScannerScanOnlyCurrentRevisions does not delete old reports
What steps did you take and what happened:
- Enable vulnerabilityScannerScanOnlyCurrentRevisions on starboard operator
- See reports generated only for most current deployments
- Deploy new revision of deployment
- See report generated for new deployment
- See old report is still there
What did you expect to happen:
- old report being deleted as I only want reports for the current deployments
Anything else you would like to add:
- The implementation in #870 simply didn’t add that part of the feature
Environment:
- Starboard version (use
starboard version):0.14.0 - Kubernetes version (use
kubectl version):
Client Version: version.Info{Major:"1", Minor:"23", GitVersion:"v1.23.2", GitCommit:"9d142434e3af351a628bffee3939e64c681afa4d", GitTreeState:"clean", BuildDate:"2022-01-19T17:27:51Z", GoVersion:"go1.17.6", Compiler:"gc", Platform:"darwin/arm64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.3", GitCommit:"ca643a4d1f7bfe34773c74f79527be4afd95bf39", GitTreeState:"clean", BuildDate:"2021-07-15T20:59:07Z", GoVersion:"go1.16.6", Compiler:"gc", Platform:"linux/amd64"}
- OS (macOS 10.15, Windows 10, Ubuntu 19.10 etc):
> sw_vers
ProductName: macOS
ProductVersion: 11.6.2
BuildVersion: 20G314
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 13
- Comments: 25 (9 by maintainers)
After going through the discussions here and in aquasecurity/starboard#858 and the associated proposal from @erikgb, I propose the following solution -
Introduce new configuration parameter vulnerabilityScannerRevisonHistoryLimit with the default value of 0. This parameter will control the number of old vulnerability reports to retain. TTLReportReconciler (https://github.com/aquasecurity/trivy-operator/blob/main/pkg/vulnerabilityreport/ttl_report.go) will be extended to support deleting older vulnerability reports in addition to deleting TTL expired reports when vulnerabilityScannerReportTTL configuration parameter is set.
This makes the vulnerabilityScannerScanOnlyCurrentRevisions configuration parameter redundant and harder to use along with the above new parameter vulnerabilityScannerRevisonHistoryLimit. I propose we remove this parameter (vulnerabilityScannerScanOnlyCurrentRevisions) to keep things simple. What this means in practice is when vulnerabilityScannerRevisonHistoryLimit is unset (= set to 0), only the latest revision is scanned and report is retained for that revision. If set to 1, then the 2 last revisions are scanned and reports retained for those 2 revisions.
If the above proposal sounds good, then I will start with the following tasks -
Comments?
We have a bunch of clusters which get cluttered with old reports fairly fast. I honestly expected from the name
vulnerabilityScannerScanOnlyCurrentRevisionsthat it would only keep new reports and remove old ones. I feel like the name is misleading.In any case, we use the grafana dashboards from starboard-exporter and having old versions floating around will balloon the amount of found CVEs. Which means the numbers in that dashboards are useless, because they don’t represent the current cluster-state. To fix this we have to manually delete old reports which is tedious.
I would say the default value probably should be 0 when the transition period has ended, as I think a majority of users prefer to only keep reports for the active replicaset.
I see, I will try replacing it with trivy operator and post here for results. Thanks so much for the quick answers.