[nadaverell]

Closes #1749.

HPAEvidenceAvailable is not a caveat — it is a safety gate. Rightsizing withholds a reduction when Fit == FitOversized && !HPAEvidenceAvailable, because shrinking a request an autoscaler is scaling on is the one mistake that matters here. A false positive therefore does not merely mislead; it lets the reduction through.

A synced, non-nil HPA informer answers an empty list for every namespace it does not watch, and it answers without an error. Both rightsizing paths read that as "this workload has no autoscaler" and set the flag. Radar falls back to namespace-scoped informers whenever cluster-wide list is denied, so this is reachable in ordinary multi-tenant operation rather than a corner case.

What changed

Availability now follows the informer's own coverage, decided per workload — a cluster-wide scan can span a watched and an unwatched namespace at once, so a single answer for the whole scan would be wrong either way.

Coverage is consulted via KindCoversNamespace, deliberately rather than asking whether the informer is cluster-wide. A multi-namespace informer is authoritative for every namespace it actually watches, and testing cluster-wide-ness would have suppressed valid recommendations in all of them.

Full access is unchanged by construction

NewResourceCache normalises an unrestricted cache to Enabled: true with an empty namespace, for which KindCoversNamespace returns true for every namespace. Both new gates are no-ops there. The reverse — that this cannot newly suppress a valid recommendation — holds for the same reason: coverage is derived from the same resourceScopeNamespaces used to build the informer, so a namespace that is watched is a namespace that is covered.

Testing

Go tests pin both directions: full access marks every workload available, and an HPA informer scoped to one namespace marks only that namespace's workloads — driven through enrichScanHPA with a real utilization-based HPA, so target matching and the hpaManaged flag are exercised rather than just the marking helper.

go build ./..., go vet, go test ./internal/prometheus/ (including -race), and the separate pkg/k8score module tests pass.

Live, against GKE with real Prometheus and kube-state-metrics. Binaries from this branch and from main were run against the same cluster under two identities.

Full access — the case that must not change:

	main	this branch
Deployment staging/koala-backend	hpaEvidenceAvailable=true, same fit and reason	identical
Cluster scan, 26 workloads / 52 rows	hpaEvidenceAvailable true on 52	true on 52
Decision fields across 52 rows	—	identical across repeat runs

Repeat scans differed only in measured values (observed, sample counts), and those differences moved between runs and workloads — live drift, not behaviour.

HPA readable in staging only — the condition the bug needs. A throwaway ServiceAccount was granted everything cluster-wide except horizontalpodautoscalers, plus a Role granting HPA reads in staging alone, which drives Radar to a namespace-scoped HPA informer (Using namespace-scoped informers for namespace(s): ["staging"]).

Deployment dev/finops (HPAs not readable in dev)	main	this branch
hpaEvidenceAvailable	true	false
recommendationReason	none	hpa_evidence_unavailable
Recommended request	cpu 100m, memory 256Mi	withheld

main suggests shrinking a workload whose autoscaler it could never have seen. In the same run, Deployment staging/koala-backend — where the informer does watch the namespace — is identical on both builds, recommendations included, so the gate suppresses only what it cannot see.

The ServiceAccount, Role, RoleBinding, ClusterRole, ClusterRoleBinding and namespace were deleted afterwards and verified gone.

Follow-up

Both paths still reject HPA evidence while any deferred informer is warming or has failed, even when the HPA informer itself is fully synced and cluster-wide — a false negative that suppresses valid recommendations. It predates this change and fixing it here would alter full-access behaviour, which this PR deliberately does not do.

🤖 Generated with Claude Code

https://claude.ai/code/session_0125QDX24TNuod7FKLwTDhvW

---

Note

Medium Risk
Changes the safety gate that blocks downsize recommendations when HPA inventory may be incomplete; behavior is intentionally stricter for namespace-scoped informers but full-access paths are designed to stay equivalent.

Overview
Fixes a safety bug where namespace-scoped HPA informers return an empty list (with no error) for namespaces they do not watch, and rightsizing treated that as proof there was no autoscaler—allowing CPU/memory reduction recommendations that should be blocked when HPA evidence is unknown.

Both the single-workload path (loadHPAManagedResources) and the cluster scan path (markScanHPAAvailable) now call KindCoversNamespace per workload namespace before treating an HPA list as authoritative. Workloads outside the informer’s scope no longer get hpaEvidenceAvailable / hpaAvailable, so oversized fits stay withheld with hpa_evidence_unavailable instead of a shrink. Unrestricted caches are unchanged because full coverage still marks every namespace as readable.

