From 8477465e882ec3d3f340073da7dafbbb1c15d04d Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Mon, 25 Dec 2023 11:03:36 +0100 Subject: [PATCH 1/4] Change recommended production deployment path from /Shared/ to ~/ --- bundle/config/mutator/process_target_mode.go | 44 +++---------------- .../{{.project_name}}/databricks.yml.tmpl | 14 +++--- 2 files changed, 15 insertions(+), 43 deletions(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 9fdb82a155..9ebb28d155 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -101,29 +101,25 @@ func transformDevelopmentMode(b *bundle.Bundle) error { } func validateDevelopmentMode(b *bundle.Bundle) error { - if path := findIncorrectPath(b, config.Development); path != "" { + if path := findNonUserPath(b); path != "" { return fmt.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) } return nil } -func findIncorrectPath(b *bundle.Bundle, mode config.Mode) string { +func findNonUserPath(b *bundle.Bundle) string { username := b.Config.Workspace.CurrentUser.UserName - containsExpected := true - if mode == config.Production { - containsExpected = false - } - if strings.Contains(b.Config.Workspace.RootPath, username) != containsExpected && b.Config.Workspace.RootPath != "" { + if b.Config.Workspace.RootPath != "" && !strings.Contains(b.Config.Workspace.RootPath, username) { return "root_path" } - if strings.Contains(b.Config.Workspace.StatePath, username) != containsExpected { + if b.Config.Workspace.StatePath != "" && !strings.Contains(b.Config.Workspace.StatePath, username) { return "state_path" } - if strings.Contains(b.Config.Workspace.FilePath, username) != containsExpected { + if b.Config.Workspace.FilePath != "" && !strings.Contains(b.Config.Workspace.FilePath, username) { return "file_path" } - if strings.Contains(b.Config.Workspace.ArtifactPath, username) != containsExpected { + if b.Config.Workspace.ArtifactPath != "" && !strings.Contains(b.Config.Workspace.ArtifactPath, username) { return "artifact_path" } return "" @@ -138,39 +134,13 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs r := b.Config.Resources for i := range r.Pipelines { if r.Pipelines[i].Development { - return fmt.Errorf("target with 'mode: production' cannot specify a pipeline with 'development: true'") + return fmt.Errorf("target with 'mode: production' cannot include a pipeline with 'development: true'") } } - if !isPrincipalUsed { - if path := findIncorrectPath(b, config.Production); path != "" { - message := "%s must not contain the current username when using 'mode: production'" - if path == "root_path" { - return fmt.Errorf(message+"\n tip: set workspace.root_path to a shared path such as /Shared/.bundle/${bundle.name}/${bundle.target}", path) - } else { - return fmt.Errorf(message, path) - } - } - - if !isRunAsSet(r) { - return fmt.Errorf("'run_as' must be set for all jobs when using 'mode: production'") - } - } return nil } -// Determines whether run_as is explicitly set for all resources. -// We do this in a best-effort fashion rather than check the top-level -// 'run_as' field because the latter is not required to be set. -func isRunAsSet(r config.Resources) bool { - for i := range r.Jobs { - if r.Jobs[i].RunAs == nil { - return false - } - } - return true -} - func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) error { switch b.Config.Bundle.Mode { case config.Development: 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 7860b32b9f..18e91c7a9f 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 @@ -32,11 +32,13 @@ targets: mode: production workspace: host: {{workspace_host}} - # We only have a single deployment copy for production, so we use a shared path. - root_path: /Shared/.bundle/prod/${bundle.name} - {{- if not is_service_principal}} + # We always use /Users/{{user_name}} for all resources to make sure we only have a single copy. + root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} run_as: - # This runs as {{user_name}} in production. We could also use a service principal here - # using service_principal_name (see https://docs.databricks.com/dev-tools/bundles/permissions.html). + {{- if is_service_principal}} + # This runs as {{user_name}} in production. We could also use a service principal here, + # see https://docs.databricks.com/dev-tools/bundles/permissions.html. user_name: {{user_name}} - {{end -}} + {{- else}} + service_principal_name: {{user_name}} + {{- end}} From ae3dcc185b6aad809d0adda6dbfe7a25dec489e8 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 27 Dec 2023 10:04:53 +0100 Subject: [PATCH 2/4] Restore run_as check --- bundle/config/mutator/process_target_mode.go | 15 +++++++++++++++ bundle/config/mutator/process_target_mode_test.go | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 9ebb28d155..592e3612cf 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -138,9 +138,24 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs } } + if !isPrincipalUsed && !isRunAsSet(r) { + return fmt.Errorf("'run_as' must be set for all jobs when using 'mode: production'") + } return nil } +// Determines whether run_as is explicitly set for all resources. +// We do this in a best-effort fashion rather than check the top-level +// 'run_as' field because the latter is not required to be set. +func isRunAsSet(r config.Resources) bool { + for i := range r.Jobs { + if r.Jobs[i].RunAs == nil { + return false + } + } + return true +} + func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) error { switch b.Config.Bundle.Mode { case config.Development: diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 8feab19116..f02d788657 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -205,7 +205,7 @@ func TestProcessTargetModeProduction(t *testing.T) { b := mockBundle(config.Production) err := validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, err, "state_path") + require.ErrorContains(t, err, "run_as") b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state" b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts" From ef4bf324bd5700d8635eeb5c7bf8eff1a799e507 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 27 Dec 2023 10:11:49 +0100 Subject: [PATCH 3/4] Add additional comment --- .../template/{{.project_name}}/databricks.yml.tmpl | 6 ++++++ 1 file changed, 6 insertions(+) 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 18e91c7a9f..92fc29b0b4 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 @@ -33,6 +33,12 @@ targets: workspace: host: {{workspace_host}} # We always use /Users/{{user_name}} for all resources to make sure we only have a single copy. + {{- /* + Internal note 2023-12: CLI versions v0.211.0 and before would show an error when using `mode: production` + with a path that doesn't say "/Shared". For now, we'll include an extra comment in the template + to explain that customers should update if they see this. + */}} + # If this path results in an error, please make sure you have a recent version of the CLI installed. root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} run_as: {{- if is_service_principal}} From 374c5f411b2920c916e815e518fd32364d880f33 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Wed, 27 Dec 2023 12:24:44 +0100 Subject: [PATCH 4/4] Fix conditional --- .../template/{{.project_name}}/databricks.yml.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 92fc29b0b4..ea432f8dbd 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 @@ -42,9 +42,9 @@ targets: root_path: /Users/{{user_name}}/.bundle/${bundle.name}/${bundle.target} run_as: {{- if is_service_principal}} + service_principal_name: {{user_name}} + {{- else}} # This runs as {{user_name}} in production. We could also use a service principal here, # see https://docs.databricks.com/dev-tools/bundles/permissions.html. user_name: {{user_name}} - {{- else}} - service_principal_name: {{user_name}} {{- end}}