From 98dd2ba5cd47a23ad7a59da6946fe192765b2298 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 24 Sep 2026 10:20:37 +0200 Subject: [PATCH] Make ignore_remote_additions when_set optional An omitted when_set means the rule always applies to a config-declared object, instead of requiring a gate field to be set. Co-authored-by: Isaac --- bundle/direct/bundle_plan.go | 13 ++++-- bundle/direct/bundle_plan_test.go | 58 +++++++++++++++++++++++++ bundle/direct/dresources/config.go | 13 +++--- bundle/direct/dresources/config_test.go | 13 +++--- 4 files changed, 81 insertions(+), 16 deletions(-) diff --git a/bundle/direct/bundle_plan.go b/bundle/direct/bundle_plan.go index d403ca53f76..96712ac4a56 100644 --- a/bundle/direct/bundle_plan.go +++ b/bundle/direct/bundle_plan.go @@ -691,7 +691,8 @@ func shouldSkipNormalized(cfg *dresources.ResourceLifecycleConfig, path *structp // reports an update, and so does a field the user removed from config (Old != nil) — that // is a deletion the user asked for, not a backend addition. // -// The rule is gated on a field within the same object (ignore_remote_additions.when_set). +// The rule may be gated on a field within the same object (ignore_remote_additions.when_set); +// when when_set is omitted the rule always applies to a declared object. // For cluster specs that gate is policy_id: an attached cluster policy supplies values // server-side — "fixed" elements always, "defaultValue" elements when the request sets // apply_policy_default_values — so the remote spec is legitimately a superset of what the @@ -729,9 +730,13 @@ func shouldSkipRemoteAddition(cfg *dresources.ResourceLifecycleConfig, path *str // are validated against the state type by TestResourcesYMLRemoteAdditionGates. continue } - value, err := structaccess.Get(object, rule.WhenSet) - if err != nil || allEmpty(value) { - continue + // A nil gate (when_set omitted) always applies to a declared object; otherwise the + // rule fires only when the gate field is set. + if rule.WhenSet != nil { + value, err := structaccess.Get(object, rule.WhenSet) + if err != nil || allEmpty(value) { + continue + } } return deployplan.ReasonRemoteAddition, true } diff --git a/bundle/direct/bundle_plan_test.go b/bundle/direct/bundle_plan_test.go index 025ef91df6e..baa7fbf140e 100644 --- a/bundle/direct/bundle_plan_test.go +++ b/bundle/direct/bundle_plan_test.go @@ -721,6 +721,64 @@ func TestShouldSkipRemoteAddition(t *testing.T) { } } +func TestShouldSkipRemoteAdditionNoGate(t *testing.T) { + // A rule with when_set omitted always applies to a declared object. + jobCluster, err := structpath.ParsePattern("job_clusters[*].new_cluster") + require.NoError(t, err) + cfg := &dresources.ResourceLifecycleConfig{ + IgnoreRemoteAdditions: []dresources.RemoteAdditionRule{ + {Field: jobCluster}, + }, + } + + state := &jobs.JobSettings{JobClusters: []jobs.JobCluster{{ + JobClusterKey: "small", + NewCluster: &compute.ClusterSpec{}, + }}} + + tests := []struct { + name string + path string + change deployplan.ChangeDesc + expected bool + }{ + { + name: "no gate: any addition inside a declared object is skipped", + path: "job_clusters[job_cluster_key='small'].new_cluster.custom_tags['CostCenter']", + change: deployplan.ChangeDesc{Remote: "dev-1234"}, + expected: true, + }, + { + name: "config disagrees with remote: still an update", + path: "job_clusters[job_cluster_key='small'].new_cluster.custom_tags['CostCenter']", + change: deployplan.ChangeDesc{New: "mine", Remote: "dev-1234"}, + expected: false, + }, + { + // The object itself is absent from config, so the addition is real drift + // even without a gate. + name: "gated object absent from config: still drift", + path: "job_clusters[job_cluster_key='other'].new_cluster.custom_tags['CostCenter']", + change: deployplan.ChangeDesc{Remote: "dev-1234"}, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path, err := structpath.ParsePath(tt.path) + require.NoError(t, err) + + change := tt.change + reason, ok := shouldSkipRemoteAddition(cfg, path, &change, state) + assert.Equal(t, tt.expected, ok) + if tt.expected { + assert.Equal(t, deployplan.ReasonRemoteAddition, reason) + } + }) + } +} + // Types for TestPrepareChangesWholeBlockOverlap: two levels of nesting under an // optional pointer. type threeWayInner struct { diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index cc54d60a678..7418742038e 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -49,15 +49,16 @@ func (b *BackendDefaultRule) UnmarshalYAML(unmarshal func(any) error) error { return nil } -// RemoteAdditionRule marks a sub-object whose contents the backend co-owns whenever the -// object's WhenSet field is set. Inside such an object, a field the config never declared -// coming back set from the remote is an addition by the backend, not drift. +// RemoteAdditionRule marks a sub-object whose contents the backend co-owns. Inside such an +// object, a field the config never declared coming back set from the remote is an addition +// by the backend, not drift. // -// Field is a prefix pattern selecting the object (omitted = the resource root); WhenSet is a -// path within that object, relative to it, whose value gates the rule. +// Field is a prefix pattern selecting the object (omitted = the resource root). WhenSet is a +// path within that object, relative to it, whose value gates the rule; when omitted the rule +// always applies to a declared object. type RemoteAdditionRule struct { Field *structpath.PatternNode `yaml:"field"` - WhenSet *structpath.PathNode `yaml:"when_set"` + WhenSet *structpath.PathNode `yaml:"when_set,omitempty"` } // ResourceLifecycleConfig defines lifecycle behavior for a resource type. diff --git a/bundle/direct/dresources/config_test.go b/bundle/direct/dresources/config_test.go index 1446a14e8e2..2d596057f21 100644 --- a/bundle/direct/dresources/config_test.go +++ b/bundle/direct/dresources/config_test.go @@ -182,17 +182,18 @@ func TestResourcesYMLRemoteAdditionGates(t *testing.T) { for _, rule := range rc.IgnoreRemoteAdditions { field := rule.Field - require.False(t, rule.WhenSet.IsRoot(), - "%s: ignore_remote_additions entry %q needs a when_set", resourceType, field.String()) if !field.IsRoot() { assert.NoError(t, structaccess.ValidatePattern(adapter.StateType(), field), "%s: ignore_remote_additions field %q does not resolve in the state type", resourceType, field.String()) } - gate, err := structpath.ParsePattern(joinPattern(field, rule.WhenSet.String())) - require.NoError(t, err) - assert.NoError(t, structaccess.ValidatePattern(adapter.StateType(), gate), - "%s: ignore_remote_additions when_set %q does not resolve under %q", resourceType, rule.WhenSet, field.String()) + // when_set is optional; a nil gate means the rule always applies. + if rule.WhenSet != nil { + gate, err := structpath.ParsePattern(joinPattern(field, rule.WhenSet.String())) + require.NoError(t, err) + assert.NoError(t, structaccess.ValidatePattern(adapter.StateType(), gate), + "%s: ignore_remote_additions when_set %q does not resolve under %q", resourceType, rule.WhenSet, field.String()) + } } } }