Skip to content

Commit

Permalink
Add stricter validations
Browse files Browse the repository at this point in the history
  • Loading branch information
lennartkats-db committed Jun 14, 2024
1 parent 7323d02 commit 4dc5f41
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 13 deletions.
38 changes: 31 additions & 7 deletions bundle/config/mutator/process_target_mode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)})

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

Expand Down Expand Up @@ -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)
Expand Down
45 changes: 39 additions & 6 deletions bundle/config/mutator/process_target_mode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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/[email protected]/.bundle/x/y/state"
b.Config.Workspace.StatePath = "/Users/[email protected]/.bundle/x/y/state"
b.Config.Workspace.FilePath = "/Users/[email protected]/.bundle/x/y/files"
b.Config.Workspace.ArtifactPath = "/Users/[email protected]/.bundle/x/y/artifacts"
diags = validateDevelopmentMode(b)
require.NoError(t, diags.Error())
}

func TestProcessTargetModeDefault(t *testing.T) {
b := mockBundle("")

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 4dc5f41

Please sign in to comment.