Allow pipelines reference environment dependencies outside sync root (#3816)

## Changes
Check that pipeline can reference environment dependencies outside sync
root

<!-- If your PR needs to be included in the release notes for next
release,
add a separate entry in NEXT_CHANGELOG.md as part of your PR. -->

Author: andrewnester

acceptance/bundle/libraries/outside_of_bundle_root/bundle/databricks.yml

@@ -1,6 +1,9 @@
 bundle:
   name: outside_of_bundle_root
 
+include:
+  - resources/*.yml
+
 variables:
   cluster:
     default:
@@ -25,4 +28,9 @@ resources:
             entry_point: main
             package_name: my_default_python
           libraries:
-            - whl: ../*.whl
+            - whl: ../lib/*.whl
+      environments:
+        - environment_key: default
+          spec:
+            dependencies:
+              - ../lib/*.whl

acceptance/bundle/libraries/outside_of_bundle_root/bundle/resources/pipeline.yml

@@ -0,0 +1,7 @@
+resources:
+  pipelines:
+    pipeline:
+      name: "test"
+      environment:
+        dependencies:
+          - ../../lib/*.whl

…ibraries/outside_of_bundle_root/test.whl …ries/outside_of_bundle_root/lib/test.whlacceptance/bundle/libraries/outside_of_bundle_root/test.whl renamed to acceptance/bundle/libraries/outside_of_bundle_root/lib/test.whl acceptance/bundle/libraries/outside_of_bundle_root/test.whl renamed to acceptance/bundle/libraries/outside_of_bundle_root/lib/test.whl

@@ -1,13 +1,32 @@
 
+>>> [CLI] bundle validate
+Name: outside_of_bundle_root
+Target: default
+Workspace:
+  User: [USERNAME]
+  Path: /Workspace/Users/[USERNAME]/.bundle/outside_of_bundle_root/default
+
+Validation OK!
+
 >>> [CLI] bundle validate -o json
 [
   {
-    "whl": "../*.whl"
+    "whl": "../lib/*.whl"
   }
 ]
 
+>>> [CLI] bundle validate -o json
+[
+  "../lib/*.whl"
+]
+
+>>> [CLI] bundle validate -o json
+[
+  "../lib/*.whl"
+]
+
 >>> [CLI] bundle deploy
-Uploading ../test.whl...
+Uploading ../lib/test.whl...
 Uploading bundle files to /Workspace/Users/[USERNAME]/.bundle/outside_of_bundle_root/default/files...
 Deploying resources...
 Updating deployment state...
@@ -21,6 +40,16 @@ Deployment complete!
   }
 ]
 
+>>> cat out.requests.txt
+[
+  "/Workspace/Users/[USERNAME]/.bundle/outside_of_bundle_root/default/artifacts/.internal/test.whl"
+]
+
+>>> cat out.requests.txt
+[
+  "/Workspace/Users/[USERNAME]/.bundle/outside_of_bundle_root/default/artifacts/.internal/test.whl"
+]
+
 >>> cat out.requests.txt
 {
   "method": "POST",

acceptance/bundle/libraries/outside_of_bundle_root/output.txt

@@ -1,12 +1,16 @@
 cd bundle
-
+trace $CLI bundle validate
 trace $CLI bundle validate -o json | jq '.resources.jobs.test.tasks[0].libraries'
+trace $CLI bundle validate -o json | jq '.resources.jobs.test.environments[0].spec.dependencies'
+trace $CLI bundle validate -o json | jq '.resources.pipelines.pipeline.environment.dependencies'
 trace $CLI bundle deploy
 
 cd ..
 
 title "Check that the job libraries are uploaded and the path is correct in the job"
 trace cat out.requests.txt | jq 'select(.path == "/api/2.2/jobs/create")' | jq '.body.tasks[0].libraries'
+trace cat out.requests.txt | jq 'select(.path == "/api/2.2/jobs/create")' | jq '.body.environments[0].spec.dependencies'
+trace cat out.requests.txt | jq 'select(.path == "/api/2.0/pipelines" and .method == "POST")' | jq '.body.environment.dependencies'
 trace cat out.requests.txt | jq 'select(.path | test("/api/2.0/workspace-files/import-file/Workspace/Users/.*/.bundle/outside_of_bundle_root/default/artifacts/.internal/test.whl"))'
 
 rm out.requests.txt

acceptance/bundle/libraries/outside_of_bundle_root/script

@@ -14,14 +14,42 @@ type pipelineRewritePattern struct {
 	skipRewrite func(string) bool
 }
 
+// Base pattern to match all libraries in all pipelines.
+var base = dyn.NewPattern(
+	dyn.Key("resources"),
+	dyn.Key("pipelines"),
+	dyn.AnyKey(),
+)
+
 func pipelineRewritePatterns() []pipelineRewritePattern {
-	// Base pattern to match all libraries in all pipelines.
-	base := dyn.NewPattern(
-		dyn.Key("resources"),
-		dyn.Key("pipelines"),
-		dyn.AnyKey(),
-	)
+	// Compile list of configuration paths to rewrite.
+	allPatterns := []pipelineRewritePattern{
+		{
+			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("notebook"), dyn.Key("path")),
+			mode:        TranslateModeNotebook,
+			skipRewrite: noSkipRewrite,
+		},
+		{
+			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("file"), dyn.Key("path")),
+			mode:        TranslateModeFile,
+			skipRewrite: noSkipRewrite,
+		},
+		{
+			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("glob"), dyn.Key("include")),
+			mode:        TranslateModeGlob,
+			skipRewrite: noSkipRewrite,
+		},
+		{
+			pattern:     base.Append(dyn.Key("root_path")),
+			mode:        TranslateModeDirectory,
+			skipRewrite: noSkipRewrite,
+		},
+	}
 
