Merge remote-tracking branch 'origin/cp-encourage-root-path' into cp-encourage-root-path

bundle/config/mutator/process_target_mode.go

@@ -2,6 +2,7 @@ package mutator
 
 import (
 	"context"
+	"fmt"
 	"strings"
 
 	"github.com/databricks/cli/bundle"
@@ -132,15 +133,19 @@ func validateProductionMode(ctx context.Context, b *bundle.Bundle, isPrincipalUs
 
 	// 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 := fmt.Sprintf(
+		"set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Users/%s/.bundle/${bundle.name}/${bundle.target}",
+		b.Config.Workspace.CurrentUser.UserName,
+	)
 	if !isExplicitRootSet(b) {
 		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.Warningf("target with 'mode: production' should specify explicit 'workspace.root_path' to make sure only one copy is deployed")
+			return diag.Warningf("target with 'mode: production' should " + advice)
 		}
-		return diag.Errorf("target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed")
+		return diag.Errorf("target with 'mode: production' must " + advice)
 	}
 	return nil
 }
@@ -158,10 +163,10 @@ func isRunAsSet(r config.Resources) bool {
 }
 
 func isExplicitRootSet(b *bundle.Bundle) bool {
-	targetConfig, ok := b.Config.Targets[b.Config.Bundle.Target]
-	if !ok || targetConfig == nil {
+	if b.Config.Targets == nil {
 		return false
 	}
+	targetConfig := b.Config.Targets[b.Config.Bundle.Target]
 	if targetConfig.Workspace == nil {
 		return false
 	}

bundle/config/mutator/process_target_mode_test.go

@@ -32,9 +32,6 @@ func mockBundle(mode config.Mode) *bundle.Bundle {
 					Branch:    "main",
 				},
 			},
-			Targets: map[string]*config.Target{
-				"": {},
-			},
 			Workspace: config.Workspace{
 				CurrentUser: &config.User{
 					ShortName: "lennart",
@@ -280,14 +277,14 @@ func TestProcessTargetModeProduction(t *testing.T) {
 	b := mockBundle(config.Production)
 
 	diags := validateProductionMode(context.Background(), b, false)
-	require.ErrorContains(t, diags.Error(), "target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed")
+	require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}")
 
 	b.Config.Workspace.StatePath = "/Shared/.bundle/x/y/state"
 	b.Config.Workspace.ArtifactPath = "/Shared/.bundle/x/y/artifacts"
 	b.Config.Workspace.FilePath = "/Shared/.bundle/x/y/files"
 
 	diags = validateProductionMode(context.Background(), b, false)
-	require.ErrorContains(t, diags.Error(), "target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed")
+	require.ErrorContains(t, diags.Error(), "target with 'mode: production' must set 'workspace.root_path' to make sure only one copy is deployed. A common practice is to use a username or principal name in this path, i.e. root_path: /Users/lennart@company.com/.bundle/${bundle.name}/${bundle.target}")
 
 	permissions := []resources.Permission{
 		{
@@ -337,8 +334,12 @@ func TestProcessTargetModeProductionOkWithRootPath(t *testing.T) {
 	require.Error(t, diags.Error())
 
 	// ... but we're okay if we specify a root path
-	b.Config.Targets[""].Workspace = &config.Workspace{
-		RootPath: "some-root-path",
+	b.Config.Targets = map[string]*config.Target{
+		"": {
+			Workspace: &config.Workspace{
+				RootPath: "some-root-path",
+			},
+		},
 	}
 	diags = validateProductionMode(context.Background(), b, false)
 	require.NoError(t, diags.Error())