Require HPA coverage before treating the evidence as conclusive

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")
+			}
+		})
+	}
+}