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..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 { @@ -87,10 +88,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"` @@ -187,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 277046e078..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)" @@ -290,10 +308,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"` @@ -1061,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 3b784bbe67..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 { @@ -133,10 +134,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 @@ -228,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/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..8a44f286e7 --- /dev/null +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller.go @@ -0,0 +1,395 @@ +// 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 +) + +// 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 := ctrlutil.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"` +} + +// 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..e526f3f184 --- /dev/null +++ b/cmd/thv-operator/controllers/mcpauthzconfig_controller_test.go @@ -0,0 +1,617 @@ +// 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" + 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" +) + +// 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 := ctrlutil.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/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/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/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/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..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 @@ -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. @@ -734,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: @@ -742,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 f323a25ff2..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 @@ -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. @@ -1029,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))' @@ -1052,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 1ae8c86f14..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 @@ -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. @@ -2121,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. @@ -2290,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_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..6191e341e5 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. @@ -737,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: @@ -745,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 9f2732ef2a..e542f98dbb 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. @@ -1032,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))' @@ -1055,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 572d3ba010..4d1cd635b5 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. @@ -2124,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. @@ -2293,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/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..e6181fb949 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: \{\}
| @@ -1939,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: \{\}
| @@ -2138,7 +2245,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: \{\}
| @@ -2175,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: \{\}
| @@ -3446,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 @@ -3477,6 +3587,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 }