From ed0991f03f61d23a695d479908ae7c6defc95590 Mon Sep 17 00:00:00 2001 From: simonfaltum Date: Tue, 9 Jun 2026 22:30:26 +0200 Subject: [PATCH] Cover resource_path and all occurrences in workspace prefix rewrite The legacy /Workspace prefix rewrite missed ${workspace.resource_path}, only fixed the first occurrence of the first matching pattern per string, and iterated patterns in nondeterministic map order. Co-authored-by: Isaac --- .../mutator/rewrite_workspace_prefix.go | 60 ++++++++++++------- .../mutator/rewrite_workspace_prefix_test.go | 38 ++++++++++-- 2 files changed, 71 insertions(+), 27 deletions(-) diff --git a/bundle/config/mutator/rewrite_workspace_prefix.go b/bundle/config/mutator/rewrite_workspace_prefix.go index e66482f8e55..b67f773f207 100644 --- a/bundle/config/mutator/rewrite_workspace_prefix.go +++ b/bundle/config/mutator/rewrite_workspace_prefix.go @@ -24,15 +24,24 @@ func (m *rewriteWorkspacePrefix) Name() string { func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := diag.Diagnostics{} - paths := map[string]string{ - "/Workspace/${workspace.root_path}": "${workspace.root_path}", - "/Workspace${workspace.root_path}": "${workspace.root_path}", - "/Workspace/${workspace.file_path}": "${workspace.file_path}", - "/Workspace${workspace.file_path}": "${workspace.file_path}", - "/Workspace/${workspace.artifact_path}": "${workspace.artifact_path}", - "/Workspace${workspace.artifact_path}": "${workspace.artifact_path}", - "/Workspace/${workspace.state_path}": "${workspace.state_path}", - "/Workspace${workspace.state_path}": "${workspace.state_path}", + // The patterns must cover every path that PrependWorkspacePrefix prefixes, + // otherwise interpolation produces a doubled "/Workspace/Workspace/..." path. + // A slice (not a map) keeps the rewrite and warning order deterministic when + // multiple patterns occur in the same string. + paths := []struct { + pattern string + replacement string + }{ + {"/Workspace/${workspace.root_path}", "${workspace.root_path}"}, + {"/Workspace${workspace.root_path}", "${workspace.root_path}"}, + {"/Workspace/${workspace.file_path}", "${workspace.file_path}"}, + {"/Workspace${workspace.file_path}", "${workspace.file_path}"}, + {"/Workspace/${workspace.artifact_path}", "${workspace.artifact_path}"}, + {"/Workspace${workspace.artifact_path}", "${workspace.artifact_path}"}, + {"/Workspace/${workspace.state_path}", "${workspace.state_path}"}, + {"/Workspace${workspace.state_path}", "${workspace.state_path}"}, + {"/Workspace/${workspace.resource_path}", "${workspace.resource_path}"}, + {"/Workspace${workspace.resource_path}", "${workspace.resource_path}"}, } err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { @@ -44,23 +53,28 @@ func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di return v, nil } - for path, replacePath := range paths { - if strings.Contains(vv, path) { - newPath := strings.Replace(vv, path, replacePath, 1) - diags = append(diags, diag.Diagnostic{ - Severity: diag.Warning, - Summary: fmt.Sprintf("substring %q found in %q. Please update this to %q.", path, vv, newPath), - Detail: "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths", - Locations: v.Locations(), - Paths: []dyn.Path{p}, - }) - - // Remove the workspace prefix from the string. - return dyn.NewValue(newPath, v.Locations()), nil + newPath := vv + for _, rewrite := range paths { + if !strings.Contains(newPath, rewrite.pattern) { + continue } + + newPath = strings.ReplaceAll(newPath, rewrite.pattern, rewrite.replacement) + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("substring %q found in %q. Please update this to %q.", rewrite.pattern, vv, newPath), + Detail: "For more information, please refer to: https://docs.databricks.com/en/release-notes/dev-tools/bundles.html#workspace-paths", + Locations: v.Locations(), + Paths: []dyn.Path{p}, + }) + } + + if newPath == vv { + return v, nil } - return v, nil + // Remove the workspace prefix from the string. + return dyn.NewValue(newPath, v.Locations()), nil }) }) if err != nil { diff --git a/bundle/config/mutator/rewrite_workspace_prefix_test.go b/bundle/config/mutator/rewrite_workspace_prefix_test.go index 3a51b211bc4..885d8dc2241 100644 --- a/bundle/config/mutator/rewrite_workspace_prefix_test.go +++ b/bundle/config/mutator/rewrite_workspace_prefix_test.go @@ -20,6 +20,7 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { ArtifactPath: "/Workspace/Users/test/artifacts", FilePath: "/Workspace/Users/test/files", StatePath: "/Workspace/Users/test/state", + ResourcePath: "/Workspace/Users/test/resources", }, Resources: config.Resources{ @@ -52,6 +53,25 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { }, }, }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "/Workspace/${workspace.resource_path}/notebook3", + }, + Libraries: []compute.Library{ + { + Jar: "/Workspace${workspace.resource_path}/jar3.jar", + }, + }, + }, + { + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "${workspace.file_path}/file2.py", + Parameters: []string{ + "--input=/Workspace/${workspace.file_path}/in.txt --output=/Workspace/${workspace.file_path}/out.txt", + "--cp=/Workspace/${workspace.root_path}/lib.jar:/Workspace${workspace.artifact_path}/dep.jar", + }, + }, + }, }, }, }, @@ -61,12 +81,17 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { } diags := bundle.Apply(t.Context(), b, RewriteWorkspacePrefix()) - require.Len(t, diags, 3) + require.Len(t, diags, 8) expectedErrors := map[string]bool{ - `substring "/Workspace/${workspace.root_path}" found in "/Workspace/${workspace.root_path}/file1.py". Please update this to "${workspace.root_path}/file1.py".`: true, - `substring "/Workspace${workspace.file_path}" found in "/Workspace${workspace.file_path}/notebook1". Please update this to "${workspace.file_path}/notebook1".`: true, - `substring "/Workspace/${workspace.artifact_path}" found in "/Workspace/${workspace.artifact_path}/jar1.jar". Please update this to "${workspace.artifact_path}/jar1.jar".`: true, + `substring "/Workspace/${workspace.root_path}" found in "/Workspace/${workspace.root_path}/file1.py". Please update this to "${workspace.root_path}/file1.py".`: true, + `substring "/Workspace${workspace.file_path}" found in "/Workspace${workspace.file_path}/notebook1". Please update this to "${workspace.file_path}/notebook1".`: true, + `substring "/Workspace/${workspace.artifact_path}" found in "/Workspace/${workspace.artifact_path}/jar1.jar". Please update this to "${workspace.artifact_path}/jar1.jar".`: true, + `substring "/Workspace/${workspace.resource_path}" found in "/Workspace/${workspace.resource_path}/notebook3". Please update this to "${workspace.resource_path}/notebook3".`: true, + `substring "/Workspace${workspace.resource_path}" found in "/Workspace${workspace.resource_path}/jar3.jar". Please update this to "${workspace.resource_path}/jar3.jar".`: true, + `substring "/Workspace/${workspace.file_path}" found in "--input=/Workspace/${workspace.file_path}/in.txt --output=/Workspace/${workspace.file_path}/out.txt". Please update this to "--input=${workspace.file_path}/in.txt --output=${workspace.file_path}/out.txt".`: true, + `substring "/Workspace/${workspace.root_path}" found in "--cp=/Workspace/${workspace.root_path}/lib.jar:/Workspace${workspace.artifact_path}/dep.jar". Please update this to "--cp=${workspace.root_path}/lib.jar:/Workspace${workspace.artifact_path}/dep.jar".`: true, + `substring "/Workspace${workspace.artifact_path}" found in "--cp=/Workspace/${workspace.root_path}/lib.jar:/Workspace${workspace.artifact_path}/dep.jar". Please update this to "--cp=${workspace.root_path}/lib.jar:${workspace.artifact_path}/dep.jar".`: true, } for _, d := range diags { @@ -80,4 +105,9 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { require.Equal(t, "${workspace.artifact_path}/jar1.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[1].Libraries[0].Jar) require.Equal(t, "${workspace.file_path}/notebook2", b.Config.Resources.Jobs["test_job"].Tasks[2].NotebookTask.NotebookPath) require.Equal(t, "${workspace.artifact_path}/jar2.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[2].Libraries[0].Jar) + require.Equal(t, "${workspace.resource_path}/notebook3", b.Config.Resources.Jobs["test_job"].Tasks[3].NotebookTask.NotebookPath) + require.Equal(t, "${workspace.resource_path}/jar3.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[3].Libraries[0].Jar) + require.Equal(t, "${workspace.file_path}/file2.py", b.Config.Resources.Jobs["test_job"].Tasks[4].SparkPythonTask.PythonFile) + require.Equal(t, "--input=${workspace.file_path}/in.txt --output=${workspace.file_path}/out.txt", b.Config.Resources.Jobs["test_job"].Tasks[4].SparkPythonTask.Parameters[0]) + require.Equal(t, "--cp=${workspace.root_path}/lib.jar:${workspace.artifact_path}/dep.jar", b.Config.Resources.Jobs["test_job"].Tasks[4].SparkPythonTask.Parameters[1]) }