feat(overrides): add CEL valueProgram support for dynamic override values by JinghanFu · Pull Request #595 · Azure/eno

JinghanFu · 2026-04-21T08:14:48Z

Summary

Adds a valueProgram field to overrides, allowing CEL expressions to dynamically compute override values from the current (live) resource state. Previously, overrides could only set static values. With valueProgram, the override value is evaluated at apply time against the actual cluster resource, enabling scenarios like preserving VPA-adjusted resource quantities.

Motivation

When external actors (e.g., VPA) modify resource fields, Eno's static overrides couldn't reference the current cluster state to decide what value to use. valueProgram solves this by letting a CEL expression read from the live resource and return a computed value.

Changes

`internal/resource/mutation/mutation.go`

Added ValueProgram cel.Program field to Op struct
Added valueProgram JSON field to jsonOp for deserialization
UnmarshalJSON: parses valueProgram string into a compiled cel.Program via enocel.Parse()
Apply: if ValueProgram is set, evaluates it against the current resource state to resolve the value dynamically; skips mutation if current is nil or CEL evaluates to null

`internal/resource/mutation/mutation_test.go`

Added tests for valueProgram evaluation, null handling, and missing current resource scenarios

`pkg/function/overrides/override.go`

Added ValueProgram string field to Override struct
validate(): if ValueProgram is non-empty, parses it as CEL to ensure validity

`pkg/function/overrides/override_test.go`

Added unit tests for ValueProgram validation and Override.Test() behavior

`internal/controllers/reconciliation/overrides_test.go`

Added integration-level tests exercising valueProgram through the full SnapshotWithOverrides → Op.Apply path

`docs/overrides.md`

Added documentation for the valueProgram field with usage examples

Testing

All existing and new unit tests pass (go test ./internal/resource/mutation/... ./pkg/function/overrides/... ./internal/cel/...)

ruinan-liu · 2026-04-21T17:50:21Z

-	Value     any    `json:"value"`
-	Condition string `json:"condition"`
+	Path         string `json:"path"`
+	Value        any    `json:"value,omitempty"`


is adding omitempty intentinoal?

ruinan-liu · 2026-04-21T17:54:16Z

@@ -55,6 +56,13 @@ func (o *Override) validate() (cel.Program, error) {
 		return nil, fmt.Errorf("failed to create program: %w", err)
 	}



nit: If value and valueProgram are mutually exclusive, can we add a logging here for telemetry?

Hi, Ruinan. May I know whether you mean checking the cases for mutually exclusive and then return nil, Errorf("error message") here (consistent with all other checks in this function) or printing this out by fmt.Printf("error message")? Thank you!

Yeah sorry maybe not here, but it would be better if we can add a logging in telemetry saying that we are currently using valueProgram instead of Value.

Not a blocker, if it's too complicated we don't need this.

Thank you, Ruinan! I put the logging in telemetry in mutation.go and they look like:
if o.ValueProgram != nil { ... logger.Info("override using valueProgram (resolved CEL value expression)", "path", o.Path.String()) } else { logger.Info("override using static default value", "path", o.Path.String()) }

ruinan-liu · 2026-04-21T17:56:10Z

+			logger.Info("CEL value expression evaluated to null, skipping mutation", "path", o.Path.String())
+			return StatusInactive, nil
+		}
+		logger.Info("resolved CEL value expression", "path", o.Path.String(), "resolvedValue", resolvedValue)


would logging resolvedValue be too sensitive to log in telemetry.

ruinan-liu · 2026-04-21T18:04:14Z

Could we also add a e2e test like in this PR? https://github.com/Azure/eno/pull/590/changes#diff-ff578928676707b352eb1dd2ee6a304cf2887ec8969d83e872ba5b4243b1f930.
You could ask copilot to generate the tests, it's pretty accurate in writing tests.

ruinan-liu · 2026-04-21T18:04:39Z

For smoke test failures, I have a fix in this PR: #526. I'm currently working on getting it to merge.

ruinan-liu

Thanks Jinghan for the Pr. I've added some minor comments. Please let me know if you have any question.

ruinan-liu

reply to comments

…lues Add valueProgram field to overrides that allows CEL expressions to compute override values dynamically from the current resource state, instead of only supporting static values. This enables use cases like preserving VPA-managed resource quantities conditionally. - Add ValueProgram field to Op and Override structs - Evaluate valueProgram CEL expressions during Op.Apply - Add validation for valueProgram in override helpers - Add comprehensive unit and integration tests - Add overrides documentation

