diff --git a/bundle/config/mutator/prepend_workspace_prefix.go b/bundle/config/mutator/prepend_workspace_prefix.go index 8ba083e22..8e01dfe71 100644 --- a/bundle/config/mutator/prepend_workspace_prefix.go +++ b/bundle/config/mutator/prepend_workspace_prefix.go @@ -24,7 +24,6 @@ func (m *prependWorkspacePrefix) Name() string { var skipPrefixes = []string{ "/Workspace", "/Volumes", - "/Shared", } func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { @@ -44,12 +43,6 @@ func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di return dyn.InvalidValue, fmt.Errorf("expected string, got %s", v.Kind()) } - for _, prefix := range skipPrefixes { - if path == prefix { - return pv, nil - } - } - for _, prefix := range skipPrefixes { if strings.HasPrefix(path, prefix) { return pv, nil diff --git a/bundle/config/mutator/prepend_workspace_prefix_test.go b/bundle/config/mutator/prepend_workspace_prefix_test.go index 20f69fefc..287c694d3 100644 --- a/bundle/config/mutator/prepend_workspace_prefix_test.go +++ b/bundle/config/mutator/prepend_workspace_prefix_test.go @@ -21,7 +21,7 @@ func TestPrependWorkspacePrefix(t *testing.T) { }, { path: "/Shared/test", - expected: "/Shared/test", + expected: "/Workspace/Shared/test", }, { path: "/Workspace/Users/test", diff --git a/bundle/config/validate/no_workspace_prefix_used.go b/bundle/config/validate/no_workspace_prefix_used.go new file mode 100644 index 000000000..dd774dd1d --- /dev/null +++ b/bundle/config/validate/no_workspace_prefix_used.go @@ -0,0 +1,58 @@ +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{} + prefixes := []string{ + "/Workspace/${workspace.root_path}", + "/Workspace/${workspace.file_path}", + "/Workspace/${workspace.artifact_path}", + "/Workspace/${workspace.state_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 _, prefix := range prefixes { + if strings.HasPrefix(vv, prefix) { + diags = append(diags, diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("%s used in the remote path %s. Please change to use %s instead", prefix, 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/config/validate/no_workspace_prefix_used_test.go b/bundle/config/validate/no_workspace_prefix_used_test.go new file mode 100644 index 000000000..7db8a3467 --- /dev/null +++ b/bundle/config/validate/no_workspace_prefix_used_test.go @@ -0,0 +1,78 @@ +package validate + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/databricks-sdk-go/service/compute" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/require" +) + +func TestNoWorkspacePrefixUsed(t *testing.T) { + b := &bundle.Bundle{ + Config: config.Root{ + Workspace: config.Workspace{ + RootPath: "/Workspace/Users/test", + ArtifactPath: "/Workspace/Users/test/artifacts", + FilePath: "/Workspace/Users/test/files", + StatePath: "/Workspace/Users/test/state", + }, + + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "test_job": { + JobSettings: &jobs.JobSettings{ + Tasks: []jobs.Task{ + { + SparkPythonTask: &jobs.SparkPythonTask{ + PythonFile: "/Workspace/${workspace.root_path}/file1.py", + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "/Workspace/${workspace.file_path}/notebook1", + }, + Libraries: []compute.Library{ + { + Jar: "/Workspace/${workspace.artifact_path}/jar1.jar", + }, + }, + }, + { + NotebookTask: &jobs.NotebookTask{ + NotebookPath: "${workspace.file_path}/notebook2", + }, + Libraries: []compute.Library{ + { + Jar: "${workspace.artifact_path}/jar2.jar", + }, + }, + }, + }, + }, + }, + }, + }, + }, + } + + diags := bundle.Apply(context.Background(), b, NoWorkspacePrefixUsed()) + require.Len(t, diags, 3) + + expectedErrors := map[string]bool{ + "/Workspace/${workspace.root_path} used in the remote path /Workspace/${workspace.root_path}/file1.py. Please change to use ${workspace.root_path}/file1.py instead": true, + "/Workspace/${workspace.file_path} used in the remote path /Workspace/${workspace.file_path}/notebook1. Please change to use ${workspace.file_path}/notebook1 instead": true, + "/Workspace/${workspace.artifact_path} used in the remote path /Workspace/${workspace.artifact_path}/jar1.jar. Please change to use ${workspace.artifact_path}/jar1.jar instead": true, + } + + for _, d := range diags { + require.Equal(t, d.Severity, diag.Error) + require.Contains(t, expectedErrors, d.Summary) + delete(expectedErrors, d.Summary) + } +} diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index efc5caa66..c57b96d72 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -47,7 +47,7 @@ type Workspace struct { // Remote workspace base path for deployment state, for artifacts, as synchronization target. // This defaults to "~/.bundle/${bundle.name}/${bundle.target}" where "~" expands to - // the current user's home directory in the workspace (e.g. `/Users/jane@doe.com`). + // the current user's home directory in the workspace (e.g. `/Workspace/Users/jane@doe.com`). RootPath string `json:"root_path,omitempty"` // Remote workspace path to synchronize local files to. diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index fbb47d8e9..c8b96579f 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -45,6 +45,8 @@ func Initialize() bundle.Mutator { mutator.DefineDefaultWorkspacePaths(), mutator.PrependWorkspacePrefix(), + validate.NoWorkspacePrefixUsed(), + mutator.SetVariables(), // Intentionally placed before ResolveVariableReferencesInLookup, ResolveResourceReferences, // ResolveVariableReferencesInComplexVariables and ResolveVariableReferences. diff --git a/internal/bundle/deploy_test.go b/internal/bundle/deploy_test.go index 736c880db..885435855 100644 --- a/internal/bundle/deploy_test.go +++ b/internal/bundle/deploy_test.go @@ -236,7 +236,7 @@ func TestAccDeployBasicBundleLogs(t *testing.T) { stdout, stderr := blackBoxRun(t, root, "bundle", "deploy") assert.Equal(t, strings.Join([]string{ - fmt.Sprintf("Uploading bundle files to /Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId), + fmt.Sprintf("Uploading bundle files to /Workspace/Users/%s/.bundle/%s/files...", currentUser.UserName, uniqueId), "Deploying resources...", "Updating deployment state...", "Deployment complete!\n", diff --git a/libs/template/templates/dbt-sql/template/{{.project_name}}/databricks.yml.tmpl b/libs/template/templates/dbt-sql/template/{{.project_name}}/databricks.yml.tmpl index f96ce4fe6..5594749a9 100644 --- a/libs/template/templates/dbt-sql/template/{{.project_name}}/databricks.yml.tmpl +++ b/libs/template/templates/dbt-sql/template/{{.project_name}}/databricks.yml.tmpl @@ -24,8 +24,8 @@ targets: mode: production workspace: host: {{workspace_host}} - # We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. - root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} + # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy. + root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} permissions: - {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}} level: CAN_MANAGE diff --git a/libs/template/templates/default-python/template/{{.project_name}}/databricks.yml.tmpl b/libs/template/templates/default-python/template/{{.project_name}}/databricks.yml.tmpl index 8544dc834..c42b822a8 100644 --- a/libs/template/templates/default-python/template/{{.project_name}}/databricks.yml.tmpl +++ b/libs/template/templates/default-python/template/{{.project_name}}/databricks.yml.tmpl @@ -21,8 +21,8 @@ targets: mode: production workspace: host: {{workspace_host}} - # We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. - root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} + # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy. + root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} permissions: - {{if is_service_principal}}service_principal{{else}}user{{end}}_name: {{user_name}} level: CAN_MANAGE diff --git a/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl b/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl index bf4690461..50e5ad97c 100644 --- a/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl +++ b/libs/template/templates/default-python/template/{{.project_name}}/resources/{{.project_name}}.pipeline.yml.tmpl @@ -15,4 +15,4 @@ resources: path: ../src/dlt_pipeline.ipynb configuration: - bundle.sourcePath: /Workspace/${workspace.file_path}/src + bundle.sourcePath: ${workspace.file_path}/src diff --git a/libs/template/templates/default-sql/template/{{.project_name}}/databricks.yml.tmpl b/libs/template/templates/default-sql/template/{{.project_name}}/databricks.yml.tmpl index 55c1aae4a..51d03e99a 100644 --- a/libs/template/templates/default-sql/template/{{.project_name}}/databricks.yml.tmpl +++ b/libs/template/templates/default-sql/template/{{.project_name}}/databricks.yml.tmpl @@ -41,8 +41,8 @@ targets: mode: production workspace: host: {{workspace_host}} - # We explicitly specify /Users/{{user_name}} to make sure we only have a single copy. - root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} + # We explicitly specify /Workspace/Users/{{user_name}} to make sure we only have a single copy. + root_path: /Workspace/Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} variables: warehouse_id: {{index ((regexp "[^/]+$").FindStringSubmatch .http_path) 0}} catalog: {{.default_catalog}}