Move CEL env initialization out of package init()

This ensures compatibility version and feature gates can be initialized before cached CEL environments are created.
kubernetes · Jul 19, 2024 · 03d48b7 · 03d48b7
1 parent 1d2ad28
commit 03d48b7
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 27 deletions.
diff --git a/pkg/apis/admissionregistration/validation/validation.go b/pkg/apis/admissionregistration/validation/validation.go
@@ -21,6 +21,7 @@ import (
 	"reflect"
 	"regexp"
 	"strings"
+	"sync"
 
 	genericvalidation "k8s.io/apimachinery/pkg/api/validation"
 	"k8s.io/apimachinery/pkg/api/validation/path"
@@ -1066,9 +1067,9 @@ func validateMatchConditionsExpression(expression string, opts validationOptions
 	}
 	var compiler plugincel.Compiler
 	if opts.strictCostEnforcement {
-		compiler = strictStatelessCELCompiler
+		compiler = getStrictStatelessCELCompiler()
 	} else {
-		compiler = nonStrictStatelessCELCompiler
+		compiler = getNonStrictStatelessCELCompiler()
 	}
 	return validateCELCondition(compiler, &matchconditions.MatchCondition{
 		Expression: expression,
@@ -1270,15 +1271,34 @@ func validateFieldRef(fieldRef string, fldPath *field.Path) field.ErrorList {
 // variable composition is not allowed, for example, when validating MatchConditions.
 // strictStatelessCELCompiler is a cel Compiler that enforces strict cost enforcement.
 // nonStrictStatelessCELCompiler is a cel Compiler that does not enforce strict cost enforcement.
-var strictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
-var nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false))
+var (
+	lazyStrictStatelessCELCompilerInit sync.Once
+	lazyStrictStatelessCELCompiler     plugincel.Compiler
+
+	lazyNonStrictStatelessCELCompilerInit sync.Once
+	lazyNonStrictStatelessCELCompiler     plugincel.Compiler
+)
+
+func getStrictStatelessCELCompiler() plugincel.Compiler {
+	lazyStrictStatelessCELCompilerInit.Do(func() {
+		lazyStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
+	})
+	return lazyStrictStatelessCELCompiler
+}
+
+func getNonStrictStatelessCELCompiler() plugincel.Compiler {
+	lazyNonStrictStatelessCELCompilerInit.Do(func() {
+		lazyNonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false))
+	})
+	return lazyNonStrictStatelessCELCompiler
+}
 
 func createCompiler(allowComposition, strictCost bool) plugincel.Compiler {
 	if !allowComposition {
 		if strictCost {
-			return strictStatelessCELCompiler
+			return getStrictStatelessCELCompiler()
 		} else {
-			return nonStrictStatelessCELCompiler
+			return getNonStrictStatelessCELCompiler()
 		}
 	}
 	compiler, err := plugincel.NewCompositedCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost))

diff --git a/pkg/apis/admissionregistration/validation/validation_test.go b/pkg/apis/admissionregistration/validation/validation_test.go
@@ -3425,14 +3425,16 @@ func TestValidateValidatingAdmissionPolicyUpdate(t *testing.T) {
 		t.Fatal(err)
 	}
 	if strictCost {
-		strictStatelessCELCompiler = plugincel.NewCompiler(extended)
+		originalCompiler := getStrictStatelessCELCompiler()
+		lazyStrictStatelessCELCompiler = plugincel.NewCompiler(extended)
 		defer func() {
-			strictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost))
+			lazyStrictStatelessCELCompiler = originalCompiler
 		}()
 	} else {
-		nonStrictStatelessCELCompiler = plugincel.NewCompiler(extended)
+		originalCompiler := getNonStrictStatelessCELCompiler()
+		lazyNonStrictStatelessCELCompiler = plugincel.NewCompiler(extended)
 		defer func() {
-			nonStrictStatelessCELCompiler = plugincel.NewCompiler(environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), strictCost))
+			lazyNonStrictStatelessCELCompiler = originalCompiler
 		}()
 	}
 

diff --git a/pkg/apis/resource/structured/namedresources/validation/validation.go b/pkg/apis/resource/structured/namedresources/validation/validation.go
@@ -153,7 +153,7 @@ func validateSelector(opts Options, selector string, fldPath *field.Path) field.
 		if opts.StoredExpressions {
 			envType = environment.StoredExpressions
 		}
-		result := namedresourcescel.Compiler.CompileCELExpression(selector, envType)
+		result := namedresourcescel.GetCompiler().CompileCELExpression(selector, envType)
 		if result.Error != nil {
 			allErrs = append(allErrs, convertCELErrorToValidationError(fldPath, selector, result.Error))
 		}