+	return allPatterns
+}
+
+func pipelineLibrariesRewritePatterns() []pipelineRewritePattern {
 	pipelineEnvironmentsPatterns := []pipelineRewritePattern{
 		{
 			pattern: dyn.NewPattern(
@@ -57,33 +85,7 @@ func pipelineRewritePatterns() []pipelineRewritePattern {
 		},
 	}
 
-	// Compile list of configuration paths to rewrite.
-	allPatterns := []pipelineRewritePattern{
-		{
-			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("notebook"), dyn.Key("path")),
-			mode:        TranslateModeNotebook,
-			skipRewrite: noSkipRewrite,
-		},
-		{
-			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("file"), dyn.Key("path")),
-			mode:        TranslateModeFile,
-			skipRewrite: noSkipRewrite,
-		},
-		{
-			pattern:     base.Append(dyn.Key("libraries"), dyn.AnyIndex(), dyn.Key("glob"), dyn.Key("include")),
-			mode:        TranslateModeGlob,
-			skipRewrite: noSkipRewrite,
-		},
-		{
-			pattern:     base.Append(dyn.Key("root_path")),
-			mode:        TranslateModeDirectory,
-			skipRewrite: noSkipRewrite,
-		},
-	}
-
-	allPatterns = append(allPatterns, pipelineEnvironmentsPatterns...)
-	allPatterns = append(allPatterns, pipelineEnvironmentsPatternsWithPipFlags...)
-	return allPatterns
+	return append(pipelineEnvironmentsPatterns, pipelineEnvironmentsPatternsWithPipFlags...)
 }
 
 func VisitPipelinePaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) {
@@ -109,3 +111,27 @@ func VisitPipelinePaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) {
 
 	return newValue, nil
 }
+
+func VisitPipelineLibrariesPaths(value dyn.Value, fn VisitFunc) (dyn.Value, error) {
+	var err error
+	newValue := value
+
+	for _, rewritePattern := range pipelineLibrariesRewritePatterns() {
+		newValue, err = dyn.MapByPattern(newValue, rewritePattern.pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
+			sv, ok := v.AsString()
+			if !ok {
+				return v, nil
+			}
+			if rewritePattern.skipRewrite(sv) {
+				return v, nil
+			}
+
+			return fn(p, rewritePattern.mode, v)
+		})
+		if err != nil {
+			return dyn.InvalidValue, err
+		}
+	}
+
+	return newValue, nil
+}

bundle/config/mutator/paths/pipeline_paths_visitor.go

@@ -46,6 +46,7 @@ func TestVisitPipelinePaths(t *testing.T) {
 	}
 
 	actual := collectVisitedPaths(t, root, VisitPipelinePaths)
