diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 47c2f24d..0446135b 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -3,10 +3,8 @@ package artifacts import ( "context" "fmt" - "path/filepath" "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" ) @@ -35,6 +33,8 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { return diag.Errorf("artifact doesn't exist: %s", m.name) } + var mutators []bundle.Mutator + // Skip building if build command is not specified or infered if artifact.BuildCommand == "" { // If no build command was specified or infered and there is no @@ -45,44 +45,13 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { // We can skip calling build mutator if there is no build command // But we still need to expand glob references in files source path. - diags := expandGlobReference(artifact) - return diags - } - - diags := bundle.Apply(ctx, b, getBuildMutator(artifact.Type, m.name)) - if diags.HasError() { - return diags + } else { + mutators = append(mutators, getBuildMutator(artifact.Type, m.name)) } // We need to expand glob reference after build mutator is applied because // if we do it before, any files that are generated by build command will // not be included into artifact.Files and thus will not be uploaded. - d := expandGlobReference(artifact) - return diags.Extend(d) -} - -func expandGlobReference(artifact *config.Artifact) diag.Diagnostics { - var diags diag.Diagnostics - - // Expand any glob reference in files source path - files := make([]config.ArtifactFile, 0, len(artifact.Files)) - for _, f := range artifact.Files { - matches, err := filepath.Glob(f.Source) - if err != nil { - return diags.Extend(diag.Errorf("unable to find files for %s: %v", f.Source, err)) - } - - if len(matches) == 0 { - return diags.Extend(diag.Errorf("no files found for %s", f.Source)) - } - - for _, match := range matches { - files = append(files, config.ArtifactFile{ - Source: match, - }) - } - } - - artifact.Files = files - return diags + mutators = append(mutators, &expandGlobs{name: m.name}) + return bundle.Apply(ctx, b, bundle.Seq(mutators...)) } diff --git a/bundle/artifacts/expand_globs.go b/bundle/artifacts/expand_globs.go new file mode 100644 index 00000000..61744405 --- /dev/null +++ b/bundle/artifacts/expand_globs.go @@ -0,0 +1,110 @@ +package artifacts + +import ( + "context" + "fmt" + "path/filepath" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type expandGlobs struct { + name string +} + +func (m *expandGlobs) Name() string { + return fmt.Sprintf("artifacts.ExpandGlobs(%s)", m.name) +} + +func createGlobError(v dyn.Value, p dyn.Path, message string) diag.Diagnostic { + // The pattern contained in v is an absolute path. + // Make it relative to the value's location to make it more readable. + source := v.MustString() + if l := v.Location(); l.File != "" { + rel, err := filepath.Rel(filepath.Dir(l.File), source) + if err == nil { + source = rel + } + } + + return diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("%s: %s", source, message), + Locations: []dyn.Location{v.Location()}, + + Paths: []dyn.Path{ + // Hack to clone the path. This path copy is mutable. + // To be addressed in a later PR. + p.Append(), + }, + } +} + +func (m *expandGlobs) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + // Base path for this mutator. + // This path is set with the list of expanded globs when done. + base := dyn.NewPath( + dyn.Key("artifacts"), + dyn.Key(m.name), + dyn.Key("files"), + ) + + // Pattern to match the source key in the files sequence. + pattern := dyn.NewPatternFromPath(base).Append( + dyn.AnyIndex(), + dyn.Key("source"), + ) + + var diags diag.Diagnostics + err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { + var output []dyn.Value + _, err := dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + if v.Kind() != dyn.KindString { + return v, nil + } + + source := v.MustString() + + // Expand any glob reference in files source path + matches, err := filepath.Glob(source) + if err != nil { + diags = diags.Append(createGlobError(v, p, err.Error())) + + // Continue processing and leave this value unchanged. + return v, nil + } + + if len(matches) == 0 { + diags = diags.Append(createGlobError(v, p, "no matching files")) + + // Continue processing and leave this value unchanged. + return v, nil + } + + for _, match := range matches { + output = append(output, dyn.V( + map[string]dyn.Value{ + "source": dyn.NewValue(match, v.Locations()), + }, + )) + } + + return v, nil + }) + + if err != nil || diags.HasError() { + return v, err + } + + // Set the expanded globs back into the configuration. + return dyn.SetByPath(v, base, dyn.V(output)) + }) + + if err != nil { + return diag.FromErr(err) + } + + return diags +} diff --git a/bundle/artifacts/expand_globs_test.go b/bundle/artifacts/expand_globs_test.go new file mode 100644 index 00000000..c9c47844 --- /dev/null +++ b/bundle/artifacts/expand_globs_test.go @@ -0,0 +1,156 @@ +package artifacts + +import ( + "context" + "fmt" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/internal/testutil" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestExpandGlobs_Nominal(t *testing.T) { + tmpDir := t.TempDir() + + testutil.Touch(t, tmpDir, "aa1.txt") + testutil.Touch(t, tmpDir, "aa2.txt") + testutil.Touch(t, tmpDir, "bb.txt") + testutil.Touch(t, tmpDir, "bc.txt") + + b := &bundle.Bundle{ + RootPath: tmpDir, + Config: config.Root{ + Artifacts: config.Artifacts{ + "test": { + Files: []config.ArtifactFile{ + {Source: "./aa*.txt"}, + {Source: "./b[bc].txt"}, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", filepath.Join(tmpDir, "databricks.yml")) + + ctx := context.Background() + diags := bundle.Apply(ctx, b, bundle.Seq( + // Run prepare first to make paths absolute. + &prepare{"test"}, + &expandGlobs{"test"}, + )) + require.NoError(t, diags.Error()) + + // Assert that the expanded paths are correct. + a, ok := b.Config.Artifacts["test"] + if !assert.True(t, ok) { + return + } + assert.Len(t, a.Files, 4) + assert.Equal(t, filepath.Join(tmpDir, "aa1.txt"), a.Files[0].Source) + assert.Equal(t, filepath.Join(tmpDir, "aa2.txt"), a.Files[1].Source) + assert.Equal(t, filepath.Join(tmpDir, "bb.txt"), a.Files[2].Source) + assert.Equal(t, filepath.Join(tmpDir, "bc.txt"), a.Files[3].Source) +} + +func TestExpandGlobs_InvalidPattern(t *testing.T) { + tmpDir := t.TempDir() + + b := &bundle.Bundle{ + RootPath: tmpDir, + Config: config.Root{ + Artifacts: config.Artifacts{ + "test": { + Files: []config.ArtifactFile{ + {Source: "a[.txt"}, + {Source: "./a[.txt"}, + {Source: "../a[.txt"}, + {Source: "subdir/a[.txt"}, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", filepath.Join(tmpDir, "databricks.yml")) + + ctx := context.Background() + diags := bundle.Apply(ctx, b, bundle.Seq( + // Run prepare first to make paths absolute. + &prepare{"test"}, + &expandGlobs{"test"}, + )) + + assert.Len(t, diags, 4) + assert.Equal(t, fmt.Sprintf("%s: syntax error in pattern", filepath.Clean("a[.txt")), diags[0].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[0].Locations[0].File) + assert.Equal(t, "artifacts.test.files[0].source", diags[0].Paths[0].String()) + assert.Equal(t, fmt.Sprintf("%s: syntax error in pattern", filepath.Clean("a[.txt")), diags[1].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[1].Locations[0].File) + assert.Equal(t, "artifacts.test.files[1].source", diags[1].Paths[0].String()) + assert.Equal(t, fmt.Sprintf("%s: syntax error in pattern", filepath.Clean("../a[.txt")), diags[2].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[2].Locations[0].File) + assert.Equal(t, "artifacts.test.files[2].source", diags[2].Paths[0].String()) + assert.Equal(t, fmt.Sprintf("%s: syntax error in pattern", filepath.Clean("subdir/a[.txt")), diags[3].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[3].Locations[0].File) + assert.Equal(t, "artifacts.test.files[3].source", diags[3].Paths[0].String()) +} + +func TestExpandGlobs_NoMatches(t *testing.T) { + tmpDir := t.TempDir() + + testutil.Touch(t, tmpDir, "a1.txt") + testutil.Touch(t, tmpDir, "a2.txt") + testutil.Touch(t, tmpDir, "b1.txt") + testutil.Touch(t, tmpDir, "b2.txt") + + b := &bundle.Bundle{ + RootPath: tmpDir, + Config: config.Root{ + Artifacts: config.Artifacts{ + "test": { + Files: []config.ArtifactFile{ + {Source: "a*.txt"}, + {Source: "b*.txt"}, + {Source: "c*.txt"}, + {Source: "d*.txt"}, + }, + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", filepath.Join(tmpDir, "databricks.yml")) + + ctx := context.Background() + diags := bundle.Apply(ctx, b, bundle.Seq( + // Run prepare first to make paths absolute. + &prepare{"test"}, + &expandGlobs{"test"}, + )) + + assert.Len(t, diags, 2) + assert.Equal(t, "c*.txt: no matching files", diags[0].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[0].Locations[0].File) + assert.Equal(t, "artifacts.test.files[2].source", diags[0].Paths[0].String()) + assert.Equal(t, "d*.txt: no matching files", diags[1].Summary) + assert.Equal(t, filepath.Join(tmpDir, "databricks.yml"), diags[1].Locations[0].File) + assert.Equal(t, "artifacts.test.files[3].source", diags[1].Paths[0].String()) + + // Assert that the original paths are unchanged. + a, ok := b.Config.Artifacts["test"] + if !assert.True(t, ok) { + return + } + + assert.Len(t, a.Files, 4) + assert.Equal(t, "a*.txt", filepath.Base(a.Files[0].Source)) + assert.Equal(t, "b*.txt", filepath.Base(a.Files[1].Source)) + assert.Equal(t, "c*.txt", filepath.Base(a.Files[2].Source)) + assert.Equal(t, "d*.txt", filepath.Base(a.Files[3].Source)) +} diff --git a/bundle/artifacts/upload_test.go b/bundle/artifacts/upload_test.go index a71610b0..202086bd 100644 --- a/bundle/artifacts/upload_test.go +++ b/bundle/artifacts/upload_test.go @@ -110,5 +110,5 @@ func TestExpandGlobFilesSourceWithNoMatches(t *testing.T) { } diags := bundle.Apply(context.Background(), b, bundle.Seq(bm, u)) - require.ErrorContains(t, diags.Error(), "no files found for") + require.ErrorContains(t, diags.Error(), "no matching files") }