From dd941078537581da78a8e6424d4c51953ddbca81 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Fri, 17 May 2024 11:26:09 +0200
Subject: [PATCH] Remove dependency on `ConfigFilePath` from path translation
 mutator (#1437)

## Changes

This is one step toward removing the `path.Paths` struct embedding from
resource types.

Going forward, we'll exclusively use the `dyn.Value` tree for location
information.

## Tests

Existing unit tests that cover path resolution with fallback behavior
pass.
---
 bundle/config/mutator/translate_paths.go      | 28 +++++++++++++++++++
 bundle/config/mutator/translate_paths_jobs.go | 19 ++++---------
 .../mutator/translate_paths_pipelines.go      | 15 ++--------
 bundle/config/paths/paths.go                  | 10 -------
 4 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/bundle/config/mutator/translate_paths.go b/bundle/config/mutator/translate_paths.go
index 018fd79c..18a09dfd 100644
--- a/bundle/config/mutator/translate_paths.go
+++ b/bundle/config/mutator/translate_paths.go
@@ -213,3 +213,31 @@ func (m *translatePaths) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnos
 
 	return diag.FromErr(err)
 }
+
+func gatherFallbackPaths(v dyn.Value, typ string) (map[string]string, error) {
+	var fallback = make(map[string]string)
+	var pattern = dyn.NewPattern(dyn.Key("resources"), dyn.Key(typ), dyn.AnyKey())
+
+	// Previous behavior was to use a resource's location as the base path to resolve
+	// relative paths in its definition. With the introduction of [dyn.Value] throughout,
+	// we can use the location of the [dyn.Value] of the relative path itself.
+	//
+	// This is more flexible, as resources may have overrides that are not
+	// located in the same directory as the resource configuration file.
+	//
+	// To maintain backwards compatibility, we allow relative paths to be resolved using
+	// the original approach as fallback if the [dyn.Value] location cannot be resolved.
+	_, err := dyn.MapByPattern(v, pattern, func(p dyn.Path, v dyn.Value) (dyn.Value, error) {
+		key := p[2].Key()
+		dir, err := v.Location().Directory()
+		if err != nil {
+			return dyn.InvalidValue, fmt.Errorf("unable to determine directory for %s: %w", p, err)
+		}
+		fallback[key] = dir
+		return v, nil
+	})
+	if err != nil {
+		return nil, err
+	}
+	return fallback, nil
+}
diff --git a/bundle/config/mutator/translate_paths_jobs.go b/bundle/config/mutator/translate_paths_jobs.go
index d4166072..58b5e0fb 100644
--- a/bundle/config/mutator/translate_paths_jobs.go
+++ b/bundle/config/mutator/translate_paths_jobs.go
@@ -55,21 +55,14 @@ func rewritePatterns(base dyn.Pattern) []jobRewritePattern {
 }
 
 func (m *translatePaths) applyJobTranslations(b *bundle.Bundle, v dyn.Value) (dyn.Value, error) {
-	var fallback = make(map[string]string)
+	fallback, err := gatherFallbackPaths(v, "jobs")
+	if err != nil {
+		return dyn.InvalidValue, err
+	}
+
+	// Do not translate job task paths if using Git source
 	var ignore []string
-	var err error
-
 	for key, job := range b.Config.Resources.Jobs {
-		dir, err := job.ConfigFileDirectory()
-		if err != nil {
-			return dyn.InvalidValue, fmt.Errorf("unable to determine directory for job %s: %w", key, err)
-		}
-
-		// If we cannot resolve the relative path using the [dyn.Value] location itself,
-		// use the job's location as fallback. This is necessary for backwards compatibility.
-		fallback[key] = dir
-
-		// Do not translate job task paths if using git source
 		if job.GitSource != nil {
 			ignore = append(ignore, key)
 		}
diff --git a/bundle/config/mutator/translate_paths_pipelines.go b/bundle/config/mutator/translate_paths_pipelines.go
index caec4198..5b2a2c34 100644
--- a/bundle/config/mutator/translate_paths_pipelines.go
+++ b/bundle/config/mutator/translate_paths_pipelines.go
@@ -8,18 +8,9 @@ import (
 )
 
 func (m *translatePaths) applyPipelineTranslations(b *bundle.Bundle, v dyn.Value) (dyn.Value, error) {
-	var fallback = make(map[string]string)
-	var err error
-
-	for key, pipeline := range b.Config.Resources.Pipelines {
-		dir, err := pipeline.ConfigFileDirectory()
-		if err != nil {
-			return dyn.InvalidValue, fmt.Errorf("unable to determine directory for pipeline %s: %w", key, err)
-		}
-
-		// If we cannot resolve the relative path using the [dyn.Value] location itself,
-		// use the pipeline's location as fallback. This is necessary for backwards compatibility.
-		fallback[key] = dir
+	fallback, err := gatherFallbackPaths(v, "pipelines")
+	if err != nil {
+		return dyn.InvalidValue, err
 	}
 
 	// Base pattern to match all libraries in all pipelines.
diff --git a/bundle/config/paths/paths.go b/bundle/config/paths/paths.go
index 68c32a48..95977ee3 100644
--- a/bundle/config/paths/paths.go
+++ b/bundle/config/paths/paths.go
@@ -1,9 +1,6 @@
 package paths
 
 import (
-	"fmt"
-	"path/filepath"
-
 	"github.com/databricks/cli/libs/dyn"
 )
 
@@ -23,10 +20,3 @@ func (p *Paths) ConfigureConfigFilePath() {
 	}
 	p.ConfigFilePath = p.DynamicValue.Location().File
 }
-
-func (p *Paths) ConfigFileDirectory() (string, error) {
-	if p.ConfigFilePath == "" {
-		return "", fmt.Errorf("config file path not configured")
-	}
-	return filepath.Dir(p.ConfigFilePath), nil
-}