Skip to content

Commit aaa2be4

Browse files
ChrisJBurnsChrisJBurns
committed
Rename MCPServer CRD port attributes for clarity
- Rename `port` to `proxyPort` - represents proxy runner port - Rename `targetPort` to `mcpPort` - represents MCP server port This change addresses user confusion about port semantics as discussed in issue #1452. The new names clearly indicate which port corresponds to which component: - proxyPort: Port exposed by the proxy runner in Kubernetes - mcpPort: Port that the MCP server listens on internally Signed-off-by: ChrisJBurns <[email protected]>
1 parent 3fffdcc commit aaa2be4

File tree

59 files changed

+1381
-192
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+1381
-192
lines changed

‎cmd/thv-operator/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ kubectl describe mcpserver <name>
212212
|---------------------|----------------------------------------------------|----------|---------|
213213
| `image` | Container image for the MCP server | Yes | - |
214214
| `transport` | Transport method (stdio, streamable-http or sse) | No | stdio |
215-
| `port` | Port to expose the MCP server on | No | 8080 |
216-
| `targetPort` | Port that MCP server listens to | No | - |
215+
| `proxyPort` | Port to expose the MCP server on | No | 8080 |
216+
| `mcpPort` | Port that MCP server listens to | No | - |
217217
| `args` | Additional arguments to pass to the MCP server | No | - |
218218
| `env` | Environment variables to set in the container | No | - |
219219
| `volumes` | Volumes to mount in the container | No | - |

‎cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,12 @@ func (m *MCPRemoteProxy) GetOIDCConfig() *OIDCConfigRef {
202202
return &m.Spec.OIDCConfig
203203
}
204204

