Skip to content

Commit 057f1b6

Browse files
denikclaude
andauthored
direct: Support wildcards in resources config; add managed job attributes (#4487)
## Changes - Support wildcards in resources.yml - Add job-related attributes to resources.yml. The list of patterns comes from bundle/configsync/defaults.go, but I could not remove those patterns there, we need to handle the case with new task added. ## Why We need to handle these to ignore known drift. ## Tests Existing tests. --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 8d2a3c0 commit 057f1b6

File tree

8 files changed

+239
-9
lines changed

8 files changed

+239
-9
lines changed

acceptance/bundle/resources/jobs/remote_matches_config/out.plan.direct.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
},
7373
"tasks[task_key='main_task'].new_cluster.aws_attributes": {
7474
"action": "skip",
75-
"reason": "server_side_default",
75+
"reason": "managed",
7676
"remote": {
7777
"availability": "SPOT_WITH_FALLBACK",
7878
"zone_id": "us-east-1c"
@@ -85,7 +85,7 @@
8585
},
8686
"tasks[task_key='main_task'].new_cluster.enable_elastic_disk": {
8787
"action": "skip",
88-
"reason": "server_side_default",
88+
"reason": "managed",
8989
"remote": false
9090
},
9191
"tasks[task_key='main_task'].notebook_task.source": {

bundle/direct/bundle_plan.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,7 @@ func addPerFieldActions(ctx context.Context, adapter *dresources.Adapter, change
437437

438438
func findMatchingRule(path *structpath.PathNode, rules []dresources.FieldRule) (string, bool) {
439439
for _, r := range rules {
440-
if path.HasPrefix(r.Field) {
440+
if path.HasPatternPrefix(r.Field) {
441441
return r.Reason, true
442442
}
443443
}

bundle/direct/dresources/adapter.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ func (a *Adapter) GeneratedResourceConfig() *ResourceLifecycleConfig {
363363

364364
func (a *Adapter) IsFieldInRecreateOnChanges(path *structpath.PathNode) bool {
365365
for _, p := range a.resourceConfig.RecreateOnChanges {
366-
if path.HasPrefix(p.Field) {
366+
if path.HasPatternPrefix(p.Field) {
367367
return true
368368
}
369369
}

bundle/direct/dresources/all_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -842,13 +842,13 @@ func TestGeneratedResourceConfig(t *testing.T) {
842842

843843
func validateResourceConfig(t *testing.T, stateType reflect.Type, cfg *ResourceLifecycleConfig) {
844844
for _, p := range cfg.RecreateOnChanges {
845-
assert.NoError(t, structaccess.ValidatePath(stateType, p.Field), "RecreateOnChanges: %s", p.Field)
845+
assert.NoError(t, structaccess.ValidatePattern(stateType, p.Field), "RecreateOnChanges: %s", p.Field)
846846
}
847847
for _, p := range cfg.UpdateIDOnChanges {
848-
assert.NoError(t, structaccess.ValidatePath(stateType, p.Field), "UpdateIDOnChanges: %s", p.Field)
848+
assert.NoError(t, structaccess.ValidatePattern(stateType, p.Field), "UpdateIDOnChanges: %s", p.Field)
849849
}
850850
for _, p := range cfg.IgnoreRemoteChanges {
851-
assert.NoError(t, structaccess.ValidatePath(stateType, p.Field), "IgnoreRemoteChanges: %s", p.Field)
851+
assert.NoError(t, structaccess.ValidatePattern(stateType, p.Field), "IgnoreRemoteChanges: %s", p.Field)
852852
}
853853
}
854854

bundle/direct/dresources/config.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ import (
1010

1111
// FieldRule represents a field path with its reason for inclusion.
1212
type FieldRule struct {
13-
Field *structpath.PathNode `yaml:"field"`
14-
Reason string `yaml:"reason"`
13+
Field *structpath.PatternNode `yaml:"field"`
14+
Reason string `yaml:"reason"`
1515
}
1616

1717
// ResourceLifecycleConfig defines lifecycle behavior for a resource type.

bundle/direct/dresources/resources.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,29 @@
1313

1414
resources:
1515

16+
jobs:
17+
ignore_remote_changes:
18+
- field: tasks[*].new_cluster.aws_attributes
19+
# These are not "output_only", because they can be set by the user as well.
20+
reason: managed
21+
- field: tasks[*].new_cluster.azure_attributes
22+
reason: managed
23+
- field: tasks[*].new_cluster.gcp_attributes
24+
reason: managed
25+
26+
- field: job_clusters[*].new_cluster.aws_attributes
27+
reason: managed
28+
- field: job_clusters[*].new_cluster.azure_attributes
29+
reason: managed
30+
- field: job_clusters[*].new_cluster.gcp_attributes
31+
reason: managed
32+
33+
# can also be added to backend_defaults with value=false once we have that
34+
- field: tasks[*].new_cluster.enable_elastic_disk
35+
reason: managed
36+
- field: job_clusters[*].new_cluster.enable_elastic_disk
37+
reason: managed
38+
1639
pipelines:
1740
recreate_on_changes:
1841
- field: storage

libs/structs/structpath/path.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,10 @@ func (p *PatternNode) Len() int {
272272
return (*PathNode)(p).Len()
273273
}
274274

275+
func (p *PatternNode) Prefix(n int) *PatternNode {
276+
return (*PatternNode)((*PathNode)(p).Prefix(n))
277+
}
278+
275279
func (p *PatternNode) String() string {
276280
return (*PathNode)(p).String()
277281
}
@@ -834,6 +838,56 @@ func nodesEqual(a, b *PathNode) bool {
834838
return a.key == b.key && a.index == b.index && a.value == b.value
835839
}
836840

841+
// HasPatternPrefix returns true if the path starts with the given pattern prefix.
842+
// Wildcards (.* and [*]) in the pattern match any single path component.
843+
// Both .* and [*] are treated interchangeably.
844+
func (p *PathNode) HasPatternPrefix(prefix *PatternNode) bool {
845+
if prefix.IsRoot() {
846+
return true
847+
}
848+
849+
if p.IsRoot() {
850+
return false
851+
}
852+
853+
pLen := p.Len()
854+
prefixLen := prefix.Len()
855+
856+
if prefixLen > pLen {
857+
return false
858+
}
859+
860+
pAtPrefixLen := p.Prefix(prefixLen)
861+
862+
currentP := pAtPrefixLen
863+
currentPrefix := prefix
864+
865+
for currentPrefix != nil {
866+
if !nodeMatchesPattern(currentP, currentPrefix) {
867+
return false
868+
}
869+
currentP = currentP.prev
870+
currentPrefix = (*PatternNode)((*PathNode)(currentPrefix).prev)
871+
}
872+
873+
return true
874+
}
875+
876+
// nodeMatchesPattern checks if a concrete path node matches a pattern node.
877+
// Wildcards (.* and [*]) match any node.
878+
func nodeMatchesPattern(concrete *PathNode, pattern *PatternNode) bool {
879+
if concrete == nil && pattern == nil {
880+
return true
881+
}
882+
if concrete == nil || pattern == nil {
883+
return false
884+
}
885+
if pattern.DotStar() || pattern.BracketStar() {
886+
return true
887+
}
888+
return nodesEqual(concrete, (*PathNode)(pattern))
889+
}
890+
837891
// MarshalYAML implements yaml.Marshaler for PathNode.
838892
func (p *PathNode) MarshalYAML() (any, error) {
839893
return p.String(), nil
@@ -856,3 +910,26 @@ func (p *PathNode) UnmarshalYAML(unmarshal func(any) error) error {
856910
*p = *(*PathNode)(parsed)
857911
return nil
858912
}
913+
914+
// MarshalYAML implements yaml.Marshaler for PatternNode.
915+
func (p *PatternNode) MarshalYAML() (any, error) {
916+
return p.String(), nil
917+
}
918+
919+
// UnmarshalYAML implements yaml.Unmarshaler for PatternNode.
920+
// Wildcards (.* and [*]) are allowed.
921+
func (p *PatternNode) UnmarshalYAML(unmarshal func(any) error) error {
922+
var s string
923+
if err := unmarshal(&s); err != nil {
924+
return err
925+
}
926+
parsed, err := parse(s, true)
927+
if err != nil {
928+
return err
929+
}
930+
if parsed == nil {
931+
return nil
932+
}
933+
*p = *parsed
934+
return nil
935+
}

libs/structs/structpath/path_test.go

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,136 @@ func TestHasPrefix(t *testing.T) {
883883
}
884884
}
885885

886+
func TestHasPatternPrefix(t *testing.T) {
887+
tests := []struct {
888+
name string
889+
path string
890+
pattern string
891+
expected bool
892+
}{
893+
{
894+
name: "empty pattern",
895+
path: "a.b.c",
896+
pattern: "",
897+
expected: true,
898+
},
899+
{
900+
name: "empty path",
901+
path: "",
902+
pattern: "a",
903+
expected: false,
904+
},
905+
{
906+
name: "exact match no wildcards",
907+
path: "config",
908+
pattern: "config",
909+
expected: true,
910+
},
911+
{
912+
name: "simple prefix no wildcards",
913+
path: "a.b",
914+
pattern: "a",
915+
expected: true,
916+
},
917+
918+
// .* wildcard
919+
{
920+
name: "dot star matches field",
921+
path: "tasks.my_task.notebook_task",
922+
pattern: "tasks.*.notebook_task",
923+
expected: true,
924+
},
925+
{
926+
name: "dot star matches any field",
927+
path: "tasks.other.notebook_task",
928+
pattern: "tasks.*.notebook_task",
929+
expected: true,
930+
},
931+
{
932+
name: "dot star matches index",
933+
path: "items[0].name",
934+
pattern: "items.*.name",
935+
expected: true,
936+
},
937+
{
938+
name: "dot star as prefix",
939+
path: "items[0].name.value",
940+
pattern: "items.*",
941+
expected: true,
942+
},
943+
944+
// [*] wildcard
945+
{
946+
name: "bracket star matches index",
947+
path: "items[0].name",
948+
pattern: "items[*].name",
949+
expected: true,
950+
},
951+
{
952+
name: "bracket star matches field",
953+
path: "tasks.my_task.notebook_task",
954+
pattern: "tasks[*].notebook_task",
955+
expected: true,
956+
},
957+
{
958+
name: "bracket star matches key-value",
959+
path: "tasks[task_key='my_task'].notebook_task",
960+
pattern: "tasks[*].notebook_task",
961+
expected: true,
962+
},
963+
964+
// Multiple wildcards
965+
{
966+
name: "multiple wildcards",
967+
path: "tasks[0].params[1].value",
968+
pattern: "tasks[*].params[*].value",
969+
expected: true,
970+
},
971+
972+
// Non-matches
973+
{
974+
name: "wildcard but wrong suffix",
975+
path: "tasks[0].notebook_task",
976+
pattern: "tasks[*].spark_task",
977+
expected: false,
978+
},
979+
{
980+
name: "pattern longer than path",
981+
path: "tasks[0]",
982+
pattern: "tasks[*].notebook_task",
983+
expected: false,
984+
},
985+
{
986+
name: "no wildcard mismatch",
987+
path: "a.b.c",
988+
pattern: "a.x.c",
989+
expected: false,
990+
},
991+
}
992+
993+
for _, tt := range tests {
994+
t.Run(tt.name, func(t *testing.T) {
995+
path, err := ParsePath(tt.path)
996+
require.NoError(t, err)
997+
998+
pattern, err := ParsePattern(tt.pattern)
999+
require.NoError(t, err)
1000+
1001+
result := path.HasPatternPrefix(pattern)
1002+
assert.Equal(t, tt.expected, result, "path.HasPatternPrefix(pattern) where path=%q, pattern=%q", tt.path, tt.pattern)
1003+
1004+
// If the full pattern matches, every prefix of it must also match.
1005+
if tt.expected {
1006+
for n := range pattern.Len() + 1 {
1007+
prefix := pattern.Prefix(n)
1008+
assert.True(t, path.HasPatternPrefix(prefix),
1009+
"path=%q pattern=%q prefix_len=%d prefix=%s", tt.path, tt.pattern, n, prefix)
1010+
}
1011+
}
1012+
})
1013+
}
1014+
}
1015+
8861016
func TestPathNodeYAMLMarshal(t *testing.T) {
8871017
tests := []struct {
8881018
name string

0 commit comments

Comments
 (0)