diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index a2385e1e05..76397ccba4 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -28,7 +28,7 @@ func (m *processTargetMode) Name() string { // Mark all resources as being for 'development' purposes, i.e. // changing their their name, adding tags, and (in the future) // marking them as 'hidden' in the UI. -func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { +func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) error { if !b.Config.Bundle.Deployment.Lock.IsExplicitlyEnabled() { log.Infof(ctx, "Development mode: disabling deployment lock since bundle.deployment.lock.enabled is not set to true") err := setConfig(b, "bundle.deployment.lock.enabled", false) @@ -78,14 +78,14 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagno return nil } -func setConfig(b *bundle.Bundle, path string, value any) diag.Diagnostics { +func setConfig(b *bundle.Bundle, path string, value any) error { err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { return dyn.Set(v, path, dyn.V(value)) }) - return diag.FromErr(err) + return err } -func setConfigMapping(b *bundle.Bundle, path string, key string, value string) diag.Diagnostics { +func setConfigMapping(b *bundle.Bundle, path string, key string, value string) error { err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) { newMapping := dyn.V(map[string]dyn.Value{key: dyn.V(value)}) @@ -100,13 +100,33 @@ func setConfigMapping(b *bundle.Bundle, path string, key string, value string) d } return dyn.Set(v, path, merged) }) - return diag.FromErr(err) + return err } func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics { + t := b.Config.Transform + u := b.Config.Workspace.CurrentUser + + // Make sure everything is paused by default to avoid surprises + if t.DefaultTriggerPauseStatus == config.Unpaused { + return diag.Diagnostics{{ + Severity: diag.Error, + Summary: "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default", + Location: b.Config.GetLocation("transform.default_trigger_pause_status"), + }} + } + + // Make sure this development copy has unique names and paths to avoid conflicts if path := findNonUserPath(b); path != "" { return diag.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path) } + if t.Prefix != "" && !strings.Contains(t.Prefix, u.ShortName) && !strings.Contains(t.Prefix, u.UserName) { + return diag.Diagnostics{{ + Severity: diag.Error, + Summary: "prefix should contain the current username or ${workspace.current_user.short_name} to ensure uniqueness when using 'mode: development'", + Location: b.Config.GetLocation("transform.prefix"), + }} + } return nil } @@ -163,10 +183,14 @@ func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Di switch b.Config.Bundle.Mode { case config.Development: diags := validateDevelopmentMode(b) - if diags != nil { + if diags.HasError() { return diags } - return transformDevelopmentMode(ctx, b) + err := transformDevelopmentMode(ctx, b) + if err != nil { + return diag.FromErr(err) + } + return diags case config.Production: isPrincipal := auth.IsServicePrincipal(b.Config.Workspace.CurrentUser.UserName) return validateProductionMode(ctx, b, isPrincipal) diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index 52c34e3495..6a49c56cae 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -9,6 +9,7 @@ import ( "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/cli/libs/tags" sdkconfig "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -200,6 +201,41 @@ func TestProcessTargetModeDevelopmentTagNormalizationForGcp(t *testing.T) { assert.Equal(t, "Hello_world", b.Config.Resources.Jobs["job1"].Tags["dev"]) } +func TestValidateDevelopmentMode(t *testing.T) { + // Test with a valid development mode bundle + b := mockBundle(config.Development) + diags := validateDevelopmentMode(b) + require.NoError(t, diags.Error()) + + // Test with a bundle that has a non-user path + b.Config.Workspace.RootPath = "/Shared/.bundle/x/y/state" + diags = validateDevelopmentMode(b) + require.ErrorContains(t, diags.Error(), "root_path") + + // Test with a bundle that has an unpaused trigger pause status + b = mockBundle(config.Development) + b.Config.Transform.DefaultTriggerPauseStatus = config.Unpaused + diags = validateDevelopmentMode(b) + require.ErrorContains(t, diags.Error(), "UNPAUSED") + + // Test with a bundle that has a prefix not containing the username or short name + b = mockBundle(config.Development) + b.Config.Transform.Prefix = "[prod]" + diags = validateDevelopmentMode(b) + require.Len(t, diags, 1) + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Contains(t, diags[0].Summary, "") + + // Test with a bundle that has valid user paths + b = mockBundle(config.Development) + b.Config.Workspace.RootPath = "/Users/lennart@company.com/.bundle/x/y/state" + b.Config.Workspace.StatePath = "/Users/lennart@company.com/.bundle/x/y/state" + b.Config.Workspace.FilePath = "/Users/lennart@company.com/.bundle/x/y/files" + b.Config.Workspace.ArtifactPath = "/Users/lennart@company.com/.bundle/x/y/artifacts" + diags = validateDevelopmentMode(b) + require.NoError(t, diags.Error()) +} + func TestProcessTargetModeDefault(t *testing.T) { b := mockBundle("") @@ -337,13 +373,13 @@ func TestDisableLockingDisabled(t *testing.T) { func TestPrefixAlreadySet(t *testing.T) { b := mockBundle(config.Development) - b.Config.Transform.Prefix = "custom_prefix_" + b.Config.Transform.Prefix = "custom_lennart_deploy_" m := bundle.Seq(ProcessTargetMode(), ApplyTransforms()) diags := bundle.Apply(context.Background(), b, m) require.NoError(t, diags.Error()) - assert.Equal(t, "custom_prefix_job1", b.Config.Resources.Jobs["job1"].Name) + assert.Equal(t, "custom_lennart_deploy_job1", b.Config.Resources.Jobs["job1"].Name) } func TestTagsAlreadySet(t *testing.T) { @@ -407,10 +443,7 @@ func TestTriggerPauseStatusWhenUnpaused(t *testing.T) { m := bundle.Seq(ProcessTargetMode(), ApplyTransforms()) diags := bundle.Apply(context.Background(), b, m) - require.NoError(t, diags.Error()) - - // Pause status should take the value from the override above - assert.Equal(t, jobs.PauseStatusUnpaused, b.Config.Resources.Jobs["job3"].Trigger.PauseStatus) + require.ErrorContains(t, diags.Error(), "target with 'mode: development' cannot set trigger pause status to UNPAUSED by default") } func TestPipelinesDevelopmentDisabled(t *testing.T) {