From e63462162afb4f8447a16f1715e222cf12851c7d Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 22:41:43 +0200 Subject: [PATCH 1/3] Fix include diagnostics pointing at the wrong include entry Co-authored-by: Isaac --- bundle/config/loader/process_root_includes.go | 15 ++++++++----- .../loader/process_root_includes_test.go | 22 +++++++++++++++++++ .../loader/testdata/non_yaml_glob/a.txt | 1 + .../loader/testdata/non_yaml_glob/b.txt | 1 + .../testdata/non_yaml_glob/databricks.yml | 5 +++++ 5 files changed, 38 insertions(+), 6 deletions(-) create mode 100644 bundle/config/loader/testdata/non_yaml_glob/a.txt create mode 100644 bundle/config/loader/testdata/non_yaml_glob/b.txt create mode 100644 bundle/config/loader/testdata/non_yaml_glob/databricks.yml diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 934c777a90a..11b095e528c 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -72,7 +72,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // For each glob, find all files to load. // Ordering of the list of globs is maintained in the output. // For matches that appear in multiple globs, only the first is kept. - for _, entry := range b.Config.Include { + for entryIndex, entry := range b.Config.Include { // Include paths must be relative. if filepath.IsAbs(entry) { return diag.Errorf("%s: includes must be relative paths", entry) @@ -92,7 +92,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // Filter matches to ones we haven't seen yet. var includes []string - for i, match := range matches { + for _, match := range matches { rel, err := filepath.Rel(b.BundleRootPath, match) if err != nil { return diag.FromErr(err) @@ -103,10 +103,13 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. seen[rel] = true if filepath.Ext(rel) != ".yaml" && filepath.Ext(rel) != ".yml" && filepath.Ext(rel) != ".json" { diags = diags.Append(diag.Diagnostic{ - Severity: diag.Error, - Summary: "Files in the 'include' configuration section must be YAML or JSON files.", - Detail: fmt.Sprintf("The file %s in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead.", rel), - Locations: b.Config.GetLocations(fmt.Sprintf("include[%d]", i)), + Severity: diag.Error, + Summary: "Files in the 'include' configuration section must be YAML or JSON files.", + Detail: fmt.Sprintf("The file %s in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead.", rel), + // Attribute the diagnostic to the include entry whose glob produced + // this match; the index of the match within the glob is unrelated + // to the entry's position in the include list. + Locations: b.Config.GetLocations(fmt.Sprintf("include[%d]", entryIndex)), }) continue } diff --git a/bundle/config/loader/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go index cb98037507f..a01fd837111 100644 --- a/bundle/config/loader/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -98,6 +98,28 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { assert.Equal(t, []string{"a.yml"}, b.Config.Include) } +func TestProcessRootIncludesNonYamlGlobLocations(t *testing.T) { + b := &bundle.Bundle{ + BundleRootPath: "testdata/non_yaml_glob", + } + + diags := bundle.Apply(t.Context(), b, loader.EntryPoint()) + require.NoError(t, diags.Error()) + + // Both matches of the single glob must be attributed to the glob's own + // include entry, not to the match index within the glob. + expected := b.Config.GetLocations("include[0]") + require.NotEmpty(t, expected) + + diags = bundle.Apply(t.Context(), b, loader.ProcessRootIncludes()) + require.Len(t, diags, 2) + assert.Contains(t, diags[0].Detail, "a.txt") + assert.Contains(t, diags[1].Detail, "b.txt") + for _, d := range diags { + assert.Equal(t, expected, d.Locations) + } +} + func TestProcessRootIncludesNotExists(t *testing.T) { b := &bundle.Bundle{ BundleRootPath: t.TempDir(), diff --git a/bundle/config/loader/testdata/non_yaml_glob/a.txt b/bundle/config/loader/testdata/non_yaml_glob/a.txt new file mode 100644 index 00000000000..286888a0ab5 --- /dev/null +++ b/bundle/config/loader/testdata/non_yaml_glob/a.txt @@ -0,0 +1 @@ +not yaml diff --git a/bundle/config/loader/testdata/non_yaml_glob/b.txt b/bundle/config/loader/testdata/non_yaml_glob/b.txt new file mode 100644 index 00000000000..286888a0ab5 --- /dev/null +++ b/bundle/config/loader/testdata/non_yaml_glob/b.txt @@ -0,0 +1 @@ +not yaml diff --git a/bundle/config/loader/testdata/non_yaml_glob/databricks.yml b/bundle/config/loader/testdata/non_yaml_glob/databricks.yml new file mode 100644 index 00000000000..1c190015016 --- /dev/null +++ b/bundle/config/loader/testdata/non_yaml_glob/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: non_yaml_glob + +include: + - "*.txt" From 12b15de0d1b29f4c04f79fb79964ea65bd5965e2 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 00:29:39 +0200 Subject: [PATCH 2/3] Remove redundant comments Co-authored-by: Isaac --- bundle/config/loader/process_root_includes.go | 4 +--- bundle/config/loader/process_root_includes_test.go | 2 -- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 11b095e528c..74803f9c06b 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -106,9 +106,7 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. Severity: diag.Error, Summary: "Files in the 'include' configuration section must be YAML or JSON files.", Detail: fmt.Sprintf("The file %s in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead.", rel), - // Attribute the diagnostic to the include entry whose glob produced - // this match; the index of the match within the glob is unrelated - // to the entry's position in the include list. + // The match's index within the glob is unrelated to the entry's position in the include list. Locations: b.Config.GetLocations(fmt.Sprintf("include[%d]", entryIndex)), }) continue diff --git a/bundle/config/loader/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go index a01fd837111..274ab22837d 100644 --- a/bundle/config/loader/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -106,8 +106,6 @@ func TestProcessRootIncludesNonYamlGlobLocations(t *testing.T) { diags := bundle.Apply(t.Context(), b, loader.EntryPoint()) require.NoError(t, diags.Error()) - // Both matches of the single glob must be attributed to the glob's own - // include entry, not to the match index within the glob. expected := b.Config.GetLocations("include[0]") require.NotEmpty(t, expected) From 89afacc404dc40b4d8e1a6cc8a0b2c31c2bed50e Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Wed, 10 Jun 2026 18:25:19 +0200 Subject: [PATCH 3/3] Cover include glob diagnostics with an acceptance test Per review feedback, demonstrate the fix through 'bundle validate' output instead of a unit test. The non_yaml_in_include acceptance test now uses a glob matching two non-YAML files; both errors point at the glob's own include entry (databricks.yml:6:5). With the previous code the first error pointed at include[0] (the resources/*.yml entry) instead. Co-authored-by: Isaac --- .../non_yaml_in_include/databricks.yml | 2 +- .../includes/non_yaml_in_include/output.txt | 9 +++++++-- .../includes/non_yaml_in_include/test2.py | 1 + .../loader/process_root_includes_test.go | 20 ------------------- .../loader/testdata/non_yaml_glob/a.txt | 1 - .../loader/testdata/non_yaml_glob/b.txt | 1 - .../testdata/non_yaml_glob/databricks.yml | 5 ----- 7 files changed, 9 insertions(+), 30 deletions(-) create mode 100644 acceptance/bundle/includes/non_yaml_in_include/test2.py delete mode 100644 bundle/config/loader/testdata/non_yaml_glob/a.txt delete mode 100644 bundle/config/loader/testdata/non_yaml_glob/b.txt delete mode 100644 bundle/config/loader/testdata/non_yaml_glob/databricks.yml diff --git a/acceptance/bundle/includes/non_yaml_in_include/databricks.yml b/acceptance/bundle/includes/non_yaml_in_include/databricks.yml index 15c68e1cf4e..7fd2b0724d7 100644 --- a/acceptance/bundle/includes/non_yaml_in_include/databricks.yml +++ b/acceptance/bundle/includes/non_yaml_in_include/databricks.yml @@ -2,5 +2,5 @@ bundle: name: non_yaml_in_includes include: - - test.py - resources/*.yml + - "*.py" diff --git a/acceptance/bundle/includes/non_yaml_in_include/output.txt b/acceptance/bundle/includes/non_yaml_in_include/output.txt index c4f259cb087..6b9fe12d7bd 100644 --- a/acceptance/bundle/includes/non_yaml_in_include/output.txt +++ b/acceptance/bundle/includes/non_yaml_in_include/output.txt @@ -1,10 +1,15 @@ Error: Files in the 'include' configuration section must be YAML or JSON files. - in databricks.yml:5:5 + in databricks.yml:6:5 The file test.py in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead. +Error: Files in the 'include' configuration section must be YAML or JSON files. + in databricks.yml:6:5 + +The file test2.py in the 'include' configuration section is not a YAML or JSON file, and only such files are supported. To include files to sync, specify them in the 'sync.include' configuration section instead. + Name: non_yaml_in_includes -Found 1 error +Found 2 errors Exit code: 1 diff --git a/acceptance/bundle/includes/non_yaml_in_include/test2.py b/acceptance/bundle/includes/non_yaml_in_include/test2.py new file mode 100644 index 00000000000..e0aa8cdbfb6 --- /dev/null +++ b/acceptance/bundle/includes/non_yaml_in_include/test2.py @@ -0,0 +1 @@ +print("Hello again") diff --git a/bundle/config/loader/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go index 274ab22837d..cb98037507f 100644 --- a/bundle/config/loader/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -98,26 +98,6 @@ func TestProcessRootIncludesRemoveDups(t *testing.T) { assert.Equal(t, []string{"a.yml"}, b.Config.Include) } -func TestProcessRootIncludesNonYamlGlobLocations(t *testing.T) { - b := &bundle.Bundle{ - BundleRootPath: "testdata/non_yaml_glob", - } - - diags := bundle.Apply(t.Context(), b, loader.EntryPoint()) - require.NoError(t, diags.Error()) - - expected := b.Config.GetLocations("include[0]") - require.NotEmpty(t, expected) - - diags = bundle.Apply(t.Context(), b, loader.ProcessRootIncludes()) - require.Len(t, diags, 2) - assert.Contains(t, diags[0].Detail, "a.txt") - assert.Contains(t, diags[1].Detail, "b.txt") - for _, d := range diags { - assert.Equal(t, expected, d.Locations) - } -} - func TestProcessRootIncludesNotExists(t *testing.T) { b := &bundle.Bundle{ BundleRootPath: t.TempDir(), diff --git a/bundle/config/loader/testdata/non_yaml_glob/a.txt b/bundle/config/loader/testdata/non_yaml_glob/a.txt deleted file mode 100644 index 286888a0ab5..00000000000 --- a/bundle/config/loader/testdata/non_yaml_glob/a.txt +++ /dev/null @@ -1 +0,0 @@ -not yaml diff --git a/bundle/config/loader/testdata/non_yaml_glob/b.txt b/bundle/config/loader/testdata/non_yaml_glob/b.txt deleted file mode 100644 index 286888a0ab5..00000000000 --- a/bundle/config/loader/testdata/non_yaml_glob/b.txt +++ /dev/null @@ -1 +0,0 @@ -not yaml diff --git a/bundle/config/loader/testdata/non_yaml_glob/databricks.yml b/bundle/config/loader/testdata/non_yaml_glob/databricks.yml deleted file mode 100644 index 1c190015016..00000000000 --- a/bundle/config/loader/testdata/non_yaml_glob/databricks.yml +++ /dev/null @@ -1,5 +0,0 @@ -bundle: - name: non_yaml_glob - -include: - - "*.txt"