Merge branch 'main' into deprecated-fields

Author: dmjb

.github/workflows/claude.yml

@@ -34,6 +34,6 @@ jobs:
 
       - name: Run Claude Code
         id: claude
-        uses: anthropics/claude-code-action@777ffcbfc9d2e2b07f3cfec41b7c7eadedd1f0dc # v1
+        uses: anthropics/claude-code-action@e8bad572273ce919ba15fec95aef0ce974464753 # v1
         with:
           anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}

.github/workflows/operator-ci.yml

@@ -3,9 +3,6 @@ name: Operator CI
 on:
   workflow_call:
   workflow_dispatch:
-  pull_request:
-    paths:
-      - 'deploy/charts/operator-crds/**'
 
 permissions:
   contents: read
@@ -154,9 +151,9 @@ jobs:
         # but we want to make sure renovate bumps the versions when new ones are released. Doing that with
         # just the number is a bit more difficult and i like simple things.
         version: [
-          "kindest/node:v1.31.9",
-          "kindest/node:v1.32.5",
-          "kindest/node:v1.33.1"
+          "kindest/node:v1.32.8",
+          "kindest/node:v1.33.4",
+          "kindest/node:v1.34.0"
         ]
 
     steps:

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/controllers/mcpserver_runconfig.go

@@ -21,6 +21,7 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/log"
 
 	mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1"
+	"github.com/stacklok/toolhive/cmd/thv-operator/pkg/oidc"
 	"github.com/stacklok/toolhive/pkg/authz"
 	"github.com/stacklok/toolhive/pkg/operator/accessors"
 	"github.com/stacklok/toolhive/pkg/runner"
@@ -306,8 +307,9 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPSer
 		return nil, fmt.Errorf("failed to process AuthzConfig: %w", err)
 	}
 
-	// Add OIDC authentication configuration if specified
-	addOIDCConfigOptions(&options, m.Spec.OIDCConfig)
+	if err := r.addOIDCConfigOptions(ctx, &options, m); err != nil {
+		return nil, fmt.Errorf("failed to process OIDCConfig: %w", err)
+	}
 
 	// Add audit configuration if specified
 	addAuditConfigOptions(&options, m.Spec.Audit)
@@ -732,34 +734,38 @@ func (r *MCPServerReconciler) addAuthzConfigOptions(
 }
 
 // addOIDCConfigOptions adds OIDC authentication configuration options to the builder options
-func addOIDCConfigOptions(
+func (r *MCPServerReconciler) addOIDCConfigOptions(
+	ctx context.Context,
 	options *[]runner.RunConfigBuilderOption,
-	oidcConfig *mcpv1alpha1.OIDCConfigRef,
-) {
-	if oidcConfig == nil {
-		return
-	}
+	m *mcpv1alpha1.MCPServer,
+) error {
 
-	// Handle inline OIDC configuration
-	if oidcConfig.Type == mcpv1alpha1.OIDCConfigTypeInline && oidcConfig.Inline != nil {
-		inline := oidcConfig.Inline
+	// Use the OIDC resolver to get configuration
+	resolver := oidc.NewResolver(r.Client)
+	oidcConfig, err := resolver.Resolve(ctx, m)
+	if err != nil {
+		return fmt.Errorf("failed to resolve OIDC configuration: %w", err)
+	}
 
-		// Add OIDC config to options
-		*options = append(*options, runner.WithOIDCConfig(
-			inline.Issuer,
-			inline.Audience,
-			inline.JWKSURL,
-			inline.IntrospectionURL,
-			inline.ClientID,
-			inline.ClientSecret,
-			inline.ThvCABundlePath,
-			inline.JWKSAuthTokenPath,
-			"", // resourceURL - not available in InlineOIDCConfig
-			inline.JWKSAllowPrivateIP,
-		))
+	if oidcConfig == nil {
+		return nil
 	}
 
-	// ConfigMap and Kubernetes types are not currently supported for OIDC configuration
+	// Add OIDC config to options
+	*options = append(*options, runner.WithOIDCConfig(
+		oidcConfig.Issuer,
+		oidcConfig.Audience,
+		oidcConfig.JWKSURL,
+		oidcConfig.IntrospectionURL,
+		oidcConfig.ClientID,
+		oidcConfig.ClientSecret,
+		oidcConfig.ThvCABundlePath,
+		oidcConfig.JWKSAuthTokenPath,
+		oidcConfig.ResourceURL,
+		oidcConfig.JWKSAllowPrivateIP,
+	))
+
+	return nil
 }
 
 // addAuditConfigOptions adds audit configuration options to the builder options