diff --git a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go index bf3144ab448..620d9e35e54 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/environment/base.go @@ -66,13 +66,6 @@ var baseOpts = []VersionedOptions{ cel.CostLimit(celconfig.PerCallLimit), }, }, - { - IntroducedVersion: version.MajorMinor(1, 0), - RemovedVersion: version.MajorMinor(1, 29), - EnvOptions: []cel.EnvOption{ - ext.Strings(ext.StringsVersion(0)), - }, - }, { IntroducedVersion: version.MajorMinor(1, 27), EnvOptions: []cel.EnvOption{ @@ -87,6 +80,15 @@ var baseOpts = []VersionedOptions{ library.Quantity(), }, }, + + // String library + { + IntroducedVersion: version.MajorMinor(1, 0), + RemovedVersion: version.MajorMinor(1, 29), + EnvOptions: []cel.EnvOption{ + ext.Strings(ext.StringsVersion(0)), + }, + }, { IntroducedVersion: version.MajorMinor(1, 29), EnvOptions: []cel.EnvOption{ 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 b424d687f6a..4bfb764576e 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 @@ -17,8 +17,11 @@ limitations under the License. package environment import ( + "sort" "testing" + "github.com/google/cel-go/cel" + "k8s.io/apimachinery/pkg/util/version" ) @@ -41,3 +44,90 @@ func BenchmarkLoadBaseEnvDifferentVersions(b *testing.B) { MustBaseEnvSet(version.MajorMinor(1, uint(i))) } } + +// TestLibraryCoverage lints the management of libraries in baseOpts by +// checking for: +// +// - No gaps and overlap in library inclusion, including when libraries are version bumped +// - RemovedVersion is always greater than IntroducedVersion +// - Libraries are not removed once added (although they can be replaced with new versions) +func TestLibraryCoverage(t *testing.T) { + vops := make([]VersionedOptions, len(baseOpts)) + copy(vops, baseOpts) + sort.SliceStable(vops, func(i, j int) bool { + return vops[i].IntroducedVersion.LessThan(vops[j].IntroducedVersion) + }) + + tracked := map[string]*versionTracker{} + + for _, vop := range vops { + if vop.RemovedVersion != nil { + if vop.IntroducedVersion == nil { + t.Errorf("VersionedOptions with RemovedVersion %v is missing required IntroducedVersion", vop.RemovedVersion) + } + if !vop.IntroducedVersion.LessThan(vop.RemovedVersion) { + t.Errorf("VersionedOptions with IntroducedVersion %s must be less than RemovedVersion %v", vop.IntroducedVersion, vop.RemovedVersion) + } + } + + for _, name := range librariesInVersions(t, vop) { + versionTracking, ok := tracked[name] + if !ok { + versionTracking = &versionTracker{} + tracked[name] = versionTracking + } + if versionTracking.added != nil { + t.Errorf("Did not expect %s library to be added again at version %v. It was already added at version %v", name, vop.IntroducedVersion, versionTracking.added) + } else { + versionTracking.added = vop.IntroducedVersion + if versionTracking.removed != nil { + if versionTracking.removed.LessThan(vop.IntroducedVersion) { + t.Errorf("Did not expect gap in presence of %s library. It was "+ + "removed in %v and not added again until %v. When versioning "+ + "libraries, introduce a new version of the library as the same "+ + "kubernetes version that the old version of the library is removed.", name, versionTracking.removed, vop.IntroducedVersion) + } else if vop.IntroducedVersion.LessThan(versionTracking.removed) { + t.Errorf("Did not expect overlap in presence of %s library. It was "+ + "added again at version %v while scheduled to be removed at %v. When versioning "+ + "libraries, introduce a new version of the library as the same "+ + "kubernetes version that the old version of the library is removed.", name, vop.IntroducedVersion, versionTracking.removed) + } + } + versionTracking.removed = nil + } + if vop.RemovedVersion != nil { + if versionTracking.removed != nil { + t.Errorf("Unexpected RemovedVersion of %v for library %s already removed at version %v", vop.RemovedVersion, name, versionTracking.removed) + } + versionTracking.added = nil + versionTracking.removed = vop.RemovedVersion + } + } + } + for name, lib := range tracked { + if lib.removed != nil { + t.Errorf("Unexpected RemovedVersion of %v for library %s without replacement. "+ + "For backward compatibility, libraries should not be removed without being replaced by a new version.", lib.removed, name) + } + } +} + +func librariesInVersions(t *testing.T, vops ...VersionedOptions) []string { + env, err := cel.NewCustomEnv() + if err != nil { + t.Fatalf("Error creating env: %v", err) + } + for _, vop := range vops { + env, err = env.Extend(vop.EnvOptions...) + if err != nil { + t.Fatalf("Error updating env: %v", err) + } + } + libs := env.Libraries() + return libs +} + +type versionTracker struct { + added *version.Version + removed *version.Version +} 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 db1347c7617..df4bf080714 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/authz.go @@ -202,6 +202,10 @@ var authzLib = &authz{} type authz struct{} +func (*authz) LibraryName() string { + return "k8s.authz" +} + var authzLibraryDecls = map[string][]cel.FunctionOpt{ "path": { cel.MemberOverload("authorizer_path", []*cel.Type{AuthorizerType, cel.StringType}, PathCheckType, 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 fbe7511fcf1..479e9ed1fe4 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 @@ -23,6 +23,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/checker" + "github.com/google/cel-go/common/types" "github.com/google/cel-go/ext" exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" @@ -538,11 +539,11 @@ func (t testSizeNode) Path() []string { return nil // not needed } -func (t testSizeNode) Type() *expr.Type { +func (t testSizeNode) Type() *types.Type { return nil // not needed } -func (t testSizeNode) Expr() *expr.Expr { +func (t testSizeNode) Expr() *exprpb.Expr { return nil // not needed } 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 0128c469396..e5288e19bd0 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 @@ -26,7 +26,7 @@ import ( func TestLibraryCompatibility(t *testing.T) { var libs []map[string][]cel.FunctionOpt - libs = append(libs, authzLibraryDecls, listsLibraryDecls, regexLibraryDecls, urlLibraryDecls) + libs = append(libs, authzLibraryDecls, listsLibraryDecls, regexLibraryDecls, urlLibraryDecls, quantityLibraryDecls) functionNames := sets.New[string]() for _, lib := range libs { for name := range lib { @@ -45,6 +45,8 @@ func TestLibraryCompatibility(t *testing.T) { "path", "group", "serviceAccount", "resource", "subresource", "namespace", "name", "check", "allowed", "reason", // Kubernetes <1.28>: "errored", "error", + // Kubernetes <1.29>: + "add", "asApproximateFloat", "asInteger", "compareTo", "isGreaterThan", "isInteger", "isLessThan", "isQuantity", "quantity", "sign", "sub", // Kubernetes <1.??>: ) 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 fe51dc87fdb..327ec93d6e2 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/lists.go @@ -95,6 +95,10 @@ var listsLib = &lists{} type lists struct{} +func (*lists) LibraryName() string { + return "k8s.lists" +} + 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 49e3dae7cdb..b4ac91c8a72 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/quantity.go @@ -22,6 +22,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" + "k8s.io/apimachinery/pkg/api/resource" apiservercel "k8s.io/apiserver/pkg/cel" ) @@ -141,6 +142,10 @@ var quantityLib = &quantity{} type quantity struct{} +func (*quantity) LibraryName() string { + return "k8s.quantity" +} + 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 17fb3d44c97..147a40f9bd2 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/regex.go @@ -51,6 +51,10 @@ var regexLib = ®ex{} type regex struct{} +func (*regex) LibraryName() string { + return "k8s.regex" +} + 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/test.go b/staging/src/k8s.io/apiserver/pkg/cel/library/test.go index 95446f63c6b..dcbc058a110 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/test.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/test.go @@ -37,6 +37,10 @@ type testLib struct { version uint32 } +func (*testLib) LibraryName() string { + return "k8s.test" +} + type TestOption func(*testLib) *testLib func TestVersion(version uint32) func(lib *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 7be054ece37..8f4ba85af7c 100644 --- a/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go +++ b/staging/src/k8s.io/apiserver/pkg/cel/library/urls.go @@ -112,6 +112,10 @@ var urlsLib = &urls{} type urls struct{} +func (*urls) LibraryName() string { + return "k8s.urls" +} + var urlLibraryDecls = map[string][]cel.FunctionOpt{ "url": { cel.Overload("string_to_url", []*cel.Type{cel.StringType}, apiservercel.URLType,