From 6d6b68962356c43082f429f8f498f09f023448a7 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 4 Jul 2023 11:17:00 +0200 Subject: [PATCH 1/4] Fixed error reporting when included invalid files in include section --- bundle/config/mutator/process_root_includes.go | 4 ++++ .../config/mutator/process_root_includes_test.go | 14 ++++++++++++++ bundle/config/root.go | 2 +- 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index ef055674ae..5285212653 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -49,6 +49,10 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error return err } + if len(matches) == 0 { + return fmt.Errorf("%s defined in 'include' section does not match any files", entry) + } + // Filter matches to ones we haven't seen yet. var includes []string for _, match := range matches { diff --git a/bundle/config/mutator/process_root_includes_test.go b/bundle/config/mutator/process_root_includes_test.go index 1e7f6d1a2b..c7d00d88bd 100644 --- a/bundle/config/mutator/process_root_includes_test.go +++ b/bundle/config/mutator/process_root_includes_test.go @@ -108,3 +108,17 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { require.NoError(t, err) assert.Equal(t, []string{"a.yml"}, bundle.Config.Include) } + +func TestProcessRootIncludesNotExists(t *testing.T) { + bundle := &bundle.Bundle{ + Config: config.Root{ + Path: t.TempDir(), + Include: []string{ + "notexist.yml", + }, + }, + } + err := mutator.ProcessRootIncludes().Apply(context.Background(), bundle) + require.Error(t, err) + assert.Contains(t, err.Error(), "notexist.yml defined in 'include' section does not match any files") +} diff --git a/bundle/config/root.go b/bundle/config/root.go index 28333e988a..8e83257330 100644 --- a/bundle/config/root.go +++ b/bundle/config/root.go @@ -118,7 +118,7 @@ func (r *Root) Load(path string) error { } err = yaml.Unmarshal(raw, r) if err != nil { - return err + return fmt.Errorf("failed to load %s: %w", path, err) } r.Path = filepath.Dir(path) From 24e857e38049e94fc901179ed93fc07d1b09bed2 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 4 Jul 2023 11:24:24 +0200 Subject: [PATCH 2/4] fix --- bundle/config/mutator/process_root_includes.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index 5285212653..08067dff6d 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "path/filepath" + "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" @@ -49,7 +50,9 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error return err } - if len(matches) == 0 { + // If the entry is not a glob pattern and no matches found, + // return an error because the file defined is not found + if len(matches) == 0 && !strings.Contains(entry, "*") { return fmt.Errorf("%s defined in 'include' section does not match any files", entry) } From d7d6f4eeac4252884795b206688fc0c180841099 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 4 Jul 2023 13:06:07 +0200 Subject: [PATCH 3/4] Added unit tests --- .../config/mutator/process_root_includes.go | 2 +- bundle/tests/include_invalid/bundle.yml | 5 +++ bundle/tests/include_test.go | 35 +++++++++++++++++++ bundle/tests/include_with_glob/bundle.yml | 7 ++++ bundle/tests/include_with_glob/job.yml | 4 +++ 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 bundle/tests/include_invalid/bundle.yml create mode 100644 bundle/tests/include_test.go create mode 100644 bundle/tests/include_with_glob/bundle.yml create mode 100644 bundle/tests/include_with_glob/job.yml diff --git a/bundle/config/mutator/process_root_includes.go b/bundle/config/mutator/process_root_includes.go index 08067dff6d..454e3a9870 100644 --- a/bundle/config/mutator/process_root_includes.go +++ b/bundle/config/mutator/process_root_includes.go @@ -52,7 +52,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error // If the entry is not a glob pattern and no matches found, // return an error because the file defined is not found - if len(matches) == 0 && !strings.Contains(entry, "*") { + if len(matches) == 0 && !strings.ContainsAny(entry, "*?[") { return fmt.Errorf("%s defined in 'include' section does not match any files", entry) } diff --git a/bundle/tests/include_invalid/bundle.yml b/bundle/tests/include_invalid/bundle.yml new file mode 100644 index 0000000000..f59e2ae0a5 --- /dev/null +++ b/bundle/tests/include_invalid/bundle.yml @@ -0,0 +1,5 @@ +bundle: + name: include_invalid + +include: + - notexists.yml diff --git a/bundle/tests/include_test.go b/bundle/tests/include_test.go new file mode 100644 index 0000000000..5318fca8fc --- /dev/null +++ b/bundle/tests/include_test.go @@ -0,0 +1,35 @@ +package config_tests + +import ( + "context" + "path/filepath" + "sort" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/exp/maps" +) + +func TestIncludeInvalid(t *testing.T) { + b, err := bundle.Load("./include_invalid") + require.NoError(t, err) + err = bundle.Apply(context.Background(), b, bundle.Seq(mutator.DefaultMutators()...)) + require.Error(t, err) + assert.Contains(t, err.Error(), "notexists.yml defined in 'include' section does not match any files") +} + +func TestIncludeWithGlob(t *testing.T) { + b := load(t, "./include_with_glob") + + // Test that both jobs were loaded. + keys := maps.Keys(b.Config.Resources.Jobs) + sort.Strings(keys) + assert.Equal(t, []string{"my_job"}, keys) + + job := b.Config.Resources.Jobs["my_job"] + assert.Equal(t, "1", job.ID) + assert.Equal(t, "include_with_glob/job.yml", filepath.ToSlash(job.ConfigFilePath)) +} diff --git a/bundle/tests/include_with_glob/bundle.yml b/bundle/tests/include_with_glob/bundle.yml new file mode 100644 index 0000000000..b1d078f9c5 --- /dev/null +++ b/bundle/tests/include_with_glob/bundle.yml @@ -0,0 +1,7 @@ +bundle: + name: include_with_glob + +include: + - "*.yml" + - "?.yml" + - "[a-z].yml" diff --git a/bundle/tests/include_with_glob/job.yml b/bundle/tests/include_with_glob/job.yml new file mode 100644 index 0000000000..3d609c529e --- /dev/null +++ b/bundle/tests/include_with_glob/job.yml @@ -0,0 +1,4 @@ +resources: + jobs: + my_job: + id: 1 From 3cce6852fafdbfd0173356cbb9acb46a475a8790 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 7 Jul 2023 12:16:16 +0200 Subject: [PATCH 4/4] Removed the comment --- bundle/tests/include_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/tests/include_test.go b/bundle/tests/include_test.go index 5318fca8fc..d704b8380b 100644 --- a/bundle/tests/include_test.go +++ b/bundle/tests/include_test.go @@ -24,7 +24,6 @@ func TestIncludeInvalid(t *testing.T) { func TestIncludeWithGlob(t *testing.T) { b := load(t, "./include_with_glob") - // Test that both jobs were loaded. keys := maps.Keys(b.Config.Resources.Jobs) sort.Strings(keys) assert.Equal(t, []string{"my_job"}, keys)