From d2affe304847aa0bef3f81fa622d0b9c70a7f975 Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Thu, 25 Jul 2024 16:33:18 -0400 Subject: [PATCH 1/4] add a type for each CEL library, register all types --- .../pkg/cel/environment/base_test.go | 25 ++++++++++ .../k8s.io/apiserver/pkg/cel/library/authz.go | 21 +++++++++ .../k8s.io/apiserver/pkg/cel/library/cidr.go | 8 ++++ .../apiserver/pkg/cel/library/format.go | 9 ++++ .../k8s.io/apiserver/pkg/cel/library/ip.go | 8 ++++ .../apiserver/pkg/cel/library/libraries.go | 46 +++++++++++++++++++ .../cel/library/library_compatibility_test.go | 41 ++++++++++++++--- .../k8s.io/apiserver/pkg/cel/library/lists.go | 8 ++++ .../apiserver/pkg/cel/library/quantity.go | 8 ++++ .../k8s.io/apiserver/pkg/cel/library/regex.go | 8 ++++ .../k8s.io/apiserver/pkg/cel/library/urls.go | 8 ++++ 11 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go index a21891115cf..d893f650c96 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go @@ -18,11 +18,14 @@ package environment import ( "sort" + "strings" "testing" "github.com/google/cel-go/cel" + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/version" + "k8s.io/apiserver/pkg/cel/library" ) // BenchmarkLoadBaseEnv is expected to be very fast, because a @@ -112,6 +115,28 @@ func TestLibraryCoverage(t *testing.T) { } } +func TestKnownLibraries(t *testing.T) { + known := sets.New[string]() + used := sets.New[string]() + + for _, lib := range library.KnownLibraries() { + known.Insert(lib.LibraryName()) + } + for _, libName := range MustBaseEnvSet(version.MajorMinor(1, 0), true).storedExpressions.Libraries() { + if strings.HasPrefix(libName, "cel.lib") { // ignore core libs + continue + } + used.Insert(libName) + } + + unexpected := used.Difference(known) + + if len(unexpected) != 0 { + t.Errorf("Expected all libraries in the base environment to be included k8s.io/apiserver/pkg/cel/library's KnownLibraries, but found missing libraries: %v", unexpected) + } + +} + func librariesInVersions(t *testing.T, vops ...VersionedOptions) []string { env, err := cel.NewCustomEnv() if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go b/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go index 1fd489fc916..0acd5a542a1 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go @@ -235,6 +235,19 @@ func (*authz) LibraryName() string { return "k8s.authz" } +func (*authz) Types() []*cel.Type { + return []*cel.Type{ + AuthorizerType, + PathCheckType, + GroupCheckType, + ResourceCheckType, + DecisionType} +} + +func (*authz) declarations() map[string][]cel.FunctionOpt { + return authzLibraryDecls +} + var authzLibraryDecls = map[string][]cel.FunctionOpt{ "path": { cel.MemberOverload("authorizer_path", []*cel.Type{AuthorizerType, cel.StringType}, PathCheckType, @@ -327,6 +340,14 @@ func (*authzSelectors) LibraryName() string { return "k8s.authzSelectors" } +func (*authzSelectors) Types() []*cel.Type { + return []*cel.Type{ResourceCheckType} +} + +func (*authzSelectors) declarations() map[string][]cel.FunctionOpt { + return authzSelectorsLibraryDecls +} + var authzSelectorsLibraryDecls = map[string][]cel.FunctionOpt{ "fieldSelector": { cel.MemberOverload("authorizer_fieldselector", []*cel.Type{ResourceCheckType, cel.StringType}, ResourceCheckType, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go index c4259daed97..8686e6c17c0 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go @@ -112,6 +112,14 @@ func (*cidrs) LibraryName() string { return "net.cidr" } +func (*cidrs) declarations() map[string][]cel.FunctionOpt { + return cidrLibraryDecls +} + +func (*cidrs) Types() []*cel.Type { + return []*cel.Type{apiservercel.CIDRType, apiservercel.IPType} +} + var cidrLibraryDecls = map[string][]cel.FunctionOpt{ "cidr": { cel.Overload("string_to_cidr", []*cel.Type{cel.StringType}, apiservercel.CIDRType, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go index c051f33c006..f76c6a29d56 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go @@ -25,6 +25,7 @@ import ( "github.com/google/cel-go/common/decls" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" + apimachineryvalidation "k8s.io/apimachinery/pkg/api/validation" "k8s.io/apimachinery/pkg/util/validation" apiservercel "k8s.io/apiserver/pkg/cel" @@ -93,6 +94,14 @@ func (*format) LibraryName() string { return "format" } +func (*format) Types() []*cel.Type { + return []*cel.Type{apiservercel.FormatType} +} + +func (*format) declarations() map[string][]cel.FunctionOpt { + return formatLibraryDecls +} + func ZeroArgumentFunctionBinding(binding func() ref.Val) decls.OverloadOpt { return func(o *decls.OverloadDecl) (*decls.OverloadDecl, error) { wrapped, err := decls.FunctionBinding(func(values ...ref.Val) ref.Val { return binding() })(o) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go b/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go index cdfeb1daf2b..b957afe769b 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go @@ -135,6 +135,14 @@ func (*ip) LibraryName() string { return "net.ip" } +func (*ip) declarations() map[string][]cel.FunctionOpt { + return ipLibraryDecls +} + +func (*ip) Types() []*cel.Type { + return []*cel.Type{apiservercel.IPType} +} + var ipLibraryDecls = map[string][]cel.FunctionOpt{ "ip": { cel.Overload("string_to_ip", []*cel.Type{cel.StringType}, apiservercel.IPType, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go b/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go new file mode 100644 index 00000000000..75a9b0e998f --- /dev/null +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go @@ -0,0 +1,46 @@ +/* +Copyright 2023 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 library + +import "github.com/google/cel-go/cel" + +// Library represents a CEL library used by kubernetes. +type Library interface { + // SingletonLibrary provides the library name and ensures the library can be safely registered into environments. + cel.SingletonLibrary + + // Types provides all custom types introduced by the library. + Types() []*cel.Type + + // declarations returns all function declarations provided by the library. + declarations() map[string][]cel.FunctionOpt +} + +// KnownLibraries returns all libraries used in Kubernetes. +func KnownLibraries() []Library { + return []Library{ + authzLib, + authzSelectorsLib, + listsLib, + regexLib, + urlsLib, + quantityLib, + ipLib, + cidrsLib, + formatLib, + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go index 81691d556f0..cedbcb04f68 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go @@ -17,19 +17,18 @@ limitations under the License. package library import ( - "testing" - "github.com/google/cel-go/cel" + "github.com/google/cel-go/common/decls" + "github.com/google/cel-go/common/types" + "testing" "k8s.io/apimachinery/pkg/util/sets" ) func TestLibraryCompatibility(t *testing.T) { - var libs []map[string][]cel.FunctionOpt - libs = append(libs, authzLibraryDecls, listsLibraryDecls, regexLibraryDecls, urlLibraryDecls, quantityLibraryDecls, ipLibraryDecls, cidrLibraryDecls, formatLibraryDecls, authzSelectorsLibraryDecls) functionNames := sets.New[string]() - for _, lib := range libs { - for name := range lib { + for _, lib := range KnownLibraries() { + for name := range lib.declarations() { functionNames[name] = struct{}{} } } @@ -66,3 +65,33 @@ func TestLibraryCompatibility(t *testing.T) { t.Errorf("Expected all functions in the libraries to be assigned to a kubernetes release, but found the missing function names: %v", missing) } } + +func TestTypeRegistration(t *testing.T) { + for _, lib := range KnownLibraries() { + registeredTypes := sets.New[*cel.Type]() + usedTypes := sets.New[*cel.Type]() + // scan all registered functions + for _, fn := range lib.declarations() { + testFn, err := decls.NewFunction("test", fn...) + if err != nil { + t.Fatal(err) + } + for _, o := range testFn.OverloadDecls() { + for _, at := range o.ArgTypes() { + switch at.Kind() { + case types.OpaqueKind, types.StructKind: + usedTypes.Insert(at) + } + } + } + } + for _, t := range lib.Types() { + registeredTypes.Insert(t) + } + unregistered := usedTypes.Difference(registeredTypes) + if len(unregistered) != 0 { + t.Errorf("Expected types to be registered with the %s library Type() functions, but they were not: %v", lib.LibraryName(), unregistered) + } + } + +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go b/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go index 327ec93d6e2..d56e72761d8 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go @@ -99,6 +99,14 @@ func (*lists) LibraryName() string { return "k8s.lists" } +func (*lists) Types() []*cel.Type { + return []*cel.Type{} +} + +func (*lists) declarations() map[string][]cel.FunctionOpt { + return listsLibraryDecls +} + var paramA = cel.TypeParamType("A") // CEL typeParams can be used to constraint to a specific trait (e.g. traits.ComparableType) if the 1st operand is the type to constrain. diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go b/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go index b4ac91c8a72..4d9f1ac3eab 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go @@ -146,6 +146,14 @@ func (*quantity) LibraryName() string { return "k8s.quantity" } +func (*quantity) Types() []*cel.Type { + return []*cel.Type{apiservercel.QuantityType} +} + +func (*quantity) declarations() map[string][]cel.FunctionOpt { + return quantityLibraryDecls +} + var quantityLibraryDecls = map[string][]cel.FunctionOpt{ "quantity": { cel.Overload("string_to_quantity", []*cel.Type{cel.StringType}, apiservercel.QuantityType, cel.UnaryBinding((stringToQuantity))), diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go b/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go index 147a40f9bd2..e8577a18517 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go @@ -55,6 +55,14 @@ func (*regex) LibraryName() string { return "k8s.regex" } +func (*regex) Types() []*cel.Type { + return []*cel.Type{} +} + +func (*regex) declarations() map[string][]cel.FunctionOpt { + return regexLibraryDecls +} + var regexLibraryDecls = map[string][]cel.FunctionOpt{ "find": { cel.MemberOverload("string_find_string", []*cel.Type{cel.StringType, cel.StringType}, cel.StringType, diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go b/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go index 8f4ba85af7c..058706d8e07 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go @@ -116,6 +116,14 @@ func (*urls) LibraryName() string { return "k8s.urls" } +func (*urls) Types() []*cel.Type { + return []*cel.Type{apiservercel.URLType} +} + +func (*urls) declarations() map[string][]cel.FunctionOpt { + return urlLibraryDecls +} + var urlLibraryDecls = map[string][]cel.FunctionOpt{ "url": { cel.Overload("string_to_url", []*cel.Type{cel.StringType}, apiservercel.URLType, From 0a2dfba067d7c75fafb9844f3cf4539153b582cf Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 27 Aug 2024 14:42:58 -0400 Subject: [PATCH 2/4] Add equality cost checking --- .../pkg/cel/environment/base_test.go | 5 +- .../k8s.io/apiserver/pkg/cel/library/cost.go | 59 ++++++++---------- .../apiserver/pkg/cel/library/cost_test.go | 61 +++++++++++++++---- .../cel/library/library_compatibility_test.go | 14 ++++- 4 files changed, 91 insertions(+), 48 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go index d893f650c96..2b68f6b2617 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base_test.go @@ -115,6 +115,8 @@ func TestLibraryCoverage(t *testing.T) { } } +// TestKnownLibraries ensures that all libraries used in the base environment are also registered with +// KnownLibraries. Other tests rely on KnownLibraries to provide an up-to-date list of CEL libraries. func TestKnownLibraries(t *testing.T) { known := sets.New[string]() used := sets.New[string]() @@ -132,9 +134,8 @@ func TestKnownLibraries(t *testing.T) { unexpected := used.Difference(known) if len(unexpected) != 0 { - t.Errorf("Expected all libraries in the base environment to be included k8s.io/apiserver/pkg/cel/library's KnownLibraries, but found missing libraries: %v", unexpected) + 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) } - } func librariesInVersions(t *testing.T, vops ...VersionedOptions) []string { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go index 63863088c02..2ffb0755d6b 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go @@ -18,17 +18,15 @@ package library import ( "fmt" - "math" - "reflect" - "github.com/google/cel-go/checker" "github.com/google/cel-go/common" "github.com/google/cel-go/common/ast" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/traits" + "math" + "strings" - "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apiserver/pkg/cel" ) @@ -50,22 +48,6 @@ var knownUnhandledFunctions = map[string]bool{ "strings.quote": true, } -// TODO: Replace this with a utility that extracts types from libraries. -var knownKubernetesRuntimeTypes = sets.New[reflect.Type]( - reflect.ValueOf(cel.URL{}).Type(), - reflect.ValueOf(cel.IP{}).Type(), - reflect.ValueOf(cel.CIDR{}).Type(), - reflect.ValueOf(&cel.Format{}).Type(), - reflect.ValueOf(cel.Quantity{}).Type(), -) -var knownKubernetesCompilerTypes = sets.New[ref.Type]( - cel.CIDRType, - cel.IPType, - cel.FormatType, - cel.QuantityType, - cel.URLType, -) - // CostEstimator implements CEL's interpretable.ActualCostEstimator and checker.CostEstimator. type CostEstimator struct { // SizeEstimator provides a CostEstimator.EstimateSize that this CostEstimator will delegate size estimation @@ -258,18 +240,16 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re unitCost := uint64(1) lhs := args[0] switch lhs.(type) { - case cel.Quantity: - return &unitCost - case cel.IP: - return &unitCost - case cel.CIDR: - return &unitCost - case *cel.Format: // Formats have a small max size. - return &unitCost - case cel.URL: // TODO: Computing the actual cost is expensive, and changing this would be a breaking change + case *cel.Quantity, cel.Quantity, + *cel.IP, cel.IP, + *cel.CIDR, cel.CIDR, + *cel.Format, // Formats have a small max size. Format takes pointer receiver. + *cel.URL, cel.URL, // TODO: Computing the actual cost is expensive, and changing this would be a breaking change + *authorizerVal, authorizerVal, *pathCheckVal, pathCheckVal, *groupCheckVal, groupCheckVal, + *resourceCheckVal, resourceCheckVal, *decisionVal, decisionVal: return &unitCost default: - if panicOnUnknown && knownKubernetesRuntimeTypes.Has(reflect.ValueOf(lhs).Type()) { + if panicOnUnknown && isKubernetesType(lhs.Type()) { panic(fmt.Errorf("CallCost: unhandled equality for Kubernetes type %T", lhs)) } } @@ -528,7 +508,8 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch } if t.Kind() == types.StructKind { switch t { - case cel.QuantityType: // O(1) cost equality checks + case cel.QuantityType, AuthorizerType, PathCheckType, // O(1) cost equality checks + GroupCheckType, ResourceCheckType, DecisionType: return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}} case cel.FormatType: 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 return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: size.Max}.MultiplyByCostFactor(common.StringTraversalCostFactor)} } } - if panicOnUnknown && knownKubernetesCompilerTypes.Has(t) { + if panicOnUnknown && isKubernetesType(t) { panic(fmt.Errorf("EstimateCallCost: unhandled equality for Kubernetes type %v", t)) } } @@ -651,3 +632,17 @@ func traversalCost(v ref.Val) uint64 { return 1 } } + +// isKubernetesType returns ture if a type is type defined by Kubernetes, +// as identified by opaque or struct types with a "kubernetes." prefix. +func isKubernetesType(t ref.Type) bool { + if tt, ok := t.(*types.Type); ok { + switch tt.Kind() { + case types.OpaqueKind, types.StructKind: + return strings.HasPrefix(tt.TypeName(), "kubernetes.") + default: + return false + } + } + return false +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go index de4eaf009ab..b629a007b98 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go @@ -19,6 +19,7 @@ package library import ( "context" "fmt" + "github.com/google/cel-go/common/types/ref" "testing" "github.com/google/cel-go/cel" @@ -30,6 +31,7 @@ import ( exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" "k8s.io/apiserver/pkg/authorization/authorizer" + apiservercel "k8s.io/apiserver/pkg/cel" ) const ( @@ -1231,10 +1233,10 @@ func TestSize(t *testing.T) { est := &CostEstimator{SizeEstimator: &testCostEstimator{}} for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - var targetNode checker.AstNode = testSizeNode{size: tc.targetSize} + var targetNode checker.AstNode = testNode{size: tc.targetSize} argNodes := make([]checker.AstNode, len(tc.argSizes)) for i, arg := range tc.argSizes { - argNodes[i] = testSizeNode{size: arg} + argNodes[i] = testNode{size: arg} } result := est.EstimateCallCost(tc.function, tc.overload, &targetNode, argNodes) if result.ResultSize == nil { @@ -1247,25 +1249,62 @@ func TestSize(t *testing.T) { } } -type testSizeNode struct { +// TestTypeEquality ensures that cost is tested for all custom types used by Kubernetes libraries. +func TestTypeEquality(t *testing.T) { + examples := map[string]ref.Val{ + // Add example ref.Val's for custom types in Kubernetes here: + "kubernetes.authorization.Authorizer": authorizerVal{}, + "kubernetes.authorization.PathCheck": pathCheckVal{}, + "kubernetes.authorization.GroupCheck": groupCheckVal{}, + "kubernetes.authorization.ResourceCheck": resourceCheckVal{}, + "kubernetes.authorization.Decision": decisionVal{}, + "kubernetes.URL": apiservercel.URL{}, + "kubernetes.Quantity": apiservercel.Quantity{}, + "net.IP": apiservercel.IP{}, + "net.CIDR": apiservercel.CIDR{}, + "kubernetes.NamedFormat": &apiservercel.Format{}, + } + + originalPanicOnUnknown := panicOnUnknown + panicOnUnknown = true + t.Cleanup(func() { panicOnUnknown = originalPanicOnUnknown }) + est := &CostEstimator{SizeEstimator: &testCostEstimator{}} + + for _, lib := range KnownLibraries() { + for _, kt := range lib.Types() { + t.Run(kt.TypeName(), func(t *testing.T) { + typeNode := testNode{size: checker.SizeEstimate{Min: 10, Max: 100}, typ: kt} + est.EstimateCallCost("_==_", "", nil, []checker.AstNode{typeNode, typeNode}) + ex, ok := examples[kt.TypeName()] + if !ok { + t.Errorf("missing example for type: %s", kt.TypeName()) + } + est.CallCost("_==_", "", []ref.Val{ex, ex}, nil) + }) + } + } +} + +type testNode struct { size checker.SizeEstimate + typ *types.Type } -var _ checker.AstNode = (*testSizeNode)(nil) +var _ checker.AstNode = (*testNode)(nil) -func (t testSizeNode) Path() []string { +func (t testNode) Path() []string { return nil // not needed } -func (t testSizeNode) Type() *types.Type { +func (t testNode) Type() *types.Type { + return t.typ // not needed +} + +func (t testNode) Expr() ast.Expr { return nil // not needed } -func (t testSizeNode) Expr() ast.Expr { - return nil // not needed -} - -func (t testSizeNode) ComputedSize() *checker.SizeEstimate { +func (t testNode) ComputedSize() *checker.SizeEstimate { return &t.size } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go index cedbcb04f68..d42a1b91a15 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go @@ -66,21 +66,29 @@ func TestLibraryCompatibility(t *testing.T) { } } +// TestTypeRegistration ensures that all custom types defined and used by Kubernetes CEL libraries +// are returned by library.Types(). Other tests depend on Types() to provide an up-to-date list of +// types declared in a library. func TestTypeRegistration(t *testing.T) { for _, lib := range KnownLibraries() { registeredTypes := sets.New[*cel.Type]() usedTypes := sets.New[*cel.Type]() - // scan all registered functions + // scan all registered function declarations for the library for _, fn := range lib.declarations() { - testFn, err := decls.NewFunction("test", fn...) + fn, err := decls.NewFunction("placeholder-not-used", fn...) if err != nil { t.Fatal(err) } - for _, o := range testFn.OverloadDecls() { + for _, o := range fn.OverloadDecls() { + // ArgTypes include both the receiver type (if present) and + // all function argument types. for _, at := range o.ArgTypes() { switch at.Kind() { + // User defined types are either Opaque or Struct. case types.OpaqueKind, types.StructKind: usedTypes.Insert(at) + default: + // skip } } } From e085f3818a3a1d04d895532cbdd233d797e0913b Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 10 Sep 2024 16:55:57 -0400 Subject: [PATCH 3/4] Move CEL semver library into common libs, fix cost tests to use registered types --- staging/src/k8s.io/apiserver/go.mod | 2 +- .../src/k8s.io/apiserver/pkg/cel/format.go | 12 +++---- .../k8s.io/apiserver/pkg/cel/library/cidr.go | 2 +- .../k8s.io/apiserver/pkg/cel/library/cost.go | 26 ++++---------- .../apiserver/pkg/cel/library/cost_test.go | 3 +- .../apiserver/pkg/cel/library/format.go | 4 +-- .../k8s.io/apiserver/pkg/cel/library/ip.go | 2 +- .../apiserver/pkg/cel/library/libraries.go | 18 ++++++++-- .../cel/library/library_compatibility_test.go | 3 +- .../pkg/cel/library}/semver_test.go | 7 ++-- .../pkg/cel/library}/semverlib.go | 35 ++++++++++++------- .../pkg}/cel/semver.go | 0 .../cel/compile.go | 5 +-- 13 files changed, 65 insertions(+), 54 deletions(-) rename staging/src/k8s.io/{dynamic-resource-allocation/cel => apiserver/pkg/cel/library}/semver_test.go (96%) rename staging/src/k8s.io/{dynamic-resource-allocation/cel => apiserver/pkg/cel/library}/semverlib.go (80%) rename staging/src/k8s.io/{dynamic-resource-allocation => apiserver/pkg}/cel/semver.go (100%) diff --git a/staging/src/k8s.io/apiserver/go.mod b/staging/src/k8s.io/apiserver/go.mod index e9eaf2449ab..dbbfb437a3d 100644 --- a/staging/src/k8s.io/apiserver/go.mod +++ b/staging/src/k8s.io/apiserver/go.mod @@ -6,6 +6,7 @@ go 1.22.0 require ( github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a + github.com/blang/semver/v4 v4.0.0 github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-systemd/v22 v22.5.0 github.com/emicklei/go-restful/v3 v3.11.0 @@ -63,7 +64,6 @@ require ( github.com/NYTimes/gziphandler v1.1.1 // indirect github.com/antlr4-go/antlr/v4 v4.13.0 // indirect github.com/beorn7/perks v1.0.1 // indirect - github.com/blang/semver/v4 v4.0.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/coreos/go-semver v0.3.1 // indirect diff --git a/staging/src/k8s.io/apiserver/pkg/cel/format.go b/staging/src/k8s.io/apiserver/pkg/cel/format.go index 1bcfddfe765..31216806f78 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/format.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/format.go @@ -41,11 +41,11 @@ type Format struct { MaxRegexSize int } -func (d *Format) ConvertToNative(typeDesc reflect.Type) (interface{}, error) { +func (d Format) ConvertToNative(typeDesc reflect.Type) (interface{}, error) { return nil, fmt.Errorf("type conversion error from 'Format' to '%v'", typeDesc) } -func (d *Format) ConvertToType(typeVal ref.Type) ref.Val { +func (d Format) ConvertToType(typeVal ref.Type) ref.Val { switch typeVal { case FormatType: return d @@ -56,18 +56,18 @@ func (d *Format) ConvertToType(typeVal ref.Type) ref.Val { } } -func (d *Format) Equal(other ref.Val) ref.Val { - otherDur, ok := other.(*Format) +func (d Format) Equal(other ref.Val) ref.Val { + otherDur, ok := other.(Format) if !ok { return types.MaybeNoSuchOverloadErr(other) } return types.Bool(d.Name == otherDur.Name) } -func (d *Format) Type() ref.Type { +func (d Format) Type() ref.Type { return FormatType } -func (d *Format) Value() interface{} { +func (d Format) Value() interface{} { return d } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go index 8686e6c17c0..2992e99e658 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cidr.go @@ -109,7 +109,7 @@ var cidrsLib = &cidrs{} type cidrs struct{} func (*cidrs) LibraryName() string { - return "net.cidr" + return "kubernetes.net.cidr" } func (*cidrs) declarations() map[string][]cel.FunctionOpt { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go index 2ffb0755d6b..14b74dc6b02 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cost.go @@ -25,7 +25,6 @@ import ( "github.com/google/cel-go/common/types/ref" "github.com/google/cel-go/common/types/traits" "math" - "strings" "k8s.io/apiserver/pkg/cel" ) @@ -201,7 +200,7 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re } case "validate": if len(args) >= 2 { - format, isFormat := args[0].Value().(*cel.Format) + format, isFormat := args[0].Value().(cel.Format) if isFormat { strSize := actualSize(args[1]) @@ -243,13 +242,14 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re case *cel.Quantity, cel.Quantity, *cel.IP, cel.IP, *cel.CIDR, cel.CIDR, - *cel.Format, // Formats have a small max size. Format takes pointer receiver. + *cel.Format, cel.Format, // Formats have a small max size. Format takes pointer receiver. *cel.URL, cel.URL, // TODO: Computing the actual cost is expensive, and changing this would be a breaking change + *cel.Semver, cel.Semver, *authorizerVal, authorizerVal, *pathCheckVal, pathCheckVal, *groupCheckVal, groupCheckVal, *resourceCheckVal, resourceCheckVal, *decisionVal, decisionVal: return &unitCost default: - if panicOnUnknown && isKubernetesType(lhs.Type()) { + if panicOnUnknown && lhs.Type() != nil && isRegisteredType(lhs.Type().TypeName()) { panic(fmt.Errorf("CallCost: unhandled equality for Kubernetes type %T", lhs)) } } @@ -509,7 +509,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch if t.Kind() == types.StructKind { switch t { case cel.QuantityType, AuthorizerType, PathCheckType, // O(1) cost equality checks - GroupCheckType, ResourceCheckType, DecisionType: + GroupCheckType, ResourceCheckType, DecisionType, cel.SemverType: return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}} case cel.FormatType: return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: cel.MaxFormatSize}.MultiplyByCostFactor(common.StringTraversalCostFactor)} @@ -523,7 +523,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: size.Max}.MultiplyByCostFactor(common.StringTraversalCostFactor)} } } - if panicOnUnknown && isKubernetesType(t) { + if panicOnUnknown && isRegisteredType(t.TypeName()) { panic(fmt.Errorf("EstimateCallCost: unhandled equality for Kubernetes type %v", t)) } } @@ -632,17 +632,3 @@ func traversalCost(v ref.Val) uint64 { return 1 } } - -// isKubernetesType returns ture if a type is type defined by Kubernetes, -// as identified by opaque or struct types with a "kubernetes." prefix. -func isKubernetesType(t ref.Type) bool { - if tt, ok := t.(*types.Type); ok { - switch tt.Kind() { - case types.OpaqueKind, types.StructKind: - return strings.HasPrefix(tt.TypeName(), "kubernetes.") - default: - return false - } - } - return false -} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go index b629a007b98..4da1934f66d 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go @@ -1262,7 +1262,8 @@ func TestTypeEquality(t *testing.T) { "kubernetes.Quantity": apiservercel.Quantity{}, "net.IP": apiservercel.IP{}, "net.CIDR": apiservercel.CIDR{}, - "kubernetes.NamedFormat": &apiservercel.Format{}, + "kubernetes.NamedFormat": apiservercel.Format{}, + "kubernetes.Semver": apiservercel.Semver{}, } originalPanicOnUnknown := panicOnUnknown diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go index f76c6a29d56..83c821c8c11 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go @@ -133,7 +133,7 @@ func (*format) ProgramOptions() []cel.ProgramOption { return []cel.ProgramOption{} } -var ConstantFormats map[string]*apiservercel.Format = map[string]*apiservercel.Format{ +var ConstantFormats = map[string]apiservercel.Format{ "dns1123Label": { Name: "DNS1123Label", ValidateFunc: func(s string) []string { return apimachineryvalidation.NameIsDNSLabel(s, false) }, @@ -261,7 +261,7 @@ var formatLibraryDecls = map[string][]cel.FunctionOpt{ } func formatValidate(arg1, arg2 ref.Val) ref.Val { - f, ok := arg1.Value().(*apiservercel.Format) + f, ok := arg1.Value().(apiservercel.Format) if !ok { return types.MaybeNoSuchOverloadErr(arg1) } diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go b/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go index b957afe769b..8edc4463a07 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/ip.go @@ -132,7 +132,7 @@ var ipLib = &ip{} type ip struct{} func (*ip) LibraryName() string { - return "net.ip" + return "kubernetes.net.ip" } func (*ip) declarations() map[string][]cel.FunctionOpt { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go b/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go index 75a9b0e998f..e3689e3e0eb 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/libraries.go @@ -1,5 +1,5 @@ /* -Copyright 2023 The Kubernetes Authors. +Copyright 2024 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. @@ -16,7 +16,9 @@ limitations under the License. package library -import "github.com/google/cel-go/cel" +import ( + "github.com/google/cel-go/cel" +) // Library represents a CEL library used by kubernetes. type Library interface { @@ -42,5 +44,17 @@ func KnownLibraries() []Library { ipLib, cidrsLib, formatLib, + semverLib, } } + +func isRegisteredType(typeName string) bool { + for _, lib := range KnownLibraries() { + for _, rt := range lib.Types() { + if rt.TypeName() == typeName { + return true + } + } + } + return false +} diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go index d42a1b91a15..d76100a8c93 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go @@ -49,7 +49,7 @@ func TestLibraryCompatibility(t *testing.T) { // Kubernetes <1.30>: "ip", "family", "isUnspecified", "isLoopback", "isLinkLocalMulticast", "isLinkLocalUnicast", "isGlobalUnicast", "ip.isCanonical", "isIP", "cidr", "containsIP", "containsCIDR", "masked", "prefixLength", "isCIDR", "string", // Kubernetes <1.31>: - "fieldSelector", "labelSelector", "validate", "format.named", + "fieldSelector", "labelSelector", "validate", "format.named", "isSemver", "major", "minor", "patch", "semver", // Kubernetes <1.??>: ) @@ -101,5 +101,4 @@ func TestTypeRegistration(t *testing.T) { t.Errorf("Expected types to be registered with the %s library Type() functions, but they were not: %v", lib.LibraryName(), unregistered) } } - } diff --git a/staging/src/k8s.io/dynamic-resource-allocation/cel/semver_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/semver_test.go similarity index 96% rename from staging/src/k8s.io/dynamic-resource-allocation/cel/semver_test.go rename to staging/src/k8s.io/apiserver/pkg/cel/library/semver_test.go index d71cfdf91ff..3b1471ddaa8 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/cel/semver_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/semver_test.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cel_test +package library_test import ( "regexp" @@ -27,7 +27,8 @@ import ( "github.com/stretchr/testify/require" "k8s.io/apimachinery/pkg/util/sets" - library "k8s.io/dynamic-resource-allocation/cel" + apiservercel "k8s.io/apiserver/pkg/cel" + library "k8s.io/apiserver/pkg/cel/library" ) func testSemver(t *testing.T, expr string, expectResult ref.Val, expectRuntimeErrPattern string, expectCompileErrs []string) { @@ -117,7 +118,7 @@ func TestSemver(t *testing.T) { { name: "parse", expr: `semver("1.2.3")`, - expectValue: library.Semver{Version: semver.MustParse("1.2.3")}, + expectValue: apiservercel.Semver{Version: semver.MustParse("1.2.3")}, }, { name: "parseInvalidVersion", diff --git a/staging/src/k8s.io/dynamic-resource-allocation/cel/semverlib.go b/staging/src/k8s.io/apiserver/pkg/cel/library/semverlib.go similarity index 80% rename from staging/src/k8s.io/dynamic-resource-allocation/cel/semverlib.go rename to staging/src/k8s.io/apiserver/pkg/cel/library/semverlib.go index c9470761e73..d8c79ae0217 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/cel/semverlib.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/semverlib.go @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package cel +package library import ( "github.com/blang/semver/v4" "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" + + apiservercel "k8s.io/apiserver/pkg/cel" ) // Semver provides a CEL function library extension for [semver.Version]. @@ -91,38 +93,45 @@ var semverLib = &semverLibType{} type semverLibType struct{} func (*semverLibType) LibraryName() string { - return "k8s.semver" + return "kubernetes.Semver" } -func (*semverLibType) CompileOptions() []cel.EnvOption { - // Defined in this function to avoid an initialization order problem. - semverLibraryDecls := map[string][]cel.FunctionOpt{ +func (*semverLibType) Types() []*cel.Type { + return []*cel.Type{apiservercel.SemverType} +} + +func (*semverLibType) declarations() map[string][]cel.FunctionOpt { + return map[string][]cel.FunctionOpt{ "semver": { - cel.Overload("string_to_semver", []*cel.Type{cel.StringType}, SemverType, cel.UnaryBinding((stringToSemver))), + cel.Overload("string_to_semver", []*cel.Type{cel.StringType}, apiservercel.SemverType, cel.UnaryBinding((stringToSemver))), }, "isSemver": { cel.Overload("is_semver_string", []*cel.Type{cel.StringType}, cel.BoolType, cel.UnaryBinding(isSemver)), }, "isGreaterThan": { - cel.MemberOverload("semver_is_greater_than", []*cel.Type{SemverType, SemverType}, cel.BoolType, cel.BinaryBinding(semverIsGreaterThan)), + cel.MemberOverload("semver_is_greater_than", []*cel.Type{apiservercel.SemverType, apiservercel.SemverType}, cel.BoolType, cel.BinaryBinding(semverIsGreaterThan)), }, "isLessThan": { - cel.MemberOverload("semver_is_less_than", []*cel.Type{SemverType, SemverType}, cel.BoolType, cel.BinaryBinding(semverIsLessThan)), + cel.MemberOverload("semver_is_less_than", []*cel.Type{apiservercel.SemverType, apiservercel.SemverType}, cel.BoolType, cel.BinaryBinding(semverIsLessThan)), }, "compareTo": { - cel.MemberOverload("semver_compare_to", []*cel.Type{SemverType, SemverType}, cel.IntType, cel.BinaryBinding(semverCompareTo)), + cel.MemberOverload("semver_compare_to", []*cel.Type{apiservercel.SemverType, apiservercel.SemverType}, cel.IntType, cel.BinaryBinding(semverCompareTo)), }, "major": { - cel.MemberOverload("semver_major", []*cel.Type{SemverType}, cel.IntType, cel.UnaryBinding(semverMajor)), + cel.MemberOverload("semver_major", []*cel.Type{apiservercel.SemverType}, cel.IntType, cel.UnaryBinding(semverMajor)), }, "minor": { - cel.MemberOverload("semver_minor", []*cel.Type{SemverType}, cel.IntType, cel.UnaryBinding(semverMinor)), + cel.MemberOverload("semver_minor", []*cel.Type{apiservercel.SemverType}, cel.IntType, cel.UnaryBinding(semverMinor)), }, "patch": { - cel.MemberOverload("semver_patch", []*cel.Type{SemverType}, cel.IntType, cel.UnaryBinding(semverPatch)), + cel.MemberOverload("semver_patch", []*cel.Type{apiservercel.SemverType}, cel.IntType, cel.UnaryBinding(semverPatch)), }, } +} +func (s *semverLibType) CompileOptions() []cel.EnvOption { + // Defined in this function to avoid an initialization order problem. + semverLibraryDecls := s.declarations() options := make([]cel.EnvOption, 0, len(semverLibraryDecls)) for name, overloads := range semverLibraryDecls { options = append(options, cel.Function(name, overloads...)) @@ -168,7 +177,7 @@ func stringToSemver(arg ref.Val) ref.Val { return types.WrapErr(err) } - return Semver{Version: v} + return apiservercel.Semver{Version: v} } func semverMajor(arg ref.Val) ref.Val { diff --git a/staging/src/k8s.io/dynamic-resource-allocation/cel/semver.go b/staging/src/k8s.io/apiserver/pkg/cel/semver.go similarity index 100% rename from staging/src/k8s.io/dynamic-resource-allocation/cel/semver.go rename to staging/src/k8s.io/apiserver/pkg/cel/semver.go diff --git a/staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go b/staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go index 7cad34651e2..3c140a1a220 100644 --- a/staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go +++ b/staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go @@ -37,6 +37,7 @@ import ( celconfig "k8s.io/apiserver/pkg/apis/cel" apiservercel "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/environment" + "k8s.io/apiserver/pkg/cel/library" ) const ( @@ -151,7 +152,7 @@ func getAttributeValue(attr resourceapi.DeviceAttribute) (any, error) { if err != nil { return nil, fmt.Errorf("parse semantic version: %w", err) } - return Semver{Version: v}, nil + return apiservercel.Semver{Version: v}, nil default: return nil, errors.New("unsupported attribute value") } @@ -236,7 +237,7 @@ func mustBuildEnv() *environment.EnvSet { EnvOptions: []cel.EnvOption{ cel.Variable(deviceVar, deviceType.CelType()), - SemverLib(), + library.SemverLib(), // https://pkg.go.dev/github.com/google/cel-go/ext#Bindings // From 430b1de921b85611b409887fe94988f81ec4d39f Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Tue, 10 Sep 2024 17:07:29 -0400 Subject: [PATCH 4/4] Test library and type names --- .../src/k8s.io/apiserver/pkg/cel/library/authz.go | 4 ++-- .../src/k8s.io/apiserver/pkg/cel/library/format.go | 2 +- .../pkg/cel/library/library_compatibility_test.go | 14 ++++++++++++-- .../src/k8s.io/apiserver/pkg/cel/library/lists.go | 2 +- .../k8s.io/apiserver/pkg/cel/library/quantity.go | 2 +- .../src/k8s.io/apiserver/pkg/cel/library/regex.go | 2 +- .../src/k8s.io/apiserver/pkg/cel/library/test.go | 2 +- .../src/k8s.io/apiserver/pkg/cel/library/urls.go | 2 +- 8 files changed, 20 insertions(+), 10 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go b/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go index 0acd5a542a1..77332cff8bd 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go @@ -232,7 +232,7 @@ var authzLib = &authz{} type authz struct{} func (*authz) LibraryName() string { - return "k8s.authz" + return "kubernetes.authz" } func (*authz) Types() []*cel.Type { @@ -337,7 +337,7 @@ var authzSelectorsLib = &authzSelectors{} type authzSelectors struct{} func (*authzSelectors) LibraryName() string { - return "k8s.authzSelectors" + return "kubernetes.authzSelectors" } func (*authzSelectors) Types() []*cel.Type { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go index 83c821c8c11..82ecffb412f 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/format.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/format.go @@ -91,7 +91,7 @@ var formatLib = &format{} type format struct{} func (*format) LibraryName() string { - return "format" + return "kubernetes.format" } func (*format) Types() []*cel.Type { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go index d76100a8c93..50b5d22882e 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/library_compatibility_test.go @@ -20,6 +20,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/common/decls" "github.com/google/cel-go/common/types" + "strings" "testing" "k8s.io/apimachinery/pkg/util/sets" @@ -28,6 +29,9 @@ import ( func TestLibraryCompatibility(t *testing.T) { functionNames := sets.New[string]() for _, lib := range KnownLibraries() { + if !strings.HasPrefix(lib.LibraryName(), "kubernetes.") { + t.Errorf("Expected all kubernetes CEL libraries to have a name package with a 'kubernetes.' prefix but got %v", lib.LibraryName()) + } for name := range lib.declarations() { functionNames[name] = struct{}{} } @@ -93,8 +97,11 @@ func TestTypeRegistration(t *testing.T) { } } } - for _, t := range lib.Types() { - registeredTypes.Insert(t) + for _, lb := range lib.Types() { + registeredTypes.Insert(lb) + if !strings.HasPrefix(lb.TypeName(), "kubernetes.") && !legacyTypeNames.Has(lb.TypeName()) { + t.Errorf("Expected all types in kubernetes CEL libraries to have a type name packaged with a 'kubernetes.' prefix but got %v", lb.TypeName()) + } } unregistered := usedTypes.Difference(registeredTypes) if len(unregistered) != 0 { @@ -102,3 +109,6 @@ func TestTypeRegistration(t *testing.T) { } } } + +// TODO: Consider renaming these to "kubernetes.net.IP" and "kubernetes.net.CIDR" if we decide not to promote them to cel-go +var legacyTypeNames = sets.New[string]("net.IP", "net.CIDR") diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go b/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go index d56e72761d8..1f61b1181e4 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go @@ -96,7 +96,7 @@ var listsLib = &lists{} type lists struct{} func (*lists) LibraryName() string { - return "k8s.lists" + return "kubernetes.lists" } func (*lists) Types() []*cel.Type { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go b/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go index 4d9f1ac3eab..236b366b465 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go @@ -143,7 +143,7 @@ var quantityLib = &quantity{} type quantity struct{} func (*quantity) LibraryName() string { - return "k8s.quantity" + return "kubernetes.quantity" } func (*quantity) Types() []*cel.Type { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go b/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go index e8577a18517..2cf8b00375c 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go @@ -52,7 +52,7 @@ var regexLib = ®ex{} type regex struct{} func (*regex) LibraryName() string { - return "k8s.regex" + return "kubernetes.regex" } func (*regex) Types() []*cel.Type { diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/test.go index dcbc058a110..282d9396208 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/test.go @@ -38,7 +38,7 @@ type testLib struct { } func (*testLib) LibraryName() string { - return "k8s.test" + return "kubernetes.test" } type TestOption func(*testLib) *testLib diff --git a/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go b/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go index 058706d8e07..4b7ffb95a4e 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go @@ -113,7 +113,7 @@ var urlsLib = &urls{} type urls struct{} func (*urls) LibraryName() string { - return "k8s.urls" + return "kubernetes.urls" } func (*urls) Types() []*cel.Type {