Tests cover scoped vs full access, the marking helper, and end-to-end enrichScanHPA with a real utilization-based HPA.

^{Reviewed by Cursor Bugbot for commit f64b5fd. Bugbot is set up for automated code reviews on this repo. Configure here.}

[qodo-code-review]

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

[qodo-code-review]

PR Summary by Qodo

Require HPA namespace coverage for conclusive rightsizing evidence

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Require informer namespace coverage before treating empty HPA results as authoritative evidence.
• Apply coverage checks to single-workload and cluster-scan rightsizing paths.
• Verify scoped and full-access behavior with end-to-end HPA tests.

Diagram

graph TD
  A["Rightsizing paths"] --> B{"Informer ready?"} -->|Yes| C{"Namespace covered?"} -->|Yes| D["List HPAs"] -->|Evidence available| E{"Reduction safe?"} -->|Yes| G["Recommend reduction"]
  B -->|No| F["Evidence unavailable"] --> E
  C -->|No| F
  E -->|No| H["Withhold reduction"]

Loading

High-Level Assessment

The per-workload KindCoversNamespace check is the appropriate approach because it uses the same scope configuration that built the informer and supports cluster-wide, single-namespace, and multi-namespace caches. Checking only whether the informer is cluster-wide was considered but would incorrectly suppress valid evidence in explicitly watched namespaces.

Files changed (3) +152 / -6

Bug fix (2) +22 / -5

rightsizing.goGate single-workload HPA evidence on namespace coverage +8/-0

Gate single-workload HPA evidence on namespace coverage

• Checks whether the HPA informer covers the requested workload namespace before listing autoscalers. Uncovered namespaces now return unavailable evidence, preventing unsafe request reductions based on an authoritative-looking empty result.

internal/prometheus/rightsizing.go

rightsizing_scan.goTrack HPA evidence coverage per scanned workload +14/-5

Track HPA evidence coverage per scanned workload

• Passes the resource cache into HPA availability marking and checks informer coverage for each workload namespace. This preserves authoritative results for watched namespaces while leaving unwatched workloads without conclusive HPA evidence.

internal/prometheus/rightsizing_scan.go

Tests (1) +130 / -1

rightsizing_scan_test.goCover scoped and cluster-wide HPA evidence behavior +130/-1

Cover scoped and cluster-wide HPA evidence behavior

• Adds regression tests for namespace-scoped and unrestricted HPA informers. The end-to-end scan test also verifies that utilization-based HPA target matching remains intact for covered workloads.

internal/prometheus/rightsizing_scan_test.go

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A test comment records change history ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The comment above TestScanHPAAvailabilityUnchangedWithFullAccess says the full-access case must
behave as it did before coverage was consulted. When readers encounter this regression test, the
explanation ties it to prior implementation history instead of the enduring full-access invariant.

Code

internal/prometheus/rightsizing_scan_test.go[R524-526]

+// With no ResourceScopes the cache is cluster-wide by default — the ordinary
+// full-access case — and must behave exactly as it did before coverage was
+// consulted.

Relevance

●●● Strong

Recent precedents accept rewriting comments that reference prior behavior into enduring invariants,
including PR #1724.

PR-#1724
PR-#1636
PR-#1614

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 3036538 prohibits code comments that describe prior behavior or explicit change
history. The added comment says the full-access case must behave as it did before coverage was
consulted.

Rule 3036538: Disallow references to tickets or PR history in code comments
internal/prometheus/rightsizing_scan_test.go[524-526]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The comment above `TestScanHPAAvailabilityUnchangedWithFullAccess` refers to behavior before coverage was consulted, embedding implementation history in a code comment.

## Fix Focus Areas
- internal/prometheus/rightsizing_scan_test.go[524-526]

## Recommended Fix
Rewrite the comment to state only the enduring invariant: an unrestricted cache covers every workload namespace and therefore marks HPA evidence available for every workload.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

✅ Compliance rules (platform): 44 rules

✅ Cross-repo context — repo relationships

Review mode: ⚖️ Balanced: This is a behavioral safety-gate change in rightsizing logic with meaningful informer-scope and multi-namespace correctness implications, but it remains localized enough for a careful single-pass review.

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