205-
// GetPort returns the port for the MCPRemoteProxy
206-
func (m *MCPRemoteProxy) GetPort() int32 {
207-
return m.Spec.Port
205+
// GetProxyPort returns the proxy port of the MCPRemoteProxy
206+
func (m *MCPRemoteProxy) GetProxyPort() int32 {
207+
if m.Spec.Port > 0 {
208+
return m.Spec.Port
209+
}
210+
211+
// default to 8080 if no port is specified
212+
return 8080
208213
}

‎cmd/thv-operator/api/v1alpha1/mcpserver_types.go‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,28 @@ type MCPServerSpec struct {
6969
// +kubebuilder:validation:Minimum=1
7070
// +kubebuilder:validation:Maximum=65535
7171
// +kubebuilder:default=8080
72+
// Deprecated: Use ProxyPort instead
7273
Port int32 `json:"port,omitempty"`
7374

7475
// TargetPort is the port that MCP server listens to
7576
// +kubebuilder:validation:Minimum=1
7677
// +kubebuilder:validation:Maximum=65535
7778
// +optional
79+
// Deprecated: Use McpPort instead
7880
TargetPort int32 `json:"targetPort,omitempty"`
7981

82+
// ProxyPort is the port to expose the proxy runner on
83+
// +kubebuilder:validation:Minimum=1
84+
// +kubebuilder:validation:Maximum=65535
85+
// +kubebuilder:default=8080
86+
ProxyPort int32 `json:"proxyPort,omitempty"`
87+
88+
// McpPort is the port that MCP server listens to
89+
// +kubebuilder:validation:Minimum=1
90+
// +kubebuilder:validation:Maximum=65535
91+
// +optional
92+
McpPort int32 `json:"mcpPort,omitempty"`
93+
8094
// Args are additional arguments to pass to the MCP server
8195
// +optional
8296
Args []string `json:"args,omitempty"`
@@ -465,9 +479,15 @@ type InlineOIDCConfig struct {
465479
ClientID string `json:"clientId,omitempty"`
466480

467481
// ClientSecret is the client secret for introspection (optional)
482+
// Deprecated: Use ClientSecretRef instead for better security
468483
// +optional
469484
ClientSecret string `json:"clientSecret,omitempty"`
470485

486+
// ClientSecretRef is a reference to a Kubernetes Secret containing the client secret
487+
// If both ClientSecret and ClientSecretRef are provided, ClientSecretRef takes precedence
488+
// +optional
489+
ClientSecretRef *SecretKeyRef `json:"clientSecretRef,omitempty"`
490+
471491
// ThvCABundlePath is the path to CA certificate bundle file for HTTPS requests
472492
// The file must be mounted into the pod (e.g., via ConfigMap or Secret volume)
473493
// +optional
@@ -719,9 +739,31 @@ func (m *MCPServer) GetOIDCConfig() *OIDCConfigRef {
719739
return m.Spec.OIDCConfig
720740
}
721741

722-
// GetPort returns the port of the MCPServer
723-
func (m *MCPServer) GetPort() int32 {
724-
return m.Spec.Port
742+
// GetProxyPort returns the proxy port of the MCPServer
743+
func (m *MCPServer) GetProxyPort() int32 {
744+
if m.Spec.ProxyPort > 0 {
745+
return m.Spec.ProxyPort
746+
}
747+
748+
// the below is deprecated and will be removed in a future version
749+
// we need to keep it here to avoid breaking changes
750+
if m.Spec.Port > 0 {
751+
return m.Spec.Port
752+
}
753+
754+
// default to 8080 if no port is specified
755+
return 8080
756+
}
757+
758+
// GetMcpPort returns the MCP port of the MCPServer
759+
func (m *MCPServer) GetMcpPort() int32 {
760+
if m.Spec.McpPort > 0 {
761+
return m.Spec.McpPort
762+
}
763+
764+
// the below is deprecated and will be removed in a future version
765+
// we need to keep it here to avoid breaking changes
766+
return m.Spec.TargetPort
725767
}
726768

727769
func init() {

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

Lines changed: 6 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎cmd/thv-operator/controllers/mcpremoteproxy_controller.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -316,7 +316,7 @@ func (r *MCPRemoteProxyReconciler) ensureService(
316316
func (r *MCPRemoteProxyReconciler) ensureServiceURL(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error {
317317
if proxy.Status.URL == "" {
318318
// Note: createProxyServiceURL uses the remote-prefixed service name
319-
proxy.Status.URL = createProxyServiceURL(proxy.Name, proxy.Namespace, proxy.Spec.Port)
319+
proxy.Status.URL = createProxyServiceURL(proxy.Name, proxy.Namespace, int32(proxy.GetProxyPort()))
320320
return r.Status().Update(ctx, proxy)
321321
}
322322
return nil
@@ -631,7 +631,7 @@ func (r *MCPRemoteProxyReconciler) containerNeedsUpdate(
631631
}
632632

633633
// Check if port has changed
634-
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != proxy.Spec.Port {
634+
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != int32(proxy.GetProxyPort()) {
635635
return true
636636
}
637637

@@ -727,7 +727,7 @@ func (r *MCPRemoteProxyReconciler) podTemplateMetadataNeedsUpdate(
727727
// serviceNeedsUpdate checks if the service needs to be updated
728728
func (*MCPRemoteProxyReconciler) serviceNeedsUpdate(service *corev1.Service, proxy *mcpv1alpha1.MCPRemoteProxy) bool {
729729
// Check if port has changed
730-
if len(service.Spec.Ports) > 0 && service.Spec.Ports[0].Port != proxy.Spec.Port {
730+
if len(service.Spec.Ports) > 0 && service.Spec.Ports[0].Port != int32(proxy.GetProxyPort()) {
731731
return true
732732
}
733733

‎cmd/thv-operator/controllers/mcpremoteproxy_deployment.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,19 @@ func (r *MCPRemoteProxyReconciler) buildEnvVarsForProxy(
151151
}
152152
}
153153

154+
// Add OIDC client secret environment variable if using inline config with secretRef
155+
if proxy.Spec.OIDCConfig.Type == "inline" && proxy.Spec.OIDCConfig.Inline != nil {
156+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
157+
ctx, r.Client, proxy.Namespace, proxy.Spec.OIDCConfig.Inline.ClientSecretRef,
158+
)
159+
if err != nil {
160+
ctxLogger := log.FromContext(ctx)
161+
ctxLogger.Error(err, "Failed to generate OIDC client secret environment variable")
162+
} else if oidcClientSecretEnvVar != nil {
163+
env = append(env, *oidcClientSecretEnvVar)
164+
}
165+
}
166+
154167
// Add user-specified environment variables
155168
if proxy.Spec.ResourceOverrides != nil && proxy.Spec.ResourceOverrides.ProxyDeployment != nil {
156169
for _, envVar := range proxy.Spec.ResourceOverrides.ProxyDeployment.Env {
@@ -240,7 +253,7 @@ func (r *MCPRemoteProxyReconciler) buildSecurityContexts(
240253
// buildContainerPorts builds container port configuration
241254
func (*MCPRemoteProxyReconciler) buildContainerPorts(proxy *mcpv1alpha1.MCPRemoteProxy) []corev1.ContainerPort {
242255
return []corev1.ContainerPort{{
243-
ContainerPort: proxy.Spec.Port,
256+
ContainerPort: int32(proxy.GetProxyPort()),
244257
Name: "http",
245258
Protocol: corev1.ProtocolTCP,
246259
}}
@@ -266,8 +279,8 @@ func (r *MCPRemoteProxyReconciler) serviceForMCPRemoteProxy(
266279
Spec: corev1.ServiceSpec{
267280
Selector: ls,
268281
Ports: []corev1.ServicePort{{
269-
Port: proxy.Spec.Port,
270-
TargetPort: intstr.FromInt(int(proxy.Spec.Port)),
282+
Port: int32(proxy.GetProxyPort()),
283+
TargetPort: intstr.FromInt(int(proxy.GetProxyPort())),
271284
Protocol: corev1.ProtocolTCP,
272285
Name: "http",
273286
}},

‎cmd/thv-operator/controllers/mcpremoteproxy_reconciler_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ func TestMCPRemoteProxyStatusProgression(t *testing.T) {
514514

515515
// Verify status URL was set
516516
assert.NotEmpty(t, updatedProxy.Status.URL)
517-
expectedURL := createProxyServiceURL(proxy.Name, proxy.Namespace, proxy.Spec.Port)
517+
expectedURL := createProxyServiceURL(proxy.Name, proxy.Namespace, int32(proxy.GetProxyPort()))
518518
assert.Equal(t, expectedURL, updatedProxy.Status.URL)
519519
}
520520

‎cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,11 +120,6 @@ func (r *MCPRemoteProxyReconciler) createRunConfigFromMCPRemoteProxy(
120120
proxyHost = envHost
121121
}
122122

123-
port := 8080
124-
if proxy.Spec.Port != 0 {
125-
port = int(proxy.Spec.Port)
126-
}
127-
128123
// Get tool configuration from MCPToolConfig if referenced
129124
var toolsFilter []string
130125
var toolsOverride map[string]runner.ToolOverride
@@ -162,7 +157,7 @@ func (r *MCPRemoteProxyReconciler) createRunConfigFromMCPRemoteProxy(
162157
// Key: Set RemoteURL instead of Image
163158
runner.WithRemoteURL(proxy.Spec.RemoteURL),
164159
// Use user-specified transport (sse or streamable-http, both use HTTPTransport internally)
165-
runner.WithTransportAndPorts(transport, port, 0),
160+
runner.WithTransportAndPorts(transport, int(proxy.GetProxyPort()), 0),
166161
runner.WithHost(proxyHost),
167162
runner.WithTrustProxyHeaders(proxy.Spec.TrustProxyHeaders),
168163
runner.WithToolsFilter(toolsFilter),

‎cmd/thv-operator/controllers/mcpserver_controller.go‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -391,7 +391,7 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
391391

392392
// Update the MCPServer status with the service URL
393393
if mcpServer.Status.URL == "" {
394-
mcpServer.Status.URL = ctrlutil.CreateProxyServiceURL(mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.Port)
394+
mcpServer.Status.URL = ctrlutil.CreateProxyServiceURL(mcpServer.Name, mcpServer.Namespace, mcpServer.Spec.ProxyPort)
395395
err = r.Status().Update(ctx, mcpServer)
396396
if err != nil {
397397
ctxLogger.Error(err, "Failed to update MCPServer status")
@@ -986,6 +986,19 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
986986
}
987987
}
988988

989+
// Add OIDC client secret environment variable if using inline config with secretRef
990+
if m.Spec.OIDCConfig != nil && m.Spec.OIDCConfig.Inline != nil {
991+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
992+
ctx, r.Client, m.Namespace, m.Spec.OIDCConfig.Inline.ClientSecretRef,
993+
)
994+
if err != nil {
995+
ctxLogger := log.FromContext(ctx)
996+
ctxLogger.Error(err, "Failed to generate OIDC client secret environment variable")
997+
} else if oidcClientSecretEnvVar != nil {
998+
env = append(env, *oidcClientSecretEnvVar)
999+
}
1000+
}
1001+
9891002
// Add user-specified proxy environment variables from ResourceOverrides
9901003
if m.Spec.ResourceOverrides != nil && m.Spec.ResourceOverrides.ProxyDeployment != nil {
9911004
for _, envVar := range m.Spec.ResourceOverrides.ProxyDeployment.Env {
@@ -1138,7 +1151,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
11381151
VolumeMounts: volumeMounts,
11391152
Resources: resources,
11401153
Ports: []corev1.ContainerPort{{
1141-
ContainerPort: m.Spec.Port,
1154+
ContainerPort: m.GetProxyPort(),
11421155
Name: "http",
11431156
Protocol: corev1.ProtocolTCP,
11441157
}},
@@ -1215,8 +1228,8 @@ func (r *MCPServerReconciler) serviceForMCPServer(ctx context.Context, m *mcpv1a
12151228
Spec: corev1.ServiceSpec{
12161229
Selector: ls, // Keep original labels for selector
12171230
Ports: []corev1.ServicePort{{
1218-
Port: m.Spec.Port,
1219-
TargetPort: intstr.FromInt(int(m.Spec.Port)),
1231+
Port: m.GetProxyPort(),
1232+
TargetPort: intstr.FromInt(int(m.GetProxyPort())),
12201233
Protocol: corev1.ProtocolTCP,
12211234
Name: "http",
12221235
}},
@@ -1370,7 +1383,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
13701383
}
13711384

13721385
// Check if the port has changed
1373-
portArg := fmt.Sprintf("--proxy-port=%d", mcpServer.Spec.Port)
1386+
portArg := fmt.Sprintf("--proxy-port=%d", mcpServer.Spec.ProxyPort)
13741387
found = false
13751388
for _, arg := range container.Args {
13761389
if arg == portArg {
@@ -1401,7 +1414,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
14011414
}
14021415

14031416
// Check if the container port has changed
1404-
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != mcpServer.Spec.Port {
1417+
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != mcpServer.Spec.ProxyPort {
14051418
return true
14061419
}
14071420

@@ -1442,6 +1455,20 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
14421455
expectedProxyEnv = append(expectedProxyEnv, tokenExchangeEnvVars...)
14431456
}
14441457

1458+
// Add OIDC client secret environment variable if using inline config with secretRef
1459+
if mcpServer.Spec.OIDCConfig != nil && mcpServer.Spec.OIDCConfig.Inline != nil {
1460+
oidcClientSecretEnvVar, err := ctrlutil.GenerateOIDCClientSecretEnvVar(
1461+
ctx, r.Client, mcpServer.Namespace, mcpServer.Spec.OIDCConfig.Inline.ClientSecretRef,
1462+
)
1463+
if err != nil {
1464+
// If we can't generate env var, consider the deployment needs update
1465+
return true
1466+
}
1467+
if oidcClientSecretEnvVar != nil {
1468+
expectedProxyEnv = append(expectedProxyEnv, *oidcClientSecretEnvVar)
1469+
}
1470+
}
1471+
14451472
// Add user-specified environment variables
14461473
if mcpServer.Spec.ResourceOverrides != nil && mcpServer.Spec.ResourceOverrides.ProxyDeployment != nil {
14471474
for _, envVar := range mcpServer.Spec.ResourceOverrides.ProxyDeployment.Env {
@@ -1508,12 +1535,12 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15081535
return true
15091536
}
15101537

1511-
// Check if the targetPort has changed
1512-
if mcpServer.Spec.TargetPort != 0 {
1513-
targetPortArg := fmt.Sprintf("--target-port=%d", mcpServer.Spec.TargetPort)
1538+
// Check if the mcpPort has changed
1539+
if mcpServer.Spec.McpPort != 0 {
1540+
mcpPortArg := fmt.Sprintf("--target-port=%d", mcpServer.Spec.McpPort)
15141541
found := false
15151542
for _, arg := range container.Args {
1516-
if arg == targetPortArg {
1543+
if arg == mcpPortArg {
15171544
found = true
15181545
break
15191546
}
@@ -1598,7 +1625,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15981625
// serviceNeedsUpdate checks if the service needs to be updated
15991626
func serviceNeedsUpdate(service *corev1.Service, mcpServer *mcpv1alpha1.MCPServer) bool {
16001627
// Check if the service port has changed
1601-
if len(service.Spec.Ports) > 0 && service.Spec.Ports[0].Port != mcpServer.Spec.Port {
1628+
if len(service.Spec.Ports) > 0 && service.Spec.Ports[0].Port != mcpServer.Spec.ProxyPort {
16021629
return true
16031630
}
16041631

‎cmd/thv-operator/controllers/mcpserver_platform_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestMCPServerReconciler_DeploymentForMCPServer_Kubernetes(t *testing.T) {
9797
Spec: mcpv1alpha1.MCPServerSpec{
9898
Image: "test-image:latest",
9999
Transport: "stdio",
100-
Port: 8080,
100+
ProxyPort: 8080,
101101
},
102102
}
103103

@@ -169,7 +169,7 @@ func TestMCPServerReconciler_DeploymentForMCPServer_OpenShift(t *testing.T) {
169169
Spec: mcpv1alpha1.MCPServerSpec{
170170
Image: "test-image:latest",
171171
Transport: "stdio",
172-
Port: 8080,
172+
ProxyPort: 8080,
173173
},
174174
}
175175

@@ -247,7 +247,7 @@ func TestMCPServerReconciler_DeploymentForMCPServer_PlatformDetectionError(t *te
247247
Spec: mcpv1alpha1.MCPServerSpec{
248248
Image: "test-image:latest",
249249
Transport: "stdio",
250-
Port: 8080,
250+
ProxyPort: 8080,
251251
},
252252
}
253253

0 commit comments

Comments
 (0)