From 6856a3916e0980525efe6018e5386203ae9dedef Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Mon, 13 Jan 2025 11:53:22 +0100 Subject: [PATCH] Allow artifact paths to be located outside the sync root path --- bundle/config/mutator/translate_paths.go | 9 ++- .../mutator/translate_paths_artifacts.go | 8 +- .../mutator/translate_paths_artifacts_test.go | 79 +++++++++++++++++++ 3 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 bundle/config/mutator/translate_paths_artifacts_test.go diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go index c18ec2056..469feeafa 100644 --- a/bundle/config/mutator/translate_paths.go +++ b/bundle/config/mutator/translate_paths.go @@ -44,9 +44,14 @@ const ( TranslateModeLocalRelativeWithPrefix ) -// translateOptions specifies how a path should be translated. +// translateOptions control path translation behavior. type translateOptions struct { + // Mode specifies how the path should be translated. Mode TranslateMode + + // AllowPathOutsideSyncRoot can be set for paths that are not tied to the sync root path. + // This is the case for artifact paths, for example. + AllowPathOutsideSyncRoot bool } type ErrIsNotebook struct { @@ -137,7 +142,7 @@ func (t *translateContext) rewritePath( if err != nil { return "", err } - if !filepath.IsLocal(localRelPath) { + if !opts.AllowPathOutsideSyncRoot && !filepath.IsLocal(localRelPath) { return "", fmt.Errorf("path %s is not contained in sync root path", localPath) } diff --git a/bundle/config/mutator/translate_paths_artifacts.go b/bundle/config/mutator/translate_paths_artifacts.go index b075031b3..3f62643e6 100644 --- a/bundle/config/mutator/translate_paths_artifacts.go +++ b/bundle/config/mutator/translate_paths_artifacts.go @@ -23,7 +23,13 @@ func (t *translateContext) artifactRewritePatterns() []artifactRewritePattern { return []artifactRewritePattern{ { base.Append(dyn.Key("path")), - translateOptions{Mode: TranslateModeLocalRelative}, + translateOptions{ + Mode: TranslateModeLocalRelative, + + // Artifact paths may be outside the sync root. + // They are the working directory for artifact builds. + AllowPathOutsideSyncRoot: true, + }, }, } } diff --git a/bundle/config/mutator/translate_paths_artifacts_test.go b/bundle/config/mutator/translate_paths_artifacts_test.go new file mode 100644 index 000000000..8672d0b2f --- /dev/null +++ b/bundle/config/mutator/translate_paths_artifacts_test.go @@ -0,0 +1,79 @@ +package mutator_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/vfs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestTranslatePathsArtifacts_InsideSyncRoot(t *testing.T) { + tmp := t.TempDir() + dir := filepath.Join(tmp, "bundle") + _ = os.MkdirAll(dir, 0o755) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Artifacts: map[string]*config.Artifact{ + "my_artifact": { + Type: "wheel", + + // Assume this is defined in a subdir to the sync root. + Path: "../my_lib", + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", []dyn.Location{{ + File: filepath.Join(dir, "config/artifacts.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the artifact path has been converted to a path relative to the sync root. + assert.Equal(t, "my_lib", b.Config.Artifacts["my_artifact"].Path) +} + +func TestTranslatePathsArtifacts_OutsideSyncRoot(t *testing.T) { + tmp := t.TempDir() + dir := filepath.Join(tmp, "bundle") + _ = os.MkdirAll(dir, 0o755) + + b := &bundle.Bundle{ + SyncRootPath: dir, + SyncRoot: vfs.MustNew(dir), + Config: config.Root{ + Artifacts: map[string]*config.Artifact{ + "my_artifact": { + Type: "wheel", + + // Assume this is defined in a subdir to the sync root. + Path: "../../common/my_lib", + }, + }, + }, + } + + bundletest.SetLocation(b, "artifacts", []dyn.Location{{ + File: filepath.Join(dir, "config/artifacts.yml"), + }}) + + diags := bundle.Apply(context.Background(), b, mutator.TranslatePaths()) + require.NoError(t, diags.Error()) + + // Assert that the artifact path has been converted to a path relative to the sync root. + assert.Equal(t, "../common/my_lib", b.Config.Artifacts["my_artifact"].Path) +}