Skip to content

Commit eb4231a

Browse files
ChrisJBurnsChrisJBurns
authored
Rename MCPServer CRD Port Attributes for Clarity (#1806)
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 0c44157 commit eb4231a

File tree

45 files changed

+205
-153
lines changed

Some content is hidden

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

45 files changed

+205
-153
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: 39 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"`
@@ -725,9 +739,31 @@ func (m *MCPServer) GetOIDCConfig() *OIDCConfigRef {
725739
return m.Spec.OIDCConfig
726740
}
727741

728-
// GetPort returns the port of the MCPServer
729-
func (m *MCPServer) GetPort() int32 {
730-
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
731767
}
732768

733769
func init() {

‎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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ func (r *MCPRemoteProxyReconciler) buildSecurityContexts(
253253
// buildContainerPorts builds container port configuration
254254
func (*MCPRemoteProxyReconciler) buildContainerPorts(proxy *mcpv1alpha1.MCPRemoteProxy) []corev1.ContainerPort {
255255
return []corev1.ContainerPort{{
256-
ContainerPort: proxy.Spec.Port,
256+
ContainerPort: int32(proxy.GetProxyPort()),
257257
Name: "http",
258258
Protocol: corev1.ProtocolTCP,
259259
}}
@@ -279,8 +279,8 @@ func (r *MCPRemoteProxyReconciler) serviceForMCPRemoteProxy(
279279
Spec: corev1.ServiceSpec{
280280
Selector: ls,
281281
Ports: []corev1.ServicePort{{
282-
Port: proxy.Spec.Port,
283-
TargetPort: intstr.FromInt(int(proxy.Spec.Port)),
282+
Port: int32(proxy.GetProxyPort()),
283+
TargetPort: intstr.FromInt(int(proxy.GetProxyPort())),
284284
Protocol: corev1.ProtocolTCP,
285285
Name: "http",
286286
}},

‎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: 11 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.GetProxyPort())
395395
err = r.Status().Update(ctx, mcpServer)
396396
if err != nil {
397397
ctxLogger.Error(err, "Failed to update MCPServer status")
@@ -1151,7 +1151,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer(
11511151
VolumeMounts: volumeMounts,
11521152
Resources: resources,
11531153
Ports: []corev1.ContainerPort{{
1154-
ContainerPort: m.Spec.Port,
1154+
ContainerPort: m.GetProxyPort(),
11551155
Name: "http",
11561156
Protocol: corev1.ProtocolTCP,
11571157
}},
@@ -1228,8 +1228,8 @@ func (r *MCPServerReconciler) serviceForMCPServer(ctx context.Context, m *mcpv1a
12281228
Spec: corev1.ServiceSpec{
12291229
Selector: ls, // Keep original labels for selector
12301230
Ports: []corev1.ServicePort{{
1231-
Port: m.Spec.Port,
1232-
TargetPort: intstr.FromInt(int(m.Spec.Port)),
1231+
Port: m.GetProxyPort(),
1232+
TargetPort: intstr.FromInt(int(m.GetProxyPort())),
12331233
Protocol: corev1.ProtocolTCP,
12341234
Name: "http",
12351235
}},
@@ -1383,7 +1383,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
13831383
}
13841384

13851385
// Check if the port has changed
1386-
portArg := fmt.Sprintf("--proxy-port=%d", mcpServer.Spec.Port)
1386+
portArg := fmt.Sprintf("--proxy-port=%d", mcpServer.GetProxyPort())
13871387
found = false
13881388
for _, arg := range container.Args {
13891389
if arg == portArg {
@@ -1414,7 +1414,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
14141414
}
14151415

14161416
// Check if the container port has changed
1417-
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != mcpServer.Spec.Port {
1417+
if len(container.Ports) > 0 && container.Ports[0].ContainerPort != mcpServer.GetProxyPort() {
14181418
return true
14191419
}
14201420

@@ -1535,12 +1535,12 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
15351535
return true
15361536
}
15371537

1538-
// Check if the targetPort has changed
1539-
if mcpServer.Spec.TargetPort != 0 {
1540-
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)
15411541
found := false
15421542
for _, arg := range container.Args {
1543-
if arg == targetPortArg {
1543+
if arg == mcpPortArg {
15441544
found = true
15451545
break
15461546
}
@@ -1625,7 +1625,7 @@ func (r *MCPServerReconciler) deploymentNeedsUpdate(
16251625
// serviceNeedsUpdate checks if the service needs to be updated
16261626
func serviceNeedsUpdate(service *corev1.Service, mcpServer *mcpv1alpha1.MCPServer) bool {
16271627
// Check if the service port has changed
1628-
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.GetProxyPort() {
16291629
return true
16301630
}
16311631

‎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

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ func TestDeploymentForMCPServerWithPodTemplateSpec(t *testing.T) {
7474
Spec: mcpv1alpha1.MCPServerSpec{
7575
Image: "test-image:latest",
7676
Transport: "stdio",
77-
Port: 8080,
77+
ProxyPort: 8080,
7878
PodTemplateSpec: podTemplateSpecToRawExtension(t, podTemplateSpec),
7979
},
8080
}
@@ -162,7 +162,7 @@ func TestDeploymentForMCPServerSecretsProviderEnv(t *testing.T) {
162162
Spec: mcpv1alpha1.MCPServerSpec{
163163
Image: "test-image:latest",
164164
Transport: "stdio",
165-
Port: 8080,
165+
ProxyPort: 8080,
166166
},
167167
}
168168

@@ -193,7 +193,7 @@ func TestDeploymentForMCPServerWithSecrets(t *testing.T) {
193193
Spec: mcpv1alpha1.MCPServerSpec{
194194
Image: "test-image:latest",
195195
Transport: "stdio",
196-
Port: 8080,
196+
ProxyPort: 8080,
197197
ServiceAccount: &customSA,
198198
Secrets: []mcpv1alpha1.SecretRef{
199199
{
@@ -290,6 +290,7 @@ func TestDeploymentForMCPServerWithSecrets(t *testing.T) {
290290
assert.NotContains(t, arg, "--secret=", "No secret CLI arguments should be present")
291291
}
292292
}
293+
293294
func TestProxyRunnerSecurityContext(t *testing.T) {
294295
t.Parallel()
295296

@@ -302,7 +303,7 @@ func TestProxyRunnerSecurityContext(t *testing.T) {
302303
Spec: mcpv1alpha1.MCPServerSpec{
303304
Image: "test-image:latest",
304305
Transport: "stdio",
305-
Port: 8080,
306+
ProxyPort: 8080,
306307
},
307308
}
308309

@@ -349,7 +350,7 @@ func TestProxyRunnerStructuredLogsEnvVar(t *testing.T) {
349350
Spec: mcpv1alpha1.MCPServerSpec{
350351
Image: "test-image:latest",
351352
Transport: "stdio",
352-
Port: 8080,
353+
ProxyPort: 8080,
353354
},
354355
}
355356

0 commit comments

Comments
 (0)