… review feedback - Merge TestAnnotateOverrides_* into single table-driven TestAnnotateOverrides_Table - Rename TestOverrideValueExpression to TestOverrideValueProgram - Fix valueExpression references in comments to valueProgram - Add StatusInvalidValueProgram for CEL value program evaluation errors - Remove '(returns Inactive)' from docs/overrides.md

- Add valueProgram field for dynamic CEL-based override values - Add telemetry logging to distinguish valueProgram vs static value usage - Add e2e tests for CEL override functionality

xiazhan · 2026-04-22T00:01:28Z

+	Path         string `json:"path"`
+	Value        any    `json:"value,omitempty"`
+	Condition    string `json:"condition"`
+	ValueProgram string `json:"valueProgram,omitempty"`


nit: Would be Expression or ValueExpression better?

I initially named it ValueExpression in override.go to reflect its string type, and ValueProgram in mutation.go to reflect its compiled cel.Program type. However, since the other three fields (Path, Condition, Value) use identical names across both files, I was concerned that having different names for this new field might introduce unnecessary confusion. So I opted to align them both as ValueProgram for consistency.

But if the naming discrepancy wouldn't cause confusion, I'm happy to rename it back to ValueExpression in override.go to more accurately reflect its string type!

…de struct

wonderyl · 2026-04-22T04:40:21Z

