From 7381948b8ba5a23da53388703aae1e1a38fbb396 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 15:04:52 +0200 Subject: [PATCH 01/11] Add SetDefault helper for declarative defaults --- .../configure_dashboard_defaults.go | 70 ------------------- .../configure_volume_defaults.go | 44 ------------ .../resourcemutator/resource_mutator.go | 20 +++--- bundle/set_default.go | 69 ++++++++++++++++++ 4 files changed, 80 insertions(+), 123 deletions(-) delete mode 100644 bundle/config/mutator/resourcemutator/configure_dashboard_defaults.go delete mode 100644 bundle/config/mutator/resourcemutator/configure_volume_defaults.go create mode 100644 bundle/set_default.go diff --git a/bundle/config/mutator/resourcemutator/configure_dashboard_defaults.go b/bundle/config/mutator/resourcemutator/configure_dashboard_defaults.go deleted file mode 100644 index c21126a569..0000000000 --- a/bundle/config/mutator/resourcemutator/configure_dashboard_defaults.go +++ /dev/null @@ -1,70 +0,0 @@ -package resourcemutator - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" -) - -type configureDashboardDefaults struct{} - -func ConfigureDashboardDefaults() bundle.Mutator { - return &configureDashboardDefaults{} -} - -func (m *configureDashboardDefaults) Name() string { - return "ConfigureDashboardDefaults" -} - -func (m *configureDashboardDefaults) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - - pattern := dyn.NewPattern( - dyn.Key("resources"), - dyn.Key("dashboards"), - dyn.AnyKey(), - ) - - // Configure defaults for all dashboards. - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - var err error - v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("parent_path")), dyn.V(b.Config.Workspace.ResourcePath)) - if err != nil { - return dyn.InvalidValue, err - } - v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("embed_credentials")), dyn.V(false)) - if err != nil { - return dyn.InvalidValue, err - } - return v, nil - }) - }) - - diags = diags.Extend(diag.FromErr(err)) - return diags -} - -func setIfNotExists(v dyn.Value, path dyn.Path, defaultValue dyn.Value) (dyn.Value, error) { - // Get the field at the specified path (if set). - _, err := dyn.GetByPath(v, path) - switch { - case dyn.IsNoSuchKeyError(err): - // OK, we'll set the default value. - break - case dyn.IsCannotTraverseNilError(err): - // Cannot traverse the value, skip it. - return v, nil - case err == nil: - // The field is set, skip it. - return v, nil - default: - // Return the error. - return v, err - } - - // Set the field at the specified path. - return dyn.SetByPath(v, path, defaultValue) -} diff --git a/bundle/config/mutator/resourcemutator/configure_volume_defaults.go b/bundle/config/mutator/resourcemutator/configure_volume_defaults.go deleted file mode 100644 index c0369c3498..0000000000 --- a/bundle/config/mutator/resourcemutator/configure_volume_defaults.go +++ /dev/null @@ -1,44 +0,0 @@ -package resourcemutator - -import ( - "context" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" -) - -type configureVolumeDefaults struct{} - -func ConfigureVolumeDefaults() bundle.Mutator { - return &configureVolumeDefaults{} -} - -func (m *configureVolumeDefaults) Name() string { - return "ConfigureVolumeDefaults" -} - -func (m *configureVolumeDefaults) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - - pattern := dyn.NewPattern( - dyn.Key("resources"), - dyn.Key("volumes"), - dyn.AnyKey(), - ) - - // Configure defaults for all volumes. - err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { - return dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - var err error - v, err = setIfNotExists(v, dyn.NewPath(dyn.Key("volume_type")), dyn.V("MANAGED")) - if err != nil { - return dyn.InvalidValue, err - } - return v, nil - }) - }) - - diags = diags.Extend(diag.FromErr(err)) - return diags -} diff --git a/bundle/config/mutator/resourcemutator/resource_mutator.go b/bundle/config/mutator/resourcemutator/resource_mutator.go index 7cf95caa44..c6a4460cd0 100644 --- a/bundle/config/mutator/resourcemutator/resource_mutator.go +++ b/bundle/config/mutator/resourcemutator/resource_mutator.go @@ -18,7 +18,7 @@ import ( // // If bundle is modified outside of 'resources' section, these changes are discarded. func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - return bundle.ApplySeq( + diags := bundle.ApplySeq( ctx, b, // Reads (typed): b.Config.RunAs, b.Config.Workspace.CurrentUser (validates run_as configuration) @@ -35,16 +35,16 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos // OR corresponding fields on ForEachTask if that is present // Overrides job compute settings with a specified cluster ID for development or testing OverrideCompute(), + ) - // Reads (dynamic): resources.dashboards.* (checks for existing parent_path and embed_credentials) - // Updates (dynamic): resources.dashboards.*.parent_path (sets to workspace.resource_path if not set) - // Updates (dynamic): resources.dashboards.*.embed_credentials (sets to false if not set) - ConfigureDashboardDefaults(), + // Dashboards: + diags = diags.Extend(bundle.SetDefault(ctx, b, b.Config.Workspace.ResourcePath, "resources", "dashboards", "*", "parent_path")) + diags = diags.Extend(bundle.SetDefault(ctx, b, false, "resources", "dashboards", "*", "embed_credentials")) - // Reads (dynamic): resources.volumes.* (checks for existing volume_type) - // Updates (dynamic): resources.volumes.*.volume_type (sets to "MANAGED" if not set) - ConfigureVolumeDefaults(), + // Volumes: + diags = diags.Extend(bundle.SetDefault(ctx, b, "MANAGED", "resources", "volumes", "*", "volume_type")) + diags = diags.Extend(bundle.ApplySeq(ctx, b, ApplyPresets(), // Reads (typed): b.Config.Resources.Jobs (checks job configurations) @@ -62,7 +62,9 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos // Updates (dynamic): resources.*.*.permissions (removes permissions entries where user_name or service_principal_name matches current user) // Removes the current user from all resource permissions as the Terraform provider implicitly grants ownership FilterCurrentUser(), - ) + )) + + return diags } // Normalization is applied multiple times if resource is modified during initialization diff --git a/bundle/set_default.go b/bundle/set_default.go new file mode 100644 index 0000000000..289a07c9e5 --- /dev/null +++ b/bundle/set_default.go @@ -0,0 +1,69 @@ +package bundle + +import ( + "context" + "fmt" + + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type setDefault struct { + pattern dyn.Pattern + key dyn.Path + value any +} + +func SetDefaultMutator(pattern dyn.Pattern, key string, value any) Mutator { + return &setDefault{ + pattern: pattern, + key: dyn.NewPath(dyn.Key(key)), + value: value, + } +} + +func (m *setDefault) Name() string { + return fmt.Sprintf("SetDefaultMutator(%v, %v, %v)", m.pattern, m.key, m.value) +} + +func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + return dyn.MapByPattern(v, m.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + _, err := dyn.GetByPath(v, m.key) + switch { + case dyn.IsNoSuchKeyError(err): + return dyn.SetByPath(v, m.key, dyn.V(m.value)) + default: + return v, err + } + }) + }) + if err != nil { + return diag.FromErr(err) + } + + return nil +} + +func SetDefault(ctx context.Context, b *Bundle, value any, pattern ...string) diag.Diagnostics { + if len(pattern) <= 2 { + return diag.FromErr(fmt.Errorf("Internal error: invalid pattern, not enough components: %#v", pattern)) + } + key := pattern[len(pattern)-1] + if key == "" || key == "*" { + return diag.FromErr(fmt.Errorf("Internal error: invalid pattern, last component must be regular key: %#v", pattern)) + } + + pat := make(dyn.Pattern, len(pattern)-1) + for i := range len(pattern) - 1 { + if pattern[i] == "*" { + pat[i] = dyn.AnyKey() + } else { + pat[i] = dyn.Key(pattern[i]) + } + // We don't support [] here + } + + m := SetDefaultMutator(pat, key, value) + return Apply(ctx, b, m) +} From a68e3df5bfb419a76c1b01dbc5bd6903b2732faa Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 17:07:52 +0200 Subject: [PATCH 02/11] fix: add test cases for wildcard (*) path parsing --- libs/dyn/path_string_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/libs/dyn/path_string_test.go b/libs/dyn/path_string_test.go index eb1816d7db..69a5fcc567 100644 --- a/libs/dyn/path_string_test.go +++ b/libs/dyn/path_string_test.go @@ -88,6 +88,24 @@ func TestNewPathFromString(t *testing.T) { input: "foo[1]bar", err: errors.New("invalid path: foo[1]bar"), }, + + // * is allowed (used in patterns) + { + input: "foo.*", + output: NewPath(Key("foo"), Key("*")), + }, + + // * is allowed (used in patterns) + { + input: "foo.*.bar", + output: NewPath(Key("foo"), Key("*"), Key("bar")), + }, + + // * is allowed (used in patterns) + { + input: "foo[*].bad", + output: NewPath(Key("foo"), Index(-1), Key("bar")), + }, } { p, err := NewPathFromString(tc.input) if tc.err != nil { From 9371ce2993afc6e8f36e48305a61040961554737 Mon Sep 17 00:00:00 2001 From: "Denis Bilenko (aider)" Date: Wed, 16 Apr 2025 17:08:09 +0200 Subject: [PATCH 03/11] feat: implement pattern parsing from strings with comprehensive tests --- libs/dyn/path_string_test.go | 6 +- libs/dyn/pattern_string.go | 97 +++++++++++++++++++++ libs/dyn/pattern_string_test.go | 144 ++++++++++++++++++++++++++++++++ 3 files changed, 244 insertions(+), 3 deletions(-) create mode 100644 libs/dyn/pattern_string.go create mode 100644 libs/dyn/pattern_string_test.go diff --git a/libs/dyn/path_string_test.go b/libs/dyn/path_string_test.go index 69a5fcc567..6d9fd62c55 100644 --- a/libs/dyn/path_string_test.go +++ b/libs/dyn/path_string_test.go @@ -101,10 +101,10 @@ func TestNewPathFromString(t *testing.T) { output: NewPath(Key("foo"), Key("*"), Key("bar")), }, - // * is allowed (used in patterns) + // This is an invalid path (but would be valid for patterns) { - input: "foo[*].bad", - output: NewPath(Key("foo"), Index(-1), Key("bar")), + input: "foo[*].bar", + err: errors.New("invalid path: foo[*].bar"), }, } { p, err := NewPathFromString(tc.input) diff --git a/libs/dyn/pattern_string.go b/libs/dyn/pattern_string.go new file mode 100644 index 0000000000..a804555ffe --- /dev/null +++ b/libs/dyn/pattern_string.go @@ -0,0 +1,97 @@ +package dyn + +import ( + "fmt" + "strconv" + "strings" +) + +// MustPatternFromString is like NewPatternFromString but panics on error. +func MustPatternFromString(input string) Pattern { + p, err := NewPatternFromString(input) + if err != nil { + panic(err) + } + return p +} + +// NewPatternFromString parses a pattern from a string. +// +// The string must be a sequence of keys and indices separated by dots. +// Indices must be enclosed in square brackets. +// The string may include a leading dot. +// The wildcard character '*' can be used to match any key or index. +// +// Examples: +// - foo.bar +// - foo[1].bar +// - foo.*.bar +// - foo[*].bar +// - . +func NewPatternFromString(input string) (Pattern, error) { + var pattern Pattern + + p := input + + for p != "" { + // Every component may have a leading dot. + if p != "" && p[0] == '.' { + p = p[1:] + } + + if p == "" { + return nil, fmt.Errorf("invalid pattern: %s", input) + } + + if p[0] == '[' { + // Find next ] + i := strings.Index(p, "]") + if i < 0 { + return nil, fmt.Errorf("invalid pattern: %s", input) + } + + // Check for wildcard + if p[1:i] == "*" { + pattern = append(pattern, AnyIndex()) + } else { + // Parse index + j, err := strconv.Atoi(p[1:i]) + if err != nil { + return nil, fmt.Errorf("invalid pattern: %s", input) + } + + // Append index + pattern = append(pattern, Index(j)) + } + + p = p[i+1:] + + // The next character must be a . or [ + if p != "" && strings.IndexAny(p, ".[") != 0 { + return nil, fmt.Errorf("invalid pattern: %s", input) + } + } else { + // Find next . or [ + i := strings.IndexAny(p, ".[") + if i < 0 { + i = len(p) + } + + if i == 0 { + return nil, fmt.Errorf("invalid pattern: %s", input) + } + + // Check for wildcard + if p[:i] == "*" { + pattern = append(pattern, AnyKey()) + } else { + // Append key + pattern = append(pattern, Key(p[:i])) + } + + p = p[i:] + } + } + + return pattern, nil +} diff --git a/libs/dyn/pattern_string_test.go b/libs/dyn/pattern_string_test.go new file mode 100644 index 0000000000..a8b001fa9e --- /dev/null +++ b/libs/dyn/pattern_string_test.go @@ -0,0 +1,144 @@ +package dyn_test + +import ( + "errors" + "testing" + + . "github.com/databricks/cli/libs/dyn" + assert "github.com/databricks/cli/libs/dyn/dynassert" +) + +func TestNewPatternFromString(t *testing.T) { + for _, tc := range []struct { + input string + output Pattern + err error + }{ + { + input: "", + output: NewPattern(), + }, + { + input: ".", + output: NewPattern(), + }, + { + input: "foo.bar", + output: NewPattern(Key("foo"), Key("bar")), + }, + { + input: "[1]", + output: NewPattern(Index(1)), + }, + { + input: "foo[1].bar", + output: NewPattern(Key("foo"), Index(1), Key("bar")), + }, + { + input: "foo.bar[1]", + output: NewPattern(Key("foo"), Key("bar"), Index(1)), + }, + { + input: "foo.bar[1][2]", + output: NewPattern(Key("foo"), Key("bar"), Index(1), Index(2)), + }, + { + input: "foo.bar[1][2][3]", + output: NewPattern(Key("foo"), Key("bar"), Index(1), Index(2), Index(3)), + }, + { + input: "foo[1234]", + output: NewPattern(Key("foo"), Index(1234)), + }, + { + input: "foo[123", + err: errors.New("invalid pattern: foo[123"), + }, + { + input: "foo[123]]", + err: errors.New("invalid pattern: foo[123]]"), + }, + { + input: "foo[[123]", + err: errors.New("invalid pattern: foo[[123]"), + }, + { + input: "foo[[123]]", + err: errors.New("invalid pattern: foo[[123]]"), + }, + { + input: "foo[foo]", + err: errors.New("invalid pattern: foo[foo]"), + }, + { + input: "foo..bar", + err: errors.New("invalid pattern: foo..bar"), + }, + { + input: "foo.bar.", + err: errors.New("invalid pattern: foo.bar."), + }, + { + // Every component may have a leading dot. + input: ".foo.[1].bar", + output: NewPattern(Key("foo"), Index(1), Key("bar")), + }, + { + // But after an index there must be a dot. + input: "foo[1]bar", + err: errors.New("invalid pattern: foo[1]bar"), + }, + // Wildcard tests + { + input: "foo.*", + output: NewPattern(Key("foo"), AnyKey()), + }, + { + input: "foo.*.bar", + output: NewPattern(Key("foo"), AnyKey(), Key("bar")), + }, + { + input: "foo[*]", + output: NewPattern(Key("foo"), AnyIndex()), + }, + { + input: "foo[*].bar", + output: NewPattern(Key("foo"), AnyIndex(), Key("bar")), + }, + { + input: "*[1]", + output: NewPattern(AnyKey(), Index(1)), + }, + { + input: "*.*", + output: NewPattern(AnyKey(), AnyKey()), + }, + { + input: "*[*]", + output: NewPattern(AnyKey(), AnyIndex()), + }, + } { + p, err := NewPatternFromString(tc.input) + if tc.err != nil { + assert.EqualError(t, err, tc.err.Error(), tc.input) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.output, p) + } + } +} + +func TestMustPatternFromString(t *testing.T) { + // Test valid pattern + p := MustPatternFromString("foo.bar") + assert.Equal(t, NewPattern(Key("foo"), Key("bar")), p) + + // Test with wildcards + p = MustPatternFromString("foo.*.bar[*]") + assert.Equal(t, NewPattern(Key("foo"), AnyKey(), Key("bar"), AnyIndex()), p) + + // Test that invalid pattern panics + assert.Panics(t, func() { + MustPatternFromString("foo[") + }) +} From d7e75a895d6f196fd96bffbf2620473f9f980d57 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 17:14:30 +0200 Subject: [PATCH 04/11] add pattern_string.go --- libs/dyn/pattern_string.go | 5 +++++ libs/dyn/pattern_string_test.go | 19 +++++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/libs/dyn/pattern_string.go b/libs/dyn/pattern_string.go index a804555ffe..0c35942007 100644 --- a/libs/dyn/pattern_string.go +++ b/libs/dyn/pattern_string.go @@ -33,6 +33,11 @@ func NewPatternFromString(input string) (Pattern, error) { p := input + // Trim leading dot. + if p != "" && p[0] == '.' { + p = p[1:] + } + for p != "" { // Every component may have a leading dot. if p != "" && p[0] == '.' { diff --git a/libs/dyn/pattern_string_test.go b/libs/dyn/pattern_string_test.go index a8b001fa9e..33462e1cff 100644 --- a/libs/dyn/pattern_string_test.go +++ b/libs/dyn/pattern_string_test.go @@ -2,6 +2,7 @@ package dyn_test import ( "errors" + "fmt" "testing" . "github.com/databricks/cli/libs/dyn" @@ -9,7 +10,7 @@ import ( ) func TestNewPatternFromString(t *testing.T) { - for _, tc := range []struct { + for ind, tc := range []struct { input string output Pattern err error @@ -118,13 +119,15 @@ func TestNewPatternFromString(t *testing.T) { output: NewPattern(AnyKey(), AnyIndex()), }, } { - p, err := NewPatternFromString(tc.input) - if tc.err != nil { - assert.EqualError(t, err, tc.err.Error(), tc.input) - } else { - assert.NoError(t, err) - assert.Equal(t, tc.output, p) - } + t.Run(fmt.Sprintf("%d %s", ind, tc.input), func(t *testing.T) { + p, err := NewPatternFromString(tc.input) + if tc.err != nil { + assert.EqualError(t, err, tc.err.Error(), tc.input) + } else { + assert.NoError(t, err) + assert.Equal(t, tc.output, p) + } + }) } } From fd131c1c944b57d6025050e83c4f393145f13d1b Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 17:50:16 +0200 Subject: [PATCH 05/11] pattern from string --- .../resourcemutator/resource_mutator.go | 18 +++++++--- bundle/set_default.go | 22 ++++-------- libs/dyn/pattern.go | 13 +++++++ libs/dyn/pattern_test.go | 34 +++++++++++++++++++ 4 files changed, 67 insertions(+), 20 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/resource_mutator.go b/bundle/config/mutator/resourcemutator/resource_mutator.go index c6a4460cd0..70d543873d 100644 --- a/bundle/config/mutator/resourcemutator/resource_mutator.go +++ b/bundle/config/mutator/resourcemutator/resource_mutator.go @@ -37,12 +37,20 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos OverrideCompute(), ) - // Dashboards: - diags = diags.Extend(bundle.SetDefault(ctx, b, b.Config.Workspace.ResourcePath, "resources", "dashboards", "*", "parent_path")) - diags = diags.Extend(bundle.SetDefault(ctx, b, false, "resources", "dashboards", "*", "embed_credentials")) + // Resource defaults: + + defaults := []struct { + pattern string + value any + }{ + {"resources.dashboards.*.parent_path", b.Config.Workspace.ResourcePath}, + {"resources.dashboards.*.embed_credentials", false}, + {"resources.volumes.*.volume_type", "MANAGED"}, + } - // Volumes: - diags = diags.Extend(bundle.SetDefault(ctx, b, "MANAGED", "resources", "volumes", "*", "volume_type")) + for _, defaultDef := range defaults { + diags = diags.Extend(bundle.SetDefault(ctx, b, defaultDef.pattern, defaultDef.value)) + } diags = diags.Extend(bundle.ApplySeq(ctx, b, ApplyPresets(), diff --git a/bundle/set_default.go b/bundle/set_default.go index 289a07c9e5..79a3fce046 100644 --- a/bundle/set_default.go +++ b/bundle/set_default.go @@ -45,23 +45,15 @@ func (m *setDefault) Apply(ctx context.Context, b *Bundle) diag.Diagnostics { return nil } -func SetDefault(ctx context.Context, b *Bundle, value any, pattern ...string) diag.Diagnostics { - if len(pattern) <= 2 { - return diag.FromErr(fmt.Errorf("Internal error: invalid pattern, not enough components: %#v", pattern)) - } - key := pattern[len(pattern)-1] - if key == "" || key == "*" { - return diag.FromErr(fmt.Errorf("Internal error: invalid pattern, last component must be regular key: %#v", pattern)) +func SetDefault(ctx context.Context, b *Bundle, pattern string, value any) diag.Diagnostics { + pat, err := dyn.NewPatternFromString(pattern) + if err != nil { + return diag.FromErr(fmt.Errorf("Internal error: invalid pattern: %s: %w", pattern, err)) } - pat := make(dyn.Pattern, len(pattern)-1) - for i := range len(pattern) - 1 { - if pattern[i] == "*" { - pat[i] = dyn.AnyKey() - } else { - pat[i] = dyn.Key(pattern[i]) - } - // We don't support [] here + pat, key := pat.Split() + if pat == nil || key == "" { + return diag.FromErr(fmt.Errorf("Internal error: invalid pattern: %s", pattern)) } m := SetDefaultMutator(pat, key, value) diff --git a/libs/dyn/pattern.go b/libs/dyn/pattern.go index 2d2e9cae7b..d48a28dd1d 100644 --- a/libs/dyn/pattern.go +++ b/libs/dyn/pattern.go @@ -32,6 +32,19 @@ func NewPatternFromPath(p Path) Pattern { return cs } +func (p Pattern) Split() (Pattern, string) { + if len(p) <= 1 { + return nil, "" + } + parent := p[:len(p)-1] + leaf := p[len(p)-1] + pc, ok := leaf.(pathComponent) + if !ok { + return nil, "" + } + return parent, pc.Key() +} + // Append appends the given components to the pattern. func (p Pattern) Append(cs ...patternComponent) Pattern { out := make(Pattern, len(p)+len(cs)) diff --git a/libs/dyn/pattern_test.go b/libs/dyn/pattern_test.go index 1b54953efe..feac12a034 100644 --- a/libs/dyn/pattern_test.go +++ b/libs/dyn/pattern_test.go @@ -1,6 +1,7 @@ package dyn_test import ( + "fmt" "testing" "github.com/databricks/cli/libs/dyn" @@ -48,3 +49,36 @@ func TestPatternAppendAlwaysNew(t *testing.T) { p2 := p.Append(dyn.Index(2)) assert.NotEqual(t, p1, p2) } + +func TestPatternSplit(t *testing.T) { + p := dyn.NewPattern( + dyn.Key("foo"), + dyn.Key("bar"), + ) + + pat, key := p.Split() + assert.Equal(t, "bar", key) + assert.Equal(t, dyn.NewPattern(dyn.Key("foo")), pat) +} + +func TestPatternSplitError(t *testing.T) { + patterns := []dyn.Pattern{ + dyn.NewPattern( + dyn.Key("foo"), + dyn.AnyKey(), + ), + dyn.NewPattern( + dyn.Key("foo"), + dyn.AnyIndex(), + ), + dyn.NewPattern(), + } + + for ind, p := range patterns { + t.Run(fmt.Sprintf("%d %#v", ind, p), func(t *testing.T) { + pat, key := p.Split() + assert.Equal(t, "", key) + assert.Empty(t, pat) + }) + } +} From 7705c68e35d53f8a34c59269cdd3794edaffd8e5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 17:54:55 +0200 Subject: [PATCH 06/11] comments --- libs/dyn/path_string_test.go | 4 ++-- libs/dyn/pattern.go | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/dyn/path_string_test.go b/libs/dyn/path_string_test.go index 6d9fd62c55..1b62151339 100644 --- a/libs/dyn/path_string_test.go +++ b/libs/dyn/path_string_test.go @@ -89,13 +89,13 @@ func TestNewPathFromString(t *testing.T) { err: errors.New("invalid path: foo[1]bar"), }, - // * is allowed (used in patterns) + // * is parsed as regular string in NewPathFromString { input: "foo.*", output: NewPath(Key("foo"), Key("*")), }, - // * is allowed (used in patterns) + // * is parsed as regular string in NewPathFromString { input: "foo.*.bar", output: NewPath(Key("foo"), Key("*"), Key("bar")), diff --git a/libs/dyn/pattern.go b/libs/dyn/pattern.go index d48a28dd1d..ae22d8e354 100644 --- a/libs/dyn/pattern.go +++ b/libs/dyn/pattern.go @@ -32,6 +32,8 @@ func NewPatternFromPath(p Path) Pattern { return cs } +// Split . into and +// The last component must be dyn.Key() and there must be at least two components. func (p Pattern) Split() (Pattern, string) { if len(p) <= 1 { return nil, "" From 437db74ffb3e1572a0561e8da0b6299ac54becf6 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 16 Apr 2025 18:02:44 +0200 Subject: [PATCH 07/11] rm unnecessary comment --- bundle/config/mutator/resourcemutator/resource_mutator.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/bundle/config/mutator/resourcemutator/resource_mutator.go b/bundle/config/mutator/resourcemutator/resource_mutator.go index 70d543873d..e0511385d9 100644 --- a/bundle/config/mutator/resourcemutator/resource_mutator.go +++ b/bundle/config/mutator/resourcemutator/resource_mutator.go @@ -37,8 +37,6 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos OverrideCompute(), ) - // Resource defaults: - defaults := []struct { pattern string value any From 15719eb07fbdfac945af7ecd479df10f87a87f33 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 17 Apr 2025 16:32:11 +0200 Subject: [PATCH 08/11] omit unnecessary condition --- libs/dyn/path_string.go | 2 +- libs/dyn/pattern_string.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/dyn/path_string.go b/libs/dyn/path_string.go index 0fa0c682d4..2f17f9279c 100644 --- a/libs/dyn/path_string.go +++ b/libs/dyn/path_string.go @@ -39,7 +39,7 @@ func NewPathFromString(input string) (Path, error) { for p != "" { // Every component may have a leading dot. - if p != "" && p[0] == '.' { + if p[0] == '.' { p = p[1:] } diff --git a/libs/dyn/pattern_string.go b/libs/dyn/pattern_string.go index 0c35942007..6d770faeb9 100644 --- a/libs/dyn/pattern_string.go +++ b/libs/dyn/pattern_string.go @@ -40,7 +40,7 @@ func NewPatternFromString(input string) (Pattern, error) { for p != "" { // Every component may have a leading dot. - if p != "" && p[0] == '.' { + if p[0] == '.' { p = p[1:] } From 88cd57a87fd272d98656d439510d9b1f691334e7 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 17 Apr 2025 16:40:54 +0200 Subject: [PATCH 09/11] Split -> SplitKey --- bundle/set_default.go | 2 +- libs/dyn/pattern.go | 8 ++++++-- libs/dyn/pattern_test.go | 12 ++++++++---- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/bundle/set_default.go b/bundle/set_default.go index 79a3fce046..eae4eeb1eb 100644 --- a/bundle/set_default.go +++ b/bundle/set_default.go @@ -51,7 +51,7 @@ func SetDefault(ctx context.Context, b *Bundle, pattern string, value any) diag. return diag.FromErr(fmt.Errorf("Internal error: invalid pattern: %s: %w", pattern, err)) } - pat, key := pat.Split() + pat, key := pat.SplitKey() if pat == nil || key == "" { return diag.FromErr(fmt.Errorf("Internal error: invalid pattern: %s", pattern)) } diff --git a/libs/dyn/pattern.go b/libs/dyn/pattern.go index ae22d8e354..38904b2245 100644 --- a/libs/dyn/pattern.go +++ b/libs/dyn/pattern.go @@ -34,7 +34,7 @@ func NewPatternFromPath(p Path) Pattern { // Split . into and // The last component must be dyn.Key() and there must be at least two components. -func (p Pattern) Split() (Pattern, string) { +func (p Pattern) SplitKey() (Pattern, string) { if len(p) <= 1 { return nil, "" } @@ -44,7 +44,11 @@ func (p Pattern) Split() (Pattern, string) { if !ok { return nil, "" } - return parent, pc.Key() + key := pc.Key() + if key == "" { + return nil, "" + } + return parent, key } // Append appends the given components to the pattern. diff --git a/libs/dyn/pattern_test.go b/libs/dyn/pattern_test.go index feac12a034..edc0c32660 100644 --- a/libs/dyn/pattern_test.go +++ b/libs/dyn/pattern_test.go @@ -50,18 +50,18 @@ func TestPatternAppendAlwaysNew(t *testing.T) { assert.NotEqual(t, p1, p2) } -func TestPatternSplit(t *testing.T) { +func TestPatternSplitKey(t *testing.T) { p := dyn.NewPattern( dyn.Key("foo"), dyn.Key("bar"), ) - pat, key := p.Split() + pat, key := p.SplitKey() assert.Equal(t, "bar", key) assert.Equal(t, dyn.NewPattern(dyn.Key("foo")), pat) } -func TestPatternSplitError(t *testing.T) { +func TestPatternSplitKeyError(t *testing.T) { patterns := []dyn.Pattern{ dyn.NewPattern( dyn.Key("foo"), @@ -71,12 +71,16 @@ func TestPatternSplitError(t *testing.T) { dyn.Key("foo"), dyn.AnyIndex(), ), + dyn.NewPattern( + dyn.Key("foo"), + dyn.Index(1), + ), dyn.NewPattern(), } for ind, p := range patterns { t.Run(fmt.Sprintf("%d %#v", ind, p), func(t *testing.T) { - pat, key := p.Split() + pat, key := p.SplitKey() assert.Equal(t, "", key) assert.Empty(t, pat) }) From d738ef7fdad2130740e87c9fa6a67adbfa1cdcd9 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Thu, 17 Apr 2025 16:44:34 +0200 Subject: [PATCH 10/11] move comments for consistency --- libs/dyn/path_string_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/libs/dyn/path_string_test.go b/libs/dyn/path_string_test.go index 1b62151339..4e69cee71e 100644 --- a/libs/dyn/path_string_test.go +++ b/libs/dyn/path_string_test.go @@ -88,21 +88,18 @@ func TestNewPathFromString(t *testing.T) { input: "foo[1]bar", err: errors.New("invalid path: foo[1]bar"), }, - - // * is parsed as regular string in NewPathFromString { + // * is parsed as regular string in NewPathFromString input: "foo.*", output: NewPath(Key("foo"), Key("*")), }, - - // * is parsed as regular string in NewPathFromString { + // * is parsed as regular string in NewPathFromString input: "foo.*.bar", output: NewPath(Key("foo"), Key("*"), Key("bar")), }, - - // This is an invalid path (but would be valid for patterns) { + // This is an invalid path (but would be valid for patterns) input: "foo[*].bar", err: errors.New("invalid path: foo[*].bar"), }, From 1f4a9e7f89b7cdb610d16934db2b29f2223465a5 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Tue, 22 Apr 2025 16:16:48 +0200 Subject: [PATCH 11/11] interrupt sequence on error in diags --- bundle/config/mutator/resourcemutator/resource_mutator.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bundle/config/mutator/resourcemutator/resource_mutator.go b/bundle/config/mutator/resourcemutator/resource_mutator.go index e0511385d9..1d85b425ca 100644 --- a/bundle/config/mutator/resourcemutator/resource_mutator.go +++ b/bundle/config/mutator/resourcemutator/resource_mutator.go @@ -37,6 +37,10 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos OverrideCompute(), ) + if diags.HasError() { + return diags + } + defaults := []struct { pattern string value any @@ -48,6 +52,9 @@ func applyInitializeMutators(ctx context.Context, b *bundle.Bundle) diag.Diagnos for _, defaultDef := range defaults { diags = diags.Extend(bundle.SetDefault(ctx, b, defaultDef.pattern, defaultDef.value)) + if diags.HasError() { + return diags + } } diags = diags.Extend(bundle.ApplySeq(ctx, b,