diff --git a/Makefile b/Makefile index 64c8e9d4e..a8c63981d 100644 --- a/Makefile +++ b/Makefile @@ -107,7 +107,7 @@ help: ## Display this help. .PHONY: manifests manifests: controller-gen ## Generate WebhookConfiguration, ClusterRole and CustomResourceDefinition objects. - $(CONTROLLER_GEN) rbac:roleName=manager-role crd webhook paths="./..." output:crd:artifacts:config=config/crd/bases + $(CONTROLLER_GEN) rbac:roleName=manager-role crd:allowDangerousTypes=true webhook paths="./..." output:crd:artifacts:config=config/crd/bases .PHONY: generate generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations. diff --git a/api/v1alpha1/olsconfig_types.go b/api/v1alpha1/olsconfig_types.go index 1bc40297a..8975b936c 100644 --- a/api/v1alpha1/olsconfig_types.go +++ b/api/v1alpha1/olsconfig_types.go @@ -37,7 +37,7 @@ type OLSConfigSpec struct { OLSDataCollectorConfig OLSDataCollectorSpec `json:"olsDataCollector,omitempty"` // MCP Server settings // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="MCP Server Settings" - MCPServers []MCPServer `json:"mcpServers,omitempty"` + MCPServers []MCPServerConfig `json:"mcpServers,omitempty"` // Feature Gates holds list of features to be enabled explicitly, otherwise they are disabled by default. // possible values: MCPServer // +kubebuilder:validation:Optional @@ -229,6 +229,10 @@ type OLSSpec struct { // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Query System Prompt",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} QuerySystemPrompt string `json:"querySystemPrompt,omitempty"` + // Tool filtering configuration for hybrid RAG retrieval. If not specified, all tools are used. + // +kubebuilder:validation:Optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Tool Filtering Configuration",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:advanced"} + ToolFilteringConfig *ToolFilteringConfig `json:"toolFilteringConfig,omitempty"` } // Persistent Storage Configuration @@ -245,11 +249,11 @@ type Storage struct { // RAGSpec defines how to retrieve a RAG databases. type RAGSpec struct { // The path to the RAG database inside of the container image - // +kubebuilder:default:="/rag/vector_db" + // +kubebuilder:default="/rag/vector_db" // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Index Path in the Image" IndexPath string `json:"indexPath,omitempty"` // The Index ID of the RAG database. Only needed if there are multiple indices in the database. - // +kubebuilder:default:="" + // +kubebuilder:default="" // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Index ID" IndexID string `json:"indexID,omitempty"` // The URL of the container image to use as a RAG source @@ -510,45 +514,127 @@ type ProxyConfig struct { ProxyCACertificateRef *corev1.LocalObjectReference `json:"proxyCACertificate,omitempty"` } -// MCPServer defines the settings for a single MCP server. -type MCPServer struct { +// ToolFilteringConfig defines configuration for tool filtering using hybrid RAG retrieval. +// If this config is present, tool filtering is enabled. If absent, all tools are used. +// The embedding model is not exposed as it's handled by the container image. +// +kubebuilder:validation:XValidation:rule="self.alpha >= 0.0 && self.alpha <= 1.0",message="alpha must be between 0.0 and 1.0" +// +kubebuilder:validation:XValidation:rule="self.threshold >= 0.0 && self.threshold <= 1.0",message="threshold must be between 0.0 and 1.0" +type ToolFilteringConfig struct { + // Weight for dense vs sparse retrieval (1.0 = full dense, 0.0 = full sparse) + // +kubebuilder:default=0.8 + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Alpha Weight" + Alpha float64 `json:"alpha,omitempty"` + + // Number of tools to retrieve + // +kubebuilder:default=10 + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=50 + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Top K" + TopK int `json:"topK,omitempty"` + + // Minimum similarity threshold for filtering results + // +kubebuilder:default=0.01 + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Similarity Threshold" + Threshold float64 `json:"threshold,omitempty"` +} + +// MCPHeaderSourceType defines the type of header value source +// +enum +type MCPHeaderSourceType string + +const ( + // MCPHeaderSourceTypeSecret uses a value from a Kubernetes secret + MCPHeaderSourceTypeSecret MCPHeaderSourceType = "secret" + // MCPHeaderSourceTypeKubernetes uses the Kubernetes service account token + MCPHeaderSourceTypeKubernetes MCPHeaderSourceType = "kubernetes" + // MCPHeaderSourceTypeClient uses the client token from the incoming request + MCPHeaderSourceTypeClient MCPHeaderSourceType = "client" +) + +// MCPHeaderValueSource defines where the header value comes from. +// Uses a discriminated union pattern following KEP-1027. +// The Type field determines which of the other fields should be set. +// Secrets must exist in the operator's namespace. +// +// Examples: +// +// # Use a secret: +// valueFrom: +// type: secret +// secretRef: +// name: my-mcp-secret +// +// # Use Kubernetes service account token: +// valueFrom: +// type: kubernetes +// +// # Pass through client token: +// valueFrom: +// type: client +// +// +kubebuilder:validation:XValidation:rule="self.type == 'secret' ? has(self.secretRef) && size(self.secretRef.name) > 0 : true",message="secretRef with non-empty name is required when type is 'secret'" +// +kubebuilder:validation:XValidation:rule="self.type != 'secret' ? !has(self.secretRef) : true",message="secretRef must not be set when type is 'kubernetes' or 'client'" +type MCPHeaderValueSource struct { + // Type specifies the source type for the header value + // +unionDiscriminator + // +kubebuilder:validation:Required + // +kubebuilder:validation:Enum=secret;kubernetes;client + // +required + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Source Type" + Type MCPHeaderSourceType `json:"type"` + + // Reference to a secret containing the header value. + // Required when Type is "secret". + // The secret must exist in the operator's namespace. + // +unionMember + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Secret Reference" + SecretRef *corev1.LocalObjectReference `json:"secretRef,omitempty"` +} + +// MCPHeader defines a header to send to the MCP server +type MCPHeader struct { + // Name of the header (e.g., "Authorization", "X-API-Key") + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:Pattern=`^[A-Za-z0-9-]+$` + // +required + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Header Name" + Name string `json:"name"` + + // Source of the header value + // +kubebuilder:validation:Required + // +required + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Value Source" + ValueFrom MCPHeaderValueSource `json:"valueFrom"` +} + +// MCPServerConfig defines the streamlined configuration for an MCP server +// This configuration only supports HTTP/HTTPS transport +type MCPServerConfig struct { // Name of the MCP server // +kubebuilder:validation:Required // +required // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Name" Name string `json:"name"` - // Streamable HTTP Transport settings - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Streamable HTTP Transport" - StreamableHTTP *MCPServerStreamableHTTPTransport `json:"streamableHTTP,omitempty"` -} -// MCPServerStreamableHTTPTransport configures the MCP server to use streamable HTTP transport. -type MCPServerStreamableHTTPTransport struct { - // URL of the MCP server + // URL of the MCP server (HTTP/HTTPS) // +kubebuilder:validation:Required // +required // +kubebuilder:validation:Pattern=`^https?://.*$` // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="URL" URL string `json:"url"` - // Timeout for the MCP server, default is 5 seconds + + // Timeout for the MCP server in seconds, default is 5 // +kubebuilder:default=5 - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Timeout in seconds" + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Timeout (seconds)" Timeout int `json:"timeout,omitempty"` - // SSE Read Timeout, default is 10 seconds - // +kubebuilder:default=10 - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="SSE Read Timeout in seconds" - SSEReadTimeout int `json:"sseReadTimeout,omitempty"` + // Headers to send to the MCP server - // the map contains the header name and the name of the secret with the content of the header. This secret - // should contain a header path in the data containing a header value. - // A special case is usage of the kubernetes token in the header. to specify this use - // a string "kubernetes" instead of the secret name - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Headers",xDescriptors={"urn:alm:descriptor:com.tectonic.ui:keyValue"} - Headers map[string]string `json:"headers,omitempty"` - // Enable Server Sent Events - // +kubebuilder:default=false - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Enable Server Sent Events" - EnableSSE bool `json:"enableSSE,omitempty"` + // Each header can reference a secret or use a special source (kubernetes token, client token) + // +optional + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Headers" + Headers []MCPHeader `json:"headers,omitempty"` } // +kubebuilder:object:root=true diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 3e5349ef1..8e007dd6e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -173,43 +173,59 @@ func (in *LimiterConfig) DeepCopy() *LimiterConfig { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MCPServer) DeepCopyInto(out *MCPServer) { +func (in *MCPHeader) DeepCopyInto(out *MCPHeader) { *out = *in - if in.StreamableHTTP != nil { - in, out := &in.StreamableHTTP, &out.StreamableHTTP - *out = new(MCPServerStreamableHTTPTransport) - (*in).DeepCopyInto(*out) + in.ValueFrom.DeepCopyInto(&out.ValueFrom) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPHeader. +func (in *MCPHeader) DeepCopy() *MCPHeader { + if in == nil { + return nil + } + out := new(MCPHeader) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPHeaderValueSource) DeepCopyInto(out *MCPHeaderValueSource) { + *out = *in + if in.SecretRef != nil { + in, out := &in.SecretRef, &out.SecretRef + *out = new(corev1.LocalObjectReference) + **out = **in } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPServer. -func (in *MCPServer) DeepCopy() *MCPServer { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPHeaderValueSource. +func (in *MCPHeaderValueSource) DeepCopy() *MCPHeaderValueSource { if in == nil { return nil } - out := new(MCPServer) + out := new(MCPHeaderValueSource) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *MCPServerStreamableHTTPTransport) DeepCopyInto(out *MCPServerStreamableHTTPTransport) { +func (in *MCPServerConfig) DeepCopyInto(out *MCPServerConfig) { *out = *in if in.Headers != nil { in, out := &in.Headers, &out.Headers - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val + *out = make([]MCPHeader, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) } } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPServerStreamableHTTPTransport. -func (in *MCPServerStreamableHTTPTransport) DeepCopy() *MCPServerStreamableHTTPTransport { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPServerConfig. +func (in *MCPServerConfig) DeepCopy() *MCPServerConfig { if in == nil { return nil } - out := new(MCPServerStreamableHTTPTransport) + out := new(MCPServerConfig) in.DeepCopyInto(out) return out } @@ -312,7 +328,7 @@ func (in *OLSConfigSpec) DeepCopyInto(out *OLSConfigSpec) { out.OLSDataCollectorConfig = in.OLSDataCollectorConfig if in.MCPServers != nil { in, out := &in.MCPServers, &out.MCPServers - *out = make([]MCPServer, len(*in)) + *out = make([]MCPServerConfig, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -424,6 +440,11 @@ func (in *OLSSpec) DeepCopyInto(out *OLSSpec) { *out = new(Storage) (*in).DeepCopyInto(*out) } + if in.ToolFilteringConfig != nil { + in, out := &in.ToolFilteringConfig, &out.ToolFilteringConfig + *out = new(ToolFilteringConfig) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OLSSpec. @@ -600,6 +621,21 @@ func (in *TLSConfig) DeepCopy() *TLSConfig { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ToolFilteringConfig) DeepCopyInto(out *ToolFilteringConfig) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ToolFilteringConfig. +func (in *ToolFilteringConfig) DeepCopy() *ToolFilteringConfig { + if in == nil { + return nil + } + out := new(ToolFilteringConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *UserDataCollectionSpec) DeepCopyInto(out *UserDataCollectionSpec) { *out = *in diff --git a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml index 128791008..a2f4ceb1d 100644 --- a/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml +++ b/bundle/manifests/lightspeed-operator.clusterserviceversion.yaml @@ -38,7 +38,7 @@ metadata: ] capabilities: Basic Install console.openshift.io/operator-monitoring-default: "true" - createdAt: "2025-12-18T10:30:55Z" + createdAt: "2026-01-23T15:14:40Z" features.operators.openshift.io/cnf: "false" features.operators.openshift.io/cni: "false" features.operators.openshift.io/csi: "false" @@ -128,34 +128,35 @@ spec: - description: MCP Server settings displayName: MCP Server Settings path: mcpServers - - description: Name of the MCP server - displayName: Name - path: mcpServers[0].name - - description: Streamable HTTP Transport settings - displayName: Streamable HTTP Transport - path: mcpServers[0].streamableHTTP - - description: Enable Server Sent Events - displayName: Enable Server Sent Events - path: mcpServers[0].streamableHTTP.enableSSE - description: |- Headers to send to the MCP server - the map contains the header name and the name of the secret with the content of the header. This secret - should contain a header path in the data containing a header value. - A special case is usage of the kubernetes token in the header. to specify this use - a string "kubernetes" instead of the secret name + Each header can reference a secret or use a special source (kubernetes token, client token) displayName: Headers - path: mcpServers[0].streamableHTTP.headers - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:keyValue - - description: SSE Read Timeout, default is 10 seconds - displayName: SSE Read Timeout in seconds - path: mcpServers[0].streamableHTTP.sseReadTimeout - - description: Timeout for the MCP server, default is 5 seconds - displayName: Timeout in seconds - path: mcpServers[0].streamableHTTP.timeout - - description: URL of the MCP server + path: mcpServers[0].headers + - description: Name of the header (e.g., "Authorization", "X-API-Key") + displayName: Header Name + path: mcpServers[0].headers[0].name + - description: Source of the header value + displayName: Value Source + path: mcpServers[0].headers[0].valueFrom + - description: |- + Reference to a secret containing the header value. + Required when Type is "secret". + The secret must exist in the operator's namespace. + displayName: Secret Reference + path: mcpServers[0].headers[0].valueFrom.secretRef + - description: Type specifies the source type for the header value + displayName: Source Type + path: mcpServers[0].headers[0].valueFrom.type + - description: Name of the MCP server + displayName: Name + path: mcpServers[0].name + - description: Timeout for the MCP server in seconds, default is 5 + displayName: Timeout (seconds) + path: mcpServers[0].timeout + - description: URL of the MCP server (HTTP/HTTPS) displayName: URL - path: mcpServers[0].streamableHTTP.url + path: mcpServers[0].url - displayName: OLS Settings path: ols - description: Additional CA certificates for TLS communication between OLS service and LLM Provider @@ -340,6 +341,20 @@ spec: path: ols.tlsSecurityProfile x-descriptors: - urn:alm:descriptor:com.tectonic.ui:advanced + - description: Tool filtering configuration for hybrid RAG retrieval. If not specified, all tools are used. + displayName: Tool Filtering Configuration + path: ols.toolFilteringConfig + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:advanced + - description: Weight for dense vs sparse retrieval (1.0 = full dense, 0.0 = full sparse) + displayName: Alpha Weight + path: ols.toolFilteringConfig.alpha + - description: Minimum similarity threshold for filtering results + displayName: Similarity Threshold + path: ols.toolFilteringConfig.threshold + - description: Number of tools to retrieve + displayName: Top K + path: ols.toolFilteringConfig.topK - description: User data collection switches displayName: User Data Collection path: ols.userDataCollection @@ -725,7 +740,7 @@ spec: provider: name: Red Hat, Inc url: https://github.com/openshift/lightspeed-service - version: 1.0.8 + version: 1.0.9 relatedImages: - name: lightspeed-service-api image: quay.io/openshift-lightspeed/lightspeed-service-api:latest diff --git a/bundle/manifests/ols.openshift.io_olsconfigs.yaml b/bundle/manifests/ols.openshift.io_olsconfigs.yaml index 0cec555bc..8f3fe4b38 100644 --- a/bundle/manifests/ols.openshift.io_olsconfigs.yaml +++ b/bundle/manifests/ols.openshift.io_olsconfigs.yaml @@ -347,45 +347,84 @@ spec: mcpServers: description: MCP Server settings items: - description: MCPServer defines the settings for a single MCP server. + description: |- + MCPServerConfig defines the streamlined configuration for an MCP server + This configuration only supports HTTP/HTTPS transport properties: + headers: + description: |- + Headers to send to the MCP server + Each header can reference a secret or use a special source (kubernetes token, client token) + items: + description: MCPHeader defines a header to send to the MCP + server + properties: + name: + description: Name of the header (e.g., "Authorization", + "X-API-Key") + minLength: 1 + pattern: ^[A-Za-z0-9-]+$ + type: string + valueFrom: + description: Source of the header value + properties: + secretRef: + description: |- + Reference to a secret containing the header value. + Required when Type is "secret". + The secret must exist in the operator's namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + type: + description: Type specifies the source type for the + header value + enum: + - secret + - kubernetes + - client + type: string + required: + - type + type: object + x-kubernetes-validations: + - message: secretRef with non-empty name is required when + type is 'secret' + rule: 'self.type == ''secret'' ? has(self.secretRef) + && size(self.secretRef.name) > 0 : true' + - message: secretRef must not be set when type is 'kubernetes' + or 'client' + rule: 'self.type != ''secret'' ? !has(self.secretRef) + : true' + required: + - name + - valueFrom + type: object + type: array name: description: Name of the MCP server type: string - streamableHTTP: - description: Streamable HTTP Transport settings - properties: - enableSSE: - default: false - description: Enable Server Sent Events - type: boolean - headers: - additionalProperties: - type: string - description: |- - Headers to send to the MCP server - the map contains the header name and the name of the secret with the content of the header. This secret - should contain a header path in the data containing a header value. - A special case is usage of the kubernetes token in the header. to specify this use - a string "kubernetes" instead of the secret name - type: object - sseReadTimeout: - default: 10 - description: SSE Read Timeout, default is 10 seconds - type: integer - timeout: - default: 5 - description: Timeout for the MCP server, default is 5 seconds - type: integer - url: - description: URL of the MCP server - pattern: ^https?://.*$ - type: string - required: - - url - type: object + timeout: + default: 5 + description: Timeout for the MCP server in seconds, default + is 5 + type: integer + url: + description: URL of the MCP server (HTTP/HTTPS) + pattern: ^https?://.*$ + type: string required: - name + - url type: object type: array ols: @@ -4688,6 +4727,31 @@ spec: - Custom type: string type: object + toolFilteringConfig: + description: Tool filtering configuration for hybrid RAG retrieval. + If not specified, all tools are used. + properties: + alpha: + default: 0.8 + description: Weight for dense vs sparse retrieval (1.0 = full + dense, 0.0 = full sparse) + type: number + threshold: + default: 0.01 + description: Minimum similarity threshold for filtering results + type: number + topK: + default: 10 + description: Number of tools to retrieve + maximum: 50 + minimum: 1 + type: integer + type: object + x-kubernetes-validations: + - message: alpha must be between 0.0 and 1.0 + rule: self.alpha >= 0.0 && self.alpha <= 1.0 + - message: threshold must be between 0.0 and 1.0 + rule: self.threshold >= 0.0 && self.threshold <= 1.0 userDataCollection: description: User data collection switches properties: diff --git a/config/crd/bases/ols.openshift.io_olsconfigs.yaml b/config/crd/bases/ols.openshift.io_olsconfigs.yaml index 1c853f946..7641b5b79 100644 --- a/config/crd/bases/ols.openshift.io_olsconfigs.yaml +++ b/config/crd/bases/ols.openshift.io_olsconfigs.yaml @@ -347,45 +347,84 @@ spec: mcpServers: description: MCP Server settings items: - description: MCPServer defines the settings for a single MCP server. + description: |- + MCPServerConfig defines the streamlined configuration for an MCP server + This configuration only supports HTTP/HTTPS transport properties: + headers: + description: |- + Headers to send to the MCP server + Each header can reference a secret or use a special source (kubernetes token, client token) + items: + description: MCPHeader defines a header to send to the MCP + server + properties: + name: + description: Name of the header (e.g., "Authorization", + "X-API-Key") + minLength: 1 + pattern: ^[A-Za-z0-9-]+$ + type: string + valueFrom: + description: Source of the header value + properties: + secretRef: + description: |- + Reference to a secret containing the header value. + Required when Type is "secret". + The secret must exist in the operator's namespace. + properties: + name: + default: "" + description: |- + Name of the referent. + This field is effectively required, but due to backwards compatibility is + allowed to be empty. Instances of this type with an empty value here are + almost certainly wrong. + More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names + type: string + type: object + x-kubernetes-map-type: atomic + type: + description: Type specifies the source type for the + header value + enum: + - secret + - kubernetes + - client + type: string + required: + - type + type: object + x-kubernetes-validations: + - message: secretRef with non-empty name is required when + type is 'secret' + rule: 'self.type == ''secret'' ? has(self.secretRef) + && size(self.secretRef.name) > 0 : true' + - message: secretRef must not be set when type is 'kubernetes' + or 'client' + rule: 'self.type != ''secret'' ? !has(self.secretRef) + : true' + required: + - name + - valueFrom + type: object + type: array name: description: Name of the MCP server type: string - streamableHTTP: - description: Streamable HTTP Transport settings - properties: - enableSSE: - default: false - description: Enable Server Sent Events - type: boolean - headers: - additionalProperties: - type: string - description: |- - Headers to send to the MCP server - the map contains the header name and the name of the secret with the content of the header. This secret - should contain a header path in the data containing a header value. - A special case is usage of the kubernetes token in the header. to specify this use - a string "kubernetes" instead of the secret name - type: object - sseReadTimeout: - default: 10 - description: SSE Read Timeout, default is 10 seconds - type: integer - timeout: - default: 5 - description: Timeout for the MCP server, default is 5 seconds - type: integer - url: - description: URL of the MCP server - pattern: ^https?://.*$ - type: string - required: - - url - type: object + timeout: + default: 5 + description: Timeout for the MCP server in seconds, default + is 5 + type: integer + url: + description: URL of the MCP server (HTTP/HTTPS) + pattern: ^https?://.*$ + type: string required: - name + - url type: object type: array ols: @@ -4688,6 +4727,31 @@ spec: - Custom type: string type: object + toolFilteringConfig: + description: Tool filtering configuration for hybrid RAG retrieval. + If not specified, all tools are used. + properties: + alpha: + default: 0.8 + description: Weight for dense vs sparse retrieval (1.0 = full + dense, 0.0 = full sparse) + type: number + threshold: + default: 0.01 + description: Minimum similarity threshold for filtering results + type: number + topK: + default: 10 + description: Number of tools to retrieve + maximum: 50 + minimum: 1 + type: integer + type: object + x-kubernetes-validations: + - message: alpha must be between 0.0 and 1.0 + rule: self.alpha >= 0.0 && self.alpha <= 1.0 + - message: threshold must be between 0.0 and 1.0 + rule: self.threshold >= 0.0 && self.threshold <= 1.0 userDataCollection: description: User data collection switches properties: diff --git a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml index 183f56121..b1cf0a094 100644 --- a/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/lightspeed-operator.clusterserviceversion.yaml @@ -94,33 +94,35 @@ spec: - description: MCP Server settings displayName: MCP Server Settings path: mcpServers + - description: |- + Headers to send to the MCP server + Each header can reference a secret or use a special source (kubernetes token, client token) + displayName: Headers + path: mcpServers[0].headers + - description: Name of the header (e.g., "Authorization", "X-API-Key") + displayName: Header Name + path: mcpServers[0].headers[0].name + - description: Source of the header value + displayName: Value Source + path: mcpServers[0].headers[0].valueFrom + - description: |- + Reference to a secret containing the header value. + Required when Type is "secret". + The secret must exist in the operator's namespace. + displayName: Secret Reference + path: mcpServers[0].headers[0].valueFrom.secretRef + - description: Type specifies the source type for the header value + displayName: Source Type + path: mcpServers[0].headers[0].valueFrom.type - description: Name of the MCP server displayName: Name path: mcpServers[0].name - - description: Streamable HTTP Transport settings - displayName: Streamable HTTP Transport - path: mcpServers[0].streamableHTTP - - description: Enable Server Sent Events - displayName: Enable Server Sent Events - path: mcpServers[0].streamableHTTP.enableSSE - - description: Headers to send to the MCP server the map contains the header - name and the name of the secret with the content of the header. This secret - should contain a header path in the data containing a header value. A special - case is usage of the kubernetes token in the header. to specify this use - a string "kubernetes" instead of the secret name - displayName: Headers - path: mcpServers[0].streamableHTTP.headers - x-descriptors: - - urn:alm:descriptor:com.tectonic.ui:keyValue - - description: SSE Read Timeout, default is 10 seconds - displayName: SSE Read Timeout in seconds - path: mcpServers[0].streamableHTTP.sseReadTimeout - - description: Timeout for the MCP server, default is 5 seconds - displayName: Timeout in seconds - path: mcpServers[0].streamableHTTP.timeout - - description: URL of the MCP server + - description: Timeout for the MCP server in seconds, default is 5 + displayName: Timeout (seconds) + path: mcpServers[0].timeout + - description: URL of the MCP server (HTTP/HTTPS) displayName: URL - path: mcpServers[0].streamableHTTP.url + path: mcpServers[0].url - displayName: OLS Settings path: ols - description: Additional CA certificates for TLS communication between OLS @@ -311,6 +313,22 @@ spec: path: ols.tlsSecurityProfile x-descriptors: - urn:alm:descriptor:com.tectonic.ui:advanced + - description: Tool filtering configuration for hybrid RAG retrieval. If not + specified, all tools are used. + displayName: Tool Filtering Configuration + path: ols.toolFilteringConfig + x-descriptors: + - urn:alm:descriptor:com.tectonic.ui:advanced + - description: Weight for dense vs sparse retrieval (1.0 = full dense, 0.0 = + full sparse) + displayName: Alpha Weight + path: ols.toolFilteringConfig.alpha + - description: Minimum similarity threshold for filtering results + displayName: Similarity Threshold + path: ols.toolFilteringConfig.threshold + - description: Number of tools to retrieve + displayName: Top K + path: ols.toolFilteringConfig.topK - description: User data collection switches displayName: User Data Collection path: ols.userDataCollection diff --git a/config/samples/ols_v1alpha1_olsconfig.yaml b/config/samples/ols_v1alpha1_olsconfig.yaml index bbd70531c..b9f1a9290 100644 --- a/config/samples/ols_v1alpha1_olsconfig.yaml +++ b/config/samples/ols_v1alpha1_olsconfig.yaml @@ -16,3 +16,36 @@ spec: models: - name: gpt-3.5-turbo-1106 name: OpenAI + # Optional: Enable MCP (Model Context Protocol) servers + # featureGates: + # - MCPServer + # mcpServers: + # # Example 1: MCP server with secret-based authentication + # - name: external-mcp + # url: https://mcp.example.com + # timeout: 30 + # headers: + # - name: Authorization + # valueFrom: + # type: secret + # secretRef: + # name: mcp-auth-secret # Secret must exist in operator's namespace + # - name: X-API-Key + # valueFrom: + # type: secret + # secretRef: + # name: mcp-api-key-secret + # # Example 2: MCP server using Kubernetes service account token + # - name: internal-mcp + # url: http://mcp-service.default.svc.cluster.local:8080 + # headers: + # - name: Authorization + # valueFrom: + # type: kubernetes + # # Example 3: MCP server using client token passthrough + # - name: proxy-mcp + # url: https://mcp-proxy.example.com + # headers: + # - name: Authorization + # valueFrom: + # type: client diff --git a/internal/controller/appserver/assets.go b/internal/controller/appserver/assets.go index a923a5439..b8fef52cb 100644 --- a/internal/controller/appserver/assets.go +++ b/internal/controller/appserver/assets.go @@ -14,7 +14,6 @@ import ( corev1 "k8s.io/api/core/v1" networkingv1 "k8s.io/api/networking/v1" rbacv1 "k8s.io/api/rbac/v1" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/client" @@ -292,30 +291,43 @@ func GenerateOLSConfigMap(r reconciler.Reconciler, ctx context.Context, cr *olsv } } - if cr.Spec.OLSConfig.IntrospectionEnabled { - appSrvConfigFile.MCPServers = []utils.MCPServerConfig{ - { - Name: "openshift", - Transport: utils.StreamableHTTP, - StreamableHTTP: &utils.StreamableHTTPTransportConfig{ - URL: fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort), - Timeout: utils.OpenShiftMCPServerTimeout, - SSEReadTimeout: utils.OpenShiftMCPServerHTTPReadTimeout, - Headers: map[string]string{utils.K8S_AUTH_HEADER: utils.KUBERNETES_PLACEHOLDER}, - }, - }, - } + // Generate MCP servers config (includes both introspection + user-defined servers) + mcpServers, err := generateMCPServerConfigs(r, cr) + if err != nil { + return nil, err + } + if len(mcpServers) > 0 { + appSrvConfigFile.MCPServers = mcpServers } - if cr.Spec.FeatureGates != nil && slices.Contains(cr.Spec.FeatureGates, utils.FeatureGateMCPServer) { - mcpServers, err := generateMCPServerConfigs(r, ctx, cr) - if err != nil { - return nil, err - } - if appSrvConfigFile.MCPServers == nil { - appSrvConfigFile.MCPServers = mcpServers + // Only add tool filtering if there are MCP servers to filter + if cr.Spec.OLSConfig.ToolFilteringConfig != nil { + if len(mcpServers) > 0 { + // Apply defaults for zero values (happens when user specifies toolFilteringConfig: {}) + cfg := cr.Spec.OLSConfig.ToolFilteringConfig + alpha, topK, threshold := cfg.Alpha, cfg.TopK, cfg.Threshold + if alpha == 0.0 { + alpha = 0.8 + } + if topK == 0 { + topK = 10 + } + if threshold == 0.0 { + threshold = 0.01 + } + + appSrvConfigFile.OLSConfig.ToolFiltering = &utils.ToolFilteringConfig{ + Alpha: alpha, + TopK: topK, + Threshold: threshold, + } } else { - appSrvConfigFile.MCPServers = append(appSrvConfigFile.MCPServers, mcpServers...) + r.GetLogger().Info( + "ToolFilteringConfig specified but no MCP servers enabled. Tool filtering will be disabled.", + "IntrospectionEnabled", cr.Spec.OLSConfig.IntrospectionEnabled, + "MCPFeatureGate", slices.Contains(cr.Spec.FeatureGates, utils.FeatureGateMCPServer), + "MCPServersCount", len(cr.Spec.MCPServers), + ) } } @@ -709,99 +721,94 @@ func getQueryFilters(cr *olsv1alpha1.OLSConfig) []utils.QueryFilters { return filters } -const ( - SSEField int = iota - StreamableHTTPField -) - -func generateMCPServerConfigs(r reconciler.Reconciler, ctx context.Context, cr *olsv1alpha1.OLSConfig) ([]utils.MCPServerConfig, error) { - if cr.Spec.MCPServers == nil { - return nil, nil - } - +func generateMCPServerConfigs(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) ([]utils.MCPServerConfig, error) { servers := []utils.MCPServerConfig{} - var overall_error error - overall_error = nil - for _, server := range cr.Spec.MCPServers { - // check all the secrets - sse, err := generateMCPStreamableHTTPTransportConfig(r, ctx, &server, SSEField) - if err != nil { - overall_error = err - continue - } - streamableHTTP, err := generateMCPStreamableHTTPTransportConfig(r, ctx, &server, StreamableHTTPField) - if err != nil { - overall_error = err - continue - } + + // Add OpenShift MCP server if introspection is enabled + if cr.Spec.OLSConfig.IntrospectionEnabled { servers = append(servers, utils.MCPServerConfig{ - Name: server.Name, - Transport: getMCPTransport(&server), - SSE: sse, - StreamableHTTP: streamableHTTP, + Name: "openshift", + URL: fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort), + Timeout: utils.OpenShiftMCPServerTimeout, + Headers: map[string]string{ + utils.K8S_AUTH_HEADER: utils.KUBERNETES_PLACEHOLDER, + }, }) } - return servers, overall_error -} -func generateMCPStreamableHTTPTransportConfig(r reconciler.Reconciler, ctx context.Context, server *olsv1alpha1.MCPServer, field int) (*utils.StreamableHTTPTransportConfig, error) { - if server == nil || server.StreamableHTTP == nil { - return nil, nil - } + // Add user-defined MCP servers + if cr.Spec.FeatureGates != nil && slices.Contains(cr.Spec.FeatureGates, utils.FeatureGateMCPServer) && cr.Spec.MCPServers != nil { + for _, server := range cr.Spec.MCPServers { + // Build MCP server config + mcpServer := utils.MCPServerConfig{ + Name: server.Name, + URL: server.URL, + } - switch field { - case SSEField: - if !server.StreamableHTTP.EnableSSE { - return nil, nil - } - case StreamableHTTPField: - if server.StreamableHTTP.EnableSSE { - return nil, nil - } - default: - return nil, nil - } + // Add timeout if specified (default is handled by lightspeed-service) + if server.Timeout > 0 { + mcpServer.Timeout = server.Timeout + } - // convert headers to paths - headers := make(map[string]string, len(server.StreamableHTTP.Headers)) - for k, v := range server.StreamableHTTP.Headers { - if v == utils.KUBERNETES_PLACEHOLDER { - headers[k] = v - } else { - secret := &corev1.Secret{} - err := r.Get(ctx, client.ObjectKey{Name: v, Namespace: r.GetNamespace()}, secret) - if err != nil { - if apierrors.IsNotFound(err) { - r.GetLogger().Error(err, fmt.Sprint("Header secret ", v, " for MCP server ", server.Name, " is not found")) - return nil, fmt.Errorf("MCP %s header secret %s is not found", server.Name, v) + // Add authorization headers if configured + if len(server.Headers) > 0 { + headers := make(map[string]string) + invalidServer := false + for _, header := range server.Headers { + if invalidServer { + break + } + headerName := header.Name + var headerValue string + + // Determine header value based on discriminator type + switch header.ValueFrom.Type { + case olsv1alpha1.MCPHeaderSourceTypeKubernetes: + headerValue = utils.KUBERNETES_PLACEHOLDER + case olsv1alpha1.MCPHeaderSourceTypeClient: + headerValue = utils.CLIENT_PLACEHOLDER + case olsv1alpha1.MCPHeaderSourceTypeSecret: + if header.ValueFrom.SecretRef == nil || header.ValueFrom.SecretRef.Name == "" { + r.GetLogger().Error( + fmt.Errorf("missing secretRef for type 'secret'"), + "Skipping MCP server: type is 'secret' but secretRef is not set", + "server", server.Name, + "header", headerName, + ) + invalidServer = true + continue + } + // Use consistent path structure: /etc/mcp/headers//header + headerValue = path.Join(utils.MCPHeadersMountRoot, header.ValueFrom.SecretRef.Name, utils.MCPSECRETDATAPATH) + default: + // This should never happen due to enum validation + r.GetLogger().Error( + fmt.Errorf("invalid MCP header type: %s", header.ValueFrom.Type), + "Skipping MCP server due to invalid header type", + "server", server.Name, + "header", headerName, + "type", header.ValueFrom.Type, + ) + invalidServer = true + continue + } + + headers[headerName] = headerValue + } + + // Skip this server if any header was invalid + if invalidServer { + continue + } + + if len(headers) > 0 { + mcpServer.Headers = headers } - r.GetLogger().Error(err, fmt.Sprint("Failed to get header", v, " for MCP server ", server.Name)) - return nil, fmt.Errorf("failed to get secret %s for MCP provider %s: %w", v, server.Name, err) - } - // make sure the secret has header path - if _, ok := secret.Data[utils.MCPSECRETDATAPATH]; !ok { - r.GetLogger().Error(err, fmt.Sprint("Header", v, " for MCP server ", server.Name, " does not contain 'header' path")) - return nil, fmt.Errorf("header %s for MCP server %s is missing key 'header'", v, server.Name) } - // update header - headers[k] = path.Join(utils.MCPHeadersMountRoot, v, utils.MCPSECRETDATAPATH) + + servers = append(servers, mcpServer) } } - return &utils.StreamableHTTPTransportConfig{ - URL: server.StreamableHTTP.URL, - Timeout: server.StreamableHTTP.Timeout, - SSEReadTimeout: server.StreamableHTTP.SSEReadTimeout, - Headers: headers, - }, nil -} - -func getMCPTransport(server *olsv1alpha1.MCPServer) utils.MCPTransport { - if server == nil || server.StreamableHTTP == nil { - return "" - } - if server.StreamableHTTP.EnableSSE { - return utils.SSE - } - return utils.StreamableHTTP + return servers, nil } diff --git a/internal/controller/appserver/assets_test.go b/internal/controller/appserver/assets_test.go index 61eeba926..a13ef1dcc 100644 --- a/internal/controller/appserver/assets_test.go +++ b/internal/controller/appserver/assets_test.go @@ -269,81 +269,68 @@ var _ = Describe("App server assets", func() { Expect(err).NotTo(HaveOccurred()) Expect(appSrvConfigFile.MCPServers).NotTo(BeEmpty()) Expect(appSrvConfigFile.MCPServers).To(ContainElement(MatchFields(IgnoreExtras, Fields{ - "Name": Equal("openshift"), - "Transport": Equal(utils.StreamableHTTP), - "StreamableHTTP": PointTo(MatchFields(IgnoreExtras, Fields{ - "URL": Equal(fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort)), - "Timeout": Equal(utils.OpenShiftMCPServerTimeout), - "SSEReadTimeout": Equal(utils.OpenShiftMCPServerHTTPReadTimeout), - "Headers": Equal(map[string]string{utils.K8S_AUTH_HEADER: utils.KUBERNETES_PLACEHOLDER}), - })), + "Name": Equal("openshift"), + "URL": Equal(fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort)), + "Timeout": Equal(utils.OpenShiftMCPServerTimeout), + "Headers": Equal(map[string]string{ + utils.K8S_AUTH_HEADER: utils.KUBERNETES_PLACEHOLDER, + }), }))) }) - It("should fail to generate configmap with additional MCP server if the headers are not configured correctly", func() { + It("should skip MCP server with missing header secret during config generation", func() { cr.Spec.FeatureGates = []olsv1alpha1.FeatureGate{utils.FeatureGateMCPServer} - utils.CreateMCPHeaderSecret(ctx, k8sClient, "garbage", false) - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ + // Note: We don't create the secret - config generation doesn't validate secrets + cr.Spec.MCPServers = []olsv1alpha1.MCPServerConfig{ { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "value3", + Name: "testMCP", + URL: "https://testMCP.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header1", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value3"}, + }, }, }, }, } + // Config generation should succeed - secret validation happens during deployment _, err := GenerateOLSConfigMap(testReconcilerInstance, context.TODO(), cr) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("MCP testMCP header secret value3 is not found")) - - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ - { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "garbage", - }, - }, - }, - } - _, err = GenerateOLSConfigMap(testReconcilerInstance, context.TODO(), cr) - Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("header garbage for MCP server testMCP is missing key 'header'")) + Expect(err).ToNot(HaveOccurred()) }) - It("should generate configmap with additional MCP server if feature gate is enabled", func() { + It("should generate configmap with additional MCP servers if feature gate is enabled", func() { cr.Spec.FeatureGates = []olsv1alpha1.FeatureGate{utils.FeatureGateMCPServer} - utils.CreateMCPHeaderSecret(ctx, k8sClient, "value1", true) - utils.CreateMCPHeaderSecret(ctx, k8sClient, "value2", true) - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ + cr.Spec.MCPServers = []olsv1alpha1.MCPServerConfig{ { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "value1", + Name: "testMCP", + URL: "https://testMCP.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header1", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value1"}, + }, }, }, }, { - Name: "testMCP2", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP2.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header2": "value2", + Name: "testMCP2", + URL: "https://testMCP2.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header2", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value2"}, + }, }, - EnableSSE: true, }, }, } @@ -353,43 +340,36 @@ var _ = Describe("App server assets", func() { err = yaml.Unmarshal([]byte(cm.Data[utils.OLSConfigFilename]), &appSrvConfigFile) Expect(err).NotTo(HaveOccurred()) Expect(appSrvConfigFile.MCPServers).To(HaveLen(2)) + Expect(appSrvConfigFile.MCPServers[0].Name).To(Equal("testMCP")) - Expect(appSrvConfigFile.MCPServers[0].Transport).To(Equal(utils.StreamableHTTP)) - Expect(appSrvConfigFile.MCPServers[0].StreamableHTTP).To(Equal(&utils.StreamableHTTPTransportConfig{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": utils.MCPHeadersMountRoot + "/value1/" + utils.MCPSECRETDATAPATH, - }, + Expect(appSrvConfigFile.MCPServers[0].URL).To(Equal("https://testMCP.com")) + Expect(appSrvConfigFile.MCPServers[0].Timeout).To(Equal(10)) + Expect(appSrvConfigFile.MCPServers[0].Headers).To(Equal(map[string]string{ + "header1": utils.MCPHeadersMountRoot + "/value1/" + utils.MCPSECRETDATAPATH, })) - Expect(appSrvConfigFile.MCPServers[0].SSE).To(BeNil()) Expect(appSrvConfigFile.MCPServers[1].Name).To(Equal("testMCP2")) - Expect(appSrvConfigFile.MCPServers[1].Transport).To(Equal(utils.SSE)) - Expect(appSrvConfigFile.MCPServers[1].SSE).To(Equal(&utils.StreamableHTTPTransportConfig{ - URL: "https://testMCP2.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header2": utils.MCPHeadersMountRoot + "/value2/" + utils.MCPSECRETDATAPATH, - }, + Expect(appSrvConfigFile.MCPServers[1].URL).To(Equal("https://testMCP2.com")) + Expect(appSrvConfigFile.MCPServers[1].Timeout).To(Equal(10)) + Expect(appSrvConfigFile.MCPServers[1].Headers).To(Equal(map[string]string{ + "header2": utils.MCPHeadersMountRoot + "/value2/" + utils.MCPSECRETDATAPATH, })) - Expect(appSrvConfigFile.MCPServers[1].StreamableHTTP).To(BeNil()) }) It("should not generate configmap with additional MCP server if feature gate is missing", func() { Expect(cr.Spec.FeatureGates).To(BeNil()) - utils.CreateMCPHeaderSecret(ctx, k8sClient, "value1", true) - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ + cr.Spec.MCPServers = []olsv1alpha1.MCPServerConfig{ { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "value1", + Name: "testMCP", + URL: "https://testMCP.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header1", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value1"}, + }, }, }, }, @@ -405,15 +385,18 @@ var _ = Describe("App server assets", func() { It("should generate configmap with additional MCP server along side the default MCP server", func() { cr.Spec.OLSConfig.IntrospectionEnabled = true cr.Spec.FeatureGates = []olsv1alpha1.FeatureGate{utils.FeatureGateMCPServer} - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ + cr.Spec.MCPServers = []olsv1alpha1.MCPServerConfig{ { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "value1", + Name: "testMCP", + URL: "https://testMCP.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header1", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value1"}, + }, }, }, }, @@ -424,34 +407,20 @@ var _ = Describe("App server assets", func() { err = yaml.Unmarshal([]byte(cm.Data[utils.OLSConfigFilename]), &appSrvConfigFile) Expect(err).NotTo(HaveOccurred()) Expect(appSrvConfigFile.MCPServers).To(HaveLen(2)) + Expect(appSrvConfigFile.MCPServers).To(ContainElement(MatchFields(IgnoreExtras, Fields{ - "Name": Equal("openshift"), - "Transport": Equal(utils.StreamableHTTP), - "StreamableHTTP": PointTo(MatchFields(IgnoreExtras, Fields{ - "URL": Equal(fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort)), - "Timeout": Equal(utils.OpenShiftMCPServerTimeout), - "SSEReadTimeout": Equal(utils.OpenShiftMCPServerHTTPReadTimeout), - })), - }))) - Expect(appSrvConfigFile.MCPServers).To(ContainElement(MatchFields(IgnoreExtras, Fields{ - "Name": Equal("testMCP"), - "Transport": Equal(utils.StreamableHTTP), - "StreamableHTTP": PointTo(MatchFields(IgnoreExtras, Fields{ - "URL": Equal("https://testMCP.com"), - "Timeout": BeNumerically("==", 10), - "SSEReadTimeout": BeNumerically("==", 10), - "Headers": Equal(map[string]string{"header1": utils.MCPHeadersMountRoot + "/value1/" + utils.MCPSECRETDATAPATH}), - })), + "Name": Equal("openshift"), + "URL": Equal(fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort)), + "Timeout": Equal(utils.OpenShiftMCPServerTimeout), }))) + Expect(appSrvConfigFile.MCPServers).To(ContainElement(MatchFields(IgnoreExtras, Fields{ - "Name": Equal("openshift"), - "Transport": Equal(utils.StreamableHTTP), - "StreamableHTTP": PointTo(MatchFields(IgnoreExtras, Fields{ - "URL": Equal(fmt.Sprintf(utils.OpenShiftMCPServerURL, utils.OpenShiftMCPServerPort)), - "Timeout": Equal(utils.OpenShiftMCPServerTimeout), - "SSEReadTimeout": Equal(utils.OpenShiftMCPServerHTTPReadTimeout), - "Headers": Equal(map[string]string{utils.K8S_AUTH_HEADER: utils.KUBERNETES_PLACEHOLDER}), - })), + "Name": Equal("testMCP"), + "URL": Equal("https://testMCP.com"), + "Timeout": Equal(10), + "Headers": Equal(map[string]string{ + "header1": utils.MCPHeadersMountRoot + "/value1/" + utils.MCPSECRETDATAPATH, + }), }))) }) It("should place APIVersion in ProviderConfig for Azure OpenAI provider", func() { diff --git a/internal/controller/appserver/reconciler_test.go b/internal/controller/appserver/reconciler_test.go index 4e712bdbc..d76f67638 100644 --- a/internal/controller/appserver/reconciler_test.go +++ b/internal/controller/appserver/reconciler_test.go @@ -1034,28 +1034,33 @@ var _ = Describe("App server reconciliator", Ordered, func() { It("should create additional volumes and volume mounts when MCP headers are defined", func() { cr.Spec.FeatureGates = []olsv1alpha1.FeatureGate{utils.FeatureGateMCPServer} - cr.Spec.MCPServers = []olsv1alpha1.MCPServer{ + cr.Spec.MCPServers = []olsv1alpha1.MCPServerConfig{ { - Name: "testMCP", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header1": "value1", + Name: "testMCP", + URL: "https://testMCP.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header1", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value1"}, + }, }, }, }, { - Name: "testMCP2", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://testMCP2.com", - Timeout: 10, - SSEReadTimeout: 10, - Headers: map[string]string{ - "header2": "value2", + Name: "testMCP2", + URL: "https://testMCP2.com", + Timeout: 10, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "header2", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "value2"}, + }, }, - EnableSSE: true, }, }, } diff --git a/internal/controller/lcore/config.go b/internal/controller/lcore/config.go index 7f1086087..785b2181f 100644 --- a/internal/controller/lcore/config.go +++ b/internal/controller/lcore/config.go @@ -6,7 +6,6 @@ import ( "path" "slices" "strings" - "sync" "sigs.k8s.io/yaml" @@ -15,61 +14,6 @@ import ( "github.com/openshift/lightspeed-operator/internal/controller/utils" ) -// Package-level cache to track which MCP servers we've already warned about -// Key format: "mcp-server--gen-" -// This ensures we only log warnings once per CR generation (when spec changes, we log again) -var mcpWarningCache = sync.Map{} - -// FilterHTTPMCPServers filters MCP servers to only include those with HTTP/HTTPS transport. -// LCore only supports StreamableHTTP transport (not SSE or Stdio). -// Logs a warning once per server per CR generation for filtered servers. -// -// Parameters: -// - r: Reconciler for logging -// - cr: OLSConfig CR (used for generation tracking) -// - servers: List of MCP servers to filter -// -// Returns: -// - Filtered list containing only servers with StreamableHTTP transport -func FilterHTTPMCPServers(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig, servers []olsv1alpha1.MCPServer) []olsv1alpha1.MCPServer { - if len(servers) == 0 { - return servers - } - - filtered := []olsv1alpha1.MCPServer{} - - for _, server := range servers { - // LCore only supports HTTP/HTTPS transport (StreamableHTTP) - if server.StreamableHTTP != nil { - filtered = append(filtered, server) - continue - } - - // Server doesn't have StreamableHTTP configured - log warning once per CR generation - warningKey := fmt.Sprintf("mcp-server-%s-gen-%d", server.Name, cr.Generation) - _, alreadyLogged := mcpWarningCache.LoadOrStore(warningKey, true) - if alreadyLogged { - // Already warned about this server in this generation, skip - continue - } - - // First time seeing this misconfigured server in this generation - log warning - r.GetLogger().Error( - fmt.Errorf("unsupported MCP server configuration for LCore"), - "MCP server skipped - LCore requires streamableHTTP transport (HTTP/HTTPS)", - "server", server.Name, - "generation", cr.Generation, - ) - } - - return filtered -} - -// ResetMCPWarningCache clears the warning cache. Useful for testing. -func ResetMCPWarningCache() { - mcpWarningCache = sync.Map{} -} - // DefaultQuerySystemPrompt is the same system prompt as lightspeed-service // (ols/customize/ols/prompts.py QUERY_SYSTEM_INSTRUCTION) const DefaultQuerySystemPrompt = `# ROLE @@ -777,7 +721,6 @@ func buildLCoreDatabaseConfig(r reconciler.Reconciler, _ *olsv1alpha1.OLSConfig) // buildLCoreMCPServersConfig configures Model Context Protocol servers // Allows integration with external context providers for agent workflows -// NOTE: LCore only supports HTTP/HTTPS transport (StreamableHTTP) // NOTE: Secret validation is performed separately during deployment generation func buildLCoreMCPServersConfig(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) []map[string]interface{} { mcpServers := []map[string]interface{}{} @@ -794,38 +737,71 @@ func buildLCoreMCPServersConfig(r reconciler.Reconciler, cr *olsv1alpha1.OLSConf }) } - // Add user-defined MCP servers - filter to HTTP-only and log warnings + // Add user-defined MCP servers if cr.Spec.FeatureGates != nil && slices.Contains(cr.Spec.FeatureGates, utils.FeatureGateMCPServer) { - // Filter to HTTP-only servers, log warnings for filtered servers - filteredServers := FilterHTTPMCPServers(r, cr, cr.Spec.MCPServers) - - for _, server := range filteredServers { + for _, server := range cr.Spec.MCPServers { // Build MCP server config mcpServer := map[string]interface{}{ "name": server.Name, - "url": server.StreamableHTTP.URL, + "url": server.URL, } // Add timeout if specified (default is handled by lightspeed-stack) - if server.StreamableHTTP.Timeout > 0 { - mcpServer["timeout"] = server.StreamableHTTP.Timeout + if server.Timeout > 0 { + mcpServer["timeout"] = server.Timeout } // Add authorization headers if configured - if len(server.StreamableHTTP.Headers) > 0 { + if len(server.Headers) > 0 { headers := make(map[string]string) - for headerName, secretRef := range server.StreamableHTTP.Headers { - if secretRef == utils.KUBERNETES_PLACEHOLDER { - // Special case: use Kubernetes service account token - // Value "kubernetes" is preserved for runtime substitution - headers[headerName] = utils.KUBERNETES_PLACEHOLDER - } else if secretRef != "" { + invalidServer := false + for _, header := range server.Headers { + if invalidServer { + break + } + headerName := header.Name + var headerValue string + + // Determine header value based on discriminator type + switch header.ValueFrom.Type { + case olsv1alpha1.MCPHeaderSourceTypeKubernetes: + headerValue = utils.KUBERNETES_PLACEHOLDER + case olsv1alpha1.MCPHeaderSourceTypeClient: + headerValue = utils.CLIENT_PLACEHOLDER + case olsv1alpha1.MCPHeaderSourceTypeSecret: + if header.ValueFrom.SecretRef == nil || header.ValueFrom.SecretRef.Name == "" { + r.GetLogger().Error( + fmt.Errorf("missing secretRef for type 'secret'"), + "Skipping MCP server: type is 'secret' but secretRef is not set", + "server", server.Name, + "header", headerName, + ) + invalidServer = true + continue + } // Use consistent path structure: /etc/mcp/headers//header - // This matches AppServer's approach and provides a clear contract - // Secret validation happens during deployment generation - headers[headerName] = path.Join(utils.MCPHeadersMountRoot, secretRef, utils.MCPSECRETDATAPATH) + headerValue = path.Join(utils.MCPHeadersMountRoot, header.ValueFrom.SecretRef.Name, utils.MCPSECRETDATAPATH) + default: + // This should never happen due to enum validation + r.GetLogger().Error( + fmt.Errorf("invalid MCP header type: %s", header.ValueFrom.Type), + "Skipping MCP server due to invalid header type", + "server", server.Name, + "header", headerName, + "type", header.ValueFrom.Type, + ) + invalidServer = true + continue } + + headers[headerName] = headerValue } + + // Skip this server if any header was invalid + if invalidServer { + continue + } + if len(headers) > 0 { mcpServer["authorization_headers"] = headers } diff --git a/internal/controller/lcore/config_test.go b/internal/controller/lcore/config_test.go index 8992e2b29..9684917e2 100644 --- a/internal/controller/lcore/config_test.go +++ b/internal/controller/lcore/config_test.go @@ -8,6 +8,7 @@ import ( "github.com/go-logr/logr" olsv1alpha1 "github.com/openshift/lightspeed-operator/api/v1alpha1" "github.com/openshift/lightspeed-operator/internal/controller/utils" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -44,188 +45,6 @@ func (m *mockReconcilerWithLogger) GetLogger() logr.Logger { return m.logger } -func TestFilterHTTPMCPServers(t *testing.T) { - // Reset the warning cache before each test - ResetMCPWarningCache() - - tests := []struct { - name string - servers []olsv1alpha1.MCPServer - expectedFiltered int - expectedWarnings int - generation int64 - }{ - { - name: "Empty slice", - servers: []olsv1alpha1.MCPServer{}, - expectedFiltered: 0, - expectedWarnings: 0, - generation: 1, - }, - { - name: "All HTTP servers", - servers: []olsv1alpha1.MCPServer{ - { - Name: "server1", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://example.com", - }, - }, - { - Name: "server2", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://example.com", - }, - }, - }, - expectedFiltered: 2, - expectedWarnings: 0, - generation: 1, - }, - { - name: "Mixed servers - only HTTP pass through", - servers: []olsv1alpha1.MCPServer{ - { - Name: "http-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://example.com", - }, - }, - { - Name: "no-transport-server", - // No transport specified - }, - }, - expectedFiltered: 1, - expectedWarnings: 1, - generation: 1, - }, - { - name: "All non-HTTP servers", - servers: []olsv1alpha1.MCPServer{ - { - Name: "server1", - // No transport specified - }, - { - Name: "server2", - // No transport specified - }, - }, - expectedFiltered: 0, - expectedWarnings: 2, - generation: 1, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Reset cache for each test case - ResetMCPWarningCache() - - // Create mock reconciler with logger - mockSink := &mockLogger{errorMessages: []string{}} - mockReconciler := &mockReconcilerWithLogger{ - mockReconciler: &mockReconciler{}, - logger: logr.New(mockSink), - } - - cr := &olsv1alpha1.OLSConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Generation: tt.generation, - }, - } - - // First call - should log warnings - filtered := FilterHTTPMCPServers(mockReconciler, cr, tt.servers) - - if len(filtered) != tt.expectedFiltered { - t.Errorf("Expected %d filtered servers, got %d", tt.expectedFiltered, len(filtered)) - } - - if len(mockSink.errorMessages) != tt.expectedWarnings { - t.Errorf("Expected %d warnings, got %d: %v", - tt.expectedWarnings, - len(mockSink.errorMessages), - mockSink.errorMessages) - } - - // Verify all filtered servers have StreamableHTTP - for _, server := range filtered { - if server.StreamableHTTP == nil { - t.Errorf("Filtered server '%s' does not have StreamableHTTP transport", server.Name) - } - } - - // Second call with same generation - should NOT log warnings (cached) - mockSink.errorMessages = []string{} - filtered2 := FilterHTTPMCPServers(mockReconciler, cr, tt.servers) - - if len(filtered2) != tt.expectedFiltered { - t.Errorf("Second call: Expected %d filtered servers, got %d", tt.expectedFiltered, len(filtered2)) - } - - if len(mockSink.errorMessages) != 0 { - t.Errorf("Second call should not log warnings (cached), but got %d warnings: %v", - len(mockSink.errorMessages), - mockSink.errorMessages) - } - }) - } -} - -func TestFilterHTTPMCPServers_GenerationChange(t *testing.T) { - ResetMCPWarningCache() - - servers := []olsv1alpha1.MCPServer{ - { - Name: "no-transport", - }, - } - - mockSink := &mockLogger{errorMessages: []string{}} - mockReconciler := &mockReconcilerWithLogger{ - mockReconciler: &mockReconciler{}, - logger: logr.New(mockSink), - } - - // First call with generation 1 - cr1 := &olsv1alpha1.OLSConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Generation: 1, - }, - } - FilterHTTPMCPServers(mockReconciler, cr1, servers) - - if len(mockSink.errorMessages) != 1 { - t.Errorf("Expected 1 warning for generation 1, got %d", len(mockSink.errorMessages)) - } - - // Second call with same generation - should NOT log - mockSink.errorMessages = []string{} - FilterHTTPMCPServers(mockReconciler, cr1, servers) - - if len(mockSink.errorMessages) != 0 { - t.Errorf("Expected 0 warnings for cached generation 1, got %d", len(mockSink.errorMessages)) - } - - // Third call with NEW generation - SHOULD log again - mockSink.errorMessages = []string{} - cr2 := &olsv1alpha1.OLSConfig{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Generation: 2, - }, - } - FilterHTTPMCPServers(mockReconciler, cr2, servers) - - if len(mockSink.errorMessages) != 1 { - t.Errorf("Expected 1 warning for new generation 2, got %d", len(mockSink.errorMessages)) - } -} - func TestBuildLCoreMCPServersConfig_NoServers(t *testing.T) { r := utils.NewTestReconciler(nil, logr.Discard(), nil, utils.OLSNamespaceDefault) @@ -302,14 +121,17 @@ func TestBuildLCoreMCPServersConfig_UserDefinedServers_KubernetesPlaceholder(t * OLSConfig: olsv1alpha1.OLSSpec{ IntrospectionEnabled: false, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { - Name: "external-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://external.example.com/mcp", - Timeout: 30, - Headers: map[string]string{ - "Authorization": utils.KUBERNETES_PLACEHOLDER, + Name: "external-server", + URL: "https://external.example.com/mcp", + Timeout: 30, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, }, }, }, @@ -361,15 +183,24 @@ func TestBuildLCoreMCPServersConfig_UserDefinedServers_WithSecretRef(t *testing. OLSConfig: olsv1alpha1.OLSSpec{ IntrospectionEnabled: false, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { - Name: "external-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://external.example.com/mcp", - Timeout: 30, - Headers: map[string]string{ - "Authorization": utils.KUBERNETES_PLACEHOLDER, - "X-Custom": "mcp-auth-secret", // This will be validated during deployment + Name: "external-server", + URL: "https://external.example.com/mcp", + Timeout: 30, + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, + }, + { + Name: "X-Custom", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "mcp-auth-secret"}, + }, }, }, }, @@ -414,13 +245,17 @@ func TestBuildLCoreMCPServersConfig_Combined(t *testing.T) { OLSConfig: olsv1alpha1.OLSSpec{ IntrospectionEnabled: true, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "user-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://user-mcp.example.com", - Headers: map[string]string{ - "Authorization": "user-secret", + URL: "http://user-mcp.example.com", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "user-secret"}, + }, }, }, }, @@ -460,36 +295,33 @@ func TestBuildLCoreMCPServersConfig_FiltersNonHTTP(t *testing.T) { OLSConfig: olsv1alpha1.OLSSpec{ IntrospectionEnabled: false, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "http-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://valid.example.com", - Headers: map[string]string{ - "Authorization": "test-secret", + URL: "http://valid.example.com", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "test-secret"}, + }, }, }, }, - { - Name: "no-transport-server", - // No transport - should be filtered out - }, }, }, } - // Reset cache to ensure filtering happens - ResetMCPWarningCache() - // Config generation doesn't validate secrets - validation happens during deployment result := buildLCoreMCPServersConfig(r, cr) if len(result) != 1 { - t.Fatalf("Expected 1 MCP server (non-HTTP filtered out), got %d", len(result)) + t.Fatalf("Expected 1 MCP server, got %d", len(result)) } if result[0]["name"] != "http-server" { - t.Errorf("Expected filtered server to be 'http-server', got '%v'", result[0]["name"]) + t.Errorf("Expected server to be 'http-server', got '%v'", result[0]["name"]) } } @@ -505,13 +337,11 @@ func TestBuildLCoreMCPServersConfig_EmptyHeadersNotAdded(t *testing.T) { OLSConfig: olsv1alpha1.OLSSpec{ IntrospectionEnabled: false, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "no-auth-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://no-auth.example.com", - // No headers specified - }, + URL: "http://no-auth.example.com", + // No headers specified }, }, }, @@ -538,15 +368,22 @@ func TestBuildLCoreMCPServersConfig_SkipsEmptySecretRefs(t *testing.T) { }, Spec: olsv1alpha1.OLSConfigSpec{ FeatureGates: []olsv1alpha1.FeatureGate{utils.FeatureGateMCPServer}, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "server-with-mixed-headers", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://example.com", - Headers: map[string]string{ - "Valid-Header": utils.KUBERNETES_PLACEHOLDER, // Should be included - "Empty-Header": "", // Should be skipped - "Kubernetes-Header": utils.KUBERNETES_PLACEHOLDER, // Should be included + URL: "http://example.com", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Valid-Header", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, + }, + { + Name: "Kubernetes-Header", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, }, }, }, @@ -565,9 +402,9 @@ func TestBuildLCoreMCPServersConfig_SkipsEmptySecretRefs(t *testing.T) { t.Fatal("Expected authorization_headers to be map[string]string") } - // Empty header should not be present - if _, exists := headers["Empty-Header"]; exists { - t.Error("Expected empty header to be skipped") + // Should have 2 headers (both kubernetes tokens) + if len(headers) != 2 { + t.Errorf("Expected 2 headers, got %d", len(headers)) } // Valid headers should be present diff --git a/internal/controller/lcore/deployment.go b/internal/controller/lcore/deployment.go index c1d379c75..2ec26a3f6 100644 --- a/internal/controller/lcore/deployment.go +++ b/internal/controller/lcore/deployment.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "path" - "slices" "strings" "time" @@ -551,44 +550,34 @@ func GenerateLCoreDeployment(r reconciler.Reconciler, cr *olsv1alpha1.OLSConfig) // PostgreSQL CA ConfigMap (service-ca.crt for OpenShift CA) lightspeedStackVolumeMounts = append(lightspeedStackVolumeMounts, utils.GetPostgresCAVolumeMount(path.Join(utils.OLSAppCertsMountRoot, "postgres-ca"))) - // Mount MCP server header secrets - only for HTTP-compatible servers - if cr.Spec.FeatureGates != nil && slices.Contains(cr.Spec.FeatureGates, utils.FeatureGateMCPServer) { - // Filter to HTTP-only servers (no logging needed here, already logged in config) - filteredServers := FilterHTTPMCPServers(r, cr, cr.Spec.MCPServers) - - for _, server := range filteredServers { - if server.StreamableHTTP != nil && server.StreamableHTTP.Headers != nil { - for headerName, secretRef := range server.StreamableHTTP.Headers { - // Skip special placeholders - if secretRef == utils.KUBERNETES_PLACEHOLDER || secretRef == "" { - continue - } - - // Validate secret exists and has correct structure - // This provides fail-fast validation consistent with AppServer - if err := validateMCPHeaderSecret(r, ctx, secretRef, server.Name, headerName); err != nil { - return nil, err - } - - volumes = append(volumes, corev1.Volume{ - Name: "header-" + secretRef, - VolumeSource: corev1.VolumeSource{ - Secret: &corev1.SecretVolumeSource{ - SecretName: secretRef, - DefaultMode: &volumeDefaultMode, - }, - }, - }) - - lightspeedStackVolumeMounts = append(lightspeedStackVolumeMounts, corev1.VolumeMount{ - Name: "header-" + secretRef, - MountPath: path.Join(utils.MCPHeadersMountRoot, secretRef), - ReadOnly: true, - }) - } + // Mount MCP server header secrets + _ = utils.ForEachExternalSecret(cr, func(name, source string) error { + if strings.HasPrefix(source, "mcp-") { + // Validate secret exists and has correct structure + // Extract server and header names from source (format: "mcp-") + serverName := strings.TrimPrefix(source, "mcp-") + if err := validateMCPHeaderSecret(r, ctx, name, serverName, ""); err != nil { + return err } + + volumes = append(volumes, corev1.Volume{ + Name: "header-" + name, + VolumeSource: corev1.VolumeSource{ + Secret: &corev1.SecretVolumeSource{ + SecretName: name, + DefaultMode: &volumeDefaultMode, + }, + }, + }) + + lightspeedStackVolumeMounts = append(lightspeedStackVolumeMounts, corev1.VolumeMount{ + Name: "header-" + name, + MountPath: path.Join(utils.MCPHeadersMountRoot, name), + ReadOnly: true, + }) } - } + return nil + }) lightspeedStackContainer.VolumeMounts = lightspeedStackVolumeMounts lightspeedStackContainer.LivenessProbe = &corev1.Probe{ diff --git a/internal/controller/lcore/deployment_test.go b/internal/controller/lcore/deployment_test.go index 289d407df..9c797af11 100644 --- a/internal/controller/lcore/deployment_test.go +++ b/internal/controller/lcore/deployment_test.go @@ -524,23 +524,34 @@ func TestGenerateLCoreDeploymentWithMCPHeaderSecrets(t *testing.T) { }, }, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "external-mcp-kubernetes-auth", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://external1.example.com", - Headers: map[string]string{ - "Authorization": utils.KUBERNETES_PLACEHOLDER, + URL: "http://external1.example.com", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, }, }, }, { Name: "external-mcp-mixed-auth", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "https://external2.example.com", - Headers: map[string]string{ - "Authorization": utils.KUBERNETES_PLACEHOLDER, - "X-Custom": utils.KUBERNETES_PLACEHOLDER, // Test multiple placeholders + URL: "https://external2.example.com", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, + }, + { + Name: "X-Custom", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, }, }, }, diff --git a/internal/controller/olsconfig_helpers_test.go b/internal/controller/olsconfig_helpers_test.go index 63b2bfaf1..f991bdc8a 100644 --- a/internal/controller/olsconfig_helpers_test.go +++ b/internal/controller/olsconfig_helpers_test.go @@ -652,13 +652,17 @@ var _ = Describe("Helper Functions", func() { }, }, }, - MCPServers: []olsv1alpha1.MCPServer{ + MCPServers: []olsv1alpha1.MCPServerConfig{ { Name: "test-mcp-server", - StreamableHTTP: &olsv1alpha1.MCPServerStreamableHTTPTransport{ - URL: "http://test-mcp-server", - Headers: map[string]string{ - "Authorization": "test-mcp-secret", + URL: "http://test-mcp-server", + Headers: []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeSecret, + SecretRef: &corev1.LocalObjectReference{Name: "test-mcp-secret"}, + }, }, }, }, @@ -748,10 +752,15 @@ var _ = Describe("Helper Functions", func() { Expect(fetchedSecret.Annotations).To(HaveKeyWithValue(utils.WatcherAnnotationKey, utils.OLSConfigName)) }) - It("should skip MCP secrets with 'kubernetes' value", func() { - // Create CR with special "kubernetes" token case - testCR.Spec.MCPServers[0].StreamableHTTP.Headers = map[string]string{ - "Authorization": "kubernetes", // Special case that should be skipped + It("should skip MCP secrets with 'kubernetes' token value", func() { + // Update CR with kubernetes token + testCR.Spec.MCPServers[0].Headers = []olsv1alpha1.MCPHeader{ + { + Name: "Authorization", + ValueFrom: olsv1alpha1.MCPHeaderValueSource{ + Type: olsv1alpha1.MCPHeaderSourceTypeKubernetes, + }, + }, } err := reconciler.annotateExternalResources(ctx, testCR) diff --git a/internal/controller/utils/constants.go b/internal/controller/utils/constants.go index 48e71d470..c674fd5d0 100644 --- a/internal/controller/utils/constants.go +++ b/internal/controller/utils/constants.go @@ -304,6 +304,8 @@ ssl_ca_file = '/etc/certs/cm-olspostgresca/service-ca.crt' K8S_AUTH_HEADER = "Authorization" // Constant, defining usage of kubernetes token KUBERNETES_PLACEHOLDER = "kubernetes" + // Constant, defining usage of client token passthrough + CLIENT_PLACEHOLDER = "client" // MCPHeadersMountRoot is the directory hosting MCP headers in the container MCPHeadersMountRoot = "/etc/mcp/headers" // Header Secret Data Path diff --git a/internal/controller/utils/types.go b/internal/controller/utils/types.go index d22da0dbb..b14dfd765 100644 --- a/internal/controller/utils/types.go +++ b/internal/controller/utils/types.go @@ -161,6 +161,19 @@ type OLSConfig struct { QuotaHandlersConfig *QuotaHandlersConfig `json:"quota_handlers,omitempty"` // User specified system prompt SystemPromptPath string `json:"system_prompt_path,omitempty"` + // Tool filtering configuration for hybrid RAG retrieval + ToolFiltering *ToolFilteringConfig `json:"tool_filtering,omitempty"` +} + +// ToolFilteringConfig defines configuration for tool filtering using hybrid RAG retrieval +// The embedding model is not exposed as it's handled by the container image +type ToolFilteringConfig struct { + // Weight for dense vs sparse retrieval (1.0 = full dense, 0.0 = full sparse) + Alpha float64 `json:"alpha,omitempty"` + // Number of tools to retrieve + TopK int `json:"top_k,omitempty"` + // Minimum similarity threshold for filtering results + Threshold float64 `json:"threshold,omitempty"` } // QuotaHandlersConfig defines the token quota configuration @@ -288,14 +301,12 @@ const ( type MCPServerConfig struct { // MCP server name Name string `json:"name"` - // MCP server transport - stdio or sse - Transport MCPTransport `json:"transport"` - // Transport settings if the transport is stdio - Stdio *StdioTransportConfig `json:"stdio,omitempty"` - // Transport settings if the transport is sse - SSE *StreamableHTTPTransportConfig `json:"sse,omitempty"` - // Transport settings if the transport is streamable_http - StreamableHTTP *StreamableHTTPTransportConfig `json:"streamable_http,omitempty"` + // MCP server URL + URL string `json:"url"` + // Headers (map of header name to file path or placeholder) + Headers map[string]string `json:"headers,omitempty"` + // Timeout in seconds + Timeout int `json:"timeout,omitempty"` } type StdioTransportConfig struct { diff --git a/internal/controller/utils/utils.go b/internal/controller/utils/utils.go index 62576de64..7de9bdcac 100644 --- a/internal/controller/utils/utils.go +++ b/internal/controller/utils/utils.go @@ -709,7 +709,6 @@ func GetSecretResourceVersion(r reconciler.Reconciler, ctx context.Context, secr return secret.ResourceVersion, nil } -// ForEachExternalSecret calls fn for each external secret referenced in the OLSConfig CR. // The callback function receives: // - name: the secret name // - source: a descriptive identifier of where the secret is used (e.g., "llm-provider-openai", "tls", "mcp-myserver") @@ -743,19 +742,18 @@ func ForEachExternalSecret(cr *olsv1alpha1.OLSConfig, fn func(name string, sourc } } - // 3. MCP server header secrets - if cr.Spec.MCPServers != nil { - for _, mcpServer := range cr.Spec.MCPServers { - if mcpServer.StreamableHTTP != nil && mcpServer.StreamableHTTP.Headers != nil { - for _, secretName := range mcpServer.StreamableHTTP.Headers { - // Skip the special "kubernetes" token placeholder - if secretName == KUBERNETES_PLACEHOLDER || secretName == "" { - continue - } - if err := fn(secretName, "mcp-"+mcpServer.Name); err != nil { - return err - } - } + // 3. MCP server header secrets (only for type "secret") + for _, mcpServer := range cr.Spec.MCPServers { + for _, header := range mcpServer.Headers { + // Only process secret references + if header.ValueFrom.Type != olsv1alpha1.MCPHeaderSourceTypeSecret { + continue + } + if header.ValueFrom.SecretRef == nil || header.ValueFrom.SecretRef.Name == "" { + continue + } + if err := fn(header.ValueFrom.SecretRef.Name, "mcp-"+mcpServer.Name); err != nil { + return err } } }