fix: support metrics collection with alternate report storage#2856
fix: support metrics collection with alternate report storage#2856rzala wants to merge 2 commits intoaquasecurity:mainfrom
Conversation
|
|
Fixes aquasecurity#2610 When alternate report storage is enabled, reports are saved to filesystem instead of Kubernetes CRDs. The metrics collector was only reading from CRDs, causing all metrics to become unavailable. This commit adds a storage abstraction layer that allows the metrics collector to read from either CRDs (default) or filesystem (when alternate storage is enabled), maintaining full backward compatibility. Additionally, adds validation to skip malformed reports without proper metadata (name/labels), preventing duplicate metric errors from stale files in alternate storage directories. Changes: - Add StorageReader interface for storage backend abstraction - Implement CRDStorageReader for reading from Kubernetes CRDs - Implement FilesystemStorageReader for reading from alternate storage - Add validation to filter out malformed reports without metadata - Update ResourcesMetricsCollector to use StorageReader - Add comprehensive unit tests for both storage backends All report types are supported: VulnerabilityReport, ExposedSecretReport, ConfigAuditReport, RbacAssessmentReport, InfraAssessmentReport, and ClusterComplianceReport.
rzala
force-pushed
the
fix/alternate-storage-metrics
branch
from
cbc0a1b to
0452d01
Compare
|
Any idea on when this PR would get merged? Thanks:) |
|
@Sydney-MF thanks for the ping |
|
@afdesk Any updates on this?:) really appreciate you for taking the time to take a look at this by the way |
i'm on it right now |
It's not my PR so I'm not so sure if I should just go about it and modify it 😅 |
|
Will have a look at the linting errors soon |
|
Hi guys again! thanks for your efforts and contribution! I need more time to investigate it. I have some concerns about performance here - StorageReader re-reads folders very ofter. |
No worries, please keep us updated since there's quite an issue with the etcd limit issue so the only way to make Trivy Operator work was for us to use alternateStorage, but then we lose the ability to see the metrics which is a big part of this tool's feature. But once again thank you for taking the time to take a look at this PR:) |
- Replace interface{} with any (gofmt)
- Fix file/directory permissions (gosec G301/G306)
- Fix import ordering (gci)
- Use %q for quoted strings in error messages (gocritic)
- Replace assert.Len with assert.Empty (testifylint)
- Mark unused context parameters with underscore (revive)
|
@Sydney-MF thanks for the interest and for nudging this along! @afdesk linting errors have been fixed in the latest push. Regarding the performance concern around frequent directory re-reads — happy to look into adding a file-watcher or in-memory cache with TTL if that's the preferred direction. Let me know what approach you'd like and I'll update the PR. |
|
Maybe fine-tune this feature if disk scanning really became a problem? We're evaluating Trivy operator and only way we can run it is 1h ttl on CRD resources and limiting targets and scanners to very narrow set. |
6 participants
Description
Fixes #2610
This PR adds support for Prometheus metrics collection when alternate report storage (filesystem-based) is enabled. Previously, enabling
alternateReportStorage.enabled: truewould cause all metrics liketrivy_image_vulnerabilitiesto stop working because the metrics collector only read from Kubernetes CRDs.Changes
StorageReaderinterface to abstract storage backend operationsCRDStorageReaderfor reading from Kubernetes CRDs (existing default behavior)FilesystemStorageReaderfor reading from alternate storage filesystemResourcesMetricsCollectorto use theStorageReaderabstractionImpact
Testing
Added comprehensive unit tests in
pkg/metrics/storage_reader_test.go:How to Test
With CRD Storage (default):
With Alternate Storage:
alternateReportStorage.enabled: trueChecklist