+    [
+      {
+        "path": "self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory",
+        "valueProgram": "self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory",


Rename here too?

wonderyl · 2026-04-22T04:40:53Z

+	StatusMissingParent       Status = "MissingParent"
+	StatusIndexOutOfRange     Status = "IndexOutOfRange"
+	StatusPathTypeMismatch    Status = "PathTypeMismatch"
+	StatusInvalidValueProgram Status = "InvalidValueProgram"


Rename here too

wonderyl · 2026-04-22T04:43:36Z

+		val, err := enocel.Eval(ctx, o.ValueProgram, comp, current, o.Path)
+		if err != nil {
+			logger.Error(err, "failed to evaluate value expression", "path", o.Path.String())
+			return StatusInvalidValueProgram, nil


should return error

wonderyl · 2026-04-22T04:46:24Z

+			return StatusInvalidValueProgram, nil
+		}
+		resolvedValue = val.Value()
+		if resolvedValue == nil || resolvedValue == structpb.NullValue_NULL_VALUE {


What if the user want to unset a value?

…ueExpression, return error, handle null unset

…e error handling - Rename Op.ValueProgram field to Op.ValueExpression for consistency with the existing jsonOp.ValueExpression and API naming convention - Wrap returned errors with fmt.Errorf to provide path context - Move error key-value pair to end of logger.Info calls for readability - Update all test case names and struct literals accordingly

ruinan-liu · 2026-04-24T17:14:49Z

 cloud.google.com/go/workflows v1.12.4/go.mod h1:yQ7HUqOkdJK4duVtMeBCAOPiN1ZF1E9pAMX51vpwB/w=
 cloud.google.com/go/workflows v1.13.0/go.mod h1:StCuY3jhBj1HYMjCPqZs7J0deQLHPhF6hDtzWJaVF+Y=
+codeberg.org/go-fonts/liberation v0.5.0/go.mod h1:zS/2e1354/mJ4pGzIIaEtm/59VFCFnYC7YV6YdGl5GU=
+codeberg.org/go-latex/latex v0.1.0/go.mod h1:LA0q/AyWIYrqVd+A9Upkgsb+IqPcmSTKc9Dny04MHMw=


Where are we using those changes?

ruinan-liu · 2026-04-24T17:22:15Z

+
+		if resolvedValue == structpb.NullValue_NULL_VALUE {
+			logger.Info("value expression evaluated to null (explicit unset)", "path", o.Path.String())
+			return StatusActive, nil


nit: Why are we returning StatusActive here?

ruinan-liu

Minor comments

xiazhan · 2026-04-24T17:29:30Z

+		if resolvedValue == structpb.NullValue_NULL_VALUE {
+			logger.Info("value expression evaluated to null (explicit unset)", "path", o.Path.String())
+			return StatusActive, nil
+		}


Where do we apply the value as "null"?

Did we verify that it works in e2e? Shall we add an e2e test case as well?

Convert structpb.NullValue_NULL_VALUE to Go nil and fall through to Apply() instead of returning early. This ensures that when a valueExpression evaluates to null, the corresponding field is actually deleted from the object rather than silently skipping the mutation.

Add e2e test verifying that a null valueExpression deletes the targeted field. Remove condition to prevent oscillation; use polling to wait for the second reconciliation to apply the deletion.

The null valueExpression override cannot work in non-SSA mode because buildNonStrategicPatch applies the override to both sides of the diff, producing an empty patch. Skip the test when DISABLE_SSA=true.

JinghanFu requested review from robbiezhang, ruinan-liu, tilnl and xiazhan as code owners April 21, 2026 08:14

JinghanFu force-pushed the jinghanfu/kcl-override-celValue branch from 107ff51 to 0b8abe2 Compare April 21, 2026 08:26

ruinan-liu reviewed Apr 21, 2026

View reviewed changes

JinghanFu added 4 commits April 22, 2026 09:50

chore: update go.work.sum

7effe96

feat(overrides): add valueProgram CEL support and telemetry logging

d239a3d

- Add valueProgram field for dynamic CEL-based override values - Add telemetry logging to distinguish valueProgram vs static value usage - Add e2e tests for CEL override functionality

JinghanFu force-pushed the jinghanfu/kcl-override-celValue branch from 7b81d01 to d239a3d Compare April 21, 2026 23:50

xiazhan reviewed Apr 22, 2026

View reviewed changes

JinghanFu added 3 commits April 22, 2026 11:21

test(e2e): update override CEL e2e test

e9c8b3d

fix(e2e): use idempotent valueProgram to prevent reconciliation loop

833d0e7

refactor(overrides): rename ValueProgram to ValueExpression in Overri…

24ec4c0

…de struct

wonderyl reviewed Apr 22, 2026

View reviewed changes

JinghanFu added 2 commits April 22, 2026 16:29

refactor(overrides): address PR feedback - rename valueProgram to val…

d691c04

…ueExpression, return error, handle null unset

ruinan-liu reviewed Apr 24, 2026

View reviewed changes

xiazhan reviewed Apr 24, 2026

View reviewed changes

JinghanFu added 2 commits April 25, 2026 10:27

test(mutation): add test cases for CEL null value field deletion

5fe02c7

JinghanFu force-pushed the jinghanfu/kcl-override-celValue branch from ee62c03 to 5fe02c7 Compare April 25, 2026 01:01

JinghanFu added 3 commits April 28, 2026 09:45

test(e2e): add CEL null valueExpression field deletion test

3c4c32c

test(e2e): add null CEL valueExpression override field-deletion test

634948d

Add e2e test verifying that a null valueExpression deletes the targeted field. Remove condition to prevent oscillation; use polling to wait for the second reconciliation to apply the deletion.

test(e2e): skip null valueExpression override test in non-SSA mode

7cb7058

The null valueExpression override cannot work in non-SSA mode because buildNonStrategicPatch applies the override to both sides of the diff, producing an empty patch. Skip the test when DISABLE_SSA=true.

		@@ -55,6 +56,13 @@ func (o *Override) validate() (cel.Program, error) {
		return nil, fmt.Errorf("failed to create program: %w", err)
		}

Conversation

JinghanFu commented Apr 21, 2026

Summary

Motivation

Changes

internal/resource/mutation/mutation.go

internal/resource/mutation/mutation_test.go

pkg/function/overrides/override.go

pkg/function/overrides/override_test.go

internal/controllers/reconciliation/overrides_test.go

docs/overrides.md

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruinan-liu commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ruinan-liu commented Apr 21, 2026

Uh oh!

ruinan-liu left a comment

Choose a reason for hiding this comment

Uh oh!

ruinan-liu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ruinan-liu left a comment

Choose a reason for hiding this comment

Uh oh!

xiazhan Apr 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xiazhan Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

`internal/resource/mutation/mutation.go`

`internal/resource/mutation/mutation_test.go`

`pkg/function/overrides/override.go`

`pkg/function/overrides/override_test.go`

`internal/controllers/reconciliation/overrides_test.go`

`docs/overrides.md`

ruinan-liu commented Apr 21, 2026 •

edited

Loading

xiazhan Apr 24, 2026 •

edited

Loading

xiazhan Apr 27, 2026 •

edited

Loading