Skip to content

Commit dc5e2f3

Browse files
committed
Wrap unversioned CEL library initializer calls with guard
1 parent 3d4a5da commit dc5e2f3

File tree

3 files changed

+30
-13
lines changed

3 files changed

+30
-13
lines changed

staging/src/k8s.io/apiserver/pkg/admission/plugin/cel/compile.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,6 @@ var hasPatchTypes = environment.VersionedOptions{
300300
IntroducedVersion: version.MajorMinor(1, 0),
301301
EnvOptions: []cel.EnvOption{
302302
common.ResolverEnvOption(&mutation.DynamicTypeResolver{}),
303-
library.JSONPatch(), // for jsonPatch.escape() function
303+
environment.UnversionedLib(library.JSONPatch), // for jsonPatch.escape() function
304304
},
305305
}

staging/src/k8s.io/apiserver/pkg/cel/environment/base.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ var baseOptsWithoutStrictCost = []VersionedOptions{
7272
cel.EagerlyValidateDeclarations(true),
7373
cel.DefaultUTCTimeZone(true),
7474

75-
library.URLs(),
76-
library.Regex(),
77-
library.Lists(),
75+
UnversionedLib(library.URLs),
76+
UnversionedLib(library.Regex),
77+
UnversionedLib(library.Lists),
7878

7979
// cel-go v0.17.7 change the cost of has() from 0 to 1, but also provided the CostEstimatorOptions option to preserve the old behavior, so we enabled it at the same time we bumped our cel version to v0.17.7.
8080
// Since it is a regression fix, we apply it uniformly to all code use v0.17.7.
@@ -92,15 +92,15 @@ var baseOptsWithoutStrictCost = []VersionedOptions{
9292
{
9393
IntroducedVersion: version.MajorMinor(1, 27),
9494
EnvOptions: []cel.EnvOption{
95-
library.Authz(),
95+
UnversionedLib(library.Authz),
9696
},
9797
},
9898
{
9999
IntroducedVersion: version.MajorMinor(1, 28),
100100
EnvOptions: []cel.EnvOption{
101101
cel.CrossTypeNumericComparisons(true),
102102
cel.OptionalTypes(),
103-
library.Quantity(),
103+
UnversionedLib(library.Quantity),
104104
},
105105
},
106106
// add the new validator in 1.29
@@ -139,15 +139,15 @@ var baseOptsWithoutStrictCost = []VersionedOptions{
139139
{
140140
IntroducedVersion: version.MajorMinor(1, 30),
141141
EnvOptions: []cel.EnvOption{
142-
library.IP(),
143-
library.CIDR(),
142+
UnversionedLib(library.IP),
143+
UnversionedLib(library.CIDR),
144144
},
145145
},
146146
// Format Library
147147
{
148148
IntroducedVersion: version.MajorMinor(1, 31),
149149
EnvOptions: []cel.EnvOption{
150-
library.Format(),
150+
UnversionedLib(library.Format),
151151
},
152152
},
153153
// Authz selectors
@@ -166,14 +166,14 @@ var baseOptsWithoutStrictCost = []VersionedOptions{
166166
return enabled
167167
},
168168
EnvOptions: []cel.EnvOption{
169-
library.AuthzSelectors(),
169+
UnversionedLib(library.AuthzSelectors),
170170
},
171171
},
172172
// Two variable comprehensions
173173
{
174174
IntroducedVersion: version.MajorMinor(1, 32),
175175
EnvOptions: []cel.EnvOption{
176-
ext.TwoVarComprehensions(),
176+
UnversionedLib(ext.TwoVarComprehensions),
177177
},
178178
},
179179
}
@@ -264,3 +264,20 @@ var (
264264
baseEnvsSingleflight = &singleflight.Group{}
265265
baseEnvsWithOptionSingleflight = &singleflight.Group{}
266266
)
267+
268+
// UnversionedLib wraps library initialization calls like ext.Sets() or library.IP()
269+
// to force compilation errors if the call evolves to include a varadic variable option.
270+
//
271+
// This provides automatic detection of a problem that is hard to catch in review--
272+
// If a CEL library used in Kubernetes is unversioned and then become versioned, and we
273+
// fail to set a desired version, the libraries defaults to the latest version, changing
274+
// CEL environment without controlled rollout, bypassing the entire purpose of the base
275+
// environment.
276+
//
277+
// If usages of this function fail to compile: add version=1 argument to all call sites
278+
// that fail compilation while removing the UnversionedLib wrapper. Next, review
279+
// the changes in the library present in higher versions and, if needed, use VersionedOptions to
280+
// the base environment to roll out to a newer version safely.
281+
func UnversionedLib(initializer func() cel.EnvOption) cel.EnvOption {
282+
return initializer()
283+
}

staging/src/k8s.io/dynamic-resource-allocation/cel/compile.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,15 +265,15 @@ func mustBuildEnv() *environment.EnvSet {
265265
EnvOptions: []cel.EnvOption{
266266
cel.Variable(deviceVar, deviceType.CelType()),
267267

268-
library.SemverLib(),
268+
environment.UnversionedLib(library.SemverLib),
269269

270270
// https://pkg.go.dev/github.com/google/cel-go/ext#Bindings
271271
//
272272
// This is useful to simplify attribute lookups because the
273273
// domain only needs to be given once:
274274
//
275275
// cel.bind(dra, device.attributes["dra.example.com"], dra.oneBool && dra.anotherBool)
276-
ext.Bindings(),
276+
ext.Bindings(ext.BindingsVersion(0)),
277277
},
278278
DeclTypes: []*apiservercel.DeclType{
279279
deviceType,

0 commit comments

Comments
 (0)