From b14920cd12f263ef5bc974550b7fa1b32c689a24 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 7 Jul 2023 12:22:58 +0200 Subject: [PATCH] Fixed error reporting when included invalid files in include section (#543) ## Changes Fixed error reporting when included invalid files in include section Case 1. When the file to include is invalid, throw an error Case 2. When the file is loaded but the schema is wrong, indicate which file is failed to load ## Tests With non-existent notexists.yml ``` databricks bundle deploy Error: notexists.yml defined in 'include' section does not match any files ``` With malformed notexists.yml ``` databricks bundle deploy Error: failed to load /Users/andrew.nester/dabs/wheel/notexists.yml: error unmarshaling JSON: json: cannot unmarshal string into Go value of type config.Root ``` --- .../config/mutator/process_root_includes.go | 7 ++++ .../mutator/process_root_includes_test.go | 14 ++++++++ bundle/config/root.go | 2 +- bundle/tests/include_invalid/bundle.yml | 5 +++ bundle/tests/include_test.go | 34 +++++++++++++++++++ bundle/tests/include_with_glob/bundle.yml | 7 ++++ bundle/tests/include_with_glob/job.yml | 4 +++ 7 files changed, 72 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 ef055674a..454e3a987 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,6 +50,12 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) error return err } + // 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.ContainsAny(entry, "*?[") { + 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 1e7f6d1a2..c7d00d88b 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 28333e988..8e8325733 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) diff --git a/bundle/tests/include_invalid/bundle.yml b/bundle/tests/include_invalid/bundle.yml new file mode 100644 index 000000000..f59e2ae0a --- /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 000000000..d704b8380 --- /dev/null +++ b/bundle/tests/include_test.go @@ -0,0 +1,34 @@ +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") + + 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 000000000..b1d078f9c --- /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 000000000..3d609c529 --- /dev/null +++ b/bundle/tests/include_with_glob/job.yml @@ -0,0 +1,4 @@ +resources: + jobs: + my_job: + id: 1