Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions acceptance/bundle/state/bad_env/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,3 @@ Error: unexpected setting for DATABRICKS_BUNDLE_ENGINE="abc" (expected 'terrafor

>>> musterr [CLI] bundle destroy --auto-approve
Error: unexpected setting for DATABRICKS_BUNDLE_ENGINE="abc" (expected 'terraform' or 'direct')

>>> musterr [CLI] bundle validate
Error: unexpected setting for DATABRICKS_BUNDLE_ENGINE="abc" (expected 'terraform' or 'direct')

Found 1 error
1 change: 0 additions & 1 deletion acceptance/bundle/state/bad_env/script
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ trace musterr $CLI bundle plan
trace musterr $CLI bundle deploy
trace musterr $CLI bundle summary
trace musterr $CLI bundle destroy --auto-approve
trace musterr $CLI bundle validate
16 changes: 3 additions & 13 deletions acceptance/bundle/state/engine_mismatch/output.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,12 @@ Available state files:

[TEST_TMP_DIR]/.databricks/bundle/default/terraform/terraform.tfstate: local terraform state serial=1 lineage="test-lineage"

=== Env var confirms config, both mismatch state: warning expected
=== Invalid env var with config set: no error, config takes priority

>>> DATABRICKS_BUNDLE_ENGINE=direct [CLI] bundle debug states
Warning: Deployment engine "direct" configured in DATABRICKS_BUNDLE_ENGINE environment variable does not match the existing state (engine "terraform"). Using "terraform" engine from the existing state.
>>> DATABRICKS_BUNDLE_ENGINE=bla [CLI] bundle debug states
Warning: Deployment engine "direct" configured in bundle.engine setting at [TEST_TMP_DIR]/databricks.yml:3:11 does not match the existing state (engine "terraform"). Using "terraform" engine from the existing state.

Available state files:
- [TEST_TMP_DIR]/.databricks/bundle/default/terraform/terraform.tfstate: local terraform state serial=1 lineage="test-lineage"

[TEST_TMP_DIR]/.databricks/bundle/default/terraform/terraform.tfstate: local terraform state serial=1 lineage="test-lineage"

=== Env var overrides config to match state: no warning expected

>>> DATABRICKS_BUNDLE_ENGINE=terraform [CLI] bundle debug states
[TEST_TMP_DIR]/.databricks/bundle/default/terraform/terraform.tfstate: local terraform state serial=1 lineage="test-lineage"

=== Invalid env var: error expected

>>> musterr [CLI] bundle debug states
Error: unexpected setting for DATABRICKS_BUNDLE_ENGINE="bla" (expected 'terraform' or 'direct')
11 changes: 2 additions & 9 deletions acceptance/bundle/state/engine_mismatch/script
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,5 @@ title 'Config mismatch with state: warning expected\n'
unset DATABRICKS_BUNDLE_ENGINE
trace $CLI bundle debug states

title 'Env var confirms config, both mismatch state: warning expected\n'
trace DATABRICKS_BUNDLE_ENGINE=direct $CLI bundle debug states

title 'Env var overrides config to match state: no warning expected\n'
trace DATABRICKS_BUNDLE_ENGINE=terraform $CLI bundle debug states

title 'Invalid env var: error expected\n'
export DATABRICKS_BUNDLE_ENGINE=bla
trace musterr $CLI bundle debug states
title 'Invalid env var with config set: no error, config takes priority\n'
trace DATABRICKS_BUNDLE_ENGINE=bla $CLI bundle debug states
12 changes: 1 addition & 11 deletions bundle/config/engine/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,11 @@ func FromEnv(ctx context.Context) (EngineType, error) {

// EngineSetting represents a requested engine type along with the source of the request.
type EngineSetting struct {
Type EngineType // effective: env var if set, else config
Type EngineType // effective resolved engine
Source string // human-readable source of Type
ConfigType EngineType // from bundle config (EngineNotSet if not configured)
}

// SettingFromEnv returns an EngineSetting from the environment variable.
// ConfigType is left as EngineNotSet and populated later by ResolveEngineSetting.
func SettingFromEnv(ctx context.Context) (EngineSetting, error) {
e, err := FromEnv(ctx)
if err != nil {
return EngineSetting{}, err
}
return EngineSetting{Type: e, Source: EnvVar + " environment variable"}, nil
}

func (e EngineType) ThisOrDefault() EngineType {
if e == EngineNotSet {
return Default
Expand Down
20 changes: 9 additions & 11 deletions bundle/config/engine/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,21 @@ import (
"github.com/stretchr/testify/require"
)

func TestSettingFromEnv(t *testing.T) {
ctx := t.Context()
ctx = env.Set(ctx, EnvVar, "direct")
req, err := SettingFromEnv(ctx)
func TestFromEnv(t *testing.T) {
ctx := env.Set(t.Context(), EnvVar, "direct")
e, err := FromEnv(ctx)
require.NoError(t, err)
assert.Equal(t, EngineDirect, req.Type)
assert.Contains(t, req.Source, EnvVar)
assert.Equal(t, EngineDirect, e)
}

func TestSettingFromEnvNotSet(t *testing.T) {
req, err := SettingFromEnv(t.Context())
func TestFromEnvNotSet(t *testing.T) {
e, err := FromEnv(t.Context())
require.NoError(t, err)
assert.Equal(t, EngineNotSet, req.Type)
assert.Equal(t, EngineNotSet, e)
}

func TestSettingFromEnvInvalid(t *testing.T) {
func TestFromEnvInvalid(t *testing.T) {
ctx := env.Set(t.Context(), EnvVar, "invalid")
_, err := SettingFromEnv(ctx)
_, err := FromEnv(ctx)
assert.Error(t, err)
}
2 changes: 1 addition & 1 deletion bundle/internal/schema/annotations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ github.com/databricks/cli/bundle/config.Bundle:
The definition of the bundle deployment. For supported attributes see [\_](/dev-tools/bundles/deployment-modes.md).
"engine":
"description": |-
The deployment engine to use. Valid values are `terraform` and `direct`. Can be overridden with the `DATABRICKS_BUNDLE_ENGINE` environment variable.
The deployment engine to use. Valid values are `terraform` and `direct`. Takes priority over `DATABRICKS_BUNDLE_ENGINE` environment variable. Default is "terraform".
"git":
"description": |-
The Git version control details that are associated with your bundle.
Expand Down
2 changes: 1 addition & 1 deletion bundle/schema/jsonschema.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bundle/schema/jsonschema_for_docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -2151,7 +2151,7 @@
"x-since-version": "v0.229.0"
},
"engine": {
"description": "The deployment engine to use. Valid values are `terraform` and `direct`. Can be overridden with the `DATABRICKS_BUNDLE_ENGINE` environment variable.",
"description": "The deployment engine to use. Valid values are `terraform` and `direct`. Takes priority over `DATABRICKS_BUNDLE_ENGINE` environment variable. Default is \"terraform\".",
"$ref": "#/$defs/github.com/databricks/cli/bundle/config/engine.EngineType"
},
"git": {
Expand Down
13 changes: 5 additions & 8 deletions cmd/bundle/generate/dashboard.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"time"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/engine"
"github.com/databricks/cli/bundle/generate"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/bundle/resources"
Expand Down Expand Up @@ -374,18 +373,16 @@ func (d *dashboard) initialize(ctx context.Context, b *bundle.Bundle) {
}

func (d *dashboard) runForResource(ctx context.Context, b *bundle.Bundle) {
envEngine, err := engine.SettingFromEnv(ctx)
if err != nil {
logdiag.LogError(ctx, err)
return
}

phases.Initialize(ctx, b)
if logdiag.HasError(ctx) {
return
}

requiredEngine := utils.ResolveEngineSetting(b, envEngine)
requiredEngine, err := utils.ResolveEngineSetting(ctx, b)
if err != nil {
logdiag.LogError(ctx, err)
return
}
ctx, stateDesc := statemgmt.PullResourcesState(ctx, b, statemgmt.AlwaysPull(true), requiredEngine)
if logdiag.HasError(ctx) {
return
Expand Down
11 changes: 4 additions & 7 deletions cmd/bundle/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"os"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/engine"
"github.com/databricks/cli/bundle/env"
"github.com/databricks/cli/bundle/phases"
"github.com/databricks/cli/bundle/resources"
Expand Down Expand Up @@ -133,11 +132,6 @@ Example usage:
cmd.Flags().BoolVar(&restart, "restart", false, "Restart the run if it is already running.")

cmd.RunE = func(cmd *cobra.Command, args []string) error {
envEngine, err := engine.SettingFromEnv(cmd.Context())
if err != nil {
return err
}

b, err := utils.ProcessBundle(cmd, utils.ProcessOptions{
SkipInitialize: true,
})
Expand Down Expand Up @@ -172,7 +166,10 @@ Example usage:
return executeScript(content, cmd, b)
}

requiredEngine := utils.ResolveEngineSetting(b, envEngine)
requiredEngine, err := utils.ResolveEngineSetting(ctx, b)
if err != nil {
return err
}
ctx, stateDesc := statemgmt.PullResourcesState(ctx, b, statemgmt.AlwaysPull(true), requiredEngine)
if logdiag.HasError(ctx) {
return root.ErrAlreadyPrinted
Expand Down
33 changes: 19 additions & 14 deletions cmd/bundle/utils/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
cmd.SetContext(ctx)
}

envEngine, err := engine.SettingFromEnv(ctx)
if err != nil {
return nil, nil, err
}

// Load bundle config and apply target
b := root.MustConfigureBundle(cmd)
if logdiag.HasError(ctx) {
Expand Down Expand Up @@ -157,7 +152,10 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
shouldReadState := opts.ReadState || opts.AlwaysPull || opts.InitIDs || opts.ErrorOnEmptyState || opts.PreDeployChecks || opts.Deploy || opts.ReadPlanPath != ""

if shouldReadState {
requiredEngine := ResolveEngineSetting(b, envEngine)
requiredEngine, err := ResolveEngineSetting(ctx, b)
if err != nil {
return b, nil, err
}

// PullResourcesState depends on stateFiler which needs b.Config.Workspace.StatePath which is set in phases.Initialize
ctx, stateDesc = statemgmt.PullResourcesState(ctx, b, statemgmt.AlwaysPull(opts.AlwaysPull), requiredEngine)
Expand Down Expand Up @@ -299,23 +297,30 @@ func ProcessBundleRet(cmd *cobra.Command, opts ProcessOptions) (*bundle.Bundle,
return b, stateDesc, nil
}

// ResolveEngineSetting combines the env var engine with the bundle config engine setting.
// Environment variable takes priority over config. ConfigType always reflects the config value.
func ResolveEngineSetting(b *bundle.Bundle, envSetting engine.EngineSetting) engine.EngineSetting {
// ResolveEngineSetting determines the effective engine setting by combining bundle config and env var.
// Priority: bundle.engine config > DATABRICKS_BUNDLE_ENGINE env var.
func ResolveEngineSetting(ctx context.Context, b *bundle.Bundle) (engine.EngineSetting, error) {
configEngine := b.Config.Bundle.Engine
if envSetting.Type != engine.EngineNotSet {
return engine.EngineSetting{Type: envSetting.Type, Source: envSetting.Source, ConfigType: configEngine}
}

if configEngine != engine.EngineNotSet {
source := "bundle.engine setting"
v := dyn.GetValue(b.Config.Value(), "bundle.engine")
if locs := v.Locations(); len(locs) > 0 {
loc := locs[0]
source = fmt.Sprintf("bundle.engine setting at %s:%d:%d", filepath.ToSlash(loc.File), loc.Line, loc.Column)
}
return engine.EngineSetting{Type: configEngine, Source: source, ConfigType: configEngine}
return engine.EngineSetting{Type: configEngine, Source: source, ConfigType: configEngine}, nil
}

envEngine, err := engine.FromEnv(ctx)
if err != nil {
return engine.EngineSetting{}, err
}
if envEngine != engine.EngineNotSet {
return engine.EngineSetting{Type: envEngine, Source: engine.EnvVar + " environment variable"}, nil
}
return engine.EngineSetting{}

return engine.EngineSetting{}, nil
}

func rejectDefinitions(ctx context.Context, b *bundle.Bundle) {
Expand Down
52 changes: 52 additions & 0 deletions cmd/bundle/utils/resolve_engine_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package utils

import (
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config"
"github.com/databricks/cli/bundle/config/engine"
"github.com/databricks/cli/libs/env"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestResolveEngineSettingConfigTakesPriority(t *testing.T) {
ctx := env.Set(t.Context(), engine.EnvVar, "terraform")
b := &bundle.Bundle{Config: config.Root{Bundle: config.Bundle{Engine: engine.EngineDirect}}}
result, err := ResolveEngineSetting(ctx, b)
require.NoError(t, err)
assert.Equal(t, engine.EngineDirect, result.Type)
assert.Equal(t, engine.EngineDirect, result.ConfigType)
}

func TestResolveEngineSettingEnvVarUsedWhenNoConfig(t *testing.T) {
ctx := env.Set(t.Context(), engine.EnvVar, "direct")
b := &bundle.Bundle{Config: config.Root{}}
result, err := ResolveEngineSetting(ctx, b)
require.NoError(t, err)
assert.Equal(t, engine.EngineDirect, result.Type)
assert.Contains(t, result.Source, engine.EnvVar)
}

func TestResolveEngineSettingNothingSet(t *testing.T) {
b := &bundle.Bundle{Config: config.Root{}}
result, err := ResolveEngineSetting(t.Context(), b)
require.NoError(t, err)
assert.Equal(t, engine.EngineNotSet, result.Type)
}

func TestResolveEngineSettingInvalidEnvVar(t *testing.T) {
ctx := env.Set(t.Context(), engine.EnvVar, "invalid")
b := &bundle.Bundle{Config: config.Root{}}
_, err := ResolveEngineSetting(ctx, b)
assert.Error(t, err)
}

func TestResolveEngineSettingInvalidEnvVarIgnoredWhenConfigSet(t *testing.T) {
ctx := env.Set(t.Context(), engine.EnvVar, "invalid")
b := &bundle.Bundle{Config: config.Root{Bundle: config.Bundle{Engine: engine.EngineDirect}}}
result, err := ResolveEngineSetting(ctx, b)
require.NoError(t, err)
assert.Equal(t, engine.EngineDirect, result.Type)
}
Loading