Require HPA coverage before treating the evidence as conclusive (#1755)

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.com/claude-code)

https://claude.ai/code/session_0125QDX24TNuod7FKLwTDhvW

<!-- CURSOR_SUMMARY -->
---

> [!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.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
f64b5fd. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: nadaverell

internal/prometheus/rightsizing.go

@@ -14,6 +14,7 @@ import (
 	"github.com/go-chi/chi/v5"
 	"github.com/skyhook-io/radar/internal/errorlog"
 	"github.com/skyhook-io/radar/internal/k8s"
+	"github.com/skyhook-io/radar/pkg/k8score"
 	"github.com/skyhook-io/radar/pkg/prom"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
@@ -336,6 +337,13 @@ func loadHPAManagedResources(cache *k8s.ResourceCache, kind, namespace, name str
 	if !cache.IsDeferredSynced() || cache.HorizontalPodAutoscalers() == nil {
 		return managed, false
 	}
+	// A namespace-scoped informer answers an empty list for every namespace it
+	// does not watch, with no error. Treating that as "no autoscaler here" is
+	// what the availability flag exists to prevent: it gates whether a
+	// reduction may be suggested at all.
+	if !cache.KindCoversNamespace(string(k8score.HorizontalPodAutoscalers), namespace) {
+		return managed, false
+	}
 	hpas, err := cache.HorizontalPodAutoscalers().HorizontalPodAutoscalers(namespace).List(labels.Everything())
 	if err != nil {
 		return managed, false

internal/prometheus/rightsizing_scan.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/skyhook-io/radar/internal/k8s"
+	"github.com/skyhook-io/radar/pkg/k8score"
 	"github.com/skyhook-io/radar/pkg/prom"
 	appsv1 "k8s.io/api/apps/v1"
 	autoscalingv2 "k8s.io/api/autoscaling/v2"
@@ -723,15 +724,15 @@ func enrichScanHPA(cache *k8s.ResourceCache, scopes map[string][]string, workloa
 		if err != nil {
 			return
 		}
-		markScanHPAAvailable(workloads, "")
+		markScanHPAAvailable(cache, workloads, "")
 	} else {
 		for _, namespace := range namespaces {
 			items, err := lister.HorizontalPodAutoscalers(namespace).List(labels.Everything())
 			if err != nil {
 				continue
 			}
 			hpas = append(hpas, items...)
-			markScanHPAAvailable(workloads, namespace)
+			markScanHPAAvailable(cache, workloads, namespace)
 		}
 	}
 	for _, hpa := range hpas {
@@ -751,11 +752,19 @@ func enrichScanHPA(cache *k8s.ResourceCache, scopes map[string][]string, workloa
 	}
 }
 
-func markScanHPAAvailable(workloads map[string]*scanWorkload, namespace string) {
+// markScanHPAAvailable records that the HPA inventory was actually readable for
+// these workloads. A successful list says nothing about namespaces the informer
+// does not watch, so coverage is checked per workload: a cluster-wide scan can
+// span both kinds of namespace at once.
+func markScanHPAAvailable(cache *k8s.ResourceCache, workloads map[string]*scanWorkload, namespace string) {
 	for _, workload := range workloads {
-		if namespace == "" || workload.namespace == namespace {
-			workload.workload.hpaAvailable = true
+		if namespace != "" && workload.namespace != namespace {
+			continue
+		}
+		if !cache.KindCoversNamespace(string(k8score.HorizontalPodAutoscalers), workload.namespace) {
+			continue
 		}
+		workload.workload.hpaAvailable = true
 	}
 }
 

internal/prometheus/rightsizing_scan_test.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	appsv1 "k8s.io/api/apps/v1"
+	autoscalingv2 "k8s.io/api/autoscaling/v2"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -338,7 +339,7 @@ func TestMarkScanHPAAvailableIsNamespaceScoped(t *testing.T) {
 		"a": {namespace: "team-a"},
 		"b": {namespace: "team-b"},
 	}
-	markScanHPAAvailable(workloads, "team-a")
+	markScanHPAAvailable(scopeTestCache(t, map[string]bool{k8score.HorizontalPodAutoscalers: true}), workloads, "team-a")
 	if !workloads["a"].workload.hpaAvailable || workloads["b"].workload.hpaAvailable {
 		t.Fatalf("HPA evidence availability crossed namespaces: %+v", workloads)
 	}
@@ -487,3 +488,131 @@ func TestScanSkipsReplicaSetWaitWithoutDeployments(t *testing.T) {
 		t.Errorf("waited %v on ReplicaSets a DaemonSet-only scan never reads", elapsed)
 	}
 }
+
+// The availability flag gates whether a reduction may be suggested at all, so
+// an HPA informer that does not watch a namespace must not report that it
+// looked there and found no autoscaler.
+func TestScanHPAAvailabilityFollowsInformerCoverage(t *testing.T) {
+	core, err := k8score.NewResourceCache(k8score.CacheConfig{
+		Client:        fake.NewClientset(),
+		ResourceTypes: map[string]bool{k8score.HorizontalPodAutoscalers: true},
+		DeferredTypes: map[string]bool{},
+		ResourceScopes: map[string]k8score.ResourceScope{
+			string(k8score.HorizontalPodAutoscalers): {Enabled: true, Namespace: "team-a"},
+		},
+	})
+	if err != nil {
+		t.Fatalf("NewResourceCache: %v", err)
+	}
+	t.Cleanup(core.Stop)
+	cache := &k8s.ResourceCache{ResourceCache: core}
+
+	workloads := map[string]*scanWorkload{
+		"a": {kind: "Deployment", namespace: "team-a", name: "watched"},
+		"b": {kind: "Deployment", namespace: "team-b", name: "unwatched"},
+	}
+	markScanHPAAvailable(cache, workloads, "")
+
+	if !workloads["a"].workload.hpaAvailable {
+		t.Error("team-a is watched; its empty HPA list is authoritative and must count as evidence")
+	}
+	if workloads["b"].workload.hpaAvailable {
+		t.Error("team-b is outside the informer's scope, so no autoscaler could have been seen there")
+	}
+}
+
+// A cache with no scope restrictions watches every namespace, so there is no
+// namespace whose HPAs went unread: every workload's empty result is a real
+// answer, and the evidence counts for all of them.
+func TestScanHPAAvailabilityWithFullAccess(t *testing.T) {
+	cache := scopeTestCache(t, map[string]bool{k8score.HorizontalPodAutoscalers: true})
+	workloads := map[string]*scanWorkload{
+		"a": {kind: "Deployment", namespace: "team-a", name: "one"},
+		"b": {kind: "Deployment", namespace: "team-b", name: "two"},
+	}
+	markScanHPAAvailable(cache, workloads, "")
+	if !workloads["a"].workload.hpaAvailable || !workloads["b"].workload.hpaAvailable {
+		t.Fatalf("full access must mark every workload available: %+v", workloads)
+	}
+}
+
+// The real caller, not just the marking helper: a utilization HPA in the
+// watched namespace must both flag its workload as HPA-managed and count as
+// evidence, while a workload in an unwatched namespace gets neither — nothing
+// was ever listed there, so "no autoscaler" was never established.
+func TestEnrichScanHPAHonoursCoverageEndToEnd(t *testing.T) {
+	utilization := int32(80)
+	hpa := &autoscalingv2.HorizontalPodAutoscaler{
+		ObjectMeta: metav1.ObjectMeta{Namespace: "team-a", Name: "api"},
+		Spec: autoscalingv2.HorizontalPodAutoscalerSpec{
+			ScaleTargetRef: autoscalingv2.CrossVersionObjectReference{Kind: "Deployment", Name: "api"},
+			Metrics: []autoscalingv2.MetricSpec{{
+				Type: autoscalingv2.ResourceMetricSourceType,
+				Resource: &autoscalingv2.ResourceMetricSource{
+					Name:   "cpu",
+					Target: autoscalingv2.MetricTarget{AverageUtilization: &utilization},
+				},
+			}},
+		},
+	}
+	for _, tc := range []struct {
+		name           string
+		scopes         map[string]k8score.ResourceScope
+		wantAAvailable bool
+		wantBAvailable bool
+	}{
+		{"full access", nil, true, true},
+		{"scoped to team-a", map[string]k8score.ResourceScope{
+			string(k8score.HorizontalPodAutoscalers): {Enabled: true, Namespace: "team-a"},
+		}, true, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			core, err := k8score.NewResourceCache(k8score.CacheConfig{
+				Client:         fake.NewClientset(hpa),
+				ResourceTypes:  map[string]bool{k8score.HorizontalPodAutoscalers: true},
+				DeferredTypes:  map[string]bool{},
+				ResourceScopes: tc.scopes,
+			})
+			if err != nil {
+				t.Fatalf("NewResourceCache: %v", err)
+			}
+			t.Cleanup(core.Stop)
+			cache := &k8s.ResourceCache{ResourceCache: core}
+			// HPAs load on the deferred path, and the evidence gate refuses to
+			// read them until that phase reports done.
+			deadline := time.Now().Add(10 * time.Second)
+			for !cache.IsDeferredSynced() {
+				if time.Now().After(deadline) {
+					t.Fatal("deferred informers never reported synced")
+				}
+				time.Sleep(20 * time.Millisecond)
+			}
+
+			workloads := map[string]*scanWorkload{
+				workloadIdentity("Deployment", "team-a", "api"): {
+					kind: "Deployment", namespace: "team-a", name: "api",
+					workload: rightsizingWorkload{hpaManaged: map[string]bool{}},
+				},
+				workloadIdentity("Deployment", "team-b", "other"): {
+					kind: "Deployment", namespace: "team-b", name: "other",
+					workload: rightsizingWorkload{hpaManaged: map[string]bool{}},
+				},
+			}
+			// A kind mapped to a nil namespace list is the cluster-wide scope; an
+			// absent key yields an empty list, which is a different branch.
+			enrichScanHPA(cache, map[string][]string{"Deployment": nil}, workloads)
+
+			a := workloads[workloadIdentity("Deployment", "team-a", "api")].workload
+			b := workloads[workloadIdentity("Deployment", "team-b", "other")].workload
+			if a.hpaAvailable != tc.wantAAvailable {
+				t.Errorf("team-a hpaAvailable = %v, want %v", a.hpaAvailable, tc.wantAAvailable)
+			}
+			if b.hpaAvailable != tc.wantBAvailable {
+				t.Errorf("team-b hpaAvailable = %v, want %v", b.hpaAvailable, tc.wantBAvailable)
+			}
+			if !a.hpaManaged["cpu"] {
+				t.Error("the HPA targets this Deployment's CPU; that must survive the coverage check")
+			}
+		})
+	}
+}