diff --git a/...duler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go b/...duler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel.go
@@ -68,15 +68,15 @@ func AddAllocation(m *Model, result *resourceapi.NamedResourcesAllocationResult)
 func NewClaimController(filter *resourceapi.NamedResourcesFilter, requests []*resourceapi.NamedResourcesRequest) (*Controller, error) {
 	c := &Controller{}
 	if filter != nil {
-		compilation := cel.Compiler.CompileCELExpression(filter.Selector, environment.StoredExpressions)
+		compilation := cel.GetCompiler().CompileCELExpression(filter.Selector, environment.StoredExpressions)
 		if compilation.Error != nil {
 			// Shouldn't happen because of validation.
 			return nil, fmt.Errorf("compile class filter CEL expression: %w", compilation.Error)
 		}
 		c.filter = &compilation
 	}
 	for _, request := range requests {
-		compilation := cel.Compiler.CompileCELExpression(request.Selector, environment.StoredExpressions)
+		compilation := cel.GetCompiler().CompileCELExpression(request.Selector, environment.StoredExpressions)
 		if compilation.Error != nil {
 			// Shouldn't happen because of validation.
 			return nil, fmt.Errorf("compile request CEL expression: %w", compilation.Error)

diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/validating/plugin.go
@@ -19,6 +19,7 @@ package validating
 import (
 	"context"
 	"io"
+	"sync"
 
 	v1 "k8s.io/api/admissionregistration/v1"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -44,23 +45,34 @@ const (
 )
 
 var (
-	compositionEnvTemplateWithStrictCost *cel.CompositionEnv = func() *cel.CompositionEnv {
-		compositionEnvTemplateWithStrictCost, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
+	lazyCompositionEnvTemplateWithStrictCostInit sync.Once
+	lazyCompositionEnvTemplateWithStrictCost     *cel.CompositionEnv
+
+	lazyCompositionEnvTemplateWithoutStrictCostInit sync.Once
+	lazyCompositionEnvTemplateWithoutStrictCost     *cel.CompositionEnv
+)
+
+func getCompositionEnvTemplateWithStrictCost() *cel.CompositionEnv {
+	lazyCompositionEnvTemplateWithStrictCostInit.Do(func() {
+		env, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), true))
 		if err != nil {
 			panic(err)
 		}
+		lazyCompositionEnvTemplateWithStrictCost = env
+	})
+	return lazyCompositionEnvTemplateWithStrictCost
+}
 
-		return compositionEnvTemplateWithStrictCost
-	}()
-	compositionEnvTemplateWithoutStrictCost *cel.CompositionEnv = func() *cel.CompositionEnv {
-		compositionEnvTemplateWithoutStrictCost, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false))
+func getCompositionEnvTemplateWithoutStrictCost() *cel.CompositionEnv {
+	lazyCompositionEnvTemplateWithoutStrictCostInit.Do(func() {
+		env, err := cel.NewCompositionEnv(cel.VariablesTypeName, environment.MustBaseEnvSet(environment.DefaultCompatibilityVersion(), false))
 		if err != nil {
 			panic(err)
 		}
-
-		return compositionEnvTemplateWithoutStrictCost
-	}()
-)
+		lazyCompositionEnvTemplateWithoutStrictCost = env
+	})
+	return lazyCompositionEnvTemplateWithoutStrictCost
+}
 
 // Register registers a plugin
 func Register(plugins *admission.Plugins) {
@@ -131,9 +143,9 @@ func compilePolicy(policy *Policy) Validator {
 	matchConditions := policy.Spec.MatchConditions
 	var compositionEnvTemplate *cel.CompositionEnv
 	if strictCost {
-		compositionEnvTemplate = compositionEnvTemplateWithStrictCost
+		compositionEnvTemplate = getCompositionEnvTemplateWithStrictCost()
 	} else {
-		compositionEnvTemplate = compositionEnvTemplateWithoutStrictCost
+		compositionEnvTemplate = getCompositionEnvTemplateWithoutStrictCost()
 	}
 	filterCompiler := cel.NewCompositedCompilerFromTemplate(compositionEnvTemplate)
 	filterCompiler.CompileAndStoreVariables(convertv1beta1Variables(policy.Spec.Variables), optionalVars, environment.StoredExpressions)

diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile.go
@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"reflect"
+	"sync"
 
 	"github.com/blang/semver/v4"
 	"github.com/google/cel-go/cel"
@@ -38,9 +39,17 @@ const (
 )
 
 var (
-	Compiler = newCompiler()
+	lazyCompilerInit sync.Once
+	lazyCompiler     *compiler
 )
 
+func GetCompiler() *compiler {
+	lazyCompilerInit.Do(func() {
+		lazyCompiler = newCompiler()
+	})
+	return lazyCompiler
+}
+
 // CompilationResult represents a compiled expression.
 type CompilationResult struct {
 	Program     cel.Program

diff --git a/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile_test.go b/staging/src/k8s.io/dynamic-resource-allocation/structured/namedresources/cel/compile_test.go
@@ -124,7 +124,7 @@ attributes.stringslice["stringslice"].isSorted()`,
 	} {
 		t.Run(name, func(t *testing.T) {
 			_, ctx := ktesting.NewTestContext(t)
-			result := Compiler.CompileCELExpression(scenario.expression, environment.StoredExpressions)
+			result := GetCompiler().CompileCELExpression(scenario.expression, environment.StoredExpressions)
 			if scenario.expectCompileError != "" && result.Error == nil {
 				t.Fatalf("expected compile error %q, got none", scenario.expectCompileError)
 			}