From 7604da5d74f318b8557b55ef74a1289784eb2e69 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 5 Sep 2024 16:45:34 +0200 Subject: [PATCH] Show warning and rewrite paths --- .../mutator/rewrite_workspace_prefix.go | 66 +++++++++++++++++++ .../rewrite_workspace_prefix_test.go} | 13 +++- .../validate/no_workspace_prefix_used.go | 60 ----------------- bundle/phases/initialize.go | 2 +- 4 files changed, 77 insertions(+), 64 deletions(-) create mode 100644 bundle/config/mutator/rewrite_workspace_prefix.go rename bundle/config/{validate/no_workspace_prefix_used_test.go => mutator/rewrite_workspace_prefix_test.go} (74%) delete mode 100644 bundle/config/validate/no_workspace_prefix_used.go diff --git a/bundle/config/mutator/rewrite_workspace_prefix.go b/bundle/config/mutator/rewrite_workspace_prefix.go new file mode 100644 index 0000000000..fe7a1b86f3 --- /dev/null +++ b/bundle/config/mutator/rewrite_workspace_prefix.go @@ -0,0 +1,66 @@ +package mutator + +import ( + "context" + "fmt" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" +) + +type rewriteWorkspacePrefix struct{} + +// RewriteWorkspacePrefix finds any strings in bundle configration that have +// workspace prefix plus workspace path variable used and removes workspace prefix from it. +func RewriteWorkspacePrefix() bundle.Mutator { + return &rewriteWorkspacePrefix{} +} + +func (m *rewriteWorkspacePrefix) Name() string { + return "RewriteWorkspacePrefix" +} + +func (m *rewriteWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + diags := diag.Diagnostics{} + paths := []string{ + "/Workspace/${workspace.root_path}", + "/Workspace/${workspace.file_path}", + "/Workspace/${workspace.artifact_path}", + "/Workspace/${workspace.state_path}", + } + + err := b.Config.Mutate(func(root dyn.Value) (dyn.Value, error) { + // Walk through the bundle configuration, check all the string leafs and + // see if any of the prefixes are used in the remote path. + return dyn.Walk(root, func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + vv, ok := v.AsString() + if !ok { + return v, nil + } + + for _, path := range paths { + if strings.Contains(vv, path) { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Warning, + Summary: fmt.Sprintf("%s used in the remote path %s. Please change to use %s instead", path, vv, strings.ReplaceAll(vv, "/Workspace/", "")), + Locations: v.Locations(), + Paths: []dyn.Path{p}, + }) + + // Remove the workspace prefix from the string. + return dyn.NewValue(strings.ReplaceAll(vv, "/Workspace/", ""), v.Locations()), nil + } + } + + return v, nil + }) + }) + + if err != nil { + return diag.FromErr(err) + } + + return diags +} diff --git a/bundle/config/validate/no_workspace_prefix_used_test.go b/bundle/config/mutator/rewrite_workspace_prefix_test.go similarity index 74% rename from bundle/config/validate/no_workspace_prefix_used_test.go rename to bundle/config/mutator/rewrite_workspace_prefix_test.go index 7db8a3467c..d0c97e0f11 100644 --- a/bundle/config/validate/no_workspace_prefix_used_test.go +++ b/bundle/config/mutator/rewrite_workspace_prefix_test.go @@ -1,4 +1,4 @@ -package validate +package mutator import ( "context" @@ -61,7 +61,7 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { }, } - diags := bundle.Apply(context.Background(), b, NoWorkspacePrefixUsed()) + diags := bundle.Apply(context.Background(), b, RewriteWorkspacePrefix()) require.Len(t, diags, 3) expectedErrors := map[string]bool{ @@ -71,8 +71,15 @@ func TestNoWorkspacePrefixUsed(t *testing.T) { } for _, d := range diags { - require.Equal(t, d.Severity, diag.Error) + require.Equal(t, d.Severity, diag.Warning) require.Contains(t, expectedErrors, d.Summary) delete(expectedErrors, d.Summary) } + + require.Equal(t, "${workspace.root_path}/file1.py", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[0].SparkPythonTask.PythonFile) + require.Equal(t, "${workspace.file_path}/notebook1", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[1].NotebookTask.NotebookPath) + 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"].JobSettings.Tasks[2].NotebookTask.NotebookPath) + require.Equal(t, "${workspace.artifact_path}/jar2.jar", b.Config.Resources.Jobs["test_job"].JobSettings.Tasks[2].Libraries[0].Jar) + } diff --git a/bundle/config/validate/no_workspace_prefix_used.go b/bundle/config/validate/no_workspace_prefix_used.go deleted file mode 100644 index 2e85395d89..0000000000 --- a/bundle/config/validate/no_workspace_prefix_used.go +++ /dev/null @@ -1,60 +0,0 @@ -package validate - -import ( - "context" - "fmt" - "strings" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" -) - -type noWorkspacePrefixUsed struct{} - -// NoWorkspacePrefixUsed ensures that no workspace prefix plus workspace path variable is used in the remote path. -func NoWorkspacePrefixUsed() bundle.Mutator { - return &noWorkspacePrefixUsed{} -} - -func (m *noWorkspacePrefixUsed) Name() string { - return "validate:no_workspace_prefix_used" -} - -func (m *noWorkspacePrefixUsed) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - diags := diag.Diagnostics{} - paths := []string{ - "/Workspace/${workspace.root_path}", - "/Workspace/${workspace.file_path}", - "/Workspace/${workspace.artifact_path}", - "/Workspace/${workspace.state_path}", - } - - // Walk through the bundle configuration, check all the string leafs and - // see if any of the prefixes are used in the remote path. - _, err := dyn.Walk(b.Config.Value(), func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - vv, ok := v.AsString() - if !ok { - return v, nil - } - - for _, path := range paths { - if strings.Contains(vv, path) { - diags = append(diags, diag.Diagnostic{ - Severity: diag.Error, - Summary: fmt.Sprintf("%s used in the remote path %s. Please change to use %s instead", path, vv, strings.ReplaceAll(vv, "/Workspace/", "")), - Locations: v.Locations(), - Paths: []dyn.Path{p}, - }) - } - } - - return v, nil - }) - - if err != nil { - return diag.FromErr(err) - } - - return diags -} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 4bc2d1a046..02b94be754 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -47,7 +47,7 @@ func Initialize() bundle.Mutator { // This mutator needs to be run before variable interpolation because it // searches for strings with variable references in them. - validate.NoWorkspacePrefixUsed(), + mutator.RewriteWorkspacePrefix(), mutator.SetVariables(), // Intentionally placed before ResolveVariableReferencesInLookup, ResolveResourceReferences,