From b2fa0d83d3131ee221c3704aa55313e9ff19fc43 Mon Sep 17 00:00:00 2001 From: Lennart Kats Date: Mon, 13 Jan 2025 10:56:43 +0100 Subject: [PATCH] WIP --- bundle/config/mutator/process_target_mode.go | 37 ++++++++++++++++++- .../mutator/process_target_mode_test.go | 5 ++- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 44b53681d..9f8fd318d 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -2,10 +2,12 @@ package mutator import ( "context" + "fmt" "strings" "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/iamutil" @@ -146,8 +148,39 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs } } - if !isPrincipalUsed && !isRunAsSet(r) { - return diag.Errorf("'run_as' must be set for all jobs when using 'mode: production'") + // We need to verify that there is only a single deployment of the current target. + // The best way to enforce this is to explicitly set root_path. + advice := "set 'workspace.root_path' to make sure only one copy is deployed" + adviceDetail := fmt.Sprintf( + "A common practice is to use a username or principal name in this path, i.e. use\n"+ + "\n"+ + " root_path: /Workspace/Users/%s/.bundle/${bundle.name}/${bundle.target}", + b.Config.Workspace.CurrentUser.UserName, + ) + if !isExplicitRootSet(b) { + cmdio.LogString(ctx, fmt.Sprintf("root_path is not set: %s", b.Config.Bundle.Target)) + if isRunAsSet(r) || isPrincipalUsed { + // Just setting run_as is not enough to guarantee a single deployment, + // and neither is setting a principal. + // We only show a warning for these cases since we didn't historically + // report an error for them. + return diag.Diagnostics{ + { + Severity: diag.Recommendation, + Summary: "target with 'mode: production' should " + advice, + Detail: adviceDetail, + Locations: b.Config.GetLocations("targets." + b.Config.Bundle.Target), + }, + } + } + return diag.Diagnostics{ + { + Severity: diag.Error, + Summary: "target with 'mode: production' must " + advice, + Detail: adviceDetail, + Locations: b.Config.GetLocations("targets." + b.Config.Bundle.Target), + }, + } } return nil } diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 097c092a6..d46e64895 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -321,7 +321,8 @@ func TestProcessTargetModeProduction(t *testing.T) { b := mockBundle(config.Production) diags := validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, diags.Error(), "run_as") + assert.ErrorContains(t, diags.Error(), "A common practice is to use a username or principal name in this path, i.e. use\n\n root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}") + assert.NotNil(t, diags[0].Locations) b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state" b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts" @@ -329,7 +330,7 @@ func TestProcessTargetModeProduction(t *testing.T) { b.Config.Workspace.ResourcePath = "/Shared/.bundle/x/y/resources" diags = validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, diags.Error(), "production") + assert.ErrorContains(t, diags.Error(), "A common practice is to use a username or principal name in this path, i.e. use\n\n root_path: /Workspace/Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}") permissions := []resources.Permission{ {