From 734e8db00dac3d038b37f31ba032a22cf0297683 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 13 Apr 2026 13:32:26 +0000 Subject: [PATCH 1/2] Add MCPAuthzConfig CRD for backend-agnostic authorization config The existing AuthzConfigRef inline variant is Cedar-specific, with fields that only make sense for the Cedar authorizer. This adds a new MCPAuthzConfig CRD that accepts any registered authorizer backend configuration via a type discriminator and an opaque config blob. The MCPAuthzConfig CRD follows the established shared-config pattern (MCPOIDCConfig, MCPExternalAuthConfig) with status conditions, hash-based change detection, referencing workload tracking, and deletion protection. A new ConfigKey() method on the AuthorizerFactory interface enables the controller to reconstruct the full runtime config JSON from the CRD spec. The AuthzConfigRef field is added to MCPServer, MCPRemoteProxy, and VirtualMCPServer as a new option alongside the existing authzConfig field, enabling incremental migration with no breaking changes. Refs: #3157 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../api/v1alpha1/mcpauthzconfig_types.go | 105 +++ .../api/v1alpha1/mcpremoteproxy_types.go | 10 +- .../api/v1alpha1/mcpserver_types.go | 10 +- .../api/v1alpha1/virtualmcpserver_types.go | 11 +- .../api/v1alpha1/zz_generated.deepcopy.go | 132 ++++ .../controllers/mcpauthzconfig_controller.go | 432 ++++++++++++ .../mcpauthzconfig_controller_test.go | 616 ++++++++++++++++++ cmd/thv-operator/main.go | 8 + .../operator-crds/crd-helm-wrapper/main.go | 1 + ...toolhive.stacklok.dev_mcpauthzconfigs.yaml | 187 ++++++ ...oolhive.stacklok.dev_mcpremoteproxies.yaml | 20 +- .../toolhive.stacklok.dev_mcpservers.yaml | 20 +- ...olhive.stacklok.dev_virtualmcpservers.yaml | 19 +- ...toolhive.stacklok.dev_mcpauthzconfigs.yaml | 191 ++++++ ...oolhive.stacklok.dev_mcpremoteproxies.yaml | 20 +- .../toolhive.stacklok.dev_mcpservers.yaml | 20 +- ...olhive.stacklok.dev_virtualmcpservers.yaml | 19 +- .../operator/templates/clusterrole/role.yaml | 3 + docs/operator/crd-api.md | 114 +++- pkg/authz/authorizers/cedar/core.go | 3 + pkg/authz/authorizers/config_test.go | 2 + pkg/authz/authorizers/http/core.go | 3 + pkg/authz/authorizers/registry.go | 5 + pkg/authz/authorizers/registry_test.go | 2 + 24 files changed, 1934 insertions(+), 19 deletions(-) create mode 100644 cmd/thv-operator/api/v1alpha1/mcpauthzconfig_types.go create mode 100644 cmd/thv-operator/controllers/mcpauthzconfig_controller.go create mode 100644 cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go create mode 100644 deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml create mode 100644 deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml diff --git a/cmd/thv-operator/api/v1alpha1/mcpauthzconfig_types.go b/cmd/thv-operator/api/v1alpha1/mcpauthzconfig_types.go new file mode 100644 index 0000000000..17444ebddf --- /dev/null +++ b/cmd/thv-operator/api/v1alpha1/mcpauthzconfig_types.go @@ -0,0 +1,105 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + runtime "k8s.io/apimachinery/pkg/runtime" +) + +// Condition type and reasons for MCPAuthzConfig status (RFC-0023) +const ( + // ConditionTypeAuthzConfigValid indicates whether the MCPAuthzConfig configuration is valid + ConditionTypeAuthzConfigValid = ConditionTypeValid + + // ConditionReasonAuthzConfigValid indicates spec validation passed + ConditionReasonAuthzConfigValid = "ConfigValid" + + // ConditionReasonAuthzConfigInvalid indicates spec validation failed + ConditionReasonAuthzConfigInvalid = "ConfigInvalid" +) + +// MCPAuthzConfigSpec defines the desired state of MCPAuthzConfig. +// MCPAuthzConfig resources are namespace-scoped and can only be referenced by +// MCPServer, MCPRemoteProxy, or VirtualMCPServer resources in the same namespace. +type MCPAuthzConfigSpec struct { + // Type identifies the authorizer backend (e.g., "cedarv1", "httpv1"). + // Must match a registered authorizer type in the factory registry. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + Type string `json:"type"` + + // Config contains the backend-specific authorization configuration. + // The structure depends on the Type field: + // - cedarv1: policies ([]string), entities_json (string), primary_upstream_provider (string), group_claim_name (string) + // - httpv1: http ({url, timeout, insecure_skip_verify}), context ({include_args, include_operation}), claim_mapping (string) + // +kubebuilder:pruning:PreserveUnknownFields + // +kubebuilder:validation:Type=object + Config runtime.RawExtension `json:"config"` +} + +// MCPAuthzConfigStatus defines the observed state of MCPAuthzConfig +type MCPAuthzConfigStatus struct { + // Conditions represent the latest available observations of the MCPAuthzConfig's state + // +listType=map + // +listMapKey=type + // +optional + Conditions []metav1.Condition `json:"conditions,omitempty"` + + // ObservedGeneration is the most recent generation observed for this MCPAuthzConfig. + // +optional + ObservedGeneration int64 `json:"observedGeneration,omitempty"` + + // ConfigHash is a hash of the current configuration for change detection + // +optional + ConfigHash string `json:"configHash,omitempty"` + + // ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. + // Each entry identifies the workload by kind and name. + // +listType=map + // +listMapKey=name + // +optional + ReferencingWorkloads []WorkloadReference `json:"referencingWorkloads,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:subresource:status +// +kubebuilder:resource:shortName=authzcfg,categories=toolhive +// +kubebuilder:printcolumn:name="Type",type=string,JSONPath=`.spec.type` +// +kubebuilder:printcolumn:name="Valid",type=string,JSONPath=`.status.conditions[?(@.type=='Valid')].status` +// +kubebuilder:printcolumn:name="Age",type=date,JSONPath=`.metadata.creationTimestamp` + +// MCPAuthzConfig is the Schema for the mcpauthzconfigs API. +// MCPAuthzConfig resources are namespace-scoped and can only be referenced by +// MCPServer, MCPRemoteProxy, or VirtualMCPServer resources within the same namespace. +// Cross-namespace references are not supported for security and isolation reasons. +type MCPAuthzConfig struct { + metav1.TypeMeta `json:",inline"` // nolint:revive + metav1.ObjectMeta `json:"metadata,omitempty"` + + Spec MCPAuthzConfigSpec `json:"spec,omitempty"` + Status MCPAuthzConfigStatus `json:"status,omitempty"` +} + +// +kubebuilder:object:root=true + +// MCPAuthzConfigList contains a list of MCPAuthzConfig +type MCPAuthzConfigList struct { + metav1.TypeMeta `json:",inline"` // nolint:revive + metav1.ListMeta `json:"metadata,omitempty"` + Items []MCPAuthzConfig `json:"items"` +} + +// MCPAuthzConfigReference references a shared MCPAuthzConfig resource. +// The referenced MCPAuthzConfig must be in the same namespace as the referencing workload. +type MCPAuthzConfigReference struct { + // Name is the name of the MCPAuthzConfig resource in the same namespace. + // +kubebuilder:validation:Required + // +kubebuilder:validation:MinLength=1 + Name string `json:"name"` +} + +func init() { + SchemeBuilder.Register(&MCPAuthzConfig{}, &MCPAuthzConfigList{}) +} diff --git a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go index ba5d278046..6079ed6532 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go @@ -87,10 +87,18 @@ type MCPRemoteProxySpec struct { // +optional HeaderForward *HeaderForwardConfig `json:"headerForward,omitempty"` - // AuthzConfig defines authorization policy configuration for the proxy + // AuthzConfig defines authorization policy configuration for the proxy. + // Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + // AuthzConfig and AuthzConfigRef are mutually exclusive. // +optional AuthzConfig *AuthzConfigRef `json:"authzConfig,omitempty"` + // AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + // The referenced MCPAuthzConfig must exist in the same namespace as this MCPRemoteProxy. + // Mutually exclusive with authzConfig. + // +optional + AuthzConfigRef *MCPAuthzConfigReference `json:"authzConfigRef,omitempty"` + // Audit defines audit logging configuration for the proxy // +optional Audit *AuditConfig `json:"audit,omitempty"` diff --git a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go index 277046e078..092a3aae49 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go @@ -290,10 +290,18 @@ type MCPServerSpec struct { // +optional OIDCConfigRef *MCPOIDCConfigReference `json:"oidcConfigRef,omitempty"` - // AuthzConfig defines authorization policy configuration for the MCP server + // AuthzConfig defines authorization policy configuration for the MCP server. + // Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + // AuthzConfig and AuthzConfigRef are mutually exclusive. // +optional AuthzConfig *AuthzConfigRef `json:"authzConfig,omitempty"` + // AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + // The referenced MCPAuthzConfig must exist in the same namespace as this MCPServer. + // Mutually exclusive with authzConfig. + // +optional + AuthzConfigRef *MCPAuthzConfigReference `json:"authzConfigRef,omitempty"` + // Audit defines audit logging configuration for the MCP server // +optional Audit *AuditConfig `json:"audit,omitempty"` diff --git a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go index 3b784bbe67..7784d4929f 100644 --- a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go @@ -133,10 +133,17 @@ type IncomingAuthConfig struct { // +optional OIDCConfigRef *MCPOIDCConfigReference `json:"oidcConfigRef,omitempty"` - // AuthzConfig defines authorization policy configuration - // Reuses MCPServer authz patterns + // AuthzConfig defines authorization policy configuration. + // Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + // AuthzConfig and AuthzConfigRef are mutually exclusive. // +optional AuthzConfig *AuthzConfigRef `json:"authzConfig,omitempty"` + + // AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + // The referenced MCPAuthzConfig must exist in the same namespace as this VirtualMCPServer. + // Mutually exclusive with authzConfig. + // +optional + AuthzConfigRef *MCPAuthzConfigReference `json:"authzConfigRef,omitempty"` } // OutgoingAuthConfig configures authentication from Virtual MCP to backend MCPServers diff --git a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go index e278931442..0bf8d1eac0 100644 --- a/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go +++ b/cmd/thv-operator/api/v1alpha1/zz_generated.deepcopy.go @@ -581,6 +581,11 @@ func (in *IncomingAuthConfig) DeepCopyInto(out *IncomingAuthConfig) { *out = new(AuthzConfigRef) (*in).DeepCopyInto(*out) } + if in.AuthzConfigRef != nil { + in, out := &in.AuthzConfigRef, &out.AuthzConfigRef + *out = new(MCPAuthzConfigReference) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new IncomingAuthConfig. @@ -708,6 +713,123 @@ func (in *KubernetesServiceAccountOIDCConfig) DeepCopy() *KubernetesServiceAccou return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPAuthzConfig) DeepCopyInto(out *MCPAuthzConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) + in.Status.DeepCopyInto(&out.Status) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfig. +func (in *MCPAuthzConfig) DeepCopy() *MCPAuthzConfig { + if in == nil { + return nil + } + out := new(MCPAuthzConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *MCPAuthzConfig) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPAuthzConfigList) DeepCopyInto(out *MCPAuthzConfigList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]MCPAuthzConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfigList. +func (in *MCPAuthzConfigList) DeepCopy() *MCPAuthzConfigList { + if in == nil { + return nil + } + out := new(MCPAuthzConfigList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *MCPAuthzConfigList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPAuthzConfigReference) DeepCopyInto(out *MCPAuthzConfigReference) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfigReference. +func (in *MCPAuthzConfigReference) DeepCopy() *MCPAuthzConfigReference { + if in == nil { + return nil + } + out := new(MCPAuthzConfigReference) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPAuthzConfigSpec) DeepCopyInto(out *MCPAuthzConfigSpec) { + *out = *in + in.Config.DeepCopyInto(&out.Config) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfigSpec. +func (in *MCPAuthzConfigSpec) DeepCopy() *MCPAuthzConfigSpec { + if in == nil { + return nil + } + out := new(MCPAuthzConfigSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *MCPAuthzConfigStatus) DeepCopyInto(out *MCPAuthzConfigStatus) { + *out = *in + if in.Conditions != nil { + in, out := &in.Conditions, &out.Conditions + *out = make([]v1.Condition, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + if in.ReferencingWorkloads != nil { + in, out := &in.ReferencingWorkloads, &out.ReferencingWorkloads + *out = make([]WorkloadReference, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new MCPAuthzConfigStatus. +func (in *MCPAuthzConfigStatus) DeepCopy() *MCPAuthzConfigStatus { + if in == nil { + return nil + } + out := new(MCPAuthzConfigStatus) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MCPExternalAuthConfig) DeepCopyInto(out *MCPExternalAuthConfig) { *out = *in @@ -1293,6 +1415,11 @@ func (in *MCPRemoteProxySpec) DeepCopyInto(out *MCPRemoteProxySpec) { *out = new(AuthzConfigRef) (*in).DeepCopyInto(*out) } + if in.AuthzConfigRef != nil { + in, out := &in.AuthzConfigRef, &out.AuthzConfigRef + *out = new(MCPAuthzConfigReference) + **out = **in + } if in.Audit != nil { in, out := &in.Audit, &out.Audit *out = new(AuditConfig) @@ -1582,6 +1709,11 @@ func (in *MCPServerSpec) DeepCopyInto(out *MCPServerSpec) { *out = new(AuthzConfigRef) (*in).DeepCopyInto(*out) } + if in.AuthzConfigRef != nil { + in, out := &in.AuthzConfigRef, &out.AuthzConfigRef + *out = new(MCPAuthzConfigReference) + **out = **in + } if in.Audit != nil { in, out := &in.Audit, &out.Audit *out = new(AuditConfig) diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go new file mode 100644 index 0000000000..4cc0930584 --- /dev/null +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go @@ -0,0 +1,432 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "context" + "encoding/json" + "fmt" + "time" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" + "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" + ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" + "github.com/stacklok/toolhive/pkg/authz/authorizers" + // Import authorizer backends so they register with the factory registry. + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/cedar" + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/http" +) + +const ( + // AuthzConfigFinalizerName is the name of the finalizer for MCPAuthzConfig + AuthzConfigFinalizerName = "mcpauthzconfig.toolhive.stacklok.dev/finalizer" + + // 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. +// +// This controller manages the lifecycle of MCPAuthzConfig resources: validation +// via the authorizer factory registry, config hash computation, finalizer management, +// reference tracking, and deletion protection when workloads reference this config. +type MCPAuthzConfigReconciler struct { + client.Client + Scheme *runtime.Scheme +} + +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpauthzconfigs,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpauthzconfigs/status,verbs=get;update;patch +// +kubebuilder:rbac:groups=toolhive.stacklok.dev,resources=mcpauthzconfigs/finalizers,verbs=update + +// Reconcile is part of the main kubernetes reconciliation loop which aims to +// move the current state of the cluster closer to the desired state. +func (r *MCPAuthzConfigReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + // Fetch the MCPAuthzConfig instance + authzConfig := &mcpv1alpha1.MCPAuthzConfig{} + err := r.Get(ctx, req.NamespacedName, authzConfig) + if err != nil { + if errors.IsNotFound(err) { + logger.Info("MCPAuthzConfig resource not found. Ignoring since object must be deleted") + return ctrl.Result{}, nil + } + logger.Error(err, "Failed to get MCPAuthzConfig") + return ctrl.Result{}, err + } + + // Check if the MCPAuthzConfig is being deleted + if !authzConfig.DeletionTimestamp.IsZero() { + return r.handleDeletion(ctx, authzConfig) + } + + // Add finalizer if it doesn't exist + if !controllerutil.ContainsFinalizer(authzConfig, AuthzConfigFinalizerName) { + controllerutil.AddFinalizer(authzConfig, AuthzConfigFinalizerName) + if err := r.Update(ctx, authzConfig); err != nil { + logger.Error(err, "Failed to add finalizer") + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: authzConfigRequeueDelay}, nil + } + + // Validate the authz configuration via the authorizer factory + if err := validateAuthzConfigSpec(authzConfig.Spec); err != nil { + logger.Error(err, "MCPAuthzConfig spec validation failed") + meta.SetStatusCondition(&authzConfig.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthzConfigValid, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthzConfigInvalid, + Message: err.Error(), + ObservedGeneration: authzConfig.Generation, + }) + if updateErr := r.Status().Update(ctx, authzConfig); updateErr != nil { + logger.Error(updateErr, "Failed to update status after validation error") + } + return ctrl.Result{}, nil // Don't requeue on validation errors - user must fix spec + } + + // Validation succeeded - set Valid=True condition + conditionChanged := meta.SetStatusCondition(&authzConfig.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeAuthzConfigValid, + Status: metav1.ConditionTrue, + Reason: mcpv1alpha1.ConditionReasonAuthzConfigValid, + Message: "Spec validation passed", + ObservedGeneration: authzConfig.Generation, + }) + + // Calculate the hash of the current configuration + configHash := ctrlutil.CalculateConfigHash(authzConfig.Spec) + + // Check if the hash has changed + hashChanged := authzConfig.Status.ConfigHash != configHash + if hashChanged { + logger.Info("MCPAuthzConfig configuration changed", + "oldHash", authzConfig.Status.ConfigHash, + "newHash", configHash) + + authzConfig.Status.ConfigHash = configHash + authzConfig.Status.ObservedGeneration = authzConfig.Generation + + if err := r.Status().Update(ctx, authzConfig); err != nil { + logger.Error(err, "Failed to update MCPAuthzConfig status") + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + + // Refresh ReferencingWorkloads list + referencingWorkloads, err := r.findReferencingWorkloads(ctx, authzConfig) + if err != nil { + logger.Error(err, "Failed to find referencing workloads") + } else if !ctrlutil.WorkloadRefsEqual(authzConfig.Status.ReferencingWorkloads, referencingWorkloads) { + authzConfig.Status.ReferencingWorkloads = referencingWorkloads + conditionChanged = true + } + + // Update condition if it changed (even without hash change) + if conditionChanged { + if err := r.Status().Update(ctx, authzConfig); err != nil { + logger.Error(err, "Failed to update MCPAuthzConfig status after condition change") + return ctrl.Result{}, err + } + } + + return ctrl.Result{}, nil +} + +// validateAuthzConfigSpec validates the MCPAuthzConfig spec by reconstructing the +// full authorizer config and delegating to the factory's ValidateConfig method. +func validateAuthzConfigSpec(spec mcpv1alpha1.MCPAuthzConfigSpec) error { + fullConfigJSON, err := BuildFullAuthzConfigJSON(spec) + if err != nil { + return err + } + + // Parse and validate via the authorizer factory + var cfg authzConfig + if err := json.Unmarshal(fullConfigJSON, &cfg); err != nil { + return fmt.Errorf("failed to parse reconstructed authz config: %w", err) + } + if cfg.Version == "" || cfg.Type == "" { + return fmt.Errorf("reconstructed config missing version or type") + } + + factory := authorizers.GetFactory(cfg.Type) + if factory == nil { + return fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", + cfg.Type, authorizers.RegisteredTypes()) + } + + return factory.ValidateConfig(fullConfigJSON) +} + +// authzConfig is a minimal struct for extracting version and type from reconstructed JSON. +type authzConfig struct { + Version string `json:"version"` + Type string `json:"type"` +} + +// BuildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a +// MCPAuthzConfig spec. The result is the same format accepted by authorizers.Config +// and used in ConfigMaps: {"version": "1.0", "type": "", "": {}}. +func BuildFullAuthzConfigJSON(spec mcpv1alpha1.MCPAuthzConfigSpec) ([]byte, error) { + factory := authorizers.GetFactory(spec.Type) + if factory == nil { + return nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", + spec.Type, authorizers.RegisteredTypes()) + } + + configKey := factory.ConfigKey() + + fullConfig := map[string]json.RawMessage{ + "version": mustMarshalJSON(authzConfigVersion), + "type": mustMarshalJSON(spec.Type), + configKey: spec.Config.Raw, + } + + result, err := json.Marshal(fullConfig) + if err != nil { + return nil, fmt.Errorf("failed to marshal full authz config: %w", err) + } + return result, nil +} + +// mustMarshalJSON marshals a value to JSON or panics. Only for simple string values. +func mustMarshalJSON(v string) json.RawMessage { + b, err := json.Marshal(v) + if err != nil { + panic(fmt.Sprintf("failed to marshal %q: %v", v, err)) + } + return b +} + +// handleDeletion handles the deletion of a MCPAuthzConfig. +// Blocks deletion while workload resources reference this config by keeping the +// finalizer and requeueing. Once all references are removed, the finalizer is removed +// and the resource can be garbage collected. +func (r *MCPAuthzConfigReconciler) handleDeletion( + ctx context.Context, + authzConfig *mcpv1alpha1.MCPAuthzConfig, +) (ctrl.Result, error) { + logger := log.FromContext(ctx) + + if controllerutil.ContainsFinalizer(authzConfig, AuthzConfigFinalizerName) { + // Check if any workloads still reference this config + referencingWorkloads, err := r.findReferencingWorkloads(ctx, authzConfig) + if err != nil { + logger.Error(err, "Failed to check referencing workloads during deletion") + return ctrl.Result{}, err + } + + if len(referencingWorkloads) > 0 { + logger.Info("MCPAuthzConfig is still referenced by workloads, blocking deletion", + "authzConfig", authzConfig.Name, + "referencingWorkloads", referencingWorkloads) + + meta.SetStatusCondition(&authzConfig.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionTypeDeletionBlocked, + Status: metav1.ConditionTrue, + Reason: "ReferencedByWorkloads", + Message: fmt.Sprintf("Cannot delete: referenced by workloads: %v", referencingWorkloads), + ObservedGeneration: authzConfig.Generation, + }) + authzConfig.Status.ReferencingWorkloads = referencingWorkloads + if updateErr := r.Status().Update(ctx, authzConfig); updateErr != nil { + logger.Error(updateErr, "Failed to update status during deletion block") + } + + // Requeue to check again later + return ctrl.Result{RequeueAfter: 30 * time.Second}, nil + } + + controllerutil.RemoveFinalizer(authzConfig, AuthzConfigFinalizerName) + if err := r.Update(ctx, authzConfig); err != nil { + logger.Error(err, "Failed to remove finalizer") + return ctrl.Result{}, err + } + logger.Info("Removed finalizer from MCPAuthzConfig", "authzConfig", authzConfig.Name) + } + + return ctrl.Result{}, nil +} + +// findReferencingWorkloads returns the workload resources (MCPServer, VirtualMCPServer, +// and MCPRemoteProxy) that reference this MCPAuthzConfig via their AuthzConfigRef field. +func (r *MCPAuthzConfigReconciler) findReferencingWorkloads( + ctx context.Context, + authzConfig *mcpv1alpha1.MCPAuthzConfig, +) ([]mcpv1alpha1.WorkloadReference, error) { + // Find referencing MCPServers + refs, err := ctrlutil.FindWorkloadRefsFromMCPServers(ctx, r.Client, authzConfig.Namespace, authzConfig.Name, + func(server *mcpv1alpha1.MCPServer) *string { + if server.Spec.AuthzConfigRef != nil { + return &server.Spec.AuthzConfigRef.Name + } + return nil + }) + if err != nil { + return nil, err + } + + // Check VirtualMCPServers + vmcpList := &mcpv1alpha1.VirtualMCPServerList{} + if err := r.List(ctx, vmcpList, client.InNamespace(authzConfig.Namespace)); err != nil { + return nil, fmt.Errorf("failed to list VirtualMCPServers: %w", err) + } + for _, vmcp := range vmcpList.Items { + if vmcp.Spec.IncomingAuth != nil && + vmcp.Spec.IncomingAuth.AuthzConfigRef != nil && + vmcp.Spec.IncomingAuth.AuthzConfigRef.Name == authzConfig.Name { + refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindVirtualMCPServer, Name: vmcp.Name}) + } + } + + // Check MCPRemoteProxies + proxyList := &mcpv1alpha1.MCPRemoteProxyList{} + if err := r.List(ctx, proxyList, client.InNamespace(authzConfig.Namespace)); err != nil { + return nil, fmt.Errorf("failed to list MCPRemoteProxies: %w", err) + } + for _, proxy := range proxyList.Items { + if proxy.Spec.AuthzConfigRef != nil && proxy.Spec.AuthzConfigRef.Name == authzConfig.Name { + refs = append(refs, mcpv1alpha1.WorkloadReference{Kind: mcpv1alpha1.WorkloadKindMCPRemoteProxy, Name: proxy.Name}) + } + } + + ctrlutil.SortWorkloadRefs(refs) + return refs, nil +} + +// SetupWithManager sets up the controller with the Manager. +// Watches MCPServer, VirtualMCPServer, and MCPRemoteProxy changes to maintain +// accurate ReferencingWorkloads status. +func (r *MCPAuthzConfigReconciler) SetupWithManager(mgr ctrl.Manager) error { + return ctrl.NewControllerManagedBy(mgr). + For(&mcpv1alpha1.MCPAuthzConfig{}). + Watches(&mcpv1alpha1.MCPServer{}, + handler.EnqueueRequestsFromMapFunc(r.mapMCPServerToAuthzConfig)). + Watches(&mcpv1alpha1.VirtualMCPServer{}, + handler.EnqueueRequestsFromMapFunc(r.mapVirtualMCPServerToAuthzConfig)). + Watches(&mcpv1alpha1.MCPRemoteProxy{}, + handler.EnqueueRequestsFromMapFunc(r.mapMCPRemoteProxyToAuthzConfig)). + Complete(r) +} + +// mapMCPServerToAuthzConfig maps MCPServer changes to MCPAuthzConfig reconciliation requests. +func (r *MCPAuthzConfigReconciler) mapMCPServerToAuthzConfig( + ctx context.Context, obj client.Object, +) []reconcile.Request { + server, ok := obj.(*mcpv1alpha1.MCPServer) + if !ok { + return nil + } + + seen := make(map[types.NamespacedName]struct{}) + var requests []reconcile.Request + + // Enqueue the currently-referenced MCPAuthzConfig (if any) + if server.Spec.AuthzConfigRef != nil { + nn := types.NamespacedName{Name: server.Spec.AuthzConfigRef.Name, Namespace: server.Namespace} + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + // Also enqueue any MCPAuthzConfig that still lists this server in + // ReferencingWorkloads — handles ref-removal and server-deletion cases. + requests = append(requests, r.findStaleRefs(ctx, server.Namespace, mcpv1alpha1.WorkloadKindMCPServer, server.Name, seen)...) + + return requests +} + +// mapVirtualMCPServerToAuthzConfig maps VirtualMCPServer changes to MCPAuthzConfig reconciliation requests. +func (r *MCPAuthzConfigReconciler) mapVirtualMCPServerToAuthzConfig( + ctx context.Context, obj client.Object, +) []reconcile.Request { + vmcp, ok := obj.(*mcpv1alpha1.VirtualMCPServer) + if !ok { + return nil + } + + seen := make(map[types.NamespacedName]struct{}) + var requests []reconcile.Request + + if vmcp.Spec.IncomingAuth != nil && vmcp.Spec.IncomingAuth.AuthzConfigRef != nil { + nn := types.NamespacedName{Name: vmcp.Spec.IncomingAuth.AuthzConfigRef.Name, Namespace: vmcp.Namespace} + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + requests = append(requests, r.findStaleRefs(ctx, vmcp.Namespace, mcpv1alpha1.WorkloadKindVirtualMCPServer, vmcp.Name, seen)...) + + return requests +} + +// mapMCPRemoteProxyToAuthzConfig maps MCPRemoteProxy changes to MCPAuthzConfig reconciliation requests. +func (r *MCPAuthzConfigReconciler) mapMCPRemoteProxyToAuthzConfig( + ctx context.Context, obj client.Object, +) []reconcile.Request { + proxy, ok := obj.(*mcpv1alpha1.MCPRemoteProxy) + if !ok { + return nil + } + + seen := make(map[types.NamespacedName]struct{}) + var requests []reconcile.Request + + if proxy.Spec.AuthzConfigRef != nil { + nn := types.NamespacedName{Name: proxy.Spec.AuthzConfigRef.Name, Namespace: proxy.Namespace} + seen[nn] = struct{}{} + requests = append(requests, reconcile.Request{NamespacedName: nn}) + } + + requests = append(requests, r.findStaleRefs(ctx, proxy.Namespace, mcpv1alpha1.WorkloadKindMCPRemoteProxy, proxy.Name, seen)...) + + return requests +} + +// findStaleRefs finds MCPAuthzConfig resources that still list a workload in their +// ReferencingWorkloads status but are not in the seen set. This handles ref-removal +// and workload-deletion cases. +func (r *MCPAuthzConfigReconciler) findStaleRefs( + ctx context.Context, + namespace, workloadKind, workloadName string, + seen map[types.NamespacedName]struct{}, +) []reconcile.Request { + authzConfigList := &mcpv1alpha1.MCPAuthzConfigList{} + if err := r.List(ctx, authzConfigList, client.InNamespace(namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list MCPAuthzConfigs for workload watch", + "workloadKind", workloadKind, "workloadName", workloadName) + return nil + } + + var requests []reconcile.Request + for _, cfg := range authzConfigList.Items { + nn := types.NamespacedName{Name: cfg.Name, Namespace: cfg.Namespace} + if _, already := seen[nn]; already { + continue + } + for _, ref := range cfg.Status.ReferencingWorkloads { + if ref.Kind == workloadKind && ref.Name == workloadName { + requests = append(requests, reconcile.Request{NamespacedName: nn}) + break + } + } + } + return requests +} diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go new file mode 100644 index 0000000000..819b5a0940 --- /dev/null +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go @@ -0,0 +1,616 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package controllers + +import ( + "encoding/json" + "testing" + "time" + + "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" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" + // Import authorizer backends so they register with the factory registry. + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/cedar" + _ "github.com/stacklok/toolhive/pkg/authz/authorizers/http" +) + +// validCedarConfig returns a RawExtension containing a valid Cedar backend config. +func validCedarConfig() runtime.RawExtension { + return runtime.RawExtension{ + Raw: []byte(`{"policies":["permit(principal, action, resource);"],"entities_json":"[]"}`), + } +} + +// validHTTPPDPConfig returns a RawExtension containing a valid HTTP PDP backend config. +func validHTTPPDPConfig() runtime.RawExtension { + return runtime.RawExtension{ + Raw: []byte(`{"http":{"url":"http://localhost:9000"},"claim_mapping":"standard"}`), + } +} + +func TestBuildFullAuthzConfigJSON(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + spec mcpv1alpha1.MCPAuthzConfigSpec + expectError bool + expectType string + expectKey string + expectVersion string + }{ + { + name: "valid Cedar config produces correct JSON", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + expectError: false, + expectType: "cedarv1", + expectKey: "cedar", + expectVersion: "1.0", + }, + { + name: "valid HTTP PDP config produces correct JSON", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "httpv1", + Config: validHTTPPDPConfig(), + }, + expectError: false, + expectType: "httpv1", + expectKey: "pdp", + expectVersion: "1.0", + }, + { + name: "unknown type returns error", + spec: mcpv1alpha1.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, err := BuildFullAuthzConfigJSON(tt.spec) + + if tt.expectError { + require.Error(t, err) + return + } + + require.NoError(t, err) + + // Verify the result is valid JSON + var parsed map[string]json.RawMessage + require.NoError(t, json.Unmarshal(result, &parsed), "Output must be valid JSON") + + // Verify "version" field + var version string + require.NoError(t, json.Unmarshal(parsed["version"], &version)) + assert.Equal(t, tt.expectVersion, version) + + // Verify "type" field + var typ string + require.NoError(t, json.Unmarshal(parsed["type"], &typ)) + assert.Equal(t, tt.expectType, typ) + + // Verify the backend-specific config key exists + _, hasKey := parsed[tt.expectKey] + assert.True(t, hasKey, "Output JSON should contain key %q", tt.expectKey) + }) + } +} + +func TestValidateAuthzConfigSpec(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + spec mcpv1alpha1.MCPAuthzConfigSpec + expectError bool + }{ + { + name: "valid Cedar config with policies", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + expectError: false, + }, + { + name: "valid HTTP PDP config", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "httpv1", + Config: validHTTPPDPConfig(), + }, + expectError: false, + }, + { + name: "Cedar config with empty policies fails validation", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: runtime.RawExtension{Raw: []byte(`{"policies":[],"entities_json":"[]"}`)}, + }, + expectError: true, + }, + { + name: "unknown authorizer type fails", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "nonexistent", + Config: runtime.RawExtension{Raw: []byte(`{}`)}, + }, + expectError: true, + }, + { + name: "empty config object fails Cedar validation", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: runtime.RawExtension{Raw: []byte(`{}`)}, + }, + expectError: true, + }, + { + name: "HTTP PDP config missing url fails validation", + spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "httpv1", + Config: runtime.RawExtension{Raw: []byte(`{"http":{},"claim_mapping":"standard"}`)}, + }, + expectError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + err := validateAuthzConfigSpec(tt.spec) + + if tt.expectError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestMCPAuthzConfigReconciler_ReconcileNotFound(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + // Empty client - no objects exist + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: "non-existent", + Namespace: "default", + }, + } + + result, err := r.Reconcile(ctx, req) + assert.NoError(t, err, "Reconciling a missing resource should not return error") + assert.Equal(t, time.Duration(0), result.RequeueAfter, "Should not requeue") +} + +func TestMCPAuthzConfigReconciler_SteadyStateNoOp(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + authzConfig := &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: "default", + Generation: 1, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authzConfig). + WithStatusSubresource(&mcpv1alpha1.MCPAuthzConfig{}). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: authzConfig.Name, + Namespace: authzConfig.Namespace, + }, + } + + // First reconcile: add finalizer + result, err := r.Reconcile(ctx, req) + require.NoError(t, err) + assert.Greater(t, result.RequeueAfter, time.Duration(0)) + + // Second reconcile: set hash and condition + _, err = r.Reconcile(ctx, req) + require.NoError(t, err) + + var afterInitial mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &afterInitial) + require.NoError(t, err) + initialHash := afterInitial.Status.ConfigHash + initialRV := afterInitial.ResourceVersion + + // Third reconcile with no changes: should be a no-op + result, err = r.Reconcile(ctx, req) + require.NoError(t, err) + assert.Equal(t, time.Duration(0), result.RequeueAfter) + + var afterSteady mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &afterSteady) + require.NoError(t, err) + assert.Equal(t, initialHash, afterSteady.Status.ConfigHash, "Hash should not change") + assert.Equal(t, initialRV, afterSteady.ResourceVersion, "ResourceVersion should not change (no writes)") +} + +func TestMCPAuthzConfigReconciler_ValidationFailureSetsCondition(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + // Invalid config: Cedar type but empty policies + authzConfig := &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "invalid-config", + Namespace: "default", + Finalizers: []string{AuthzConfigFinalizerName}, + Generation: 1, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: runtime.RawExtension{Raw: []byte(`{"policies":[],"entities_json":"[]"}`)}, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authzConfig). + WithStatusSubresource(&mcpv1alpha1.MCPAuthzConfig{}). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: authzConfig.Name, + Namespace: authzConfig.Namespace, + }, + } + + // Reconcile should not return error (validation failures are not requeued) + _, err := r.Reconcile(ctx, req) + require.NoError(t, err) + + // Check that the Valid condition is set to False + var updatedConfig mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) + require.NoError(t, err) + + var foundCondition bool + for _, cond := range updatedConfig.Status.Conditions { + if cond.Type == mcpv1alpha1.ConditionTypeAuthzConfigValid { + foundCondition = true + assert.Equal(t, metav1.ConditionFalse, cond.Status, "Valid condition should be False") + assert.Equal(t, mcpv1alpha1.ConditionReasonAuthzConfigInvalid, cond.Reason) + break + } + } + assert.True(t, foundCondition, "Should have a Valid condition") +} + +func TestMCPAuthzConfigReconciler_handleDeletion(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + authzConfig *mcpv1alpha1.MCPAuthzConfig + existingWorkloads []client.Object + expectFinalizerRemoved bool + expectRequeue bool + }{ + { + name: "no referencing workloads removes finalizer", + authzConfig: &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: "default", + Finalizers: []string{AuthzConfigFinalizerName}, + DeletionTimestamp: &metav1.Time{ + Time: time.Now(), + }, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + }, + existingWorkloads: nil, + expectFinalizerRemoved: true, + expectRequeue: false, + }, + { + name: "referencing MCPServer blocks deletion", + authzConfig: &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: "default", + Finalizers: []string{AuthzConfigFinalizerName}, + DeletionTimestamp: &metav1.Time{ + Time: time.Now(), + }, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + }, + existingWorkloads: []client.Object{ + &mcpv1alpha1.MCPServer{ + ObjectMeta: metav1.ObjectMeta{ + Name: "referencing-server", + Namespace: "default", + }, + Spec: mcpv1alpha1.MCPServerSpec{ + AuthzConfigRef: &mcpv1alpha1.MCPAuthzConfigReference{ + Name: "test-config", + }, + }, + }, + }, + expectFinalizerRemoved: false, + expectRequeue: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + + objs := []client.Object{tt.authzConfig} + objs = append(objs, tt.existingWorkloads...) + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objs...). + WithStatusSubresource(&mcpv1alpha1.MCPAuthzConfig{}). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + result, err := r.handleDeletion(ctx, tt.authzConfig) + + assert.NoError(t, err) + + if tt.expectFinalizerRemoved { + assert.NotContains(t, tt.authzConfig.Finalizers, AuthzConfigFinalizerName, + "Finalizer should be removed") + assert.Equal(t, time.Duration(0), result.RequeueAfter) + } + + if tt.expectRequeue { + assert.Greater(t, result.RequeueAfter, time.Duration(0), + "Should requeue when workloads still reference the config") + assert.Contains(t, tt.authzConfig.Finalizers, AuthzConfigFinalizerName, + "Finalizer should remain when workloads reference the config") + } + }) + } +} + +func TestMCPAuthzConfigReconciler_ConfigChangeTriggersHashUpdate(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + authzConfig := &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-config", + Namespace: "default", + Generation: 1, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: validCedarConfig(), + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authzConfig). + WithStatusSubresource(&mcpv1alpha1.MCPAuthzConfig{}). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: authzConfig.Name, + Namespace: authzConfig.Namespace, + }, + } + + // First reconciliation - add finalizer + result, err := r.Reconcile(ctx, req) + require.NoError(t, err) + assert.Greater(t, result.RequeueAfter, time.Duration(0), "Should requeue after adding finalizer") + + // Second reconciliation - calculate hash + result, err = r.Reconcile(ctx, req) + require.NoError(t, err) + assert.Equal(t, time.Duration(0), result.RequeueAfter) + + // Get updated config and check hash was set + var updatedConfig mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &updatedConfig) + require.NoError(t, err) + assert.NotEmpty(t, updatedConfig.Status.ConfigHash, "Config hash should be set") + firstHash := updatedConfig.Status.ConfigHash + + // Update the config spec (simulate a change: add a second policy) + updatedConfig.Spec.Config = runtime.RawExtension{ + Raw: []byte(`{"policies":["permit(principal, action, resource);","forbid(principal, action, resource) when { resource.sensitive == true };"],"entities_json":"[]"}`), + } + updatedConfig.Generation = 2 + err = fakeClient.Update(ctx, &updatedConfig) + require.NoError(t, err) + + // Third reconciliation - should detect change and update hash + _, err = r.Reconcile(ctx, req) + require.NoError(t, err) + + // Get final config and verify hash changed + var finalConfig mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &finalConfig) + require.NoError(t, err) + assert.NotEmpty(t, finalConfig.Status.ConfigHash, "Config hash should still be set") + assert.NotEqual(t, firstHash, finalConfig.Status.ConfigHash, "Hash should change when spec changes") + assert.Equal(t, int64(2), finalConfig.Status.ObservedGeneration, "ObservedGeneration should be updated") +} + +func TestMCPAuthzConfigReconciler_ValidationRecovery(t *testing.T) { + t.Parallel() + + ctx := t.Context() + + scheme := runtime.NewScheme() + require.NoError(t, mcpv1alpha1.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + + // Start with invalid config: Cedar with empty policies + authzConfig := &mcpv1alpha1.MCPAuthzConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: "recovery-config", + Namespace: "default", + Finalizers: []string{AuthzConfigFinalizerName}, + Generation: 1, + }, + Spec: mcpv1alpha1.MCPAuthzConfigSpec{ + Type: "cedarv1", + Config: runtime.RawExtension{Raw: []byte(`{"policies":[],"entities_json":"[]"}`)}, + }, + } + + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(authzConfig). + WithStatusSubresource(&mcpv1alpha1.MCPAuthzConfig{}). + Build() + + r := &MCPAuthzConfigReconciler{ + Client: fakeClient, + Scheme: scheme, + } + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: authzConfig.Name, + Namespace: authzConfig.Namespace, + }, + } + + // Reconcile invalid config - should set Valid=False + _, err := r.Reconcile(ctx, req) + require.NoError(t, err) + + var invalidConfig mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &invalidConfig) + require.NoError(t, err) + + var foundFalse bool + for _, cond := range invalidConfig.Status.Conditions { + if cond.Type == mcpv1alpha1.ConditionTypeAuthzConfigValid { + assert.Equal(t, metav1.ConditionFalse, cond.Status) + foundFalse = true + } + } + require.True(t, foundFalse, "Should have Valid=False condition") + assert.Empty(t, invalidConfig.Status.ConfigHash, "Hash should not be set for invalid config") + + // Fix the config by adding a valid policy + invalidConfig.Spec.Config = validCedarConfig() + invalidConfig.Generation = 2 + err = fakeClient.Update(ctx, &invalidConfig) + require.NoError(t, err) + + // Reconcile again - should set Valid=True and compute hash + _, err = r.Reconcile(ctx, req) + require.NoError(t, err) + + var recoveredConfig mcpv1alpha1.MCPAuthzConfig + err = fakeClient.Get(ctx, req.NamespacedName, &recoveredConfig) + require.NoError(t, err) + + var foundTrue bool + for _, cond := range recoveredConfig.Status.Conditions { + if cond.Type == mcpv1alpha1.ConditionTypeAuthzConfigValid { + assert.Equal(t, metav1.ConditionTrue, cond.Status, "Valid condition should recover to True") + assert.Equal(t, mcpv1alpha1.ConditionReasonAuthzConfigValid, cond.Reason) + foundTrue = true + } + } + assert.True(t, foundTrue, "Should have Valid=True condition after fix") + assert.NotEmpty(t, recoveredConfig.Status.ConfigHash, "Hash should be set after recovery") +} diff --git a/cmd/thv-operator/main.go b/cmd/thv-operator/main.go index d2e3134aa3..c1f8d28f30 100644 --- a/cmd/thv-operator/main.go +++ b/cmd/thv-operator/main.go @@ -294,6 +294,14 @@ func setupServerControllers(mgr ctrl.Manager, enableRegistry bool) error { return fmt.Errorf("unable to create controller MCPOIDCConfig: %w", err) } + // Set up MCPAuthzConfig controller + if err := (&controllers.MCPAuthzConfigReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr); err != nil { + return fmt.Errorf("unable to create controller MCPAuthzConfig: %w", err) + } + // Set up MCPTelemetryConfig controller if err := (&controllers.MCPTelemetryConfigReconciler{ Client: mgr.GetClient(), diff --git a/deploy/charts/operator-crds/crd-helm-wrapper/main.go b/deploy/charts/operator-crds/crd-helm-wrapper/main.go index 67b7d60cfc..e2cfdc9a22 100644 --- a/deploy/charts/operator-crds/crd-helm-wrapper/main.go +++ b/deploy/charts/operator-crds/crd-helm-wrapper/main.go @@ -45,6 +45,7 @@ var crdFeatureFlags = map[string][]string{ "virtualmcpcompositetooldefinitions": {"virtualMcp"}, "mcpoidcconfigs": {"server"}, "mcptelemetryconfigs": {"server"}, + "mcpauthzconfigs": {"server", "virtualMcp"}, "mcpexternalauthconfigs": {"server", "virtualMcp"}, "mcpserverentries": {"server", "virtualMcp"}, } diff --git a/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml new file mode 100644 index 0000000000..ff2bc77270 --- /dev/null +++ b/deploy/charts/operator-crds/files/crds/toolhive.stacklok.dev_mcpauthzconfigs.yaml @@ -0,0 +1,187 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.17.3 + name: mcpauthzconfigs.toolhive.stacklok.dev +spec: + group: toolhive.stacklok.dev + names: + categories: + - toolhive + kind: MCPAuthzConfig + listKind: MCPAuthzConfigList + plural: mcpauthzconfigs + shortNames: + - authzcfg + singular: mcpauthzconfig + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.type + name: Type + type: string + - jsonPath: .status.conditions[?(@.type=='Valid')].status + name: Valid + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + MCPAuthzConfig is the Schema for the mcpauthzconfigs API. + MCPAuthzConfig resources are namespace-scoped and can only be referenced by + MCPServer, MCPRemoteProxy, or VirtualMCPServer resources within the same namespace. + Cross-namespace references are not supported for security and isolation reasons. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + MCPAuthzConfigSpec defines the desired state of MCPAuthzConfig. + MCPAuthzConfig resources are namespace-scoped and can only be referenced by + MCPServer, MCPRemoteProxy, or VirtualMCPServer resources in the same namespace. + properties: + config: + description: |- + Config contains the backend-specific authorization configuration. + The structure depends on the Type field: + - cedarv1: policies ([]string), entities_json (string), primary_upstream_provider (string), group_claim_name (string) + - httpv1: http ({url, timeout, insecure_skip_verify}), context ({include_args, include_operation}), claim_mapping (string) + type: object + x-kubernetes-preserve-unknown-fields: true + type: + description: |- + Type identifies the authorizer backend (e.g., "cedarv1", "httpv1"). + Must match a registered authorizer type in the factory registry. + minLength: 1 + type: string + required: + - config + - type + type: object + status: + description: MCPAuthzConfigStatus defines the observed state of MCPAuthzConfig + properties: + conditions: + description: Conditions represent the latest available observations + of the MCPAuthzConfig's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + configHash: + description: ConfigHash is a hash of the current configuration for + change detection + type: string + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this MCPAuthzConfig. + format: int64 + type: integer + referencingWorkloads: + description: |- + ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. + Each entry identifies the workload by kind and name. + items: + description: |- + WorkloadReference identifies a workload that references a shared configuration resource. + Namespace is implicit — cross-namespace references are not supported. + properties: + kind: + description: Kind is the type of workload resource + enum: + - MCPServer + - VirtualMCPServer + - MCPRemoteProxy + type: string + name: + description: Name is the name of the workload resource + minLength: 1 + type: string + required: + - kind + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} 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 22f478538f..d0f8815fa3 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 @@ -94,8 +94,10 @@ spec: - name type: object authzConfig: - description: AuthzConfig defines authorization policy configuration - for the proxy + description: |- + AuthzConfig defines authorization policy configuration for the proxy. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -150,6 +152,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this MCPRemoteProxy. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource in + the same namespace. + minLength: 1 + type: string + required: + - name + type: object endpointPrefix: description: |- EndpointPrefix is the path prefix to prepend to SSE endpoint URLs. 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 f323a25ff2..77c9eb801c 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 @@ -99,8 +99,10 @@ spec: - name type: object authzConfig: - description: AuthzConfig defines authorization policy configuration - for the MCP server + description: |- + AuthzConfig defines authorization policy configuration for the MCP server. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -155,6 +157,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this MCPServer. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource in + the same namespace. + minLength: 1 + type: string + required: + - name + type: object backendReplicas: description: |- BackendReplicas is the desired number of MCP server backend pod replicas. 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 1ae8c86f14..2a929bcb49 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 @@ -1793,8 +1793,9 @@ spec: properties: authzConfig: description: |- - AuthzConfig defines authorization policy configuration - Reuses MCPServer authz patterns + AuthzConfig defines authorization policy configuration. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -1849,6 +1850,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this VirtualMCPServer. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource + in the same namespace. + minLength: 1 + type: string + required: + - name + type: object oidcConfig: description: |- OIDCConfig defines OIDC authentication configuration. diff --git a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml new file mode 100644 index 0000000000..441c0af59f --- /dev/null +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpauthzconfigs.yaml @@ -0,0 +1,191 @@ +{{- if or .Values.crds.install.server .Values.crds.install.virtualMcp }} +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + {{- if .Values.crds.keep }} + helm.sh/resource-policy: keep + {{- end }} + controller-gen.kubebuilder.io/version: v0.17.3 + name: mcpauthzconfigs.toolhive.stacklok.dev +spec: + group: toolhive.stacklok.dev + names: + categories: + - toolhive + kind: MCPAuthzConfig + listKind: MCPAuthzConfigList + plural: mcpauthzconfigs + shortNames: + - authzcfg + singular: mcpauthzconfig + scope: Namespaced + versions: + - additionalPrinterColumns: + - jsonPath: .spec.type + name: Type + type: string + - jsonPath: .status.conditions[?(@.type=='Valid')].status + name: Valid + type: string + - jsonPath: .metadata.creationTimestamp + name: Age + type: date + name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + MCPAuthzConfig is the Schema for the mcpauthzconfigs API. + MCPAuthzConfig resources are namespace-scoped and can only be referenced by + MCPServer, MCPRemoteProxy, or VirtualMCPServer resources within the same namespace. + Cross-namespace references are not supported for security and isolation reasons. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: |- + MCPAuthzConfigSpec defines the desired state of MCPAuthzConfig. + MCPAuthzConfig resources are namespace-scoped and can only be referenced by + MCPServer, MCPRemoteProxy, or VirtualMCPServer resources in the same namespace. + properties: + config: + description: |- + Config contains the backend-specific authorization configuration. + The structure depends on the Type field: + - cedarv1: policies ([]string), entities_json (string), primary_upstream_provider (string), group_claim_name (string) + - httpv1: http ({url, timeout, insecure_skip_verify}), context ({include_args, include_operation}), claim_mapping (string) + type: object + x-kubernetes-preserve-unknown-fields: true + type: + description: |- + Type identifies the authorizer backend (e.g., "cedarv1", "httpv1"). + Must match a registered authorizer type in the factory registry. + minLength: 1 + type: string + required: + - config + - type + type: object + status: + description: MCPAuthzConfigStatus defines the observed state of MCPAuthzConfig + properties: + conditions: + description: Conditions represent the latest available observations + of the MCPAuthzConfig's state + items: + description: Condition contains details for one aspect of the current + state of this API Resource. + properties: + lastTransitionTime: + description: |- + lastTransitionTime is the last time the condition transitioned from one status to another. + This should be when the underlying condition changed. If that is not known, then using the time when the API field changed is acceptable. + format: date-time + type: string + message: + description: |- + message is a human readable message indicating details about the transition. + This may be an empty string. + maxLength: 32768 + type: string + observedGeneration: + description: |- + observedGeneration represents the .metadata.generation that the condition was set based upon. + For instance, if .metadata.generation is currently 12, but the .status.conditions[x].observedGeneration is 9, the condition is out of date + with respect to the current state of the instance. + format: int64 + minimum: 0 + type: integer + reason: + description: |- + reason contains a programmatic identifier indicating the reason for the condition's last transition. + Producers of specific condition types may define expected values and meanings for this field, + and whether the values are considered a guaranteed API. + The value should be a CamelCase string. + This field may not be empty. + maxLength: 1024 + minLength: 1 + pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$ + type: string + status: + description: status of the condition, one of True, False, Unknown. + enum: + - "True" + - "False" + - Unknown + type: string + type: + description: type of condition in CamelCase or in foo.example.com/CamelCase. + maxLength: 316 + pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$ + type: string + required: + - lastTransitionTime + - message + - reason + - status + - type + type: object + type: array + x-kubernetes-list-map-keys: + - type + x-kubernetes-list-type: map + configHash: + description: ConfigHash is a hash of the current configuration for + change detection + type: string + observedGeneration: + description: ObservedGeneration is the most recent generation observed + for this MCPAuthzConfig. + format: int64 + type: integer + referencingWorkloads: + description: |- + ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig. + Each entry identifies the workload by kind and name. + items: + description: |- + WorkloadReference identifies a workload that references a shared configuration resource. + Namespace is implicit — cross-namespace references are not supported. + properties: + kind: + description: Kind is the type of workload resource + enum: + - MCPServer + - VirtualMCPServer + - MCPRemoteProxy + type: string + name: + description: Name is the name of the workload resource + minLength: 1 + type: string + required: + - kind + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + type: object + type: object + served: true + storage: true + subresources: + status: {} +{{- end }} 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 e477567068..ad4aeec5f9 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -97,8 +97,10 @@ spec: - name type: object authzConfig: - description: AuthzConfig defines authorization policy configuration - for the proxy + description: |- + AuthzConfig defines authorization policy configuration for the proxy. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -153,6 +155,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this MCPRemoteProxy. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource in + the same namespace. + minLength: 1 + type: string + required: + - name + type: object endpointPrefix: description: |- EndpointPrefix is the path prefix to prepend to SSE endpoint URLs. 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 9f2732ef2a..0f5f5980d8 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml @@ -102,8 +102,10 @@ spec: - name type: object authzConfig: - description: AuthzConfig defines authorization policy configuration - for the MCP server + description: |- + AuthzConfig defines authorization policy configuration for the MCP server. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -158,6 +160,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this MCPServer. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource in + the same namespace. + minLength: 1 + type: string + required: + - name + type: object backendReplicas: description: |- BackendReplicas is the desired number of MCP server backend pod replicas. 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 572d3ba010..ed65160ea5 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -1796,8 +1796,9 @@ spec: properties: authzConfig: description: |- - AuthzConfig defines authorization policy configuration - Reuses MCPServer authz patterns + AuthzConfig defines authorization policy configuration. + Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead. + AuthzConfig and AuthzConfigRef are mutually exclusive. properties: configMap: description: |- @@ -1852,6 +1853,20 @@ spec: - message: inline must be set when type is 'inline', and must not be set otherwise rule: 'self.type == ''inline'' ? has(self.inline) : !has(self.inline)' + authzConfigRef: + description: |- + AuthzConfigRef references a shared MCPAuthzConfig resource for authorization. + The referenced MCPAuthzConfig must exist in the same namespace as this VirtualMCPServer. + Mutually exclusive with authzConfig. + properties: + name: + description: Name is the name of the MCPAuthzConfig resource + in the same namespace. + minLength: 1 + type: string + required: + - name + type: object oidcConfig: description: |- OIDCConfig defines OIDC authentication configuration. diff --git a/deploy/charts/operator/templates/clusterrole/role.yaml b/deploy/charts/operator/templates/clusterrole/role.yaml index 120b549df6..a6d3ceeb35 100644 --- a/deploy/charts/operator/templates/clusterrole/role.yaml +++ b/deploy/charts/operator/templates/clusterrole/role.yaml @@ -99,6 +99,7 @@ rules: - toolhive.stacklok.dev resources: - embeddingservers + - mcpauthzconfigs - mcpexternalauthconfigs - mcpgroups - mcpoidcconfigs @@ -119,6 +120,7 @@ rules: - toolhive.stacklok.dev resources: - embeddingservers/finalizers + - mcpauthzconfigs/finalizers - mcpexternalauthconfigs/finalizers - mcpgroups/finalizers - mcpoidcconfigs/finalizers @@ -132,6 +134,7 @@ rules: - toolhive.stacklok.dev resources: - embeddingservers/status + - mcpauthzconfigs/status - mcpexternalauthconfigs/status - mcpgroups/status - mcpoidcconfigs/status diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 4789885ba7..1b439ca388 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -735,6 +735,8 @@ _Appears in:_ ### Resource Types - [api.v1alpha1.EmbeddingServer](#apiv1alpha1embeddingserver) - [api.v1alpha1.EmbeddingServerList](#apiv1alpha1embeddingserverlist) +- [api.v1alpha1.MCPAuthzConfig](#apiv1alpha1mcpauthzconfig) +- [api.v1alpha1.MCPAuthzConfigList](#apiv1alpha1mcpauthzconfiglist) - [api.v1alpha1.MCPExternalAuthConfig](#apiv1alpha1mcpexternalauthconfig) - [api.v1alpha1.MCPExternalAuthConfigList](#apiv1alpha1mcpexternalauthconfiglist) - [api.v1alpha1.MCPGroup](#apiv1alpha1mcpgroup) @@ -1286,7 +1288,8 @@ _Appears in:_ | `type` _string_ | Type defines the authentication type: anonymous or oidc
When no authentication is required, explicitly set this to "anonymous" | | Enum: [anonymous oidc]
Required: \{\}
| | `oidcConfig` _[api.v1alpha1.OIDCConfigRef](#apiv1alpha1oidcconfigref)_ | OIDCConfig defines OIDC authentication configuration.
Deprecated: Use OIDCConfigRef to reference a shared MCPOIDCConfig resource instead.
This field will be removed in v1beta1. OIDCConfig and OIDCConfigRef are mutually exclusive. | | Optional: \{\}
| | `oidcConfigRef` _[api.v1alpha1.MCPOIDCConfigReference](#apiv1alpha1mcpoidcconfigreference)_ | OIDCConfigRef references a shared MCPOIDCConfig resource for OIDC authentication.
The referenced MCPOIDCConfig must exist in the same namespace as this VirtualMCPServer.
Per-server overrides (audience, scopes) are specified here; shared provider config
lives in the MCPOIDCConfig resource. Mutually exclusive with oidcConfig. | | Optional: \{\}
| -| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration
Reuses MCPServer authz patterns | | Optional: \{\}
| +| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration.
Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead.
AuthzConfig and AuthzConfigRef are mutually exclusive. | | Optional: \{\}
| +| `authzConfigRef` _[api.v1alpha1.MCPAuthzConfigReference](#apiv1alpha1mcpauthzconfigreference)_ | AuthzConfigRef references a shared MCPAuthzConfig resource for authorization.
The referenced MCPAuthzConfig must exist in the same namespace as this VirtualMCPServer.
Mutually exclusive with authzConfig. | | Optional: \{\}
| #### api.v1alpha1.InlineAuthzConfig @@ -1404,6 +1407,108 @@ _Appears in:_ | `useClusterAuth` _boolean_ | UseClusterAuth enables using the Kubernetes cluster's CA bundle and service account token.
When true, uses /var/run/secrets/kubernetes.io/serviceaccount/ca.crt for TLS verification
and /var/run/secrets/kubernetes.io/serviceaccount/token for bearer token authentication.
Defaults to true if not specified. | | Optional: \{\}
| +#### api.v1alpha1.MCPAuthzConfig + + + +MCPAuthzConfig is the Schema for the mcpauthzconfigs API. +MCPAuthzConfig resources are namespace-scoped and can only be referenced by +MCPServer, MCPRemoteProxy, or VirtualMCPServer resources within the same namespace. +Cross-namespace references are not supported for security and isolation reasons. + + + +_Appears in:_ +- [api.v1alpha1.MCPAuthzConfigList](#apiv1alpha1mcpauthzconfiglist) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `apiVersion` _string_ | `toolhive.stacklok.dev/v1alpha1` | | | +| `kind` _string_ | `MCPAuthzConfig` | | | +| `kind` _string_ | Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | | Optional: \{\}
| +| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | | Optional: \{\}
| +| `metadata` _[ObjectMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#objectmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | | +| `spec` _[api.v1alpha1.MCPAuthzConfigSpec](#apiv1alpha1mcpauthzconfigspec)_ | | | | +| `status` _[api.v1alpha1.MCPAuthzConfigStatus](#apiv1alpha1mcpauthzconfigstatus)_ | | | | + + +#### api.v1alpha1.MCPAuthzConfigList + + + +MCPAuthzConfigList contains a list of MCPAuthzConfig + + + + + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `apiVersion` _string_ | `toolhive.stacklok.dev/v1alpha1` | | | +| `kind` _string_ | `MCPAuthzConfigList` | | | +| `kind` _string_ | Kind is a string value representing the REST resource this object represents.
Servers may infer this from the endpoint the client submits requests to.
Cannot be updated.
In CamelCase.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | | Optional: \{\}
| +| `apiVersion` _string_ | APIVersion defines the versioned schema of this representation of an object.
Servers should convert recognized schemas to the latest internal value, and
may reject unrecognized values.
More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | | Optional: \{\}
| +| `metadata` _[ListMeta](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#listmeta-v1-meta)_ | Refer to Kubernetes API documentation for fields of `metadata`. | | | +| `items` _[api.v1alpha1.MCPAuthzConfig](#apiv1alpha1mcpauthzconfig) array_ | | | | + + +#### api.v1alpha1.MCPAuthzConfigReference + + + +MCPAuthzConfigReference references a shared MCPAuthzConfig resource. +The referenced MCPAuthzConfig must be in the same namespace as the referencing workload. + + + +_Appears in:_ +- [api.v1alpha1.IncomingAuthConfig](#apiv1alpha1incomingauthconfig) +- [api.v1alpha1.MCPRemoteProxySpec](#apiv1alpha1mcpremoteproxyspec) +- [api.v1alpha1.MCPServerSpec](#apiv1alpha1mcpserverspec) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `name` _string_ | Name is the name of the MCPAuthzConfig resource in the same namespace. | | MinLength: 1
Required: \{\}
| + + +#### api.v1alpha1.MCPAuthzConfigSpec + + + +MCPAuthzConfigSpec defines the desired state of MCPAuthzConfig. +MCPAuthzConfig resources are namespace-scoped and can only be referenced by +MCPServer, MCPRemoteProxy, or VirtualMCPServer resources in the same namespace. + + + +_Appears in:_ +- [api.v1alpha1.MCPAuthzConfig](#apiv1alpha1mcpauthzconfig) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `type` _string_ | Type identifies the authorizer backend (e.g., "cedarv1", "httpv1").
Must match a registered authorizer type in the factory registry. | | MinLength: 1
Required: \{\}
| +| `config` _[RawExtension](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#rawextension-runtime-pkg)_ | Config contains the backend-specific authorization configuration.
The structure depends on the Type field:
- cedarv1: policies ([]string), entities_json (string), primary_upstream_provider (string), group_claim_name (string)
- httpv1: http (\{url, timeout, insecure_skip_verify\}), context (\{include_args, include_operation\}), claim_mapping (string) | | Type: object
| + + +#### api.v1alpha1.MCPAuthzConfigStatus + + + +MCPAuthzConfigStatus defines the observed state of MCPAuthzConfig + + + +_Appears in:_ +- [api.v1alpha1.MCPAuthzConfig](#apiv1alpha1mcpauthzconfig) + +| Field | Description | Default | Validation | +| --- | --- | --- | --- | +| `conditions` _[Condition](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.27/#condition-v1-meta) array_ | Conditions represent the latest available observations of the MCPAuthzConfig's state | | Optional: \{\}
| +| `observedGeneration` _integer_ | ObservedGeneration is the most recent generation observed for this MCPAuthzConfig. | | Optional: \{\}
| +| `configHash` _string_ | ConfigHash is a hash of the current configuration for change detection | | Optional: \{\}
| +| `referencingWorkloads` _[api.v1alpha1.WorkloadReference](#apiv1alpha1workloadreference) array_ | ReferencingWorkloads is a list of workload resources that reference this MCPAuthzConfig.
Each entry identifies the workload by kind and name. | | Optional: \{\}
| + + #### api.v1alpha1.MCPExternalAuthConfig @@ -1904,7 +2009,8 @@ _Appears in:_ | `externalAuthConfigRef` _[api.v1alpha1.ExternalAuthConfigRef](#apiv1alpha1externalauthconfigref)_ | ExternalAuthConfigRef references a MCPExternalAuthConfig resource for token exchange.
When specified, the proxy will exchange validated incoming tokens for remote service tokens.
The referenced MCPExternalAuthConfig must exist in the same namespace as this MCPRemoteProxy. | | Optional: \{\}
| | `authServerRef` _[api.v1alpha1.AuthServerRef](#apiv1alpha1authserverref)_ | AuthServerRef optionally references a resource that configures an embedded
OAuth 2.0/OIDC authorization server to authenticate MCP clients.
Currently the only supported kind is MCPExternalAuthConfig (type: embeddedAuthServer). | | Optional: \{\}
| | `headerForward` _[api.v1alpha1.HeaderForwardConfig](#apiv1alpha1headerforwardconfig)_ | HeaderForward configures headers to inject into requests to the remote MCP server.
Use this to add custom headers like X-Tenant-ID or correlation IDs. | | Optional: \{\}
| -| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration for the proxy | | Optional: \{\}
| +| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration for the proxy.
Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead.
AuthzConfig and AuthzConfigRef are mutually exclusive. | | Optional: \{\}
| +| `authzConfigRef` _[api.v1alpha1.MCPAuthzConfigReference](#apiv1alpha1mcpauthzconfigreference)_ | AuthzConfigRef references a shared MCPAuthzConfig resource for authorization.
The referenced MCPAuthzConfig must exist in the same namespace as this MCPRemoteProxy.
Mutually exclusive with authzConfig. | | Optional: \{\}
| | `audit` _[api.v1alpha1.AuditConfig](#apiv1alpha1auditconfig)_ | Audit defines audit logging configuration for the proxy | | Optional: \{\}
| | `toolConfigRef` _[api.v1alpha1.ToolConfigRef](#apiv1alpha1toolconfigref)_ | ToolConfigRef references a MCPToolConfig resource for tool filtering and renaming.
The referenced MCPToolConfig must exist in the same namespace as this MCPRemoteProxy.
Cross-namespace references are not supported for security and isolation reasons.
If specified, this allows filtering and overriding tools from the remote MCP server. | | Optional: \{\}
| | `telemetry` _[api.v1alpha1.TelemetryConfig](#apiv1alpha1telemetryconfig)_ | Telemetry defines observability configuration for the proxy | | Optional: \{\}
| @@ -2138,7 +2244,8 @@ _Appears in:_ | `resourceOverrides` _[api.v1alpha1.ResourceOverrides](#apiv1alpha1resourceoverrides)_ | ResourceOverrides allows overriding annotations and labels for resources created by the operator | | Optional: \{\}
| | `oidcConfig` _[api.v1alpha1.OIDCConfigRef](#apiv1alpha1oidcconfigref)_ | OIDCConfig defines OIDC authentication configuration for the MCP server.
Deprecated: Use OIDCConfigRef to reference a shared MCPOIDCConfig resource instead.
This field will be removed in v1beta1. OIDCConfig and OIDCConfigRef are mutually exclusive. | | Optional: \{\}
| | `oidcConfigRef` _[api.v1alpha1.MCPOIDCConfigReference](#apiv1alpha1mcpoidcconfigreference)_ | OIDCConfigRef references a shared MCPOIDCConfig resource for OIDC authentication.
The referenced MCPOIDCConfig must exist in the same namespace as this MCPServer.
Per-server overrides (audience, scopes) are specified here; shared provider config
lives in the MCPOIDCConfig resource. Mutually exclusive with oidcConfig. | | Optional: \{\}
| -| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration for the MCP server | | Optional: \{\}
| +| `authzConfig` _[api.v1alpha1.AuthzConfigRef](#apiv1alpha1authzconfigref)_ | AuthzConfig defines authorization policy configuration for the MCP server.
Deprecated: Use AuthzConfigRef to reference a shared MCPAuthzConfig resource instead.
AuthzConfig and AuthzConfigRef are mutually exclusive. | | Optional: \{\}
| +| `authzConfigRef` _[api.v1alpha1.MCPAuthzConfigReference](#apiv1alpha1mcpauthzconfigreference)_ | AuthzConfigRef references a shared MCPAuthzConfig resource for authorization.
The referenced MCPAuthzConfig must exist in the same namespace as this MCPServer.
Mutually exclusive with authzConfig. | | Optional: \{\}
| | `audit` _[api.v1alpha1.AuditConfig](#apiv1alpha1auditconfig)_ | Audit defines audit logging configuration for the MCP server | | Optional: \{\}
| | `toolConfigRef` _[api.v1alpha1.ToolConfigRef](#apiv1alpha1toolconfigref)_ | ToolConfigRef references a MCPToolConfig resource for tool filtering and renaming.
The referenced MCPToolConfig must exist in the same namespace as this MCPServer.
Cross-namespace references are not supported for security and isolation reasons. | | Optional: \{\}
| | `externalAuthConfigRef` _[api.v1alpha1.ExternalAuthConfigRef](#apiv1alpha1externalauthconfigref)_ | ExternalAuthConfigRef references a MCPExternalAuthConfig resource for external authentication.
The referenced MCPExternalAuthConfig must exist in the same namespace as this MCPServer. | | Optional: \{\}
| @@ -3477,6 +3584,7 @@ Namespace is implicit — cross-namespace references are not supported. _Appears in:_ +- [api.v1alpha1.MCPAuthzConfigStatus](#apiv1alpha1mcpauthzconfigstatus) - [api.v1alpha1.MCPExternalAuthConfigStatus](#apiv1alpha1mcpexternalauthconfigstatus) - [api.v1alpha1.MCPOIDCConfigStatus](#apiv1alpha1mcpoidcconfigstatus) - [api.v1alpha1.MCPTelemetryConfigStatus](#apiv1alpha1mcptelemetryconfigstatus) diff --git a/pkg/authz/authorizers/cedar/core.go b/pkg/authz/authorizers/cedar/core.go index bc1eb0a964..7803f52ed4 100644 --- a/pkg/authz/authorizers/cedar/core.go +++ b/pkg/authz/authorizers/cedar/core.go @@ -98,6 +98,9 @@ func InjectUpstreamProvider(src *authorizers.Config, providerName string) (*auth // Factory implements the authorizers.AuthorizerFactory interface for Cedar. type Factory struct{} +// ConfigKey returns the JSON key for Cedar-specific configuration ("cedar"). +func (*Factory) ConfigKey() string { return "cedar" } + // ValidateConfig validates the Cedar-specific configuration. // It receives the full raw config and extracts the Cedar-specific portion. func (*Factory) ValidateConfig(rawConfig json.RawMessage) error { diff --git a/pkg/authz/authorizers/config_test.go b/pkg/authz/authorizers/config_test.go index cd30d58c8f..e65c3e786d 100644 --- a/pkg/authz/authorizers/config_test.go +++ b/pkg/authz/authorizers/config_test.go @@ -20,6 +20,8 @@ const testConfigType = "test-config-type" // testFactory is a simple test factory for config tests type testFactory struct{} +func (*testFactory) ConfigKey() string { return "test" } + func (*testFactory) ValidateConfig(rawConfig json.RawMessage) error { var config struct { TestField string `json:"test_field"` diff --git a/pkg/authz/authorizers/http/core.go b/pkg/authz/authorizers/http/core.go index f51da15dee..173dfe1130 100644 --- a/pkg/authz/authorizers/http/core.go +++ b/pkg/authz/authorizers/http/core.go @@ -21,6 +21,9 @@ func init() { // Factory implements the authorizers.AuthorizerFactory interface for HTTP PDPs. type Factory struct{} +// ConfigKey returns the JSON key for HTTP PDP-specific configuration ("pdp"). +func (*Factory) ConfigKey() string { return "pdp" } + // ValidateConfig validates the HTTP PDP configuration. func (*Factory) ValidateConfig(rawConfig json.RawMessage) error { config, err := parseConfig(rawConfig) diff --git a/pkg/authz/authorizers/registry.go b/pkg/authz/authorizers/registry.go index c0cd7e3c41..ced43d55d0 100644 --- a/pkg/authz/authorizers/registry.go +++ b/pkg/authz/authorizers/registry.go @@ -14,6 +14,11 @@ import ( // (e.g., Cedar, OPA) implements this interface to provide validation and // instantiation of authorizers from their specific configuration format. type AuthorizerFactory interface { + // ConfigKey returns the JSON key under which the backend-specific + // configuration is nested in the full authorizer config blob. + // For example, Cedar returns "cedar" and HTTP PDP returns "pdp". + ConfigKey() string + // ValidateConfig validates the authorizer-specific configuration. // The rawConfig is the JSON-encoded authorizer configuration. ValidateConfig(rawConfig json.RawMessage) error diff --git a/pkg/authz/authorizers/registry_test.go b/pkg/authz/authorizers/registry_test.go index ddfa03b9ce..f2e925a1f2 100644 --- a/pkg/authz/authorizers/registry_test.go +++ b/pkg/authz/authorizers/registry_test.go @@ -18,6 +18,8 @@ type mockFactory struct { authorizer Authorizer } +func (*mockFactory) ConfigKey() string { return "mock" } + func (f *mockFactory) ValidateConfig(_ json.RawMessage) error { return f.validateErr } From 440ffe3c38f75ca19be64cd2cafb305183d5a442 Mon Sep 17 00:00:00 2001 From: Juan Antonio Osorio Date: Mon, 13 Apr 2026 14:17:20 +0000 Subject: [PATCH 2/2] Wire MCPAuthzConfig references into workload controllers MCPAuthzConfig resources created in PR 1 need to be consumed by the workload controllers (MCPServer, MCPRemoteProxy, VirtualMCPServer) so that shared authorization configs actually take effect at runtime. - Add CEL mutual exclusion for authzConfig/authzConfigRef on all specs - Add AuthzConfigHash to all workload status types for change detection - Add condition constants (AuthzConfigRefValidated) and reasons - Move BuildFullAuthzConfigJSON to controllerutil for shared use - Add controllerutil helpers: GetAuthzConfigForWorkload, ValidateAuthzConfigReady, GenerateAuthzVolumeConfigFromRef, EnsureAuthzConfigMapFromRef, AddAuthzConfigRefOptions - Add handleAuthzConfig to MCPServer, MCPRemoteProxy, VirtualMCPServer controllers following the OIDC config ref pattern - Add MCPAuthzConfig watches to all three workload controllers - Update runconfig builders to resolve AuthzConfigRef - Update vmcpconfig converter to extract Cedar policies from MCPAuthzConfig for VirtualMCPServer runtime config - Add SetAuthzConfigHash to VirtualMCPServer status manager Co-Authored-By: Claude Opus 4.6 (1M context) --- .../api/v1alpha1/mcpremoteproxy_types.go | 5 + .../api/v1alpha1/mcpserver_types.go | 22 +++ .../api/v1alpha1/virtualmcpserver_types.go | 6 + .../controllers/mcpauthzconfig_controller.go | 39 +--- .../mcpauthzconfig_controller_test.go | 3 +- .../controllers/mcpremoteproxy_controller.go | 124 +++++++++++++ .../controllers/mcpremoteproxy_runconfig.go | 22 ++- .../controllers/mcpserver_controller.go | 139 ++++++++++++++- .../controllers/mcpserver_runconfig.go | 11 +- .../virtualmcpserver_controller.go | 122 ++++++++++++- cmd/thv-operator/pkg/controllerutil/authz.go | 167 ++++++++++++++++++ .../pkg/virtualmcpserverstatus/collector.go | 13 ++ .../mocks/mock_collector.go | 12 ++ .../pkg/virtualmcpserverstatus/types.go | 3 + cmd/thv-operator/pkg/vmcpconfig/converter.go | 51 +++++- ...oolhive.stacklok.dev_mcpremoteproxies.yaml | 7 + .../toolhive.stacklok.dev_mcpservers.yaml | 7 + ...olhive.stacklok.dev_virtualmcpservers.yaml | 8 + ...oolhive.stacklok.dev_mcpremoteproxies.yaml | 7 + .../toolhive.stacklok.dev_mcpservers.yaml | 7 + ...olhive.stacklok.dev_virtualmcpservers.yaml | 8 + docs/operator/crd-api.md | 3 + 22 files changed, 736 insertions(+), 50 deletions(-) diff --git a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go index 6079ed6532..5df931181d 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go @@ -38,6 +38,7 @@ type HeaderFromSecret struct { // MCPRemoteProxySpec defines the desired state of MCPRemoteProxy // // +kubebuilder:validation:XValidation:rule="!(has(self.oidcConfig) && has(self.oidcConfigRef))",message="oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig" +// +kubebuilder:validation:XValidation:rule="!(has(self.authzConfig) && has(self.authzConfigRef))",message="authzConfig and authzConfigRef are mutually exclusive; use authzConfigRef to reference a shared MCPAuthzConfig" // //nolint:lll // CEL validation rules exceed line length limit type MCPRemoteProxySpec struct { @@ -195,6 +196,10 @@ type MCPRemoteProxyStatus struct { // +optional OIDCConfigHash string `json:"oidcConfigHash,omitempty"` + // AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection + // +optional + AuthzConfigHash string `json:"authzConfigHash,omitempty"` + // Message provides additional information about the current phase // +optional Message string `json:"message,omitempty"` diff --git a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go index 092a3aae49..fe58cb7fe4 100644 --- a/cmd/thv-operator/api/v1alpha1/mcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/mcpserver_types.go @@ -86,6 +86,23 @@ const ( ConditionReasonOIDCConfigRefError = "OIDCConfigRefError" ) +// Condition type for MCPAuthzConfig reference validation +const ( + // ConditionAuthzConfigRefValidated indicates whether the AuthzConfigRef is valid + ConditionAuthzConfigRefValidated = "AuthzConfigRefValidated" +) + +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" +) + const ( // ConditionReasonCABundleRefValid indicates the CABundleRef is valid and the ConfigMap exists ConditionReasonCABundleRefValid = "CABundleRefValid" @@ -190,6 +207,7 @@ const SessionStorageProviderRedis = "redis" // MCPServerSpec defines the desired state of MCPServer // // +kubebuilder:validation:XValidation:rule="!(has(self.oidcConfig) && has(self.oidcConfigRef))",message="oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig" +// +kubebuilder:validation:XValidation:rule="!(has(self.authzConfig) && has(self.authzConfigRef))",message="authzConfig and authzConfigRef are mutually exclusive; use authzConfigRef to reference a shared MCPAuthzConfig" // +kubebuilder:validation:XValidation:rule="!(has(self.telemetry) && has(self.telemetryConfigRef))",message="telemetry and telemetryConfigRef are mutually exclusive; migrate to telemetryConfigRef" // +kubebuilder:validation:XValidation:rule="!has(self.rateLimiting) || (has(self.sessionStorage) && self.sessionStorage.provider == 'redis')",message="rateLimiting requires sessionStorage with provider 'redis'" // +kubebuilder:validation:XValidation:rule="!(has(self.rateLimiting) && has(self.rateLimiting.perUser)) || has(self.oidcConfig) || has(self.oidcConfigRef) || has(self.externalAuthConfigRef)",message="rateLimiting.perUser requires authentication (oidcConfig, oidcConfigRef, or externalAuthConfigRef)" @@ -1069,6 +1087,10 @@ type MCPServerStatus struct { // +optional TelemetryConfigHash string `json:"telemetryConfigHash,omitempty"` + // AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection + // +optional + AuthzConfigHash string `json:"authzConfigHash,omitempty"` + // URL is the URL where the MCP server can be accessed // +optional URL string `json:"url,omitempty"` diff --git a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go index 7784d4929f..8c918082dc 100644 --- a/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go +++ b/cmd/thv-operator/api/v1alpha1/virtualmcpserver_types.go @@ -111,6 +111,7 @@ type EmbeddingServerRef struct { // // +kubebuilder:validation:XValidation:rule="self.type == 'oidc' ? (has(self.oidcConfig) || has(self.oidcConfigRef)) : true",message="spec.incomingAuth.oidcConfig or oidcConfigRef is required when type is oidc" // +kubebuilder:validation:XValidation:rule="!(has(self.oidcConfig) && has(self.oidcConfigRef))",message="oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig" +// +kubebuilder:validation:XValidation:rule="!(has(self.authzConfig) && has(self.authzConfigRef))",message="authzConfig and authzConfigRef are mutually exclusive; use authzConfigRef to reference a shared MCPAuthzConfig" // //nolint:lll // CEL validation rules exceed line length limit type IncomingAuthConfig struct { @@ -235,6 +236,11 @@ type VirtualMCPServerStatus struct { // Only populated when IncomingAuth.OIDCConfigRef is set. // +optional OIDCConfigHash string `json:"oidcConfigHash,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"` } // VirtualMCPServerPhase represents the lifecycle phase of a VirtualMCPServer diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go index 4cc0930584..8a44f286e7 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go @@ -35,9 +35,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. @@ -154,7 +151,7 @@ func (r *MCPAuthzConfigReconciler) Reconcile(ctx context.Context, req ctrl.Reque // validateAuthzConfigSpec validates the MCPAuthzConfig spec by reconstructing the // full authorizer config and delegating to the factory's ValidateConfig method. func validateAuthzConfigSpec(spec mcpv1alpha1.MCPAuthzConfigSpec) error { - fullConfigJSON, err := BuildFullAuthzConfigJSON(spec) + fullConfigJSON, err := ctrlutil.BuildFullAuthzConfigJSON(spec) if err != nil { return err } @@ -183,40 +180,6 @@ type authzConfig struct { Type string `json:"type"` } -// BuildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a -// MCPAuthzConfig spec. The result is the same format accepted by authorizers.Config -// and used in ConfigMaps: {"version": "1.0", "type": "", "": {}}. -func BuildFullAuthzConfigJSON(spec mcpv1alpha1.MCPAuthzConfigSpec) ([]byte, error) { - factory := authorizers.GetFactory(spec.Type) - if factory == nil { - return nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", - spec.Type, authorizers.RegisteredTypes()) - } - - configKey := factory.ConfigKey() - - fullConfig := map[string]json.RawMessage{ - "version": mustMarshalJSON(authzConfigVersion), - "type": mustMarshalJSON(spec.Type), - configKey: spec.Config.Raw, - } - - result, err := json.Marshal(fullConfig) - if err != nil { - return nil, fmt.Errorf("failed to marshal full authz config: %w", err) - } - return result, nil -} - -// mustMarshalJSON marshals a value to JSON or panics. Only for simple string values. -func mustMarshalJSON(v string) json.RawMessage { - b, err := json.Marshal(v) - if err != nil { - panic(fmt.Sprintf("failed to marshal %q: %v", v, err)) - } - return b -} - // handleDeletion handles the deletion of a MCPAuthzConfig. // Blocks deletion while workload resources reference this config by keeping the // finalizer and requeueing. Once all references are removed, the finalizer is removed diff --git a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go index 819b5a0940..e526f3f184 100644 --- a/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go @@ -19,6 +19,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" + ctrlutil "github.com/stacklok/toolhive/cmd/thv-operator/pkg/controllerutil" // Import authorizer backends so they register with the factory registry. _ "github.com/stacklok/toolhive/pkg/authz/authorizers/cedar" _ "github.com/stacklok/toolhive/pkg/authz/authorizers/http" @@ -85,7 +86,7 @@ func TestBuildFullAuthzConfigJSON(t *testing.T) { t.Run(tt.name, func(t *testing.T) { t.Parallel() - result, err := BuildFullAuthzConfigJSON(tt.spec) + result, err := ctrlutil.BuildFullAuthzConfigJSON(tt.spec) if tt.expectError { require.Error(t, err) diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go index 95d6146404..d699b16114 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_controller.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_controller.go @@ -158,6 +158,16 @@ func (r *MCPRemoteProxyReconciler) validateAndHandleConfigs(ctx context.Context, return err } + // Handle MCPAuthzConfig + if err := r.handleAuthzConfig(ctx, proxy); err != nil { + ctxLogger.Error(err, "Failed to handle MCPAuthzConfig") + proxy.Status.Phase = mcpv1alpha1.MCPRemoteProxyPhaseFailed + if statusErr := r.Status().Update(ctx, proxy); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update MCPRemoteProxy status after MCPAuthzConfig error") + } + return err + } + return nil } @@ -210,6 +220,85 @@ func (r *MCPRemoteProxyReconciler) ensureAuthzConfigMapForProxy(ctx context.Cont ) } +// handleAuthzConfig validates and tracks the hash of the referenced MCPAuthzConfig. +// It updates the MCPRemoteProxy status when the authorization configuration changes and sets +// the AuthzConfigRefValidated condition. +func (r *MCPRemoteProxyReconciler) handleAuthzConfig(ctx context.Context, proxy *mcpv1alpha1.MCPRemoteProxy) error { + ctxLogger := log.FromContext(ctx) + + if proxy.Spec.AuthzConfigRef == nil { + meta.RemoveStatusCondition(&proxy.Status.Conditions, mcpv1alpha1.ConditionAuthzConfigRefValidated) + if proxy.Status.AuthzConfigHash != "" { + proxy.Status.AuthzConfigHash = "" + if err := r.Status().Update(ctx, proxy); err != nil { + return fmt.Errorf("failed to clear MCPAuthzConfig hash from status: %w", err) + } + } + return nil + } + + // Fetch the referenced MCPAuthzConfig + authzConfig, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, proxy.Namespace, proxy.Spec.AuthzConfigRef) + if err != nil { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionAuthzConfigRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthzConfigRefNotFound, + Message: fmt.Sprintf("MCPAuthzConfig %s not found: %v", proxy.Spec.AuthzConfigRef.Name, err), + ObservedGeneration: proxy.Generation, + }) + if statusErr := r.Status().Update(ctx, proxy); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig lookup error") + } + return err + } + + // Validate that the MCPAuthzConfig is ready + if err := ctrlutil.ValidateAuthzConfigReady(authzConfig); err != nil { + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionAuthzConfigRefValidated, + Status: metav1.ConditionFalse, + Reason: mcpv1alpha1.ConditionReasonAuthzConfigRefNotValid, + Message: err.Error(), + ObservedGeneration: proxy.Generation, + }) + if statusErr := r.Status().Update(ctx, proxy); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig validation check") + } + return err + } + + // Detect whether the condition is transitioning to True + prevCondition := meta.FindStatusCondition(proxy.Status.Conditions, mcpv1alpha1.ConditionAuthzConfigRefValidated) + needsUpdate := prevCondition == nil || prevCondition.Status != metav1.ConditionTrue + + meta.SetStatusCondition(&proxy.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionAuthzConfigRefValidated, + Status: metav1.ConditionTrue, + Reason: mcpv1alpha1.ConditionReasonAuthzConfigRefValid, + Message: fmt.Sprintf("MCPAuthzConfig %s is valid and ready", proxy.Spec.AuthzConfigRef.Name), + ObservedGeneration: proxy.Generation, + }) + + if proxy.Status.AuthzConfigHash != authzConfig.Status.ConfigHash { + ctxLogger.Info("MCPAuthzConfig has changed, updating MCPRemoteProxy", + "proxy", proxy.Name, + "authzConfig", authzConfig.Name, + "oldHash", proxy.Status.AuthzConfigHash, + "newHash", authzConfig.Status.ConfigHash) + proxy.Status.AuthzConfigHash = authzConfig.Status.ConfigHash + needsUpdate = true + } + + if needsUpdate { + if err := r.Status().Update(ctx, proxy); err != nil { + return fmt.Errorf("failed to update MCPAuthzConfig status: %w", err) + } + } + + return nil +} + // getRunConfigChecksum fetches the RunConfig ConfigMap checksum annotation for this proxy. // Uses the shared RunConfigChecksumFetcher to maintain consistency with MCPServer. func (r *MCPRemoteProxyReconciler) getRunConfigChecksum( @@ -1470,5 +1559,40 @@ func (r *MCPRemoteProxyReconciler) SetupWithManager(mgr ctrl.Manager) error { &mcpv1alpha1.MCPOIDCConfig{}, handler.EnqueueRequestsFromMapFunc(r.mapOIDCConfigToMCPRemoteProxy), ). + Watches( + &mcpv1alpha1.MCPAuthzConfig{}, + handler.EnqueueRequestsFromMapFunc(r.mapAuthzConfigToMCPRemoteProxy), + ). Complete(r) } + +// mapAuthzConfigToMCPRemoteProxy maps MCPAuthzConfig changes to MCPRemoteProxy reconciliation requests. +func (r *MCPRemoteProxyReconciler) mapAuthzConfigToMCPRemoteProxy( + ctx context.Context, obj client.Object, +) []reconcile.Request { + authzConfig, ok := obj.(*mcpv1alpha1.MCPAuthzConfig) + if !ok { + return nil + } + + proxyList := &mcpv1alpha1.MCPRemoteProxyList{} + if err := r.List(ctx, proxyList, client.InNamespace(authzConfig.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list MCPRemoteProxies for MCPAuthzConfig watch") + return nil + } + + var requests []reconcile.Request + for _, proxy := range proxyList.Items { + if proxy.Spec.AuthzConfigRef != nil && + proxy.Spec.AuthzConfigRef.Name == authzConfig.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: proxy.Name, + Namespace: proxy.Namespace, + }, + }) + } + } + + return requests +} diff --git a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go index 21655b24f9..c73365bbb4 100644 --- a/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go +++ b/cmd/thv-operator/controllers/mcpremoteproxy_runconfig.go @@ -135,9 +135,8 @@ func (r *MCPRemoteProxyReconciler) createRunConfigFromMCPRemoteProxy( runconfig.AddTelemetryConfigOptions(ctx, &options, proxy.Spec.Telemetry, proxy.Name) // Add authorization configuration if specified - - if err := ctrlutil.AddAuthzConfigOptions(ctx, r.Client, proxy.Namespace, proxy.Spec.AuthzConfig, &options); err != nil { - return nil, fmt.Errorf("failed to process AuthzConfig: %w", err) + if err := r.addAuthzOptions(ctx, proxy, &options); err != nil { + return nil, err } // Add OIDC configuration (required for proxy mode) @@ -320,3 +319,20 @@ func addHeaderForwardConfigOptions(proxy *mcpv1alpha1.MCPRemoteProxy, options *[ *options = append(*options, runner.WithHeaderForwardSecrets(headerSecrets)) } } + +// addAuthzOptions resolves authorization configuration from either an MCPAuthzConfig +// reference or the deprecated inline AuthzConfig and adds it to the builder options. +func (r *MCPRemoteProxyReconciler) addAuthzOptions( + ctx context.Context, + proxy *mcpv1alpha1.MCPRemoteProxy, + options *[]runner.RunConfigBuilderOption, +) error { + if proxy.Spec.AuthzConfigRef != nil { + authzCfg, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, proxy.Namespace, proxy.Spec.AuthzConfigRef) + if err != nil { + return fmt.Errorf("failed to get MCPAuthzConfig: %w", err) + } + return ctrlutil.AddAuthzConfigRefOptions(authzCfg, options) + } + return ctrlutil.AddAuthzConfigOptions(ctx, r.Client, proxy.Namespace, proxy.Spec.AuthzConfig, options) +} diff --git a/cmd/thv-operator/controllers/mcpserver_controller.go b/cmd/thv-operator/controllers/mcpserver_controller.go index 68c6f96d47..de822f5716 100644 --- a/cmd/thv-operator/controllers/mcpserver_controller.go +++ b/cmd/thv-operator/controllers/mcpserver_controller.go @@ -292,6 +292,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 = mcpv1alpha1.MCPServerPhaseFailed + setReadyCondition(mcpServer, metav1.ConditionFalse, mcpv1alpha1.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 + } + // Validate MCPServer image against enforcing registries imageValidator := validation.NewImageValidator(r.Client, mcpServer.Namespace, r.ImageValidation) err = imageValidator.ValidateImage(ctx, mcpServer.Spec.Image, mcpServer.ObjectMeta) @@ -1196,7 +1207,13 @@ func (r *MCPServerReconciler) deploymentForMCPServer( } // Add volume mounts for authorization configuration - authzVolumeMount, authzVolume := ctrlutil.GenerateAuthzVolumeConfig(m.Spec.AuthzConfig, m.Name) + var authzVolumeMount *corev1.VolumeMount + var authzVolume *corev1.Volume + if m.Spec.AuthzConfigRef != nil { + authzVolumeMount, authzVolume = ctrlutil.GenerateAuthzVolumeConfigFromRef(m.Name) + } else { + authzVolumeMount, authzVolume = ctrlutil.GenerateAuthzVolumeConfig(m.Spec.AuthzConfig, m.Name) + } if authzVolumeMount != nil { volumeMounts = append(volumeMounts, *authzVolumeMount) volumes = append(volumes, *authzVolume) @@ -2321,6 +2338,91 @@ func (r *MCPServerReconciler) ensureAuthzConfigMap(ctx context.Context, m *mcpv1 ) } +// handleAuthzConfig validates and tracks the hash of the referenced MCPAuthzConfig. +// It updates the MCPServer status when the authorization configuration changes and sets +// the AuthzConfigRefValidated condition. +func (r *MCPServerReconciler) handleAuthzConfig(ctx context.Context, m *mcpv1alpha1.MCPServer) error { + ctxLogger := log.FromContext(ctx) + + if m.Spec.AuthzConfigRef == nil { + // No MCPAuthzConfig referenced, clear any stored hash + if m.Status.AuthzConfigHash != "" { + m.Status.AuthzConfigHash = "" + if err := r.Status().Update(ctx, m); err != nil { + return fmt.Errorf("failed to clear MCPAuthzConfig hash from status: %w", err) + } + } + return nil + } + + // Fetch the referenced MCPAuthzConfig + authzConfig, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, m.Namespace, m.Spec.AuthzConfigRef) + if err != nil { + setAuthzConfigRefCondition(m, metav1.ConditionFalse, + mcpv1alpha1.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 err + } + + // Validate that the MCPAuthzConfig is ready + if err := ctrlutil.ValidateAuthzConfigReady(authzConfig); err != nil { + setAuthzConfigRefCondition(m, metav1.ConditionFalse, + mcpv1alpha1.ConditionReasonAuthzConfigRefNotValid, err.Error()) + if statusErr := r.Status().Update(ctx, m); statusErr != nil { + ctxLogger.Error(statusErr, "Failed to update status after MCPAuthzConfig validation check") + } + return err + } + + // Ensure ConfigMap with reconstructed full config for volume mount + if err := ctrlutil.EnsureAuthzConfigMapFromRef( + ctx, r.Client, r.Scheme, m, m.Namespace, m.Name, authzConfig, labelsForInlineAuthzConfig(m.Name), + ); err != nil { + ctxLogger.Error(err, "Failed to ensure authzref ConfigMap") + return err + } + + // Detect whether the condition is transitioning to True + prevCondition := meta.FindStatusCondition(m.Status.Conditions, mcpv1alpha1.ConditionAuthzConfigRefValidated) + needsUpdate := prevCondition == nil || prevCondition.Status != metav1.ConditionTrue + + setAuthzConfigRefCondition(m, metav1.ConditionTrue, + mcpv1alpha1.ConditionReasonAuthzConfigRefValid, + fmt.Sprintf("MCPAuthzConfig %s is valid and ready", m.Spec.AuthzConfigRef.Name)) + + if m.Status.AuthzConfigHash != authzConfig.Status.ConfigHash { + ctxLogger.Info("MCPAuthzConfig has changed, updating MCPServer", + "mcpserver", m.Name, + "authzConfig", authzConfig.Name, + "oldHash", m.Status.AuthzConfigHash, + "newHash", 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 MCPAuthzConfig status: %w", err) + } + } + + return nil +} + +// setAuthzConfigRefCondition sets the AuthzConfigRefValidated status condition +func setAuthzConfigRefCondition(m *mcpv1alpha1.MCPServer, status metav1.ConditionStatus, reason, message string) { + meta.SetStatusCondition(&m.Status.Conditions, metav1.Condition{ + Type: mcpv1alpha1.ConditionAuthzConfigRefValidated, + Status: status, + Reason: reason, + Message: message, + ObservedGeneration: m.Generation, + }) +} + // int32Ptr returns a pointer to an int32 func int32Ptr(i int32) *int32 { return &i @@ -2540,5 +2642,40 @@ func (r *MCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { Watches(&mcpv1alpha1.MCPExternalAuthConfig{}, externalAuthConfigHandler). Watches(&mcpv1alpha1.MCPOIDCConfig{}, oidcConfigHandler). Watches(&mcpv1alpha1.MCPTelemetryConfig{}, telemetryConfigHandler). + Watches( + &mcpv1alpha1.MCPAuthzConfig{}, + handler.EnqueueRequestsFromMapFunc(r.mapAuthzConfigToMCPServer), + ). Complete(r) } + +// mapAuthzConfigToMCPServer maps MCPAuthzConfig changes to MCPServer reconciliation requests. +func (r *MCPServerReconciler) mapAuthzConfigToMCPServer( + ctx context.Context, obj client.Object, +) []reconcile.Request { + authzConfig, ok := obj.(*mcpv1alpha1.MCPAuthzConfig) + if !ok { + return nil + } + + mcpServerList := &mcpv1alpha1.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 +} diff --git a/cmd/thv-operator/controllers/mcpserver_runconfig.go b/cmd/thv-operator/controllers/mcpserver_runconfig.go index 92c1f98d1b..c5b3f01011 100644 --- a/cmd/thv-operator/controllers/mcpserver_runconfig.go +++ b/cmd/thv-operator/controllers/mcpserver_runconfig.go @@ -200,8 +200,15 @@ func (r *MCPServerReconciler) createRunConfigFromMCPServer(m *mcpv1alpha1.MCPSer } // Add authorization configuration if specified - - if err := ctrlutil.AddAuthzConfigOptions(ctx, r.Client, m.Namespace, m.Spec.AuthzConfig, &options); err != nil { + if m.Spec.AuthzConfigRef != nil { + authzCfg, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, m.Namespace, m.Spec.AuthzConfigRef) + if err != nil { + return nil, fmt.Errorf("failed to get MCPAuthzConfig: %w", err) + } + if err := ctrlutil.AddAuthzConfigRefOptions(authzCfg, &options); err != nil { + return nil, fmt.Errorf("failed to process AuthzConfigRef: %w", err) + } + } else if err := ctrlutil.AddAuthzConfigOptions(ctx, r.Client, m.Namespace, m.Spec.AuthzConfig, &options); err != nil { return nil, fmt.Errorf("failed to process AuthzConfig: %w", err) } diff --git a/cmd/thv-operator/controllers/virtualmcpserver_controller.go b/cmd/thv-operator/controllers/virtualmcpserver_controller.go index f55dabfbe1..99a87a28ac 100644 --- a/cmd/thv-operator/controllers/virtualmcpserver_controller.go +++ b/cmd/thv-operator/controllers/virtualmcpserver_controller.go @@ -157,11 +157,10 @@ func (r *VirtualMCPServerReconciler) Reconcile(ctx context.Context, req ctrl.Req return ctrl.Result{}, nil } - // Validate MCPOIDCConfigRef if referenced (before resource creation). - // handleOIDCConfig is a no-op when OIDCConfigRef is nil. - if err := r.handleOIDCConfig(ctx, vmcp, statusManager); err != nil { + // Validate referenced config resources (OIDC, Authz) before resource creation. + if err := r.validateConfigRefs(ctx, vmcp, statusManager); err != nil { if applyErr := r.applyStatusUpdates(ctx, vmcp, statusManager); applyErr != nil { - ctxLogger.Error(applyErr, "Failed to apply status updates after MCPOIDCConfig validation error") + ctxLogger.Error(applyErr, "Failed to apply status updates after config ref validation error") } return ctrl.Result{}, err } @@ -2227,6 +2226,12 @@ func (r *VirtualMCPServerReconciler) SetupWithManager(mgr ctrl.Manager) error { &mcpv1alpha1.MCPOIDCConfig{}, handler.EnqueueRequestsFromMapFunc(r.mapOIDCConfigToVirtualMCPServer), ). + // Watch referenced MCPAuthzConfigs so that validity/hash changes + // trigger VirtualMCPServer reconciliation. + Watches( + &mcpv1alpha1.MCPAuthzConfig{}, + handler.EnqueueRequestsFromMapFunc(r.mapAuthzConfigToVirtualMCPServer), + ). Complete(r) } @@ -2876,6 +2881,19 @@ func generateHMACSecret() (string, error) { return base64.StdEncoding.EncodeToString(secret), nil } +// validateConfigRefs validates referenced config resources (OIDC, Authz) before +// resource creation. Each handler is a no-op when the corresponding ref is nil. +func (r *VirtualMCPServerReconciler) validateConfigRefs( + ctx context.Context, + vmcp *mcpv1alpha1.VirtualMCPServer, + statusManager virtualmcpserverstatus.StatusManager, +) error { + if err := r.handleOIDCConfig(ctx, vmcp, statusManager); err != nil { + return err + } + return r.handleAuthzConfig(ctx, vmcp, statusManager) +} + // handleOIDCConfig validates and tracks the hash of the referenced MCPOIDCConfig. // It sets the OIDCConfigRefValidated condition and triggers reconciliation when // the OIDC configuration changes. @@ -2986,6 +3004,102 @@ func (r *VirtualMCPServerReconciler) updateOIDCConfigReferencingWorkloads( return nil } +// handleAuthzConfig validates and tracks the hash of the referenced MCPAuthzConfig. +// It sets the AuthzConfigRefValidated condition and triggers reconciliation when +// the authorization configuration changes. +func (r *VirtualMCPServerReconciler) handleAuthzConfig( + ctx context.Context, + vmcp *mcpv1alpha1.VirtualMCPServer, + statusManager virtualmcpserverstatus.StatusManager, +) error { + if vmcp.Spec.IncomingAuth == nil || vmcp.Spec.IncomingAuth.AuthzConfigRef == nil { + // No MCPAuthzConfig referenced, clear any stored hash + if vmcp.Status.AuthzConfigHash != "" { + statusManager.SetAuthzConfigHash("") + } + return nil + } + + ref := vmcp.Spec.IncomingAuth.AuthzConfigRef + + // Get the referenced MCPAuthzConfig + authzConfig, err := ctrlutil.GetAuthzConfigForWorkload(ctx, r.Client, vmcp.Namespace, ref) + if err != nil { + statusManager.SetCondition( + mcpv1alpha1.ConditionAuthzConfigRefValidated, + mcpv1alpha1.ConditionReasonAuthzConfigRefNotFound, + fmt.Sprintf("MCPAuthzConfig %s not found: %v", ref.Name, err), + metav1.ConditionFalse, + ) + return err + } + + // Validate that the MCPAuthzConfig is ready + if err := ctrlutil.ValidateAuthzConfigReady(authzConfig); err != nil { + statusManager.SetCondition( + mcpv1alpha1.ConditionAuthzConfigRefValidated, + mcpv1alpha1.ConditionReasonAuthzConfigRefNotValid, + err.Error(), + metav1.ConditionFalse, + ) + return err + } + + // Set valid condition + statusManager.SetCondition( + mcpv1alpha1.ConditionAuthzConfigRefValidated, + mcpv1alpha1.ConditionReasonAuthzConfigRefValid, + fmt.Sprintf("MCPAuthzConfig %s is valid and ready", ref.Name), + metav1.ConditionTrue, + ) + + // Check if the MCPAuthzConfig hash has changed + if vmcp.Status.AuthzConfigHash != authzConfig.Status.ConfigHash { + ctxLogger := log.FromContext(ctx) + ctxLogger.Info("MCPAuthzConfig has changed, updating VirtualMCPServer", + "vmcp", vmcp.Name, + "authzConfig", authzConfig.Name, + "oldHash", vmcp.Status.AuthzConfigHash, + "newHash", authzConfig.Status.ConfigHash) + + statusManager.SetAuthzConfigHash(authzConfig.Status.ConfigHash) + } + + return nil +} + +// mapAuthzConfigToVirtualMCPServer maps MCPAuthzConfig changes to VirtualMCPServer reconciliation requests. +func (r *VirtualMCPServerReconciler) mapAuthzConfigToVirtualMCPServer( + ctx context.Context, obj client.Object, +) []reconcile.Request { + authzConfig, ok := obj.(*mcpv1alpha1.MCPAuthzConfig) + if !ok { + return nil + } + + vmcpList := &mcpv1alpha1.VirtualMCPServerList{} + if err := r.List(ctx, vmcpList, client.InNamespace(authzConfig.Namespace)); err != nil { + log.FromContext(ctx).Error(err, "Failed to list VirtualMCPServers for MCPAuthzConfig watch") + return nil + } + + var requests []reconcile.Request + for _, vmcp := range vmcpList.Items { + if vmcp.Spec.IncomingAuth != nil && + vmcp.Spec.IncomingAuth.AuthzConfigRef != nil && + vmcp.Spec.IncomingAuth.AuthzConfigRef.Name == authzConfig.Name { + requests = append(requests, reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: vmcp.Name, + Namespace: vmcp.Namespace, + }, + }) + } + } + + return requests +} + // mapOIDCConfigToVirtualMCPServer maps MCPOIDCConfig changes to VirtualMCPServer reconciliation requests. func (r *VirtualMCPServerReconciler) mapOIDCConfigToVirtualMCPServer( ctx context.Context, obj client.Object, diff --git a/cmd/thv-operator/pkg/controllerutil/authz.go b/cmd/thv-operator/pkg/controllerutil/authz.go index 71958d2812..19d9c2108c 100644 --- a/cmd/thv-operator/pkg/controllerutil/authz.go +++ b/cmd/thv-operator/pkg/controllerutil/authz.go @@ -11,6 +11,7 @@ import ( "strings" 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" @@ -20,6 +21,7 @@ import ( mcpv1alpha1 "github.com/stacklok/toolhive/cmd/thv-operator/api/v1alpha1" "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/authz/authorizers/cedar" "github.com/stacklok/toolhive/pkg/runner" ) @@ -269,3 +271,168 @@ func AddAuthzConfigOptions( return fmt.Errorf("unknown authz config type: %s", authzRef.Type) } } + +// GetAuthzConfigForWorkload fetches the MCPAuthzConfig referenced by a workload. +// Returns nil if the ref is nil. +func GetAuthzConfigForWorkload( + ctx context.Context, + c client.Client, + namespace string, + ref *mcpv1alpha1.MCPAuthzConfigReference, +) (*mcpv1alpha1.MCPAuthzConfig, error) { + if ref == nil { + return nil, nil + } + + authzConfig := &mcpv1alpha1.MCPAuthzConfig{} + if err := c.Get(ctx, types.NamespacedName{ + Name: ref.Name, + Namespace: namespace, + }, authzConfig); err != nil { + return nil, fmt.Errorf("failed to get MCPAuthzConfig %s/%s: %w", namespace, ref.Name, err) + } + + return authzConfig, nil +} + +// ValidateAuthzConfigReady checks that the MCPAuthzConfig has a Valid=True condition. +// Returns an error if the config is not ready. +func ValidateAuthzConfigReady(authzConfig *mcpv1alpha1.MCPAuthzConfig) error { + validCondition := meta.FindStatusCondition(authzConfig.Status.Conditions, mcpv1alpha1.ConditionTypeAuthzConfigValid) + if validCondition == nil || validCondition.Status != metav1.ConditionTrue { + msg := fmt.Sprintf("MCPAuthzConfig %s is not valid", authzConfig.Name) + if validCondition != nil { + msg = fmt.Sprintf("MCPAuthzConfig %s is not valid: %s", authzConfig.Name, validCondition.Message) + } + return fmt.Errorf("%s", msg) + } + return nil +} + +// GenerateAuthzVolumeConfigFromRef generates volume mount and volume for an MCPAuthzConfig +// reference. It creates volumes that mount a ConfigMap named "{resourceName}-authzref" containing +// the reconstructed full authorization config at /etc/toolhive/authz/authz.json. +func GenerateAuthzVolumeConfigFromRef(resourceName string) (*corev1.VolumeMount, *corev1.Volume) { + configMapName := fmt.Sprintf("%s-authzref", resourceName) + + 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: configMapName, + }, + Items: []corev1.KeyToPath{ + { + Key: DefaultAuthzKey, + Path: DefaultAuthzKey, + }, + }, + }, + }, + } + + return volumeMount, volume +} + +// EnsureAuthzConfigMapFromRef creates or updates a ConfigMap containing the reconstructed +// full authorization config JSON from an MCPAuthzConfig reference. +func EnsureAuthzConfigMapFromRef( + ctx context.Context, + c client.Client, + scheme *runtime.Scheme, + owner client.Object, + namespace string, + resourceName string, + authzConfig *mcpv1alpha1.MCPAuthzConfig, + labels map[string]string, +) error { + fullConfigJSON, err := BuildFullAuthzConfigJSON(authzConfig.Spec) + if err != nil { + return fmt.Errorf("failed to build full authz config JSON: %w", err) + } + + configMapName := fmt.Sprintf("%s-authzref", resourceName) + configMap := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: configMapName, + Namespace: namespace, + Labels: labels, + }, + Data: map[string]string{ + DefaultAuthzKey: string(fullConfigJSON), + }, + } + + configMapsClient := configmaps.NewClient(c, scheme) + if _, err := configMapsClient.UpsertWithOwnerReference(ctx, configMap, owner); err != nil { + return fmt.Errorf("failed to upsert authzref ConfigMap: %w", err) + } + + return nil +} + +// BuildFullAuthzConfigJSON reconstructs the full authorizer config JSON from a +// MCPAuthzConfig spec. Extracted here so both the MCPAuthzConfig controller and +// workload controllers can use it. +func BuildFullAuthzConfigJSON(spec mcpv1alpha1.MCPAuthzConfigSpec) ([]byte, error) { + factory := authorizers.GetFactory(spec.Type) + if factory == nil { + return nil, fmt.Errorf("unsupported authorizer type: %s (registered types: %v)", + spec.Type, authorizers.RegisteredTypes()) + } + + configKey := factory.ConfigKey() + + fullConfig := map[string]json.RawMessage{ + "version": mustMarshalJSON(authzConfigVersion), + "type": mustMarshalJSON(spec.Type), + configKey: spec.Config.Raw, + } + + result, err := json.Marshal(fullConfig) + if err != nil { + return nil, fmt.Errorf("failed to marshal full authz config: %w", err) + } + return result, nil +} + +const authzConfigVersion = "1.0" + +func mustMarshalJSON(v string) json.RawMessage { + b, err := json.Marshal(v) + if err != nil { + panic(fmt.Sprintf("failed to marshal %q: %v", v, err)) + } + return b +} + +// AddAuthzConfigRefOptions resolves an MCPAuthzConfig reference and adds the +// authorization configuration to builder options. +func AddAuthzConfigRefOptions( + authzConfig *mcpv1alpha1.MCPAuthzConfig, + options *[]runner.RunConfigBuilderOption, +) error { + fullConfigJSON, err := BuildFullAuthzConfigJSON(authzConfig.Spec) + if err != nil { + return fmt.Errorf("failed to build full authz config: %w", err) + } + + var cfg authz.Config + if err := json.Unmarshal(fullConfigJSON, &cfg); err != nil { + return fmt.Errorf("failed to parse authz config: %w", err) + } + + if err := cfg.Validate(); err != nil { + return fmt.Errorf("invalid authz config from MCPAuthzConfig %s: %w", authzConfig.Name, err) + } + + *options = append(*options, runner.WithAuthzConfig(&cfg)) + return nil +} diff --git a/cmd/thv-operator/pkg/virtualmcpserverstatus/collector.go b/cmd/thv-operator/pkg/virtualmcpserverstatus/collector.go index e6bbd6a8bb..e86352655f 100644 --- a/cmd/thv-operator/pkg/virtualmcpserverstatus/collector.go +++ b/cmd/thv-operator/pkg/virtualmcpserverstatus/collector.go @@ -26,6 +26,7 @@ type StatusCollector struct { url *string observedGeneration *int64 oidcConfigHash *string + authzConfigHash *string conditions map[string]metav1.Condition discoveredBackends []mcpv1alpha1.DiscoveredBackend } @@ -79,6 +80,12 @@ func (s *StatusCollector) SetOIDCConfigHash(hash string) { s.hasChanges = true } +// SetAuthzConfigHash sets the authz config hash to be updated. +func (s *StatusCollector) SetAuthzConfigHash(hash string) { + s.authzConfigHash = &hash + s.hasChanges = true +} + // SetGroupRefValidatedCondition sets the GroupRef validation condition. func (s *StatusCollector) SetGroupRefValidatedCondition(reason, message string, status metav1.ConditionStatus) { s.SetCondition(mcpv1alpha1.ConditionTypeVirtualMCPServerGroupRefValidated, reason, message, status) @@ -181,6 +188,11 @@ func (s *StatusCollector) UpdateStatus(ctx context.Context, vmcpStatus *mcpv1alp vmcpStatus.OIDCConfigHash = *s.oidcConfigHash } + // Apply authz config hash change + if s.authzConfigHash != nil { + vmcpStatus.AuthzConfigHash = *s.authzConfigHash + } + // Apply condition changes for _, condition := range s.conditions { if condition.Status == "" { @@ -208,6 +220,7 @@ func (s *StatusCollector) UpdateStatus(ctx context.Context, vmcpStatus *mcpv1alp "phase", s.phase, "message", s.message, "oidcConfigHash", s.oidcConfigHash, + "authzConfigHash", s.authzConfigHash, "conditionsCount", len(s.conditions), "discoveredBackendsCount", len(s.discoveredBackends)) return true diff --git a/cmd/thv-operator/pkg/virtualmcpserverstatus/mocks/mock_collector.go b/cmd/thv-operator/pkg/virtualmcpserverstatus/mocks/mock_collector.go index 889486a63f..6f26f85fc1 100644 --- a/cmd/thv-operator/pkg/virtualmcpserverstatus/mocks/mock_collector.go +++ b/cmd/thv-operator/pkg/virtualmcpserverstatus/mocks/mock_collector.go @@ -90,6 +90,18 @@ func (mr *MockStatusManagerMockRecorder) SetAuthServerConfigValidatedCondition(r return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAuthServerConfigValidatedCondition", reflect.TypeOf((*MockStatusManager)(nil).SetAuthServerConfigValidatedCondition), reason, message, status) } +// SetAuthzConfigHash mocks base method. +func (m *MockStatusManager) SetAuthzConfigHash(hash string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetAuthzConfigHash", hash) +} + +// SetAuthzConfigHash indicates an expected call of SetAuthzConfigHash. +func (mr *MockStatusManagerMockRecorder) SetAuthzConfigHash(hash any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetAuthzConfigHash", reflect.TypeOf((*MockStatusManager)(nil).SetAuthzConfigHash), hash) +} + // SetCompositeToolRefsValidatedCondition mocks base method. func (m *MockStatusManager) SetCompositeToolRefsValidatedCondition(reason, message string, status v1.ConditionStatus) { m.ctrl.T.Helper() diff --git a/cmd/thv-operator/pkg/virtualmcpserverstatus/types.go b/cmd/thv-operator/pkg/virtualmcpserverstatus/types.go index 0820405dfb..c9e713ba8e 100644 --- a/cmd/thv-operator/pkg/virtualmcpserverstatus/types.go +++ b/cmd/thv-operator/pkg/virtualmcpserverstatus/types.go @@ -35,6 +35,9 @@ type StatusManager interface { // SetOIDCConfigHash sets the OIDC config hash for change detection SetOIDCConfigHash(hash string) + // SetAuthzConfigHash sets the authz config hash for change detection + SetAuthzConfigHash(hash string) + // SetGroupRefValidatedCondition sets the GroupRef validation condition SetGroupRefValidatedCondition(reason, message string, status metav1.ConditionStatus) diff --git a/cmd/thv-operator/pkg/vmcpconfig/converter.go b/cmd/thv-operator/pkg/vmcpconfig/converter.go index 4e8d51eed8..d398e32cad 100644 --- a/cmd/thv-operator/pkg/vmcpconfig/converter.go +++ b/cmd/thv-operator/pkg/vmcpconfig/converter.go @@ -6,6 +6,7 @@ package vmcpconfig import ( "context" + "encoding/json" "fmt" "github.com/go-logr/logr" @@ -182,7 +183,13 @@ func (c *Converter) convertIncomingAuth( } // Convert authorization configuration - if vmcp.Spec.IncomingAuth.AuthzConfig != nil { + if vmcp.Spec.IncomingAuth.AuthzConfigRef != nil { + authzCfg, err := c.resolveAuthzConfigRef(ctx, vmcp) + if err != nil { + return nil, err + } + incoming.Authz = authzCfg + } else if vmcp.Spec.IncomingAuth.AuthzConfig != nil { // Map Kubernetes API types to vmcp config types // API "inline" maps to vmcp "cedar" authzType := vmcp.Spec.IncomingAuth.AuthzConfig.Type @@ -204,6 +211,48 @@ func (c *Converter) convertIncomingAuth( return incoming, nil } +// resolveAuthzConfigRef resolves authorization configuration from an MCPAuthzConfig reference. +// Currently supports Cedar-type configs by extracting policies from the MCPAuthzConfig spec. +// Non-Cedar types are not yet supported in the VirtualMCPServer runtime config +// (vmcpconfig.AuthzConfig is Cedar-specific). +func (c *Converter) resolveAuthzConfigRef( + ctx context.Context, + vmcp *mcpv1alpha1.VirtualMCPServer, +) (*vmcpconfig.AuthzConfig, error) { + ref := vmcp.Spec.IncomingAuth.AuthzConfigRef + + authzConfig, err := controllerutil.GetAuthzConfigForWorkload(ctx, c.k8sClient, vmcp.Namespace, ref) + if err != nil { + return nil, fmt.Errorf("failed to get MCPAuthzConfig %s: %w", ref.Name, err) + } + + if err := controllerutil.ValidateAuthzConfigReady(authzConfig); err != nil { + return nil, err + } + + // For Cedar-type configs, extract policies from the raw config + if authzConfig.Spec.Type == "cedarv1" { + var cedarOpts struct { + Policies []string `json:"policies"` + PrimaryUpstreamProvider string `json:"primary_upstream_provider"` + } + if err := json.Unmarshal(authzConfig.Spec.Config.Raw, &cedarOpts); err != nil { + return nil, fmt.Errorf("failed to parse Cedar config from MCPAuthzConfig %s: %w", ref.Name, err) + } + return &vmcpconfig.AuthzConfig{ + Type: "cedar", + Policies: cedarOpts.Policies, + PrimaryUpstreamProvider: cedarOpts.PrimaryUpstreamProvider, + }, nil + } + + // Non-Cedar types: vmcpconfig.AuthzConfig is Cedar-specific, so we can only + // set the type. Full support for other backends is a follow-up. + return &vmcpconfig.AuthzConfig{ + Type: authzConfig.Spec.Type, + }, nil +} + // resolveOIDCConfig resolves OIDC configuration from either an MCPOIDCConfig reference // or legacy inline OIDCConfig. Returns nil when no OIDC config is present. // Fails closed: returns an error when OIDC is configured but resolution fails, 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 d0f8815fa3..037c9770f9 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 @@ -750,6 +750,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; use + authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' status: description: MCPRemoteProxyStatus defines the observed state of MCPRemoteProxy properties: @@ -758,6 +761,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 77c9eb801c..4ebf5b1952 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 @@ -1045,6 +1045,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; use + authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' - message: telemetry and telemetryConfigRef are mutually exclusive; migrate to telemetryConfigRef rule: '!(has(self.telemetry) && has(self.telemetryConfigRef))' @@ -1068,6 +1071,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 2a929bcb49..30eed15e0c 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 @@ -2136,6 +2136,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; + use authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' outgoingAuth: description: |- OutgoingAuth configures authentication from Virtual MCP to backend MCPServers. @@ -2305,6 +2308,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 healthy/ready backends 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 ad4aeec5f9..6191e341e5 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpremoteproxies.yaml @@ -753,6 +753,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; use + authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' status: description: MCPRemoteProxyStatus defines the observed state of MCPRemoteProxy properties: @@ -761,6 +764,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 0f5f5980d8..e542f98dbb 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_mcpservers.yaml @@ -1048,6 +1048,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; use + authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' - message: telemetry and telemetryConfigRef are mutually exclusive; migrate to telemetryConfigRef rule: '!(has(self.telemetry) && has(self.telemetryConfigRef))' @@ -1071,6 +1074,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 ed65160ea5..4d1cd635b5 100644 --- a/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml +++ b/deploy/charts/operator-crds/templates/toolhive.stacklok.dev_virtualmcpservers.yaml @@ -2139,6 +2139,9 @@ spec: - message: oidcConfig and oidcConfigRef are mutually exclusive; use oidcConfigRef to reference a shared MCPOIDCConfig rule: '!(has(self.oidcConfig) && has(self.oidcConfigRef))' + - message: authzConfig and authzConfigRef are mutually exclusive; + use authzConfigRef to reference a shared MCPAuthzConfig + rule: '!(has(self.authzConfig) && has(self.authzConfigRef))' outgoingAuth: description: |- OutgoingAuth configures authentication from Virtual MCP to backend MCPServers. @@ -2308,6 +2311,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 healthy/ready backends diff --git a/docs/operator/crd-api.md b/docs/operator/crd-api.md index 1b439ca388..e6181fb949 100644 --- a/docs/operator/crd-api.md +++ b/docs/operator/crd-api.md @@ -2045,6 +2045,7 @@ _Appears in:_ | `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: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection | | Optional: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection | | Optional: \{\}
| | `message` _string_ | Message provides additional information about the current phase | | Optional: \{\}
| @@ -2282,6 +2283,7 @@ _Appears in:_ | `authServerConfigHash` _string_ | AuthServerConfigHash is the hash of the referenced authServerRef spec,
used to detect configuration changes and trigger reconciliation. | | 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: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection | | Optional: \{\}
| | `url` _string_ | URL is the URL where the MCP server can be accessed | | Optional: \{\}
| | `phase` _[api.v1alpha1.MCPServerPhase](#apiv1alpha1mcpserverphase)_ | Phase is the current phase of the MCPServer | | Enum: [Pending Ready Failed Terminating Stopped]
Optional: \{\}
| | `message` _string_ | Message provides additional information about the current phase | | Optional: \{\}
| @@ -3553,6 +3555,7 @@ _Appears in:_ | `discoveredBackends` _[api.v1alpha1.DiscoveredBackend](#apiv1alpha1discoveredbackend) array_ | DiscoveredBackends lists discovered backend configurations from the MCPGroup | | Optional: \{\}
| | `backendCount` _integer_ | BackendCount is the number of healthy/ready backends
(excludes unavailable, degraded, and unknown backends) | | Optional: \{\}
| | `oidcConfigHash` _string_ | OIDCConfigHash is the hash of the referenced MCPOIDCConfig spec for change detection.
Only populated when IncomingAuth.OIDCConfigRef is set. | | Optional: \{\}
| +| `authzConfigHash` _string_ | AuthzConfigHash is the hash of the referenced MCPAuthzConfig spec for change detection.
Only populated when IncomingAuth.AuthzConfigRef is set. | | Optional: \{\}
| #### api.v1alpha1.Volume