operator: Embed Authz config from ConfigMap into runconfig (#1893)

operator: embed Authz config from ConfigMap into runconfig; avoid --authz-config with --from-configmap; add unit and Chainsaw tests

Author: JAORMX

cmd/thv-operator/controllers/mcpserver_runconfig.go

@@ -10,7 +10,9 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"time"
 
+	"gopkg.in/yaml.v3"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -27,6 +29,9 @@ import (
 // defaultProxyHost is the default host for proxy binding
 const defaultProxyHost = "0.0.0.0"
 
+// defaultAPITimeout is the default timeout for Kubernetes API calls made during reconciliation
+const defaultAPITimeout = 15 * time.Second
+
 // RunConfig management methods
 
 // computeConfigMapChecksum computes a SHA256 checksum of the ConfigMap content for change detection
@@ -295,7 +300,12 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPSer
 	addTelemetryConfigOptions(&options, m.Spec.Telemetry, m.Name)
 
 	// Add authorization configuration if specified
-	addAuthzConfigOptions(&options, m.Spec.AuthzConfig)
+	ctx, cancel := context.WithTimeout(context.Background(), defaultAPITimeout)
+	defer cancel()
+
+	if err := r.addAuthzConfigOptions(ctx, m, &options, m.Spec.AuthzConfig); err != nil {
+		return nil, fmt.Errorf("failed to process AuthzConfig: %w", err)
+	}
 
 	// Add OIDC authentication configuration if specified
 	addOIDCConfigOptions(&options, m.Spec.OIDCConfig)
@@ -623,18 +633,25 @@ func addTelemetryConfigOptions(
 }
 
 // addAuthzConfigOptions adds authorization configuration options to the builder options
-func addAuthzConfigOptions(
+// Supports both inline and ConfigMap-based configurations.
+func (r *MCPServerReconciler) addAuthzConfigOptions(
+	ctx context.Context,
+	m *mcpv1alpha1.MCPServer,
 	options *[]runner.RunConfigBuilderOption,
-	authzConfig *mcpv1alpha1.AuthzConfigRef,
-) {
-	if authzConfig == nil {
-		return
+	authzRef *mcpv1alpha1.AuthzConfigRef,
+) error {
+	if authzRef == nil {
+		return nil
 	}
 
-	// Handle inline authorization configuration
-	if authzConfig.Type == mcpv1alpha1.AuthzConfigTypeInline && authzConfig.Inline != nil {
-		policies := authzConfig.Inline.Policies
-		entitiesJSON := authzConfig.Inline.EntitiesJSON
+	switch authzRef.Type {
+	case mcpv1alpha1.AuthzConfigTypeInline:
+		if authzRef.Inline == nil {
+			return fmt.Errorf("inline authz config type specified but inline config is nil")
+		}
+
+		policies := authzRef.Inline.Policies
+		entitiesJSON := authzRef.Inline.EntitiesJSON
 
 		// Create authorization config
 		authzCfg := &authz.Config{
@@ -648,9 +665,64 @@ func addAuthzConfigOptions(
 
 		// Add authorization config to options
 		*options = append(*options, runner.WithAuthzConfig(authzCfg))
-	}
+		return nil
+
+	case mcpv1alpha1.AuthzConfigTypeConfigMap:
+		// Validate reference
+		if authzRef.ConfigMap == nil || authzRef.ConfigMap.Name == "" {
+			return fmt.Errorf("configMap authz config type specified but reference is missing name")
+		}
+		key := authzRef.ConfigMap.Key
+		if key == "" {
+			key = "authz.json"
+		}
 
-	// ConfigMap type is not currently implemented
+		// Ensure we have a Kubernetes client to fetch the ConfigMap
+		if r.Client == nil {
+			return fmt.Errorf("kubernetes client is not configured for ConfigMap authz resolution")
+		}
+
+		// Fetch the ConfigMap from the same namespace as the MCPServer
+		var cm corev1.ConfigMap
+		if err := r.Get(ctx, types.NamespacedName{
+			Namespace: m.Namespace,
+			Name:      authzRef.ConfigMap.Name,
+		}, &cm); err != nil {
+			return fmt.Errorf("failed to get Authz ConfigMap %s/%s: %w", m.Namespace, authzRef.ConfigMap.Name, err)
+		}
+
+		raw, ok := cm.Data[key]
+		if !ok {
+			return fmt.Errorf("authz ConfigMap %s/%s is missing key %q", m.Namespace, authzRef.ConfigMap.Name, key)
+		}
+		if strings.TrimSpace(raw) == "" {
+			return fmt.Errorf("authz ConfigMap %s/%s key %q is empty", m.Namespace, authzRef.ConfigMap.Name, key)
+		}
+
+		// Unmarshal into authz.Config supporting YAML or JSON
+		var cfg authz.Config
+		// Try YAML first (it also handles JSON)
+		if err := yaml.Unmarshal([]byte(raw), &cfg); err != nil {
+			// Fallback to JSON explicitly for clearer error paths
+			if err2 := json.Unmarshal([]byte(raw), &cfg); err2 != nil {
+				return fmt.Errorf("failed to parse authz config from ConfigMap %s/%s key %q: %v; json fallback error: %v",
+					m.Namespace, authzRef.ConfigMap.Name, key, err, err2)
+			}
+		}
+
+		// Validate the config
+		if err := cfg.Validate(); err != nil {
+			return fmt.Errorf("invalid authz config from ConfigMap %s/%s key %q: %w",
+				m.Namespace, authzRef.ConfigMap.Name, key, err)
+		}
+
+		*options = append(*options, runner.WithAuthzConfig(&cfg))
+		return nil
+
+	default:
+		// Unknown type
+		return fmt.Errorf("unknown authz config type: %s", authzRef.Type)
+	}
 }
 
 // addOIDCConfigOptions adds OIDC authentication configuration options to the builder options

cmd/thv-operator/controllers/mcpserver_runconfig_test.go

@@ -26,6 +26,7 @@ const (
 	stdioTransport          = "stdio"
 	sseProxyMode            = "sse"
 	streamableHTTPProxyMode = "streamable-http"
+	defaultAuthzKey         = "authz.json"
 )
 
 func createRunConfigTestScheme() *runtime.Scheme {
@@ -456,7 +457,7 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
 						Type: mcpv1alpha1.AuthzConfigTypeConfigMap,
 						ConfigMap: &mcpv1alpha1.ConfigMapAuthzRef{
 							Name: "test-authz-config",
-							Key:  "authz.json",
+							Key:  defaultAuthzKey,
 						},
 					},
 				},
@@ -465,9 +466,14 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
 			expected: func(t *testing.T, config *runner.RunConfig) {
 				assert.Equal(t, "authz-configmap-server", config.Name)
 
-				// For ConfigMap type, authorization config should not be set directly in RunConfig
-				// since it will be handled by proxyrunner when reading from ConfigMap
-				assert.Nil(t, config.AuthzConfig)
+				// For ConfigMap type, with new feature, authorization config is embedded in RunConfig
+				require.NotNil(t, config.AuthzConfig)
+				assert.Equal(t, "v1", config.AuthzConfig.Version)
+				assert.Equal(t, authz.ConfigTypeCedarV1, config.AuthzConfig.Type)
+				require.NotNil(t, config.AuthzConfig.Cedar)
+				assert.Len(t, config.AuthzConfig.Cedar.Policies, 1)
+				assert.Contains(t, config.AuthzConfig.Cedar.Policies[0], "call_tool")
+				assert.Equal(t, "[]", config.AuthzConfig.Cedar.EntitiesJSON)
 			},
 		},
 		{
@@ -594,7 +600,54 @@ func TestCreateRunConfigFromMCPServer(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			r := &MCPServerReconciler{}
+
+			// Build reconciler; if test uses ConfigMap-based authz, provide a fake client with that ConfigMap
+			var r *MCPServerReconciler
+			if tt.mcpServer != nil &&
+				tt.mcpServer.Spec.AuthzConfig != nil &&
+				tt.mcpServer.Spec.AuthzConfig.Type == mcpv1alpha1.AuthzConfigTypeConfigMap &&
+				tt.mcpServer.Spec.AuthzConfig.ConfigMap != nil {
+
+				scheme := createRunConfigTestScheme()
+
+				// Prepare a ConfigMap with authorization configuration content
+				cm := &corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      tt.mcpServer.Spec.AuthzConfig.ConfigMap.Name,
+						Namespace: tt.mcpServer.Namespace,
+					},
+					Data: map[string]string{
+						func() string {
+							if k := tt.mcpServer.Spec.AuthzConfig.ConfigMap.Key; k != "" {
+								return k
+							}
+							return defaultAuthzKey
+						}(): `{
+							"version": "v1",
+							"type": "cedarv1",
+							"cedar": {
+								"policies": [
+									"permit(principal, action == Action::\"call_tool\", resource == Tool::\"weather\");"
+								],
+								"entities_json": "[]"
+							}
+						}`,
+					},
+				}
+
+				fakeClient := fake.NewClientBuilder().
+					WithScheme(scheme).
+					WithRuntimeObjects(cm).
+					Build()
+
+				r = &MCPServerReconciler{
+					Client: fakeClient,
+					Scheme: scheme,
+				}
+			} else {
+				r = &MCPServerReconciler{}
+			}
+
 			result, err := r.createRunConfigFromMCPServer(tt.mcpServer)
 			require.NoError(t, err)
 			assert.NotNil(t, result)
@@ -1053,7 +1106,7 @@ func TestEnsureRunConfigConfigMap(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 			testScheme := createRunConfigTestScheme()
-			objects := []runtime.Object{}
+			objects := []runtime.Object{tt.mcpServer}
 			if tt.existingCM != nil {
 				objects = append(objects, tt.existingCM)
 			}
@@ -1100,6 +1153,87 @@ func TestEnsureRunConfigConfigMap(t *testing.T) {
 			}
 		})
 	}
+
+	// Additional test: ConfigMap-based Authz referenced externally should be embedded into runconfig.json
+	t.Run("configmap with external authorization configuration", func(t *testing.T) {
+		t.Parallel()
+		testScheme := createRunConfigTestScheme()
+
+		mcpServer := &mcpv1alpha1.MCPServer{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "authz-cm-ext",
+				Namespace: "toolhive-system",
+			},
+			Spec: mcpv1alpha1.MCPServerSpec{
+				Image:     "ghcr.io/example/server:v1.0.0",
+				Transport: "stdio",
+				Port:      8080,
+				AuthzConfig: &mcpv1alpha1.AuthzConfigRef{
+					Type: mcpv1alpha1.AuthzConfigTypeConfigMap,
+					ConfigMap: &mcpv1alpha1.ConfigMapAuthzRef{
+						Name: "ext-authz-config",
+						Key:  "authz.json",
+					},
+				},
+			},
+		}
+
+		authzCM := &corev1.ConfigMap{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "ext-authz-config",
+				Namespace: "toolhive-system",
+			},
+			Data: map[string]string{
+				"authz.json": `{
+					"version": "v1",
+					"type": "cedarv1",
+					"cedar": {
+						"policies": [
+							"permit(principal, action == Action::\"call_tool\", resource == Tool::\"weather\");",
+							"permit(principal, action == Action::\"get_prompt\", resource == Prompt::\"greeting\");"
+						],
+						"entities_json": "[{\"uid\": {\"type\": \"User\", \"id\": \"user1\"}, \"attrs\": {}}]"
+					}
+				}`,
+			},
+		}
+
+		fakeClient := fake.NewClientBuilder().
+			WithScheme(testScheme).
+			WithRuntimeObjects(mcpServer, authzCM).
+			Build()
+
+		reconciler := &MCPServerReconciler{
+			Client: fakeClient,
+			Scheme: testScheme,
+		}
+
+		err := reconciler.ensureRunConfigConfigMap(context.TODO(), mcpServer)
+		require.NoError(t, err)
+
+		// Fetch the generated runconfig ConfigMap
+		configMapName := fmt.Sprintf("%s-runconfig", mcpServer.Name)
+		configMap := &corev1.ConfigMap{}
+		err = fakeClient.Get(context.TODO(), types.NamespacedName{
+			Name:      configMapName,
+			Namespace: mcpServer.Namespace,
+		}, configMap)
+		require.NoError(t, err)
+
+		// Validate that authz config is embedded
+		var runConfig runner.RunConfig
+		err = json.Unmarshal([]byte(configMap.Data["runconfig.json"]), &runConfig)
+		require.NoError(t, err)
+
+		require.NotNil(t, runConfig.AuthzConfig)
+		assert.Equal(t, "v1", runConfig.AuthzConfig.Version)
+		assert.Equal(t, authz.ConfigTypeCedarV1, runConfig.AuthzConfig.Type)
+		require.NotNil(t, runConfig.AuthzConfig.Cedar)
+		assert.Len(t, runConfig.AuthzConfig.Cedar.Policies, 2)
+		assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"call_tool", resource == Tool::"weather");`)
+		assert.Contains(t, runConfig.AuthzConfig.Cedar.Policies, `permit(principal, action == Action::"get_prompt", resource == Prompt::"greeting");`)
+		assert.Equal(t, `[{"uid": {"type": "User", "id": "user1"}, "attrs": {}}]`, runConfig.AuthzConfig.Cedar.EntitiesJSON)
+	})
 }
 
 // TestRunConfigContentEquals tests the content comparison logic

test/e2e/chainsaw/operator/single-tenancy/test-scenarios/authz-configmap-ref/assert-mcpserver-pod-running.yaml

@@ -0,0 +1,9 @@
+apiVersion: v1
+kind: Pod
+metadata:
+  namespace: toolhive-system
+  labels:
+    app: mcpserver
+    app.kubernetes.io/instance: authz-configmap-ref-test
+status:
+  phase: Running

test/e2e/chainsaw/operator/single-tenancy/test-scenarios/authz-configmap-ref/assert-mcpserver-running.yaml

@@ -0,0 +1,7 @@
+apiVersion: toolhive.stacklok.dev/v1alpha1
+kind: MCPServer
+metadata:
+  name: authz-configmap-ref-test
+  namespace: toolhive-system
+status:
+  phase: Running