+	actual = append(actual, collectVisitedPaths(t, root, VisitPipelineLibrariesPaths)...)
 	expected := []dyn.Path{
 		dyn.MustPathFromString("resources.pipelines.pipeline0.libraries[0].file.path"),
 		dyn.MustPathFromString("resources.pipelines.pipeline0.libraries[1].notebook.path"),

bundle/config/mutator/paths/pipeline_paths_visitor_test.go

@@ -15,6 +15,7 @@ func VisitPaths(root dyn.Value, fn VisitFunc) (dyn.Value, error) {
 		VisitArtifactPaths,
 		VisitDashboardPaths,
 		VisitPipelinePaths,
+		VisitPipelineLibrariesPaths,
 	}
 
 	newRoot := root

bundle/config/mutator/paths/visitor.go

@@ -326,7 +326,8 @@ func (m *translatePaths) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn
 	return applyTranslations(ctx, b, t, []func(context.Context, dyn.Value) (dyn.Value, error){
 		t.applyJobTranslations(paths.VisitJobPaths, false),
 		t.applyJobTranslations(paths.VisitJobLibrariesPaths, true),
-		t.applyPipelineTranslations,
+		t.applyPipelineTranslations(paths.VisitPipelinePaths, false),
+		t.applyPipelineTranslations(paths.VisitPipelineLibrariesPaths, true),
 		t.applyArtifactTranslations,
 		t.applyAppsTranslations,
 	})

bundle/config/mutator/translate_paths.go

@@ -12,55 +12,58 @@ import (
 	"github.com/databricks/cli/libs/logdiag"
 )
 
-func (t *translateContext) applyPipelineTranslations(ctx context.Context, v dyn.Value) (dyn.Value, error) {
-	var err error
+func (t *translateContext) applyPipelineTranslations(visitor visitFunc, allowOutsideSyncRoot bool) translateFunc {
+	return func(ctx context.Context, v dyn.Value) (dyn.Value, error) {
+		var err error
 
-	fallback, err := gatherFallbackPaths(v, "pipelines")
-	if err != nil {
-		return dyn.InvalidValue, err
-	}
-
-	return paths.VisitPipelinePaths(v, func(p dyn.Path, mode paths.TranslateMode, v dyn.Value) (dyn.Value, error) {
-		key := p[2].Key()
-		opts := translateOptions{
-			Mode: mode,
-		}
-
-		// Handle path as if it's relative to the bundle root
-		nv, err := t.rewriteValue(ctx, p, v, t.b.BundleRootPath, opts)
-		if err == nil {
-			return nv, nil
+		fallback, err := gatherFallbackPaths(v, "pipelines")
+		if err != nil {
+			return dyn.InvalidValue, err
 		}
 
-		// If we failed to rewrite the path, it uses an old path format which relied on fallback.
-		if fallback[key] != "" {
-			dir, nerr := locationDirectory(v.Location())
-			if nerr != nil {
-				return dyn.InvalidValue, nerr
+		return visitor(v, func(p dyn.Path, mode paths.TranslateMode, v dyn.Value) (dyn.Value, error) {
+			key := p[2].Key()
+			opts := translateOptions{
+				Mode:                     mode,
+				AllowPathOutsideSyncRoot: allowOutsideSyncRoot,
 			}
 
-			dirRel, nerr := filepath.Rel(t.b.BundleRootPath, dir)
-			if nerr != nil {
-				return dyn.InvalidValue, nerr
+			// Handle path as if it's relative to the bundle root
+			nv, err := t.rewriteValue(ctx, p, v, t.b.BundleRootPath, opts)
+			if err == nil {
+				return nv, nil
 			}
 
-			originalPath, nerr := filepath.Rel(dirRel, v.MustString())
-			if nerr != nil {
-				return dyn.InvalidValue, nerr
-			}
+			// If we failed to rewrite the path, it uses an old path format which relied on fallback.
+			if fallback[key] != "" {
+				dir, nerr := locationDirectory(v.Location())
+				if nerr != nil {
+					return dyn.InvalidValue, nerr
+				}
 
-			originalValue := dyn.NewValue(originalPath, v.Locations())
-			nv, nerr := t.rewriteValue(ctx, p, originalValue, fallback[key], opts)
-			if nerr == nil {
-				logdiag.LogDiag(ctx, diag.Diagnostic{
-					Severity:  diag.Error,
-					Summary:   fmt.Sprintf("path %s is defined relative to the %s directory (%s). Please update the path to be relative to the file where it is defined or use earlier version of CLI (0.261.0 or earlier).", originalPath, fallback[key], v.Location()),
-					Locations: v.Locations(),
-				})
-				return nv, nil
+				dirRel, nerr := filepath.Rel(t.b.BundleRootPath, dir)
+				if nerr != nil {
+					return dyn.InvalidValue, nerr
+				}
+
+				originalPath, nerr := filepath.Rel(dirRel, v.MustString())
+				if nerr != nil {
+					return dyn.InvalidValue, nerr
+				}
+
+				originalValue := dyn.NewValue(originalPath, v.Locations())
+				nv, nerr := t.rewriteValue(ctx, p, originalValue, fallback[key], opts)
+				if nerr == nil {
+					logdiag.LogDiag(ctx, diag.Diagnostic{
+						Severity:  diag.Error,
+						Summary:   fmt.Sprintf("path %s is defined relative to the %s directory (%s). Please update the path to be relative to the file where it is defined or use earlier version of CLI (0.261.0 or earlier).", originalPath, fallback[key], v.Location()),
+						Locations: v.Locations(),
+					})
+					return nv, nil
+				}
 			}
-		}
 
-		return dyn.InvalidValue, err
-	})
+			return dyn.InvalidValue, err
+		})
+	}
 }