ROX-35938: Add secured cluster cert expiry to expiry metrics tracker#21938
Draft
parametalol wants to merge 2 commits into
Draft
ROX-35938: Add secured cluster cert expiry to expiry metrics tracker#21938parametalol wants to merge 2 commits into
parametalol wants to merge 2 commits into
Conversation
The rox_central_cert_exp_hours metric only covered the four values in the v1.GetCertExpiry_Component proto enum (Central, Scanner, Scanner V4, Central DB), so secured cluster Sensor cert expiry was invisible to it. Sensor cert expiry can't be one more enum value since there can be any number of secured clusters, so track() now also walks the cluster datastore and emits a synthetic SECURED_CLUSTER component per cluster (name carried in a new Name label), sourced from the existing storage.ClusterCertExpiryStatus. Switched the tracker from MakeGlobalTrackerBase to the scoped MakeTrackerBase, since Cluster is a SAC-scoped resource and this tracker now surfaces per-cluster data - matching the convention already used by the clusters/image_vulnerabilities/node_vulnerabilities trackers in this package. Prompt: "open Jira bug for this problem, then create a branch with michael/ROX-<jira number> prefix, commit the change", following up on "I need secured cluster cert expiry. Can we add an artificial 'SECURED_CLUSTER' component to the tracker, and for secured clusters add `name` label with the secured cluster name?" Partially generated with the help of Claude Code.
Prompt: "PR", following the CHANGELOG.md convention already used for ROX-35181 (another custom Prometheus metric addition). Partially generated with the help of Claude Code.
|
Skipping CI for Draft Pull Request. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe expiry metrics tracker now accepts a cluster datastore, reports Sensor certificate expiry for connected secured clusters using the ChangesSecured cluster expiry metrics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CustomMetricsRunner
participant ExpiryTracker
participant ExpiryService
participant ClusterDataStore
CustomMetricsRunner->>ExpiryTracker: collect expiry findings
ExpiryTracker->>ExpiryService: read certificate expiry
ExpiryTracker->>ClusterDataStore: WalkClusters
ClusterDataStore-->>ExpiryTracker: Sensor certificate status
ExpiryTracker-->>CustomMetricsRunner: SECURED_CLUSTER finding with Name
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Build Images ReadyImages are ready for commit d1169e5. To use with deploy scripts: export MAIN_IMAGE_TAG=4.12.x-583-gd1169e5d8e |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #21938 +/- ##
==========================================
- Coverage 51.40% 51.35% -0.05%
==========================================
Files 2857 2857
Lines 178776 178847 +71
==========================================
- Hits 91899 91853 -46
- Misses 78853 78954 +101
- Partials 8024 8040 +16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The
rox_central_cert_exp_hourscustom Prometheus metric only reportedCentral, Scanner, Scanner V4, and Central DB cert expiry — the four values in
the
v1.GetCertExpiry_Componentproto enum. Secured cluster (Sensor) certexpiry was missing: it can't be one more enum value, since there can be any
number of secured clusters, and Central doesn't dial out to Sensor the way it
dials Scanner to check the cert.
This adds a synthetic
SECURED_CLUSTERcomponent (not a new proto enumvalue) sourced from the already-existing
storage.ClusterCertExpiryStatus(populated whenever Sensor connects), with the cluster name carried in a new
Namelabel. Clusters that haven't connected yet (no cert data) are skipped.Also switches the tracker from
MakeGlobalTrackerBaseto the scopedMakeTrackerBase:Clusteris a SAC-scoped resource, and this tracker nowsurfaces per-cluster data, so it needs to respect each caller's Access Scope
the same way the
clusters/image_vulnerabilities/node_vulnerabilitiestrackers in this package already do.
Fixes ROX-35938.
User-facing documentation
Testing and quality
Automated testing
How I validated my change
central/metrics/custom/expiry/tracker_test.goto cover a connected secured cluster (produces aSECURED_CLUSTERfinding with the rightNameand computed hours) and a never-connected one (produces no finding).go build ./central/metrics/...,go vet ./central/metrics/custom/...,golangci-lint run ./central/metrics/custom/..., andgo test ./central/metrics/custom/...all pass.