diff --git a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go index c98b395a73..ba5d278046 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go @@ -76,6 +76,12 @@ type MCPRemoteProxySpec struct { // +optional ExternalAuthConfigRef *ExternalAuthConfigRef `json:"externalAuthConfigRef,omitempty"` + // AuthServerRef optionally references a resource that configures an embedded + // OAuth 2.0/OIDC authorization server to authenticate MCP clients. + // Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + // +optional + AuthServerRef *AuthServerRef `json:"authServerRef,omitempty"` + // HeaderForward configures headers to inject into requests to the remote MCP server. // Use this to add custom headers like X-Tenant-ID or correlation IDs. // +optional @@ -172,6 +178,11 @@ type MCPRemoteProxyStatus struct { // +optional ExternalAuthConfigHash string `json:"externalAuthConfigHash,omitempty"` + // AuthServerConfigHash is the hash of the referenced authServerRef spec, + // used to detect configuration changes and trigger reconciliation. + // +optional + AuthServerConfigHash string `json:"authServerConfigHash,omitempty"` + // OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection // +optional OIDCConfigHash string `json:"oidcConfigHash,omitempty"` @@ -219,6 +230,9 @@ const ( // ConditionTypeMCPRemoteProxyExternalAuthConfigValidated indicates whether the ExternalAuthConfigRef is valid ConditionTypeMCPRemoteProxyExternalAuthConfigValidated = "ExternalAuthConfigValidated" + // ConditionTypeMCPRemoteProxyAuthServerRefValidated indicates whether the AuthServerRef is valid + ConditionTypeMCPRemoteProxyAuthServerRefValidated = "AuthServerRefValidated" + // ConditionTypeConfigurationValid indicates whether the proxy spec has passed all pre-deployment validation checks ConditionTypeConfigurationValid = "ConfigurationValid" ) @@ -277,6 +291,24 @@ const ( // for MCPRemoteProxy (use VirtualMCPServer for multi-upstream). ConditionReasonMCPRemoteProxyExternalAuthConfigMultiUpstream = "MultiUpstreamNotSupported" + // ConditionReasonMCPRemoteProxyAuthServerRefValid indicates the AuthServerRef is valid + ConditionReasonMCPRemoteProxyAuthServerRefValid = "AuthServerRefValid" + + // ConditionReasonMCPRemoteProxyAuthServerRefNotFound indicates the referenced auth server config was not found + ConditionReasonMCPRemoteProxyAuthServerRefNotFound = "AuthServerRefNotFound" + + // ConditionReasonMCPRemoteProxyAuthServerRefFetchError indicates an error occurred fetching the auth server config + ConditionReasonMCPRemoteProxyAuthServerRefFetchError = "AuthServerRefFetchError" + + // ConditionReasonMCPRemoteProxyAuthServerRefInvalidKind indicates the authServerRef kind is not supported + ConditionReasonMCPRemoteProxyAuthServerRefInvalidKind = "AuthServerRefInvalidKind" + + // ConditionReasonMCPRemoteProxyAuthServerRefInvalidType indicates the referenced config is not an embeddedAuthServer + ConditionReasonMCPRemoteProxyAuthServerRefInvalidType = "AuthServerRefInvalidType" + + // ConditionReasonMCPRemoteProxyAuthServerRefMultiUpstream indicates multi-upstream is not supported + ConditionReasonMCPRemoteProxyAuthServerRefMultiUpstream = "MultiUpstreamNotSupported" + // ConditionReasonConfigurationValid indicates all configuration validations passed ConditionReasonConfigurationValid = "ConfigurationValid" diff --git a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go index dd665f4250..543de8db7c 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go @@ -108,6 +108,31 @@ const ( ConditionReasonExternalAuthConfigMultiUpstream = "MultiUpstreamNotSupported" ) +const ( + // ConditionTypeAuthServerRefValidated indicates whether the AuthServerRef is valid + ConditionTypeAuthServerRefValidated = "AuthServerRefValidated" +) + +const ( + // ConditionReasonAuthServerRefValid indicates the referenced auth server config is valid + ConditionReasonAuthServerRefValid = "AuthServerRefValid" + + // ConditionReasonAuthServerRefNotFound indicates the referenced auth server config was not found + ConditionReasonAuthServerRefNotFound = "AuthServerRefNotFound" + + // ConditionReasonAuthServerRefFetchError indicates an error occurred fetching the auth server config + ConditionReasonAuthServerRefFetchError = "AuthServerRefFetchError" + + // ConditionReasonAuthServerRefInvalidKind indicates the authServerRef kind is not supported + ConditionReasonAuthServerRefInvalidKind = "AuthServerRefInvalidKind" + + // ConditionReasonAuthServerRefInvalidType indicates the referenced config is not an embeddedAuthServer + ConditionReasonAuthServerRefInvalidType = "AuthServerRefInvalidType" + + // ConditionReasonAuthServerRefMultiUpstream indicates multi-upstream is not supported + ConditionReasonAuthServerRefMultiUpstream = "MultiUpstreamNotSupported" +) + // ConditionTelemetryConfigRefValidated indicates whether the TelemetryConfigRef is valid const ConditionTelemetryConfigRefValidated = "TelemetryConfigRefValidated" @@ -270,6 +295,12 @@ type MCPServerSpec struct { // +optional ExternalAuthConfigRef *ExternalAuthConfigRef `json:"externalAuthConfigRef,omitempty"` + // AuthServerRef optionally references a resource that configures an embedded + // OAuth 2.0/OIDC authorization server to authenticate MCP clients. + // Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + // +optional + AuthServerRef *AuthServerRef `json:"authServerRef,omitempty"` + // TelemetryConfigRef references an MCPTelemetryConfig resource for shared telemetry configuration. // The referenced MCPTelemetryConfig must exist in the same namespace as this MCPServer. // Cross-namespace references are not supported for security and isolation reasons. @@ -836,6 +867,21 @@ type ExternalAuthConfigRef struct { Name string `json:"name"` } +// AuthServerRef defines a reference to a resource that configures an embedded +// OAuth 2.0/OIDC authorization server. Currently only MCPExternalAuthConfig is supported; +// the enum will be extended when a dedicated auth server CRD is introduced. +type AuthServerRef struct { + // Kind identifies the type of the referenced resource. + // +kubebuilder:validation:Enum=MCPExternalAuthConfig + // +kubebuilder:default=MCPExternalAuthConfig + Kind string `json:"kind"` + + // Name is the name of the referenced resource in the same namespace. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + Name string `json:"name"` +} + // ToolConfigRef defines a reference to a MCPToolConfig resource. // The referenced MCPToolConfig must be in the same namespace as the MCPServer. type ToolConfigRef struct { @@ -971,6 +1017,11 @@ type MCPServerStatus struct { // +optional ExternalAuthConfigHash string `json:"externalAuthConfigHash,omitempty"` + // AuthServerConfigHash is the hash of the referenced authServerRef spec, + // used to detect configuration changes and trigger reconciliation. + // +optional + AuthServerConfigHash string `json:"authServerConfigHash,omitempty"` + // OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection // +optional OIDCConfigHash string `json:"oidcConfigHash,omitempty"` diff --git a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go index 3e68cd08a2..689d9311c1 100644 --- a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -84,6 +84,21 @@ func (in *AuditConfig) DeepCopy() *AuditConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *AuthServerRef) DeepCopyInto(out *AuthServerRef) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthServerRef. +func (in *AuthServerRef) DeepCopy() *AuthServerRef { + if in == nil { + return nil + } + out := new(AuthServerRef) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AuthServerStorageConfig) DeepCopyInto(out *AuthServerStorageConfig) { *out = *in @@ -1716,6 +1731,11 @@ func (in *MCPRemoteProxySpec) DeepCopyInto(out *MCPRemoteProxySpec) { *out = new(ExternalAuthConfigRef) **out = **in } + if in.AuthServerRef != nil { + in, out := &in.AuthServerRef, &out.AuthServerRef + *out = new(AuthServerRef) + **out = **in + } if in.HeaderForward != nil { in, out := &in.HeaderForward, &out.HeaderForward *out = new(HeaderForwardConfig) @@ -2030,6 +2050,11 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { *out = new(ExternalAuthConfigRef) **out = **in } + if in.AuthServerRef != nil { + in, out := &in.AuthServerRef, &out.AuthServerRef + *out = new(AuthServerRef) + **out = **in + } if in.TelemetryConfigRef != nil { in, out := &in.TelemetryConfigRef, &out.TelemetryConfigRef *out = new(MCPTelemetryConfigReference) diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go index 5fa509d022..f8bf938ea2 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller.go @@ -30,6 +30,10 @@ const ( // externalAuthConfigRequeueDelay is the delay before requeuing after adding a finalizer externalAuthConfigRequeueDelay = 500 * time.Millisecond + + // authServerRefKindMCPExternalAuthConfig is the Kind value on a TypedLocalObjectReference + // that identifies the ref as pointing to an MCPExternalAuthConfig resource. + authServerRefKindMCPExternalAuthConfig = "MCPExternalAuthConfig" ) // MCPExternalAuthConfigReconciler reconciles a MCPExternalAuthConfig object @@ -237,91 +241,259 @@ func (r *MCPExternalAuthConfigReconciler) handleDeletion( } // findReferencingMCPServers finds all MCPServers that reference the given MCPExternalAuthConfig +// via either externalAuthConfigRef or authServerRef. +// It queries separately for each ref field and merges with deduplication, so a server +// that has externalAuthConfigRef pointing to config "A" and authServerRef pointing to +// config "B" will be found when reconciling either config. func (r *MCPExternalAuthConfigReconciler) findReferencingMCPServers( ctx context.Context, externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig, ) ([]mcpv1alpha1.MCPServer, error) { - return ctrlutil.FindReferencingMCPServers(ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, + byExtAuth, err := ctrlutil.FindReferencingMCPServers(ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, func(server *mcpv1alpha1.MCPServer) *string { if server.Spec.ExternalAuthConfigRef != nil { return &server.Spec.ExternalAuthConfigRef.Name } return nil }) + if err != nil { + return nil, err + } + + byAuthServer, err := ctrlutil.FindReferencingMCPServers(ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, + func(server *mcpv1alpha1.MCPServer) *string { + if server.Spec.AuthServerRef != nil && server.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig { + return &server.Spec.AuthServerRef.Name + } + return nil + }) + if err != nil { + return nil, err + } + + // Merge and deduplicate + seen := make(map[string]struct{}, len(byExtAuth)) + result := make([]mcpv1alpha1.MCPServer, 0, len(byExtAuth)+len(byAuthServer)) + for _, s := range byExtAuth { + seen[s.Name] = struct{}{} + result = append(result, s) + } + for _, s := range byAuthServer { + if _, ok := seen[s.Name]; !ok { + result = append(result, s) + } + } + return result, nil } -// findReferencingWorkloads returns the workload resources (MCPServer) -// that reference this MCPExternalAuthConfig via their ExternalAuthConfigRef field. -func (r *MCPExternalAuthConfigReconciler) findReferencingWorkloads( +// findReferencingMCPRemoteProxies finds all MCPRemoteProxies that reference the given MCPExternalAuthConfig +// via either externalAuthConfigRef or authServerRef. +// It queries separately for each ref field and merges with deduplication, so a proxy +// that has externalAuthConfigRef pointing to config "A" and authServerRef pointing to +// config "B" will be found when reconciling either config. +func (r *MCPExternalAuthConfigReconciler) findReferencingMCPRemoteProxies( ctx context.Context, externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig, -) ([]mcpv1alpha1.WorkloadReference, error) { - return ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, - func(server *mcpv1alpha1.MCPServer) *string { - if server.Spec.ExternalAuthConfigRef != nil { - return &server.Spec.ExternalAuthConfigRef.Name +) ([]mcpv1alpha1.MCPRemoteProxy, error) { + byExtAuth, err := ctrlutil.FindReferencingMCPRemoteProxies( + ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, + func(proxy *mcpv1alpha1.MCPRemoteProxy) *string { + if proxy.Spec.ExternalAuthConfigRef != nil { + return &proxy.Spec.ExternalAuthConfigRef.Name + } + return nil + }) + if err != nil { + return nil, err + } + + byAuthServer, err := ctrlutil.FindReferencingMCPRemoteProxies( + ctx, r.Client, externalAuthConfig.Namespace, externalAuthConfig.Name, + func(proxy *mcpv1alpha1.MCPRemoteProxy) *string { + if proxy.Spec.AuthServerRef != nil && proxy.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig { + return &proxy.Spec.AuthServerRef.Name } return nil }) + if err != nil { + return nil, err + } + + // Merge and deduplicate + seen := make(map[string]struct{}, len(byExtAuth)) + result := make([]mcpv1alpha1.MCPRemoteProxy, 0, len(byExtAuth)+len(byAuthServer)) + for _, p := range byExtAuth { + seen[p.Name] = struct{}{} + result = append(result, p) + } + for _, p := range byAuthServer { + if _, ok := seen[p.Name]; !ok { + result = append(result, p) + } + } + return result, nil +} + +// findReferencingWorkloads returns the workload resources (MCPServer and MCPRemoteProxy) +// that reference this MCPExternalAuthConfig via their ExternalAuthConfigRef or AuthServerRef field. +// It queries separately for each ref field and merges the results, so both fields are always checked. +func (r *MCPExternalAuthConfigReconciler) findReferencingWorkloads( + ctx context.Context, + externalAuthConfig *mcpv1alpha1.MCPExternalAuthConfig, +) ([]mcpv1alpha1.WorkloadReference, error) { + servers, err := r.findReferencingMCPServers(ctx, externalAuthConfig) + if err != nil { + return nil, err + } + refs := make([]mcpv1alpha1.WorkloadReference, 0, len(servers)) + for _, server := range servers { + refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPServer, Name: server.Name}) + } + + proxies, err := r.findReferencingMCPRemoteProxies(ctx, externalAuthConfig) + if err != nil { + return nil, err + } + for _, proxy := range proxies { + refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPRemoteProxy, Name: proxy.Name}) + } + + ctrlutil.SortWorkloadRefs(refs) + return refs, nil } // SetupWithManager sets up the controller with the Manager. -// Watches MCPServer changes to maintain accurate ReferencingWorkloads status. +// Watches MCPServer and MCPRemoteProxy changes to maintain accurate ReferencingWorkloads status. func (r *MCPExternalAuthConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { - // Watch MCPServer changes to update ReferencingWorkloads on referenced MCPExternalAuthConfigs. - // This handler enqueues both the currently-referenced MCPExternalAuthConfig AND any - // MCPExternalAuthConfig that still lists this server in ReferencingWorkloads (covers the - // case where a server removes its externalAuthConfigRef — the previously-referenced - // config needs to reconcile and clean up the stale entry). - mcpServerHandler := handler.EnqueueRequestsFromMapFunc( - func(ctx context.Context, obj client.Object) []reconcile.Request { - server, ok := obj.(*mcpv1alpha1.MCPServer) - if !ok { - return nil - } + return ctrl.NewControllerManagedBy(mgr). + For(&mcpv1alpha1.MCPExternalAuthConfig{}). + Watches( + &mcpv1alpha1.MCPServer{}, + handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToExternalAuthConfig), + ). + Watches( + &mcpv1alpha1.MCPRemoteProxy{}, + handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToExternalAuthConfig), + ). + Complete(r) +} - seen := make(map[types.NamespacedName]struct{}) - var requests []reconcile.Request +// mapMCPServerToExternalAuthConfig maps MCPServer changes to MCPExternalAuthConfig reconciliation requests. +// Enqueues both the currently-referenced config(s) and any config that still lists this +// MCPServer in ReferencingWorkloads (handles ref-removal / deletion). +func (r *MCPExternalAuthConfigReconciler) mapMCPServerToExternalAuthConfig( + ctx context.Context, obj client.Object, +) []reconcile.Request { + server, ok := obj.(*mcpv1alpha1.MCPServer) + if !ok { + return nil + } - // Enqueue the currently-referenced MCPExternalAuthConfig (if any) - if server.Spec.ExternalAuthConfigRef != nil { - nn := types.NamespacedName{ - Name: server.Spec.ExternalAuthConfigRef.Name, - Namespace: server.Namespace, - } - seen[nn] = struct{}{} + seen := make(map[types.NamespacedName]struct{}) + var requests []reconcile.Request + + // Enqueue the currently-referenced MCPExternalAuthConfig (if any) + if server.Spec.ExternalAuthConfigRef != nil { + nn := types.NamespacedName{ + Name: server.Spec.ExternalAuthConfigRef.Name, + Namespace: server.Namespace, + } + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + // Enqueue the MCPExternalAuthConfig referenced via authServerRef (if any) + if server.Spec.AuthServerRef != nil && server.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig { + nn := types.NamespacedName{ + Name: server.Spec.AuthServerRef.Name, + Namespace: server.Namespace, + } + if _, already := seen[nn]; !already { + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + } + + // Also enqueue any MCPExternalAuthConfig that still lists this server in + // ReferencingWorkloads — handles ref-removal and server-deletion cases. + extAuthConfigList := &mcpv1alpha1.MCPExternalAuthConfigList{} + if err := r.List(ctx, extAuthConfigList, client.InNamespace(server.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list MCPExternalAuthConfigs for MCPServer watch") + return requests + } + for _, cfg := range extAuthConfigList.Items { + nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace} + if _, already := seen[nn]; already { + continue + } + for _, ref := range cfg.Status.ReferencingWorkloads { + if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name { requests = append(requests, reconcile.Request{NamespacedName: nn}) + break } + } + } - // Also enqueue any MCPExternalAuthConfig that still lists this server in - // ReferencingWorkloads — handles ref-removal and server-deletion cases. - extAuthConfigList := &mcpv1alpha1.MCPExternalAuthConfigList{} - if err := r.List(ctx, extAuthConfigList, client.InNamespace(server.Namespace)); err != nil { - log.FromContext(ctx).Error(err, "Failed to list MCPExternalAuthConfigs for MCPServer watch") - return requests - } - for _, cfg := range extAuthConfigList.Items { - nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace} - if _, already := seen[nn]; already { - continue - } - for _, ref := range cfg.Status.ReferencingWorkloads { - if ref.Kind == mcpv1alpha1.WorkloadKindMCPServer && ref.Name == server.Name { - requests = append(requests, reconcile.Request{NamespacedName: nn}) - break - } - } - } + return requests +} - return requests - }, - ) +// mapMCPRemoteProxyToExternalAuthConfig maps MCPRemoteProxy changes to MCPExternalAuthConfig reconciliation requests. +// Enqueues both the currently-referenced config(s) and any config that still lists this +// MCPRemoteProxy in ReferencingWorkloads (handles ref-removal / deletion). +func (r *MCPExternalAuthConfigReconciler) mapMCPRemoteProxyToExternalAuthConfig( + ctx context.Context, obj client.Object, +) []reconcile.Request { + proxy, ok := obj.(*mcpv1alpha1.MCPRemoteProxy) + if !ok { + return nil + } - return ctrl.NewControllerManagedBy(mgr). - For(&mcpv1alpha1.MCPExternalAuthConfig{}). - // Watch for MCPServers and reconcile the referenced MCPExternalAuthConfig when they change - Watches(&mcpv1alpha1.MCPServer{}, mcpServerHandler). - Complete(r) + seen := make(map[types.NamespacedName]struct{}) + var requests []reconcile.Request + + // Enqueue the currently-referenced MCPExternalAuthConfig via externalAuthConfigRef (if any) + if proxy.Spec.ExternalAuthConfigRef != nil { + nn := types.NamespacedName{ + Name: proxy.Spec.ExternalAuthConfigRef.Name, + Namespace: proxy.Namespace, + } + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + // Enqueue the MCPExternalAuthConfig referenced via authServerRef (if any) + if proxy.Spec.AuthServerRef != nil && proxy.Spec.AuthServerRef.Kind == authServerRefKindMCPExternalAuthConfig { + nn := types.NamespacedName{ + Name: proxy.Spec.AuthServerRef.Name, + Namespace: proxy.Namespace, + } + if _, already := seen[nn]; !already { + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + } + + // Also enqueue any MCPExternalAuthConfig that still lists this proxy in + // ReferencingWorkloads — handles ref-removal and proxy-deletion cases. + extAuthConfigList := &mcpv1alpha1.MCPExternalAuthConfigList{} + if err := r.List(ctx, extAuthConfigList, client.InNamespace(proxy.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list MCPExternalAuthConfigs for MCPRemoteProxy watch") + return requests + } + for _, cfg := range extAuthConfigList.Items { + nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace} + if _, already := seen[nn]; already { + continue + } + for _, ref := range cfg.Status.ReferencingWorkloads { + if ref.Kind == mcpv1alpha1.WorkloadKindMCPRemoteProxy && ref.Name == proxy.Name { + requests = append(requests, reconcile.Request{NamespacedName: nn}) + break + } + } + } + + return requests } // updateReferencingWorkloads finds referencing workloads and updates the status if the list changed diff --git a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go index 0a53d6fdaa..2f56a8dda2 100644 --- a/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpexternalauthconfig_controller_test.go @@ -870,3 +870,354 @@ func TestMCPExternalAuthConfigReconciler_ReferencingWorkloadsRemovedOnServerDele assert.Empty(t, updatedConfig.Status.ReferencingWorkloads, "ReferencingWorkloads should be empty after server deletion") } + +func TestMCPExternalAuthConfigReconciler_findReferencingWorkloads_authServerRef(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + externalAuthConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-server-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + }, + } + + // Server referencing via authServerRef + serverViaAuthServerRef := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-via-authserverref", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "auth-server-config", + }, + }, + } + + // Server referencing via externalAuthConfigRef + serverViaExtAuth := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-via-extauth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "auth-server-config", + }, + }, + } + + // Server not referencing this config at all + serverNoRef := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-no-ref", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(externalAuthConfig, serverViaAuthServerRef, serverViaExtAuth, serverNoRef). + Build() + + r := &MCPExternalAuthConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + ctx := t.Context() + refs, err := r.findReferencingWorkloads(ctx, externalAuthConfig) + require.NoError(t, err) + + assert.Len(t, refs, 2, "Should find 2 referencing workloads (one via authServerRef, one via externalAuthConfigRef)") + assert.Contains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-via-authserverref"}) + assert.Contains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-via-extauth"}) + assert.NotContains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-no-ref"}) +} + +func TestMCPExternalAuthConfigReconciler_findReferencingWorkloads_bothRefsOnSameServer(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + // A server has externalAuthConfigRef pointing to "token-exchange-config" + // AND authServerRef pointing to "embedded-auth-config". + // Both configs should discover this server during reconciliation. + + tokenExchangeConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "token-exchange-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeTokenExchange, + TokenExchange: &mcpv1alpha1.TokenExchangeConfig{ + TokenURL: "https://oauth.example.com/token", + ClientID: "test-client", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "test-secret", + Key: "client-secret", + }, + Audience: "backend-service", + }, + }, + } + + embeddedAuthConfig := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-auth-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + }, + } + + // Server with both refs pointing to different configs + serverWithBothRefs := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-with-both-refs", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "token-exchange-config", + }, + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "embedded-auth-config", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tokenExchangeConfig, embeddedAuthConfig, serverWithBothRefs). + Build() + + r := &MCPExternalAuthConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + ctx := t.Context() + + // Reconciling the token-exchange-config should find the server via externalAuthConfigRef + refsForTokenExchange, err := r.findReferencingWorkloads(ctx, tokenExchangeConfig) + require.NoError(t, err) + assert.Len(t, refsForTokenExchange, 1, "token-exchange-config should find server via externalAuthConfigRef") + assert.Contains(t, refsForTokenExchange, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-with-both-refs"}) + + // Reconciling the embedded-auth-config should find the server via authServerRef + refsForEmbedded, err := r.findReferencingWorkloads(ctx, embeddedAuthConfig) + require.NoError(t, err) + assert.Len(t, refsForEmbedded, 1, "embedded-auth-config should find server via authServerRef") + assert.Contains(t, refsForEmbedded, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-with-both-refs"}) + + // Also verify findReferencingMCPServers returns the server for both configs + serversForTokenExchange, err := r.findReferencingMCPServers(ctx, tokenExchangeConfig) + require.NoError(t, err) + assert.Len(t, serversForTokenExchange, 1) + assert.Equal(t, "server-with-both-refs", serversForTokenExchange[0].Name) + + serversForEmbedded, err := r.findReferencingMCPServers(ctx, embeddedAuthConfig) + require.NoError(t, err) + assert.Len(t, serversForEmbedded, 1) + assert.Equal(t, "server-with-both-refs", serversForEmbedded[0].Name) +} + +func TestMCPExternalAuthConfigReconciler_findReferencingMCPServers_deduplicates(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + // A server has both externalAuthConfigRef and authServerRef pointing to the SAME config. + // The server should appear only once in the results. + config := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "shared-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeTokenExchange, + TokenExchange: &mcpv1alpha1.TokenExchangeConfig{ + TokenURL: "https://oauth.example.com/token", + ClientID: "test-client", + ClientSecretRef: &mcpv1alpha1.SecretKeyRef{ + Name: "test-secret", + Key: "client-secret", + }, + Audience: "backend-service", + }, + }, + } + + server := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-both-same", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "shared-config", + }, + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "shared-config", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(config, server). + Build() + + r := &MCPExternalAuthConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + ctx := t.Context() + servers, err := r.findReferencingMCPServers(ctx, config) + require.NoError(t, err) + assert.Len(t, servers, 1, "Server should appear only once even when both refs point to the same config") + assert.Equal(t, "server-both-same", servers[0].Name) +} + +func TestMCPExternalAuthConfigReconciler_findReferencingWorkloads_mcpRemoteProxy(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + config := &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + }, + } + + // MCPRemoteProxy referencing via externalAuthConfigRef + proxyViaExtAuth := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-via-extauth", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "auth-config", + }, + }, + } + + // MCPRemoteProxy referencing via authServerRef + proxyViaAuthServerRef := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-via-authserverref", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "auth-config", + }, + }, + } + + // MCPRemoteProxy not referencing this config + proxyNoRef := &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "proxy-no-ref", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + }, + } + + // MCPServer also referencing the same config + server := &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "server-ref", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test-image", + ExternalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "auth-config", + }, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(config, proxyViaExtAuth, proxyViaAuthServerRef, proxyNoRef, server). + Build() + + r := &MCPExternalAuthConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + ctx := t.Context() + refs, err := r.findReferencingWorkloads(ctx, config) + require.NoError(t, err) + + assert.Len(t, refs, 3, "Should find 3 referencing workloads (1 MCPServer + 2 MCPRemoteProxies)") + assert.Contains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPServer", Name: "server-ref"}) + assert.Contains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPRemoteProxy", Name: "proxy-via-extauth"}) + assert.Contains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPRemoteProxy", Name: "proxy-via-authserverref"}) + assert.NotContains(t, refs, mcpv1alpha1.WorkloadReference{Kind: "MCPRemoteProxy", Name: "proxy-no-ref"}) +} diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_authserverref_test.go b/cmd/thv-operator/controllers/mcpremoteproxy_authserverref_test.go new file mode 100644 index 0000000000..eccf8caf1c --- /dev/null +++ b/cmd/thv-operator/controllers/mcpremoteproxy_authserverref_test.go @@ -0,0 +1,217 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" +) + +func TestMCPRemoteProxyReconciler_handleAuthServerRef(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + proxy func() *mcpv1alpha1.MCPRemoteProxy + authConfig func() *mcpv1alpha1.MCPExternalAuthConfig + expectError bool + errContains string + expectHash string + conditionStatus metav1.ConditionStatus + conditionReason string + }{ + { + name: "nil authServerRef removes condition and clears hash", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{RemoteURL: "https://remote.example.com"}, + Status: mcpv1alpha1.MCPRemoteProxyStatus{ + AuthServerConfigHash: "old-hash", + }, + } + }, + expectHash: "", + }, + { + name: "unsupported kind sets InvalidKind condition", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "Secret", Name: "foo"}, + }, + } + }, + expectError: true, + errContains: "unsupported authServerRef kind", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefInvalidKind, + }, + { + name: "not found sets NotFound condition", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "missing"}, + }, + } + }, + expectError: true, + errContains: "not found", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefNotFound, + }, + { + name: "wrong type sets InvalidType condition", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "sts-config"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "sts-config", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeAWSSts, + AWSSts: &mcpv1alpha1.AWSStsConfig{ + Region: "us-east-1", + }, + }, + } + }, + expectError: true, + errContains: "only embeddedAuthServer is supported", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefInvalidType, + }, + { + name: "multi-upstream sets MultiUpstream condition", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "multi"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "multi", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + {Name: "a", Type: mcpv1alpha1.UpstreamProviderTypeOIDC, OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{IssuerURL: "https://a.com", ClientID: "a"}}, + {Name: "b", Type: mcpv1alpha1.UpstreamProviderTypeOIDC, OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{IssuerURL: "https://b.com", ClientID: "b"}}, + }, + }, + }, + Status: mcpv1alpha1.MCPExternalAuthConfigStatus{ConfigHash: "multi-hash"}, + } + }, + expectError: true, + errContains: "only 1 is supported", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefMultiUpstream, + }, + { + name: "valid ref sets Valid condition and updates hash", + proxy: func() *mcpv1alpha1.MCPRemoteProxy { + return &mcpv1alpha1.MCPRemoteProxy{ + ObjectMeta: metav1.ObjectMeta{Name: "proxy", Namespace: "default"}, + Spec: mcpv1alpha1.MCPRemoteProxySpec{ + RemoteURL: "https://remote.example.com", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "valid"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "valid", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{{Name: "key", Key: "pem"}}, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{{Name: "hmac", Key: "secret"}}, + }, + }, + Status: mcpv1alpha1.MCPExternalAuthConfigStatus{ConfigHash: "valid-hash"}, + } + }, + expectHash: "valid-hash", + conditionStatus: metav1.ConditionTrue, + conditionReason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefValid, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + proxy := tt.proxy() + objs := []runtime.Object{proxy} + if tt.authConfig != nil { + objs = append(objs, tt.authConfig()) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objs...). + WithStatusSubresource(&mcpv1alpha1.MCPRemoteProxy{}). + Build() + + reconciler := &MCPRemoteProxyReconciler{ + Client: fakeClient, + Scheme: scheme, + } + err := reconciler.handleAuthServerRef(ctx, proxy) + + if tt.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectHash, proxy.Status.AuthServerConfigHash) + } + + cond := meta.FindStatusCondition(proxy.Status.Conditions, mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated) + if tt.conditionStatus != "" { + require.NotNil(t, cond, "AuthServerRefValidated condition should be present") + assert.Equal(t, tt.conditionStatus, cond.Status) + assert.Equal(t, tt.conditionReason, cond.Reason) + } else { + assert.Nil(t, cond, "AuthServerRefValidated condition should be removed") + } + }) + } +} diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index fd6807329e..95d6146404 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -138,6 +138,16 @@ func (r *MCPRemoteProxyReconciler) validateAndHandleConfigs(ctx context.Context, return err } + // Handle authServerRef config hash tracking + if err := r.handleAuthServerRef(ctx, proxy); err != nil { + ctxLogger.Error(err, "Failed to handle authServerRef") + proxy.Status.Phase = mcpv1alpha1.MCPRemoteProxyPhaseFailed + if statusErr := r.Status().Update(ctx, proxy); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update MCPRemoteProxy status after authServerRef error") + } + return err + } + // Handle MCPOIDCConfig if err := r.handleOIDCConfig(ctx, proxy); err != nil { ctxLogger.Error(err, "Failed to handle MCPOIDCConfig") @@ -712,6 +722,117 @@ func (r *MCPRemoteProxyReconciler) handleExternalAuthConfig(ctx context.Context, return nil } +// handleAuthServerRef validates and tracks the hash of the referenced authServerRef config. +// It updates the MCPRemoteProxy status when the auth server configuration changes and sets +// the AuthServerRefValidated condition. +func (r *MCPRemoteProxyReconciler) handleAuthServerRef(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error { + ctxLogger := log.FromContext(ctx) + if proxy.Spec.AuthServerRef == nil { + meta.RemoveStatusCondition(&proxy.Status.Conditions, mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated) + if proxy.Status.AuthServerConfigHash != "" { + proxy.Status.AuthServerConfigHash = "" + if err := r.Status().Update(ctx, proxy); err != nil { + return fmt.Errorf("failed to clear authServerRef hash from status: %w", err) + } + } + return nil + } + + // Only MCPExternalAuthConfig kind is supported + if proxy.Spec.AuthServerRef.Kind != "MCPExternalAuthConfig" { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefInvalidKind, + Message: fmt.Sprintf("unsupported authServerRef kind %q: only MCPExternalAuthConfig is supported", + proxy.Spec.AuthServerRef.Kind), + ObservedGeneration: proxy.Generation, + }) + return fmt.Errorf("unsupported authServerRef kind %q: only MCPExternalAuthConfig is supported", + proxy.Spec.AuthServerRef.Kind) + } + + // Fetch the referenced MCPExternalAuthConfig + authConfig, err := ctrlutil.GetExternalAuthConfigByName(ctx, r.Client, proxy.Namespace, proxy.Spec.AuthServerRef.Name) + if err != nil { + if errors.IsNotFound(err) { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefNotFound, + Message: fmt.Sprintf("MCPExternalAuthConfig '%s' not found in namespace '%s'", + proxy.Spec.AuthServerRef.Name, proxy.Namespace), + ObservedGeneration: proxy.Generation, + }) + return fmt.Errorf("MCPExternalAuthConfig '%s' not found in namespace '%s'", + proxy.Spec.AuthServerRef.Name, proxy.Namespace) + } + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefFetchError, + Message: fmt.Sprintf("Failed to fetch MCPExternalAuthConfig '%s'", proxy.Spec.AuthServerRef.Name), + ObservedGeneration: proxy.Generation, + }) + return fmt.Errorf("failed to get authServerRef MCPExternalAuthConfig %s: %w", proxy.Spec.AuthServerRef.Name, err) + } + + // Validate the config type is embeddedAuthServer + if authConfig.Spec.Type != mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefInvalidType, + Message: fmt.Sprintf("authServerRef '%s' has type %q, but only embeddedAuthServer is supported", + proxy.Spec.AuthServerRef.Name, authConfig.Spec.Type), + ObservedGeneration: proxy.Generation, + }) + return fmt.Errorf("authServerRef '%s' has type %q, but only embeddedAuthServer is supported", + proxy.Spec.AuthServerRef.Name, authConfig.Spec.Type) + } + + // MCPRemoteProxy supports only single-upstream embedded auth server configs + if embeddedCfg := authConfig.Spec.EmbeddedAuthServer; embeddedCfg != nil && len(embeddedCfg.UpstreamProviders) > 1 { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefMultiUpstream, + Message: fmt.Sprintf("MCPRemoteProxy supports only one upstream provider (found %d); "+ + "use VirtualMCPServer for multi-upstream", + len(embeddedCfg.UpstreamProviders)), + ObservedGeneration: proxy.Generation, + }) + return fmt.Errorf("MCPRemoteProxy %s/%s: embedded auth server has %d upstream providers, "+ + "but only 1 is supported; use VirtualMCPServer", + proxy.Namespace, proxy.Name, len(embeddedCfg.UpstreamProviders)) + } + + // AuthServerRef valid + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeMCPRemoteProxyAuthServerRefValidated, + Status: metav1.ConditionTrue, + Reason: mcpv1alpha1.ConditionReasonMCPRemoteProxyAuthServerRefValid, + Message: fmt.Sprintf("AuthServerRef '%s' is valid", authConfig.Name), + ObservedGeneration: proxy.Generation, + }) + + // Check if the config hash has changed + if proxy.Status.AuthServerConfigHash != authConfig.Status.ConfigHash { + ctxLogger.Info("authServerRef config has changed, updating MCPRemoteProxy", + "proxy", proxy.Name, + "authServerRef", authConfig.Name, + "oldHash", proxy.Status.AuthServerConfigHash, + "newHash", authConfig.Status.ConfigHash) + + proxy.Status.AuthServerConfigHash = authConfig.Status.ConfigHash + if err := r.Status().Update(ctx, proxy); err != nil { + return fmt.Errorf("failed to update authServerRef hash in status: %w", err) + } + } + + return nil +} + // handleOIDCConfig validates and tracks the hash of the referenced MCPOIDCConfig. // It updates the MCPRemoteProxy status when the OIDC configuration changes and sets // the OIDCConfigRefValidated condition. @@ -1289,8 +1410,10 @@ func (r *MCPRemoteProxyReconciler) SetupWithManager(mgr ctrl.Manager) error { // Find MCPRemoteProxies that reference this MCPExternalAuthConfig var requests []reconcile.Request for _, proxy := range proxyList.Items { - if proxy.Spec.ExternalAuthConfigRef != nil && - proxy.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name { + if (proxy.Spec.ExternalAuthConfigRef != nil && + proxy.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name) || + (proxy.Spec.AuthServerRef != nil && + proxy.Spec.AuthServerRef.Name == externalAuthConfig.Name) { requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Name: proxy.Name, diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go b/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go index 08dd738559..33749b298f 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_deployment.go @@ -32,14 +32,15 @@ func (r *MCPRemoteProxyReconciler) deploymentForMCPRemoteProxy( volumeMounts, volumes := r.buildVolumesForProxy(proxy) env := r.buildEnvVarsForProxy(ctx, proxy) - // Add embedded auth server volumes and env vars if configured (single call for efficiency) - if proxy.Spec.ExternalAuthConfigRef != nil { - authServerVolumes, authServerMounts, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfig( - ctx, r.Client, proxy.Namespace, proxy.Spec.ExternalAuthConfigRef, + // Add embedded auth server volumes and env vars. AuthServerRef takes precedence; + // externalAuthConfigRef is used as a fallback (legacy path). + configName := ctrlutil.EmbeddedAuthServerConfigName(proxy.Spec.ExternalAuthConfigRef, proxy.Spec.AuthServerRef) + if configName != "" { + authServerVolumes, authServerMounts, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfigByName( + ctx, r.Client, proxy.Namespace, configName, ) if err != nil { - ctxLogger := log.FromContext(ctx) - ctxLogger.Error(err, "Failed to generate embedded auth server configuration") + log.FromContext(ctx).Error(err, "Failed to generate auth server configuration") return nil } volumes = append(volumes, authServerVolumes...) diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go index 6a22e9b669..21655b24f9 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go @@ -156,6 +156,14 @@ func (r *MCPRemoteProxyReconciler) createRunConfigFromMCPRemoteProxy( return nil, fmt.Errorf("failed to process ExternalAuthConfig: %w", err) } + // Validate authServerRef/externalAuthConfigRef conflict and add authServerRef options + if err := ctrlutil.ValidateAndAddAuthServerRefOptions( + ctx, r.Client, proxy.Namespace, proxy.Name, proxy.Spec.AuthServerRef, + proxy.Spec.ExternalAuthConfigRef, resolvedOIDCConfig, &options, + ); err != nil { + return nil, fmt.Errorf("failed to process authServerRef: %w", err) + } + // Add audit configuration if specified runconfig.AddAuditConfigOptions(&options, proxy.Spec.Audit) diff --git a/cmd/thv-operator/controllers/mcpserver_authserverref_test.go b/cmd/thv-operator/controllers/mcpserver_authserverref_test.go new file mode 100644 index 0000000000..6bf883f638 --- /dev/null +++ b/cmd/thv-operator/controllers/mcpserver_authserverref_test.go @@ -0,0 +1,215 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" + "github.com/stacklok/toolhive/pkg/container/kubernetes" +) + +func TestMCPServerReconciler_handleAuthServerRef(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mcpServer func() *mcpv1alpha1.MCPServer + authConfig func() *mcpv1alpha1.MCPExternalAuthConfig + expectError bool + errContains string + expectHash string + conditionStatus metav1.ConditionStatus + conditionReason string + }{ + { + name: "nil authServerRef removes condition and clears hash", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{Image: "test"}, + Status: mcpv1alpha1.MCPServerStatus{ + AuthServerConfigHash: "old-hash", + }, + } + }, + expectHash: "", + }, + { + name: "unsupported kind sets InvalidKind condition", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "Secret", Name: "foo"}, + }, + } + }, + expectError: true, + errContains: "unsupported authServerRef kind", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonAuthServerRefInvalidKind, + }, + { + name: "not found sets NotFound condition", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "missing"}, + }, + } + }, + expectError: true, + errContains: "not found", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonAuthServerRefNotFound, + }, + { + name: "wrong type sets InvalidType condition", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "sts-config"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "sts-config", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeAWSSts, + AWSSts: &mcpv1alpha1.AWSStsConfig{ + Region: "us-east-1", + }, + }, + } + }, + expectError: true, + errContains: "only embeddedAuthServer is supported", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonAuthServerRefInvalidType, + }, + { + name: "multi-upstream sets MultiUpstream condition", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "multi"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "multi", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + UpstreamProviders: []mcpv1alpha1.UpstreamProviderConfig{ + {Name: "a", Type: mcpv1alpha1.UpstreamProviderTypeOIDC, OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{IssuerURL: "https://a.com", ClientID: "a"}}, + {Name: "b", Type: mcpv1alpha1.UpstreamProviderTypeOIDC, OIDCConfig: &mcpv1alpha1.OIDCUpstreamConfig{IssuerURL: "https://b.com", ClientID: "b"}}, + }, + }, + }, + Status: mcpv1alpha1.MCPExternalAuthConfigStatus{ConfigHash: "multi-hash"}, + } + }, + expectError: true, + errContains: "only 1 is supported", + conditionStatus: metav1.ConditionFalse, + conditionReason: mcpv1alpha1.ConditionReasonAuthServerRefMultiUpstream, + }, + { + name: "valid ref sets Valid condition and updates hash", + mcpServer: func() *mcpv1alpha1.MCPServer { + return &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "server", Namespace: "default"}, + Spec: mcpv1alpha1.MCPServerSpec{ + Image: "test", + AuthServerRef: &mcpv1alpha1.AuthServerRef{Kind: "MCPExternalAuthConfig", Name: "valid"}, + }, + } + }, + authConfig: func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "valid", Namespace: "default"}, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{{Name: "key", Key: "pem"}}, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{{Name: "hmac", Key: "secret"}}, + }, + }, + Status: mcpv1alpha1.MCPExternalAuthConfigStatus{ConfigHash: "valid-hash"}, + } + }, + expectHash: "valid-hash", + conditionStatus: metav1.ConditionTrue, + conditionReason: mcpv1alpha1.ConditionReasonAuthServerRefValid, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + server := tt.mcpServer() + objs := []runtime.Object{server} + if tt.authConfig != nil { + objs = append(objs, tt.authConfig()) + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objs...). + WithStatusSubresource(&mcpv1alpha1.MCPServer{}). + Build() + + reconciler := newTestMCPServerReconciler(fakeClient, scheme, kubernetes.PlatformKubernetes) + err := reconciler.handleAuthServerRef(ctx, server) + + if tt.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + assert.Equal(t, tt.expectHash, server.Status.AuthServerConfigHash) + } + + cond := meta.FindStatusCondition(server.Status.Conditions, mcpv1alpha1.ConditionTypeAuthServerRefValidated) + if tt.conditionStatus != "" { + require.NotNil(t, cond, "AuthServerRefValidated condition should be present") + assert.Equal(t, tt.conditionStatus, cond.Status) + assert.Equal(t, tt.conditionReason, cond.Reason) + } else { + assert.Nil(t, cond, "AuthServerRefValidated condition should be removed") + } + }) + } +} diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 036b9f2bf1..2fa91a60ca 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -269,6 +269,17 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } + // Check if authServerRef is referenced and handle config hash tracking + if err := r.handleAuthServerRef(ctx, mcpServer); err != nil { + ctxLogger.Error(err, "Failed to handle authServerRef") + mcpServer.Status.Phase = mcpv1alpha1.MCPServerPhaseFailed + setReadyCondition(mcpServer, metav1.ConditionFalse, mcpv1alpha1.ConditionReasonNotReady, err.Error()) + if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update MCPServer status after authServerRef error") + } + return ctrl.Result{}, err + } + // Check if MCPOIDCConfig is referenced and handle it if err := r.handleOIDCConfig(ctx, mcpServer); err != nil { ctxLogger.Error(err, "Failed to handle MCPOIDCConfig") @@ -1219,14 +1230,14 @@ func (r *MCPServerReconciler) deploymentForMCPServer( } } - // Add embedded auth server volumes and env vars if configured - if m.Spec.ExternalAuthConfigRef != nil { - authServerVolumes, authServerMounts, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfig( - ctx, r.Client, m.Namespace, m.Spec.ExternalAuthConfigRef, + // Add embedded auth server volumes and env vars. AuthServerRef takes precedence; + // externalAuthConfigRef is used as a fallback (legacy path). + if configName := ctrlutil.EmbeddedAuthServerConfigName(m.Spec.ExternalAuthConfigRef, m.Spec.AuthServerRef); configName != "" { + authServerVolumes, authServerMounts, authServerEnvVars, err := ctrlutil.GenerateAuthServerConfigByName( + ctx, r.Client, m.Namespace, configName, ) if err != nil { - ctxLogger := log.FromContext(ctx) - ctxLogger.Error(err, "Failed to generate embedded auth server configuration") + log.FromContext(ctx).Error(err, "Failed to generate auth server configuration") return nil } volumes = append(volumes, authServerVolumes...) @@ -2050,6 +2061,116 @@ func (r *MCPServerReconciler) handleExternalAuthConfig(ctx context.Context, m *m return nil } +// handleAuthServerRef validates and tracks the hash of the referenced authServerRef config. +// It updates the MCPServer status when the auth server configuration changes and sets +// the AuthServerRefValidated condition. +func (r *MCPServerReconciler) handleAuthServerRef(ctx context.Context, m *mcpv1alpha1.MCPServer) error { + ctxLogger := log.FromContext(ctx) + if m.Spec.AuthServerRef == nil { + meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1alpha1.ConditionTypeAuthServerRefValidated) + if m.Status.AuthServerConfigHash != "" { + m.Status.AuthServerConfigHash = "" + if err := r.Status().Update(ctx, m); err != nil { + return fmt.Errorf("failed to clear authServerRef hash from status: %w", err) + } + } + return nil + } + + // Only MCPExternalAuthConfig kind is supported + if m.Spec.AuthServerRef.Kind != "MCPExternalAuthConfig" { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefInvalidKind, + Message: fmt.Sprintf("unsupported authServerRef kind %q: only MCPExternalAuthConfig is supported", + m.Spec.AuthServerRef.Kind), + ObservedGeneration: m.Generation, + }) + return fmt.Errorf("unsupported authServerRef kind %q: only MCPExternalAuthConfig is supported", m.Spec.AuthServerRef.Kind) + } + + // Fetch the referenced MCPExternalAuthConfig + authConfig, err := ctrlutil.GetExternalAuthConfigByName(ctx, r.Client, m.Namespace, m.Spec.AuthServerRef.Name) + if err != nil { + if errors.IsNotFound(err) { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefNotFound, + Message: fmt.Sprintf("MCPExternalAuthConfig '%s' not found in namespace '%s'", + m.Spec.AuthServerRef.Name, m.Namespace), + ObservedGeneration: m.Generation, + }) + return fmt.Errorf("MCPExternalAuthConfig '%s' not found in namespace '%s'", + m.Spec.AuthServerRef.Name, m.Namespace) + } + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefFetchError, + Message: fmt.Sprintf("Failed to fetch MCPExternalAuthConfig '%s'", m.Spec.AuthServerRef.Name), + ObservedGeneration: m.Generation, + }) + return fmt.Errorf("failed to get authServerRef MCPExternalAuthConfig %s: %w", m.Spec.AuthServerRef.Name, err) + } + + // Validate the config type is embeddedAuthServer + if authConfig.Spec.Type != mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefInvalidType, + Message: fmt.Sprintf("authServerRef '%s' has type %q, but only embeddedAuthServer is supported", + m.Spec.AuthServerRef.Name, authConfig.Spec.Type), + ObservedGeneration: m.Generation, + }) + return fmt.Errorf("authServerRef '%s' has type %q, but only embeddedAuthServer is supported", + m.Spec.AuthServerRef.Name, authConfig.Spec.Type) + } + + // MCPServer supports only single-upstream embedded auth server configs + if embeddedCfg := authConfig.Spec.EmbeddedAuthServer; embeddedCfg != nil && len(embeddedCfg.UpstreamProviders) > 1 { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefMultiUpstream, + Message: fmt.Sprintf("MCPServer supports only one upstream provider (found %d); "+ + "use VirtualMCPServer for multi-upstream", + len(embeddedCfg.UpstreamProviders)), + ObservedGeneration: m.Generation, + }) + return fmt.Errorf("MCPServer %s/%s: embedded auth server has %d upstream providers, "+ + "but only 1 is supported; use VirtualMCPServer", + m.Namespace, m.Name, len(embeddedCfg.UpstreamProviders)) + } + + // AuthServerRef valid + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthServerRefValidated, + Status: metav1.ConditionTrue, + Reason: mcpv1alpha1.ConditionReasonAuthServerRefValid, + Message: fmt.Sprintf("AuthServerRef '%s' is valid", authConfig.Name), + ObservedGeneration: m.Generation, + }) + + // Check if the config hash has changed + if m.Status.AuthServerConfigHash != authConfig.Status.ConfigHash { + ctxLogger.Info("authServerRef config has changed, updating MCPServer", + "mcpserver", m.Name, + "authServerRef", authConfig.Name, + "oldHash", m.Status.AuthServerConfigHash, + "newHash", authConfig.Status.ConfigHash) + + m.Status.AuthServerConfigHash = authConfig.Status.ConfigHash + if err := r.Status().Update(ctx, m); err != nil { + return fmt.Errorf("failed to update authServerRef hash in status: %w", err) + } + } + + return nil +} + // handleOIDCConfig validates and tracks the hash of the referenced MCPOIDCConfig. // It updates the MCPServer status when the OIDC configuration changes and sets // the OIDCConfigRefValidated condition. @@ -2306,8 +2427,10 @@ func (r *MCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { // Find MCPServers that reference this MCPExternalAuthConfig var requests []reconcile.Request for _, server := range mcpServerList.Items { - if server.Spec.ExternalAuthConfigRef != nil && - server.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name { + if (server.Spec.ExternalAuthConfigRef != nil && + server.Spec.ExternalAuthConfigRef.Name == externalAuthConfig.Name) || + (server.Spec.AuthServerRef != nil && + server.Spec.AuthServerRef.Name == externalAuthConfig.Name) { requests = append(requests, reconcile.Request{ NamespacedName: types.NamespacedName{ Name: server.Name, diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig.go b/cmd/thv-operator/controllers/mcpserver_runconfig.go index 03fd6dc8d3..25767c26c0 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig.go @@ -261,6 +261,14 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPSer return nil, fmt.Errorf("failed to process ExternalAuthConfig: %w", err) } + // Validate authServerRef/externalAuthConfigRef conflict and add authServerRef options + if err := ctrlutil.ValidateAndAddAuthServerRefOptions( + ctx, r.Client, m.Namespace, m.Name, m.Spec.AuthServerRef, + m.Spec.ExternalAuthConfigRef, resolvedOIDCConfig, &options, + ); err != nil { + return nil, fmt.Errorf("failed to process authServerRef: %w", err) + } + // Add audit configuration if specified runconfig.AddAuditConfigOptions(&options, m.Spec.Audit) diff --git a/cmd/thv-operator/pkg/controllerutil/authserver.go b/cmd/thv-operator/pkg/controllerutil/authserver.go index 0e39c66b58..e3057082b4 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver.go @@ -9,6 +9,7 @@ import ( "strings" corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" k8sptr "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -89,30 +90,40 @@ func buildUpstreamSecretBindings( return bindings } -// GenerateAuthServerConfig generates volumes, volume mounts, and environment variables -// for the embedded auth server if the external auth config is of type embeddedAuthServer. -// -// This is a convenience function that combines GenerateAuthServerVolumes and GenerateAuthServerEnvVars, -// with the added logic to fetch and check the MCPExternalAuthConfig type. -// -// Returns empty slices if externalAuthConfigRef is nil or if the auth type is not embeddedAuthServer. -func GenerateAuthServerConfig( +// EmbeddedAuthServerConfigName returns the config name that should be used for +// embedded auth server volume/env generation, or empty string if neither ref applies. +// AuthServerRef takes precedence; externalAuthConfigRef is used as a fallback. +func EmbeddedAuthServerConfigName( + extAuthRef *mcpv1alpha1.ExternalAuthConfigRef, + authServerRef *mcpv1alpha1.AuthServerRef, +) string { + if authServerRef != nil { + return authServerRef.Name + } + if extAuthRef != nil { + return extAuthRef.Name + } + return "" +} + +// GenerateAuthServerConfigByName fetches an MCPExternalAuthConfig by name and, if its type +// is embeddedAuthServer, returns the corresponding volumes, volume mounts, and env vars. +// Returns empty slices (no error) if the config type is not embeddedAuthServer, because +// this function may be called via the externalAuthConfigRef fallback path where non-embedded +// types (headerInjection, tokenExchange, etc.) are valid — they simply don't need auth +// server volumes. Type validation for the authServerRef path is handled earlier by +// handleAuthServerRef which sets an InvalidType condition. +func GenerateAuthServerConfigByName( ctx context.Context, c client.Client, namespace string, - externalAuthConfigRef *mcpv1alpha1.ExternalAuthConfigRef, + configName string, ) ([]corev1.Volume, []corev1.VolumeMount, []corev1.EnvVar, error) { - if externalAuthConfigRef == nil { - return nil, nil, nil, nil - } - - // Fetch the MCPExternalAuthConfig - externalAuthConfig, err := GetExternalAuthConfigByName(ctx, c, namespace, externalAuthConfigRef.Name) + externalAuthConfig, err := GetExternalAuthConfigByName(ctx, c, namespace, configName) if err != nil { return nil, nil, nil, fmt.Errorf("failed to get MCPExternalAuthConfig: %w", err) } - // Only process embeddedAuthServer type if externalAuthConfig.Spec.Type != mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer { return nil, nil, nil, nil } @@ -122,10 +133,7 @@ func GenerateAuthServerConfig( return nil, nil, nil, fmt.Errorf("embedded auth server configuration is nil for type embeddedAuthServer") } - // Generate volumes and mounts volumes, volumeMounts := GenerateAuthServerVolumes(authServerConfig) - - // Generate environment variables envVars := GenerateAuthServerEnvVars(authServerConfig) return volumes, volumeMounts, envVars, nil @@ -672,3 +680,102 @@ func buildUserInfoRunConfig( return config } + +// ValidateAndAddAuthServerRefOptions performs conflict validation between authServerRef +// and externalAuthConfigRef, then resolves authServerRef if present. +// Returns error if both fields point to an embedded auth server configuration. +func ValidateAndAddAuthServerRefOptions( + ctx context.Context, + c client.Client, + namespace string, + mcpServerName string, + authServerRef *mcpv1alpha1.AuthServerRef, + externalAuthConfigRef *mcpv1alpha1.ExternalAuthConfigRef, + oidcConfig *oidc.OIDCConfig, + options *[]runner.RunConfigBuilderOption, +) error { + // Conflict validation: both authServerRef and externalAuthConfigRef pointing to + // embedded auth server is an error (use one or the other, not both) + if authServerRef != nil && externalAuthConfigRef != nil { + extConfig, err := GetExternalAuthConfigByName(ctx, c, namespace, externalAuthConfigRef.Name) + if err != nil { + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to fetch externalAuthConfigRef for conflict validation: %w", err) + } + // Not found - skip conflict check, will be caught by AddExternalAuthConfigOptions + } else if extConfig.Spec.Type == mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer { + return fmt.Errorf( + "conflict: both authServerRef and externalAuthConfigRef reference an embedded auth server; " + + "use authServerRef for the embedded auth server and externalAuthConfigRef for outgoing auth only", + ) + } + } + + // Add auth server ref configuration if specified + return AddAuthServerRefOptions(ctx, c, namespace, mcpServerName, authServerRef, oidcConfig, options) +} + +// AddAuthServerRefOptions resolves an authServerRef (TypedLocalObjectReference), +// validates the kind and type, and appends the corresponding RunConfigBuilderOption. +// Returns nil if authServerRef is nil (no-op). +// Returns error if the kind is not MCPExternalAuthConfig, the type is not embeddedAuthServer, +// or if fetching or building the config fails. +func AddAuthServerRefOptions( + ctx context.Context, + c client.Client, + namespace string, + mcpServerName string, + authServerRef *mcpv1alpha1.AuthServerRef, + oidcConfig *oidc.OIDCConfig, + options *[]runner.RunConfigBuilderOption, +) error { + if authServerRef == nil { + return nil + } + + // Validate the Kind + if authServerRef.Kind != "MCPExternalAuthConfig" { + return fmt.Errorf("unsupported authServerRef kind %q: only MCPExternalAuthConfig is supported", authServerRef.Kind) + } + + // Fetch the MCPExternalAuthConfig + externalAuthConfig, err := GetExternalAuthConfigByName(ctx, c, namespace, authServerRef.Name) + if err != nil { + return fmt.Errorf("failed to get MCPExternalAuthConfig for authServerRef: %w", err) + } + + // Validate the type is embeddedAuthServer + if externalAuthConfig.Spec.Type != mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer { + return fmt.Errorf( + "authServerRef must reference a MCPExternalAuthConfig with type %q, got %q", + mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, externalAuthConfig.Spec.Type, + ) + } + + authServerConfig := externalAuthConfig.Spec.EmbeddedAuthServer + if authServerConfig == nil { + return fmt.Errorf("embedded auth server configuration is nil for type embeddedAuthServer") + } + + // Validate OIDC config is provided with ResourceURL (required for embedded auth server) + if oidcConfig == nil { + return fmt.Errorf("OIDC config is required for embedded auth server: OIDCConfigRef must be set on the MCPServer") + } + if oidcConfig.ResourceURL == "" { + return fmt.Errorf("OIDC config resourceUrl is required for embedded auth server: set resourceUrl in OIDCConfigRef") + } + + // Build the embedded auth server config for runner + embeddedConfig, err := BuildAuthServerRunConfig( + namespace, mcpServerName, authServerConfig, + []string{oidcConfig.ResourceURL}, oidcConfig.Scopes, + ) + if err != nil { + return fmt.Errorf("failed to build embedded auth server config: %w", err) + } + + // Add the configuration option + *options = append(*options, runner.WithEmbeddedAuthServerConfig(embeddedConfig)) + + return nil +} diff --git a/cmd/thv-operator/pkg/controllerutil/authserver_test.go b/cmd/thv-operator/pkg/controllerutil/authserver_test.go index 3814fdfd7d..07d0b53616 100644 --- a/cmd/thv-operator/pkg/controllerutil/authserver_test.go +++ b/cmd/thv-operator/pkg/controllerutil/authserver_test.go @@ -5,6 +5,7 @@ package controllerutil import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -12,7 +13,9 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" "github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc" @@ -520,7 +523,7 @@ func TestGenerateAuthServerEnvVars(t *testing.T) { } } -func TestGenerateAuthServerConfig(t *testing.T) { +func TestGenerateAuthServerConfigByName(t *testing.T) { t.Parallel() scheme := runtime.NewScheme() @@ -529,7 +532,7 @@ func TestGenerateAuthServerConfig(t *testing.T) { tests := []struct { name string - externalAuthRef *mcpv1alpha1.ExternalAuthConfigRef + configName string externalAuthCfg *mcpv1alpha1.MCPExternalAuthConfig wantVolumes bool wantMounts bool @@ -538,18 +541,8 @@ func TestGenerateAuthServerConfig(t *testing.T) { errContains string }{ { - name: "nil external auth ref returns empty slices", - externalAuthRef: nil, - wantVolumes: false, - wantMounts: false, - wantEnvVars: false, - wantErr: false, - }, - { - name: "non-embeddedAuthServer type returns empty slices", - externalAuthRef: &mcpv1alpha1.ExternalAuthConfigRef{ - Name: "token-exchange-config", - }, + name: "non-embeddedAuthServer type returns empty slices", + configName: "token-exchange-config", externalAuthCfg: &mcpv1alpha1.MCPExternalAuthConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "token-exchange-config", @@ -569,10 +562,8 @@ func TestGenerateAuthServerConfig(t *testing.T) { wantErr: false, }, { - name: "embeddedAuthServer type with valid config", - externalAuthRef: &mcpv1alpha1.ExternalAuthConfigRef{ - Name: "embedded-auth-config", - }, + name: "embeddedAuthServer type with valid config", + configName: "embedded-auth-config", externalAuthCfg: &mcpv1alpha1.MCPExternalAuthConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "embedded-auth-config", @@ -612,10 +603,8 @@ func TestGenerateAuthServerConfig(t *testing.T) { wantErr: false, }, { - name: "embeddedAuthServer type with nil embedded config", - externalAuthRef: &mcpv1alpha1.ExternalAuthConfigRef{ - Name: "bad-auth-config", - }, + name: "embeddedAuthServer type with nil embedded config", + configName: "bad-auth-config", externalAuthCfg: &mcpv1alpha1.MCPExternalAuthConfig{ ObjectMeta: metav1.ObjectMeta{ Name: "bad-auth-config", @@ -633,10 +622,8 @@ func TestGenerateAuthServerConfig(t *testing.T) { errContains: "embedded auth server configuration is nil", }, { - name: "non-existent external auth config", - externalAuthRef: &mcpv1alpha1.ExternalAuthConfigRef{ - Name: "non-existent", - }, + name: "non-existent external auth config", + configName: "non-existent", externalAuthCfg: nil, // No config to create wantVolumes: false, wantMounts: false, @@ -661,8 +648,8 @@ func TestGenerateAuthServerConfig(t *testing.T) { Build() ctx := context.Background() - volumes, mounts, envVars, err := GenerateAuthServerConfig( - ctx, fakeClient, "default", tt.externalAuthRef, + volumes, mounts, envVars, err := GenerateAuthServerConfigByName( + ctx, fakeClient, "default", tt.configName, ) if tt.wantErr { @@ -1561,3 +1548,314 @@ func TestBuildAuthServerRunConfig_WithRedisStorage(t *testing.T) { assert.Equal(t, "mymaster", config.Storage.RedisConfig.SentinelConfig.MasterName) assert.Equal(t, authrunner.RedisUsernameEnvVar, config.Storage.RedisConfig.ACLUserConfig.UsernameEnvVar) } + +func TestAddAuthServerRefOptions(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + newValidEmbeddedAuthConfig := func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "auth-server-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + }, + } + } + + newUnauthenticatedConfig := func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "unauth-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeUnauthenticated, + }, + } + } + + validOIDCConfig := &oidc.OIDCConfig{ + ResourceURL: "https://mcp.example.com", + Scopes: []string{"openid"}, + } + + tests := []struct { + name string + authServerRef *mcpv1alpha1.AuthServerRef + oidcConfig *oidc.OIDCConfig + objects func() []runtime.Object + wantErr bool + errContains string + wantOptions int + }{ + { + name: "nil ref returns nil", + authServerRef: nil, + oidcConfig: validOIDCConfig, + wantErr: false, + wantOptions: 0, + }, + { + name: "unsupported kind returns error", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "Foo", + Name: "some-config", + }, + oidcConfig: validOIDCConfig, + wantErr: true, + errContains: "unsupported authServerRef kind", + }, + { + name: "non-existent config returns error", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "non-existent", + }, + oidcConfig: validOIDCConfig, + wantErr: true, + errContains: "failed to get MCPExternalAuthConfig", + }, + { + name: "wrong type returns error", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "unauth-config", + }, + oidcConfig: validOIDCConfig, + objects: func() []runtime.Object { return []runtime.Object{newUnauthenticatedConfig()} }, + wantErr: true, + errContains: "must reference a MCPExternalAuthConfig with type", + }, + { + name: "valid ref appends option", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "auth-server-config", + }, + oidcConfig: validOIDCConfig, + objects: func() []runtime.Object { return []runtime.Object{newValidEmbeddedAuthConfig()} }, + wantErr: false, + wantOptions: 1, + }, + { + name: "nil OIDC config returns error for valid ref", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "auth-server-config", + }, + oidcConfig: nil, + objects: func() []runtime.Object { return []runtime.Object{newValidEmbeddedAuthConfig()} }, + wantErr: true, + errContains: "OIDC config is required", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + + builder := fake.NewClientBuilder().WithScheme(scheme) + if tt.objects != nil { + builder = builder.WithRuntimeObjects(tt.objects()...) + } + fakeClient := builder.Build() + + var options []runner.RunConfigBuilderOption + err := AddAuthServerRefOptions( + ctx, fakeClient, "default", "test-server", + tt.authServerRef, tt.oidcConfig, &options, + ) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + assert.Len(t, options, tt.wantOptions) + } + }) + } +} + +func TestValidateAndAddAuthServerRefOptions(t *testing.T) { + t.Parallel() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + newEmbeddedAuthConfig := func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "embedded-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeEmbeddedAuthServer, + EmbeddedAuthServer: &mcpv1alpha1.EmbeddedAuthServerConfig{ + Issuer: "https://auth.example.com", + AuthorizationEndpointBaseURL: "https://auth.example.com", + SigningKeySecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "signing-key", Key: "private.pem"}, + }, + HMACSecretRefs: []mcpv1alpha1.SecretKeyRef{ + {Name: "hmac-secret", Key: "hmac"}, + }, + }, + }, + } + } + + newAWSStsConfig := func() *mcpv1alpha1.MCPExternalAuthConfig { + return &mcpv1alpha1.MCPExternalAuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "aws-sts-config", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPExternalAuthConfigSpec{ + Type: mcpv1alpha1.ExternalAuthTypeAWSSts, + AWSSts: &mcpv1alpha1.AWSStsConfig{ + Region: "us-east-1", + }, + }, + } + } + + validOIDC := &oidc.OIDCConfig{ + ResourceURL: "https://mcp.example.com", + Scopes: []string{"openid"}, + } + + tests := []struct { + name string + authServerRef *mcpv1alpha1.AuthServerRef + externalAuthConfigRef *mcpv1alpha1.ExternalAuthConfigRef + oidcConfig *oidc.OIDCConfig + objects func() []runtime.Object + wantErr bool + errContains string + wantOptions int + }{ + { + name: "both nil is a no-op", + authServerRef: nil, + externalAuthConfigRef: nil, + oidcConfig: validOIDC, + wantErr: false, + wantOptions: 0, + }, + { + name: "authServerRef set with nil externalAuthConfigRef succeeds", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "embedded-config", + }, + externalAuthConfigRef: nil, + oidcConfig: validOIDC, + objects: func() []runtime.Object { return []runtime.Object{newEmbeddedAuthConfig()} }, + wantErr: false, + wantOptions: 1, + }, + { + name: "both refs pointing to embeddedAuthServer returns conflict error", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "embedded-config", + }, + externalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "embedded-config", + }, + oidcConfig: validOIDC, + objects: func() []runtime.Object { return []runtime.Object{newEmbeddedAuthConfig()} }, + wantErr: true, + errContains: "conflict: both authServerRef and externalAuthConfigRef", + }, + { + name: "authServerRef embedded + externalAuthConfigRef awsSts succeeds", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "embedded-config", + }, + externalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "aws-sts-config", + }, + oidcConfig: validOIDC, + objects: func() []runtime.Object { return []runtime.Object{newEmbeddedAuthConfig(), newAWSStsConfig()} }, + wantErr: false, + wantOptions: 1, + }, + { + name: "non-NotFound fetch error for externalAuthConfigRef is returned", + authServerRef: &mcpv1alpha1.AuthServerRef{ + Kind: "MCPExternalAuthConfig", + Name: "embedded-config", + }, + externalAuthConfigRef: &mcpv1alpha1.ExternalAuthConfigRef{ + Name: "will-error", + }, + oidcConfig: validOIDC, + objects: func() []runtime.Object { return []runtime.Object{newEmbeddedAuthConfig()} }, + wantErr: true, + errContains: "failed to fetch externalAuthConfigRef for conflict validation", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + + builder := fake.NewClientBuilder().WithScheme(scheme) + if tt.objects != nil { + builder = builder.WithRuntimeObjects(tt.objects()...) + } + + // For the "non-NotFound fetch error" test case, inject a Get interceptor + // that returns a transient error for the specific resource name. + if tt.name == "non-NotFound fetch error for externalAuthConfigRef is returned" { + builder = builder.WithInterceptorFuncs(interceptor.Funcs{ + Get: func(ctx context.Context, c client.WithWatch, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + if key.Name == "will-error" { + return fmt.Errorf("transient API error") + } + return c.Get(ctx, key, obj, opts...) + }, + }) + } + + fakeClient := builder.Build() + + var options []runner.RunConfigBuilderOption + err := ValidateAndAddAuthServerRefOptions( + ctx, fakeClient, "default", "test-server", + tt.authServerRef, tt.externalAuthConfigRef, + tt.oidcConfig, &options, + ) + + if tt.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), tt.errContains) + } else { + require.NoError(t, err) + assert.Len(t, options, tt.wantOptions) + } + }) + } +} diff --git a/cmd/thv-operator/pkg/controllerutil/config.go b/cmd/thv-operator/pkg/controllerutil/config.go index 24461ab048..772cda9be2 100644 --- a/cmd/thv-operator/pkg/controllerutil/config.go +++ b/cmd/thv-operator/pkg/controllerutil/config.go @@ -74,6 +74,31 @@ func FindReferencingMCPServers( return referencingServers, nil } +// FindReferencingMCPRemoteProxies finds MCPRemoteProxies in the given namespace that reference a config resource. +// The refExtractor function should return the config name from an MCPRemoteProxy if it references the config, +// or nil if it doesn't reference any config of this type. +func FindReferencingMCPRemoteProxies( + ctx context.Context, + c client.Client, + namespace string, + configName string, + refExtractor func(*mcpv1alpha1.MCPRemoteProxy) *string, +) ([]mcpv1alpha1.MCPRemoteProxy, error) { + proxyList := &mcpv1alpha1.MCPRemoteProxyList{} + if err := c.List(ctx, proxyList, client.InNamespace(namespace)); err != nil { + return nil, fmt.Errorf("failed to list MCPRemoteProxies: %w", err) + } + + var referencingProxies []mcpv1alpha1.MCPRemoteProxy + for _, proxy := range proxyList.Items { + if refName := refExtractor(&proxy); refName != nil && *refName == configName { + referencingProxies = append(referencingProxies, proxy) + } + } + + return referencingProxies, nil +} + // CompareWorkloadRefs compares two WorkloadReference values by Kind then Name. // Suitable for use with slices.SortFunc. func CompareWorkloadRefs(a, b mcpv1alpha1.WorkloadReference) int { diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml index 99346ab9ed..22f478538f 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -72,6 +72,27 @@ spec: When true, enables audit logging with default configuration type: boolean type: object + authServerRef: + description: |- + AuthServerRef optionally references a resource that configures an embedded + OAuth 2.0/OIDC authorization server to authenticate MCP clients. + Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + properties: + kind: + default: MCPExternalAuthConfig + description: Kind identifies the type of the referenced resource. + enum: + - MCPExternalAuthConfig + type: string + name: + description: Name is the name of the referenced resource in the + same namespace. + minLength: 1 + type: string + required: + - kind + - name + type: object authzConfig: description: AuthzConfig defines authorization policy configuration for the proxy @@ -716,6 +737,11 @@ spec: status: description: MCPRemoteProxyStatus defines the observed state of MCPRemoteProxy properties: + authServerConfigHash: + description: |- + AuthServerConfigHash is the hash of the referenced authServerRef spec, + used to detect configuration changes and trigger reconciliation. + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml index 4a7059b1fe..386d51ea24 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml @@ -77,6 +77,27 @@ spec: When true, enables audit logging with default configuration type: boolean type: object + authServerRef: + description: |- + AuthServerRef optionally references a resource that configures an embedded + OAuth 2.0/OIDC authorization server to authenticate MCP clients. + Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + properties: + kind: + default: MCPExternalAuthConfig + description: Kind identifies the type of the referenced resource. + enum: + - MCPExternalAuthConfig + type: string + name: + description: Name is the name of the referenced resource in the + same namespace. + minLength: 1 + type: string + required: + - kind + - name + type: object authzConfig: description: AuthzConfig defines authorization policy configuration for the MCP server @@ -968,6 +989,11 @@ spec: status: description: MCPServerStatus defines the observed state of MCPServer properties: + authServerConfigHash: + description: |- + AuthServerConfigHash is the hash of the referenced authServerRef spec, + used to detect configuration changes and trigger reconciliation. + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml index 8a1a4520e9..e477567068 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -75,6 +75,27 @@ spec: When true, enables audit logging with default configuration type: boolean type: object + authServerRef: + description: |- + AuthServerRef optionally references a resource that configures an embedded + OAuth 2.0/OIDC authorization server to authenticate MCP clients. + Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + properties: + kind: + default: MCPExternalAuthConfig + description: Kind identifies the type of the referenced resource. + enum: + - MCPExternalAuthConfig + type: string + name: + description: Name is the name of the referenced resource in the + same namespace. + minLength: 1 + type: string + required: + - kind + - name + type: object authzConfig: description: AuthzConfig defines authorization policy configuration for the proxy @@ -719,6 +740,11 @@ spec: status: description: MCPRemoteProxyStatus defines the observed state of MCPRemoteProxy properties: + authServerConfigHash: + description: |- + AuthServerConfigHash is the hash of the referenced authServerRef spec, + used to detect configuration changes and trigger reconciliation. + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml index a7232513e7..6edc561958 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml @@ -80,6 +80,27 @@ spec: When true, enables audit logging with default configuration type: boolean type: object + authServerRef: + description: |- + AuthServerRef optionally references a resource that configures an embedded + OAuth 2.0/OIDC authorization server to authenticate MCP clients. + Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). + properties: + kind: + default: MCPExternalAuthConfig + description: Kind identifies the type of the referenced resource. + enum: + - MCPExternalAuthConfig + type: string + name: + description: Name is the name of the referenced resource in the + same namespace. + minLength: 1 + type: string + required: + - kind + - name + type: object authzConfig: description: AuthzConfig defines authorization policy configuration for the MCP server @@ -971,6 +992,11 @@ spec: status: description: MCPServerStatus defines the observed state of MCPServer properties: + authServerConfigHash: + description: |- + AuthServerConfigHash is the hash of the referenced authServerRef spec, + used to detect configuration changes and trigger reconciliation. + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index f0340c6514..95b7df9e49 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -817,6 +817,26 @@ _Appears in:_ | `enabled` _boolean_ | Enabled controls whether audit logging is enabled
When true, enables audit logging with default configuration | false | Optional: \{\}
| +#### api.v1alpha1.AuthServerRef + + + +AuthServerRef defines a reference to a resource that configures an embedded +OAuth 2.0/OIDC authorization server. Currently only MCPExternalAuthConfig is supported; +the enum will be extended when a dedicated auth server CRD is introduced. + + + +_Appears in:_ +- [api.v1alpha1.MCPRemoteProxySpec](#apiv1alpha1mcpremoteproxyspec) +- [api.v1alpha1.MCPServerSpec](#apiv1alpha1mcpserverspec) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `kind` _string_ | Kind identifies the type of the referenced resource. | MCPExternalAuthConfig | Enum: [MCPExternalAuthConfig]
| +| `name` _string_ | Name is the name of the referenced resource in the same namespace. | | MinLength: 1
Required: \{\}
| + + #### api.v1alpha1.AuthServerStorageConfig @@ -2240,6 +2260,7 @@ _Appears in:_ | `oidcConfig` _[api.v1alpha1.OIDCConfigRef](#apiv1alpha1oidcconfigref)_ | OIDCConfig defines OIDC authentication configuration for the proxy.
Deprecated: Use OIDCConfigRef to reference a shared MCPOIDCConfig resource instead.
This field will be removed in v1beta1. OIDCConfig and OIDCConfigRef are mutually exclusive. | | Optional: \{\}
| | `oidcConfigRef` _[api.v1alpha1.MCPOIDCConfigReference](#apiv1alpha1mcpoidcconfigreference)_ | OIDCConfigRef references a shared MCPOIDCConfig resource for OIDC authentication.
The referenced MCPOIDCConfig must exist in the same namespace as this MCPRemoteProxy.
Per-server overrides (audience, scopes) are specified here; shared provider config
lives in the MCPOIDCConfig resource. | | Optional: \{\}
| | `externalAuthConfigRef` _[api.v1alpha1.ExternalAuthConfigRef](#apiv1alpha1externalauthconfigref)_ | ExternalAuthConfigRef references a MCPExternalAuthConfig resource for token exchange.
When specified, the proxy will exchange validated incoming tokens for remote service tokens.
The referenced MCPExternalAuthConfig must exist in the same namespace as this MCPRemoteProxy. | | Optional: \{\}
| +| `authServerRef` _[api.v1alpha1.AuthServerRef](#apiv1alpha1authserverref)_ | AuthServerRef optionally references a resource that configures an embedded
OAuth 2.0/OIDC authorization server to authenticate MCP clients.
Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). | | Optional: \{\}
| | `headerForward` _[api.v1alpha1.HeaderForwardConfig](#apiv1alpha1headerforwardconfig)_ | HeaderForward configures headers to inject into requests to the remote MCP server.
Use this to add custom headers like X-Tenant-ID or correlation IDs. | | Optional: \{\}
| | `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration for the proxy | | Optional: \{\}
| | `audit` _[api.v1alpha1.AuditConfig](#apiv1alpha1auditconfig)_ | Audit defines audit logging configuration for the proxy | | Optional: \{\}
| @@ -2274,6 +2295,7 @@ _Appears in:_ | `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPRemoteProxy's state | | Optional: \{\}
| | `toolConfigHash` _string_ | ToolConfigHash stores the hash of the referenced ToolConfig for change detection | | Optional: \{\}
| | `externalAuthConfigHash` _string_ | ExternalAuthConfigHash is the hash of the referenced MCPExternalAuthConfig spec | | Optional: \{\}
| +| `authServerConfigHash` _string_ | AuthServerConfigHash is the hash of the referenced authServerRef spec,
used to detect configuration changes and trigger reconciliation. | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection | | Optional: \{\}
| | `message` _string_ | Message provides additional information about the current phase | | Optional: \{\}
| @@ -2478,6 +2500,7 @@ _Appears in:_ | `audit` _[api.v1alpha1.AuditConfig](#apiv1alpha1auditconfig)_ | Audit defines audit logging configuration for the MCP server | | Optional: \{\}
| | `toolConfigRef` _[api.v1alpha1.ToolConfigRef](#apiv1alpha1toolconfigref)_ | ToolConfigRef references a MCPToolConfig resource for tool filtering and renaming.
The referenced MCPToolConfig must exist in the same namespace as this MCPServer.
Cross-namespace references are not supported for security and isolation reasons. | | Optional: \{\}
| | `externalAuthConfigRef` _[api.v1alpha1.ExternalAuthConfigRef](#apiv1alpha1externalauthconfigref)_ | ExternalAuthConfigRef references a MCPExternalAuthConfig resource for external authentication.
The referenced MCPExternalAuthConfig must exist in the same namespace as this MCPServer. | | Optional: \{\}
| +| `authServerRef` _[api.v1alpha1.AuthServerRef](#apiv1alpha1authserverref)_ | AuthServerRef optionally references a resource that configures an embedded
OAuth 2.0/OIDC authorization server to authenticate MCP clients.
Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). | | Optional: \{\}
| | `telemetryConfigRef` _[api.v1alpha1.MCPTelemetryConfigReference](#apiv1alpha1mcptelemetryconfigreference)_ | TelemetryConfigRef references an MCPTelemetryConfig resource for shared telemetry configuration.
The referenced MCPTelemetryConfig must exist in the same namespace as this MCPServer.
Cross-namespace references are not supported for security and isolation reasons.
Mutually exclusive with the deprecated inline Telemetry field. | | Optional: \{\}
| | `telemetry` _[api.v1alpha1.TelemetryConfig](#apiv1alpha1telemetryconfig)_ | Telemetry defines inline observability configuration for the MCP server.
Deprecated: Use TelemetryConfigRef to reference a shared MCPTelemetryConfig resource instead.
This field will be removed in a future release. Setting both telemetry and telemetryConfigRef
is rejected by CEL validation. | | Optional: \{\}
| | `trustProxyHeaders` _boolean_ | TrustProxyHeaders indicates whether to trust X-Forwarded-* headers from reverse proxies
When enabled, the proxy will use X-Forwarded-Proto, X-Forwarded-Host, X-Forwarded-Port,
and X-Forwarded-Prefix headers to construct endpoint URLs | false | Optional: \{\}
| @@ -2507,6 +2530,7 @@ _Appears in:_ | `observedGeneration` _integer_ | ObservedGeneration reflects the generation most recently observed by the controller | | Optional: \{\}
| | `toolConfigHash` _string_ | ToolConfigHash stores the hash of the referenced ToolConfig for change detection | | Optional: \{\}
| | `externalAuthConfigHash` _string_ | ExternalAuthConfigHash is the hash of the referenced MCPExternalAuthConfig spec | | Optional: \{\}
| +| `authServerConfigHash` _string_ | AuthServerConfigHash is the hash of the referenced authServerRef spec,
used to detect configuration changes and trigger reconciliation. | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection | | Optional: \{\}
| | `telemetryConfigHash` _string_ | TelemetryConfigHash is the hash of the referenced MCPTelemetryConfig spec for change detection | | Optional: \{\}
| | `url` _string_ | URL is the URL where the MCP server can be accessed | | Optional: \{\}
|