From 17b1d0b1c3674775fa8ae8d1856224a799161a37 Mon Sep 17 00:00:00 2001 From: Jinghan Fu Date: Fri, 8 May 2026 01:59:51 +1000 Subject: [PATCH 1/2] feat(overrides): add CEL valueExpression support for dynamic override values --- docs/overrides.md | 23 + e2e/overrideCEL_test.go | 214 +++++++++ internal/cel/cel.go | 3 + .../reconciliation/overrides_test.go | 364 +++++++++++++++ internal/resource/mutation/mutation.go | 86 +++- internal/resource/mutation/mutation_test.go | 431 +++++++++++++++++- internal/resource/mutation/parser.go | 34 +- internal/resource/resource.go | 47 +- internal/resource/resource_test.go | 19 +- pkg/function/overrides/override.go | 22 +- pkg/function/overrides/override_test.go | 234 ++++++---- 11 files changed, 1299 insertions(+), 178 deletions(-) create mode 100644 e2e/overrideCEL_test.go diff --git a/docs/overrides.md b/docs/overrides.md index cc225374..9c4d2f27 100644 --- a/docs/overrides.md +++ b/docs/overrides.md @@ -47,6 +47,29 @@ Supported fields: - `composition.metadata.labels` - `composition.metadata.annotations` +## Dynamic Values with CEL + +Use `valueExpression` instead of `value` to dynamically resolve the override value from the resource's current state using a CEL expression: + +```yaml +annotations: + eno.azure.io/overrides: | + [ + { + "path": "self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory", + "valueExpression": "self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory", + "condition": "has(self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory)" + } + ] +``` + +This reads the current value of `maxAllowed.memory` from the live resource and preserves it, allowing customer overrides to persist across reconciliation cycles. + +- `valueExpression` is evaluated against the **current** (actual) resource state, same as `condition` +- If the current resource doesn't exist yet, the override is skipped +- You can use both `condition` and `valueExpression` together — the condition is checked first +- `value` and `valueExpression` are mutually exclusive; if both are set, `valueExpression` takes precedence + ## Overriding Annotations Override these Eno annotations to modify `eno-reconciler` behavior at runtime: diff --git a/e2e/overrideCEL_test.go b/e2e/overrideCEL_test.go new file mode 100644 index 00000000..14867a60 --- /dev/null +++ b/e2e/overrideCEL_test.go @@ -0,0 +1,214 @@ +package e2e + +import ( + "context" + "testing" + "time" + + flow "github.com/Azure/go-workflow" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + + apiv1 "github.com/Azure/eno/api/v1" + fw "github.com/Azure/eno/e2e/framework" +) + +func TestOverrides_CELValueExpression_EndToEnd(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + cli := fw.NewClient(t) + + synthName := fw.UniqueName("override-cel-synth") + compName := fw.UniqueName("override-cel-comp") + cmName := fw.UniqueName("override-cel-cm") + + // ✅ Idempotent override — always produces the same value + overrideJSON := `[{ + "path": "self.data.foo", + "condition": "has(self.data.foo)", + "valueExpression": "'cel-override-value'" + }]` + + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: "default", + Annotations: map[string]string{ + "eno.azure.io/overrides": overrideJSON, + }, + }, + Data: map[string]string{ + "foo": "original", + }, + } + + synth := fw.NewMinimalSynthesizer( + synthName, + fw.WithCommand(fw.ToCommand(cm)), + ) + + comp := fw.NewComposition( + compName, + "default", + fw.WithSynthesizerRefs(apiv1.SynthesizerRef{Name: synthName}), + ) + + compKey := types.NamespacedName{ + Name: compName, + Namespace: "default", + } + + createSynth := fw.CreateStep(t, "createSynthesizer", cli, synth) + createComp := fw.CreateStep(t, "createComposition", cli, comp) + + waitReady := flow.Func("waitReady", func(ctx context.Context) error { + fw.WaitForCompositionReady(t, ctx, cli, compKey, 3*time.Minute) + return nil + }) + + verifyOverrideApplied := flow.Func("verifyOverrideApplied", func(ctx context.Context) error { + got := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: "default", + }, + } + fw.WaitForResourceExists(t, ctx, cli, got, 60*time.Second) + assert.Equal(t, "cel-override-value", got.Data["foo"]) + return nil + }) + + deleteComp := fw.DeleteStep(t, "deleteComposition", cli, comp) + + verifyCMDeleted := flow.Func("verifyConfigMapDeleted", func(ctx context.Context) error { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: "default", + }, + } + fw.WaitForResourceDeleted(t, ctx, cli, obj, 2*time.Minute) + return nil + }) + + cleanupSynth := fw.CleanupStep(t, "cleanupSynthesizer", cli, synth) + + w := new(flow.Workflow) + w.Add( + flow.Step(createComp).DependsOn(createSynth), + flow.Step(waitReady).DependsOn(createComp), + flow.Step(verifyOverrideApplied).DependsOn(waitReady), + flow.Step(deleteComp).DependsOn(verifyOverrideApplied), + flow.Step(verifyCMDeleted).DependsOn(deleteComp), + flow.Step(cleanupSynth).DependsOn(verifyCMDeleted), + ) + + require.NoError(t, w.Do(ctx)) +} + +func TestOverrides_CELValueExpression_NullNoOp_EndToEnd(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) + defer cancel() + + cli := fw.NewClient(t) + + synthName := fw.UniqueName("override-null-synth") + compName := fw.UniqueName("override-null-comp") + cmName := fw.UniqueName("override-null-cm") + + // valueExpression evaluates to null -> override is skipped (no-op) + overrideJSON := `[{ + "path": "self.data.foo", + "valueExpression": "null" + }]` + + cm := &corev1.ConfigMap{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "ConfigMap", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: "default", + Annotations: map[string]string{ + "eno.azure.io/overrides": overrideJSON, + }, + }, + Data: map[string]string{ + "foo": "should-remain", + "bar": "keep-me", + }, + } + + synth := fw.NewMinimalSynthesizer( + synthName, + fw.WithCommand(fw.ToCommand(cm)), + ) + + comp := fw.NewComposition( + compName, + "default", + fw.WithSynthesizerRefs(apiv1.SynthesizerRef{Name: synthName}), + ) + + compKey := types.NamespacedName{ + Name: compName, + Namespace: "default", + } + + createSynth := fw.CreateStep(t, "createSynthesizer", cli, synth) + createComp := fw.CreateStep(t, "createComposition", cli, comp) + + waitReady := flow.Func("waitReady", func(ctx context.Context) error { + fw.WaitForCompositionReady(t, ctx, cli, compKey, 3*time.Minute) + return nil + }) + + verifyNullNoOp := flow.Func("verifyNullNoOp", func(ctx context.Context) error { + got := &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: cmName, Namespace: "default"}} + fw.WaitForResourceExists(t, ctx, cli, got, 60*time.Second) + assert.Equal(t, "should-remain", got.Data["foo"], "expected null valueExpression to be treated as no-op") + assert.Equal(t, "keep-me", got.Data["bar"], "expected 'bar' to be untouched") + return nil + }) + + deleteComp := fw.DeleteStep(t, "deleteComposition", cli, comp) + + verifyCMDeleted := flow.Func("verifyConfigMapDeleted", func(ctx context.Context) error { + obj := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: cmName, + Namespace: "default", + }, + } + fw.WaitForResourceDeleted(t, ctx, cli, obj, 2*time.Minute) + return nil + }) + + cleanupSynth := fw.CleanupStep(t, "cleanupSynthesizer", cli, synth) + + w := new(flow.Workflow) + w.Add( + flow.Step(createComp).DependsOn(createSynth), + flow.Step(waitReady).DependsOn(createComp), + flow.Step(verifyNullNoOp).DependsOn(waitReady), + flow.Step(deleteComp).DependsOn(verifyNullNoOp), + flow.Step(verifyCMDeleted).DependsOn(deleteComp), + flow.Step(cleanupSynth).DependsOn(verifyCMDeleted), + ) + + require.NoError(t, w.Do(ctx)) +} diff --git a/internal/cel/cel.go b/internal/cel/cel.go index 6bd04552..49efb5d3 100644 --- a/internal/cel/cel.go +++ b/internal/cel/cel.go @@ -54,6 +54,9 @@ func Parse(expr string) (cel.Program, error) { func Eval(ctx context.Context, prgm cel.Program, comp *apiv1.Composition, self *unstructured.Unstructured, fm FieldMetadata) (ref.Val, error) { args := map[string]any{ "composition": func() any { return newCompositionMap(comp) }, // cel will only execute this if the composition is referenced in the expression + // Always provide `self` so CEL expressions can safely use guards like + // has(self.metadata.annotations) even when the resource does not exist yet. + "self": map[string]any{}, } if self != nil { args["self"] = self.Object diff --git a/internal/controllers/reconciliation/overrides_test.go b/internal/controllers/reconciliation/overrides_test.go index 4bb27ce3..7d8f604c 100644 --- a/internal/controllers/reconciliation/overrides_test.go +++ b/internal/controllers/reconciliation/overrides_test.go @@ -1074,3 +1074,367 @@ func TestMigratingFieldManagersFieldRemoval(t *testing.T) { assert.True(t, hasEno, "eno should own the fields") assert.False(t, hasLegacy, "legacy-tool should no longer own any fields") } + +// TestOverrideValueExpression proves that valueExpression can read a field from the current +// (live) resource and use it as the override value, gated by an annotation-based condition. +func TestOverrideValueExpression(t *testing.T) { + ctx := testutil.NewContext(t) + mgr := testutil.NewManager(t) + upstream := mgr.GetClient() + + registerControllers(t, mgr) + testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) { + output := &krmv1.ResourceList{} + output.Items = []*unstructured.Unstructured{{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-obj", + "namespace": "default", + "annotations": map[string]any{ + "eno.azure.io/reconcile-interval": "10ms", + "eno.azure.io/overrides": `[{ + "path": "self.data.foo", + "valueExpression": "self.data.foo", + "condition": "'allow-override-foo' in self.metadata.annotations && self.metadata.annotations['allow-override-foo'] == 'true'" + }]`, + }, + }, + "data": map[string]any{"foo": "default-value"}, + }, + }} + return output, nil + }) + + setupTestSubject(t, mgr) + mgr.Start(t) + _, comp := writeGenericComposition(t, upstream) + + testutil.Eventually(t, func() bool { + return upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Ready != nil + }) + + cm := &corev1.ConfigMap{} + cm.Name = "test-obj" + cm.Namespace = "default" + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["foo"] == "default-value" + }) + + err := retry.RetryOnConflict(testutil.Backoff, func() error { + err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + if err != nil { + return err + } + if cm.Annotations == nil { + cm.Annotations = map[string]string{} + } + cm.Annotations["allow-override-foo"] = "true" + cm.Data["foo"] = "customer-value" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + // The customer's value should be preserved because: + // 1. condition evaluates to true (annotation is set) + // 2. valueExpression reads "self.data.foo" from current (live) resource = "customer-value" + // 3. That value is applied to the mutated resource, so reconciliation won't overwrite it + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["foo"] == "customer-value" + }) + + err = retry.RetryOnConflict(testutil.Backoff, func() error { + err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + if err != nil { + return err + } + delete(cm.Annotations, "allow-override-foo") + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["foo"] == "default-value" + }) +} + +// TestOverrideVPAMinMax covers the allow-override-min annotation with 4 cases: +// - Annotation OFF, no changes: defaults preserved +// - Annotation OFF, customer changes values: Eno reverts to defaults +// - Annotation ON, no changes: defaults preserved (valueExpression reads same defaults) +// - Annotation ON, customer changes values: customer values preserved +func TestOverrideVPAMinMax(t *testing.T) { + ctx := testutil.NewContext(t) + mgr := testutil.NewManager(t) + upstream := mgr.GetClient() + + registerControllers(t, mgr) + testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) { + output := &krmv1.ResourceList{} + output.Items = []*unstructured.Unstructured{{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "vpa-min-max", + "namespace": "default", + "annotations": map[string]any{ + "eno.azure.io/reconcile-interval": "10ms", + "eno.azure.io/overrides": `[ + {"path": "self.data['min-cpu']", "valueExpression": "self.data['min-cpu']", "condition": "has(self.metadata.annotations) && 'allow-override-min' in self.metadata.annotations && self.metadata.annotations['allow-override-min'] == 'true'"}, + {"path": "self.data['min-memory']", "valueExpression": "self.data['min-memory']", "condition": "has(self.metadata.annotations) && 'allow-override-min' in self.metadata.annotations && self.metadata.annotations['allow-override-min'] == 'true'"}, + {"path": "self.data['max-cpu']", "valueExpression": "self.data['max-cpu']", "condition": "has(self.metadata.annotations) && 'allow-override-max' in self.metadata.annotations && self.metadata.annotations['allow-override-max'] == 'true'"}, + {"path": "self.data['max-memory']", "valueExpression": "self.data['max-memory']", "condition": "has(self.metadata.annotations) && 'allow-override-max' in self.metadata.annotations && self.metadata.annotations['allow-override-max'] == 'true'"} + ]`, + }, + }, + "data": map[string]any{ + "min-cpu": "100m", + "min-memory": "128Mi", + "max-cpu": "2", + "max-memory": "4Gi", + }, + }, + }} + return output, nil + }) + + setupTestSubject(t, mgr) + mgr.Start(t) + _, comp := writeGenericComposition(t, upstream) + + testutil.Eventually(t, func() bool { + return upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Ready != nil + }) + + cm := &corev1.ConfigMap{} + cm.Name = "vpa-min-max" + cm.Namespace = "default" + + // === Case 1: Annotation OFF, no changes — defaults preserved === + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "100m" && cm.Data["min-memory"] == "128Mi" && + cm.Data["max-cpu"] == "2" && cm.Data["max-memory"] == "4Gi" + }) + + // === Case 2: Annotation OFF, customer changes values — Eno reverts === + err := retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + cm.Data["min-cpu"] = "500m" + cm.Data["min-memory"] = "1Gi" + cm.Data["max-cpu"] = "16" + cm.Data["max-memory"] = "32Gi" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "100m" && cm.Data["min-memory"] == "128Mi" && + cm.Data["max-cpu"] == "2" && cm.Data["max-memory"] == "4Gi" + }) + + // === Case 3: Annotation ON (min only), no value changes — defaults preserved === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + if cm.Annotations == nil { + cm.Annotations = map[string]string{} + } + cm.Annotations["allow-override-min"] = "true" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + // valueExpression reads self.data['min-cpu'] from live = "100m" (same as default), so no visible change + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "100m" && cm.Data["min-memory"] == "128Mi" && + cm.Data["max-cpu"] == "2" && cm.Data["max-memory"] == "4Gi" + }) + + // === Case 4: Annotation ON (min), customer changes min values — preserved === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + cm.Annotations["allow-override-min"] = "true" + cm.Data["min-cpu"] = "500m" + cm.Data["min-memory"] = "1Gi" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "500m" && cm.Data["min-memory"] == "1Gi" && + cm.Data["max-cpu"] == "2" && cm.Data["max-memory"] == "4Gi" + }) + + // === Case 4b: Annotation ON (max), customer changes max values — preserved === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + cm.Annotations["allow-override-max"] = "true" + cm.Data["max-cpu"] = "16" + cm.Data["max-memory"] = "32Gi" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "500m" && cm.Data["min-memory"] == "1Gi" && + cm.Data["max-cpu"] == "16" && cm.Data["max-memory"] == "32Gi" + }) + + // === Cleanup: Remove both annotations — Eno reverts everything === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + delete(cm.Annotations, "allow-override-min") + delete(cm.Annotations, "allow-override-max") + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["min-cpu"] == "100m" && cm.Data["min-memory"] == "128Mi" && + cm.Data["max-cpu"] == "2" && cm.Data["max-memory"] == "4Gi" + }) +} + +// TestOverrideVPAUpdateMode covers the allow-override-update-mode annotation with 4 cases: +// - Annotation OFF, no changes: default "Auto" preserved +// - Annotation OFF, customer changes value: Eno reverts to "Auto" +// - Annotation ON, no changes: default "Auto" preserved +// - Annotation ON, customer changes value: customer value preserved +func TestOverrideVPAUpdateMode(t *testing.T) { + ctx := testutil.NewContext(t) + mgr := testutil.NewManager(t) + upstream := mgr.GetClient() + + registerControllers(t, mgr) + testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) { + output := &krmv1.ResourceList{} + output.Items = []*unstructured.Unstructured{{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "vpa-update-mode", + "namespace": "default", + "annotations": map[string]any{ + "eno.azure.io/reconcile-interval": "10ms", + "eno.azure.io/overrides": `[ + {"path": "self.data['update-mode']", "valueExpression": "self.data['update-mode']", "condition": "has(self.metadata.annotations) && 'allow-override-update-mode' in self.metadata.annotations && self.metadata.annotations['allow-override-update-mode'] == 'true'"} + ]`, + }, + }, + "data": map[string]any{ + "update-mode": "Auto", + }, + }, + }} + return output, nil + }) + + setupTestSubject(t, mgr) + mgr.Start(t) + _, comp := writeGenericComposition(t, upstream) + + testutil.Eventually(t, func() bool { + return upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Ready != nil + }) + + cm := &corev1.ConfigMap{} + cm.Name = "vpa-update-mode" + cm.Namespace = "default" + + // === Case 1: Annotation OFF, no changes — default "Auto" === + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["update-mode"] == "Auto" + }) + + // === Case 2: Annotation OFF, customer changes to "Off" — Eno reverts === + err := retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + cm.Data["update-mode"] = "Off" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["update-mode"] == "Auto" + }) + + // === Case 3: Annotation ON, no value changes — default preserved === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + if cm.Annotations == nil { + cm.Annotations = map[string]string{} + } + cm.Annotations["allow-override-update-mode"] = "true" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["update-mode"] == "Auto" + }) + + // === Case 4: Annotation ON, customer changes to "Off" — preserved === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + cm.Annotations["allow-override-update-mode"] = "true" + cm.Data["update-mode"] = "Off" + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + time.Sleep(100 * time.Millisecond) + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["update-mode"] == "Off" + }) + + // === Cleanup: Remove annotation — Eno reverts === + err = retry.RetryOnConflict(testutil.Backoff, func() error { + if err := mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm); err != nil { + return err + } + delete(cm.Annotations, "allow-override-update-mode") + return mgr.DownstreamClient.Update(ctx, cm) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + mgr.DownstreamClient.Get(ctx, client.ObjectKeyFromObject(cm), cm) + return cm.Data["update-mode"] == "Auto" + }) +} diff --git a/internal/resource/mutation/mutation.go b/internal/resource/mutation/mutation.go index cbfebe49..2d12734a 100644 --- a/internal/resource/mutation/mutation.go +++ b/internal/resource/mutation/mutation.go @@ -10,6 +10,7 @@ import ( apiv1 "github.com/Azure/eno/api/v1" "github.com/go-logr/logr" "github.com/google/cel-go/cel" + "google.golang.org/protobuf/types/known/structpb" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" enocel "github.com/Azure/eno/internal/cel" @@ -24,26 +25,29 @@ var ( type Status string const ( - StatusActive Status = "Active" - StatusInactive Status = "Inactive" - StatusInvalidCondition Status = "InvalidCondition" - StatusMissingParent Status = "MissingParent" - StatusIndexOutOfRange Status = "IndexOutOfRange" - StatusPathTypeMismatch Status = "PathTypeMismatch" + StatusActive Status = "Active" + StatusInactive Status = "Inactive" + StatusInvalidCondition Status = "InvalidCondition" + StatusMissingParent Status = "MissingParent" + StatusIndexOutOfRange Status = "IndexOutOfRange" + StatusPathTypeMismatch Status = "PathTypeMismatch" + StatusInvalidValueExpression Status = "InvalidValueExpression" ) // Op is an operation that conditionally assigns a value to a path within an object. // Designed to be sent over the wire as JSON. type Op struct { - Path *PathExpr - Condition cel.Program - Value any + Path *PathExpr + Condition cel.Program + Value any + ValueExpression cel.Program } type jsonOp struct { - Path string `json:"path"` - Condition string `json:"condition"` - Value any `json:"value"` + Path string `json:"path"` + Condition string `json:"condition"` + Value any `json:"value"` + ValueExpression string `json:"valueExpression"` } func (o *Op) UnmarshalJSON(data []byte) error { @@ -52,6 +56,11 @@ func (o *Op) UnmarshalJSON(data []byte) error { if err != nil { return err } + + if j.Value != nil && j.ValueExpression != "" { + return fmt.Errorf("value and valueExpression are mutually exclusive for path %q", j.Path) + } + o.Value = j.Value o.Path, err = ParsePathExpr(j.Path) @@ -66,6 +75,13 @@ func (o *Op) UnmarshalJSON(data []byte) error { } } + if j.ValueExpression != "" { + o.ValueExpression, err = enocel.Parse(j.ValueExpression) + if err != nil { + return fmt.Errorf("parsing valueExpression: %w", err) + } + } + return nil } @@ -76,11 +92,13 @@ func (o *Op) Apply(ctx context.Context, comp *apiv1.Composition, current, mutate if o.Condition != nil { val, err := enocel.Eval(ctx, o.Condition, comp, current, o.Path) - if err != nil && current == nil { - if !strings.HasPrefix(err.Error(), "no such ") { // e.g. "no such property" or "no such key" - logger.Info("override condition is invalid", "error", err, "path", o.Path.String()) - } else { - logger.Info("condition evaluation failed on missing resource", "error", err, "path", o.Path.String()) + if err != nil { + if current == nil { + if strings.HasPrefix(err.Error(), "no such ") { + logger.Info("condition evaluation failed on missing resource", "error", err, "path", o.Path.String()) + } else { + logger.Info("override condition is invalid", "error", err, "path", o.Path.String()) + } } return StatusInvalidCondition, nil } @@ -89,11 +107,37 @@ func (o *Op) Apply(ctx context.Context, comp *apiv1.Composition, current, mutate return StatusInactive, nil // condition not met } } - logger.Info("applying mutation to path", "path", o.Path.String(), "valueType", fmt.Sprintf("%T", o.Value)) - status, err := o.Path.Apply(mutated.Object, o.Value) + logger.Info("applying mutation to path", "path", o.Path.String(), "valueType", fmt.Sprintf("%T", o.Value), "hasValueExpression", o.ValueExpression != nil) + resolvedValue := o.Value + if o.ValueExpression != nil { + if current == nil { + logger.Info("skipping CEL value evaluation - current resource is nil", "path", o.Path.String()) + return StatusInactive, nil + } + val, err := enocel.Eval(ctx, o.ValueExpression, comp, current, o.Path) + if err != nil { + // Fail open: keep the synthesized/default value when user-provided CEL is invalid. + logger.Error(err, "failed to evaluate value expression - skipping override and keeping synthesized value", "path", o.Path.String()) + return StatusInvalidValueExpression, nil + } + resolvedValue = val.Value() + + if resolvedValue == structpb.NullValue_NULL_VALUE { + resolvedValue = nil + } + + if resolvedValue == nil { + // Treat null from valueExpression as "no override" (fail-open), not a delete. + logger.Info("CEL value expression evaluated to null, skipping mutation", "path", o.Path.String()) + return StatusInactive, nil + } + } + status, err := o.Path.Apply(mutated.Object, resolvedValue) + if err != nil { - logger.Error(err, "failed to apply mutation", "path", o.Path.String(), "status", status) - return status, err + // Fail open: invalid mutation paths should not block reconciliation. + logger.Error(err, "failed to apply mutation - skipping override and keeping synthesized value", "path", o.Path.String(), "status", status) + return status, nil } logger.Info("successfully applied mutation", "path", o.Path.String(), "status", status) return status, nil diff --git a/internal/resource/mutation/mutation_test.go b/internal/resource/mutation/mutation_test.go index 6cac7f8d..852de247 100644 --- a/internal/resource/mutation/mutation_test.go +++ b/internal/resource/mutation/mutation_test.go @@ -69,7 +69,7 @@ func TestApply(t *testing.T) { path: "self.foo.bar", obj: map[string]any{"another": "baz"}, value: 123, - expected: map[string]any{"another": "baz"}, + expected: map[string]any{"another": "baz", "foo": map[string]any{"bar": 123}}, status: StatusActive, }, { @@ -93,7 +93,7 @@ func TestApply(t *testing.T) { path: `self.foo["bar"]`, obj: map[string]any{"another": "baz"}, value: 123, - expected: map[string]any{"another": "baz"}, + expected: map[string]any{"another": "baz", "foo": map[string]any{"bar": 123}}, status: StatusActive, }, { @@ -321,6 +321,30 @@ func TestApply(t *testing.T) { }, }, }, + { + name: "Map_CreateMissingIntermediateMap", + path: "self.spec.minAllowed.memory", + obj: map[string]any{"spec": map[string]any{}}, + value: "20Mi", + expected: map[string]any{"spec": map[string]any{"minAllowed": map[string]any{"memory": "20Mi"}}}, + status: StatusActive, + }, + { + name: "Map_CreateMissingIntermediateMap_NilValue", + path: "self.spec.minAllowed.memory", + obj: map[string]any{"spec": map[string]any{}}, + value: nil, + expected: map[string]any{"spec": map[string]any{}}, + status: StatusActive, + }, + { + name: "Map_CreateDeeplyNestedMissingMaps", + path: "self.a.b.c.d", + obj: map[string]any{}, + value: "val", + expected: map[string]any{"a": map[string]any{"b": map[string]any{"c": map[string]any{"d": "val"}}}}, + status: StatusActive, + }, { name: "Root", path: "self", @@ -446,6 +470,240 @@ func TestOpApply(t *testing.T) { "foo": "bar", }}, }, + { + name: "CELValueExpression_ResolvesFromCurrent", + op: Op{ + Path: mustParsePathExpr("self.foo"), + ValueExpression: mustParseCEL("self.bar"), + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "bar": "resolved-value", + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "foo": "resolved-value", + }}, + }, + { + name: "CELValueExpression_NilCurrent_Skipped", + op: Op{ + Path: mustParsePathExpr("self.foo"), + ValueExpression: mustParseCEL("self.bar"), + }, + current: nil, + mutated: &unstructured.Unstructured{Object: map[string]any{}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{}}, + }, + { + name: "CELValueExpression_WithCondition", + op: Op{ + Path: mustParsePathExpr("self.foo"), + Condition: mustParseCEL("has(self.bar)"), + ValueExpression: mustParseCEL("self.bar"), + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "bar": "dynamic-val", + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "foo": "dynamic-val", + }}, + }, + { + name: "CELValueExpression_NullResult_SkipsMutation", + op: Op{ + Path: mustParsePathExpr("self.foo"), + ValueExpression: mustParseCEL("null"), + }, + current: &unstructured.Unstructured{Object: map[string]any{"bar": "val"}}, + mutated: &unstructured.Unstructured{Object: map[string]any{"foo": "old-value"}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{"foo": "old-value"}}, + }, + { + name: "CELValueExpression_NullResult_NoFieldToDelete", + op: Op{ + Path: mustParsePathExpr("self.foo"), + ValueExpression: mustParseCEL("null"), + }, + current: &unstructured.Unstructured{Object: map[string]any{"bar": "val"}}, + mutated: &unstructured.Unstructured{Object: map[string]any{}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{}}, + }, + { + name: "UpdateMode_NullValueExpression_KeepsSynthesizedDefault", + op: Op{ + Path: mustParsePathExpr("self.spec.updatePolicy.updateMode"), + ValueExpression: mustParseCEL("null"), + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{"updatePolicy": map[string]any{}}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{"updatePolicy": map[string]any{"updateMode": "Recreate"}}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{"updatePolicy": map[string]any{"updateMode": "Recreate"}}, + }}, + }, + { + name: "StaticNilValue_DeletesField", + op: Op{ + Path: mustParsePathExpr("self.foo"), + Value: nil, + }, + current: &unstructured.Unstructured{Object: map[string]any{"bar": "val"}}, + mutated: &unstructured.Unstructured{Object: map[string]any{"foo": "old-value"}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{}}, + }, + { + name: "ConditionMet_AnnotationExists_MutationApplied", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{ + "override-min-max": "enabled", + }, + }, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{"minAllowed": map[string]any{"cpu": "100m"}}, + }}, + }, + { + name: "ConditionNotMet_AnnotationDisabled_NoMutation", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{ + "override-min-max": "disabled", + }, + }, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + }, + { + name: "Bug1_AnnotationKeyMissing_CurrentExists_SilentlyInactive", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{}, + }, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + }, + { + name: "Bug1_NoAnnotationsMap_CurrentExists_SilentlyInactive", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{}, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + }, + { + name: "Bug1_NilCurrent_ConditionRefsSelf_InvalidCondition", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: nil, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + }, + { + name: "HasGuard_AnnotationMissing_ConditionFalse_NoMutation", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`has(self.metadata.annotations) && 'override-min-max' in self.metadata.annotations && self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{}, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + }, + { + name: "HasGuard_AnnotationPresent_ConditionTrue_MutationApplied", + op: Op{ + Path: mustParsePathExpr("self.spec.minAllowed.cpu"), + Condition: mustParseCEL(`has(self.metadata.annotations) && 'override-min-max' in self.metadata.annotations && self.metadata.annotations['override-min-max'] == 'enabled'`), + Value: "100m", + }, + current: &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{ + "override-min-max": "enabled", + }, + }, + "spec": map[string]any{}, + }}, + mutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{}, + }}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "spec": map[string]any{"minAllowed": map[string]any{"cpu": "100m"}}, + }}, + }, + { + name: "StaticValue_StillWorks", + op: Op{ + Path: mustParsePathExpr("self.foo"), + Value: "static-val", + }, + current: &unstructured.Unstructured{Object: map[string]any{}}, + mutated: &unstructured.Unstructured{Object: map[string]any{}}, + expectedMutated: &unstructured.Unstructured{Object: map[string]any{ + "foo": "static-val", + }}, + }, } for _, tc := range testCases { @@ -465,6 +723,46 @@ func TestOpApply(t *testing.T) { } } +func TestOpApplyValueExpressionErrorFailsOpen(t *testing.T) { + ctx := context.Background() + + op := Op{ + Path: mustParsePathExpr("self.foo"), + ValueExpression: mustParseCEL("self.bar.baz"), + } + + current := &unstructured.Unstructured{Object: map[string]any{ + "bar": "not-a-map", + }} + mutated := &unstructured.Unstructured{Object: map[string]any{ + "foo": "synthesized-default", + }} + + status, err := op.Apply(ctx, &apiv1.Composition{}, current, mutated) + require.NoError(t, err) + assert.Equal(t, StatusInvalidValueExpression, status) + assert.Equal(t, map[string]any{"foo": "synthesized-default"}, mutated.Object) +} + +func TestOpApplyPathTypeMismatchFailsOpen(t *testing.T) { + ctx := context.Background() + + op := Op{ + Path: mustParsePathExpr("self.foo[*]"), + Value: "new-value", + } + + current := &unstructured.Unstructured{Object: map[string]any{}} + mutated := &unstructured.Unstructured{Object: map[string]any{ + "foo": "scalar", + }} + + status, err := op.Apply(ctx, &apiv1.Composition{}, current, mutated) + require.NoError(t, err) + assert.Equal(t, StatusPathTypeMismatch, status) + assert.Equal(t, map[string]any{"foo": "scalar"}, mutated.Object) +} + // TestInvalidPathInJson proves that the non-nil ops with nil paths left after failed unmarshalling will not panic when applied. func TestInvalidPathInJson(t *testing.T) { overridesJson := "[\n { \"path\": \"self.spec.template.spec.containers[name='operator'].resources.requests.cpu\", \"value\": \"250m\", \"condition\": \"self.spec.template.spec.containers[name='operator'].resources.requests.cpu == '250m'\" } ]" @@ -478,6 +776,127 @@ func TestInvalidPathInJson(t *testing.T) { } } +func TestVPAOverrideMatrix(t *testing.T) { + ctx := context.Background() + + const ( + defaultMin = "10Mi" + defaultMax = "30Mi" + defaultMode = "Recreate" + ) + + ops := []Op{ + { + Path: mustParsePathExpr(`self.spec.resourcePolicy.containerPolicies[containerName="cost-analysis-agent"].minAllowed.memory`), + Condition: mustParseCEL(`self.metadata.annotations['autoscaler.addons.k8s.io/override-min-max'] == 'enabled'`), + ValueExpression: mustParseCEL(`self.spec.resourcePolicy.containerPolicies[0].minAllowed.memory`), + }, + { + Path: mustParsePathExpr(`self.spec.resourcePolicy.containerPolicies[containerName="cost-analysis-agent"].maxAllowed.memory`), + Condition: mustParseCEL(`self.metadata.annotations['autoscaler.addons.k8s.io/override-min-max'] == 'enabled'`), + ValueExpression: mustParseCEL(`self.spec.resourcePolicy.containerPolicies[0].maxAllowed.memory`), + }, + { + Path: mustParsePathExpr("self.spec.updatePolicy.updateMode"), + Condition: mustParseCEL(`self.metadata.annotations['autoscaler.addons.k8s.io/override-update-mode'] == 'enabled'`), + ValueExpression: mustParseCEL("self.spec.updatePolicy.updateMode"), + }, + } + + testCases := []struct { + name string + minMaxOverride string + updateModeOverride string + currentMin string + currentMax string + currentUpdateMode string + expectedMin string + expectedMax string + expectedUpdateMode string + }{ + {name: "Case01_EnabledEnabled", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "99Mi", currentMax: "99Mi", currentUpdateMode: "Off", expectedMin: "99Mi", expectedMax: "99Mi", expectedUpdateMode: "Off"}, + {name: "Case02_DisabledDisabled", minMaxOverride: "disabled", updateModeOverride: "disabled", currentMin: "99Mi", currentMax: "99Mi", currentUpdateMode: "Off", expectedMin: defaultMin, expectedMax: defaultMax, expectedUpdateMode: defaultMode}, + {name: "Case03_EnabledDisabled", minMaxOverride: "enabled", updateModeOverride: "disabled", currentMin: "99Mi", currentMax: "99Mi", currentUpdateMode: "Off", expectedMin: "99Mi", expectedMax: "99Mi", expectedUpdateMode: defaultMode}, + {name: "Case04_DisabledEnabled", minMaxOverride: "disabled", updateModeOverride: "enabled", currentMin: "99Mi", currentMax: "99Mi", currentUpdateMode: "Off", expectedMin: defaultMin, expectedMax: defaultMax, expectedUpdateMode: "Off"}, + {name: "Case05_EnabledEnabled_ChangeMin", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "50Mi", currentMax: "30Mi", currentUpdateMode: "Recreate", expectedMin: "50Mi", expectedMax: "30Mi", expectedUpdateMode: "Recreate"}, + {name: "Case06_EnabledEnabled_ChangeMax", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "10Mi", currentMax: "100Mi", currentUpdateMode: "Recreate", expectedMin: "10Mi", expectedMax: "100Mi", expectedUpdateMode: "Recreate"}, + {name: "Case07_EnabledEnabled_ChangeMode", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "10Mi", currentMax: "30Mi", currentUpdateMode: "Auto", expectedMin: "10Mi", expectedMax: "30Mi", expectedUpdateMode: "Auto"}, + {name: "Case08_EnabledEnabled_ChangeMinMax", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "20Mi", currentMax: "60Mi", currentUpdateMode: "Recreate", expectedMin: "20Mi", expectedMax: "60Mi", expectedUpdateMode: "Recreate"}, + {name: "Case09_EnabledEnabled_ChangeMinMode", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "25Mi", currentMax: "30Mi", currentUpdateMode: "Initial", expectedMin: "25Mi", expectedMax: "30Mi", expectedUpdateMode: "Initial"}, + {name: "Case10_EnabledEnabled_ChangeAll", minMaxOverride: "enabled", updateModeOverride: "enabled", currentMin: "15Mi", currentMax: "50Mi", currentUpdateMode: "Auto", expectedMin: "15Mi", expectedMax: "50Mi", expectedUpdateMode: "Auto"}, + {name: "Case11_DisabledDisabled_ChangeAll", minMaxOverride: "disabled", updateModeOverride: "disabled", currentMin: "15Mi", currentMax: "50Mi", currentUpdateMode: "Auto", expectedMin: defaultMin, expectedMax: defaultMax, expectedUpdateMode: defaultMode}, + {name: "Case12_EnabledDisabled_ChangeAll", minMaxOverride: "enabled", updateModeOverride: "disabled", currentMin: "15Mi", currentMax: "50Mi", currentUpdateMode: "Auto", expectedMin: "15Mi", expectedMax: "50Mi", expectedUpdateMode: defaultMode}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + current := buildCostAnalysisVPA(tc.minMaxOverride, tc.updateModeOverride, tc.currentMin, tc.currentMax, tc.currentUpdateMode) + mutated := buildCostAnalysisVPA(tc.minMaxOverride, tc.updateModeOverride, defaultMin, defaultMax, defaultMode) + + for i := range ops { + _, err := ops[i].Apply(ctx, &apiv1.Composition{}, current, mutated) + require.NoError(t, err) + } + + gotMin, gotMax, gotMode := readCostAnalysisVPA(mutated) + assert.Equal(t, tc.expectedMin, gotMin) + assert.Equal(t, tc.expectedMax, gotMax) + assert.Equal(t, tc.expectedUpdateMode, gotMode) + }) + } +} + +func buildCostAnalysisVPA(minMaxOverride, updateModeOverride, min, max, updateMode string) *unstructured.Unstructured { + return &unstructured.Unstructured{Object: map[string]any{ + "metadata": map[string]any{ + "annotations": map[string]any{ + "autoscaler.addons.k8s.io/override-min-max": minMaxOverride, + "autoscaler.addons.k8s.io/override-update-mode": updateModeOverride, + }, + }, + "spec": map[string]any{ + "updatePolicy": map[string]any{ + "updateMode": updateMode, + }, + "resourcePolicy": map[string]any{ + "containerPolicies": []any{ + map[string]any{ + "containerName": "cost-analysis-agent", + "minAllowed": map[string]any{"memory": min}, + "maxAllowed": map[string]any{"memory": max}, + }, + }, + }, + }, + }} +} + +func readCostAnalysisVPA(obj *unstructured.Unstructured) (min, max, mode string) { + mode, _, _ = unstructured.NestedString(obj.Object, "spec", "updatePolicy", "updateMode") + policies, _, _ := unstructured.NestedSlice(obj.Object, "spec", "resourcePolicy", "containerPolicies") + for _, p := range policies { + m, ok := p.(map[string]any) + if !ok { + continue + } + if m["containerName"] != "cost-analysis-agent" { + continue + } + if minAllowed, ok := m["minAllowed"].(map[string]any); ok { + if v, ok := minAllowed["memory"].(string); ok { + min = v + } + } + if maxAllowed, ok := m["maxAllowed"].(map[string]any); ok { + if v, ok := maxAllowed["memory"].(string); ok { + max = v + } + } + break + } + return min, max, mode +} + // helper functions for tests func mustParsePathExpr(path string) *PathExpr { expr, err := ParsePathExpr(path) @@ -494,3 +913,11 @@ func mustParseCondition(cond string) cel.Program { } return prog } + +func mustParseCEL(expr string) cel.Program { + prog, err := enocel.Parse(expr) + if err != nil { + panic(fmt.Sprintf("failed to parse CEL expression %q: %v", expr, err)) + } + return prog +} diff --git a/internal/resource/mutation/parser.go b/internal/resource/mutation/parser.go index e443a2c9..ff95d151 100644 --- a/internal/resource/mutation/parser.go +++ b/internal/resource/mutation/parser.go @@ -181,16 +181,32 @@ func (p *PathExpr) apply(path *PathExpr, startIndex int, obj any, value any) (St return StatusActive, nil } - child := m[keyStr] - if child != nil { - status, err := p.apply(path, startIndex+i+1, child, value) - if err != nil { - return status, err + child, exists := m[keyStr] + if child == nil { + if !exists && value != nil { + // Key is missing entirely — create intermediate map so overrides + // targeting nested paths (e.g. minAllowed.memory) work even when + // the intermediate key doesn't exist in the desired state. + child = map[string]any{} + m[keyStr] = child + } else { + // Key exists but is explicitly nil, or value is nil — skip + return StatusActive, nil } - if value == nil { - if nextMap, ok := child.(map[string]any); ok && len(nextMap) == 0 { - delete(m, keyStr) - } + } + childLen := 0 + if cm, ok := child.(map[string]any); ok { + childLen = len(cm) + } + status, err := p.apply(path, startIndex+i+1, child, value) + if err != nil { + return status, err + } + // Clean up intermediate maps that became empty as a result of deletion, + // but not maps that were already empty before the operation. + if value == nil { + if nextMap, ok := child.(map[string]any); ok && len(nextMap) == 0 && childLen > 0 { + delete(m, keyStr) } } return StatusActive, nil diff --git a/internal/resource/resource.go b/internal/resource/resource.go index 8c248525..75d996cb 100644 --- a/internal/resource/resource.go +++ b/internal/resource/resource.go @@ -24,11 +24,6 @@ import ( "k8s.io/apimachinery/pkg/types" ) -const ( - ReservedReadinessGroupLowerBound = -100 - ReservedReadinessGroupUpperBound = -60 -) - var patchGVK = schema.GroupVersionKind{ Group: "eno.azure.io", Version: "v1", @@ -70,6 +65,7 @@ type Resource struct { readinessGroup int deletionGroup *int overrides []*mutation.Op + overrideParseError error latestKnownState atomic.Pointer[apiv1.ResourceState] } @@ -174,6 +170,7 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict } if err != nil { logger.Error(err, "invalid override json") + res.overrideParseError = err } } @@ -203,11 +200,6 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict if err != nil { logger.Info("invalid readiness group - ignoring") } else { - if rg >= ReservedReadinessGroupLowerBound && rg <= ReservedReadinessGroupUpperBound { - logger.Info(fmt.Sprintf("WARNING: user-supplied readiness-group is in Eno reserved range [%d, %d]", - ReservedReadinessGroupLowerBound, ReservedReadinessGroupUpperBound), - "kind", res.GVK.Kind, "readinessGroup", rg) - } res.readinessGroup = rg } } @@ -223,10 +215,6 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict if err != nil { logger.Info("invalid deletion group - ignoring") } else { - if rg >= -ReservedReadinessGroupUpperBound && rg <= -ReservedReadinessGroupLowerBound { - logger.Info(fmt.Sprintf("WARNING: user-supplied deletion-group is in Eno reserved range [%d, %d]", -ReservedReadinessGroupUpperBound, -ReservedReadinessGroupLowerBound), - "kind", res.GVK.Kind, "deletionGroup", rg) - } res.deletionGroup = &rg } } @@ -255,28 +243,6 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict res.ReadinessChecks = append(res.ReadinessChecks, check) } sort.Slice(res.ReadinessChecks, func(i, j int) bool { return res.ReadinessChecks[i].Name < res.ReadinessChecks[j].Name }) - // This indicates that this is an infrastructure Kind - if defaultGrp, ok := managedCreateOrder[res.GVK.Kind]; ok { - if _, ok := anno[readinessGroupKey]; !ok { - logger.Info("assigning default readiness group to managed kind", - "kind", res.GVK.Kind, "defaultGroup", defaultGrp) - res.readinessGroup = defaultGrp - } else { - logger.Info("user-specified readiness group present, skipping default for managed kind", - "kind", res.GVK.Kind, "readinessGroup", res.readinessGroup) - } - - if _, ok := anno[deletionGroupKey]; !ok { - logger.Info("assigning default deletion group to managed kind", - "kind", res.GVK.Kind, "defaultGroup", defaultGrp) - delGroup := -defaultGrp - res.deletionGroup = &delGroup - } else { - logger.Info("user-specified deletion group present, skipping default for managed kind", - "kind", res.GVK.Kind, "deletionGroup", *res.deletionGroup) - } - } - logger.Info("resource created successfully") return res, nil } @@ -310,15 +276,20 @@ func (r *Resource) Snapshot(ctx context.Context, comp *apiv1.Composition, actual // SnapshotWithOverrides is identical to Snapshot but applies the overrides from another resource // (presumably a newer version of the same object). func (r *Resource) SnapshotWithOverrides(ctx context.Context, comp *apiv1.Composition, actual *unstructured.Unstructured, overrideRes *Resource) (*Snapshot, error) { + logger := logr.FromContextOrDiscard(ctx) copy := r.parsed.DeepCopy() - overrideStatus := make([]string, len(overrideRes.overrides)) + overrideStatus := make([]string, 0, len(overrideRes.overrides)+1) + if overrideRes.overrideParseError != nil { + logger.Info("override annotation parse error", "error", overrideRes.overrideParseError) + overrideStatus = append(overrideStatus, fmt.Sprintf("eno.azure.io/overrides=InvalidJSON(%v)", overrideRes.overrideParseError)) + } for i, op := range overrideRes.overrides { status, err := op.Apply(ctx, comp, actual, copy) if err != nil { return nil, fmt.Errorf("applying override %d: %w", i+1, err) } - overrideStatus[i] = fmt.Sprintf("%s=%s", op.Path, status) + overrideStatus = append(overrideStatus, fmt.Sprintf("%s=%s", op.Path, status)) } snap := &Snapshot{ diff --git a/internal/resource/resource_test.go b/internal/resource/resource_test.go index 35f7fe5a..bf72517b 100644 --- a/internal/resource/resource_test.go +++ b/internal/resource/resource_test.go @@ -30,8 +30,8 @@ var newResourceTests = []struct { { Name: "configmap", Manifest: `{ - "apiVersion": "example.io/v1", - "kind": "Widget", + "apiVersion": "v1", + "kind": "ConfigMap", "metadata": { "name": "foo", "annotations": { @@ -51,14 +51,14 @@ var newResourceTests = []struct { } }`, Assert: func(t *testing.T, r *Snapshot) { - assert.Equal(t, schema.GroupVersionKind{Group: "example.io", Version: "v1", Kind: "Widget"}, r.GVK) + assert.Equal(t, schema.GroupVersionKind{Version: "v1", Kind: "ConfigMap"}, r.GVK) assert.Len(t, r.ReadinessChecks, 2) assert.Equal(t, time.Second*10, r.ReconcileInterval.Duration) assert.Equal(t, Ref{ Name: "foo", Namespace: "", - Group: "example.io", - Kind: "Widget", + Group: "", + Kind: "ConfigMap", }, r.Ref) assert.True(t, r.Disable) assert.True(t, r.DisableUpdates) @@ -110,8 +110,8 @@ var newResourceTests = []struct { { Name: "zero-readiness-group", Manifest: `{ - "apiVersion": "example.io/v1", - "kind": "Widget", + "apiVersion": "v1", + "kind": "ConfigMap", "metadata": { "name": "foo", "annotations": { @@ -213,8 +213,8 @@ var newResourceTests = []struct { { Name: "negative-readiness-group", Manifest: `{ - "apiVersion": "example.io/v1", - "kind": "Widget", + "apiVersion": "v1", + "kind": "ConfigMap", "metadata": { "name": "foo", "annotations": { @@ -422,6 +422,7 @@ var newResourceTests = []struct { }`, Assert: func(t *testing.T, r *Snapshot) { assert.Len(t, r.overrides, 0) + assert.Contains(t, r.OverrideStatus(), "eno.azure.io/overrides=InvalidJSON") }, }, { diff --git a/pkg/function/overrides/override.go b/pkg/function/overrides/override.go index 99439f2b..203c1b64 100644 --- a/pkg/function/overrides/override.go +++ b/pkg/function/overrides/override.go @@ -16,9 +16,10 @@ import ( // trying to do type Override = intmut.Op will get you an erro about extending methods. // could make it a composition but not going down that path yet type Override struct { - Path string `json:"path"` - Value any `json:"value"` - Condition string `json:"condition"` + Path string `json:"path"` + Value any `json:"value"` + Condition string `json:"condition"` + ValueExpression string `json:"valueExpression"` } func (o *Override) validate() (cel.Program, error) { @@ -36,6 +37,11 @@ func (o *Override) validate() (cel.Program, error) { if o.Condition == "" { return nil, fmt.Errorf("condition is required") } + + if o.ValueExpression != "" && o.Value != nil { + return nil, fmt.Errorf("value and valueExpression are mutually exclusive for path %q", o.Path) + } + // Parse the expression celEnv := intcel.Env ast, issues := celEnv.Parse(o.Condition) @@ -55,7 +61,13 @@ func (o *Override) validate() (cel.Program, error) { return nil, fmt.Errorf("failed to create program: %w", err) } - //Value can be null which is abit wierd. + if o.ValueExpression != "" { + _, err = intcel.Parse(o.ValueExpression) + if err != nil { + return nil, fmt.Errorf("failed to parse valueExpression: %w", err) + } + } + return p, nil } @@ -88,7 +100,7 @@ func (o *Override) Test(data map[string]interface{}) (bool, error) { // String is for debugging only because escaped json cel is hard to read. func (o *Override) String() string { //not actual json becuse escaping is hard to read. - return fmt.Sprintf("{Path: %s,\n Value: %v,\n Condition: %s}", o.Path, o.Value, o.Condition) + return fmt.Sprintf("{Path: %s,\n Value: %v,\n Condition: %s,\n ValueExpression: %s}", o.Path, o.Value, o.Condition, o.ValueExpression) } // AnnotateOverrides will take care of appropriatly serializng your overrides to annotations diff --git a/pkg/function/overrides/override_test.go b/pkg/function/overrides/override_test.go index 70f258b8..5ad3d074 100644 --- a/pkg/function/overrides/override_test.go +++ b/pkg/function/overrides/override_test.go @@ -69,112 +69,158 @@ func TestOverrideValidate(t *testing.T) { } } -func TestAnnotateOverrides_Success(t *testing.T) { - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", +func TestValueExpression(t *testing.T) { + tests := []struct { + name string + o overrides.Override + wantErr bool + }{ + { + name: "ValidValueExpression", + o: overrides.Override{ + Path: "self.metadata.name", + Condition: "true", + ValueExpression: "self.metadata.name", + }, + wantErr: false, + }, + { + name: "InvalidValueExpression", + o: overrides.Override{ + Path: "self.metadata.name", + Condition: "true", + ValueExpression: "1 +", + }, + wantErr: true, + }, + { + name: "ValueExpressionWithoutValue", + o: overrides.Override{ + Path: "self.spec.foo", + Condition: "has(self.spec.foo)", + ValueExpression: "self.spec.foo", + }, + wantErr: false, }, - } - ov1 := overrides.Override{ - Path: "metadata.name", - Condition: "true", - } - ov2 := overrides.Override{ - Path: "metadata.namespace", - Condition: "false", - } - ovs := []overrides.Override{ov1, ov2} - err := overrides.AnnotateOverrides(obj, ovs) - if err != nil { - t.Fatalf("AnnotateOverrides() untriggered error: %v", err) - } - - anns := obj.GetAnnotations() - val, ok := anns["eno.azure.io/overrides"] - if !ok { - t.Fatalf("triggered annotation eno.azure.io/overrides to be set") } - var got []overrides.Override - if err := json.Unmarshal([]byte(val), &got); err != nil { - t.Fatalf("failed to unmarshal annotation value: %v", err) - } - if len(got) != 2 { - t.Fatalf("triggered 2 overrides, got %d", len(got)) - } - if got[0].Path != ov1.Path || got[0].Condition != ov1.Condition { - t.Errorf("untriggered first override marshaled, want %+v, got %+v", ov1, got[0]) - } - if got[1].Path != ov2.Path || got[1].Condition != ov2.Condition { - t.Errorf("untriggered second override marshaled, want %+v, got %+v", ov2, got[1]) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := tt.o.Test(map[string]any{ + "self": map[string]any{ + "metadata": map[string]any{"name": "test"}, + "spec": map[string]any{"foo": "bar"}, + }, + }) + if (err != nil) != tt.wantErr { + t.Errorf("Test() error = %v, wantErr %v", err, tt.wantErr) + } + }) } } - -func TestAnnotateOverrides_ExistingAnnotation(t *testing.T) { - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", +func TestAnnotateOverrides_Table(t *testing.T) { + tests := []struct { + name string + existingAnnotation string + overrides []overrides.Override + expectedCount int + expectedOverrides []overrides.Override + }{ + { + name: "ValueExpression", + overrides: []overrides.Override{{ + Path: "self.data.foo", + Condition: "has(self.data.foo)", + ValueExpression: "self.data.foo", + }}, + expectedCount: 1, + expectedOverrides: []overrides.Override{{ + Path: "self.data.foo", + Condition: "has(self.data.foo)", + ValueExpression: "self.data.foo", + }}, + }, + { + name: "MultipleOverrides", + overrides: []overrides.Override{ + {Path: "metadata.name", Condition: "true"}, + {Path: "metadata.namespace", Condition: "false"}, + }, + expectedCount: 2, + expectedOverrides: []overrides.Override{ + {Path: "metadata.name", Condition: "true"}, + {Path: "metadata.namespace", Condition: "false"}, + }, + }, + { + name: "ExistingAnnotation_MergesOverrides", + existingAnnotation: `[{"path":"metadata.name","condition":"true"}]`, + overrides: []overrides.Override{ + {Path: "metadata.namespace", Condition: "true"}, + }, + expectedCount: 2, + expectedOverrides: []overrides.Override{ + {Path: "metadata.name", Condition: "true"}, + {Path: "metadata.namespace", Condition: "true"}, + }, + }, + { + name: "InvalidOverride_StillSerializes", + overrides: []overrides.Override{ + {Path: "", Condition: "true"}, + }, + expectedCount: 1, + expectedOverrides: []overrides.Override{ + {Path: "", Condition: "true"}, + }, }, } - // Pre-set the annotation to simulate existing override - existingOverride := overrides.Override{ - Path: "metadata.name", - Condition: "true", - } - obj.SetAnnotations(map[string]string{ - "eno.azure.io/overrides": "[{\"path\":\"metadata.name\",\"condition\":\"true\"}]", - }) - // Add a new override - newOverride := overrides.Override{ - Path: "metadata.namespace", - Condition: "true", - } - err := overrides.AnnotateOverrides(obj, []overrides.Override{newOverride}) - if err != nil { - t.Fatalf("triggered to merge %s", err) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + obj := &unstructured.Unstructured{ + Object: map[string]interface{}{ + "apiVersion": "v1", + "kind": "ConfigMap", + }, + } + if tt.existingAnnotation != "" { + obj.SetAnnotations(map[string]string{ + "eno.azure.io/overrides": tt.existingAnnotation, + }) + } - anns := obj.GetAnnotations() - val, ok := anns["eno.azure.io/overrides"] - if !ok { - t.Fatalf("triggered annotation eno.azure.io/overrides to be set") - } + err := overrides.AnnotateOverrides(obj, tt.overrides) + if err != nil { + t.Fatalf("AnnotateOverrides() error: %v", err) + } - var got []overrides.Override - if err := json.Unmarshal([]byte(val), &got); err != nil { - t.Fatalf("failed to unmarshal annotation value: %v", err) - } - if len(got) != 2 { - t.Fatalf("triggered 2 overrides, got %d", len(got)) - } + anns := obj.GetAnnotations() + val, ok := anns["eno.azure.io/overrides"] + if !ok { + t.Fatalf("expected annotation to be set") + } - // Verify that existing override comes first, new override comes second - if got[0].Path != existingOverride.Path || got[0].Condition != existingOverride.Condition { - t.Errorf("expected first override to be existing override %+v, got %+v", existingOverride, got[0]) - } - if got[1].Path != newOverride.Path || got[1].Condition != newOverride.Condition { - t.Errorf("expected second override to be new override %+v, got %+v", newOverride, got[1]) - } -} + var got []overrides.Override + if err := json.Unmarshal([]byte(val), &got); err != nil { + t.Fatalf("failed to unmarshal: %v", err) + } + if len(got) != tt.expectedCount { + t.Fatalf("expected %d overrides, got %d", tt.expectedCount, len(got)) + } -func TestAnnotateOverrides_InvalidOverrideAllowed(t *testing.T) { - obj := &unstructured.Unstructured{ - Object: map[string]interface{}{ - "apiVersion": "v1", - "kind": "ConfigMap", - }, - } - // Invalid override: empty Path - ov := overrides.Override{ - Path: "", - Condition: "true", - } - err := overrides.AnnotateOverrides(obj, []overrides.Override{ov}) - if err != nil { - t.Fatal("AnnotateOverrides() should not validate just serialize") + for i, expected := range tt.expectedOverrides { + if got[i].Path != expected.Path { + t.Errorf("override[%d].Path = %q, want %q", i, got[i].Path, expected.Path) + } + if got[i].Condition != expected.Condition { + t.Errorf("override[%d].Condition = %q, want %q", i, got[i].Condition, expected.Condition) + } + if got[i].ValueExpression != expected.ValueExpression { + t.Errorf("override[%d].ValueExpression = %q, want %q", i, got[i].ValueExpression, expected.ValueExpression) + } + } + }) } } From 52f319cd31b4b4d3ca7c3d1393fec6ed8de225c8 Mon Sep 17 00:00:00 2001 From: Jinghan Fu Date: Fri, 8 May 2026 10:12:28 +1000 Subject: [PATCH 2/2] fix(resource): restore managed ordering defaults while preserving override parse status --- internal/resource/resource.go | 35 ++++++++++++++++++++++++++++++ internal/resource/resource_test.go | 4 +++- 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/internal/resource/resource.go b/internal/resource/resource.go index 75d996cb..abae1de6 100644 --- a/internal/resource/resource.go +++ b/internal/resource/resource.go @@ -24,6 +24,11 @@ import ( "k8s.io/apimachinery/pkg/types" ) +const ( + ReservedReadinessGroupLowerBound = -100 + ReservedReadinessGroupUpperBound = -60 +) + var patchGVK = schema.GroupVersionKind{ Group: "eno.azure.io", Version: "v1", @@ -200,6 +205,11 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict if err != nil { logger.Info("invalid readiness group - ignoring") } else { + if rg >= ReservedReadinessGroupLowerBound && rg <= ReservedReadinessGroupUpperBound { + logger.Info(fmt.Sprintf("WARNING: user-supplied readiness-group is in Eno reserved range [%d, %d]", + ReservedReadinessGroupLowerBound, ReservedReadinessGroupUpperBound), + "kind", res.GVK.Kind, "readinessGroup", rg) + } res.readinessGroup = rg } } @@ -215,6 +225,10 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict if err != nil { logger.Info("invalid deletion group - ignoring") } else { + if rg >= -ReservedReadinessGroupUpperBound && rg <= -ReservedReadinessGroupLowerBound { + logger.Info(fmt.Sprintf("WARNING: user-supplied deletion-group is in Eno reserved range [%d, %d]", -ReservedReadinessGroupUpperBound, -ReservedReadinessGroupLowerBound), + "kind", res.GVK.Kind, "deletionGroup", rg) + } res.deletionGroup = &rg } } @@ -243,6 +257,27 @@ func newResource(ctx context.Context, parsed *unstructured.Unstructured, strict res.ReadinessChecks = append(res.ReadinessChecks, check) } sort.Slice(res.ReadinessChecks, func(i, j int) bool { return res.ReadinessChecks[i].Name < res.ReadinessChecks[j].Name }) + // This indicates that this is an infrastructure Kind + if defaultGrp, ok := managedCreateOrder[res.GVK.Kind]; ok { + if _, ok := anno[readinessGroupKey]; !ok { + logger.Info("assigning default readiness group to managed kind", + "kind", res.GVK.Kind, "defaultGroup", defaultGrp) + res.readinessGroup = defaultGrp + } else { + logger.Info("user-specified readiness group present, skipping default for managed kind", + "kind", res.GVK.Kind, "readinessGroup", res.readinessGroup) + } + + if _, ok := anno[deletionGroupKey]; !ok { + logger.Info("assigning default deletion group to managed kind", + "kind", res.GVK.Kind, "defaultGroup", defaultGrp) + delGroup := -defaultGrp + res.deletionGroup = &delGroup + } else { + logger.Info("user-specified deletion group present, skipping default for managed kind", + "kind", res.GVK.Kind, "deletionGroup", *res.deletionGroup) + } + } logger.Info("resource created successfully") return res, nil } diff --git a/internal/resource/resource_test.go b/internal/resource/resource_test.go index bf72517b..42cff796 100644 --- a/internal/resource/resource_test.go +++ b/internal/resource/resource_test.go @@ -123,7 +123,9 @@ var newResourceTests = []struct { assert.Equal(t, int(0), r.readinessGroup) assert.False(t, r.DisableUpdates) assert.False(t, r.Replace) - assert.Nil(t, r.deletionGroup) + if assert.NotNil(t, r.deletionGroup) { + assert.Equal(t, -managedCreateOrder["ConfigMap"], *r.deletionGroup) + } }, }, {