Prevent silent partial reads on older agents

AgentBacked.Get falls back from full-object get_object to status-only
get_status when an older agent doesn't support get_object. That is
correct for a status-only consumer (the VM read), but a full-object
consumer — the cloud-provider SA token read (.data.token), and the
future KubeOVN read-modify-patch ops (.spec/.metadata.labels) — would
silently receive a partial object missing the fields it needs. The SA
bootstrap in particular would poll a token-less Secret until timeout.

Gate the fallback on a new per-capability StatusFallbackOK flag, set
only on the VM family. Every other routable-Get family now returns a
clear error (still wrapping ErrOpNotRoutable) on an old agent instead
of a silently-partial object, so the failure is immediate and
actionable. Routed.Get is terminal on agent activation, so the error
propagates rather than degrading.

Adds a companion test asserting a full-object family errors (and never
calls GetStatus) on OP_UNSUPPORTED, alongside the VM fallback test.

Addresses the review on this PR.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RC3BDHCFmJBdxvGVW69FAr

Author: HiranAdikari

dc-api/internal/providers/clusteraccess/agent.go

@@ -117,13 +117,14 @@ func (a *AgentBacked) ref(gvr schema.GroupVersionResource, ns, name string) (age
 // what the read-modify-patch write ops (NAT/DNS/cloud-provider SA) need: they
 // read .spec/.data to modify it.
 //
-// COMPAT: an OLDER agent that predates get_object replies OP_UNSUPPORTED; this
-// FALLS BACK to Session.GetStatus and synthesizes the prior status-only partial
-// object (apiVersion, kind, metadata.name/namespace, resourceVersion,
-// generation, status). The behavior degrades to the prior status-only read
-// rather than erroring, keeping the change non-breaking during a rolling deploy.
-// Callers that only read status (e.g. the VM read slice's
-// status.printableStatus) are unaffected by which path served them.
+// COMPAT: an OLDER agent that predates get_object replies OP_UNSUPPORTED. For a
+// family whose Get consumers read status alone (StatusFallbackOK — the VM read
+// slice), this FALLS BACK to Session.GetStatus and synthesizes the prior
+// status-only partial object, keeping that read non-breaking during a rolling
+// deploy. For a FULL-object family (a Secret's .data, the KubeOVN .spec/.labels
+// read-modify-patch ops) the status-only partial would drop the fields the caller
+// needs, so a CLEAR error is returned instead of a silent partial — the zone's
+// agent must be upgraded. See AgentCapability.StatusFallbackOK.
 //
 // Found==false (on either path) is translated to a k8serrors.NewNotFound-shaped
 // error so callers' existing IsNotFound / "not found" checks fire unchanged on
@@ -152,13 +153,26 @@ func (a *AgentBacked) Get(ctx context.Context, gvr schema.GroupVersionResource,
 		// (agent unavailable, RBAC, timeout) propagates.
 		terr := a.translateErr(err)
 		if errors.Is(terr, agentgw.ErrOpNotRoutable) {
-			a.log.Info().
-				Str("seam", "agent").
-				Str("region", a.region).Str("zone", a.zone).
-				Str("api_version", ref.APIVersion).Str("kind", ref.Kind).
-				Str("namespace", ref.Namespace).Str("name", ref.Name).
-				Msg("agent does not support get_object; falling back to status-only get_status")
-			return a.getStatusFallback(ctx, gvr, ref, ns, name)
+			// OP_UNSUPPORTED: the agent predates get_object. Degrading to the
+			// status-only get_status is valid ONLY for families whose Get consumers
+			// read status alone (VMs). For a full-object consumer (a Secret's
+			// .data.token, the KubeOVN read-modify-patch ops' .spec/.metadata.labels)
+			// the synthesized partial would silently drop the very fields the caller
+			// needs — e.g. the cloud-provider SA bootstrap would poll a token-less
+			// Secret until it times out. So gate the fallback on the family's
+			// StatusFallbackOK; otherwise surface a clear, immediate error (Routed.Get
+			// is terminal on agent activation, so this propagates rather than silently
+			// degrading). ErrOpNotRoutable stays wrapped for callers' errors.Is checks.
+			if StatusFallbackOK(gvr) {
+				a.log.Info().
+					Str("seam", "agent").
+					Str("region", a.region).Str("zone", a.zone).
+					Str("api_version", ref.APIVersion).Str("kind", ref.Kind).
+					Str("namespace", ref.Namespace).Str("name", ref.Name).
+					Msg("agent does not support get_object; falling back to status-only get_status")
+				return a.getStatusFallback(ctx, gvr, ref, ns, name)
+			}
+			return nil, fmt.Errorf("clusteraccess: full-object read of %s in zone %s/%s requires an agent that supports get_object, but the zone's agent predates it (upgrade the agent): %w", gvr.String(), a.region, a.zone, terr)
 		}
 		return nil, terr
 	}

dc-api/internal/providers/clusteraccess/capabilities.go

@@ -48,6 +48,18 @@ type AgentCapability struct {
 	// get_status/watch_status read+watch, and SSA-create needs create+patch. A
 	// capability that grants no RBAC (pre-seeded GVK-only) leaves this nil.
 	AgentVerbs []Verb
+	// StatusFallbackOK allows AgentBacked.Get to degrade to a status-only read
+	// (Session.GetStatus) when an OLDER agent predates full-object get_object and
+	// replies OP_UNSUPPORTED. Set it ONLY for families whose Get consumers read
+	// status alone (the VM read slice's status.printableStatus). Families whose Get
+	// consumers need the full object — Secrets (.data.token) and the KubeOVN
+	// read-modify-patch ops that read .spec / .metadata.labels — MUST leave this
+	// false: the status-only partial the fallback synthesizes drops .spec/.data/
+	// .labels, so degrading to it would silently return a useless object (the
+	// cloud-provider SA bootstrap would poll a token-less Secret until timeout).
+	// With the flag false, an old agent yields a clear error instead — see
+	// AgentBacked.Get.
+	StatusFallbackOK bool
 }
 
 // vmCapability is the one truly onboarded capability in phase 1.
@@ -71,6 +83,11 @@ var vmCapability = AgentCapability{
 	// needs create+patch. Reproduces the old rule verbatim:
 	// get,list,watch,create,patch,delete.
 	AgentVerbs: []Verb{VerbGet, VerbList, VerbWatch, VerbCreate, VerbApply, VerbDelete},
+	// GetVM reads only status.printableStatus, so the status-only fallback is a
+	// valid degrade for an older agent without get_object (preserves the
+	// rolling-deploy compat added with the full-object Get). Every other
+	// routable-Get family reads the full object and leaves this false.
+	StatusFallbackOK: true,
 }
 
 // The network families (NAD, Vpc, Subnet) are onboarded for the kubeovn CRD
@@ -223,15 +240,21 @@ var (
 	derivedGVKTable   map[schema.GroupVersionResource]gvk
 	derivedRouteSet   map[schema.GroupVersionResource]map[Verb]bool
 	derivedUnionRoute map[Verb]bool
+
+	derivedStatusFallback map[schema.GroupVersionResource]bool
 )
 
 func buildDerived() {
 	derivedOnce.Do(func() {
 		derivedGVKTable = make(map[schema.GroupVersionResource]gvk, len(AgentCapabilities))
 		derivedRouteSet = make(map[schema.GroupVersionResource]map[Verb]bool, len(AgentCapabilities))
 		derivedUnionRoute = make(map[Verb]bool)
+		derivedStatusFallback = make(map[schema.GroupVersionResource]bool, len(AgentCapabilities))
 		for _, c := range AgentCapabilities {
 			derivedGVKTable[c.GVR] = gvk{APIVersion: c.APIVersion, Kind: c.Kind}
+			if c.StatusFallbackOK {
+				derivedStatusFallback[c.GVR] = true
+			}
 			if len(c.RouteVerbs) > 0 {
 				set := make(map[Verb]bool, len(c.RouteVerbs))
 				for _, v := range c.RouteVerbs {
@@ -286,6 +309,16 @@ func RoutableVerbs(gvr schema.GroupVersionResource) (map[Verb]bool, bool) {
 	return out, true
 }
 
+// StatusFallbackOK reports whether AgentBacked.Get may degrade a routed read of
+// this GVR to a status-only get_status when an older agent lacks get_object. True
+// only for families whose Get consumers read status alone (VMs); false (the
+// default) for full-object consumers, which then get a clear error on an old
+// agent instead of a silently-partial object. See AgentCapability.StatusFallbackOK.
+func StatusFallbackOK(gvr schema.GroupVersionResource) bool {
+	buildDerived()
+	return derivedStatusFallback[gvr]
+}
+
 // IsReadVerb classifies a seam Verb as a read for the reads/writes toggle split.
 // VerbList is included so it falls on the read side once it becomes routable.
 func IsReadVerb(v Verb) bool {

dc-api/internal/providers/clusteraccess/clusteraccess_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"strings"
 	"testing"
 	"time"
 
@@ -326,6 +327,46 @@ func TestAgentBackedGet_FallsBackToStatusOnUnsupported(t *testing.T) {
 	}
 }
 
+// TestAgentBackedGet_FullObjectFamilyErrorsOnUnsupported proves the other half of
+// the compat gate: a FULL-object family (Secrets — StatusFallbackOK=false) whose
+// caller needs .data must NOT degrade to the status-only partial on an older
+// agent. Get returns a clear error (still wrapping ErrOpNotRoutable) and never
+// calls GetStatus — so the cloud-provider SA bootstrap fails fast instead of
+// polling a token-less Secret until timeout.
+func TestAgentBackedGet_FullObjectFamilyErrorsOnUnsupported(t *testing.T) {
+	secretsGVR := schema.GroupVersionResource{Group: "", Version: "v1", Resource: "secrets"}
+	if verbs, ok := RoutableVerbs(secretsGVR); !ok || !verbs[VerbGet] {
+		t.Fatal("precondition: secrets Get must be a routable family")
+	}
+	if StatusFallbackOK(secretsGVR) {
+		t.Fatal("precondition: secrets must NOT permit the status-only fallback")
+	}
+	sess := &fakeSession{
+		getObject: func(ref agentgw.ResourceRef) (agentgw.GetObjectResult, error) {
+			return agentgw.GetObjectResult{}, &agentgw.AgentError{Code: agentgw.CodeOpUnsupported, Message: "unknown op"}
+		},
+		getStatus: func(ref agentgw.ResourceRef) (agentgw.StatusSnapshot, error) {
+			t.Error("full-object family must NOT fall back to GetStatus")
+			return agentgw.StatusSnapshot{}, nil
+		},
+	}
+	a := NewAgentBacked(sess, "lk", "zone-1", "dc-api", DefaultGVKMapper(), zerolog.Nop())
+
+	_, err := a.Get(context.Background(), secretsGVR, "dc-t-p", "sa-token", metav1.GetOptions{})
+	if err == nil {
+		t.Fatal("Get must return an error for a full-object family on OP_UNSUPPORTED, got nil")
+	}
+	if !errors.Is(err, agentgw.ErrOpNotRoutable) {
+		t.Errorf("error must wrap ErrOpNotRoutable for callers' errors.Is checks, got %v", err)
+	}
+	if sess.getStatusCalled {
+		t.Error("Get fell back to GetStatus for a full-object family; it must error instead")
+	}
+	if !strings.Contains(err.Error(), "get_object") {
+		t.Errorf("error should mention get_object upgrade guidance, got %q", err.Error())
+	}
+}
+
 func TestAgentBackedGet_NotFoundIsK8sNotFound(t *testing.T) {
 	sess := &fakeSession{
 		getObject: func(ref agentgw.ResourceRef) (agentgw.GetObjectResult, error) {