Forces use of runconfig via configmap for Operator and ProxyRunner (#2196)

* hardcodes default to true

Signed-off-by: ChrisJBurns <[email protected]>

* removes uneeded tests

Signed-off-by: ChrisJBurns <[email protected]>

* amends tests for runconfig configmap

Signed-off-by: ChrisJBurns <[email protected]>

* removes local oject ref

Signed-off-by: ChrisJBurns <[email protected]>

---------

Signed-off-by: ChrisJBurns <[email protected]>

Author: ChrisJBurns

cmd/thv-operator/controllers/mcpserver_controller.go

@@ -84,9 +84,6 @@ var defaultRBACRules = []rbacv1.PolicyRule{
 // mcpContainerName is the name of the mcp container used in pod templates
 const mcpContainerName = "mcp"
 
-// trueValue is the string value "true" used for environment variable comparisons
-const trueValue = "true"
-
 // Restart annotation keys for triggering pod restart
 const (
 	RestartedAtAnnotationKey          = "mcpserver.toolhive.stacklok.dev/restarted-at"
@@ -752,7 +749,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer(ctx context.Context, m *mcp
 	volumes := []corev1.Volume{}
 
 	// Check if global ConfigMap mode is enabled via environment variable
-	useConfigMap := os.Getenv("TOOLHIVE_USE_CONFIGMAP") == trueValue
+	useConfigMap := true
 	if useConfigMap {
 		// Also add pod template patch for secrets and service account (same as regular flags approach)
 		// If service account is not specified, use the default MCP server service account

cmd/thv-operator/controllers/mcpserver_pod_template_test.go

@@ -294,112 +294,6 @@ func TestDeploymentForMCPServerWithSecrets(t *testing.T) {
 		assert.NotContains(t, arg, "--secret=", "No secret CLI arguments should be present")
 	}
 }
-
-func TestDeploymentForMCPServerWithEnvVars(t *testing.T) {
-	t.Parallel()
-	// Create a test MCPServer with environment variables
-	mcpServer := &mcpv1alpha1.MCPServer{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-mcp-server-env",
-			Namespace: "default",
-		},
-		Spec: mcpv1alpha1.MCPServerSpec{
-			Image:     "test-image:latest",
-			Transport: "stdio",
-			Port:      8080,
-			Env: []mcpv1alpha1.EnvVar{
-				{
-					Name:  "API_KEY",
-					Value: "secret-key-123",
-				},
-				{
-					Name:  "DEBUG_MODE",
-					Value: "true",
-				},
-			},
-		},
-	}
-
-	// Create a new scheme for this test to avoid race conditions
-	s := runtime.NewScheme()
-	_ = scheme.AddToScheme(s)
-	s.AddKnownTypes(mcpv1alpha1.GroupVersion, &mcpv1alpha1.MCPServer{})
-	s.AddKnownTypes(mcpv1alpha1.GroupVersion, &mcpv1alpha1.MCPServerList{})
-
-	// Create a reconciler with the scheme
-	r := &MCPServerReconciler{
-		Scheme: s,
-	}
-
-	// Generate the deployment
-	ctx := context.Background()
-	deployment := r.deploymentForMCPServer(ctx, mcpServer)
-	require.NotNil(t, deployment, "Deployment should not be nil")
-
-	// Check that environment variables are passed as --env flags in the container args
-	container := deployment.Spec.Template.Spec.Containers[0]
-
-	// Verify that the environment variables are NOT set as container environment variables
-	for _, env := range container.Env {
-		assert.NotEqual(t, "API_KEY", env.Name, "API_KEY should not be set as container env var")
-		assert.NotEqual(t, "DEBUG_MODE", env.Name, "DEBUG_MODE should not be set as container env var")
-	}
-
-	// Verify that the environment variables are passed as --env flags in the args
-	apiKeyArgFound := false
-	debugModeArgFound := false
-	for _, arg := range container.Args {
-		if arg == "--env=API_KEY=secret-key-123" {
-			apiKeyArgFound = true
-		}
-		if arg == "--env=DEBUG_MODE=true" {
-			debugModeArgFound = true
-		}
-	}
-	assert.True(t, apiKeyArgFound, "API_KEY should be passed as --env flag")
-	assert.True(t, debugModeArgFound, "DEBUG_MODE should be passed as --env flag")
-}
-
-func TestDeploymentForMCPServerWithProxyMode(t *testing.T) {
-	t.Parallel()
-
-	// Create a test MCPServer with ProxyMode
-	mcpServer := &mcpv1alpha1.MCPServer{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-proxy-mode",
-			Namespace: "default",
-		},
-		Spec: mcpv1alpha1.MCPServerSpec{
-			Image:     "test-image:latest",
-			Transport: "stdio",
-			Port:      8080,
-			ProxyMode: "streamable-http",
-		},
-	}
-
-	// Create a reconciler
-	s := runtime.NewScheme()
-	_ = scheme.AddToScheme(s)
-	s.AddKnownTypes(mcpv1alpha1.GroupVersion, &mcpv1alpha1.MCPServer{})
-	s.AddKnownTypes(mcpv1alpha1.GroupVersion, &mcpv1alpha1.MCPServerList{})
-	r := &MCPServerReconciler{Scheme: s}
-
-	// Generate deployment and check for --proxy-mode flag
-	deployment := r.deploymentForMCPServer(context.Background(), mcpServer)
-	require.NotNil(t, deployment)
-
-	// Verify --proxy-mode flag is present
-	container := deployment.Spec.Template.Spec.Containers[0]
-	found := false
-	for _, arg := range container.Args {
-		if arg == "--proxy-mode=streamable-http" {
-			found = true
-			break
-		}
-	}
-	assert.True(t, found, "--proxy-mode=streamable-http flag should be present in container args")
-}
-
 func TestProxyRunnerSecurityContext(t *testing.T) {
 	t.Parallel()
 

cmd/thv-operator/controllers/mcpserver_resource_overrides_test.go

@@ -320,29 +320,6 @@ func TestResourceOverrides(t *testing.T) {
 			assert.Equal(t, tt.expectedServiceLabels, service.Labels)
 			assert.Equal(t, tt.expectedServiceAnns, service.Annotations)
 
-			// For test case with Vault Agent Injection, verify --env-file-dir argument is present
-			if tt.name == "with Vault Agent Injection pod template annotations" {
-				require.Len(t, deployment.Spec.Template.Spec.Containers, 1)
-				container := deployment.Spec.Template.Spec.Containers[0]
-
-				// Check that --env-file-dir=/vault/secrets argument is present
-				found := false
-				for _, arg := range container.Args {
-					if arg == "--env-file-dir=/vault/secrets" {
-						found = true
-						break
-					}
-				}
-				assert.True(t, found, "Expected --env-file-dir=/vault/secrets argument when vault.hashicorp.com/agent-inject=true")
-
-				// Verify pod template has the expected Vault annotations
-				expectedTemplateAnnotations := map[string]string{
-					"vault.hashicorp.com/agent-inject": "true",
-					"vault.hashicorp.com/role":         "toolhive-mcp-workloads",
-				}
-				assert.Equal(t, expectedTemplateAnnotations, deployment.Spec.Template.Annotations)
-			}
-
 			// For test cases with environment variables, verify they are set correctly
 			if tt.name == "with proxy environment variables" || tt.name == "with both metadata overrides and proxy environment variables" {
 				require.Len(t, deployment.Spec.Template.Spec.Containers, 1)
@@ -428,41 +405,6 @@ func TestMergeStringMaps(t *testing.T) {
 	}
 }
 
-func TestDeploymentNeedsUpdateServiceAccount(t *testing.T) {
-	t.Parallel()
-
-	scheme := runtime.NewScheme()
-	require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
-
-	client := fake.NewClientBuilder().WithScheme(scheme).Build()
-	r := &MCPServerReconciler{
-		Client: client,
-		Scheme: scheme,
-	}
-
-	mcpServer := &mcpv1alpha1.MCPServer{
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      "test-server",
-			Namespace: "default",
-		},
-		Spec: mcpv1alpha1.MCPServerSpec{
-			Image: "test-image",
-			Port:  8080,
-		},
-	}
-
-	// Create a deployment using the current implementation
-	ctx := context.Background()
-	deployment := r.deploymentForMCPServer(ctx, mcpServer)
-	require.NotNil(t, deployment)
-
-	// Test with the current deployment - this should NOT need update
-	needsUpdate := r.deploymentNeedsUpdate(context.Background(), deployment, mcpServer)
-
-	// With the service account bug fixed, this should now return false
-	assert.False(t, needsUpdate, "deploymentNeedsUpdate should return false when deployment matches MCPServer spec")
-}
-
 func TestDeploymentNeedsUpdateProxyEnv(t *testing.T) {
 	t.Parallel()
 
@@ -651,75 +593,3 @@ func TestDeploymentNeedsUpdateProxyEnv(t *testing.T) {
 		})
 	}
 }
-
-func TestDeploymentNeedsUpdateToolsFilter(t *testing.T) {
-	t.Parallel()
-
-	scheme := runtime.NewScheme()
-	require.NoError(t, mcpv1alpha1.AddToScheme(scheme))
-
-	client := fake.NewClientBuilder().WithScheme(scheme).Build()
-	r := &MCPServerReconciler{
-		Client: client,
-		Scheme: scheme,
-	}
-
-	tests := []struct {
-		name               string
-		initialToolsFilter []string
-		newToolsFilter     []string
-		expectEnvChange    bool
-	}{
-		{
-			name:               "empty tools filter",
-			initialToolsFilter: nil,
-			newToolsFilter:     nil,
-			expectEnvChange:    false,
-		},
-		{
-			name:               "tools filter not changed",
-			initialToolsFilter: []string{"tool1", "tool2"},
-			newToolsFilter:     []string{"tool1", "tool2"},
-			expectEnvChange:    false,
-		},
-		{
-			name:               "tools filter changed",
-			initialToolsFilter: []string{"tool1", "tool2"},
-			newToolsFilter:     []string{"tool2", "tool3"},
-			expectEnvChange:    true,
-		},
-		{
-			name:               "tools filter change order",
-			initialToolsFilter: []string{"tool1", "tool2"},
-			newToolsFilter:     []string{"tool2", "tool1"},
-			expectEnvChange:    false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			t.Parallel()
-
-			mcpServer := &mcpv1alpha1.MCPServer{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      "test-server",
-					Namespace: "default",
-				},
-				Spec: mcpv1alpha1.MCPServerSpec{
-					Image:       "test-image",
-					Port:        8080,
-					ToolsFilter: tt.initialToolsFilter,
-				},
-			}
-
-			ctx := context.Background()
-			deployment := r.deploymentForMCPServer(ctx, mcpServer)
-			require.NotNil(t, deployment)
-
-			mcpServer.Spec.ToolsFilter = tt.newToolsFilter
-
-			needsUpdate := r.deploymentNeedsUpdate(context.Background(), deployment, mcpServer)
-			assert.Equal(t, tt.expectEnvChange, needsUpdate)
-		})
-	}
-}

