Skip to content

Commit b735b7c

Browse files
HiranAdikariclaude
andcommitted
Guard Get routability for consistency with List
The List op gained a routability guard from review feedback so a GVK-mapped but non-routable family (the pre-seeded virtualmachineinstances) is refused rather than routed to the agent. Get resolves the same GVR->GVK mapping and had the same gap, so the guard is applied to Get too: it returns ErrOpNotRoutable for a mapped-but-direct-only family before calling the agent, and the caller then falls back to the direct path. Added a regression test mirroring the List one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RC3BDHCFmJBdxvGVW69FAr
1 parent ece3050 commit b735b7c

2 files changed

Lines changed: 30 additions & 0 deletions

File tree

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ func (a *AgentBacked) ref(gvr schema.GroupVersionResource, ns, name string) (age
129129
// error so callers' existing IsNotFound / "not found" checks fire unchanged on
130130
// both seams.
131131
func (a *AgentBacked) Get(ctx context.Context, gvr schema.GroupVersionResource, ns, name string, _ metav1.GetOptions) (*unstructured.Unstructured, error) {
132+
// Routability guard FIRST (mirrors List): a GVR can be GVK-mapped yet not be a
133+
// routable family for Get — e.g. the pre-seeded virtualmachineinstances, which
134+
// has no RouteVerbs. Refuse rather than issue a read the agent's SA may not be
135+
// permitted to serve; the caller falls back to the direct path.
136+
if verbs, ok := RoutableVerbs(gvr); !ok || !verbs[VerbGet] {
137+
return nil, fmt.Errorf("clusteraccess: get of %s not routable: %w", gvr.String(), agentgw.ErrOpNotRoutable)
138+
}
132139
ref, err := a.ref(gvr, ns, name)
133140
if err != nil {
134141
return nil, err

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,11 @@ type fakeSession struct {
4040
listCalled bool // set true the moment List is invoked
4141

4242
getStatusCalled bool // set by GetStatus, asserts the OP_UNSUPPORTED fallback fired
43+
getObjectCalled bool // set by GetObject, asserts the routability guard fired first
4344
}
4445

4546
func (f *fakeSession) GetObject(ctx context.Context, ref agentgw.ResourceRef) (agentgw.GetObjectResult, error) {
47+
f.getObjectCalled = true
4648
f.lastRef = ref
4749
if f.block {
4850
<-ctx.Done()
@@ -192,6 +194,27 @@ func TestAgentBackedList_PreSeededGVKNotRoutable(t *testing.T) {
192194
}
193195
}
194196

197+
// TestAgentBackedGet_PreSeededGVKNotRoutable mirrors the List guard for Get: the
198+
// pre-seeded virtualmachineinstances GVR is GVK-mapped but has no RouteVerbs, so
199+
// Get must be REFUSED with ErrOpNotRoutable and the agent's GetObject never invoked.
200+
func TestAgentBackedGet_PreSeededGVKNotRoutable(t *testing.T) {
201+
vmiGVR := schema.GroupVersionResource{Group: "kubevirt.io", Version: "v1", Resource: "virtualmachineinstances"}
202+
if _, _, ok := DefaultGVKMapper().GVK(vmiGVR); !ok {
203+
t.Fatalf("precondition: VMI GVR %s must be GVK-mapped", vmiGVR)
204+
}
205+
206+
sess := &fakeSession{}
207+
a := NewAgentBacked(sess, "lk", "zone-1", "dc-api", DefaultGVKMapper(), zerolog.Nop())
208+
209+
_, err := a.Get(context.Background(), vmiGVR, "dc-t-p", "vmi-1", metav1.GetOptions{})
210+
if !errors.Is(err, agentgw.ErrOpNotRoutable) {
211+
t.Errorf("mapped-but-not-routable GVR must yield ErrOpNotRoutable for Get, got %v", err)
212+
}
213+
if sess.getObjectCalled {
214+
t.Error("agent GetObject must NOT be invoked for a non-routable family")
215+
}
216+
}
217+
195218
func TestAgentBackedList_AgentUnavailableIsRetryable(t *testing.T) {
196219
sess := &fakeSession{
197220
list: func(ref agentgw.ListRef) (agentgw.ListResult, error) {

0 commit comments

Comments
 (0)