From 91503cfb5646b550edb197c92d522b47f35119ef Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Fri, 17 Apr 2026 02:42:12 +0000 Subject: [PATCH 1/9] feat: add dynamic authorization using CEL in AccessPolicy --- api/v0alpha0/accesspolicy_types.go | 23 +++++++- api/v0alpha0/zz_generated.deepcopy.go | 20 +++++++ go.mod | 30 +++++----- go.sum | 80 +++++++++++++-------------- pkg/translator/accesspolicy.go | 20 +++++++ pkg/translator/accesspolicy_test.go | 44 +++++++++++++++ 6 files changed, 162 insertions(+), 55 deletions(-) diff --git a/api/v0alpha0/accesspolicy_types.go b/api/v0alpha0/accesspolicy_types.go index 8b0740cc..6e5f2801 100644 --- a/api/v0alpha0/accesspolicy_types.go +++ b/api/v0alpha0/accesspolicy_types.go @@ -131,7 +131,8 @@ type AuthorizationSourceServiceAccount struct { // +kubebuilder:validation:XValidation:message="tools must be specified when type is set to 'InlineTools'",rule="self.type == 'InlineTools' ? has(self.tools) : true" // +kubebuilder:validation:XValidation:message="externalAuth must be specified when type is set to 'ExternalAuth'",rule="self.type == 'ExternalAuth' ? has(self.externalAuth) : true" -// +kubebuilder:validation:XValidation:message="only one of tools or externalAuth can be specified",rule="!(has(self.tools) && has(self.externalAuth))" +// +kubebuilder:validation:XValidation:message="cel must be specified when type is set to 'CEL'",rule="self.type == 'CEL' ? has(self.cel) : true" +// +kubebuilder:validation:XValidation:message="only one of tools, externalAuth or cel can be specified",rule="[has(self.tools), has(self.externalAuth), has(self.cel)].filter(x, x).size() <= 1" type AuthorizationRule struct { // +unionDiscriminator // +required @@ -148,10 +149,24 @@ type AuthorizationRule struct { // // +optional ExternalAuth *gwapiv1.HTTPExternalAuthFilter `json:"externalAuth,omitempty"` + + // CEL specifies a CEL expression for authorization. + // +optional + CEL *CELRule `json:"cel,omitempty"` +} + +// CELRule specifies a CEL expression for authorization. +type CELRule struct { + // Expression is the CEL expression to evaluate. + // +required + Expression string `json:"expression"` + // Message is the custom error message to return if the expression evaluates to false. + // +optional + Message string `json:"message,omitempty"` } // AuthorizationRuleType identifies a type of authorization rule. -// +kubebuilder:validation:Enum=InlineTools;ExternalAuth +// +kubebuilder:validation:Enum=InlineTools;ExternalAuth;CEL type AuthorizationRuleType string const ( @@ -162,6 +177,10 @@ const ( // AuthorizationRuleTypeExternalAuth is used to identify authorization rules // evaluated by an external auth service. AuthorizationRuleTypeExternalAuth AuthorizationRuleType = "ExternalAuth" + + // AuthorizationRuleTypeCEL is used to identify authorization rules + // evaluated by a CEL expression. + AuthorizationRuleTypeCEL AuthorizationRuleType = "CEL" ) const ( diff --git a/api/v0alpha0/zz_generated.deepcopy.go b/api/v0alpha0/zz_generated.deepcopy.go index c21ce3bb..579840a8 100644 --- a/api/v0alpha0/zz_generated.deepcopy.go +++ b/api/v0alpha0/zz_generated.deepcopy.go @@ -111,6 +111,11 @@ func (in *AuthorizationRule) DeepCopyInto(out *AuthorizationRule) { *out = new(v1.HTTPExternalAuthFilter) (*in).DeepCopyInto(*out) } + if in.CEL != nil { + in, out := &in.CEL, &out.CEL + *out = new(CELRule) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new AuthorizationRule. @@ -176,6 +181,21 @@ func (in *BackendStatus) DeepCopy() *BackendStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *CELRule) DeepCopyInto(out *CELRule) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new CELRule. +func (in *CELRule) DeepCopy() *CELRule { + if in == nil { + return nil + } + out := new(CELRule) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MCPBackend) DeepCopyInto(out *MCPBackend) { *out = *in diff --git a/go.mod b/go.mod index 3475a418..26b207ce 100644 --- a/go.mod +++ b/go.mod @@ -11,9 +11,10 @@ require ( github.com/envoyproxy/go-control-plane v0.14.0 github.com/envoyproxy/go-control-plane/envoy v1.36.0 github.com/fsnotify/fsnotify v1.9.0 + github.com/google/cel-go v0.26.0 github.com/google/go-cmp v0.7.0 github.com/google/subcommands v1.2.0 - google.golang.org/grpc v1.75.1 + google.golang.org/grpc v1.80.0 google.golang.org/protobuf v1.36.11 k8s.io/api v0.35.1 k8s.io/apimachinery v0.35.1 @@ -24,12 +25,13 @@ require ( ) require ( - cel.dev/expr v0.24.0 // indirect - github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443 // indirect + cel.dev/expr v0.25.1 // indirect + github.com/antlr4-go/antlr/v4 v4.13.0 // indirect + github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.13.0 // indirect github.com/envoyproxy/go-control-plane/ratelimit v0.1.0 // indirect - github.com/envoyproxy/protoc-gen-validate v1.2.1 // indirect + github.com/envoyproxy/protoc-gen-validate v1.3.0 // indirect github.com/fatih/color v1.18.0 // indirect github.com/fxamacker/cbor/v2 v2.9.0 // indirect github.com/go-logr/logr v1.4.3 // indirect @@ -53,20 +55,22 @@ require ( github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect github.com/spf13/cobra v1.9.1 // indirect github.com/spf13/pflag v1.0.10 // indirect + github.com/stoewer/go-strcase v1.3.0 // indirect github.com/x448/float16 v0.8.4 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/mod v0.31.0 // indirect - golang.org/x/net v0.49.0 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect + golang.org/x/mod v0.33.0 // indirect + golang.org/x/net v0.52.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.40.0 // indirect - golang.org/x/term v0.39.0 // indirect - golang.org/x/text v0.33.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.42.0 // indirect + golang.org/x/term v0.41.0 // indirect + golang.org/x/text v0.35.0 // indirect golang.org/x/time v0.14.0 // indirect - golang.org/x/tools v0.40.0 // indirect - google.golang.org/genproto/googleapis/api v0.0.0-20250728155136-f173205681a0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20250826171959-ef028d996bc1 // indirect + golang.org/x/tools v0.42.0 // indirect + google.golang.org/genproto/googleapis/api v0.0.0-20260406210006-6f92a3bedf2d // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index 79ccbba6..08fc50b6 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -cel.dev/expr v0.24.0 h1:56OvJKSH3hDGL0ml5uSxZmz3/3Pq4tJ+fb1unVLAFcY= -cel.dev/expr v0.24.0/go.mod h1:hLPLo1W4QUmuYdA72RBX06QTs6MXw941piREPl3Yfiw= +cel.dev/expr v0.25.1 h1:1KrZg61W6TWSxuNZ37Xy49ps13NUovb66QLprthtwi4= +cel.dev/expr v0.25.1/go.mod h1:hrXvqGP6G6gyx8UAHSHJ5RGk//1Oj5nXQ2NI02Nrsg4= github.com/Masterminds/semver/v3 v3.4.0 h1:Zog+i5UMtVoCU8oKka5P7i9q9HgrJeGzI9SA1Xbatp0= github.com/Masterminds/semver/v3 v3.4.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM= github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI= @@ -12,8 +12,8 @@ github.com/cenkalti/backoff/v4 v4.3.0 h1:MyRJ/UdXutAwSAT+s3wNd7MfTIcy71VQueUuFK3 github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyYozVcomhLiZE= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= -github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443 h1:aQ3y1lwWyqYPiWZThqv1aFbZMiM9vblcSArJRf2Irls= -github.com/cncf/xds/go v0.0.0-20250501225837-2ac532fd4443/go.mod h1:W+zGtBO5Y1IgJhy4+A9GOqVhqLpfZi+vwmdNXUehLA8= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/Tv1FZd4SCg8axKApyNyRsAt/w= +github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -27,8 +27,8 @@ github.com/envoyproxy/go-control-plane/envoy v1.36.0 h1:yg/JjO5E7ubRyKX3m07GF3re github.com/envoyproxy/go-control-plane/envoy v1.36.0/go.mod h1:ty89S1YCCVruQAm9OtKeEkQLTb+Lkz0k8v9W0Oxsv98= github.com/envoyproxy/go-control-plane/ratelimit v0.1.0 h1:/G9QYbddjL25KvtKTv3an9lx6VBE2cnb8wp1vEGNYGI= github.com/envoyproxy/go-control-plane/ratelimit v0.1.0/go.mod h1:Wk+tMFAFbCXaJPzVVHnPgRKdUdwW/KdbRt94AzgRee4= -github.com/envoyproxy/protoc-gen-validate v1.2.1 h1:DEo3O99U8j4hBFwbJfrz9VtgcDfUKS7KJ7spH3d86P8= -github.com/envoyproxy/protoc-gen-validate v1.2.1/go.mod h1:d/C80l/jxXLdfEIhX1W2TmLfsJ31lvEjwamM4DxlWXU= +github.com/envoyproxy/protoc-gen-validate v1.3.0 h1:TvGH1wof4H33rezVKWSpqKz5NXWg5VPuZ0uONDT6eb4= +github.com/envoyproxy/protoc-gen-validate v1.3.0/go.mod h1:HvYl7zwPa5mffgyeTUHA9zHIH36nmrm7oCbo4YKoSWA= github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= github.com/felixge/httpsnoop v1.0.4 h1:NFTV2Zj1bL4mc9sqWACXbQFVBBg2W3GPvqp8/ESS2Wg= @@ -143,24 +143,24 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= -go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= -go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0 h1:yd02MEjBdJkG3uabWP9apV+OuWRIXGDuJEUJbOHmCFU= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.58.0/go.mod h1:umTcuxiv1n/s/S6/c2AT/g2CQ7u5C59sHDNmfSwgz7Q= -go.opentelemetry.io/otel v1.37.0 h1:9zhNfelUvx0KBfu/gb+ZgeAfAgtWrfHJZcAqFC228wQ= -go.opentelemetry.io/otel v1.37.0/go.mod h1:ehE/umFRLnuLa/vSccNq9oS1ErUlkkK71gMcN34UG8I= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0 h1:OeNbIYk/2C15ckl7glBlOBp5+WlYsOElzTNmiPW/x60= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.34.0/go.mod h1:7Bept48yIeqxP2OZ9/AqIpYS94h2or0aB4FypJTc8ZM= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0 h1:tgJ0uaNS4c98WRNUEx5U3aDlrDOI5Rs+1Vifcw4DJ8U= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.34.0/go.mod h1:U7HYyW0zt/a9x5J1Kjs+r1f/d4ZHnYFclhYY2+YbeoE= -go.opentelemetry.io/otel/metric v1.37.0 h1:mvwbQS5m0tbmqML4NqK+e3aDiO02vsf/WgbsdpcPoZE= -go.opentelemetry.io/otel/metric v1.37.0/go.mod h1:04wGrZurHYKOc+RKeye86GwKiTb9FKm1WHtO+4EVr2E= -go.opentelemetry.io/otel/sdk v1.37.0 h1:ItB0QUqnjesGRvNcmAcU0LyvkVyGJ2xftD29bWdDvKI= -go.opentelemetry.io/otel/sdk v1.37.0/go.mod h1:VredYzxUvuo2q3WRcDnKDjbdvmO0sCzOvVAiY+yUkAg= -go.opentelemetry.io/otel/sdk/metric v1.37.0 h1:90lI228XrB9jCMuSdA0673aubgRobVZFhbjxHHspCPc= -go.opentelemetry.io/otel/sdk/metric v1.37.0/go.mod h1:cNen4ZWfiD37l5NhS+Keb5RXVWZWpRE+9WyVCpbo5ps= -go.opentelemetry.io/otel/trace v1.37.0 h1:HLdcFNbRQBE2imdSEgm/kwqmQj1Or1l/7bW6mxVK7z4= -go.opentelemetry.io/otel/trace v1.37.0/go.mod h1:TlgrlQ+PtQO5XFerSPUYG0JSgGyryXewPGyayAWSBS0= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= go.opentelemetry.io/proto/otlp v1.7.1 h1:gTOMpGDb0WTBOP8JaO72iL3auEZhVmAQg4ipjOVAtj4= go.opentelemetry.io/proto/otlp v1.7.1/go.mod h1:b2rVh6rfI/s2pHWNlB7ILJcRALpcNDzKhACevjI+ZnE= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= @@ -176,42 +176,42 @@ golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0 golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY= golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= -golang.org/x/mod v0.31.0 h1:HaW9xtz0+kOcWKwli0ZXy79Ix+UW/vOfmWI5QVd2tgI= -golang.org/x/mod v0.31.0/go.mod h1:43JraMp9cGx1Rx3AqioxrbrhNsLl2l/iNAvuBkrezpg= +golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= +golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20200226121028-0de0cce0169b/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= +golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= -golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/term v0.39.0 h1:RclSuaJf32jOqZz74CkPA9qFuVTX7vhLlpfj/IGWlqY= -golang.org/x/term v0.39.0/go.mod h1:yxzUCTP/U+FzoxfdKmLaA0RV1WgE0VY7hXBwKtY/4ww= +golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= +golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= +golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= -golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= +golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200619180055-7c47624df98f/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= -golang.org/x/tools v0.40.0 h1:yLkxfA+Qnul4cs9QA3KnlFu0lVmd8JJfoq+E41uSutA= -golang.org/x/tools v0.40.0/go.mod h1:Ik/tzLRlbscWpqqMRjyWYDisX8bG13FrdXp3o4Sr9lc= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= golang.org/x/tools/go/expect v0.1.1-deprecated h1:jpBZDwmgPhXsKZC6WhL20P4b/wmnpsEAGHaNy0n/rJM= golang.org/x/tools/go/expect v0.1.1-deprecated/go.mod h1:eihoPOH+FgIqa3FpoTwguz/bVUSGBlGQU67vpBeOrBY= golang.org/x/tools/go/packages/packagestest v0.1.1-deprecated h1:1h2MnaIAIXISqTFKdENegdpAgUXz6NrPEsbIeWaBRvM= @@ -220,14 +220,14 @@ golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= -gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= -gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= -google.golang.org/genproto/googleapis/api v0.0.0-20250728155136-f173205681a0 h1:0UOBWO4dC+e51ui0NFKSPbkHHiQ4TmrEfEZMLDyRmY8= -google.golang.org/genproto/googleapis/api v0.0.0-20250728155136-f173205681a0/go.mod h1:8ytArBbtOy2xfht+y2fqKd5DRDJRUQhqbyEnQ4bDChs= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250826171959-ef028d996bc1 h1:pmJpJEvT846VzausCQ5d7KreSROcDqmO388w5YbnltA= -google.golang.org/genproto/googleapis/rpc v0.0.0-20250826171959-ef028d996bc1/go.mod h1:GmFNa4BdJZ2a8G+wCe9Bg3wwThLrJun751XstdJt5Og= -google.golang.org/grpc v1.75.1 h1:/ODCNEuf9VghjgO3rqLcfg8fiOP0nSluljWFlDxELLI= -google.golang.org/grpc v1.75.1/go.mod h1:JtPAzKiq4v1xcAB2hydNlWI2RnF85XXcV0mhKXr2ecQ= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/api v0.0.0-20260406210006-6f92a3bedf2d h1:/aDRtSZJjyLQzm75d+a1wOJaqyKBMvIAfeQmoa3ORiI= +google.golang.org/genproto/googleapis/api v0.0.0-20260406210006-6f92a3bedf2d/go.mod h1:etfGUgejTiadZAUaEP14NP97xi1RGeawqkjDARA/UOs= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d h1:wT2n40TBqFY6wiwazVK9/iTWbsQrgk5ZfCSVFLO9LQA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.80.0 h1:Xr6m2WmWZLETvUNvIUmeD5OAagMw3FiKmMlTdViWsHM= +google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07bwF4= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index 0720a013..e3885d43 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -24,6 +24,8 @@ import ( rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3" hcm "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" + "github.com/google/cel-go/cel" + "github.com/google/cel-go/ext" "google.golang.org/protobuf/types/known/anypb" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -249,6 +251,24 @@ func (t *Translator) translateAccessPolicyToRBAC(accessPolicy *agenticv0alpha0.X rbacPolicy.Permissions = []*rbacconfigv3.Permission{buildToolsCallMethodPermission()} addPolicyToRBACShadowRules(rbacConfig, policyName, rbacPolicy) } + case agenticv0alpha0.AuthorizationRuleTypeCEL: + if rule.Authorization.CEL != nil { + env, err := cel.NewEnv( + cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + ext.Strings(), + ) + if err != nil { + klog.Errorf("Failed to create CEL environment: %v", err) + continue + } + ast, issues := env.Compile(rule.Authorization.CEL.Expression) + if issues != nil && issues.Err() != nil { + klog.Errorf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) + continue + } + rbacPolicy.Condition = ast.Expr() + } + } } diff --git a/pkg/translator/accesspolicy_test.go b/pkg/translator/accesspolicy_test.go index f2ee1f4d..db8124a3 100644 --- a/pkg/translator/accesspolicy_test.go +++ b/pkg/translator/accesspolicy_test.go @@ -71,6 +71,7 @@ type expectedRule struct { principal string permissions []string isExternalAuth bool + hasCelCondition bool } func TestBuildGatewayLevelRBACFilters(t *testing.T) { @@ -732,6 +733,40 @@ func TestTranslateAccessPolicyToRBAC(t *testing.T) { }, expectShadowStatPrefix: true, }, + { + name: "one rule with CEL", + accessPolicy: func() *agenticv0alpha0.XAccessPolicy { + p := newTestAccessPolicy("policy-cel", "default", "dummy", "Gateway", "spiffe://example.com/ns/default/sa/caller") + p.Spec.Rules[0].Authorization = &agenticv0alpha0.AuthorizationRule{ + Type: agenticv0alpha0.AuthorizationRuleTypeCEL, + CEL: &agenticv0alpha0.CELRule{ + Expression: "request.mcp.tool_name.startsWith('verify_')", + Message: "Agent is restricted to verification tools.", + }, + } + return p + }(), + expectedRules: map[string]expectedRule{ + "rule-1": { + principal: "spiffe://example.com/ns/default/sa/caller", + hasCelCondition: true, + }, + }, + }, + { + name: "one rule with invalid CEL", + accessPolicy: func() *agenticv0alpha0.XAccessPolicy { + p := newTestAccessPolicy("policy-cel-invalid", "default", "dummy", "Gateway", "spiffe://example.com/ns/default/sa/caller") + p.Spec.Rules[0].Authorization = &agenticv0alpha0.AuthorizationRule{ + Type: agenticv0alpha0.AuthorizationRuleTypeCEL, + CEL: &agenticv0alpha0.CELRule{ + Expression: "request.mcp.tool_name.startsWith(", + }, + } + return p + }(), + expectedRules: map[string]expectedRule{}, + }, } for _, tc := range tests { @@ -882,6 +917,15 @@ func verifyPolicy(t *testing.T, rbacPolicy *rbacconfigv3.Policy, expected expect } } + // Verify CEL Condition + if expected.hasCelCondition { + if rbacPolicy.GetCondition() == nil { + t.Errorf("expected CEL condition but found none") + } + } else if rbacPolicy.GetCondition() != nil { + t.Errorf("did not expect CEL condition but found one") + } + // Verify Tools Permissions if len(rbacPolicy.GetPermissions()) == 0 { t.Errorf("expected permissions for tools, but found none") From cb0408b6bcc09033ac105ff6a4a4c97c2f83b836 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 04:22:08 +0000 Subject: [PATCH 2/9] Address PR comments: add validation, extend tests --- api/v0alpha0/accesspolicy_types.go | 4 ++ pkg/controller/accesspolicy.go | 54 +++++++++++++++++ pkg/translator/accesspolicy.go | 1 - tests/cel/accesspolicy_test.go | 35 ++++++++++- .../examples/invalid/cel-missing-fields.yaml | 19 ++++++ tests/crd/examples/valid/cel-policy.yaml | 20 +++++++ tests/e2e/controller_test.go | 40 +++++++++++++ tests/e2e/testdata/cel-policy.yaml | 20 +++++++ tests/go.mod | 18 +++--- tests/go.sum | 60 +++++++++---------- 10 files changed, 230 insertions(+), 41 deletions(-) create mode 100644 tests/crd/examples/invalid/cel-missing-fields.yaml create mode 100644 tests/crd/examples/valid/cel-policy.yaml create mode 100644 tests/e2e/testdata/cel-policy.yaml diff --git a/api/v0alpha0/accesspolicy_types.go b/api/v0alpha0/accesspolicy_types.go index 6e5f2801..d82142a6 100644 --- a/api/v0alpha0/accesspolicy_types.go +++ b/api/v0alpha0/accesspolicy_types.go @@ -203,6 +203,10 @@ const ( // This reason is used with the "Accepted" condition when the policy // was rejected because the maximum number of policies per target was exceeded. PolicyLimitPerTargetExceeded gwapiv1.PolicyConditionReason = "LimitPerTargetExceeded" + + // This reason is used with the "Accepted" condition when the policy + // was rejected because it contains an invalid CEL expression. + PolicyReasonInvalidCEL gwapiv1.PolicyConditionReason = "InvalidCEL" ) // AccessPolicyStatus defines the observed state of AccessPolicy. diff --git a/pkg/controller/accesspolicy.go b/pkg/controller/accesspolicy.go index 35b482f5..04c9eece 100644 --- a/pkg/controller/accesspolicy.go +++ b/pkg/controller/accesspolicy.go @@ -32,6 +32,8 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/ptr" + "github.com/google/cel-go/cel" + "github.com/google/cel-go/ext" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" agenticv0alpha0 "sigs.k8s.io/kube-agentic-networking/api/v0alpha0" @@ -78,6 +80,11 @@ func (c *Controller) onAccessPolicyAdd(obj interface{}) { return } + // Validate CEL expressions. + if !c.validateCELSpec(context.Background(), policy) { + return + } + c.enqueueGatewaysForAccessPolicy(policy) } @@ -95,6 +102,11 @@ func (c *Controller) onAccessPolicyUpdate(old, newObj interface{}) { } } + // Validate CEL expressions. + if !c.validateCELSpec(context.Background(), newPolicy) { + return + } + c.enqueueGatewaysForAccessPolicy(newPolicy) } } @@ -250,6 +262,48 @@ func (c *Controller) isPolicyUnderTargetLimit(ctx context.Context, policy *agent return shouldAccept } +// validateCELSpec validates that all CEL expressions in the policy are syntactically valid. +// It returns true if all expressions are valid, false otherwise. +// It also updates the policy status accordingly. +func (c *Controller) validateCELSpec(ctx context.Context, policy *agenticv0alpha0.XAccessPolicy) bool { + if policy.Spec.Rules == nil { + return true + } + + var failureMessage string + shouldAccept := true + + for _, rule := range policy.Spec.Rules { + if rule.Authorization != nil && rule.Authorization.Type == agenticv0alpha0.AuthorizationRuleTypeCEL && rule.Authorization.CEL != nil { + env, err := cel.NewEnv( + cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + ext.Strings(), + ) + if err != nil { + shouldAccept = false + failureMessage = fmt.Sprintf("Failed to create CEL environment: %v", err) + break + } + _, issues := env.Compile(rule.Authorization.CEL.Expression) + if issues != nil && issues.Err() != nil { + shouldAccept = false + failureMessage = fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) + break + } + } + } + + if !shouldAccept { + for _, targetRef := range policy.Spec.TargetRefs { + if err := c.updateAccessPolicyStatus(ctx, policy, targetRef, false, agenticv0alpha0.PolicyReasonInvalidCEL, failureMessage); err != nil { + runtime.HandleError(fmt.Errorf("failed to update AccessPolicy status: %w", err)) + } + } + } + + return shouldAccept +} + // seniorPoliciesAtLimit returns true if the number of policies with higher seniority // targeting the same resource has already reached the configured maximum limit. func (c *Controller) seniorPoliciesAtLimit(policy *agenticv0alpha0.XAccessPolicy, allPoliciesForTarget []*agenticv0alpha0.XAccessPolicy) bool { diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index e3885d43..77ba11fb 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -268,7 +268,6 @@ func (t *Translator) translateAccessPolicyToRBAC(accessPolicy *agenticv0alpha0.X } rbacPolicy.Condition = ast.Expr() } - } } diff --git a/tests/cel/accesspolicy_test.go b/tests/cel/accesspolicy_test.go index 0753706d..829c8496 100644 --- a/tests/cel/accesspolicy_test.go +++ b/tests/cel/accesspolicy_test.go @@ -195,7 +195,7 @@ func TestValidateXAccessPolicy(t *testing.T) { }, } }, - wantErrors: []string{"only one of tools or externalAuth can be specified"}, + wantErrors: []string{"only one of tools, externalAuth or cel can be specified"}, }, { desc: "multiple ExternalAuth authorization rules specified", @@ -231,6 +231,39 @@ func TestValidateXAccessPolicy(t *testing.T) { }, wantErrors: []string{"a maximum of one rule per policy can specify 'ExternalAuth' authorization type"}, }, + { + desc: "valid CEL authorization", + mutate: func(p *v0alpha0.XAccessPolicy) { + p.Spec.Rules[0].Authorization = &v0alpha0.AuthorizationRule{ + Type: v0alpha0.AuthorizationRuleTypeCEL, + CEL: &v0alpha0.CELRule{ + Expression: "request.mcp.tool_name.startsWith('verify_')", + }, + } + }, + }, + { + desc: "missing cel field for CEL type", + mutate: func(p *v0alpha0.XAccessPolicy) { + p.Spec.Rules[0].Authorization = &v0alpha0.AuthorizationRule{ + Type: v0alpha0.AuthorizationRuleTypeCEL, + } + }, + wantErrors: []string{"cel must be specified when type is set to 'CEL'"}, + }, + { + desc: "multiple authorization types specified with CEL", + mutate: func(p *v0alpha0.XAccessPolicy) { + p.Spec.Rules[0].Authorization = &v0alpha0.AuthorizationRule{ + Type: v0alpha0.AuthorizationRuleTypeCEL, + Tools: []string{"tool-1"}, + CEL: &v0alpha0.CELRule{ + Expression: "true", + }, + } + }, + wantErrors: []string{"only one of tools, externalAuth or cel can be specified"}, + }, { desc: "heterogeneous target kinds", mutate: func(p *v0alpha0.XAccessPolicy) { diff --git a/tests/crd/examples/invalid/cel-missing-fields.yaml b/tests/crd/examples/invalid/cel-missing-fields.yaml new file mode 100644 index 00000000..d72d9476 --- /dev/null +++ b/tests/crd/examples/invalid/cel-missing-fields.yaml @@ -0,0 +1,19 @@ +apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: cel-missing-fields + namespace: default +spec: + targetRefs: + - group: agentic.prototype.x-k8s.io + kind: XBackend + name: my-backend + rules: + - name: invalid-cel + source: + type: ServiceAccount + serviceAccount: + name: my-agent + authorization: + type: CEL + # missing cel field diff --git a/tests/crd/examples/valid/cel-policy.yaml b/tests/crd/examples/valid/cel-policy.yaml new file mode 100644 index 00000000..436990b5 --- /dev/null +++ b/tests/crd/examples/valid/cel-policy.yaml @@ -0,0 +1,20 @@ +apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: cel-policy + namespace: default +spec: + targetRefs: + - group: agentic.prototype.x-k8s.io + kind: XBackend + name: my-backend + rules: + - name: allow-verify-tools + source: + type: ServiceAccount + serviceAccount: + name: my-agent + authorization: + type: CEL + cel: + expression: "request.mcp.tool_name.startsWith('verify_')" diff --git a/tests/e2e/controller_test.go b/tests/e2e/controller_test.go index 481ed180..d0c07637 100644 --- a/tests/e2e/controller_test.go +++ b/tests/e2e/controller_test.go @@ -225,6 +225,46 @@ func TestControllerE2E(t *testing.T) { }, }, }, + }) + + t.Run("Case 6: CEL policy (allows only get-sum)", func(t *testing.T) { + // Clean up previous policies first to avoid interference + runKubectl(t, "delete", "xaccesspolicy", "e2e-gateway-level-policy", "-n", namespace, "--ignore-not-found") + deleteFromNamespace(t, "testdata/backend-policy.yaml", namespace) + + applyToNamespace(t, "testdata/cel-policy.yaml", namespace) + // Wait for xDS propagation + time.Sleep(xdsUpdateWaitTime) + + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Result: &mcpResult{ + IsError: false, + Content: []mcpContent{ + { + Type: "text", + Text: "The sum of 2 and 3 is 5.", + }, + }, + }, + }, + }, + ) + + mcp.assertToolCall(t, "echo", `{"message":"hello"}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, ) }) } diff --git a/tests/e2e/testdata/cel-policy.yaml b/tests/e2e/testdata/cel-policy.yaml new file mode 100644 index 00000000..80eb9b00 --- /dev/null +++ b/tests/e2e/testdata/cel-policy.yaml @@ -0,0 +1,20 @@ +apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: e2e-cel-policy + namespace: e2e-test-ns +spec: + targetRefs: + - group: agentic.prototype.x-k8s.io + kind: XBackend + name: e2e-mcp-backend + rules: + - name: allow-get-sum-only + source: + type: ServiceAccount + serviceAccount: + name: e2e-tester-sa + authorization: + type: CEL + cel: + expression: "request.mcp.tool_name == 'get-sum'" diff --git a/tests/go.mod b/tests/go.mod index 44aeb193..1ee95280 100644 --- a/tests/go.mod +++ b/tests/go.mod @@ -52,17 +52,17 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/mod v0.32.0 // indirect - golang.org/x/net v0.49.0 // indirect + golang.org/x/mod v0.33.0 // indirect + golang.org/x/net v0.52.0 // indirect golang.org/x/oauth2 v0.34.0 // indirect - golang.org/x/sync v0.19.0 // indirect - golang.org/x/sys v0.40.0 // indirect - golang.org/x/term v0.39.0 // indirect - golang.org/x/text v0.33.0 // indirect + golang.org/x/sync v0.20.0 // indirect + golang.org/x/sys v0.42.0 // indirect + golang.org/x/term v0.41.0 // indirect + golang.org/x/text v0.35.0 // indirect golang.org/x/time v0.14.0 // indirect - golang.org/x/tools v0.41.0 // indirect - google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda // indirect - google.golang.org/grpc v1.78.0 // indirect + golang.org/x/tools v0.42.0 // indirect + google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d // indirect + google.golang.org/grpc v1.80.0 // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/evanphx/json-patch.v4 v4.13.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/tests/go.sum b/tests/go.sum index 020fbf2b..32204c8f 100644 --- a/tests/go.sum +++ b/tests/go.sum @@ -98,16 +98,16 @@ github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= -go.opentelemetry.io/otel v1.38.0 h1:RkfdswUDRimDg0m2Az18RKOsnI8UDzppJAtj01/Ymk8= -go.opentelemetry.io/otel v1.38.0/go.mod h1:zcmtmQ1+YmQM9wrNsTGV/q/uyusom3P8RxwExxkZhjM= -go.opentelemetry.io/otel/metric v1.38.0 h1:Kl6lzIYGAh5M159u9NgiRkmoMKjvbsKtYRwgfrA6WpA= -go.opentelemetry.io/otel/metric v1.38.0/go.mod h1:kB5n/QoRM8YwmUahxvI3bO34eVtQf2i4utNVLr9gEmI= -go.opentelemetry.io/otel/sdk v1.38.0 h1:l48sr5YbNf2hpCUj/FoGhW9yDkl+Ma+LrVl8qaM5b+E= -go.opentelemetry.io/otel/sdk v1.38.0/go.mod h1:ghmNdGlVemJI3+ZB5iDEuk4bWA3GkTpW+DOoZMYBVVg= -go.opentelemetry.io/otel/sdk/metric v1.38.0 h1:aSH66iL0aZqo//xXzQLYozmWrXxyFkBJ6qT5wthqPoM= -go.opentelemetry.io/otel/sdk/metric v1.38.0/go.mod h1:dg9PBnW9XdQ1Hd6ZnRz689CbtrUp0wMMs9iPcgT9EZA= -go.opentelemetry.io/otel/trace v1.38.0 h1:Fxk5bKrDZJUH+AMyyIXGcFAPah0oRcT+LuNtJrmcNLE= -go.opentelemetry.io/otel/trace v1.38.0/go.mod h1:j1P9ivuFsTceSWe1oY+EeW3sc+Pp42sO++GHkg4wwhs= +go.opentelemetry.io/otel v1.39.0 h1:8yPrr/S0ND9QEfTfdP9V+SiwT4E0G7Y5MO7p85nis48= +go.opentelemetry.io/otel v1.39.0/go.mod h1:kLlFTywNWrFyEdH0oj2xK0bFYZtHRYUdv1NklR/tgc8= +go.opentelemetry.io/otel/metric v1.39.0 h1:d1UzonvEZriVfpNKEVmHXbdf909uGTOQjA0HF0Ls5Q0= +go.opentelemetry.io/otel/metric v1.39.0/go.mod h1:jrZSWL33sD7bBxg1xjrqyDjnuzTUB0x1nBERXd7Ftcs= +go.opentelemetry.io/otel/sdk v1.39.0 h1:nMLYcjVsvdui1B/4FRkwjzoRVsMK8uL/cj0OyhKzt18= +go.opentelemetry.io/otel/sdk v1.39.0/go.mod h1:vDojkC4/jsTJsE+kh+LXYQlbL8CgrEcwmt1ENZszdJE= +go.opentelemetry.io/otel/sdk/metric v1.39.0 h1:cXMVVFVgsIf2YL6QkRF4Urbr/aMInf+2WKg+sEJTtB8= +go.opentelemetry.io/otel/sdk/metric v1.39.0/go.mod h1:xq9HEVH7qeX69/JnwEfp6fVq5wosJsY1mt4lLfYdVew= +go.opentelemetry.io/otel/trace v1.39.0 h1:2d2vfpEDmCJ5zVYz7ijaJdOF59xLomrvj7bjt6/qCJI= +go.opentelemetry.io/otel/trace v1.39.0/go.mod h1:88w4/PnZSazkGzz/w84VHpQafiU4EtqqlVdxWy+rNOA= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= @@ -118,32 +118,32 @@ go.yaml.in/yaml/v2 v2.4.3 h1:6gvOSjQoTB3vt1l+CU+tSyi/HOjfOjRLJ4YwYZGwRO0= go.yaml.in/yaml/v2 v2.4.3/go.mod h1:zSxWcmIDjOzPXpjlTTbAsKokqkDNAVtZO0WOMiT90s8= go.yaml.in/yaml/v3 v3.0.4 h1:tfq32ie2Jv2UxXFdLJdh3jXuOzWiL1fo0bu/FbuKpbc= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= -golang.org/x/mod v0.32.0 h1:9F4d3PHLljb6x//jOyokMv3eX+YDeepZSEo3mFJy93c= -golang.org/x/mod v0.32.0/go.mod h1:SgipZ/3h2Ci89DlEtEXWUk/HteuRin+HHhN+WbNhguU= -golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= -golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/mod v0.33.0 h1:tHFzIWbBifEmbwtGz65eaWyGiGZatSrT9prnU8DbVL8= +golang.org/x/mod v0.33.0/go.mod h1:swjeQEj+6r7fODbD2cqrnje9PnziFuw4bmLbBZFrQ5w= +golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= +golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= golang.org/x/oauth2 v0.34.0 h1:hqK/t4AKgbqWkdkcAeI8XLmbK+4m4G5YeQRrmiotGlw= golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwEA= -golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4= -golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI= -golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= -golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/term v0.39.0 h1:RclSuaJf32jOqZz74CkPA9qFuVTX7vhLlpfj/IGWlqY= -golang.org/x/term v0.39.0/go.mod h1:yxzUCTP/U+FzoxfdKmLaA0RV1WgE0VY7hXBwKtY/4ww= -golang.org/x/text v0.33.0 h1:B3njUFyqtHDUI5jMn1YIr5B0IE2U0qck04r6d4KPAxE= -golang.org/x/text v0.33.0/go.mod h1:LuMebE6+rBincTi9+xWTY8TztLzKHc/9C1uBCG27+q8= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= +golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= +golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= +golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= +golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= golang.org/x/time v0.14.0 h1:MRx4UaLrDotUKUdCIqzPC48t1Y9hANFKIRpNx+Te8PI= golang.org/x/time v0.14.0/go.mod h1:eL/Oa2bBBK0TkX57Fyni+NgnyQQN4LitPmob2Hjnqw4= -golang.org/x/tools v0.41.0 h1:a9b8iMweWG+S0OBnlU36rzLp20z1Rp10w+IY2czHTQc= -golang.org/x/tools v0.41.0/go.mod h1:XSY6eDqxVNiYgezAVqqCeihT4j1U2CCsqvH3WhQpnlg= +golang.org/x/tools v0.42.0 h1:uNgphsn75Tdz5Ji2q36v/nsFSfR/9BRFvqhGBaJGd5k= +golang.org/x/tools v0.42.0/go.mod h1:Ma6lCIwGZvHK6XtgbswSoWroEkhugApmsXyrUmBhfr0= gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= -gonum.org/v1/gonum v0.16.0 h1:5+ul4Swaf3ESvrOnidPp4GZbzf0mxVQpDCYUQE7OJfk= -gonum.org/v1/gonum v0.16.0/go.mod h1:fef3am4MQ93R2HHpKnLk4/Tbh/s0+wqD5nfa6Pnwy4E= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda h1:i/Q+bfisr7gq6feoJnS/DlpdwEL4ihp41fvRiM3Ork0= -google.golang.org/genproto/googleapis/rpc v0.0.0-20251029180050-ab9386a59fda/go.mod h1:7i2o+ce6H/6BluujYR+kqX3GKH+dChPTQU19wjRPiGk= -google.golang.org/grpc v1.78.0 h1:K1XZG/yGDJnzMdd/uZHAkVqJE+xIDOcmdSFZkBUicNc= -google.golang.org/grpc v1.78.0/go.mod h1:I47qjTo4OKbMkjA/aOOwxDIiPSBofUtQUI5EfpWvW7U= +gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4= +gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d h1:wT2n40TBqFY6wiwazVK9/iTWbsQrgk5ZfCSVFLO9LQA= +google.golang.org/genproto/googleapis/rpc v0.0.0-20260406210006-6f92a3bedf2d/go.mod h1:4Hqkh8ycfw05ld/3BWL7rJOSfebL2Q+DVDeRgYgxUU8= +google.golang.org/grpc v1.80.0 h1:Xr6m2WmWZLETvUNvIUmeD5OAagMw3FiKmMlTdViWsHM= +google.golang.org/grpc v1.80.0/go.mod h1:ho/dLnxwi3EDJA4Zghp7k2Ec1+c2jqup0bFkw07bwF4= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= google.golang.org/protobuf v1.36.11/go.mod h1:HTf+CrKn2C3g5S8VImy6tdcUvCska2kB7j23XfzDpco= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From c7d84f0072e6d23c407b43348ca05ea4e3359819 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 04:28:44 +0000 Subject: [PATCH 3/9] Refactor validateCELSpec in accesspolicy controller --- pkg/controller/accesspolicy.go | 42 +++++++++++++++------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/pkg/controller/accesspolicy.go b/pkg/controller/accesspolicy.go index 04c9eece..e21d4143 100644 --- a/pkg/controller/accesspolicy.go +++ b/pkg/controller/accesspolicy.go @@ -270,38 +270,34 @@ func (c *Controller) validateCELSpec(ctx context.Context, policy *agenticv0alpha return true } - var failureMessage string - shouldAccept := true + env, err := cel.NewEnv( + cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + ext.Strings(), + ) + if err != nil { + c.rejectPolicyForAllTargets(ctx, policy, fmt.Sprintf("Failed to create CEL environment: %v", err)) + return false + } for _, rule := range policy.Spec.Rules { if rule.Authorization != nil && rule.Authorization.Type == agenticv0alpha0.AuthorizationRuleTypeCEL && rule.Authorization.CEL != nil { - env, err := cel.NewEnv( - cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), - ext.Strings(), - ) - if err != nil { - shouldAccept = false - failureMessage = fmt.Sprintf("Failed to create CEL environment: %v", err) - break - } - _, issues := env.Compile(rule.Authorization.CEL.Expression) - if issues != nil && issues.Err() != nil { - shouldAccept = false - failureMessage = fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) - break + if _, issues := env.Compile(rule.Authorization.CEL.Expression); issues != nil && issues.Err() != nil { + msg := fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) + c.rejectPolicyForAllTargets(ctx, policy, msg) + return false } } } - if !shouldAccept { - for _, targetRef := range policy.Spec.TargetRefs { - if err := c.updateAccessPolicyStatus(ctx, policy, targetRef, false, agenticv0alpha0.PolicyReasonInvalidCEL, failureMessage); err != nil { - runtime.HandleError(fmt.Errorf("failed to update AccessPolicy status: %w", err)) - } + return true +} + +func (c *Controller) rejectPolicyForAllTargets(ctx context.Context, policy *agenticv0alpha0.XAccessPolicy, message string) { + for _, targetRef := range policy.Spec.TargetRefs { + if err := c.updateAccessPolicyStatus(ctx, policy, targetRef, false, agenticv0alpha0.PolicyReasonInvalidCEL, message); err != nil { + runtime.HandleError(fmt.Errorf("failed to update AccessPolicy status: %w", err)) } } - - return shouldAccept } // seniorPoliciesAtLimit returns true if the number of policies with higher seniority From 7740f5db89619ed1033f922d7927e737fcbae8e2 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 04:32:26 +0000 Subject: [PATCH 4/9] Add more E2E test cases for CEL authorization --- tests/e2e/controller_test.go | 72 ++++++++++++++++++++++++ tests/e2e/testdata/cel-all-failures.yaml | 20 +++++++ tests/e2e/testdata/cel-multi-rule.yaml | 29 ++++++++++ 3 files changed, 121 insertions(+) create mode 100644 tests/e2e/testdata/cel-all-failures.yaml create mode 100644 tests/e2e/testdata/cel-multi-rule.yaml diff --git a/tests/e2e/controller_test.go b/tests/e2e/controller_test.go index d0c07637..a1e27083 100644 --- a/tests/e2e/controller_test.go +++ b/tests/e2e/controller_test.go @@ -254,6 +254,78 @@ func TestControllerE2E(t *testing.T) { }, ) + mcp.assertToolCall(t, "echo", `{"message":"hello"}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, + ) + }) + + t.Run("Case 7: CEL policy - All failures (allows nothing)", func(t *testing.T) { + deleteFromNamespace(t, "testdata/cel-policy.yaml", namespace) + applyToNamespace(t, "testdata/cel-all-failures.yaml", namespace) + // Wait for xDS propagation + time.Sleep(xdsUpdateWaitTime) + + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, + ) + + mcp.assertToolCall(t, "echo", `{"message":"hello"}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, + ) + }) + + t.Run("Case 8: CEL policy - Mix of failure and success (allows get-sum)", func(t *testing.T) { + deleteFromNamespace(t, "testdata/cel-all-failures.yaml", namespace) + applyToNamespace(t, "testdata/cel-multi-rule.yaml", namespace) + // Wait for xDS propagation + time.Sleep(xdsUpdateWaitTime) + + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Result: &mcpResult{ + IsError: false, + Content: []mcpContent{ + { + Type: "text", + Text: "The sum of 2 and 3 is 5.", + }, + }, + }, + }, + }, + ) + + mcp.assertToolCall(t, "echo", `{"message":"hello"}`, mcpResponse{ StatusCode: 200, diff --git a/tests/e2e/testdata/cel-all-failures.yaml b/tests/e2e/testdata/cel-all-failures.yaml new file mode 100644 index 00000000..e2d5f436 --- /dev/null +++ b/tests/e2e/testdata/cel-all-failures.yaml @@ -0,0 +1,20 @@ +apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: e2e-cel-all-failures + namespace: e2e-test-ns +spec: + targetRefs: + - group: agentic.prototype.x-k8s.io + kind: XBackend + name: e2e-mcp-backend + rules: + - name: allow-nothing + source: + type: ServiceAccount + serviceAccount: + name: e2e-tester-sa + authorization: + type: CEL + cel: + expression: "request.mcp.tool_name == 'non-existent'" diff --git a/tests/e2e/testdata/cel-multi-rule.yaml b/tests/e2e/testdata/cel-multi-rule.yaml new file mode 100644 index 00000000..cb30c704 --- /dev/null +++ b/tests/e2e/testdata/cel-multi-rule.yaml @@ -0,0 +1,29 @@ +apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: e2e-cel-mix + namespace: e2e-test-ns +spec: + targetRefs: + - group: agentic.prototype.x-k8s.io + kind: XBackend + name: e2e-mcp-backend + rules: + - name: allow-get-sum + source: + type: ServiceAccount + serviceAccount: + name: e2e-tester-sa + authorization: + type: CEL + cel: + expression: "request.mcp.tool_name == 'get-sum'" + - name: allow-non-existent + source: + type: ServiceAccount + serviceAccount: + name: e2e-tester-sa + authorization: + type: CEL + cel: + expression: "request.mcp.tool_name == 'non-existent'" From 78f6fa43947f4a40e29e0ebe79216b05578ca789 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 04:56:20 +0000 Subject: [PATCH 5/9] Fix CEL rule translation to allow tool calls --- pkg/controller/accesspolicy.go | 1 + pkg/translator/accesspolicy.go | 1 + pkg/translator/accesspolicy_test.go | 17 +++++++++-------- tests/e2e/controller_test.go | 2 ++ tests/e2e/testdata/cel-multi-rule.yaml | 4 ++-- tests/e2e/testdata/cel-policy.yaml | 2 +- 6 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/controller/accesspolicy.go b/pkg/controller/accesspolicy.go index e21d4143..f2fa62a1 100644 --- a/pkg/controller/accesspolicy.go +++ b/pkg/controller/accesspolicy.go @@ -272,6 +272,7 @@ func (c *Controller) validateCELSpec(ctx context.Context, policy *agenticv0alpha env, err := cel.NewEnv( cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), ext.Strings(), ) if err != nil { diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index 77ba11fb..5b55b1d0 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -267,6 +267,7 @@ func (t *Translator) translateAccessPolicyToRBAC(accessPolicy *agenticv0alpha0.X continue } rbacPolicy.Condition = ast.Expr() + rbacPolicy.Permissions = []*rbacconfigv3.Permission{buildToolsCallMethodPermission()} } } } diff --git a/pkg/translator/accesspolicy_test.go b/pkg/translator/accesspolicy_test.go index db8124a3..c1c4e4b4 100644 --- a/pkg/translator/accesspolicy_test.go +++ b/pkg/translator/accesspolicy_test.go @@ -68,9 +68,9 @@ var ( ) type expectedRule struct { - principal string - permissions []string - isExternalAuth bool + principal string + permissions []string + isExternalAuth bool hasCelCondition bool } @@ -738,7 +738,7 @@ func TestTranslateAccessPolicyToRBAC(t *testing.T) { accessPolicy: func() *agenticv0alpha0.XAccessPolicy { p := newTestAccessPolicy("policy-cel", "default", "dummy", "Gateway", "spiffe://example.com/ns/default/sa/caller") p.Spec.Rules[0].Authorization = &agenticv0alpha0.AuthorizationRule{ - Type: agenticv0alpha0.AuthorizationRuleTypeCEL, + Type: agenticv0alpha0.AuthorizationRuleTypeCEL, CEL: &agenticv0alpha0.CELRule{ Expression: "request.mcp.tool_name.startsWith('verify_')", Message: "Agent is restricted to verification tools.", @@ -750,6 +750,7 @@ func TestTranslateAccessPolicyToRBAC(t *testing.T) { "rule-1": { principal: "spiffe://example.com/ns/default/sa/caller", hasCelCondition: true, + permissions: []string{constants.ToolsCallMethod}, }, }, }, @@ -758,7 +759,7 @@ func TestTranslateAccessPolicyToRBAC(t *testing.T) { accessPolicy: func() *agenticv0alpha0.XAccessPolicy { p := newTestAccessPolicy("policy-cel-invalid", "default", "dummy", "Gateway", "spiffe://example.com/ns/default/sa/caller") p.Spec.Rules[0].Authorization = &agenticv0alpha0.AuthorizationRule{ - Type: agenticv0alpha0.AuthorizationRuleTypeCEL, + Type: agenticv0alpha0.AuthorizationRuleTypeCEL, CEL: &agenticv0alpha0.CELRule{ Expression: "request.mcp.tool_name.startsWith(", }, @@ -945,14 +946,14 @@ func verifyPolicy(t *testing.T, rbacPolicy *rbacconfigv3.Policy, expected expect return } - if expected.isExternalAuth { + if expected.isExternalAuth || expected.hasCelCondition { if len(rbacPolicy.GetPermissions()) != 1 { - t.Errorf("expected 1 permission for external auth, got %d", len(rbacPolicy.GetPermissions())) + t.Errorf("expected 1 permission for external auth or CEL, got %d", len(rbacPolicy.GetPermissions())) return } methodRule := rbacPolicy.GetPermissions()[0].GetSourcedMetadata() if methodRule == nil || methodRule.GetMetadataMatcher().GetValue().GetStringMatch().GetExact() != constants.ToolsCallMethod { - t.Errorf("expected tools/call method permission for external auth") + t.Errorf("expected tools/call method permission for external auth or CEL") } return } diff --git a/tests/e2e/controller_test.go b/tests/e2e/controller_test.go index a1e27083..6a95f93d 100644 --- a/tests/e2e/controller_test.go +++ b/tests/e2e/controller_test.go @@ -323,6 +323,7 @@ func TestControllerE2E(t *testing.T) { }, }, }, + ) @@ -339,6 +340,7 @@ func TestControllerE2E(t *testing.T) { }, ) }) + } // TestExternalAuthE2E verifies the ExternalAuth authorization feature including: diff --git a/tests/e2e/testdata/cel-multi-rule.yaml b/tests/e2e/testdata/cel-multi-rule.yaml index cb30c704..f4ed3d53 100644 --- a/tests/e2e/testdata/cel-multi-rule.yaml +++ b/tests/e2e/testdata/cel-multi-rule.yaml @@ -17,7 +17,7 @@ spec: authorization: type: CEL cel: - expression: "request.mcp.tool_name == 'get-sum'" + expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'get-sum'" - name: allow-non-existent source: type: ServiceAccount @@ -26,4 +26,4 @@ spec: authorization: type: CEL cel: - expression: "request.mcp.tool_name == 'non-existent'" + expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'non-existent'" diff --git a/tests/e2e/testdata/cel-policy.yaml b/tests/e2e/testdata/cel-policy.yaml index 80eb9b00..06406bc6 100644 --- a/tests/e2e/testdata/cel-policy.yaml +++ b/tests/e2e/testdata/cel-policy.yaml @@ -17,4 +17,4 @@ spec: authorization: type: CEL cel: - expression: "request.mcp.tool_name == 'get-sum'" + expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'get-sum'" From cb99c4e4bcecb79df613bab2c9dd1cc1f2f17f47 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 07:07:22 +0000 Subject: [PATCH 6/9] Fixing E2E tests --- pkg/controller/accesspolicy.go | 4 +++- pkg/translator/accesspolicy.go | 5 ++++- tests/e2e/testdata/cel-multi-rule.yaml | 4 ++-- tests/e2e/testdata/cel-policy.yaml | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/controller/accesspolicy.go b/pkg/controller/accesspolicy.go index f2fa62a1..5f58c8e8 100644 --- a/pkg/controller/accesspolicy.go +++ b/pkg/controller/accesspolicy.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "reflect" + "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -282,7 +283,8 @@ func (c *Controller) validateCELSpec(ctx context.Context, policy *agenticv0alpha for _, rule := range policy.Spec.Rules { if rule.Authorization != nil && rule.Authorization.Type == agenticv0alpha0.AuthorizationRuleTypeCEL && rule.Authorization.CEL != nil { - if _, issues := env.Compile(rule.Authorization.CEL.Expression); issues != nil && issues.Err() != nil { + expression := strings.ReplaceAll(rule.Authorization.CEL.Expression, "request.mcp.tool_name", "metadata.filter_metadata['mcp_proxy'].params.name") + if _, issues := env.Compile(expression); issues != nil && issues.Err() != nil { msg := fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) c.rejectPolicyForAllTargets(ctx, policy, msg) return false diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index 5b55b1d0..a5866855 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -18,6 +18,7 @@ package translator import ( "fmt" + "strings" rbacconfigv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" @@ -255,13 +256,15 @@ func (t *Translator) translateAccessPolicyToRBAC(accessPolicy *agenticv0alpha0.X if rule.Authorization.CEL != nil { env, err := cel.NewEnv( cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), ext.Strings(), ) if err != nil { klog.Errorf("Failed to create CEL environment: %v", err) continue } - ast, issues := env.Compile(rule.Authorization.CEL.Expression) + expression := strings.ReplaceAll(rule.Authorization.CEL.Expression, "request.mcp.tool_name", "metadata.filter_metadata['mcp_proxy'].params.name") + ast, issues := env.Compile(expression) if issues != nil && issues.Err() != nil { klog.Errorf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) continue diff --git a/tests/e2e/testdata/cel-multi-rule.yaml b/tests/e2e/testdata/cel-multi-rule.yaml index f4ed3d53..cb30c704 100644 --- a/tests/e2e/testdata/cel-multi-rule.yaml +++ b/tests/e2e/testdata/cel-multi-rule.yaml @@ -17,7 +17,7 @@ spec: authorization: type: CEL cel: - expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'get-sum'" + expression: "request.mcp.tool_name == 'get-sum'" - name: allow-non-existent source: type: ServiceAccount @@ -26,4 +26,4 @@ spec: authorization: type: CEL cel: - expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'non-existent'" + expression: "request.mcp.tool_name == 'non-existent'" diff --git a/tests/e2e/testdata/cel-policy.yaml b/tests/e2e/testdata/cel-policy.yaml index 06406bc6..80eb9b00 100644 --- a/tests/e2e/testdata/cel-policy.yaml +++ b/tests/e2e/testdata/cel-policy.yaml @@ -17,4 +17,4 @@ spec: authorization: type: CEL cel: - expression: "metadata.filter_metadata['mcp_proxy'].params.name == 'get-sum'" + expression: "request.mcp.tool_name == 'get-sum'" From e7035c426484680cf20556e0b30affa44f3c4d93 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Sun, 26 Apr 2026 18:59:33 +0000 Subject: [PATCH 7/9] refactor cel initialization and usage --- pkg/controller/accesspolicy.go | 19 ++----------- pkg/translator/accesspolicy.go | 50 +++++++++++++++++++++++++--------- 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/pkg/controller/accesspolicy.go b/pkg/controller/accesspolicy.go index 5f58c8e8..7f224258 100644 --- a/pkg/controller/accesspolicy.go +++ b/pkg/controller/accesspolicy.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "reflect" - "strings" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" @@ -33,14 +32,13 @@ import ( "k8s.io/klog/v2" "k8s.io/utils/ptr" - "github.com/google/cel-go/cel" - "github.com/google/cel-go/ext" gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" agenticv0alpha0 "sigs.k8s.io/kube-agentic-networking/api/v0alpha0" "sigs.k8s.io/kube-agentic-networking/api/v0alpha0/helpers" agenticinformers "sigs.k8s.io/kube-agentic-networking/k8s/client/informers/externalversions/api/v0alpha0" "sigs.k8s.io/kube-agentic-networking/pkg/constants" + "sigs.k8s.io/kube-agentic-networking/pkg/translator" ) // AccessPolicyTargetRefIndex is the index name for looking up AccessPolicies by target ref (namespace/name of XBackend). @@ -271,21 +269,10 @@ func (c *Controller) validateCELSpec(ctx context.Context, policy *agenticv0alpha return true } - env, err := cel.NewEnv( - cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), - cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), - ext.Strings(), - ) - if err != nil { - c.rejectPolicyForAllTargets(ctx, policy, fmt.Sprintf("Failed to create CEL environment: %v", err)) - return false - } - for _, rule := range policy.Spec.Rules { if rule.Authorization != nil && rule.Authorization.Type == agenticv0alpha0.AuthorizationRuleTypeCEL && rule.Authorization.CEL != nil { - expression := strings.ReplaceAll(rule.Authorization.CEL.Expression, "request.mcp.tool_name", "metadata.filter_metadata['mcp_proxy'].params.name") - if _, issues := env.Compile(expression); issues != nil && issues.Err() != nil { - msg := fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) + if _, err := translator.CompileCelExpression(rule.Authorization.CEL.Expression); err != nil { + msg := fmt.Sprintf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, err) c.rejectPolicyForAllTargets(ctx, policy, msg) return false } diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index a5866855..429be4b4 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -18,7 +18,8 @@ package translator import ( "fmt" - "strings" + "regexp" + "sync" rbacconfigv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" @@ -254,19 +255,9 @@ func (t *Translator) translateAccessPolicyToRBAC(accessPolicy *agenticv0alpha0.X } case agenticv0alpha0.AuthorizationRuleTypeCEL: if rule.Authorization.CEL != nil { - env, err := cel.NewEnv( - cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), - cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), - ext.Strings(), - ) + ast, err := CompileCelExpression(rule.Authorization.CEL.Expression) if err != nil { - klog.Errorf("Failed to create CEL environment: %v", err) - continue - } - expression := strings.ReplaceAll(rule.Authorization.CEL.Expression, "request.mcp.tool_name", "metadata.filter_metadata['mcp_proxy'].params.name") - ast, issues := env.Compile(expression) - if issues != nil && issues.Err() != nil { - klog.Errorf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, issues.Err()) + klog.Errorf("Failed to compile CEL expression %q: %v", rule.Authorization.CEL.Expression, err) continue } rbacPolicy.Condition = ast.Expr() @@ -514,3 +505,36 @@ func buildAllowHTTPGetPolicy() *rbacconfigv3.Policy { }, } } + +var ( + celEnv *cel.Env + celEnvErr error + celEnvOnce sync.Once + mcpToolNameRegex = regexp.MustCompile(`\brequest\.mcp\.tool_name\b`) +) + +// GetCelEnv returns the shared CEL environment. +func GetCelEnv() (*cel.Env, error) { + celEnvOnce.Do(func() { + celEnv, celEnvErr = cel.NewEnv( + cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), + ext.Strings(), + ) + }) + return celEnv, celEnvErr +} + +// CompileCelExpression compiles a CEL expression after applying macro replacements. +func CompileCelExpression(expression string) (*cel.Ast, error) { + env, err := GetCelEnv() + if err != nil { + return nil, err + } + replaced := mcpToolNameRegex.ReplaceAllString(expression, "metadata.filter_metadata['mcp_proxy'].params.name") + ast, issues := env.Compile(replaced) + if issues != nil && issues.Err() != nil { + return nil, issues.Err() + } + return ast, nil +} From aa3e5a207501000d223026591678fae380b875f9 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 4 May 2026 20:45:05 +0000 Subject: [PATCH 8/9] refactor(cel): extract CEL compilation to cel.go and implement AST caching - Move `celEnv`, `GetCelEnv` and `CompileCelExpression` from `accesspolicy.go` to a new file `cel.go` within the `translator` package to clean up code organization. - Implement a thread-safe `sync.RWMutex` cache for compiled `cel.Ast` expressions to avoid duplicate compilation overhead between the controller validation and Envoy filter rendering. - Add comprehensive unit test coverage in `cel_test.go` verifying singleton initialization, macro expansion (`request.mcp.tool_name`), syntax errors, and concurrent safety. --- pkg/translator/accesspolicy.go | 36 --------- pkg/translator/cel.go | 74 ++++++++++++++++++ pkg/translator/cel_test.go | 134 +++++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 36 deletions(-) create mode 100644 pkg/translator/cel.go create mode 100644 pkg/translator/cel_test.go diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index 429be4b4..bfd46a12 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -18,16 +18,12 @@ package translator import ( "fmt" - "regexp" - "sync" rbacconfigv3 "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" rbacv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/rbac/v3" hcm "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" matcherv3 "github.com/envoyproxy/go-control-plane/envoy/type/matcher/v3" - "github.com/google/cel-go/cel" - "github.com/google/cel-go/ext" "google.golang.org/protobuf/types/known/anypb" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/types" @@ -506,35 +502,3 @@ func buildAllowHTTPGetPolicy() *rbacconfigv3.Policy { } } -var ( - celEnv *cel.Env - celEnvErr error - celEnvOnce sync.Once - mcpToolNameRegex = regexp.MustCompile(`\brequest\.mcp\.tool_name\b`) -) - -// GetCelEnv returns the shared CEL environment. -func GetCelEnv() (*cel.Env, error) { - celEnvOnce.Do(func() { - celEnv, celEnvErr = cel.NewEnv( - cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), - cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), - ext.Strings(), - ) - }) - return celEnv, celEnvErr -} - -// CompileCelExpression compiles a CEL expression after applying macro replacements. -func CompileCelExpression(expression string) (*cel.Ast, error) { - env, err := GetCelEnv() - if err != nil { - return nil, err - } - replaced := mcpToolNameRegex.ReplaceAllString(expression, "metadata.filter_metadata['mcp_proxy'].params.name") - ast, issues := env.Compile(replaced) - if issues != nil && issues.Err() != nil { - return nil, issues.Err() - } - return ast, nil -} diff --git a/pkg/translator/cel.go b/pkg/translator/cel.go new file mode 100644 index 00000000..06132a19 --- /dev/null +++ b/pkg/translator/cel.go @@ -0,0 +1,74 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package translator + +import ( + "regexp" + "sync" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/ext" +) + +var ( + celEnv *cel.Env + celEnvErr error + celEnvOnce sync.Once + mcpToolNameRegex = regexp.MustCompile(`\brequest\.mcp\.tool_name\b`) + + astCacheMutex sync.RWMutex + astCache = make(map[string]*cel.Ast) +) + +// GetCelEnv returns the shared CEL environment. +func GetCelEnv() (*cel.Env, error) { + celEnvOnce.Do(func() { + celEnv, celEnvErr = cel.NewEnv( + cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), + cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), + ext.Strings(), + ) + }) + return celEnv, celEnvErr +} + +// CompileCelExpression compiles a CEL expression after applying macro replacements. +// It uses an internal in-memory cache to avoid duplicate compilation overhead. +func CompileCelExpression(expression string) (*cel.Ast, error) { + astCacheMutex.RLock() + if ast, ok := astCache[expression]; ok { + astCacheMutex.RUnlock() + return ast, nil + } + astCacheMutex.RUnlock() + + env, err := GetCelEnv() + if err != nil { + return nil, err + } + replaced := mcpToolNameRegex.ReplaceAllString(expression, "metadata.filter_metadata['mcp_proxy'].params.name") + ast, issues := env.Compile(replaced) + if issues != nil && issues.Err() != nil { + return nil, issues.Err() + } + + astCacheMutex.Lock() + astCache[expression] = ast + astCacheMutex.Unlock() + + return ast, nil +} diff --git a/pkg/translator/cel_test.go b/pkg/translator/cel_test.go new file mode 100644 index 00000000..9fa9d0e0 --- /dev/null +++ b/pkg/translator/cel_test.go @@ -0,0 +1,134 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package translator + +import ( + "strings" + "sync" + "testing" +) + +func TestGetCelEnv(t *testing.T) { + env, err := GetCelEnv() + if err != nil { + t.Fatalf("GetCelEnv() returned error: %v", err) + } + if env == nil { + t.Fatal("GetCelEnv() returned nil environment") + } + + // Check if it returns the exact same instance on second call + env2, err := GetCelEnv() + if err != nil { + t.Fatalf("GetCelEnv() second call returned error: %v", err) + } + if env != env2 { + t.Error("GetCelEnv() did not return the same singleton instance") + } +} + +func TestCompileCelExpression(t *testing.T) { + tests := []struct { + name string + expression string + wantErr bool + errContains string + }{ + { + name: "valid standard expression", + expression: "metadata.filter_metadata['mcp_proxy'].params.name.startsWith('verify_')", + wantErr: false, + }, + { + name: "valid macro replacement", + expression: "request.mcp.tool_name.startsWith('verify_')", + wantErr: false, + }, + { + name: "valid complex expression with multiple macros and logical operators", + expression: "request.mcp.tool_name == 'fetch_data' || request.mcp.tool_name.startsWith('read_')", + wantErr: false, + }, + { + name: "invalid syntax error", + expression: "request.mcp.tool_name.startsWith(", + wantErr: true, + errContains: "Syntax error", + }, + { + name: "undeclared variable error", + expression: "unknown_variable.foo == 'bar'", + wantErr: true, + errContains: "undeclared reference", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ast, err := CompileCelExpression(tt.expression) + if (err != nil) != tt.wantErr { + t.Fatalf("CompileCelExpression() error = %v, wantErr %v", err, tt.wantErr) + } + if !tt.wantErr && ast == nil { + t.Error("CompileCelExpression() returned nil ast for valid expression") + } + if tt.wantErr && err != nil && tt.errContains != "" { + if !strings.Contains(err.Error(), tt.errContains) { + t.Errorf("CompileCelExpression() error %q does not contain expected substring %q", err.Error(), tt.errContains) + } + } + }) + } +} + +func TestCompileCelExpressionCachingAndConcurrency(t *testing.T) { + expression := "request.mcp.tool_name.startsWith('safe_')" + + // Compile first time + ast1, err := CompileCelExpression(expression) + if err != nil { + t.Fatalf("First compilation failed: %v", err) + } + + // Compile second time - should be cached + ast2, err := CompileCelExpression(expression) + if err != nil { + t.Fatalf("Second compilation failed: %v", err) + } + + // In Go, pointers to the exact same underlying structure should be identical if we cache the *cel.Ast pointer + if ast1 != ast2 { + t.Error("CompileCelExpression() did not return cached AST pointer on duplicate call") + } + + // Concurrency test to check for races + var wg sync.WaitGroup + concurrency := 10 + wg.Add(concurrency) + + for i := 0; i < concurrency; i++ { + go func() { + defer wg.Done() + _, err := CompileCelExpression(expression) + if err != nil { + t.Errorf("Concurrent compilation failed: %v", err) + } + }() + } + wg.Wait() +} + From beb5b4f97ab168a096b554ae95a9021bb7621351 Mon Sep 17 00:00:00 2001 From: Avinash Patnala Date: Mon, 4 May 2026 20:58:54 +0000 Subject: [PATCH 9/9] fix(cel): sync CRD manifests, fix gofumpt, and update test API groups - Update k8s/crds/agentic.networking.x-k8s.io_xaccesspolicies.yaml manifest via make generate. - Fix gofumpt trailing blank newlines at EOF in accesspolicy.go and cel_test.go. - Synchronize API group version and target refs to 'agentic.networking.x-k8s.io' in five new YAML test data manifests to fix E2E and CRD validation pre-submits. - Extend E2E tests to verify native Envoy CEL variables (method, path, headers) are correctly enforced alongside custom macros. --- ...c.networking.x-k8s.io_xaccesspolicies.yaml | 21 +++- pkg/translator/accesspolicy.go | 1 - pkg/translator/cel.go | 64 ++++++++-- pkg/translator/cel_test.go | 19 ++- .../examples/invalid/cel-missing-fields.yaml | 4 +- tests/crd/examples/valid/cel-policy.yaml | 4 +- tests/e2e/controller_test.go | 112 ++++++++++++++++-- tests/e2e/testdata/cel-all-failures.yaml | 4 +- tests/e2e/testdata/cel-macros.yaml | 23 ++++ tests/e2e/testdata/cel-multi-rule.yaml | 4 +- tests/e2e/testdata/cel-policy.yaml | 4 +- 11 files changed, 227 insertions(+), 33 deletions(-) create mode 100644 tests/e2e/testdata/cel-macros.yaml diff --git a/k8s/crds/agentic.networking.x-k8s.io_xaccesspolicies.yaml b/k8s/crds/agentic.networking.x-k8s.io_xaccesspolicies.yaml index 73bd75ee..77532c9e 100644 --- a/k8s/crds/agentic.networking.x-k8s.io_xaccesspolicies.yaml +++ b/k8s/crds/agentic.networking.x-k8s.io_xaccesspolicies.yaml @@ -52,6 +52,19 @@ spec: description: Authorization specifies the authorization rule to be applied to requests from the source. properties: + cel: + description: CEL specifies a CEL expression for authorization. + properties: + expression: + description: Expression is the CEL expression to evaluate. + type: string + message: + description: Message is the custom error message to + return if the expression evaluates to false. + type: string + required: + - expression + type: object externalAuth: description: |- ExternalAuth specifies an external auth filter to be used for authorization. @@ -303,6 +316,7 @@ spec: enum: - InlineTools - ExternalAuth + - CEL type: string required: - type @@ -314,8 +328,11 @@ spec: 'ExternalAuth' rule: 'self.type == ''ExternalAuth'' ? has(self.externalAuth) : true' - - message: only one of tools or externalAuth can be specified - rule: '!(has(self.tools) && has(self.externalAuth))' + - message: cel must be specified when type is set to 'CEL' + rule: 'self.type == ''CEL'' ? has(self.cel) : true' + - message: only one of tools, externalAuth or cel can be specified + rule: '[has(self.tools), has(self.externalAuth), has(self.cel)].filter(x, + x).size() <= 1' name: description: Name specifies the name of the rule. maxLength: 63 diff --git a/pkg/translator/accesspolicy.go b/pkg/translator/accesspolicy.go index bfd46a12..687c6a1e 100644 --- a/pkg/translator/accesspolicy.go +++ b/pkg/translator/accesspolicy.go @@ -501,4 +501,3 @@ func buildAllowHTTPGetPolicy() *rbacconfigv3.Policy { }, } } - diff --git a/pkg/translator/cel.go b/pkg/translator/cel.go index 06132a19..59ed928a 100644 --- a/pkg/translator/cel.go +++ b/pkg/translator/cel.go @@ -1,5 +1,5 @@ /* -Copyright 2025 The Kubernetes Authors. +Copyright The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -17,26 +17,62 @@ limitations under the License. package translator import ( + "fmt" "regexp" + "strings" "sync" "github.com/google/cel-go/cel" "github.com/google/cel-go/ext" ) +const ( + celVarRequestMCPToolName = "request.mcp.tool_name" + celVarRequestPath = "request.path" + celVarRequestURLPath = "request.url_path" + celVarRequestMethod = "request.method" + celVarRequestHost = "request.host" + celVarRequestHeaders = "request.headers" + celVarRequestTime = "request.time" +) + var ( - celEnv *cel.Env - celEnvErr error - celEnvOnce sync.Once - mcpToolNameRegex = regexp.MustCompile(`\brequest\.mcp\.tool_name\b`) + celEnv *cel.Env + celEnvErr error + celEnvOnce sync.Once astCacheMutex sync.RWMutex astCache = make(map[string]*cel.Ast) + + // variableRegex matches any `request.*` identifier chains in a CEL expression. + variableRegex = regexp.MustCompile(`\brequest\.[a-zA-Z0-9_.]+`) ) +func isValidVariable(match string) bool { + switch { + case strings.HasPrefix(match, celVarRequestMCPToolName): + return true + case strings.HasPrefix(match, celVarRequestPath): + return true + case strings.HasPrefix(match, celVarRequestURLPath): + return true + case strings.HasPrefix(match, celVarRequestMethod): + return true + case strings.HasPrefix(match, celVarRequestHost): + return true + case strings.HasPrefix(match, celVarRequestHeaders): + return true + case strings.HasPrefix(match, celVarRequestTime): + return true + default: + return false + } +} + // GetCelEnv returns the shared CEL environment. func GetCelEnv() (*cel.Env, error) { celEnvOnce.Do(func() { + // Define request/source loosely to natively support Envoy's selectExpr AST format. celEnv, celEnvErr = cel.NewEnv( cel.Variable("request", cel.MapType(cel.StringType, cel.AnyType)), cel.Variable("metadata", cel.MapType(cel.StringType, cel.AnyType)), @@ -46,7 +82,7 @@ func GetCelEnv() (*cel.Env, error) { return celEnv, celEnvErr } -// CompileCelExpression compiles a CEL expression after applying macro replacements. +// CompileCelExpression validates and compiles a CEL expression after applying macro replacements. // It uses an internal in-memory cache to avoid duplicate compilation overhead. func CompileCelExpression(expression string) (*cel.Ast, error) { astCacheMutex.RLock() @@ -56,16 +92,28 @@ func CompileCelExpression(expression string) (*cel.Ast, error) { } astCacheMutex.RUnlock() + // 1. Validate variables in the original expression to ensure they are on the allowlist + matches := variableRegex.FindAllString(expression, -1) + for _, match := range matches { + if !isValidVariable(match) { + return nil, fmt.Errorf("unsupported CEL variable: %s", match) + } + } + + // 2. Translate our custom variables to Envoy-native variables + translatedExpr := strings.ReplaceAll(expression, celVarRequestMCPToolName, "metadata.filter_metadata['mcp_proxy'].params.name") + + // 3. Compile the translated expression exactly once env, err := GetCelEnv() if err != nil { return nil, err } - replaced := mcpToolNameRegex.ReplaceAllString(expression, "metadata.filter_metadata['mcp_proxy'].params.name") - ast, issues := env.Compile(replaced) + ast, issues := env.Compile(translatedExpr) if issues != nil && issues.Err() != nil { return nil, issues.Err() } + // 4. Cache the result astCacheMutex.Lock() astCache[expression] = ast astCacheMutex.Unlock() diff --git a/pkg/translator/cel_test.go b/pkg/translator/cel_test.go index 9fa9d0e0..f616cecc 100644 --- a/pkg/translator/cel_test.go +++ b/pkg/translator/cel_test.go @@ -1,5 +1,5 @@ /* -Copyright 2025 The Kubernetes Authors. +Copyright The Kubernetes Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -50,7 +50,7 @@ func TestCompileCelExpression(t *testing.T) { }{ { name: "valid standard expression", - expression: "metadata.filter_metadata['mcp_proxy'].params.name.startsWith('verify_')", + expression: "request.path.startsWith('/verify_')", wantErr: false, }, { @@ -60,7 +60,7 @@ func TestCompileCelExpression(t *testing.T) { }, { name: "valid complex expression with multiple macros and logical operators", - expression: "request.mcp.tool_name == 'fetch_data' || request.mcp.tool_name.startsWith('read_')", + expression: "request.mcp.tool_name == 'fetch_data' || request.path.startsWith('/read_')", wantErr: false, }, { @@ -69,6 +69,18 @@ func TestCompileCelExpression(t *testing.T) { wantErr: true, errContains: "Syntax error", }, + { + name: "unsupported variable error", + expression: "request.unrecognized == 'bar'", + wantErr: true, + errContains: "unsupported CEL variable: request.unrecognized", + }, + { + name: "unsupported source variable error", + expression: "source.unknown == 'foo'", + wantErr: true, + errContains: "undeclared reference", + }, { name: "undeclared variable error", expression: "unknown_variable.foo == 'bar'", @@ -131,4 +143,3 @@ func TestCompileCelExpressionCachingAndConcurrency(t *testing.T) { } wg.Wait() } - diff --git a/tests/crd/examples/invalid/cel-missing-fields.yaml b/tests/crd/examples/invalid/cel-missing-fields.yaml index d72d9476..8d7b8c4e 100644 --- a/tests/crd/examples/invalid/cel-missing-fields.yaml +++ b/tests/crd/examples/invalid/cel-missing-fields.yaml @@ -1,11 +1,11 @@ -apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +apiVersion: agentic.networking.x-k8s.io/v0alpha0 kind: XAccessPolicy metadata: name: cel-missing-fields namespace: default spec: targetRefs: - - group: agentic.prototype.x-k8s.io + - group: agentic.networking.x-k8s.io kind: XBackend name: my-backend rules: diff --git a/tests/crd/examples/valid/cel-policy.yaml b/tests/crd/examples/valid/cel-policy.yaml index 436990b5..eb54c613 100644 --- a/tests/crd/examples/valid/cel-policy.yaml +++ b/tests/crd/examples/valid/cel-policy.yaml @@ -1,11 +1,11 @@ -apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +apiVersion: agentic.networking.x-k8s.io/v0alpha0 kind: XAccessPolicy metadata: name: cel-policy namespace: default spec: targetRefs: - - group: agentic.prototype.x-k8s.io + - group: agentic.networking.x-k8s.io kind: XBackend name: my-backend rules: diff --git a/tests/e2e/controller_test.go b/tests/e2e/controller_test.go index 6a95f93d..f99895cc 100644 --- a/tests/e2e/controller_test.go +++ b/tests/e2e/controller_test.go @@ -225,17 +225,16 @@ func TestControllerE2E(t *testing.T) { }, }, }, + ) }) t.Run("Case 6: CEL policy (allows only get-sum)", func(t *testing.T) { // Clean up previous policies first to avoid interference runKubectl(t, "delete", "xaccesspolicy", "e2e-gateway-level-policy", "-n", namespace, "--ignore-not-found") deleteFromNamespace(t, "testdata/backend-policy.yaml", namespace) - + applyToNamespace(t, "testdata/cel-policy.yaml", namespace) - // Wait for xDS propagation - time.Sleep(xdsUpdateWaitTime) - + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, mcpResponse{ StatusCode: 200, @@ -271,8 +270,6 @@ func TestControllerE2E(t *testing.T) { t.Run("Case 7: CEL policy - All failures (allows nothing)", func(t *testing.T) { deleteFromNamespace(t, "testdata/cel-policy.yaml", namespace) applyToNamespace(t, "testdata/cel-all-failures.yaml", namespace) - // Wait for xDS propagation - time.Sleep(xdsUpdateWaitTime) mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, mcpResponse{ @@ -304,8 +301,6 @@ func TestControllerE2E(t *testing.T) { t.Run("Case 8: CEL policy - Mix of failure and success (allows get-sum)", func(t *testing.T) { deleteFromNamespace(t, "testdata/cel-all-failures.yaml", namespace) applyToNamespace(t, "testdata/cel-multi-rule.yaml", namespace) - // Wait for xDS propagation - time.Sleep(xdsUpdateWaitTime) mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, mcpResponse{ @@ -323,10 +318,47 @@ func TestControllerE2E(t *testing.T) { }, }, }, + ) + mcp.assertToolCall(t, "echo", `{"message":"hello"}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, ) + }) + + // Case 9: CEL policy - Macros and functions + t.Run("Case 9: CEL policy - Macros and functions (startsWith, contains, matches)", func(t *testing.T) { + deleteFromNamespace(t, "testdata/cel-multi-rule.yaml", namespace) + applyToNamespace(t, "testdata/cel-macros.yaml", namespace) + // get-sum satisfies startsWith('/mc'), contains('2025'), and matches('^get-[a-z]+$') + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Result: &mcpResult{ + IsError: false, + Content: []mcpContent{ + { + Type: "text", + Text: "The sum of 2 and 3 is 5.", + }, + }, + }, + }, + }, + ) + // echo fails the regex match for request.mcp.tool_name.matches('^get-[a-z]+$') mcp.assertToolCall(t, "echo", `{"message":"hello"}`, mcpResponse{ StatusCode: 200, @@ -341,6 +373,70 @@ func TestControllerE2E(t *testing.T) { ) }) + // --- Individual CEL Variable Tests via Dynamic Patching --- + t.Run("Cases 10-15: CEL policy - Individual Native Variables", func(t *testing.T) { + // Clean up previous policies + deleteFromNamespace(t, "testdata/cel-macros.yaml", namespace) + + // Apply the base CEL policy + applyToNamespace(t, "testdata/cel-policy.yaml", namespace) + + celCases := []struct { + name string + validExpr string + failExpression string + }{ + {"request.path", "request.path == '/mcp'", "request.path == '/wrong'"}, + {"request.url_path", "request.url_path == '/mcp'", "request.url_path == '/wrong'"}, + {"request.method", "request.method == 'POST'", "request.method == 'GET'"}, + {"request.host", "request.host.endsWith(':10001')", "request.host.endsWith(':9999')"}, + {"request.headers", "request.headers['mcp-protocol-version'] == '2025-11-25'", "request.headers['mcp-protocol-version'] == '1999-01-01'"}, + {"request.time", "request.time.getFullYear() >= 2025", "request.time.getFullYear() < 2000"}, + } + + for _, tc := range celCases { + t.Logf("Testing CEL native variable: %s", tc.name) + + // 1. Success Path: Patch to valid expression + validPatch := `[{"op": "replace", "path": "/spec/rules/0/authorization/cel/expression", "value": "` + tc.validExpr + `"}]` + runKubectl(t, "patch", "xaccesspolicy", "e2e-cel-policy", "-n", namespace, "--type=json", "-p", validPatch) + + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Result: &mcpResult{ + IsError: false, + Content: []mcpContent{ + { + Type: "text", + Text: "The sum of 2 and 3 is 5.", + }, + }, + }, + }, + }, + ) + + // 2. Failure Path: Patch to invalid expression + failPatch := `[{"op": "replace", "path": "/spec/rules/0/authorization/cel/expression", "value": "` + tc.failExpression + `"}]` + runKubectl(t, "patch", "xaccesspolicy", "e2e-cel-policy", "-n", namespace, "--type=json", "-p", failPatch) + + mcp.assertToolCall(t, "get-sum", `{"a":2,"b":3}`, + mcpResponse{ + StatusCode: 200, + Body: respBody{ + JSONRPC: "2.0", + Error: &mcpError{ + Code: 403, + Message: "Access to this tool is forbidden.", + }, + }, + }, + ) + } + }) } // TestExternalAuthE2E verifies the ExternalAuth authorization feature including: diff --git a/tests/e2e/testdata/cel-all-failures.yaml b/tests/e2e/testdata/cel-all-failures.yaml index e2d5f436..3b5556ab 100644 --- a/tests/e2e/testdata/cel-all-failures.yaml +++ b/tests/e2e/testdata/cel-all-failures.yaml @@ -1,11 +1,11 @@ -apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +apiVersion: agentic.networking.x-k8s.io/v0alpha0 kind: XAccessPolicy metadata: name: e2e-cel-all-failures namespace: e2e-test-ns spec: targetRefs: - - group: agentic.prototype.x-k8s.io + - group: agentic.networking.x-k8s.io kind: XBackend name: e2e-mcp-backend rules: diff --git a/tests/e2e/testdata/cel-macros.yaml b/tests/e2e/testdata/cel-macros.yaml new file mode 100644 index 00000000..5b58c07d --- /dev/null +++ b/tests/e2e/testdata/cel-macros.yaml @@ -0,0 +1,23 @@ +apiVersion: agentic.networking.x-k8s.io/v0alpha0 +kind: XAccessPolicy +metadata: + name: e2e-cel-macros + namespace: e2e-test-ns +spec: + targetRefs: + - group: agentic.networking.x-k8s.io + kind: XBackend + name: e2e-mcp-backend + rules: + - name: allow-macros + source: + type: ServiceAccount + serviceAccount: + name: e2e-tester-sa + authorization: + type: CEL + cel: + expression: >- + request.path.startsWith('/mc') && + request.headers['mcp-protocol-version'].contains('2025') && + request.mcp.tool_name.matches('^get-[a-z]+$') diff --git a/tests/e2e/testdata/cel-multi-rule.yaml b/tests/e2e/testdata/cel-multi-rule.yaml index cb30c704..b749ed4c 100644 --- a/tests/e2e/testdata/cel-multi-rule.yaml +++ b/tests/e2e/testdata/cel-multi-rule.yaml @@ -1,11 +1,11 @@ -apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +apiVersion: agentic.networking.x-k8s.io/v0alpha0 kind: XAccessPolicy metadata: name: e2e-cel-mix namespace: e2e-test-ns spec: targetRefs: - - group: agentic.prototype.x-k8s.io + - group: agentic.networking.x-k8s.io kind: XBackend name: e2e-mcp-backend rules: diff --git a/tests/e2e/testdata/cel-policy.yaml b/tests/e2e/testdata/cel-policy.yaml index 80eb9b00..762b8e1e 100644 --- a/tests/e2e/testdata/cel-policy.yaml +++ b/tests/e2e/testdata/cel-policy.yaml @@ -1,11 +1,11 @@ -apiVersion: agentic.prototype.x-k8s.io/v0alpha0 +apiVersion: agentic.networking.x-k8s.io/v0alpha0 kind: XAccessPolicy metadata: name: e2e-cel-policy namespace: e2e-test-ns spec: targetRefs: - - group: agentic.prototype.x-k8s.io + - group: agentic.networking.x-k8s.io kind: XBackend name: e2e-mcp-backend rules: