diff --git a/bundle/config/mutator/cleanup_targets.go b/bundle/config/mutator/cleanup_targets.go new file mode 100644 index 00000000..870b5be9 --- /dev/null +++ b/bundle/config/mutator/cleanup_targets.go @@ -0,0 +1,29 @@ +package mutator + +import ( + "context" + "fmt" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/diag" +) + +type cleanupTargets struct { + name string +} + +// CleanupTargets cleans up configuration properties before the configuration +// is reported by the 'bundle summary' command. +func CleanupTargets() bundle.Mutator { + return &cleanupTargets{} +} + +func (m *cleanupTargets) Name() string { + return fmt.Sprintf("Cleanup(%s)", m.name) +} + +func (m *cleanupTargets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + b.Config.Targets = nil + b.Config.Environments = nil + return nil +} diff --git a/bundle/config/mutator/process_target_mode.go b/bundle/config/mutator/process_target_mode.go index 92ed2868..8bcff0ca 100644 --- a/bundle/config/mutator/process_target_mode.go +++ b/bundle/config/mutator/process_target_mode.go @@ -130,8 +130,17 @@ 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. + 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.Errorf("target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed") } return nil } @@ -148,6 +157,14 @@ func isRunAsSet(r config.Resources) bool { return true } +func isExplicitRootSet(b *bundle.Bundle) bool { + targetConfig := b.Config.Targets[b.Config.Bundle.Target] + if targetConfig.Workspace == nil { + return false + } + return targetConfig.Workspace.RootPath != "" +} + func (m *processTargetMode) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { 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 1c8671b4..804371c0 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -32,6 +32,9 @@ func mockBundle(mode config.Mode) *bundle.Bundle { Branch: "main", }, }, + Targets: map[string]*config.Target{ + "": {}, + }, Workspace: config.Workspace{ CurrentUser: &config.User{ ShortName: "lennart", @@ -277,14 +280,14 @@ func TestProcessTargetModeProduction(t *testing.T) { b := mockBundle(config.Production) diags := validateProductionMode(context.Background(), b, false) - require.ErrorContains(t, diags.Error(), "run_as") + require.ErrorContains(t, diags.Error(), "target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed") 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(), "production") + require.ErrorContains(t, diags.Error(), "target with 'mode: production' must specify explicit 'workspace.root_path' to make sure only one copy is deployed") permissions := []resources.Permission{ { @@ -326,6 +329,21 @@ func TestProcessTargetModeProductionOkForPrincipal(t *testing.T) { require.NoError(t, diags.Error()) } +func TestProcessTargetModeProductionOkWithRootPath(t *testing.T) { + b := mockBundle(config.Production) + + // Our target has all kinds of problems when not using service principals ... + diags := validateProductionMode(context.Background(), b, false) + 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", + } + diags = validateProductionMode(context.Background(), b, false) + require.NoError(t, diags.Error()) +} + // Make sure that we have test coverage for all resource types func TestAllResourcesMocked(t *testing.T) { b := mockBundle(config.Development) diff --git a/bundle/config/mutator/select_target.go b/bundle/config/mutator/select_target.go index 178686b6..2333c97d 100644 --- a/bundle/config/mutator/select_target.go +++ b/bundle/config/mutator/select_target.go @@ -49,9 +49,5 @@ func (m *selectTarget) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnosti // TODO: remove when Environments section is not supported anymore. b.Config.Bundle.Environment = b.Config.Bundle.Target - // Clear targets after loading. - b.Config.Targets = nil - b.Config.Environments = nil - return nil } diff --git a/cmd/bundle/summary.go b/cmd/bundle/summary.go index 5a64b46c..b3493a81 100644 --- a/cmd/bundle/summary.go +++ b/cmd/bundle/summary.go @@ -8,6 +8,7 @@ import ( "path/filepath" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/deploy/terraform" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" @@ -60,7 +61,7 @@ func newSummaryCommand() *cobra.Command { } } - diags = bundle.Apply(ctx, b, terraform.Load()) + diags = bundle.Apply(ctx, b, bundle.Seq(terraform.Load(), mutator.CleanupTargets())) if err := diags.Error(); err != nil { return err } diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 496d5d2b..88b16922 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" "github.com/databricks/cli/bundle/config/validate" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/bundle/render" @@ -65,6 +66,7 @@ func newValidateCommand() *cobra.Command { return nil case flags.OutputJSON: + bundle.Apply(ctx, b, mutator.CleanupTargets()) return renderJsonOutput(cmd, b, diags) default: return fmt.Errorf("unknown output type %s", root.OutputType(cmd))