Skip to content

Commit 2111396

Browse files
ChrisJBurnsclaude
andcommitted
Extract resolveOIDCConfig helper in VirtualMCPServer converter
Collapse the two OIDC resolution branches (MCPOIDCConfigRef and legacy inline OIDCConfig) into a single resolveOIDCConfig method that returns an immutable (*vmcpconfig.OIDCConfig, error). This simplifies convertIncomingAuth by making incoming auth construction a single assignment rather than a mutable struct with conditional field mutation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 862f9b8 commit 2111396

1 file changed

Lines changed: 51 additions & 37 deletions

File tree

cmd/thv-operator/pkg/vmcpconfig/converter.go

Lines changed: 51 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,53 @@ func (c *Converter) convertIncomingAuth(
171171
ctx context.Context,
172172
vmcp *mcpv1alpha1.VirtualMCPServer,
173173
) (*vmcpconfig.IncomingAuthConfig, error) {
174-
ctxLogger := log.FromContext(ctx)
174+
oidcConfig, err := c.resolveOIDCConfig(ctx, vmcp)
175+
if err != nil {
176+
return nil, err
177+
}
175178

176179
incoming := &vmcpconfig.IncomingAuthConfig{
177180
Type: vmcp.Spec.IncomingAuth.Type,
181+
OIDC: oidcConfig,
182+
}
183+
184+
// Convert authorization configuration
185+
if vmcp.Spec.IncomingAuth.AuthzConfig != nil {
186+
// Map Kubernetes API types to vmcp config types
187+
// API "inline" maps to vmcp "cedar"
188+
authzType := vmcp.Spec.IncomingAuth.AuthzConfig.Type
189+
if authzType == authzLabelValueInline {
190+
authzType = "cedar"
191+
}
192+
193+
incoming.Authz = &vmcpconfig.AuthzConfig{
194+
Type: authzType,
195+
}
196+
197+
// Handle inline policies
198+
if vmcp.Spec.IncomingAuth.AuthzConfig.Type == authzLabelValueInline && vmcp.Spec.IncomingAuth.AuthzConfig.Inline != nil {
199+
incoming.Authz.Policies = vmcp.Spec.IncomingAuth.AuthzConfig.Inline.Policies
200+
}
201+
// TODO: Load policies from ConfigMap if Type is "configMap"
178202
}
179203

180-
// Convert OIDC configuration if present.
204+
return incoming, nil
205+
}
206+
207+
// resolveOIDCConfig resolves OIDC configuration from either an MCPOIDCConfig reference
208+
// or legacy inline OIDCConfig. Returns nil when no OIDC config is present.
209+
// Fails closed: returns an error when OIDC is configured but resolution fails,
210+
// preventing deployment without authentication when OIDC is explicitly requested.
211+
func (c *Converter) resolveOIDCConfig(
212+
ctx context.Context,
213+
vmcp *mcpv1alpha1.VirtualMCPServer,
214+
) (*vmcpconfig.OIDCConfig, error) {
215+
if vmcp.Spec.IncomingAuth == nil {
216+
return nil, nil
217+
}
218+
219+
ctxLogger := log.FromContext(ctx)
220+
181221
// New path: resolve from MCPOIDCConfig reference (preferred over legacy inline OIDCConfig)
182222
if vmcp.Spec.IncomingAuth.OIDCConfigRef != nil {
183223
oidcCfg, err := controllerutil.GetOIDCConfigForServer(
@@ -186,7 +226,7 @@ func (c *Converter) convertIncomingAuth(
186226
return nil, fmt.Errorf("failed to get MCPOIDCConfig %s: %w",
187227
vmcp.Spec.IncomingAuth.OIDCConfigRef.Name, err)
188228
}
189-
resolvedConfig, err := c.oidcResolver.ResolveFromConfigRef(
229+
resolved, err := c.oidcResolver.ResolveFromConfigRef(
190230
ctx, vmcp.Spec.IncomingAuth.OIDCConfigRef, oidcCfg,
191231
vmcp.Name, vmcp.Namespace, vmcp.GetProxyPort())
192232
if err != nil {
@@ -197,50 +237,24 @@ func (c *Converter) convertIncomingAuth(
197237
return nil, fmt.Errorf("OIDC resolution failed from MCPOIDCConfig %q: %w",
198238
vmcp.Spec.IncomingAuth.OIDCConfigRef.Name, err)
199239
}
200-
if resolvedConfig != nil {
201-
incoming.OIDC = mapResolvedOIDCToVmcpConfigFromRef(resolvedConfig, oidcCfg)
202-
}
203-
} else if vmcp.Spec.IncomingAuth.OIDCConfig != nil {
204-
// Legacy path: resolve from inline OIDCConfig
205-
// Use the OIDC resolver to handle all OIDC types (kubernetes, configMap, inline)
206-
// VirtualMCPServer implements OIDCConfigurable, so the resolver can work with it directly
207-
resolvedConfig, err := c.oidcResolver.Resolve(ctx, vmcp)
240+
return mapResolvedOIDCToVmcpConfigFromRef(resolved, oidcCfg), nil
241+
}
242+
243+
// Legacy path: resolve from inline OIDCConfig
244+
if vmcp.Spec.IncomingAuth.OIDCConfig != nil {
245+
resolved, err := c.oidcResolver.Resolve(ctx, vmcp)
208246
if err != nil {
209247
ctxLogger.Error(err, "failed to resolve OIDC config",
210248
"vmcp", vmcp.Name,
211249
"namespace", vmcp.Namespace,
212250
"oidcType", vmcp.Spec.IncomingAuth.OIDCConfig.Type)
213-
// Fail closed: return error when OIDC is configured but resolution fails
214-
// This prevents deploying without authentication when OIDC is explicitly requested
215251
return nil, fmt.Errorf("OIDC resolution failed for type %q: %w",
216252
vmcp.Spec.IncomingAuth.OIDCConfig.Type, err)
217253
}
218-
if resolvedConfig != nil {
219-
incoming.OIDC = mapResolvedOIDCToVmcpConfig(resolvedConfig, vmcp.Spec.IncomingAuth.OIDCConfig)
220-
}
254+
return mapResolvedOIDCToVmcpConfig(resolved, vmcp.Spec.IncomingAuth.OIDCConfig), nil
221255
}
222256

223-
// Convert authorization configuration
224-
if vmcp.Spec.IncomingAuth.AuthzConfig != nil {
225-
// Map Kubernetes API types to vmcp config types
226-
// API "inline" maps to vmcp "cedar"
227-
authzType := vmcp.Spec.IncomingAuth.AuthzConfig.Type
228-
if authzType == authzLabelValueInline {
229-
authzType = "cedar"
230-
}
231-
232-
incoming.Authz = &vmcpconfig.AuthzConfig{
233-
Type: authzType,
234-
}
235-
236-
// Handle inline policies
237-
if vmcp.Spec.IncomingAuth.AuthzConfig.Type == authzLabelValueInline && vmcp.Spec.IncomingAuth.AuthzConfig.Inline != nil {
238-
incoming.Authz.Policies = vmcp.Spec.IncomingAuth.AuthzConfig.Inline.Policies
239-
}
240-
// TODO: Load policies from ConfigMap if Type is "configMap"
241-
}
242-
243-
return incoming, nil
257+
return nil, nil
244258
}
245259

246260
// mapResolvedOIDCToVmcpConfig maps from oidc.OIDCConfig (resolved by the OIDC resolver)

0 commit comments

Comments
 (0)