From ef51555ab65b5569c9e96b9230eef066cf0c69d7 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Thu, 16 Oct 2025 14:35:51 +0300 Subject: [PATCH] Refactor OIDC resolver to use generic OIDCConfigurable interface This change makes the OIDC resolver more flexible and reusable across different resource types by using a generic interface instead of being tied to MCPServer specifically. Key changes: - Add OIDCConfigurable interface with GetName(), GetNamespace(), GetOIDCConfig(), and GetPort() methods - Refactor Resolve() method to accept OIDCConfigurable interface instead of MCPServer directly - Update helper methods (resolveKubernetesConfig and resolveConfigMapConfig) to accept namespace string instead of MCPServer object - Implement OIDCConfigurable interface for MCPServer type - Remove duplicate MCPServer-specific resolution methods This enables both MCPServer and future resource types (like MCPRemoteProxy) to share the same OIDC configuration resolution logic without code duplication. The MCPServer controller continues to work unchanged, calling resolver.Resolve(ctx, mcpServer). All existing tests pass without modification. --- .../api/v1alpha1/mcpserver_types.go | 20 ++++++++ cmd/thv-operator/pkg/oidc/resolver.go | 46 +++++++++++-------- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go index 4decfcd72a..9668acf920 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go @@ -659,6 +659,26 @@ type MCPServerList struct { Items []MCPServer `json:"items"` } +// GetName returns the name of the MCPServer +func (m *MCPServer) GetName() string { + return m.Name +} + +// GetNamespace returns the namespace of the MCPServer +func (m *MCPServer) GetNamespace() string { + return m.Namespace +} + +// GetOIDCConfig returns the OIDC configuration reference +func (m *MCPServer) GetOIDCConfig() *OIDCConfigRef { + return m.Spec.OIDCConfig +} + +// GetPort returns the port of the MCPServer +func (m *MCPServer) GetPort() int32 { + return m.Spec.Port +} + func init() { SchemeBuilder.Register(&MCPServer{}, &MCPServerList{}) } diff --git a/cmd/thv-operator/pkg/oidc/resolver.go b/cmd/thv-operator/pkg/oidc/resolver.go index 91ff7d4d14..4c961662d4 100644 --- a/cmd/thv-operator/pkg/oidc/resolver.go +++ b/cmd/thv-operator/pkg/oidc/resolver.go @@ -36,10 +36,20 @@ type OIDCConfig struct { //nolint:revive // Keeping OIDCConfig name for backward JWKSAllowPrivateIP bool } +// OIDCConfigurable is an interface for resources that have OIDC configuration +// +//nolint:revive // Intentionally named OIDCConfigurable for clarity +type OIDCConfigurable interface { + GetName() string + GetNamespace() string + GetOIDCConfig() *mcpv1alpha1.OIDCConfigRef + GetPort() int32 +} + // Resolver is the interface for resolving OIDC configuration from various sources type Resolver interface { - // Resolve takes an MCPServer and its OIDC configuration reference and returns the resolved OIDC config - Resolve(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) (*OIDCConfig, error) + // Resolve takes any resource implementing OIDCConfigurable and resolves its OIDC config + Resolve(ctx context.Context, resource OIDCConfigurable) (*OIDCConfig, error) } // NewResolver creates a new OIDC configuration resolver @@ -55,25 +65,24 @@ type resolver struct { client client.Client } -// Resolve resolves the OIDC configuration based on the type specified in OIDCConfigRef -func (r *resolver) Resolve(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer) (*OIDCConfig, error) { - if mcpServer.Spec.OIDCConfig == nil { +// Resolve resolves the OIDC configuration from any resource implementing OIDCConfigurable +func (r *resolver) Resolve(ctx context.Context, resource OIDCConfigurable) (*OIDCConfig, error) { + oidcConfig := resource.GetOIDCConfig() + if oidcConfig == nil { return nil, nil } - oidcConfig := mcpServer.Spec.OIDCConfig - // Calculate resource URL for RFC 9728 compliance resourceURL := oidcConfig.ResourceURL if resourceURL == "" { - resourceURL = createServiceURL(mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Port) + resourceURL = createServiceURL(resource.GetName(), resource.GetNamespace(), resource.GetPort()) } switch oidcConfig.Type { case mcpv1alpha1.OIDCConfigTypeKubernetes: - return r.resolveKubernetesConfig(ctx, oidcConfig.Kubernetes, resourceURL, mcpServer) + return r.resolveKubernetesConfig(ctx, oidcConfig.Kubernetes, resourceURL, resource.GetNamespace()) case mcpv1alpha1.OIDCConfigTypeConfigMap: - return r.resolveConfigMapConfig(ctx, oidcConfig.ConfigMap, resourceURL, mcpServer) + return r.resolveConfigMapConfig(ctx, oidcConfig.ConfigMap, resourceURL, resource.GetNamespace()) case mcpv1alpha1.OIDCConfigTypeInline: return r.resolveInlineConfig(oidcConfig.Inline, resourceURL) default: @@ -81,17 +90,16 @@ func (r *resolver) Resolve(ctx context.Context, mcpServer *mcpv1alpha1.MCPServer } } -// resolveKubernetesConfig resolves OIDC configuration for Kubernetes type +// resolveKubernetesConfig resolves Kubernetes OIDC config using namespace directly func (*resolver) resolveKubernetesConfig( ctx context.Context, config *mcpv1alpha1.KubernetesOIDCConfig, resourceURL string, - mcpServer *mcpv1alpha1.MCPServer, + namespace string, ) (*OIDCConfig, error) { - // Set defaults if config is nil if config == nil { ctxLogger := log.FromContext(ctx) - ctxLogger.Info("Kubernetes OIDCConfig is nil, using default configuration", "mcpServer", mcpServer.Name) + ctxLogger.Info("Kubernetes OIDCConfig is nil, using default configuration", "namespace", namespace) defaultUseClusterAuth := true config = &mcpv1alpha1.KubernetesOIDCConfig{ UseClusterAuth: &defaultUseClusterAuth, @@ -99,7 +107,7 @@ func (*resolver) resolveKubernetesConfig( } // Handle UseClusterAuth with default of true if nil - useClusterAuth := true // default value + useClusterAuth := true if config.UseClusterAuth != nil { useClusterAuth = *config.UseClusterAuth } @@ -134,12 +142,12 @@ func (*resolver) resolveKubernetesConfig( return result, nil } -// resolveConfigMapConfig resolves OIDC configuration from a ConfigMap +// resolveConfigMapConfig resolves ConfigMap OIDC config using namespace directly func (r *resolver) resolveConfigMapConfig( ctx context.Context, configRef *mcpv1alpha1.ConfigMapOIDCRef, resourceURL string, - mcpServer *mcpv1alpha1.MCPServer, + namespace string, ) (*OIDCConfig, error) { if configRef == nil { return nil, nil @@ -153,11 +161,11 @@ func (r *resolver) resolveConfigMapConfig( configMap := &corev1.ConfigMap{} err := r.client.Get(ctx, types.NamespacedName{ Name: configRef.Name, - Namespace: mcpServer.Namespace, + Namespace: namespace, }, configMap) if err != nil { return nil, fmt.Errorf("failed to get OIDC ConfigMap %s/%s: %w", - mcpServer.Namespace, configRef.Name, err) + namespace, configRef.Name, err) } config := &OIDCConfig{