Skip to content

Commit e9feee1

Browse files
ChrisJBurnsclaude
andcommitted
Wire MCPAuthzConfig references into the MCPRemoteProxy controller
Implements Stage 3 of the MCPAuthzConfig ref-wiring design: resolves spec.authzConfigRef for MCPRemoteProxy at runtime (all registered backends via the existing authz factory). Mirrors the MCPServer (Stage 2) pattern: - handleAuthzConfig: fetches + validates the ref, tracks ConfigHash, sets/clears AuthzConfigRefValidated condition (nil-ref removes condition so it never lingers stale-True) - fetchAndValidateAuthzConfig: sets NotFound/NotValid conditions with the appropriate reason constants - mapAuthzConfigToMCPRemoteProxy + SetupWithManager watch: re-queues referencing proxies when the shared config changes - ensureAuthzConfigMapForProxy: also materializes the ref-derived authz ConfigMap (EnsureAuthzConfigMapFromRef) alongside the existing inline ConfigMap path - buildVolumesForProxy: mounts the ref ConfigMap under the same authz-config volume (only when inline is absent — duplicate guard) - createRunConfigFromMCPRemoteProxy: calls AddAuthzConfigRefOptions to inject authz into the runner RunConfig Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 3fb59cf commit e9feee1

4 files changed

Lines changed: 446 additions & 2 deletions

