diff --git a/cmd/thv-operator/controllers/mcpserver_authzconfig_test.go b/cmd/thv-operator/controllers/mcpserver_authzconfig_test.go new file mode 100644 index 0000000000..0307c9918c --- /dev/null +++ b/cmd/thv-operator/controllers/mcpserver_authzconfig_test.go @@ -0,0 +1,228 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + "github.com/stacklok/toolhive/pkg/container/kubernetes" +) + +// authzConfigForTest builds an MCPAuthzConfig with the given validity and hash. +func authzConfigForTest(name string, valid bool, hash string) *mcpv1beta1.MCPAuthzConfig { + status := metav1.ConditionFalse + if valid { + status = metav1.ConditionTrue + } + return &mcpv1beta1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: mcpv1beta1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: runtime.RawExtension{Raw: []byte(`{"policies":["permit(principal, action, resource);"],"entities_json":"[]"}`)}, + }, + Status: mcpv1beta1.MCPAuthzConfigStatus{ + ConfigHash: hash, + Conditions: []metav1.Condition{{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, + Status: status, + Reason: "Test", + }}, + }, + } +} + +func TestMCPServerReconciler_handleAuthzConfig(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + mcpServer *mcpv1beta1.MCPServer + authzConfig *mcpv1beta1.MCPAuthzConfig + expectError bool + expectErrContains string + expectHash string + expectHashCleared bool + expectConditionStatus *metav1.ConditionStatus + expectConditionReason string + expectConditionGone bool + }{ + { + name: "no ref clears stored hash and condition", + mcpServer: &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerSpec{Image: "img"}, + Status: mcpv1beta1.MCPServerStatus{ + AuthzConfigHash: "old", + Conditions: []metav1.Condition{{ + Type: mcpv1beta1.ConditionAuthzConfigRefValidated, + Status: metav1.ConditionTrue, + Reason: mcpv1beta1.ConditionReasonAuthzConfigRefValid, + }}, + }, + }, + expectHashCleared: true, + expectConditionGone: true, + }, + { + name: "referenced config not found sets NotFound condition", + mcpServer: &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerSpec{ + Image: "img", + AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "missing"}, + }, + }, + expectError: true, + expectErrContains: "not found", + expectConditionStatus: conditionStatusPtr(metav1.ConditionFalse), + expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefNotFound, + }, + { + name: "referenced config not valid sets NotValid condition", + mcpServer: &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerSpec{ + Image: "img", + AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "bad"}, + }, + }, + authzConfig: authzConfigForTest("bad", false, ""), + expectError: true, + expectErrContains: "not valid", + expectConditionStatus: conditionStatusPtr(metav1.ConditionFalse), + expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefNotValid, + }, + { + name: "valid ref sets condition True and tracks hash", + mcpServer: &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerSpec{ + Image: "img", + AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "ok"}, + }, + }, + authzConfig: authzConfigForTest("ok", true, "hash-123"), + expectHash: "hash-123", + expectConditionStatus: conditionStatusPtr(metav1.ConditionTrue), + expectConditionReason: mcpv1beta1.ConditionReasonAuthzConfigRefValid, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + objs := []runtime.Object{tt.mcpServer} + if tt.authzConfig != nil { + objs = append(objs, tt.authzConfig) + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithRuntimeObjects(objs...). + WithStatusSubresource(&mcpv1beta1.MCPServer{}, &mcpv1beta1.MCPAuthzConfig{}). + Build() + + reconciler := newTestMCPServerReconciler(fakeClient, scheme, kubernetes.PlatformKubernetes) + + err := reconciler.handleAuthzConfig(ctx, tt.mcpServer) + if tt.expectError { + assert.Error(t, err) + if tt.expectErrContains != "" { + assert.ErrorContains(t, err, tt.expectErrContains) + } + } else { + assert.NoError(t, err) + } + + if tt.expectHash != "" { + assert.Equal(t, tt.expectHash, tt.mcpServer.Status.AuthzConfigHash) + } + if tt.expectHashCleared { + assert.Empty(t, tt.mcpServer.Status.AuthzConfigHash) + } + + cond := meta.FindStatusCondition(tt.mcpServer.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + if tt.expectConditionGone { + assert.Nil(t, cond, "AuthzConfigRefValidated condition must be cleared when the ref is removed") + } + if tt.expectConditionStatus != nil { + require.NotNil(t, cond, "expected AuthzConfigRefValidated condition") + assert.Equal(t, *tt.expectConditionStatus, cond.Status) + assert.Equal(t, tt.expectConditionReason, cond.Reason) + } + }) + } +} + +// TestMCPServerReconciler_handleAuthzConfig_Transitions exercises the stateful +// bookkeeping (needsUpdate from the prior condition, and recovery) that the +// static single-state cases above do not: valid -> invalid -> valid. +func TestMCPServerReconciler_handleAuthzConfig_Transitions(t *testing.T) { + t.Parallel() + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + authzConfig := authzConfigForTest("cfg", true, "h1") + server := &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: "s", Namespace: "default"}, + Spec: mcpv1beta1.MCPServerSpec{ + Image: "img", + AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: "cfg"}, + }, + } + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(server, authzConfig). + WithStatusSubresource(&mcpv1beta1.MCPServer{}, &mcpv1beta1.MCPAuthzConfig{}). + Build() + r := newTestMCPServerReconciler(fakeClient, scheme, kubernetes.PlatformKubernetes) + + condStatus := func() *metav1.Condition { + return meta.FindStatusCondition(server.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + } + + // valid -> condition True + hash tracked + require.NoError(t, r.handleAuthzConfig(ctx, server)) + require.NotNil(t, condStatus()) + assert.Equal(t, metav1.ConditionTrue, condStatus().Status) + assert.Equal(t, "h1", server.Status.AuthzConfigHash) + + // flip the referenced config to invalid -> condition transitions to False/NotValid + authzConfig.Status.Conditions = []metav1.Condition{{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionFalse, Reason: "Invalidated", + }} + require.NoError(t, fakeClient.Status().Update(ctx, authzConfig)) + + assert.Error(t, r.handleAuthzConfig(ctx, server)) + require.NotNil(t, condStatus()) + assert.Equal(t, metav1.ConditionFalse, condStatus().Status) + assert.Equal(t, mcpv1beta1.ConditionReasonAuthzConfigRefNotValid, condStatus().Reason) + + // recover the config -> condition transitions back to True + authzConfig.Status.Conditions = []metav1.Condition{{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionTrue, Reason: "Test", + }} + require.NoError(t, fakeClient.Status().Update(ctx, authzConfig)) + + require.NoError(t, r.handleAuthzConfig(ctx, server)) + require.NotNil(t, condStatus()) + assert.Equal(t, metav1.ConditionTrue, condStatus().Status) +} diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 73b17fd0e9..5b4ae11ffc 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -138,6 +138,10 @@ const ( // authzLabelValueInline is the label value for inline authorization configuration authzLabelValueInline = "inline" + + // authzLabelValueRef is the label value for a ConfigMap materialized from a + // referenced MCPAuthzConfig (spec.authzConfigRef) + authzLabelValueRef = "ref" ) const defaultTerminationGracePeriodSeconds = int64(30) @@ -155,6 +159,7 @@ func (r *MCPServerReconciler) detectPlatform(ctx context.Context) (kubernetes.Pl // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpservers/finalizers,verbs=update // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcptoolconfigs,verbs=get;list;watch // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpoidcconfigs,verbs=get;list;watch +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpauthzconfigs,verbs=get;list;watch // +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcptelemetryconfigs,verbs=get;list;watch // +kubebuilder:rbac:groups="",resources=configmaps,verbs=create;delete;get;list;patch;update;watch // +kubebuilder:rbac:groups="",resources=services,verbs=create;delete;get;list;patch;update;watch @@ -317,6 +322,17 @@ func (r *MCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( return ctrl.Result{}, err } + // Check if MCPAuthzConfig is referenced and handle it + if err := r.handleAuthzConfig(ctx, mcpServer); err != nil { + ctxLogger.Error(err, "Failed to handle MCPAuthzConfig") + mcpServer.Status.Phase = mcpv1beta1.MCPServerPhaseFailed + setReadyCondition(mcpServer, metav1.ConditionFalse, mcpv1beta1.ConditionReasonNotReady, err.Error()) + if statusErr := r.Status().Update(ctx, mcpServer); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update MCPServer status after MCPAuthzConfig error") + } + return ctrl.Result{}, err + } + // Update the MCPServer status with the pod status if err := r.updateMCPServerStatus(ctx, mcpServer); err != nil { ctxLogger.Error(err, "Failed to update MCPServer status") @@ -1242,13 +1258,24 @@ func (r *MCPServerReconciler) deploymentForMCPServer( }) } - // Add volume mounts for authorization configuration + // Add volume mounts for authorization configuration (inline spec.authzConfig). authzVolumeMount, authzVolume := ctrlutil.GenerateAuthzVolumeConfig(m.Spec.AuthzConfig, m.Name) if authzVolumeMount != nil { volumeMounts = append(volumeMounts, *authzVolumeMount) volumes = append(volumes, *authzVolume) } + // Add the volume mount for a referenced MCPAuthzConfig (spec.authzConfigRef). + // Inline and ref are mutually exclusive (CRD XValidation) and share the + // "authz-config" volume name, so only add the ref volume when the inline one + // was not added. This keeps a hypothetical CEL regression degrading to "one + // volume wins" rather than an invalid pod spec with a duplicate volume name. + if m.Spec.AuthzConfigRef != nil && authzVolumeMount == nil { + refMount, refVolume := ctrlutil.GenerateAuthzVolumeConfigFromRef(m.Name) + volumeMounts = append(volumeMounts, *refMount) + volumes = append(volumes, *refVolume) + } + // Add OIDC CA bundle volume if configured via MCPOIDCConfigRef if m.Spec.OIDCConfigRef != nil { oidcCfg, err := ctrlutil.GetOIDCConfigForServer(ctx, r.Client, m.Namespace, m.Spec.OIDCConfigRef) @@ -2103,6 +2130,14 @@ func labelsForInlineAuthzConfig(name string) map[string]string { return labels } +// labelsForAuthzConfigRef returns the labels for ConfigMaps materialized from a +// referenced MCPAuthzConfig. +func labelsForAuthzConfigRef(name string) map[string]string { + labels := labelsForMCPServer(name) + labels[authzLabelKey] = authzLabelValueRef + return labels +} + // getToolhiveRunnerImage returns the image to use for the toolhive runner container func getToolhiveRunnerImage() string { // Get the image from the environment variable or use a default @@ -2421,6 +2456,117 @@ func setOIDCConfigRefCondition(m *mcpv1beta1.MCPServer, status metav1.ConditionS }) } +// handleAuthzConfig validates the referenced MCPAuthzConfig, tracks its hash on +// the MCPServer status, and sets the AuthzConfigRefValidated condition. When the +// ref is cleared it removes both the hash and the condition so a stale "valid" +// signal does not linger. ReferencingWorkloads on the MCPAuthzConfig is owned by +// the MCPAuthzConfig controller (#5511); this controller never writes it. +// +// Revocation semantics (fail-stale, not fail-open): if a previously-valid ref +// later becomes invalid or missing, this returns an error and Reconcile stops +// before updating the deployment, so an already-running workload keeps enforcing +// its last-applied authz policy while the MCPServer is marked Failed/Ready=False. +// It is not torn down and does not revert to no-authz. This matches the +// OIDC/ExternalAuth/Telemetry ref handlers; hard fail-closed-on-revocation would +// require a separate, product-signed-off mechanism. +func (r *MCPServerReconciler) handleAuthzConfig(ctx context.Context, m *mcpv1beta1.MCPServer) error { + if m.Spec.AuthzConfigRef == nil { + // No MCPAuthzConfig referenced: clear any stored hash and remove the + // condition so it does not remain stale-True after the ref is removed. + changed := false + if m.Status.AuthzConfigHash != "" { + m.Status.AuthzConfigHash = "" + changed = true + } + if meta.RemoveStatusCondition(&m.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) { + changed = true + } + if changed { + if err := r.Status().Update(ctx, m); err != nil { + return fmt.Errorf("failed to clear MCPAuthzConfig hash from MCPServer status: %w", err) + } + } + return nil + } + + authzConfig, err := r.fetchAndValidateAuthzConfig(ctx, m) + if err != nil { + return err + } + + prevCondition := meta.FindStatusCondition(m.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + needsUpdate := prevCondition == nil || prevCondition.Status != metav1.ConditionTrue + + setAuthzConfigRefCondition(m, metav1.ConditionTrue, + mcpv1beta1.ConditionReasonAuthzConfigRefValid, + fmt.Sprintf("MCPAuthzConfig %s is valid and ready", m.Spec.AuthzConfigRef.Name)) + + if m.Status.AuthzConfigHash != authzConfig.Status.ConfigHash { + m.Status.AuthzConfigHash = authzConfig.Status.ConfigHash + needsUpdate = true + } + + if needsUpdate { + if err := r.Status().Update(ctx, m); err != nil { + return fmt.Errorf("failed to update MCPServer status after validating MCPAuthzConfig: %w", err) + } + } + + return nil +} + +// fetchAndValidateAuthzConfig fetches the referenced MCPAuthzConfig, validates it +// is ready, and sets the appropriate failure condition on the MCPServer if not. +func (r *MCPServerReconciler) fetchAndValidateAuthzConfig( + ctx context.Context, m *mcpv1beta1.MCPServer, +) (*mcpv1beta1.MCPAuthzConfig, error) { + ctxLogger := log.FromContext(ctx) + + authzConfig, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, m.Namespace, m.Spec.AuthzConfigRef) + if err != nil { + setAuthzConfigRefCondition(m, metav1.ConditionFalse, + mcpv1beta1.ConditionReasonAuthzConfigRefNotFound, + fmt.Sprintf("MCPAuthzConfig %s not found: %v", m.Spec.AuthzConfigRef.Name, err)) + if statusErr := r.Status().Update(ctx, m); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig lookup error") + } + return nil, err + } + + if authzConfig == nil { + setAuthzConfigRefCondition(m, metav1.ConditionFalse, + mcpv1beta1.ConditionReasonAuthzConfigRefNotFound, + fmt.Sprintf("MCPAuthzConfig %s not found", m.Spec.AuthzConfigRef.Name)) + if statusErr := r.Status().Update(ctx, m); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig not found") + } + return nil, fmt.Errorf("MCPAuthzConfig %s not found", m.Spec.AuthzConfigRef.Name) + } + + if err := ctrlutil.ValidateAuthzConfigReady(authzConfig); err != nil { + setAuthzConfigRefCondition(m, metav1.ConditionFalse, + mcpv1beta1.ConditionReasonAuthzConfigRefNotValid, + fmt.Sprintf("MCPAuthzConfig %s is not valid: %v", m.Spec.AuthzConfigRef.Name, err)) + if statusErr := r.Status().Update(ctx, m); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig validation check") + } + return nil, err + } + + return authzConfig, nil +} + +// setAuthzConfigRefCondition sets the AuthzConfigRefValidated status condition +func setAuthzConfigRefCondition(m *mcpv1beta1.MCPServer, status metav1.ConditionStatus, reason, message string) { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionAuthzConfigRefValidated, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: m.Generation, + }) +} + // handleWebhookConfig validates and tracks the hash of the referenced MCPWebhookConfig. func (r *MCPServerReconciler) handleWebhookConfig(ctx context.Context, m *mcpv1beta1.MCPServer) error { ctxLogger := log.FromContext(ctx) @@ -2463,10 +2609,26 @@ func (r *MCPServerReconciler) handleWebhookConfig(ctx context.Context, m *mcpv1b return nil } -// ensureAuthzConfigMap ensures the authorization ConfigMap exists for inline configuration +// ensureAuthzConfigMap ensures the authorization ConfigMap exists for inline +// configuration (spec.authzConfig) and for a referenced MCPAuthzConfig +// (spec.authzConfigRef). The two are mutually exclusive (CRD XValidation), so at +// most one ConfigMap is materialized per reconcile. func (r *MCPServerReconciler) ensureAuthzConfigMap(ctx context.Context, m *mcpv1beta1.MCPServer) error { - return ctrlutil.EnsureAuthzConfigMap( + if err := ctrlutil.EnsureAuthzConfigMap( ctx, r.Client, r.Scheme, m, m.Namespace, m.Name, m.Spec.AuthzConfig, labelsForInlineAuthzConfig(m.Name), + ); err != nil { + return err + } + + if m.Spec.AuthzConfigRef == nil { + return nil + } + authzConfig, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, m.Namespace, m.Spec.AuthzConfigRef) + if err != nil { + return err + } + return ctrlutil.EnsureAuthzConfigMapFromRef( + ctx, r.Client, r.Scheme, m, m.Namespace, m.Name, authzConfig, labelsForAuthzConfigRef(m.Name), ) } @@ -2632,6 +2794,38 @@ func (r *MCPServerReconciler) validateRateLimitConfig(ctx context.Context, mcpSe } } +// mapAuthzConfigToServers maps MCPAuthzConfig changes to reconciliation requests +// for the MCPServers that reference it via spec.authzConfigRef. +func (r *MCPServerReconciler) mapAuthzConfigToServers( + ctx context.Context, obj client.Object, +) []reconcile.Request { + authzConfig, ok := obj.(*mcpv1beta1.MCPAuthzConfig) + if !ok { + return nil + } + + mcpServerList := &mcpv1beta1.MCPServerList{} + if err := r.List(ctx, mcpServerList, client.InNamespace(authzConfig.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list MCPServers for MCPAuthzConfig watch") + return nil + } + + var requests []reconcile.Request + for _, server := range mcpServerList.Items { + if server.Spec.AuthzConfigRef != nil && + server.Spec.AuthzConfigRef.Name == authzConfig.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: server.Name, + Namespace: server.Namespace, + }, + }) + } + } + + return requests +} + // mapWebhookConfigToServers maps MCPWebhookConfig changes to MCPServer reconciliation requests. func (r *MCPServerReconciler) mapWebhookConfigToServers( ctx context.Context, obj client.Object, @@ -2733,6 +2927,9 @@ func (r *MCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { }, ) + // Create a handler that maps MCPAuthzConfig changes to MCPServer reconciliation requests + authzConfigHandler := handler.EnqueueRequestsFromMapFunc(r.mapAuthzConfigToServers) + telemetryConfigHandler := handler.EnqueueRequestsFromMapFunc(r.mapTelemetryConfigToServers) webhookConfigHandler := handler.EnqueueRequestsFromMapFunc(r.mapWebhookConfigToServers) @@ -2742,6 +2939,7 @@ func (r *MCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { Owns(&corev1.Service{}). Watches(&mcpv1beta1.MCPExternalAuthConfig{}, externalAuthConfigHandler). Watches(&mcpv1beta1.MCPOIDCConfig{}, oidcConfigHandler). + Watches(&mcpv1beta1.MCPAuthzConfig{}, authzConfigHandler). Watches(&mcpv1beta1.MCPTelemetryConfig{}, telemetryConfigHandler). Watches(&mcpv1alpha1.MCPWebhookConfig{}, webhookConfigHandler). Complete(r) diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig.go b/cmd/thv-operator/controllers/mcpserver_runconfig.go index cdd19bf373..7bb2d785d7 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig.go @@ -204,6 +204,13 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1beta1.MCPServ return nil, fmt.Errorf("failed to process AuthzConfig: %w", err) } + // Resolve a referenced MCPAuthzConfig (spec.authzConfigRef) into runtime authz. + // Mutually exclusive with the inline spec.authzConfig handled above. Backend- + // agnostic: the proxy runner's authz factory dispatches on type (cedarv1, httpv1). + if err := ctrlutil.AddAuthzConfigRefOptions(ctx, r.Client, m.Namespace, m.Spec.AuthzConfigRef, &options); err != nil { + return nil, fmt.Errorf("failed to process AuthzConfigRef: %w", err) + } + // Resolve OIDC configuration from either legacy OIDCConfig or new MCPOIDCConfigRef. // Resolve once and reuse for both RunConfig options and embedded auth server config. var resolvedOIDCConfig *oidc.OIDCConfig diff --git a/cmd/thv-operator/pkg/controllerutil/authz_ref.go b/cmd/thv-operator/pkg/controllerutil/authz_ref.go index 12c4fe9927..d2b3a24c9b 100644 --- a/cmd/thv-operator/pkg/controllerutil/authz_ref.go +++ b/cmd/thv-operator/pkg/controllerutil/authz_ref.go @@ -208,6 +208,13 @@ func EnsureAuthzConfigMapFromRef( // GenerateAuthzVolumeConfigFromRef returns the volume mount + volume for the // ConfigMap materialized by EnsureAuthzConfigMapFromRef, mirroring // GenerateAuthzVolumeConfig for the inline path. +// +// Note: in operator-managed deployments this mounted file is NOT the active +// enforcement path — the proxy builds its authz middleware from the authz config +// embedded in the RunConfig (via AddAuthzConfigRefOptions → WithAuthzConfig). The +// mount exists for parity with the inline path and for potential future +// file-based consumption (config.AuthzConfigPath); the operator does not set +// that path today. func GenerateAuthzVolumeConfigFromRef(resourceName string) (*corev1.VolumeMount, *corev1.Volume) { volumeMount := &corev1.VolumeMount{ Name: "authz-config", diff --git a/cmd/thv-operator/test-integration/mcp-server/mcpserver_authzconfigref_integration_test.go b/cmd/thv-operator/test-integration/mcp-server/mcpserver_authzconfigref_integration_test.go new file mode 100644 index 0000000000..ab1d888e3a --- /dev/null +++ b/cmd/thv-operator/test-integration/mcp-server/mcpserver_authzconfigref_integration_test.go @@ -0,0 +1,187 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" +) + +// The MCPAuthzConfig controller is not registered in this suite, so we pre-seed +// the config's Valid condition + ConfigHash directly; the MCPServer controller +// (which is registered) only reads them. +var _ = Describe("MCPServer AuthzConfigRef Integration Tests", func() { + const ( + timeout = time.Second * 30 + interval = time.Millisecond * 250 + ) + + // seedAuthzConfig creates an MCPAuthzConfig and stamps its status (Valid + // condition + ConfigHash) as the MCPAuthzConfig controller would. + seedAuthzConfig := func(name, namespace, typ, rawConfig, hash string, valid bool) *mcpv1beta1.MCPAuthzConfig { + cfg := &mcpv1beta1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: mcpv1beta1.MCPAuthzConfigSpec{ + Type: typ, + Config: runtime.RawExtension{Raw: []byte(rawConfig)}, + }, + } + Expect(k8sClient.Create(ctx, cfg)).To(Succeed()) + + status := metav1.ConditionFalse + if valid { + status = metav1.ConditionTrue + } + cfg.Status.ConfigHash = hash + meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, + Status: status, + Reason: "Test", + Message: "seeded by integration test", + }) + Expect(k8sClient.Status().Update(ctx, cfg)).To(Succeed()) + return cfg + } + + newServer := func(name, namespace, authzRefName string) *mcpv1beta1.MCPServer { + return &mcpv1beta1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: namespace}, + Spec: mcpv1beta1.MCPServerSpec{ + Image: "example/mcp-server:v1.0.0", + Transport: "streamable-http", + AuthzConfigRef: &mcpv1beta1.MCPAuthzConfigReference{Name: authzRefName}, + }, + } + } + + const ( + cedarConfig = `{"policies":["permit(principal, action, resource);"],"entities_json":"[]"}` + httpConfig = `{"http":{"url":"https://pdp.example.com"},"claim_mapping":"standard"}` + ) + + DescribeTable("a valid referenced MCPAuthzConfig is validated and hash-tracked, for any backend", + func(nsName, cfgName, srvName, typ, rawConfig string) { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: nsName}} + _ = k8sClient.Create(ctx, ns) + + seedAuthzConfig(cfgName, nsName, typ, rawConfig, "hash-1", true) + Expect(k8sClient.Create(ctx, newServer(srvName, nsName, cfgName))).To(Succeed()) + + By("setting AuthzConfigRefValidated=True and tracking the hash") + Eventually(func(g Gomega) { + var got mcpv1beta1.MCPServer + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName, Namespace: nsName}, &got)).To(Succeed()) + cond := meta.FindStatusCondition(got.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionTrue)) + g.Expect(got.Status.AuthzConfigHash).To(Equal("hash-1")) + }, timeout, interval).Should(Succeed()) + + By("materializing the authz ConfigMap the proxy mounts (any backend)") + Eventually(func(g Gomega) { + var cm corev1.ConfigMap + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName + "-authz-ref", Namespace: nsName}, &cm)).To(Succeed()) + g.Expect(cm.Data).To(HaveKey("authz.json")) + g.Expect(cm.Data["authz.json"]).To(ContainSubstring(typ)) + }, timeout, interval).Should(Succeed()) + }, + Entry("cedarv1", "authzref-cedar", "authz-cedar", "srv-cedar", "cedarv1", cedarConfig), + Entry("httpv1", "authzref-http", "authz-http", "srv-http", "httpv1", httpConfig), + ) + + Context("when the referenced MCPAuthzConfig changes", Ordered, func() { + const ( + namespace = "authzref-watch" + cfgName = "authz-watch" + srvName = "srv-watch" + ) + BeforeAll(func() { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + _ = k8sClient.Create(ctx, ns) + seedAuthzConfig(cfgName, namespace, "cedarv1", cedarConfig, "hash-1", true) + Expect(k8sClient.Create(ctx, newServer(srvName, namespace, cfgName))).To(Succeed()) + }) + + It("reflects a config hash change on the referencing MCPServer", func() { + Eventually(func(g Gomega) { + var got mcpv1beta1.MCPServer + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName, Namespace: namespace}, &got)).To(Succeed()) + g.Expect(got.Status.AuthzConfigHash).To(Equal("hash-1")) + }, timeout, interval).Should(Succeed()) + + By("bumping the config hash") + var cfg mcpv1beta1.MCPAuthzConfig + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cfgName, Namespace: namespace}, &cfg)).To(Succeed()) + cfg.Status.ConfigHash = "hash-2" + meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionTrue, Reason: "Test", + }) + Expect(k8sClient.Status().Update(ctx, &cfg)).To(Succeed()) + + // The MCPServer controller watches MCPAuthzConfig, so the change is + // picked up without an external nudge. (This asserts the observable + // outcome; it does not attempt to prove the watch is the sole trigger.) + By("observing the MCPServer eventually reflect the new hash") + Eventually(func(g Gomega) { + var got mcpv1beta1.MCPServer + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName, Namespace: namespace}, &got)).To(Succeed()) + g.Expect(got.Status.AuthzConfigHash).To(Equal("hash-2")) + }, timeout, interval).Should(Succeed()) + }) + + It("transitions AuthzConfigRefValidated to False when the config becomes invalid", func() { + By("flagging the referenced config invalid") + var cfg mcpv1beta1.MCPAuthzConfig + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: cfgName, Namespace: namespace}, &cfg)).To(Succeed()) + meta.SetStatusCondition(&cfg.Status.Conditions, metav1.Condition{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, Status: metav1.ConditionFalse, Reason: "Invalidated", + }) + Expect(k8sClient.Status().Update(ctx, &cfg)).To(Succeed()) + + By("observing the MCPServer condition flip to False/NotValid") + Eventually(func(g Gomega) { + var got mcpv1beta1.MCPServer + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName, Namespace: namespace}, &got)).To(Succeed()) + cond := meta.FindStatusCondition(got.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(mcpv1beta1.ConditionReasonAuthzConfigRefNotValid)) + }, timeout, interval).Should(Succeed()) + }) + }) + + Context("when the referenced MCPAuthzConfig is not valid", Ordered, func() { + const ( + namespace = "authzref-invalid" + cfgName = "authz-invalid" + srvName = "srv-invalid" + ) + BeforeAll(func() { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}} + _ = k8sClient.Create(ctx, ns) + seedAuthzConfig(cfgName, namespace, "cedarv1", cedarConfig, "", false) + Expect(k8sClient.Create(ctx, newServer(srvName, namespace, cfgName))).To(Succeed()) + }) + + It("sets AuthzConfigRefValidated=False with reason NotValid", func() { + Eventually(func(g Gomega) { + var got mcpv1beta1.MCPServer + g.Expect(k8sClient.Get(ctx, types.NamespacedName{Name: srvName, Namespace: namespace}, &got)).To(Succeed()) + cond := meta.FindStatusCondition(got.Status.Conditions, mcpv1beta1.ConditionAuthzConfigRefValidated) + g.Expect(cond).NotTo(BeNil()) + g.Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + g.Expect(cond.Reason).To(Equal(mcpv1beta1.ConditionReasonAuthzConfigRefNotValid)) + }, timeout, interval).Should(Succeed()) + }) + }) +})