From accfd98e20a7c322b7d38a2da35b123d48c0c63e Mon Sep 17 00:00:00 2001 From: Joe Betz Date: Wed, 6 Nov 2024 15:32:15 -0500 Subject: [PATCH] Wrap unversioned CEL library initializer calls with guard Kubernetes-commit: dc5e2f3fa295276029535359246154021861fdd6 --- pkg/admission/plugin/cel/compile.go | 2 +- pkg/cel/environment/base.go | 37 +++++++++++++++++++++-------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/pkg/admission/plugin/cel/compile.go b/pkg/admission/plugin/cel/compile.go index 8088c3325..f0fff1304 100644 --- a/pkg/admission/plugin/cel/compile.go +++ b/pkg/admission/plugin/cel/compile.go @@ -300,6 +300,6 @@ var hasPatchTypes = environment.VersionedOptions{ IntroducedVersion: version.MajorMinor(1, 0), EnvOptions: []cel.EnvOption{ common.ResolverEnvOption(&mutation.DynamicTypeResolver{}), - library.JSONPatch(), // for jsonPatch.escape() function + environment.UnversionedLib(library.JSONPatch), // for jsonPatch.escape() function }, } diff --git a/pkg/cel/environment/base.go b/pkg/cel/environment/base.go index 3e9827c82..b210377ae 100644 --- a/pkg/cel/environment/base.go +++ b/pkg/cel/environment/base.go @@ -72,9 +72,9 @@ var baseOptsWithoutStrictCost = []VersionedOptions{ cel.EagerlyValidateDeclarations(true), cel.DefaultUTCTimeZone(true), - library.URLs(), - library.Regex(), - library.Lists(), + UnversionedLib(library.URLs), + UnversionedLib(library.Regex), + UnversionedLib(library.Lists), // 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. // Since it is a regression fix, we apply it uniformly to all code use v0.17.7. @@ -92,7 +92,7 @@ var baseOptsWithoutStrictCost = []VersionedOptions{ { IntroducedVersion: version.MajorMinor(1, 27), EnvOptions: []cel.EnvOption{ - library.Authz(), + UnversionedLib(library.Authz), }, }, { @@ -100,7 +100,7 @@ var baseOptsWithoutStrictCost = []VersionedOptions{ EnvOptions: []cel.EnvOption{ cel.CrossTypeNumericComparisons(true), cel.OptionalTypes(), - library.Quantity(), + UnversionedLib(library.Quantity), }, }, // add the new validator in 1.29 @@ -139,15 +139,15 @@ var baseOptsWithoutStrictCost = []VersionedOptions{ { IntroducedVersion: version.MajorMinor(1, 30), EnvOptions: []cel.EnvOption{ - library.IP(), - library.CIDR(), + UnversionedLib(library.IP), + UnversionedLib(library.CIDR), }, }, // Format Library { IntroducedVersion: version.MajorMinor(1, 31), EnvOptions: []cel.EnvOption{ - library.Format(), + UnversionedLib(library.Format), }, }, // Authz selectors @@ -166,14 +166,14 @@ var baseOptsWithoutStrictCost = []VersionedOptions{ return enabled }, EnvOptions: []cel.EnvOption{ - library.AuthzSelectors(), + UnversionedLib(library.AuthzSelectors), }, }, // Two variable comprehensions { IntroducedVersion: version.MajorMinor(1, 32), EnvOptions: []cel.EnvOption{ - ext.TwoVarComprehensions(), + UnversionedLib(ext.TwoVarComprehensions), }, }, } @@ -264,3 +264,20 @@ var ( baseEnvsSingleflight = &singleflight.Group{} baseEnvsWithOptionSingleflight = &singleflight.Group{} ) + +// UnversionedLib wraps library initialization calls like ext.Sets() or library.IP() +// to force compilation errors if the call evolves to include a varadic variable option. +// +// This provides automatic detection of a problem that is hard to catch in review-- +// If a CEL library used in Kubernetes is unversioned and then become versioned, and we +// fail to set a desired version, the libraries defaults to the latest version, changing +// CEL environment without controlled rollout, bypassing the entire purpose of the base +// environment. +// +// If usages of this function fail to compile: add version=1 argument to all call sites +// that fail compilation while removing the UnversionedLib wrapper. Next, review +// the changes in the library present in higher versions and, if needed, use VersionedOptions to +// the base environment to roll out to a newer version safely. +func UnversionedLib(initializer func() cel.EnvOption) cel.EnvOption { + return initializer() +}