Encourage the use of root_path in production to ensure single deployment

This commit is contained in:
Lennart Kats 2024-08-22 22:09:18 +02:00
parent 7fe08c2386
commit 2dad625b84
No known key found for this signature in database
GPG Key ID: 1EB8B57673197023
6 changed files with 72 additions and 9 deletions

View File

@ -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
}

View File

@ -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:

View File

@ -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)

View File

@ -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
}

View File

@ -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
}

View File

@ -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))