Skip to content

Commit e3a81ab

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request kubernetes#126368 from jpbetz/organize-cel-libraries
Improve structure of CEL libraries to ensure cost tests kept accurate with introduction of new types
2 parents a1a645b + 430b1de commit e3a81ab

File tree

20 files changed

+328
-86
lines changed

20 files changed

+328
-86
lines changed

‎staging/src/k8s.io/apiserver/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ go 1.22.0
66

77
require (
88
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a
9+
github.com/blang/semver/v4 v4.0.0
910
github.com/coreos/go-oidc v2.2.1+incompatible
1011
github.com/coreos/go-systemd/v22 v22.5.0
1112
github.com/emicklei/go-restful/v3 v3.11.0
@@ -64,7 +65,6 @@ require (
6465
github.com/NYTimes/gziphandler v1.1.1 // indirect
6566
github.com/antlr4-go/antlr/v4 v4.13.0 // indirect
6667
github.com/beorn7/perks v1.0.1 // indirect
67-
github.com/blang/semver/v4 v4.0.0 // indirect
6868
github.com/cenkalti/backoff/v4 v4.3.0 // indirect
6969
github.com/cespare/xxhash/v2 v2.3.0 // indirect
7070
github.com/coreos/go-semver v0.3.1 // indirect

‎staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@ package environment
1818

1919
import (
2020
"sort"
21+
"strings"
2122
"testing"
2223

2324
"github.com/google/cel-go/cel"
2425

26+
"k8s.io/apimachinery/pkg/util/sets"
2527
"k8s.io/apimachinery/pkg/util/version"
28+
"k8s.io/apiserver/pkg/cel/library"
2629
)
2730

2831
// BenchmarkLoadBaseEnv is expected to be very fast, because a
@@ -112,6 +115,29 @@ func TestLibraryCoverage(t *testing.T) {
112115
}
113116
}
114117

118+
// TestKnownLibraries ensures that all libraries used in the base environment are also registered with
119+
// KnownLibraries. Other tests rely on KnownLibraries to provide an up-to-date list of CEL libraries.
120+
func TestKnownLibraries(t *testing.T) {
121+
known := sets.New[string]()
122+
used := sets.New[string]()
123+
124+
for _, lib := range library.KnownLibraries() {
125+
known.Insert(lib.LibraryName())
126+
}
127+
for _, libName := range MustBaseEnvSet(version.MajorMinor(1, 0), true).storedExpressions.Libraries() {
128+
if strings.HasPrefix(libName, "cel.lib") { // ignore core libs
129+
continue
130+
}
131+
used.Insert(libName)
132+
}
133+
134+
unexpected := used.Difference(known)
135+
136+
if len(unexpected) != 0 {
137+
t.Errorf("Expected all libraries in the base environment to be included in k8s.io/apiserver/pkg/cel/library's KnownLibraries, but found missing libraries: %v", unexpected)
138+
}
139+
}
140+
115141
func librariesInVersions(t *testing.T, vops ...VersionedOptions) []string {
116142
env, err := cel.NewCustomEnv()
117143
if err != nil {

‎staging/src/k8s.io/apiserver/pkg/cel/format.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,11 +41,11 @@ type Format struct {
4141
MaxRegexSize int
4242
}
4343

44-
func (d *Format) ConvertToNative(typeDesc reflect.Type) (interface{}, error) {
44+
func (d Format) ConvertToNative(typeDesc reflect.Type) (interface{}, error) {
4545
return nil, fmt.Errorf("type conversion error from 'Format' to '%v'", typeDesc)
4646
}
4747

48-
func (d *Format) ConvertToType(typeVal ref.Type) ref.Val {
48+
func (d Format) ConvertToType(typeVal ref.Type) ref.Val {
4949
switch typeVal {
5050
case FormatType:
5151
return d
@@ -56,18 +56,18 @@ func (d *Format) ConvertToType(typeVal ref.Type) ref.Val {
5656
}
5757
}
5858

59-
func (d *Format) Equal(other ref.Val) ref.Val {
60-
otherDur, ok := other.(*Format)
59+
func (d Format) Equal(other ref.Val) ref.Val {
60+
otherDur, ok := other.(Format)
6161
if !ok {
6262
return types.MaybeNoSuchOverloadErr(other)
6363
}
6464
return types.Bool(d.Name == otherDur.Name)
6565
}
6666

67-
func (d *Format) Type() ref.Type {
67+
func (d Format) Type() ref.Type {
6868
return FormatType
6969
}
7070

71-
func (d *Format) Value() interface{} {
71+
func (d Format) Value() interface{} {
7272
return d
7373
}

‎staging/src/k8s.io/apiserver/pkg/cel/library/authz.go‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,20 @@ var authzLib = &authz{}
232232
type authz struct{}
233233

234234
func (*authz) LibraryName() string {
235-
return "k8s.authz"
235+
return "kubernetes.authz"
236+
}
237+
238+
func (*authz) Types() []*cel.Type {
239+
return []*cel.Type{
240+
AuthorizerType,
241+
PathCheckType,
242+
GroupCheckType,
243+
ResourceCheckType,
244+
DecisionType}
245+
}
246+
247+
func (*authz) declarations() map[string][]cel.FunctionOpt {
248+
return authzLibraryDecls
236249
}
237250

238251
var authzLibraryDecls = map[string][]cel.FunctionOpt{
@@ -324,7 +337,15 @@ var authzSelectorsLib = &authzSelectors{}
324337
type authzSelectors struct{}
325338

326339
func (*authzSelectors) LibraryName() string {
327-
return "k8s.authzSelectors"
340+
return "kubernetes.authzSelectors"
341+
}
342+
343+
func (*authzSelectors) Types() []*cel.Type {
344+
return []*cel.Type{ResourceCheckType}
345+
}
346+
347+
func (*authzSelectors) declarations() map[string][]cel.FunctionOpt {
348+
return authzSelectorsLibraryDecls
328349
}
329350

330351
var authzSelectorsLibraryDecls = map[string][]cel.FunctionOpt{

‎staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,15 @@ var cidrsLib = &cidrs{}
109109
type cidrs struct{}
110110

111111
func (*cidrs) LibraryName() string {
112-
return "net.cidr"
112+
return "kubernetes.net.cidr"
113+
}
114+
115+
func (*cidrs) declarations() map[string][]cel.FunctionOpt {
116+
return cidrLibraryDecls
117+
}
118+
119+
func (*cidrs) Types() []*cel.Type {
120+
return []*cel.Type{apiservercel.CIDRType, apiservercel.IPType}
113121
}
114122

115123
var cidrLibraryDecls = map[string][]cel.FunctionOpt{

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost.go‎

Lines changed: 14 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,14 @@ package library
1818

1919
import (
2020
"fmt"
21-
"math"
22-
"reflect"
23-
2421
"github.com/google/cel-go/checker"
2522
"github.com/google/cel-go/common"
2623
"github.com/google/cel-go/common/ast"
2724
"github.com/google/cel-go/common/types"
2825
"github.com/google/cel-go/common/types/ref"
2926
"github.com/google/cel-go/common/types/traits"
27+
"math"
3028

31-
"k8s.io/apimachinery/pkg/util/sets"
3229
"k8s.io/apiserver/pkg/cel"
3330
)
3431

@@ -50,22 +47,6 @@ var knownUnhandledFunctions = map[string]bool{
5047
"strings.quote": true,
5148
}
5249

53-
// TODO: Replace this with a utility that extracts types from libraries.
54-
var knownKubernetesRuntimeTypes = sets.New[reflect.Type](
55-
reflect.ValueOf(cel.URL{}).Type(),
56-
reflect.ValueOf(cel.IP{}).Type(),
57-
reflect.ValueOf(cel.CIDR{}).Type(),
58-
reflect.ValueOf(&cel.Format{}).Type(),
59-
reflect.ValueOf(cel.Quantity{}).Type(),
60-
)
61-
var knownKubernetesCompilerTypes = sets.New[ref.Type](
62-
cel.CIDRType,
63-
cel.IPType,
64-
cel.FormatType,
65-
cel.QuantityType,
66-
cel.URLType,
67-
)
68-
6950
// CostEstimator implements CEL's interpretable.ActualCostEstimator and checker.CostEstimator.
7051
type CostEstimator struct {
7152
// SizeEstimator provides a CostEstimator.EstimateSize that this CostEstimator will delegate size estimation
@@ -219,7 +200,7 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re
219200
}
220201
case "validate":
221202
if len(args) >= 2 {
222-
format, isFormat := args[0].Value().(*cel.Format)
203+
format, isFormat := args[0].Value().(cel.Format)
223204
if isFormat {
224205
strSize := actualSize(args[1])
225206

@@ -258,18 +239,17 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re
258239
unitCost := uint64(1)
259240
lhs := args[0]
260241
switch lhs.(type) {
261-
case cel.Quantity:
262-
return &unitCost
263-
case cel.IP:
264-
return &unitCost
265-
case cel.CIDR:
266-
return &unitCost
267-
case *cel.Format: // Formats have a small max size.
268-
return &unitCost
269-
case cel.URL: // TODO: Computing the actual cost is expensive, and changing this would be a breaking change
242+
case *cel.Quantity, cel.Quantity,
243+
*cel.IP, cel.IP,
244+
*cel.CIDR, cel.CIDR,
245+
*cel.Format, cel.Format, // Formats have a small max size. Format takes pointer receiver.
246+
*cel.URL, cel.URL, // TODO: Computing the actual cost is expensive, and changing this would be a breaking change
247+
*cel.Semver, cel.Semver,
248+
*authorizerVal, authorizerVal, *pathCheckVal, pathCheckVal, *groupCheckVal, groupCheckVal,
249+
*resourceCheckVal, resourceCheckVal, *decisionVal, decisionVal:
270250
return &unitCost
271251
default:
272-
if panicOnUnknown && knownKubernetesRuntimeTypes.Has(reflect.ValueOf(lhs).Type()) {
252+
if panicOnUnknown && lhs.Type() != nil && isRegisteredType(lhs.Type().TypeName()) {
273253
panic(fmt.Errorf("CallCost: unhandled equality for Kubernetes type %T", lhs))
274254
}
275255
}
@@ -528,7 +508,8 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
528508
}
529509
if t.Kind() == types.StructKind {
530510
switch t {
531-
case cel.QuantityType: // O(1) cost equality checks
511+
case cel.QuantityType, AuthorizerType, PathCheckType, // O(1) cost equality checks
512+
GroupCheckType, ResourceCheckType, DecisionType, cel.SemverType:
532513
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}}
533514
case cel.FormatType:
534515
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: cel.MaxFormatSize}.MultiplyByCostFactor(common.StringTraversalCostFactor)}
@@ -542,7 +523,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
542523
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: size.Max}.MultiplyByCostFactor(common.StringTraversalCostFactor)}
543524
}
544525
}
545-
if panicOnUnknown && knownKubernetesCompilerTypes.Has(t) {
526+
if panicOnUnknown && isRegisteredType(t.TypeName()) {
546527
panic(fmt.Errorf("EstimateCallCost: unhandled equality for Kubernetes type %v", t))
547528
}
548529
}

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go‎

Lines changed: 49 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ package library
1919
import (
2020
"context"
2121
"fmt"
22+
"github.com/google/cel-go/common/types/ref"
2223
"testing"
2324

2425
"github.com/google/cel-go/cel"
@@ -30,6 +31,7 @@ import (
3031
exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1"
3132

3233
"k8s.io/apiserver/pkg/authorization/authorizer"
34+
apiservercel "k8s.io/apiserver/pkg/cel"
3335
)
3436

3537
const (
@@ -1231,10 +1233,10 @@ func TestSize(t *testing.T) {
12311233
est := &CostEstimator{SizeEstimator: &testCostEstimator{}}
12321234
for _, tc := range cases {
12331235
t.Run(tc.name, func(t *testing.T) {
1234-
var targetNode checker.AstNode = testSizeNode{size: tc.targetSize}
1236+
var targetNode checker.AstNode = testNode{size: tc.targetSize}
12351237
argNodes := make([]checker.AstNode, len(tc.argSizes))
12361238
for i, arg := range tc.argSizes {
1237-
argNodes[i] = testSizeNode{size: arg}
1239+
argNodes[i] = testNode{size: arg}
12381240
}
12391241
result := est.EstimateCallCost(tc.function, tc.overload, &targetNode, argNodes)
12401242
if result.ResultSize == nil {
@@ -1247,25 +1249,63 @@ func TestSize(t *testing.T) {
12471249
}
12481250
}
12491251

1250-
type testSizeNode struct {
1252+
// TestTypeEquality ensures that cost is tested for all custom types used by Kubernetes libraries.
1253+
func TestTypeEquality(t *testing.T) {
1254+
examples := map[string]ref.Val{
1255+
// Add example ref.Val's for custom types in Kubernetes here:
1256+
"kubernetes.authorization.Authorizer": authorizerVal{},
1257+
"kubernetes.authorization.PathCheck": pathCheckVal{},
1258+
"kubernetes.authorization.GroupCheck": groupCheckVal{},
1259+
"kubernetes.authorization.ResourceCheck": resourceCheckVal{},
1260+
"kubernetes.authorization.Decision": decisionVal{},
1261+
"kubernetes.URL": apiservercel.URL{},
1262+
"kubernetes.Quantity": apiservercel.Quantity{},
1263+
"net.IP": apiservercel.IP{},
1264+
"net.CIDR": apiservercel.CIDR{},
1265+
"kubernetes.NamedFormat": apiservercel.Format{},
1266+
"kubernetes.Semver": apiservercel.Semver{},
1267+
}
1268+
1269+
originalPanicOnUnknown := panicOnUnknown
1270+
panicOnUnknown = true
1271+
t.Cleanup(func() { panicOnUnknown = originalPanicOnUnknown })
1272+
est := &CostEstimator{SizeEstimator: &testCostEstimator{}}
1273+
1274+
for _, lib := range KnownLibraries() {
1275+
for _, kt := range lib.Types() {
1276+
t.Run(kt.TypeName(), func(t *testing.T) {
1277+
typeNode := testNode{size: checker.SizeEstimate{Min: 10, Max: 100}, typ: kt}
1278+
est.EstimateCallCost("_==_", "", nil, []checker.AstNode{typeNode, typeNode})
1279+
ex, ok := examples[kt.TypeName()]
1280+
if !ok {
1281+
t.Errorf("missing example for type: %s", kt.TypeName())
1282+
}
1283+
est.CallCost("_==_", "", []ref.Val{ex, ex}, nil)
1284+
})
1285+
}
1286+
}
1287+
}
1288+
1289+
type testNode struct {
12511290
size checker.SizeEstimate
1291+
typ *types.Type
12521292
}
12531293

1254-
var _ checker.AstNode = (*testSizeNode)(nil)
1294+
var _ checker.AstNode = (*testNode)(nil)
12551295

1256-
func (t testSizeNode) Path() []string {
1296+
func (t testNode) Path() []string {
12571297
return nil // not needed
12581298
}
12591299

1260-
func (t testSizeNode) Type() *types.Type {
1261-
return nil // not needed
1300+
func (t testNode) Type() *types.Type {
1301+
return t.typ // not needed
12621302
}
12631303

1264-
func (t testSizeNode) Expr() ast.Expr {
1304+
func (t testNode) Expr() ast.Expr {
12651305
return nil // not needed
12661306
}
12671307

1268-
func (t testSizeNode) ComputedSize() *checker.SizeEstimate {
1308+
func (t testNode) ComputedSize() *checker.SizeEstimate {
12691309
return &t.size
12701310
}
12711311

‎staging/src/k8s.io/apiserver/pkg/cel/library/format.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/google/cel-go/common/decls"
2626
"github.com/google/cel-go/common/types"
2727
"github.com/google/cel-go/common/types/ref"
28+
2829
apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation"
2930
"k8s.io/apimachinery/pkg/util/validation"
3031
apiservercel "k8s.io/apiserver/pkg/cel"
@@ -90,7 +91,15 @@ var formatLib = &format{}
9091
type format struct{}
9192

9293
func (*format) LibraryName() string {
93-
return "format"
94+
return "kubernetes.format"
95+
}
96+
97+
func (*format) Types() []*cel.Type {
98+
return []*cel.Type{apiservercel.FormatType}
99+
}
100+
101+
func (*format) declarations() map[string][]cel.FunctionOpt {
102+
return formatLibraryDecls
94103
}
95104

96105
func ZeroArgumentFunctionBinding(binding func() ref.Val) decls.OverloadOpt {
@@ -124,7 +133,7 @@ func (*format) ProgramOptions() []cel.ProgramOption {
124133
return []cel.ProgramOption{}
125134
}
126135

127-
var ConstantFormats map[string]*apiservercel.Format = map[string]*apiservercel.Format{
136+
var ConstantFormats = map[string]apiservercel.Format{
128137
"dns1123Label": {
129138
Name: "DNS1123Label",
130139
ValidateFunc: func(s string) []string { return apimachineryvalidation.NameIsDNSLabel(s, false) },
@@ -252,7 +261,7 @@ var formatLibraryDecls = map[string][]cel.FunctionOpt{
252261
}
253262

254263
func formatValidate(arg1, arg2 ref.Val) ref.Val {
255-
f, ok := arg1.Value().(*apiservercel.Format)
264+
f, ok := arg1.Value().(apiservercel.Format)
256265
if !ok {
257266
return types.MaybeNoSuchOverloadErr(arg1)
258267
}

0 commit comments

Comments
 (0)