diff --git a/cmd/thv-operator/api/v1beta1/mcpremoteproxy_types.go b/cmd/thv-operator/api/v1beta1/mcpremoteproxy_types.go index 2a3aa6e6e9..53111c97af 100644 --- a/cmd/thv-operator/api/v1beta1/mcpremoteproxy_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpremoteproxy_types.go @@ -244,6 +244,10 @@ type MCPRemoteProxyStatus struct { // +optional AuthServerConfigHash string `json:"authServerConfigHash,omitempty"` + // AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection + // +optional + AuthzConfigHash string `json:"authzConfigHash,omitempty"` + // OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection // +optional OIDCConfigHash string `json:"oidcConfigHash,omitempty"` diff --git a/cmd/thv-operator/api/v1beta1/mcpserver_types.go b/cmd/thv-operator/api/v1beta1/mcpserver_types.go index d8bf1a151b..ce7161c203 100644 --- a/cmd/thv-operator/api/v1beta1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1beta1/mcpserver_types.go @@ -58,6 +58,9 @@ const ( const ( // ConditionOIDCConfigRefValidated indicates whether the OIDCConfigRef is valid ConditionOIDCConfigRefValidated = "OIDCConfigRefValidated" + + // ConditionAuthzConfigRefValidated indicates whether the AuthzConfigRef is valid + ConditionAuthzConfigRefValidated = "AuthzConfigRefValidated" ) const ( @@ -74,6 +77,20 @@ const ( ConditionReasonOIDCConfigRefError = "OIDCConfigRefError" ) +const ( + // ConditionReasonAuthzConfigRefValid indicates the referenced MCPAuthzConfig is valid and ready + ConditionReasonAuthzConfigRefValid = "AuthzConfigRefValid" + + // ConditionReasonAuthzConfigRefNotFound indicates the referenced MCPAuthzConfig was not found + ConditionReasonAuthzConfigRefNotFound = "AuthzConfigRefNotFound" + + // ConditionReasonAuthzConfigRefNotValid indicates the referenced MCPAuthzConfig is not valid + ConditionReasonAuthzConfigRefNotValid = "AuthzConfigRefNotValid" + + // ConditionReasonAuthzConfigRefError indicates an error occurred validating the AuthzConfigRef + ConditionReasonAuthzConfigRefError = "AuthzConfigRefError" +) + const ( // ConditionReasonCABundleRefValid indicates the CABundleRef is valid and the ConfigMap exists ConditionReasonCABundleRefValid = "CABundleRefValid" @@ -898,6 +915,10 @@ type MCPServerStatus struct { // +optional AuthServerConfigHash string `json:"authServerConfigHash,omitempty"` + // AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection + // +optional + AuthzConfigHash string `json:"authzConfigHash,omitempty"` + // OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection // +optional OIDCConfigHash string `json:"oidcConfigHash,omitempty"` diff --git a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go index 41666e024f..4456060d79 100644 --- a/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go +++ b/cmd/thv-operator/api/v1beta1/virtualmcpserver_types.go @@ -294,6 +294,11 @@ type VirtualMCPServerStatus struct { // +optional BackendCount int32 `json:"backendCount,omitempty"` + // AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection. + // Only populated when IncomingAuth.AuthzConfigRef is set. + // +optional + AuthzConfigHash string `json:"authzConfigHash,omitempty"` + // OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection. // Only populated when IncomingAuth.OIDCConfigRef is set. // +optional diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go index fbbfa4e872..4707cb7d26 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go @@ -25,7 +25,6 @@ import ( mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" - "github.com/stacklok/toolhive/pkg/authz/authorizers" ) const ( @@ -34,9 +33,6 @@ const ( // authzConfigRequeueDelay is the delay before requeuing after adding a finalizer authzConfigRequeueDelay = 500 * time.Millisecond - - // authzConfigVersion is the default version for reconstructed authz configs - authzConfigVersion = "1.0" ) // MCPAuthzConfigReconciler reconciles a MCPAuthzConfig object. @@ -208,48 +204,17 @@ func (*MCPAuthzConfigReconciler) validateAuthzConfig(authzConfig *mcpv1beta1.MCP return err } - // buildFullAuthzConfigJSON returns the registered factory alongside the + // BuildFullAuthzConfigJSON returns the registered factory alongside the // envelope, so we don't have to re-Unmarshal the JSON or re-look-up the - // factory just to dispatch ValidateConfig. - fullConfigJSON, factory, err := buildFullAuthzConfigJSON(authzConfig.Spec) + // factory just to dispatch ValidateConfig. It lives in controllerutil so the + // workload controllers can reuse it without an import cycle. + fullConfigJSON, factory, err := ctrlutil.BuildFullAuthzConfigJSON(authzConfig.Spec) if err != nil { return err } return factory.ValidateConfig(fullConfigJSON) } -// buildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a -// MCPAuthzConfig spec and returns it alongside the resolved factory. The JSON -// shape is the same one accepted by authorizers.Config and stored in -// ConfigMaps: {"version": "1.0", "type": "", "": {}}. -// Returning the factory together with the JSON lets callers (notably -// validateAuthzConfig) skip a second registry lookup. -func buildFullAuthzConfigJSON(spec mcpv1beta1.MCPAuthzConfigSpec) ([]byte, authorizers.AuthorizerFactory, error) { - factory := authorizers.GetFactory(spec.Type) - if factory == nil { - return nil, nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", - spec.Type, authorizers.RegisteredTypes()) - } - - if len(spec.Config.Raw) == 0 { - return nil, nil, fmt.Errorf("config field is empty") - } - - versionJSON, _ := json.Marshal(authzConfigVersion) - typeJSON, _ := json.Marshal(spec.Type) - fullConfig := map[string]json.RawMessage{ - "version": versionJSON, - "type": typeJSON, - factory.ConfigKey(): spec.Config.Raw, - } - - result, err := json.Marshal(fullConfig) - if err != nil { - return nil, nil, fmt.Errorf("failed to marshal full authz config: %w", err) - } - return result, factory, nil -} - // canonicalizeSpecForHash returns a copy of spec whose Config.Raw has been // re-marshalled into canonical JSON form (sorted keys, no extra whitespace). // The returned value is suitable for ctrlutil.CalculateConfigHash and produces diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go index ad8ffaa54f..501e99483c 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go @@ -4,7 +4,6 @@ package controllers import ( - "encoding/json" "testing" "time" @@ -56,96 +55,6 @@ func newAuthzTestReconciler(t *testing.T, objs ...client.Object) (*MCPAuthzConfi return &MCPAuthzConfigReconciler{Client: fakeClient, Scheme: scheme}, fakeClient } -func Test_buildFullAuthzConfigJSON(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - spec mcpv1beta1.MCPAuthzConfigSpec - expectError bool - expectType string - expectKey string - expectVersion string - }{ - { - name: "valid Cedar config produces correct JSON", - spec: mcpv1beta1.MCPAuthzConfigSpec{ - Type: "cedarv1", - Config: validCedarConfig(), - }, - expectType: "cedarv1", - expectKey: "cedar", - expectVersion: "1.0", - }, - { - name: "valid HTTP PDP config produces correct JSON", - spec: mcpv1beta1.MCPAuthzConfigSpec{ - Type: "httpv1", - Config: validHTTPPDPConfig(), - }, - expectType: "httpv1", - expectKey: "pdp", - expectVersion: "1.0", - }, - { - name: "unknown type returns error", - spec: mcpv1beta1.MCPAuthzConfigSpec{ - Type: "unknown-type", - Config: runtime.RawExtension{Raw: []byte(`{}`)}, - }, - expectError: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Parallel() - - result, factory, err := buildFullAuthzConfigJSON(tt.spec) - - if tt.expectError { - require.Error(t, err) - assert.Nil(t, factory, "factory must be nil when buildFullAuthzConfigJSON errors") - return - } - - require.NoError(t, err) - require.NotNil(t, factory, "factory must accompany a successful build") - assert.Equal(t, tt.expectKey, factory.ConfigKey(), - "returned factory's ConfigKey must match the nested envelope key") - - var parsed map[string]json.RawMessage - require.NoError(t, json.Unmarshal(result, &parsed), "Output must be valid JSON") - - var version string - require.NoError(t, json.Unmarshal(parsed["version"], &version)) - assert.Equal(t, tt.expectVersion, version) - - var typ string - require.NoError(t, json.Unmarshal(parsed["type"], &typ)) - assert.Equal(t, tt.expectType, typ) - - _, hasKey := parsed[tt.expectKey] - assert.True(t, hasKey, "Output JSON should contain key %q", tt.expectKey) - }) - } -} - -func Test_buildFullAuthzConfigJSON_EmptyConfigRaw(t *testing.T) { - t.Parallel() - - spec := mcpv1beta1.MCPAuthzConfigSpec{ - Type: "cedarv1", - Config: runtime.RawExtension{Raw: []byte{}}, - } - - result, factory, err := buildFullAuthzConfigJSON(spec) - require.Error(t, err) - assert.Nil(t, result) - assert.Nil(t, factory, "factory must be nil when buildFullAuthzConfigJSON errors") - assert.Contains(t, err.Error(), "config field is empty") -} - func TestCanonicalizeSpecForHash(t *testing.T) { t.Parallel() diff --git a/cmd/thv-operator/pkg/controllerutil/authz_ref.go b/cmd/thv-operator/pkg/controllerutil/authz_ref.go new file mode 100644 index 0000000000..12c4fe9927 --- /dev/null +++ b/cmd/thv-operator/pkg/controllerutil/authz_ref.go @@ -0,0 +1,231 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllerutil + +import ( + "context" + "encoding/json" + "fmt" + + 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" + "sigs.k8s.io/controller-runtime/pkg/client" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + "github.com/stacklok/toolhive/cmd/thv-operator/pkg/kubernetes/configmaps" + "github.com/stacklok/toolhive/pkg/authz" + "github.com/stacklok/toolhive/pkg/authz/authorizers" + "github.com/stacklok/toolhive/pkg/runner" +) + +// authzRefConfigMapName is the name of the ConfigMap materialized from a +// referenced MCPAuthzConfig for a given workload. The "-authz-ref" suffix keeps +// it distinct from the inline path's "-authz-inline" ConfigMap so the two +// ConfigMaps never overwrite each other. +// +// The volume name and mount path ("authz-config", "/etc/toolhive/authz") are +// deliberately shared with the inline GenerateAuthzVolumeConfig: a workload +// mounts at most one authz volume because spec.authzConfig and +// spec.authzConfigRef are mutually exclusive (enforced by CRD XValidation), so +// the proxy reads authz.json from the same path regardless of the source. The +// shared volume name is therefore intentional, not a collision risk. +func authzRefConfigMapName(resourceName string) string { + return fmt.Sprintf("%s-authz-ref", resourceName) +} + +// BuildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a +// MCPAuthzConfig spec and returns it alongside the resolved factory. The JSON +// shape is the one accepted by authz.Config / authorizers.Config and stored in +// ConfigMaps: {"version": "1.0", "type": "", "": {}}. +// It is backend-agnostic — the per-backend key is supplied by the factory's +// ConfigKey() (e.g. "cedar" for cedarv1, "pdp" for httpv1) — so both the +// MCPAuthzConfig controller and the workload controllers can use it without +// special-casing the backend. Returning the factory together with the JSON lets +// callers skip a second registry lookup when they also need to validate. +func BuildFullAuthzConfigJSON(spec mcpv1beta1.MCPAuthzConfigSpec) ([]byte, authorizers.AuthorizerFactory, error) { + factory := authorizers.GetFactory(spec.Type) + if factory == nil { + return nil, nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", + spec.Type, authorizers.RegisteredTypes()) + } + + if len(spec.Config.Raw) == 0 { + return nil, nil, fmt.Errorf("config field is empty") + } + + // Marshaling a string constant and a plain string field cannot fail. + versionJSON, _ := json.Marshal(AuthzConfigVersion) + typeJSON, _ := json.Marshal(spec.Type) + fullConfig := map[string]json.RawMessage{ + "version": versionJSON, + "type": typeJSON, + factory.ConfigKey(): spec.Config.Raw, + } + + result, err := json.Marshal(fullConfig) + if err != nil { + return nil, nil, fmt.Errorf("failed to marshal full authz config: %w", err) + } + return result, factory, nil +} + +// GetAuthzConfigForWorkload fetches the MCPAuthzConfig referenced by ref in the +// given namespace. Returns (nil, nil) when ref is nil so callers can invoke it +// unconditionally. Mirrors GetOIDCConfigForServer. +func GetAuthzConfigForWorkload( + ctx context.Context, + c client.Client, + namespace string, + ref *mcpv1beta1.MCPAuthzConfigReference, +) (*mcpv1beta1.MCPAuthzConfig, error) { + if ref == nil { + return nil, nil + } + var authzConfig mcpv1beta1.MCPAuthzConfig + if err := c.Get(ctx, types.NamespacedName{Namespace: namespace, Name: ref.Name}, &authzConfig); err != nil { + return nil, fmt.Errorf("failed to get MCPAuthzConfig %s/%s: %w", namespace, ref.Name, err) + } + return &authzConfig, nil +} + +// ValidateAuthzConfigReady returns an error unless the referenced config's +// ConditionTypeAuthzConfigValid condition is True — i.e. the MCPAuthzConfig +// controller has validated the spec. A workload must not consume a config that +// its owning controller has flagged invalid. +func ValidateAuthzConfigReady(authzConfig *mcpv1beta1.MCPAuthzConfig) error { + if authzConfig == nil { + return fmt.Errorf("authz config is nil") + } + if !meta.IsStatusConditionTrue(authzConfig.Status.Conditions, mcpv1beta1.ConditionTypeAuthzConfigValid) { + return fmt.Errorf("MCPAuthzConfig %s/%s is not valid (condition %q is not True)", + authzConfig.Namespace, authzConfig.Name, mcpv1beta1.ConditionTypeAuthzConfigValid) + } + return nil +} + +// buildAuthzConfigFromRef builds a validated *authz.Config from a referenced +// MCPAuthzConfig, backend-agnostically (cedarv1, httpv1, ...). The resulting +// config is safe to embed into a RunConfig via runner.WithAuthzConfig. +func buildAuthzConfigFromRef(authzConfig *mcpv1beta1.MCPAuthzConfig) (*authz.Config, error) { + data, _, err := BuildFullAuthzConfigJSON(authzConfig.Spec) + if err != nil { + return nil, err + } + var cfg authz.Config + if err := json.Unmarshal(data, &cfg); err != nil { + return nil, fmt.Errorf("failed to parse authz config from MCPAuthzConfig %s/%s: %w", + authzConfig.Namespace, authzConfig.Name, err) + } + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("invalid authz config from MCPAuthzConfig %s/%s: %w", + authzConfig.Namespace, authzConfig.Name, err) + } + return &cfg, nil +} + +// AddAuthzConfigRefOptions resolves the referenced MCPAuthzConfig into an +// authz.Config (any registered backend) and appends runner.WithAuthzConfig. +// No-op when ref is nil. Parallel to AddAuthzConfigOptions for the inline +// spec.authzConfig path. The referenced config must exist and be Valid. +func AddAuthzConfigRefOptions( + ctx context.Context, + c client.Client, + namespace string, + ref *mcpv1beta1.MCPAuthzConfigReference, + options *[]runner.RunConfigBuilderOption, +) error { + if ref == nil { + return nil + } + authzConfig, err := GetAuthzConfigForWorkload(ctx, c, namespace, ref) + if err != nil { + return err + } + if err := ValidateAuthzConfigReady(authzConfig); err != nil { + return err + } + cfg, err := buildAuthzConfigFromRef(authzConfig) + if err != nil { + return err + } + *options = append(*options, runner.WithAuthzConfig(cfg)) + return nil +} + +// EnsureAuthzConfigMapFromRef materializes the referenced MCPAuthzConfig's +// backend config into a ConfigMap (key DefaultAuthzKey) owned by the workload, +// so the proxy can read it from a mounted volume — mirroring EnsureAuthzConfigMap +// for the inline path. No-op when authzConfig is nil. +func EnsureAuthzConfigMapFromRef( + ctx context.Context, + c client.Client, + scheme *runtime.Scheme, + owner client.Object, + namespace string, + resourceName string, + authzConfig *mcpv1beta1.MCPAuthzConfig, + labels map[string]string, +) error { + if authzConfig == nil { + return nil + } + + // Defense in depth: the workload controller validates readiness before + // materializing, but guard here too so this exported helper never writes a + // ConfigMap from a config its owning controller has flagged invalid. + if err := ValidateAuthzConfigReady(authzConfig); err != nil { + return err + } + + data, _, err := BuildFullAuthzConfigJSON(authzConfig.Spec) + if err != nil { + return fmt.Errorf("failed to build authz config from MCPAuthzConfig %s/%s: %w", + authzConfig.Namespace, authzConfig.Name, err) + } + + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: authzRefConfigMapName(resourceName), + Namespace: namespace, + Labels: labels, + }, + Data: map[string]string{ + DefaultAuthzKey: string(data), + }, + } + + configMapsClient := configmaps.NewClient(c, scheme) + if _, err := configMapsClient.UpsertWithOwnerReference(ctx, configMap, owner); err != nil { + return fmt.Errorf("failed to upsert authz-ref ConfigMap: %w", err) + } + return nil +} + +// GenerateAuthzVolumeConfigFromRef returns the volume mount + volume for the +// ConfigMap materialized by EnsureAuthzConfigMapFromRef, mirroring +// GenerateAuthzVolumeConfig for the inline path. +func GenerateAuthzVolumeConfigFromRef(resourceName string) (*corev1.VolumeMount, *corev1.Volume) { + volumeMount := &corev1.VolumeMount{ + Name: "authz-config", + MountPath: "/etc/toolhive/authz", + ReadOnly: true, + } + volume := &corev1.Volume{ + Name: "authz-config", + VolumeSource: corev1.VolumeSource{ + ConfigMap: &corev1.ConfigMapVolumeSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: authzRefConfigMapName(resourceName), + }, + Items: []corev1.KeyToPath{ + {Key: DefaultAuthzKey, Path: DefaultAuthzKey}, + }, + }, + }, + } + return volumeMount, volume +} diff --git a/cmd/thv-operator/pkg/controllerutil/authz_ref_test.go b/cmd/thv-operator/pkg/controllerutil/authz_ref_test.go new file mode 100644 index 0000000000..96ae897f93 --- /dev/null +++ b/cmd/thv-operator/pkg/controllerutil/authz_ref_test.go @@ -0,0 +1,237 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllerutil + +import ( + "context" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + mcpv1beta1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1beta1" + // Blank-imported so the cedarv1 and httpv1 authorizer factories register + // themselves; BuildFullAuthzConfigJSON / AddAuthzConfigRefOptions resolve + // the backend via the authorizers registry. + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/cedar" + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/http" + "github.com/stacklok/toolhive/pkg/runner" +) + +// cedarRefConfig is the backend-specific config blob a cedarv1 MCPAuthzConfig holds. +const cedarRefConfig = `{"policies":["permit(principal, action, resource);"],"entities_json":"[]"}` + +// httpRefConfig is the backend-specific config blob an httpv1 MCPAuthzConfig holds. +const httpRefConfig = `{"http":{"url":"https://pdp.example.com"},"claim_mapping":"standard"}` + +func authzScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + require.NoError(t, mcpv1beta1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + return scheme +} + +func newAuthzConfig(name, typ, rawConfig string, valid bool) *mcpv1beta1.MCPAuthzConfig { + cfg := &mcpv1beta1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "default"}, + Spec: mcpv1beta1.MCPAuthzConfigSpec{ + Type: typ, + Config: runtime.RawExtension{Raw: []byte(rawConfig)}, + }, + } + status := metav1.ConditionFalse + if valid { + status = metav1.ConditionTrue + } + cfg.Status.Conditions = []metav1.Condition{{ + Type: mcpv1beta1.ConditionTypeAuthzConfigValid, + Status: status, + Reason: "Test", + }} + return cfg +} + +func TestBuildFullAuthzConfigJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + spec mcpv1beta1.MCPAuthzConfigSpec + wantErr bool + wantType string + wantKey string // the backend config key the envelope must carry + }{ + { + name: "cedarv1", + spec: mcpv1beta1.MCPAuthzConfigSpec{Type: "cedarv1", Config: runtime.RawExtension{Raw: []byte(cedarRefConfig)}}, + wantType: "cedarv1", + wantKey: "cedar", + }, + { + name: "httpv1", + spec: mcpv1beta1.MCPAuthzConfigSpec{Type: "httpv1", Config: runtime.RawExtension{Raw: []byte(httpRefConfig)}}, + wantType: "httpv1", + wantKey: "pdp", + }, + { + name: "unregistered type", + spec: mcpv1beta1.MCPAuthzConfigSpec{Type: "nopev1", Config: runtime.RawExtension{Raw: []byte(`{}`)}}, + wantErr: true, + }, + { + name: "empty config", + spec: mcpv1beta1.MCPAuthzConfigSpec{Type: "cedarv1"}, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + data, factory, err := BuildFullAuthzConfigJSON(tt.spec) + if tt.wantErr { + assert.Error(t, err) + assert.Nil(t, data, "data must be nil on error") + assert.Nil(t, factory, "factory must be nil on error") + return + } + require.NoError(t, err) + require.NotNil(t, factory) + + var envelope map[string]json.RawMessage + require.NoError(t, json.Unmarshal(data, &envelope)) + assert.JSONEq(t, `"`+tt.wantType+`"`, string(envelope["type"])) + assert.JSONEq(t, `"`+AuthzConfigVersion+`"`, string(envelope["version"]), "envelope version must be AuthzConfigVersion") + assert.Contains(t, envelope, tt.wantKey, "envelope must carry the backend config under its ConfigKey") + }) + } +} + +func TestGetAuthzConfigForWorkload(t *testing.T) { + t.Parallel() + ctx := context.Background() + scheme := authzScheme(t) + + cfg := newAuthzConfig("authz", "cedarv1", cedarRefConfig, true) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cfg).Build() + + t.Run("nil ref returns nil", func(t *testing.T) { + t.Parallel() + got, err := GetAuthzConfigForWorkload(ctx, c, "default", nil) + require.NoError(t, err) + assert.Nil(t, got) + }) + t.Run("found", func(t *testing.T) { + t.Parallel() + got, err := GetAuthzConfigForWorkload(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "authz"}) + require.NoError(t, err) + require.NotNil(t, got) + assert.Equal(t, "authz", got.Name) + }) + t.Run("not found", func(t *testing.T) { + t.Parallel() + _, err := GetAuthzConfigForWorkload(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "missing"}) + assert.Error(t, err) + }) +} + +func TestValidateAuthzConfigReady(t *testing.T) { + t.Parallel() + tests := []struct { + name string + cfg *mcpv1beta1.MCPAuthzConfig + wantErr bool + }{ + {name: "valid true", cfg: newAuthzConfig("a", "cedarv1", cedarRefConfig, true)}, + {name: "valid false", cfg: newAuthzConfig("a", "cedarv1", cedarRefConfig, false), wantErr: true}, + { + name: "condition absent", + cfg: &mcpv1beta1.MCPAuthzConfig{ObjectMeta: metav1.ObjectMeta{Name: "a", Namespace: "default"}}, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := ValidateAuthzConfigReady(tt.cfg) + if tt.wantErr { + assert.Error(t, err) + return + } + assert.NoError(t, err) + }) + } +} + +func TestAddAuthzConfigRefOptions(t *testing.T) { + t.Parallel() + ctx := context.Background() + scheme := authzScheme(t) + + cedarCfg := newAuthzConfig("cedar-authz", "cedarv1", cedarRefConfig, true) + httpCfg := newAuthzConfig("http-authz", "httpv1", httpRefConfig, true) + notReady := newAuthzConfig("notready-authz", "cedarv1", cedarRefConfig, false) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cedarCfg, httpCfg, notReady).Build() + + t.Run("nil ref is a no-op", func(t *testing.T) { + t.Parallel() + var opts []runner.RunConfigBuilderOption + require.NoError(t, AddAuthzConfigRefOptions(ctx, c, "default", nil, &opts)) + assert.Empty(t, opts) + }) + t.Run("cedarv1 ref appends one option", func(t *testing.T) { + t.Parallel() + var opts []runner.RunConfigBuilderOption + require.NoError(t, AddAuthzConfigRefOptions(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "cedar-authz"}, &opts)) + assert.Len(t, opts, 1) + }) + t.Run("httpv1 ref appends one option (backend-agnostic)", func(t *testing.T) { + t.Parallel() + var opts []runner.RunConfigBuilderOption + require.NoError(t, AddAuthzConfigRefOptions(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "http-authz"}, &opts)) + assert.Len(t, opts, 1) + }) + t.Run("not found returns error", func(t *testing.T) { + t.Parallel() + var opts []runner.RunConfigBuilderOption + assert.Error(t, AddAuthzConfigRefOptions(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "missing"}, &opts)) + }) + t.Run("not ready returns error", func(t *testing.T) { + t.Parallel() + var opts []runner.RunConfigBuilderOption + assert.Error(t, AddAuthzConfigRefOptions(ctx, c, "default", &mcpv1beta1.MCPAuthzConfigReference{Name: "notready-authz"}, &opts)) + }) +} + +func TestEnsureAuthzConfigMapFromRefAndVolume(t *testing.T) { + t.Parallel() + ctx := context.Background() + scheme := authzScheme(t) + + owner := &mcpv1beta1.MCPServer{ObjectMeta: metav1.ObjectMeta{Name: "srv", Namespace: "default"}} + authzCfg := newAuthzConfig("authz", "cedarv1", cedarRefConfig, true) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(owner).Build() + + require.NoError(t, EnsureAuthzConfigMapFromRef(ctx, c, scheme, owner, "default", "srv", authzCfg, map[string]string{"app": "srv"})) + + mount, volume := GenerateAuthzVolumeConfigFromRef("srv") + require.NotNil(t, mount) + require.NotNil(t, volume) + require.NotNil(t, volume.ConfigMap) + cmName := volume.ConfigMap.Name + + var cm corev1.ConfigMap + require.NoError(t, c.Get(ctx, client.ObjectKey{Namespace: "default", Name: cmName}, &cm)) + body, ok := cm.Data[DefaultAuthzKey] + require.True(t, ok, "ConfigMap must carry the authz payload under DefaultAuthzKey") + assert.Contains(t, body, `"type":"cedarv1"`) + assert.Contains(t, body, "cedar") +} diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml index 777c310691..3d4d72bb67 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -647,6 +647,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state @@ -1380,6 +1384,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml index 124d7131e6..cb457b954e 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpservers.yaml @@ -860,6 +860,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state @@ -1808,6 +1812,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml index d038549c06..2ce8be7595 100644 --- a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2873,6 +2873,11 @@ spec: status: description: VirtualMCPServerStatus defines the observed state of VirtualMCPServer properties: + authzConfigHash: + description: |- + AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection. + Only populated when IncomingAuth.AuthzConfigRef is set. + type: string backendCount: description: |- BackendCount is the number of routable backends (ready + unauthenticated). @@ -5890,6 +5895,11 @@ spec: status: description: VirtualMCPServerStatus defines the observed state of VirtualMCPServer properties: + authzConfigHash: + description: |- + AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection. + Only populated when IncomingAuth.AuthzConfigRef is set. + type: string backendCount: description: |- BackendCount is the number of routable backends (ready + unauthenticated). diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml index f54568e66e..848eb2b640 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -650,6 +650,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state @@ -1383,6 +1387,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPRemoteProxy's state diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml index 523aa92ae6..c7cf88ed87 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml @@ -863,6 +863,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state @@ -1811,6 +1815,10 @@ spec: AuthServerConfigHash is the hash of the referenced authServerRef spec, used to detect configuration changes and trigger reconciliation. type: string + authzConfigHash: + description: AuthzConfigHash is the hash of the referenced MCPAuthzConfig + spec for change detection + type: string conditions: description: Conditions represent the latest available observations of the MCPServer's state diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml index 1905b38ebb..6536a0add5 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2876,6 +2876,11 @@ spec: status: description: VirtualMCPServerStatus defines the observed state of VirtualMCPServer properties: + authzConfigHash: + description: |- + AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection. + Only populated when IncomingAuth.AuthzConfigRef is set. + type: string backendCount: description: |- BackendCount is the number of routable backends (ready + unauthenticated). @@ -5893,6 +5898,11 @@ spec: status: description: VirtualMCPServerStatus defines the observed state of VirtualMCPServer properties: + authzConfigHash: + description: |- + AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection. + Only populated when IncomingAuth.AuthzConfigRef is set. + type: string backendCount: description: |- BackendCount is the number of routable backends (ready + unauthenticated). diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 671aec4fde..2401820257 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -2312,6 +2312,7 @@ _Appears in:_ | `telemetryConfigHash` _string_ | TelemetryConfigHash stores the hash of the referenced MCPTelemetryConfig for change detection | | Optional: \{\}
| | `externalAuthConfigHash` _string_ | ExternalAuthConfigHash is the hash of the referenced MCPExternalAuthConfig spec | | Optional: \{\}
| | `authServerConfigHash` _string_ | AuthServerConfigHash is the hash of the referenced authServerRef spec,
used to detect configuration changes and trigger reconciliation. | | Optional: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection | | Optional: \{\}
| | `message` _string_ | Message provides additional information about the current phase | | Optional: \{\}
| @@ -2547,6 +2548,7 @@ _Appears in:_ | `toolConfigHash` _string_ | ToolConfigHash stores the hash of the referenced ToolConfig for change detection | | Optional: \{\}
| | `externalAuthConfigHash` _string_ | ExternalAuthConfigHash is the hash of the referenced MCPExternalAuthConfig spec | | Optional: \{\}
| | `authServerConfigHash` _string_ | AuthServerConfigHash is the hash of the referenced authServerRef spec,
used to detect configuration changes and trigger reconciliation. | | Optional: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection | | Optional: \{\}
| | `telemetryConfigHash` _string_ | TelemetryConfigHash is the hash of the referenced MCPTelemetryConfig spec for change detection | | Optional: \{\}
| | `webhookConfigHash` _string_ | WebhookConfigHash is the hash of the referenced MCPWebhookConfig spec | | Optional: \{\}
| @@ -3849,6 +3851,7 @@ _Appears in:_ | `url` _string_ | URL is the URL where the Virtual MCP server can be accessed | | Optional: \{\}
| | `discoveredBackends` _[api.v1beta1.DiscoveredBackend](#apiv1beta1discoveredbackend) array_ | DiscoveredBackends lists discovered backend configurations from the MCPGroup | | Optional: \{\}
| | `backendCount` _integer_ | BackendCount is the number of routable backends (ready + unauthenticated).
Excludes unavailable, degraded, and unknown backends. | | Optional: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection.
Only populated when IncomingAuth.AuthzConfigRef is set. | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection.
Only populated when IncomingAuth.OIDCConfigRef is set. | | Optional: \{\}
| | `telemetryConfigHash` _string_ | TelemetryConfigHash is the hash of the referenced MCPTelemetryConfig spec for change detection.
Only populated when TelemetryConfigRef is set. | | Optional: \{\}
|