Skip to content

Commit 3079635

Browse files
ChrisJBurnsclaude
andcommitted
Replace ReferencingServers with structured ReferencingWorkloads on MCPOIDCConfig
RFC-0023 specifies a structured referencingWorkloads list with kind and name fields rather than plain strings. This replaces the ReferencingServers []string field on MCPOIDCConfig with ReferencingWorkloads []WorkloadReference, a new type with Kind and Name fields that explicitly identifies each referencing workload. The WorkloadReference type supports MCPServer, VirtualMCPServer, and MCPRemoteProxy kinds, making the data model self-describing and type-safe. The MCPServer controller and MCPOIDCConfig controller are updated to use the new structured type throughout: reference tracking, deletion protection, watch handlers, and status updates. The same migration is needed for MCPToolConfig, MCPTelemetryConfig, and MCPExternalAuthConfig — tracked in #4491. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent c3fadd1 commit 3079635

10 files changed

Lines changed: 177 additions & 76 deletions

File tree

cmd/thv-operator/api/v1alpha1/mcpoidcconfig_types.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,20 @@ type InlineOIDCSharedConfig struct {
132132
InsecureAllowHTTP bool `json:"insecureAllowHTTP"`
133133
}
134134

135+
// WorkloadReference identifies a workload that references a shared configuration resource.
136+
// Namespace is implicit — cross-namespace references are not supported.
137+
type WorkloadReference struct {
138+
// Kind is the type of workload resource
139+
// +kubebuilder:validation:Enum=MCPServer;VirtualMCPServer;MCPRemoteProxy
140+
// +kubebuilder:validation:Required
141+
Kind string `json:"kind"`
142+
143+
// Name is the name of the workload resource
144+
// +kubebuilder:validation:Required
145+
// +kubebuilder:validation:MinLength=1
146+
Name string `json:"name"`
147+
}
148+
135149
// MCPOIDCConfigStatus defines the observed state of MCPOIDCConfig
136150
type MCPOIDCConfigStatus struct {
137151
// Conditions represent the latest available observations of the MCPOIDCConfig's state
@@ -146,9 +160,10 @@ type MCPOIDCConfigStatus struct {
146160
// +optional
147161
ConfigHash string `json:"configHash,omitempty"`
148162

149-
// ReferencingServers is a list of MCPServer resources that reference this MCPOIDCConfig
163+
// ReferencingWorkloads is a list of workload resources that reference this MCPOIDCConfig.
164+
// Each entry identifies the workload by kind and name.
150165
// +optional
151-
ReferencingServers []string `json:"referencingServers,omitempty"`
166+
ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"`
152167
}
153168

154169
// +kubebuilder:object:root=true

cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 18 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

cmd/thv-operator/controllers/mcpoidcconfig_controller.go

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,12 @@ func (r *MCPOIDCConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reques
124124
return ctrl.Result{}, nil
125125
}
126126

127-
// Refresh ReferencingServers list
128-
referencingServers, err := r.findReferencingServers(ctx, oidcConfig)
127+
// Refresh ReferencingWorkloads list
128+
referencingWorkloads, err := r.findReferencingWorkloads(ctx, oidcConfig)
129129
if err != nil {
130-
logger.Error(err, "Failed to find referencing servers")
131-
} else if !slices.Equal(oidcConfig.Status.ReferencingServers, referencingServers) {
132-
oidcConfig.Status.ReferencingServers = referencingServers
130+
logger.Error(err, "Failed to find referencing workloads")
131+
} else if !slices.Equal(oidcConfig.Status.ReferencingWorkloads, referencingWorkloads) {
132+
oidcConfig.Status.ReferencingWorkloads = referencingWorkloads
133133
conditionChanged = true
134134
}
135135

@@ -160,26 +160,26 @@ func (r *MCPOIDCConfigReconciler) handleDeletion(
160160
logger := log.FromContext(ctx)
161161

162162
if controllerutil.ContainsFinalizer(oidcConfig, OIDCConfigFinalizerName) {
163-
// Check if any MCPServers still reference this config
164-
referencingServers, err := r.findReferencingServers(ctx, oidcConfig)
163+
// Check if any workloads still reference this config
164+
referencingWorkloads, err := r.findReferencingWorkloads(ctx, oidcConfig)
165165
if err != nil {
166-
logger.Error(err, "Failed to check referencing servers during deletion")
166+
logger.Error(err, "Failed to check referencing workloads during deletion")
167167
return ctrl.Result{}, err
168168
}
169169

170-
if len(referencingServers) > 0 {
171-
logger.Info("MCPOIDCConfig is still referenced by MCPServers, blocking deletion",
170+
if len(referencingWorkloads) > 0 {
171+
logger.Info("MCPOIDCConfig is still referenced by workloads, blocking deletion",
172172
"oidcConfig", oidcConfig.Name,
173-
"referencingServers", referencingServers)
173+
"referencingWorkloads", referencingWorkloads)
174174

175175
meta.SetStatusCondition(&oidcConfig.Status.Conditions, metav1.Condition{
176176
Type: "DeletionBlocked",
177177
Status: metav1.ConditionTrue,
178-
Reason: "ReferencedByServers",
179-
Message: fmt.Sprintf("Cannot delete: referenced by MCPServers: %v", referencingServers),
178+
Reason: "ReferencedByWorkloads",
179+
Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads),
180180
ObservedGeneration: oidcConfig.Generation,
181181
})
182-
oidcConfig.Status.ReferencingServers = referencingServers
182+
oidcConfig.Status.ReferencingWorkloads = referencingWorkloads
183183
if updateErr := r.Status().Update(ctx, oidcConfig); updateErr != nil {
184184
logger.Error(updateErr, "Failed to update status during deletion block")
185185
}
@@ -199,32 +199,32 @@ func (r *MCPOIDCConfigReconciler) handleDeletion(
199199
return ctrl.Result{}, nil
200200
}
201201

202-
// findReferencingServers returns the names of MCPServer resources that reference
203-
// this MCPOIDCConfig via their OIDCConfigRef field.
204-
func (r *MCPOIDCConfigReconciler) findReferencingServers(
202+
// findReferencingWorkloads returns the workload resources (MCPServer)
203+
// that reference this MCPOIDCConfig via their OIDCConfigRef field.
204+
func (r *MCPOIDCConfigReconciler) findReferencingWorkloads(
205205
ctx context.Context,
206206
oidcConfig *mcpv1alpha1.MCPOIDCConfig,
207-
) ([]string, error) {
207+
) ([]mcpv1alpha1.WorkloadReference, error) {
208208
mcpServerList := &mcpv1alpha1.MCPServerList{}
209209
if err := r.List(ctx, mcpServerList, client.InNamespace(oidcConfig.Namespace)); err != nil {
210210
return nil, fmt.Errorf("failed to list MCPServers: %w", err)
211211
}
212212

213-
var refs []string
213+
var refs []mcpv1alpha1.WorkloadReference
214214
for _, server := range mcpServerList.Items {
215215
if server.Spec.OIDCConfigRef != nil && server.Spec.OIDCConfigRef.Name == oidcConfig.Name {
216-
refs = append(refs, server.Name)
216+
refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: server.Name})
217217
}
218218
}
219219
return refs, nil
220220
}
221221

222222
// SetupWithManager sets up the controller with the Manager.
223-
// Watches MCPServer changes to maintain accurate ReferencingServers status.
223+
// Watches MCPServer changes to maintain accurate ReferencingWorkloads status.
224224
func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
225-
// Watch MCPServer changes to update ReferencingServers on referenced MCPOIDCConfigs.
225+
// Watch MCPServer changes to update ReferencingWorkloads on referenced MCPOIDCConfigs.
226226
// This handler enqueues both the currently-referenced MCPOIDCConfig AND any
227-
// MCPOIDCConfig that still lists this server in ReferencingServers (covers the
227+
// MCPOIDCConfig that still lists this server in ReferencingWorkloads (covers the
228228
// case where a server removes its oidcConfigRef — the previously-referenced
229229
// config needs to reconcile and clean up the stale entry).
230230
mcpServerHandler := handler.EnqueueRequestsFromMapFunc(
@@ -248,7 +248,7 @@ func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
248248
}
249249

250250
// Also enqueue any MCPOIDCConfig that still lists this server in
251-
// ReferencingServers — handles ref-removal and server-deletion cases.
251+
// ReferencingWorkloads — handles ref-removal and server-deletion cases.
252252
oidcConfigList := &mcpv1alpha1.MCPOIDCConfigList{}
253253
if err := r.List(ctx, oidcConfigList, client.InNamespace(server.Namespace)); err != nil {
254254
log.FromContext(ctx).Error(err, "Failed to list MCPOIDCConfigs for MCPServer watch")
@@ -259,8 +259,8 @@ func (r *MCPOIDCConfigReconciler) SetupWithManager(mgr ctrl.Manager) error {
259259
if _, already := seen[nn]; already {
260260
continue
261261
}
262-
for _, ref := range cfg.Status.ReferencingServers {
263-
if ref == server.Name {
262+
for _, ref := range cfg.Status.ReferencingWorkloads {
263+
if ref.Kind == "MCPServer" && ref.Name == server.Name {
264264
requests = append(requests, reconcile.Request{NamespacedName: nn})
265265
break
266266
}

cmd/thv-operator/controllers/mcpserver_controller.go

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2085,9 +2085,9 @@ func (r *MCPServerReconciler) handleOIDCConfig(ctx context.Context, m *mcpv1alph
20852085
return fmt.Errorf("%s", msg)
20862086
}
20872087

2088-
// Update ReferencingServers on the MCPOIDCConfig status
2089-
if err := r.updateOIDCConfigReferencingServers(ctx, oidcConfig, m.Name); err != nil {
2090-
ctxLogger.Error(err, "Failed to update MCPOIDCConfig ReferencingServers")
2088+
// Update ReferencingWorkloads on the MCPOIDCConfig status
2089+
if err := r.updateOIDCConfigReferencingWorkloads(ctx, oidcConfig, m.Name); err != nil {
2090+
ctxLogger.Error(err, "Failed to update MCPOIDCConfig ReferencingWorkloads")
20912091
// Non-fatal: continue with reconciliation
20922092
}
20932093

@@ -2124,24 +2124,29 @@ func setOIDCConfigRefCondition(m *mcpv1alpha1.MCPServer, status metav1.Condition
21242124
})
21252125
}
21262126

2127-
// updateOIDCConfigReferencingServers ensures the MCPServer name is listed in
2128-
// the MCPOIDCConfig's ReferencingServers status field.
2129-
func (r *MCPServerReconciler) updateOIDCConfigReferencingServers(
2127+
// updateOIDCConfigReferencingWorkloads ensures the MCPServer is listed in
2128+
// the MCPOIDCConfig's ReferencingWorkloads status field.
2129+
func (r *MCPServerReconciler) updateOIDCConfigReferencingWorkloads(
21302130
ctx context.Context,
21312131
oidcConfig *mcpv1alpha1.MCPOIDCConfig,
21322132
serverName string,
21332133
) error {
2134+
ref := mcpv1alpha1.WorkloadReference{
2135+
Kind: "MCPServer",
2136+
Name: serverName,
2137+
}
2138+
21342139
// Check if already listed
2135-
for _, name := range oidcConfig.Status.ReferencingServers {
2136-
if name == serverName {
2140+
for _, entry := range oidcConfig.Status.ReferencingWorkloads {
2141+
if entry.Kind == ref.Kind && entry.Name == ref.Name {
21372142
return nil
21382143
}
21392144
}
21402145

2141-
// Add the server name
2142-
oidcConfig.Status.ReferencingServers = append(oidcConfig.Status.ReferencingServers, serverName)
2146+
// Add the workload reference
2147+
oidcConfig.Status.ReferencingWorkloads = append(oidcConfig.Status.ReferencingWorkloads, ref)
21432148
if err := r.Status().Update(ctx, oidcConfig); err != nil {
2144-
return fmt.Errorf("failed to update MCPOIDCConfig ReferencingServers: %w", err)
2149+
return fmt.Errorf("failed to update MCPOIDCConfig ReferencingWorkloads: %w", err)
21452150
}
21462151

21472152
return nil

cmd/thv-operator/controllers/mcpserver_oidcconfig_test.go

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -203,49 +203,53 @@ func TestMCPServerReconciler_handleOIDCConfig(t *testing.T) {
203203
if tt.expectReferencingServer && tt.oidcConfig != nil {
204204
var updated mcpv1alpha1.MCPOIDCConfig
205205
require.NoError(t, fakeClient.Get(ctx, client.ObjectKeyFromObject(tt.oidcConfig), &updated))
206-
assert.Contains(t, updated.Status.ReferencingServers, tt.mcpServer.Name)
206+
expectedRef := mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: tt.mcpServer.Name}
207+
assert.Contains(t, updated.Status.ReferencingWorkloads, expectedRef)
207208
}
208209
})
209210
}
210211
}
211212

212-
func TestMCPServerReconciler_updateOIDCConfigReferencingServers(t *testing.T) {
213+
func TestMCPServerReconciler_updateOIDCConfigReferencingWorkloads(t *testing.T) {
213214
t.Parallel()
214215

215-
t.Run("adds new server name", func(t *testing.T) {
216+
existingRef := mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "existing"}
217+
218+
t.Run("adds new server reference", func(t *testing.T) {
216219
t.Parallel()
217220
ctx := t.Context()
218221
scheme := runtime.NewScheme()
219222
require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
220223

221224
cfg := &mcpv1alpha1.MCPOIDCConfig{
222225
ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "default"},
223-
Status: mcpv1alpha1.MCPOIDCConfigStatus{ReferencingServers: []string{"existing"}},
226+
Status: mcpv1alpha1.MCPOIDCConfigStatus{ReferencingWorkloads: []mcpv1alpha1.WorkloadReference{existingRef}},
224227
}
225228
fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cfg).
226229
WithStatusSubresource(&mcpv1alpha1.MCPOIDCConfig{}).Build()
227230
r := newTestMCPServerReconciler(fc, scheme, kubernetes.PlatformKubernetes)
228231

229-
require.NoError(t, r.updateOIDCConfigReferencingServers(ctx, cfg, "new"))
230-
assert.ElementsMatch(t, []string{"existing", "new"}, cfg.Status.ReferencingServers)
232+
require.NoError(t, r.updateOIDCConfigReferencingWorkloads(ctx, cfg, "new"))
233+
newRef := mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "new"}
234+
assert.ElementsMatch(t, []mcpv1alpha1.WorkloadReference{existingRef, newRef}, cfg.Status.ReferencingWorkloads)
231235
})
232236

233-
t.Run("does not duplicate existing name", func(t *testing.T) {
237+
t.Run("does not duplicate existing reference", func(t *testing.T) {
234238
t.Parallel()
235239
ctx := t.Context()
236240
scheme := runtime.NewScheme()
237241
require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
238242

239243
cfg := &mcpv1alpha1.MCPOIDCConfig{
240244
ObjectMeta: metav1.ObjectMeta{Name: "cfg", Namespace: "default"},
241-
Status: mcpv1alpha1.MCPOIDCConfigStatus{ReferencingServers: []string{"existing"}},
245+
Status: mcpv1alpha1.MCPOIDCConfigStatus{ReferencingWorkloads: []mcpv1alpha1.WorkloadReference{existingRef}},
242246
}
243247
fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cfg).
244248
WithStatusSubresource(&mcpv1alpha1.MCPOIDCConfig{}).Build()
245249
r := newTestMCPServerReconciler(fc, scheme, kubernetes.PlatformKubernetes)
246250

247-
require.NoError(t, r.updateOIDCConfigReferencingServers(ctx, cfg, "existing"))
248-
assert.Len(t, cfg.Status.ReferencingServers, 1)
251+
require.NoError(t, r.updateOIDCConfigReferencingWorkloads(ctx, cfg, "existing"))
252+
assert.Len(t, cfg.Status.ReferencingWorkloads, 1)
249253
})
250254
}
251255

cmd/thv-operator/test-integration/mcp-oidc-config/mcpoidcconfig_controller_integration_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@ import (
1717
const (
1818
timeout = time.Second * 30
1919
interval = time.Millisecond * 250
20+
21+
// conditionTypeValid is the MCPOIDCConfig "Valid" condition type
22+
conditionTypeValid = "Valid"
2023
)
2124

2225
var _ = Describe("MCPOIDCConfig Controller", func() {
@@ -61,7 +64,7 @@ var _ = Describe("MCPOIDCConfig Controller", func() {
6164
return false
6265
}
6366
for _, cond := range fetched.Status.Conditions {
64-
if cond.Type == "Valid" && cond.Status == metav1.ConditionTrue {
67+
if cond.Type == conditionTypeValid && cond.Status == metav1.ConditionTrue {
6568
return true
6669
}
6770
}

0 commit comments

Comments
 (0)