From 43c71f661b5d85ae9441038a1e36654cd77f9561 Mon Sep 17 00:00:00 2001 From: Tristan Swadell Date: Wed, 22 Jul 2026 22:20:06 +0000 Subject: [PATCH] Allow validators to be reconfigured --- cel/validator.go | 14 ++++-- cel/validator_test.go | 110 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) diff --git a/cel/validator.go b/cel/validator.go index 905b99990..5c49705da 100644 --- a/cel/validator.go +++ b/cel/validator.go @@ -81,12 +81,20 @@ type ASTValidatorFactory func(*env.Validator) (ASTValidator, error) // ASTValidators configures a set of ASTValidator instances into the target environment. // -// Validators are applied in the order in which the are specified and are treated as singletons. -// The same ASTValidator with a given name will not be applied more than once. +// Validators are applied in the order in which they are specified. +// If an ASTValidator with the same name is already configured, it will be replaced. func ASTValidators(validators ...ASTValidator) EnvOption { return func(e *Env) (*Env, error) { for _, v := range validators { - if !e.HasValidator(v.Name()) { + found := false + for i, existing := range e.validators { + if existing.Name() == v.Name() { + e.validators[i] = v + found = true + break + } + } + if !found { e.validators = append(e.validators, v) } } diff --git a/cel/validator_test.go b/cel/validator_test.go index 76f30d13e..606a5b6d9 100644 --- a/cel/validator_test.go +++ b/cel/validator_test.go @@ -18,6 +18,7 @@ import ( "reflect" "testing" + celenv "github.com/google/cel-go/common/env" "github.com/google/cel-go/common/operators" "github.com/google/cel-go/common/overloads" "github.com/google/cel-go/common/types" @@ -416,3 +417,112 @@ func TestValidatorConfig(t *testing.T) { t.Error("config.Set() with incorrect value type did not fail") } } + +func TestOverrideValidator(t *testing.T) { + // Base environment configured with comprehension nesting limit of 2. + baseEnv, err := NewEnv( + ASTValidators(ValidateComprehensionNestingLimit(2)), + ) + if err != nil { + t.Fatalf("NewEnv() failed: %v", err) + } + + expr := `[1, 2, 3].map(i, [4, 5, 6].map(j, [7, 8, 9].map(k, i * j * k)))` + + // Fails in base environment with limit 2. + _, iss := baseEnv.Compile(expr) + if iss.Err() == nil { + t.Fatalf("baseEnv.Compile() succeeded, expected nesting limit error") + } + + // Extend environment overriding limit to 3. + extEnv, err := baseEnv.Extend( + ASTValidators(ValidateComprehensionNestingLimit(3)), + ) + if err != nil { + t.Fatalf("baseEnv.Extend() failed: %v", err) + } + + // Succeeds in extended environment with limit 3. + _, iss = extEnv.Compile(expr) + if iss.Err() != nil { + t.Fatalf("extEnv.Compile() failed: %v", iss.Err()) + } + + // Verify base environment still fails (immutability check). + _, iss = baseEnv.Compile(expr) + if iss.Err() == nil { + t.Fatalf("baseEnv.Compile() succeeded after Extend, expected baseEnv to remain unchanged") + } + + // Extend environment overriding limit to 1 (stricter limit). + stricterEnv, err := extEnv.Extend( + ASTValidators(ValidateComprehensionNestingLimit(1)), + ) + if err != nil { + t.Fatalf("extEnv.Extend() failed: %v", err) + } + + expr2 := `[1, 2, 3].exists(i, [4, 5, 6].filter(j, j % i != 0).size() > 0)` + // 2 levels deep: succeeds in extEnv (limit 3), fails in stricterEnv (limit 1). + _, iss = extEnv.Compile(expr2) + if iss.Err() != nil { + t.Fatalf("extEnv.Compile() for 2 levels failed: %v", iss.Err()) + } + _, iss = stricterEnv.Compile(expr2) + if iss.Err() == nil { + t.Fatalf("stricterEnv.Compile() for 2 levels succeeded, expected error") + } +} + +func TestOverrideValidatorFromConfig(t *testing.T) { + baseEnv, err := NewEnv( + ASTValidators(ValidateComprehensionNestingLimit(2)), + ) + if err != nil { + t.Fatalf("NewEnv() failed: %v", err) + } + + conf := celenv.NewConfig("override_validator"). + AddValidators(celenv.NewValidator(nestingLimitValidatorName).SetConfig(map[string]any{"limit": 3})) + + extEnv, err := baseEnv.Extend(FromConfig(conf)) + if err != nil { + t.Fatalf("baseEnv.Extend(FromConfig) failed: %v", err) + } + + expr := `[1, 2, 3].map(i, [4, 5, 6].map(j, [7, 8, 9].map(k, i * j * k)))` + _, iss := extEnv.Compile(expr) + if iss.Err() != nil { + t.Fatalf("extEnv.Compile() failed: %v", iss.Err()) + } +} + +func TestOverrideValidatorPreservesOrder(t *testing.T) { + v1 := ValidateDurationLiterals() + v2 := ValidateComprehensionNestingLimit(2) + v3 := ValidateTimestampLiterals() + + env1, err := NewEnv(ASTValidators(v1, v2, v3)) + if err != nil { + t.Fatalf("NewEnv() failed: %v", err) + } + + v2New := ValidateComprehensionNestingLimit(5) + env2, err := env1.Extend(ASTValidators(v2New)) + if err != nil { + t.Fatalf("Extend() failed: %v", err) + } + + validators := env2.Validators() + if len(validators) != 3 { + t.Fatalf("got %d validators, wanted 3", len(validators)) + } + if validators[0].Name() != v1.Name() || validators[1].Name() != v2.Name() || validators[2].Name() != v3.Name() { + t.Fatalf("validator order or names unexpected: %v, %v, %v", validators[0].Name(), validators[1].Name(), validators[2].Name()) + } + if val, ok := validators[1].(nestingLimitValidator); !ok || val.limit != 5 { + t.Fatalf("expected overridden nesting limit validator with limit 5, got %v", validators[1]) + } +} +