File tree

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc.
2+
// SPDX-License-Identifier: Apache-2.0
3+
4+
package controllers
5+
6+
import (
7+
"testing"
8+
9+
"github.com/stretchr/testify/assert"
10+
"github.com/stretchr/testify/require"
11+
"k8s.io/apimachinery/pkg/api/meta"
12+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
13+
"k8s.io/apimachinery/pkg/runtime"
14+
"k8s.io/apimachinery/pkg/types"
15+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
16+
17+
mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1"
18+
)
19+
20+
func TestMCPRemoteProxyReconciler_handleAuthzConfig(t *testing.T) {
21+
t.Parallel()
22+
23+
tests := []struct {
24+
name string
25+
proxy *mcpv1beta1.MCPRemoteProxy
26+
authzConfig *mcpv1beta1.MCPAuthzConfig
27+
expectError bool
28+
expectErrContains string
29+
expectHash string
30+
expectHashCleared bool
31+
expectConditionStatus *metav1.ConditionStatus
32+
expectConditionReason string
33+
expectConditionGone bool
34+
}{
35+
{
36+
name: "no ref clears stored hash and condition",
37+
proxy: &mcpv1beta1.MCPRemoteProxy{
38+
ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "default"},
39+
Status: mcpv1beta1.MCPRemoteProxyStatus{
40+
AuthzConfigHash: "old",
41+
Conditions: []metav1.Condition{{
42+
Type: mcpv1beta1.ConditionAuthzConfigRefValidated,
43+
Status: metav1.ConditionTrue,
44+
Reason: mcpv1beta1.ConditionReasonAuthzConfigRefValid,
45+
}},
46+
},
47+
},
48+
expectHashCleared: true,
49+
expectConditionGone: true,
50+
},
51+
{
52+
name: "referenced config not found sets NotFound condition",
53+
proxy: &mcpv1beta1.MCPRemoteProxy{
54+
ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "default"},
55+
Spec: mcpv1beta1.MCPRemoteProxySpec{
56+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "missing"},
57+
},
58+
},
59+
expectError: true,
60+
expectErrContains: "not found",
61+
expectConditionStatus: conditionStatusPtr(metav1.ConditionFalse),
62+
expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefNotFound,
63+
},
64+
{
65+
name: "referenced config not valid sets NotValid condition",
66+
proxy: &mcpv1beta1.MCPRemoteProxy{
67+
ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "default"},
68+
Spec: mcpv1beta1.MCPRemoteProxySpec{
69+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "bad"},
70+
},
71+
},
72+
authzConfig: authzConfigForTest("bad", false, ""),
73+
expectError: true,
74+
expectErrContains: "not valid",
75+
expectConditionStatus: conditionStatusPtr(metav1.ConditionFalse),
76+
expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefNotValid,
77+
},
78+
{
79+
name: "valid ref sets condition True and tracks hash",
80+
proxy: &mcpv1beta1.MCPRemoteProxy{
81+
ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "default"},
82+
Spec: mcpv1beta1.MCPRemoteProxySpec{
83+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "ok"},
84+
},
85+
},
86+
authzConfig: authzConfigForTest("ok", true, "hash-123"),
87+
expectHash: "hash-123",
88+
expectConditionStatus: conditionStatusPtr(metav1.ConditionTrue),
89+
expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefValid,
90+
},
91+
}
92+
93+
for _, tt := range tests {
94+
t.Run(tt.name, func(t *testing.T) {
95+
t.Parallel()
96+
ctx := t.Context()
97+
98+
scheme := runtime.NewScheme()
99+
require.NoError(t, mcpv1beta1.AddToScheme(scheme))
100+
101+
objs := []runtime.Object{tt.proxy}
102+
if tt.authzConfig != nil {
103+
objs = append(objs, tt.authzConfig)
104+
}
105+
fakeClient := fake.NewClientBuilder().
106+
WithScheme(scheme).
107+
WithRuntimeObjects(objs...).
108+
WithStatusSubresource(&mcpv1beta1.MCPRemoteProxy{}, &mcpv1beta1.MCPAuthzConfig{}).
109+
Build()
110+
111+
reconciler := &MCPRemoteProxyReconciler{
112+
Client: fakeClient,
113+
Scheme: scheme,
114+
}
115+
116+
err := reconciler.handleAuthzConfig(ctx, tt.proxy)
117+
if tt.expectError {
118+
assert.Error(t, err)
119+
if tt.expectErrContains != "" {
120+
assert.ErrorContains(t, err, tt.expectErrContains)
121+
}
122+
} else {
123+
assert.NoError(t, err)
124+
}
125+
126+
if tt.expectHash != "" {
127+
assert.Equal(t, tt.expectHash, tt.proxy.Status.AuthzConfigHash)
128+
}
129+
if tt.expectHashCleared {
130+
assert.Empty(t, tt.proxy.Status.AuthzConfigHash)
131+
}
132+
133+
cond := meta.FindStatusCondition(tt.proxy.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated)
134+
if tt.expectConditionGone {
135+
assert.Nil(t, cond, "AuthzConfigRefValidated condition must be cleared when the ref is removed")
136+
}
137+
if tt.expectConditionStatus != nil {
138+
require.NotNil(t, cond, "expected AuthzConfigRefValidated condition")
139+
assert.Equal(t, *tt.expectConditionStatus, cond.Status)
140+
assert.Equal(t, tt.expectConditionReason, cond.Reason)
141+
}
142+
})
143+
}
144+
}
145+
146+
// TestMCPRemoteProxyReconciler_handleAuthzConfig_Transitions exercises the stateful
147+
// bookkeeping (needsUpdate from the prior condition, and recovery) that the
148+
// static single-state cases above do not: valid -> invalid -> valid.
149+
func TestMCPRemoteProxyReconciler_handleAuthzConfig_Transitions(t *testing.T) {
150+
t.Parallel()
151+
ctx := t.Context()
152+
153+
scheme := runtime.NewScheme()
154+
require.NoError(t, mcpv1beta1.AddToScheme(scheme))
155+
156+
authzConfig := authzConfigForTest("cfg", true, "h1")
157+
proxy := &mcpv1beta1.MCPRemoteProxy{
158+
ObjectMeta: metav1.ObjectMeta{Name: "p", Namespace: "default"},
159+
Spec: mcpv1beta1.MCPRemoteProxySpec{
160+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "cfg"},
161+
},
162+
}
163+
fakeClient := fake.NewClientBuilder().
164+
WithScheme(scheme).
165+
WithObjects(proxy, authzConfig).
166+
WithStatusSubresource(&mcpv1beta1.MCPRemoteProxy{}, &mcpv1beta1.MCPAuthzConfig{}).
167+
Build()
168+
r := &MCPRemoteProxyReconciler{
169+
Client: fakeClient,
170+
Scheme: scheme,
171+
}
172+
173+
condStatus := func() *metav1.Condition {
174+
return meta.FindStatusCondition(proxy.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated)
175+
}
176+
177+
// valid -> condition True + hash tracked
178+
require.NoError(t, r.handleAuthzConfig(ctx, proxy))
179+
require.NotNil(t, condStatus())
180+
assert.Equal(t, metav1.ConditionTrue, condStatus().Status)
181+
assert.Equal(t, "h1", proxy.Status.AuthzConfigHash)
182+
183+
// flip the referenced config to invalid -> condition transitions to False/NotValid
184+
authzConfig.Status.Conditions = []metav1.Condition{{
185+
Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionFalse, Reason: "Invalidated",
186+
}}
187+
require.NoError(t, fakeClient.Status().Update(ctx, authzConfig))
188+
189+
assert.Error(t, r.handleAuthzConfig(ctx, proxy))
190+
require.NotNil(t, condStatus())
191+
assert.Equal(t, metav1.ConditionFalse, condStatus().Status)
192+
assert.Equal(t, mcpv1beta1.ConditionReasonAuthzConfigRefNotValid, condStatus().Reason)
193+
194+
// recover the config -> condition transitions back to True
195+
authzConfig.Status.Conditions = []metav1.Condition{{
196+
Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionTrue, Reason: "Test",
197+
}}
198+
require.NoError(t, fakeClient.Status().Update(ctx, authzConfig))
199+
200+
require.NoError(t, r.handleAuthzConfig(ctx, proxy))
201+
require.NotNil(t, condStatus())
202+
assert.Equal(t, metav1.ConditionTrue, condStatus().Status)
203+
}
204+
205+
// TestMapAuthzConfigToMCPRemoteProxy verifies that only proxies referencing the
206+
// changed MCPAuthzConfig are enqueued for reconciliation.
207+
func TestMapAuthzConfigToMCPRemoteProxy(t *testing.T) {
208+
t.Parallel()
209+
210+
scheme := runtime.NewScheme()
211+
require.NoError(t, mcpv1beta1.AddToScheme(scheme))
212+
213+
proxy1 := &mcpv1beta1.MCPRemoteProxy{
214+
ObjectMeta: metav1.ObjectMeta{Name: "proxy1", Namespace: "default"},
215+
Spec: mcpv1beta1.MCPRemoteProxySpec{
216+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "shared-authz"},
217+
},
218+
}
219+
proxy2 := &mcpv1beta1.MCPRemoteProxy{
220+
ObjectMeta: metav1.ObjectMeta{Name: "proxy2", Namespace: "default"},
221+
Spec: mcpv1beta1.MCPRemoteProxySpec{
222+
AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "other-authz"},
223+
},
224+
}
225+
proxy3 := &mcpv1beta1.MCPRemoteProxy{
226+
ObjectMeta: metav1.ObjectMeta{Name: "proxy3", Namespace: "default"},
227+
Spec: mcpv1beta1.MCPRemoteProxySpec{}, // no ref
228+
}
229+
230+
fakeClient := fake.NewClientBuilder().
231+
WithScheme(scheme).
232+
WithObjects(proxy1, proxy2, proxy3).
233+
Build()
234+
235+
reconciler := &MCPRemoteProxyReconciler{
236+
Client: fakeClient,
237+
Scheme: scheme,
238+
}
239+
240+
ctx := t.Context()
241+
242+
authzConfig := &mcpv1beta1.MCPAuthzConfig{
243+
ObjectMeta: metav1.ObjectMeta{Name: "shared-authz", Namespace: "default"},
244+
}
245+
246+
requests := reconciler.mapAuthzConfigToMCPRemoteProxy(ctx, authzConfig)
247+
248+
require.Len(t, requests, 1)
249+
assert.Equal(t, types.NamespacedName{Name: "proxy1", Namespace: "default"}, requests[0].NamespacedName)
250+
}

0 commit comments

Comments
 (0)