cmd/thv-operator/test-integration/mcp-server/mcpserver_controller_integration_test.go

@@ -141,8 +141,29 @@ var _ = Describe("MCPServer Controller Integration Tests", func() {
 			// Verify there's exactly one container (the toolhive proxy runner)
 			Expect(deployment.Spec.Template.Spec.Containers).To(HaveLen(1))
 
+			templateSpec := deployment.Spec.Template.Spec
+
+			foundRunconfigVolume := false
+			for _, v := range templateSpec.Volumes {
+				if v.Name == "runconfig" && v.ConfigMap != nil && v.ConfigMap.Name == (mcpServerName+"-runconfig") {
+					foundRunconfigVolume = true
+					break
+				}
+			}
+			Expect(foundRunconfigVolume).To(BeTrue(), "Deployment should have a volume sourced from runconfig ConfigMap")
+
 			container := deployment.Spec.Template.Spec.Containers[0]
 
+			// Verify that the runconfig ConfigMap is mounted as a volume
+			foundRunconfigMount := false
+			for _, vm := range container.VolumeMounts {
+				if vm.Name == "runconfig" && vm.MountPath == "/etc/runconfig" {
+					foundRunconfigMount = true
+					break
+				}
+			}
+			Expect(foundRunconfigMount).To(BeTrue(), "runconfig ConfigMap should be mounted at /etc/runconfig")
+
 			// Verify container name and image
 			Expect(container.Name).To(Equal("toolhive"))
 			Expect(container.Image).To(Equal(getExpectedRunnerImage()))
@@ -168,25 +189,8 @@ var _ = Describe("MCPServer Controller Integration Tests", func() {
 			// Verify container args contain the required parameters
 			Expect(container.Args).To(ContainElement("run"))
 			Expect(container.Args).To(ContainElement("--foreground=true"))
-			Expect(container.Args).To(ContainElement(fmt.Sprintf("--proxy-port=%d", mcpServer.Spec.Port)))
-			Expect(container.Args).To(ContainElement(fmt.Sprintf("--name=%s", mcpServerName)))
-			Expect(container.Args).To(ContainElement(fmt.Sprintf("--transport=%s", mcpServer.Spec.Transport)))
-			Expect(container.Args).To(ContainElement(fmt.Sprintf("--proxy-mode=%s", mcpServer.Spec.ProxyMode)))
 			Expect(container.Args).To(ContainElement(mcpServer.Spec.Image))
 
-			// Verify that user args are appended after "--"
-			dashIndex := -1
-			for i, arg := range container.Args {
-				if arg == "--" {
-					dashIndex = i
-					break
-				}
-			}
-			if len(mcpServer.Spec.Args) > 0 {
-				Expect(dashIndex).To(BeNumerically(">=", 0))
-				Expect(container.Args[dashIndex+1:]).To(ContainElement("--verbose"))
-			}
-
 			// Verify container ports
 			Expect(container.Ports).To(HaveLen(1))
 			Expect(container.Ports[0].Name).To(Equal("http"))
@@ -208,6 +212,26 @@ var _ = Describe("MCPServer Controller Integration Tests", func() {
 
 		})
 
+		It("Should create the RunConfig ConfigMap", func() {
+
+			// Wait for Service to be created (using the correct naming pattern)
+			configMap := &corev1.ConfigMap{}
+			configMapName := mcpServerName + "-runconfig"
+			Eventually(func() error {
+				return k8sClient.Get(ctx, types.NamespacedName{
+					Name:      configMapName,
+					Namespace: namespace,
+				}, configMap)
+			}, timeout, interval).Should(Succeed())
+
+			// Verify owner reference is set correctly
+			verifyOwnerReference(configMap.OwnerReferences, createdMCPServer, "ConfigMap")
+
+			// Verify Service configuration
+			Expect(configMap.Data).To(HaveKey("runconfig.json"))
+			Expect(configMap.Annotations).To(HaveKey("toolhive.stacklok.dev/content-checksum"))
+		})
+
 		It("Should create a Service for the MCPServer Proxy", func() {
 
 			// Wait for Service to be created (using the correct naming pattern)
@@ -306,7 +330,6 @@ var _ = Describe("MCPServer Controller Integration Tests", func() {
 					return err
 				}
 				mcpServer.Spec.Image = "example/mcp-server:v2"
-				mcpServer.Spec.Port = 9090
 				return k8sClient.Update(ctx, mcpServer)
 			}, timeout, interval).Should(Succeed())
 
@@ -322,16 +345,12 @@ var _ = Describe("MCPServer Controller Integration Tests", func() {
 				container := deployment.Spec.Template.Spec.Containers[0]
 				// Check if the new image is in the args
 				hasNewImage := false
-				hasNewPort := false
 				for _, arg := range container.Args {
 					if arg == "example/mcp-server:v2" {
 						hasNewImage = true
 					}
-					if arg == "--proxy-port=9090" {
-						hasNewPort = true
-					}
 				}
-				return hasNewImage && hasNewPort
+				return hasNewImage
 			}, timeout, interval).Should(BeTrue())
 		})
 	})