From f90f6f9dccb24dcb45e03d529e51f15b34a3dbea Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 16:28:35 +0100 Subject: [PATCH 1/5] Keep an explicit false on optional bundle config bools Thirteen omitempty bool fields of CLI-defined config structs (variables.*.sensitive, bundle.deployment.lock.force, workspace.azure_use_msi and experimental_is_unified_host, artifacts.*.dynamic_version, targets.*.default, experimental.{immutable_folder,python_wheel_wrapper,use_legacy_run_as, skip_artifact_cleanup,skip_name_prefix_for_schema} and experimental.pydabs.enabled) become *bool, like lock.enabled already is. As plain bools, an explicit false was dropped by the typed-to-dynamic conversion that runs after every mutator: it never showed in the config, and a target could not override true with false (e.g. dynamic_version: false in a target lost to true at the root). Readers use config.IsExplicitlyEnabled, so behavior is otherwise unchanged; PyDABs keeps treating only enabled: true as configured. A unit test guards that no CLI-defined struct reintroduces an omitempty bool without ForceSendFields; bundle.force is skipped since it is readonly (set from --force, not by users). Co-authored-by: Isaac --- acceptance/bundle/refschema/output/root.txt | 46 ++++++++--------- .../validate/explicit_false/databricks.yml | 36 +++++++++++++ .../validate/explicit_false/out.test.toml | 2 + .../bundle/validate/explicit_false/output.txt | 27 ++++++++++ .../bundle/validate/explicit_false/script | 9 ++++ bundle/artifacts/build.go | 2 +- bundle/artifacts/upload.go | 3 +- bundle/bundle.go | 2 +- bundle/bundle_test.go | 2 +- bundle/config/artifact.go | 2 +- bundle/config/experimental.go | 12 ++--- bundle/config/lock.go | 2 +- bundle/config/mutator/apply_sync_mode.go | 2 +- bundle/config/mutator/apply_sync_mode_test.go | 10 ++-- .../mutator/artifacts_dynamic_version.go | 2 +- .../config/mutator/python/python_mutator.go | 2 +- .../mutator/python/python_mutator_test.go | 2 +- .../mutator/resourcemutator/apply_presets.go | 2 +- .../config/mutator/resourcemutator/run_as.go | 5 +- .../config/mutator/select_default_target.go | 3 +- .../mutator/select_default_target_test.go | 8 +-- bundle/config/optional_bools_test.go | 50 +++++++++++++++++++ bundle/config/target.go | 2 +- .../validate/sensitive_variable_references.go | 5 +- .../sensitive_variable_references_test.go | 6 +-- bundle/config/variable/variable.go | 2 +- bundle/config/workspace.go | 6 +-- bundle/deploy/lock/acquire.go | 3 +- bundle/direct/dresources/redact_config.go | 2 +- .../direct/dresources/redact_config_test.go | 12 ++--- bundle/libraries/remote_path.go | 3 +- bundle/run/job.go | 3 +- .../trampoline/conditional_transform_test.go | 2 +- bundle/trampoline/python_dbr_warning.go | 3 +- bundle/trampoline/python_dbr_warning_test.go | 2 +- bundle/trampoline/python_wheel.go | 3 +- cmd/apps/deploy_bundle_test.go | 9 ++-- cmd/apps/import.go | 2 +- cmd/bundle/debug/list_targets.go | 2 +- cmd/bundle/utils/utils.go | 2 +- cmd/bundle/utils/utils_test.go | 4 +- 41 files changed, 219 insertions(+), 85 deletions(-) create mode 100644 acceptance/bundle/validate/explicit_false/databricks.yml create mode 100644 acceptance/bundle/validate/explicit_false/out.test.toml create mode 100644 acceptance/bundle/validate/explicit_false/output.txt create mode 100644 acceptance/bundle/validate/explicit_false/script create mode 100644 bundle/config/optional_bools_test.go diff --git a/acceptance/bundle/refschema/output/root.txt b/acceptance/bundle/refschema/output/root.txt index dcbd46b145c..e3ccaceef5c 100644 --- a/acceptance/bundle/refschema/output/root.txt +++ b/acceptance/bundle/refschema/output/root.txt @@ -9,7 +9,7 @@ __locations.version int INPUT artifacts config.Artifacts INPUT artifacts.* *config.Artifact INPUT artifacts.*.build string INPUT -artifacts.*.dynamic_version bool INPUT +artifacts.*.dynamic_version *bool INPUT artifacts.*.executable exec.ExecutableType INPUT artifacts.*.files []config.ArtifactFile INPUT artifacts.*.files[*] config.ArtifactFile INPUT @@ -33,7 +33,7 @@ bundle.deployment.fail_on_active_runs bool INPUT bundle.deployment.latest_version_id int INPUT bundle.deployment.lock config.Lock INPUT bundle.deployment.lock.enabled *bool INPUT -bundle.deployment.lock.force bool INPUT +bundle.deployment.lock.force *bool INPUT bundle.engine engine.EngineType INPUT bundle.environment string INPUT bundle.force bool INPUT @@ -55,7 +55,7 @@ environments.* *config.Target INPUT environments.*.artifacts config.Artifacts INPUT environments.*.artifacts.* *config.Artifact INPUT environments.*.artifacts.*.build string INPUT -environments.*.artifacts.*.dynamic_version bool INPUT +environments.*.artifacts.*.dynamic_version *bool INPUT environments.*.artifacts.*.executable exec.ExecutableType INPUT environments.*.artifacts.*.files []config.ArtifactFile INPUT environments.*.artifacts.*.files[*] config.ArtifactFile INPUT @@ -79,7 +79,7 @@ environments.*.bundle.deployment.fail_on_active_runs bool INPUT environments.*.bundle.deployment.latest_version_id int INPUT environments.*.bundle.deployment.lock config.Lock INPUT environments.*.bundle.deployment.lock.enabled *bool INPUT -environments.*.bundle.deployment.lock.force bool INPUT +environments.*.bundle.deployment.lock.force *bool INPUT environments.*.bundle.engine engine.EngineType INPUT environments.*.bundle.environment string INPUT environments.*.bundle.force bool INPUT @@ -97,7 +97,7 @@ environments.*.bundle.terraform.exec_path string INPUT environments.*.bundle.uuid string INPUT environments.*.cluster_id string INPUT environments.*.compute_id string INPUT -environments.*.default bool INPUT +environments.*.default *bool INPUT environments.*.git config.Git INPUT environments.*.git.actual_branch string INPUT environments.*.git.branch string INPUT @@ -148,7 +148,7 @@ environments.*.variables.*.lookup.pipeline string INPUT environments.*.variables.*.lookup.query string INPUT environments.*.variables.*.lookup.service_principal string INPUT environments.*.variables.*.lookup.warehouse string INPUT -environments.*.variables.*.sensitive bool INPUT +environments.*.variables.*.sensitive *bool INPUT environments.*.variables.*.type variable.VariableType INPUT environments.*.variables.*.value any INPUT environments.*.workspace *config.Workspace INPUT @@ -159,7 +159,7 @@ environments.*.workspace.azure_client_id string INPUT environments.*.workspace.azure_environment string INPUT environments.*.workspace.azure_login_app_id string INPUT environments.*.workspace.azure_tenant_id string INPUT -environments.*.workspace.azure_use_msi bool INPUT +environments.*.workspace.azure_use_msi *bool INPUT environments.*.workspace.azure_workspace_resource_id string INPUT environments.*.workspace.client_id string INPUT environments.*.workspace.current_user *config.User INPUT @@ -204,7 +204,7 @@ environments.*.workspace.current_user.schemas []iam.UserSchema INPUT environments.*.workspace.current_user.schemas[*] iam.UserSchema INPUT environments.*.workspace.current_user.short_name string INPUT environments.*.workspace.current_user.userName string INPUT -environments.*.workspace.experimental_is_unified_host bool INPUT +environments.*.workspace.experimental_is_unified_host *bool INPUT environments.*.workspace.file_path string INPUT environments.*.workspace.google_service_account string INPUT environments.*.workspace.host string INPUT @@ -217,19 +217,19 @@ environments.*.workspace.sync_mode config.SyncMode INPUT environments.*.workspace.workspace_id string INPUT experimental *config.Experimental INPUT experimental.deployment_history *bool INPUT -experimental.immutable_folder bool INPUT +experimental.immutable_folder *bool INPUT experimental.pydabs config.PyDABs INPUT -experimental.pydabs.enabled bool INPUT +experimental.pydabs.enabled *bool INPUT experimental.python config.Python INPUT experimental.python.mutators []string INPUT experimental.python.mutators[*] string INPUT experimental.python.venv_path string INPUT -experimental.python_wheel_wrapper bool INPUT +experimental.python_wheel_wrapper *bool INPUT experimental.scripts map[config.ScriptHook]config.Command INPUT experimental.scripts.* config.Command INPUT -experimental.skip_artifact_cleanup bool INPUT -experimental.skip_name_prefix_for_schema bool INPUT -experimental.use_legacy_run_as bool INPUT +experimental.skip_artifact_cleanup *bool INPUT +experimental.skip_name_prefix_for_schema *bool INPUT +experimental.use_legacy_run_as *bool INPUT include []string INPUT include[*] string INPUT permissions []resources.Permission INPUT @@ -272,7 +272,7 @@ targets.* *config.Target INPUT targets.*.artifacts config.Artifacts INPUT targets.*.artifacts.* *config.Artifact INPUT targets.*.artifacts.*.build string INPUT -targets.*.artifacts.*.dynamic_version bool INPUT +targets.*.artifacts.*.dynamic_version *bool INPUT targets.*.artifacts.*.executable exec.ExecutableType INPUT targets.*.artifacts.*.files []config.ArtifactFile INPUT targets.*.artifacts.*.files[*] config.ArtifactFile INPUT @@ -296,7 +296,7 @@ targets.*.bundle.deployment.fail_on_active_runs bool INPUT targets.*.bundle.deployment.latest_version_id int INPUT targets.*.bundle.deployment.lock config.Lock INPUT targets.*.bundle.deployment.lock.enabled *bool INPUT -targets.*.bundle.deployment.lock.force bool INPUT +targets.*.bundle.deployment.lock.force *bool INPUT targets.*.bundle.engine engine.EngineType INPUT targets.*.bundle.environment string INPUT targets.*.bundle.force bool INPUT @@ -314,7 +314,7 @@ targets.*.bundle.terraform.exec_path string INPUT targets.*.bundle.uuid string INPUT targets.*.cluster_id string INPUT targets.*.compute_id string INPUT -targets.*.default bool INPUT +targets.*.default *bool INPUT targets.*.git config.Git INPUT targets.*.git.actual_branch string INPUT targets.*.git.branch string INPUT @@ -365,7 +365,7 @@ targets.*.variables.*.lookup.pipeline string INPUT targets.*.variables.*.lookup.query string INPUT targets.*.variables.*.lookup.service_principal string INPUT targets.*.variables.*.lookup.warehouse string INPUT -targets.*.variables.*.sensitive bool INPUT +targets.*.variables.*.sensitive *bool INPUT targets.*.variables.*.type variable.VariableType INPUT targets.*.variables.*.value any INPUT targets.*.workspace *config.Workspace INPUT @@ -376,7 +376,7 @@ targets.*.workspace.azure_client_id string INPUT targets.*.workspace.azure_environment string INPUT targets.*.workspace.azure_login_app_id string INPUT targets.*.workspace.azure_tenant_id string INPUT -targets.*.workspace.azure_use_msi bool INPUT +targets.*.workspace.azure_use_msi *bool INPUT targets.*.workspace.azure_workspace_resource_id string INPUT targets.*.workspace.client_id string INPUT targets.*.workspace.current_user *config.User INPUT @@ -421,7 +421,7 @@ targets.*.workspace.current_user.schemas []iam.UserSchema INPUT targets.*.workspace.current_user.schemas[*] iam.UserSchema INPUT targets.*.workspace.current_user.short_name string INPUT targets.*.workspace.current_user.userName string INPUT -targets.*.workspace.experimental_is_unified_host bool INPUT +targets.*.workspace.experimental_is_unified_host *bool INPUT targets.*.workspace.file_path string INPUT targets.*.workspace.google_service_account string INPUT targets.*.workspace.host string INPUT @@ -449,7 +449,7 @@ variables.*.lookup.pipeline string INPUT variables.*.lookup.query string INPUT variables.*.lookup.service_principal string INPUT variables.*.lookup.warehouse string INPUT -variables.*.sensitive bool INPUT +variables.*.sensitive *bool INPUT variables.*.type variable.VariableType INPUT variables.*.value any INPUT workspace config.Workspace INPUT @@ -460,7 +460,7 @@ workspace.azure_client_id string INPUT workspace.azure_environment string INPUT workspace.azure_login_app_id string INPUT workspace.azure_tenant_id string INPUT -workspace.azure_use_msi bool INPUT +workspace.azure_use_msi *bool INPUT workspace.azure_workspace_resource_id string INPUT workspace.client_id string INPUT workspace.current_user *config.User INPUT @@ -505,7 +505,7 @@ workspace.current_user.schemas []iam.UserSchema INPUT workspace.current_user.schemas[*] iam.UserSchema INPUT workspace.current_user.short_name string INPUT workspace.current_user.userName string INPUT -workspace.experimental_is_unified_host bool INPUT +workspace.experimental_is_unified_host *bool INPUT workspace.file_path string INPUT workspace.google_service_account string INPUT workspace.host string INPUT diff --git a/acceptance/bundle/validate/explicit_false/databricks.yml b/acceptance/bundle/validate/explicit_false/databricks.yml new file mode 100644 index 00000000000..5e8f6e657e8 --- /dev/null +++ b/acceptance/bundle/validate/explicit_false/databricks.yml @@ -0,0 +1,36 @@ +bundle: + name: test-bundle + deployment: + lock: + force: false + +workspace: + azure_use_msi: false + +variables: + plain: + default: x + sensitive: false + +artifacts: + whl: + type: whl + build: "true" + dynamic_version: false + overridden: + type: whl + build: "true" + dynamic_version: true + +experimental: + python_wheel_wrapper: false + use_legacy_run_as: false + skip_artifact_cleanup: false + skip_name_prefix_for_schema: false + +targets: + dev: + default: true + artifacts: + overridden: + dynamic_version: false diff --git a/acceptance/bundle/validate/explicit_false/out.test.toml b/acceptance/bundle/validate/explicit_false/out.test.toml new file mode 100644 index 00000000000..a927a5fbc06 --- /dev/null +++ b/acceptance/bundle/validate/explicit_false/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DMS = ["", "true"] diff --git a/acceptance/bundle/validate/explicit_false/output.txt b/acceptance/bundle/validate/explicit_false/output.txt new file mode 100644 index 00000000000..5308e136bc7 --- /dev/null +++ b/acceptance/bundle/validate/explicit_false/output.txt @@ -0,0 +1,27 @@ + +>>> [CLI] bundle validate -o json +{ + "lock_force": false, + "workspace": { + "azure_use_msi": false + }, + "variables": { + "plain": { + "sensitive": false + } + }, + "artifacts": { + "overridden": { + "dynamic_version": false + }, + "whl": { + "dynamic_version": false + } + }, + "experimental": { + "python_wheel_wrapper": false, + "skip_artifact_cleanup": false, + "skip_name_prefix_for_schema": false, + "use_legacy_run_as": false + } +} diff --git a/acceptance/bundle/validate/explicit_false/script b/acceptance/bundle/validate/explicit_false/script new file mode 100644 index 00000000000..32061afb6c5 --- /dev/null +++ b/acceptance/bundle/validate/explicit_false/script @@ -0,0 +1,9 @@ +# An explicit false is kept in the config, and a target can override true with false +# (artifacts.overridden.dynamic_version). +trace $CLI bundle validate -o json | jq '{ + lock_force: .bundle.deployment.lock.force, + workspace: .workspace | {azure_use_msi}, + variables: .variables | map_values({sensitive}), + artifacts: .artifacts | map_values({dynamic_version}), + experimental +}' diff --git a/bundle/artifacts/build.go b/bundle/artifacts/build.go index 6c7ab622737..10d1f0aa03c 100644 --- a/bundle/artifacts/build.go +++ b/bundle/artifacts/build.go @@ -79,7 +79,7 @@ func (m *build) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { } } - if a.Type == "whl" && a.DynamicVersion && cacheDir != "" { + if a.Type == "whl" && config.IsExplicitlyEnabled(a.DynamicVersion) && cacheDir != "" { b.Metrics.AddBoolValue(metrics.ArtifactDynamicVersionIsSet, true) for ind, artifactFile := range a.Files { patchedWheel := makePatchedWheel(ctx, cacheDir, artifactName, artifactFile.Source) diff --git a/bundle/artifacts/upload.go b/bundle/artifacts/upload.go index 341c1ee632e..c5b6e225d9f 100644 --- a/bundle/artifacts/upload.go +++ b/bundle/artifacts/upload.go @@ -4,6 +4,7 @@ import ( "context" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/filer" @@ -26,7 +27,7 @@ func (m *cleanUp) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics return diags } - skipArtifactsCleanup := b.Config.Experimental != nil && b.Config.Experimental.SkipArtifactCleanup + skipArtifactsCleanup := b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.SkipArtifactCleanup) b.Metrics.AddBoolValue("skip_artifact_cleanup", skipArtifactsCleanup) if skipArtifactsCleanup { log.Info(ctx, "Skip cleaning up artifacts folder") diff --git a/bundle/bundle.go b/bundle/bundle.go index 6762b38b91e..08586813a14 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -441,7 +441,7 @@ func (b *Bundle) SyncMode() config.SyncMode { switch { case b.Config.Workspace.SyncMode != "": return b.Config.Workspace.SyncMode - case b.Config.Experimental != nil && b.Config.Experimental.ImmutableFolder: + case b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.ImmutableFolder): return config.SyncModeImmutable case config.IsExplicitlyEnabled(sourceLinked): return config.SyncModeInPlace diff --git a/bundle/bundle_test.go b/bundle/bundle_test.go index 69c8fa5bdcb..846799880db 100644 --- a/bundle/bundle_test.go +++ b/bundle/bundle_test.go @@ -229,7 +229,7 @@ func TestBundleSyncMode(t *testing.T) { b := &Bundle{Config: config.Root{ Workspace: config.Workspace{SyncMode: tt.syncMode}, Presets: config.Presets{SourceLinkedDeployment: tt.sourceLinked}, - Experimental: &config.Experimental{ImmutableFolder: tt.immutable}, + Experimental: &config.Experimental{ImmutableFolder: new(tt.immutable)}, }} assert.Equal(t, tt.expected, b.SyncMode()) assert.Equal(t, tt.expected == config.SyncModeImmutable, b.IsImmutableFolder()) diff --git a/bundle/config/artifact.go b/bundle/config/artifact.go index ad5c79ae9b6..f29cac79212 100644 --- a/bundle/config/artifact.go +++ b/bundle/config/artifact.go @@ -59,7 +59,7 @@ type Artifact struct { Executable exec.ExecutableType `json:"executable,omitempty"` - DynamicVersion bool `json:"dynamic_version,omitempty"` + DynamicVersion *bool `json:"dynamic_version,omitempty"` // Include narrows a `tgz` artifact to these subpaths of `path`, relative to it; // empty packs the whole `path`. .gitignore is honored; the bundle-wide diff --git a/bundle/config/experimental.go b/bundle/config/experimental.go index 4065d17bfa8..bfbd02f5adb 100644 --- a/bundle/config/experimental.go +++ b/bundle/config/experimental.go @@ -10,7 +10,7 @@ type Experimental struct { // path. // // ImmutableFolder is deprecated, use workspace.sync_mode: immutable instead. - ImmutableFolder bool `json:"immutable_folder,omitempty"` + ImmutableFolder *bool `json:"immutable_folder,omitempty"` // By default Python wheel tasks deployed as is to Databricks platform. // If notebook wrapper required (for example, used in DBR < 13.1 or other configuration differences), users can provide a following experimental setting @@ -18,7 +18,7 @@ type Experimental struct { // python_wheel_wrapper: true // In this case the configured wheel task will be deployed as a notebook task which install defined wheel in runtime and executes it. // For more details see https://github.com/databricks/cli/pull/797 and https://github.com/databricks/cli/pull/635 - PythonWheelWrapper bool `json:"python_wheel_wrapper,omitempty"` + PythonWheelWrapper *bool `json:"python_wheel_wrapper,omitempty"` // Enable legacy run_as behavior. That is: // - Set the run_as identity as the owner of any pipelines in the bundle. @@ -31,7 +31,7 @@ type Experimental struct { // made in https://github.com/databricks/cli/pull/1233. This flag might // be removed in the future once we have a proper workaround like allowing IS_OWNER // as a top-level permission in the DAB. - UseLegacyRunAs bool `json:"use_legacy_run_as,omitempty"` + UseLegacyRunAs *bool `json:"use_legacy_run_as,omitempty"` // PyDABs is deprecated use Python instead. PyDABs PyDABs `json:"pydabs,omitempty"` @@ -42,7 +42,7 @@ type Experimental struct { // SkipArtifactCleanup determines whether to skip cleaning up the .internal folder // containing build artifacts such as wheels. When set to true, the .internal folder // and its contents will be preserved after bundle operations complete. - SkipArtifactCleanup bool `json:"skip_artifact_cleanup,omitempty"` + SkipArtifactCleanup *bool `json:"skip_artifact_cleanup,omitempty"` // SkipNamePrefixForSchema skips adding the `presets.name_prefix` prefix // to UC schemas defined in the bundle. Currently this is a opt-in behavior @@ -50,7 +50,7 @@ type Experimental struct { // customer data. // Eventually this can be made the default once we have native CRUD in DABs // at which point we can deprecate or remove this field all together. - SkipNamePrefixForSchema bool `json:"skip_name_prefix_for_schema,omitempty"` + SkipNamePrefixForSchema *bool `json:"skip_name_prefix_for_schema,omitempty"` // DeploymentHistory opts the bundle into the deployment metadata // service (DMS), which records deployment history and tracks what changed @@ -84,7 +84,7 @@ type Python struct { // PyDABs is deprecated use Python instead type PyDABs struct { // Enabled is a flag to enable the feature. - Enabled bool `json:"enabled,omitempty"` + Enabled *bool `json:"enabled,omitempty"` } type ( diff --git a/bundle/config/lock.go b/bundle/config/lock.go index 10e9e1c9c56..671d0f48ad3 100644 --- a/bundle/config/lock.go +++ b/bundle/config/lock.go @@ -8,7 +8,7 @@ type Lock struct { // Force acquisition of deployment lock even if it is currently held. // This may be necessary if a prior deployment failed to release the lock. - Force bool `json:"force,omitempty"` + Force *bool `json:"force,omitempty"` } // IsEnabled checks if the deployment lock is enabled. diff --git a/bundle/config/mutator/apply_sync_mode.go b/bundle/config/mutator/apply_sync_mode.go index f362cce2905..57b564920cd 100644 --- a/bundle/config/mutator/apply_sync_mode.go +++ b/bundle/config/mutator/apply_sync_mode.go @@ -121,7 +121,7 @@ func warnDeprecatedSyncMode(b *bundle.Bundle) diag.Diagnostics { override = syncModePath } - if b.Config.Experimental != nil && b.Config.Experimental.ImmutableFolder { + if b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.ImmutableFolder) { diags = diags.Append(deprecatedSyncModeWarning(b, immutableFolderPath, override)) if override == nil { override = immutableFolderPath diff --git a/bundle/config/mutator/apply_sync_mode_test.go b/bundle/config/mutator/apply_sync_mode_test.go index 8a2a2a98640..fadbab93123 100644 --- a/bundle/config/mutator/apply_sync_mode_test.go +++ b/bundle/config/mutator/apply_sync_mode_test.go @@ -169,7 +169,7 @@ func TestApplySyncMode(t *testing.T) { name: "deprecated immutable_folder", ctx: dbr.MockRuntime(testContext, dbr.Environment{IsDbr: true, Version: "15.4"}), mutateBundle: func(b *bundle.Bundle) { - b.Config.Experimental = &config.Experimental{ImmutableFolder: true} + b.Config.Experimental = &config.Experimental{ImmutableFolder: new(true)} }, expectedValue: config.SyncModeImmutable, expectedWarning: `experimental.immutable_folder is deprecated, please set workspace.sync_mode to "immutable" instead`, @@ -178,7 +178,7 @@ func TestApplySyncMode(t *testing.T) { name: "deprecated immutable_folder, dev mode true, bundle in Workspace, databricks runtime", ctx: dbr.MockRuntime(testContext, dbr.Environment{IsDbr: true, Version: "15.4"}), mutateBundle: func(b *bundle.Bundle) { - b.Config.Experimental = &config.Experimental{ImmutableFolder: true} + b.Config.Experimental = &config.Experimental{ImmutableFolder: new(true)} b.Config.Bundle.Mode = config.Development }, expectedValue: config.SyncModeImmutable, @@ -188,7 +188,7 @@ func TestApplySyncMode(t *testing.T) { name: "deprecated immutable_folder, deprecated source_linked_deployment disabled", ctx: dbr.MockRuntime(testContext, dbr.Environment{IsDbr: true, Version: "15.4"}), mutateBundle: func(b *bundle.Bundle) { - b.Config.Experimental = &config.Experimental{ImmutableFolder: true} + b.Config.Experimental = &config.Experimental{ImmutableFolder: new(true)} b.Config.Presets.SourceLinkedDeployment = &disabled }, expectedValue: config.SyncModeImmutable, @@ -198,7 +198,7 @@ func TestApplySyncMode(t *testing.T) { name: "deprecated immutable_folder, deprecated source_linked_deployment enabled", ctx: dbr.MockRuntime(testContext, dbr.Environment{IsDbr: true, Version: "15.4"}), mutateBundle: func(b *bundle.Bundle) { - b.Config.Experimental = &config.Experimental{ImmutableFolder: true} + b.Config.Experimental = &config.Experimental{ImmutableFolder: new(true)} b.Config.Presets.SourceLinkedDeployment = &enabled }, expectedValue: config.SyncModeImmutable, @@ -218,7 +218,7 @@ func TestApplySyncMode(t *testing.T) { name: "deprecated immutable_folder with sync_mode", ctx: dbr.MockRuntime(testContext, dbr.Environment{IsDbr: true, Version: "15.4"}), mutateBundle: func(b *bundle.Bundle) { - b.Config.Experimental = &config.Experimental{ImmutableFolder: true} + b.Config.Experimental = &config.Experimental{ImmutableFolder: new(true)} }, initialValue: config.SyncModeOverwrite, expectedValue: config.SyncModeOverwrite, diff --git a/bundle/config/mutator/artifacts_dynamic_version.go b/bundle/config/mutator/artifacts_dynamic_version.go index 9d66a7619b7..eab64bbacf7 100644 --- a/bundle/config/mutator/artifacts_dynamic_version.go +++ b/bundle/config/mutator/artifacts_dynamic_version.go @@ -31,7 +31,7 @@ func (m *artifactsUseDynamicVersion) Apply(ctx context.Context, b *bundle.Bundle // This has no effect since we only apply DynamicVersion if type is "whl" but it makes output cleaner. continue } - a.DynamicVersion = true + a.DynamicVersion = new(true) } return nil diff --git a/bundle/config/mutator/python/python_mutator.go b/bundle/config/mutator/python/python_mutator.go index 2397defd153..757ab50e0fb 100644 --- a/bundle/config/mutator/python/python_mutator.go +++ b/bundle/config/mutator/python/python_mutator.go @@ -129,7 +129,7 @@ func getOpts(b *bundle.Bundle, phase phase) (opts, error) { } // using reflect.DeepEquals in case we add more fields - pydabsEnabled := !reflect.DeepEqual(experimental.PyDABs, config.PyDABs{}) + pydabsEnabled := config.IsExplicitlyEnabled(experimental.PyDABs.Enabled) experimentalPythonEnabled := !reflect.DeepEqual(experimental.Python, config.Python{}) pythonEnabled := !reflect.DeepEqual(b.Config.Python, config.Python{}) diff --git a/bundle/config/mutator/python/python_mutator_test.go b/bundle/config/mutator/python/python_mutator_test.go index f3378b1bd51..8e1b3369897 100644 --- a/bundle/config/mutator/python/python_mutator_test.go +++ b/bundle/config/mutator/python/python_mutator_test.go @@ -404,7 +404,7 @@ func TestGetOps_PyDABs(t *testing.T) { Config: config.Root{ Experimental: &config.Experimental{ PyDABs: config.PyDABs{ - Enabled: true, + Enabled: new(true), }, }, }, diff --git a/bundle/config/mutator/resourcemutator/apply_presets.go b/bundle/config/mutator/resourcemutator/apply_presets.go index 9f2faab8398..2c20366f5fc 100644 --- a/bundle/config/mutator/resourcemutator/apply_presets.go +++ b/bundle/config/mutator/resourcemutator/apply_presets.go @@ -213,7 +213,7 @@ func (m *applyPresets) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnos // Schemas: Prefix for _, s := range r.Schemas { - if b.Config.Experimental != nil && b.Config.Experimental.SkipNamePrefixForSchema { + if b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.SkipNamePrefixForSchema) { break } diff --git a/bundle/config/mutator/resourcemutator/run_as.go b/bundle/config/mutator/resourcemutator/run_as.go index b946db77235..0a6943cf3f1 100644 --- a/bundle/config/mutator/resourcemutator/run_as.go +++ b/bundle/config/mutator/resourcemutator/run_as.go @@ -6,6 +6,7 @@ import ( "slices" "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/dyn" @@ -226,7 +227,7 @@ func setPipelineOwnersToRunAsIdentity(b *bundle.Bundle) { func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { // Track the use of the legacy run_as mode. - b.Metrics.AddBoolValue("experimental.use_legacy_run_as", b.Config.Experimental != nil && b.Config.Experimental.UseLegacyRunAs) + b.Metrics.AddBoolValue("experimental.use_legacy_run_as", b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.UseLegacyRunAs)) // Track whether top level run_as is set. b.Metrics.AddBoolValue("run_as_set", b.Config.Value().Get("run_as").Kind() != dyn.KindInvalid) @@ -238,7 +239,7 @@ func (m *setRunAs) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { // User has opted to use the legacy behavior of run_as with the // experimental.use_legacy_run_as flag. - if b.Config.Experimental != nil && b.Config.Experimental.UseLegacyRunAs { + if b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.UseLegacyRunAs) { if b.Config.Value().Get("run_as").Get("group_name").Kind() != dyn.KindInvalid { return diag.Errorf("run_as.group_name is not supported with experimental.use_legacy_run_as; disable experimental.use_legacy_run_as to use a group identity") } diff --git a/bundle/config/mutator/select_default_target.go b/bundle/config/mutator/select_default_target.go index ad8132a46aa..ee290a95db9 100644 --- a/bundle/config/mutator/select_default_target.go +++ b/bundle/config/mutator/select_default_target.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" ) @@ -36,7 +37,7 @@ func (m *selectDefaultTarget) Apply(ctx context.Context, b *bundle.Bundle) diag. // Multiple targets means we look for the `default` flag. var defaults []string for name, env := range b.Config.Targets { - if env != nil && env.Default { + if env != nil && config.IsExplicitlyEnabled(env.Default) { defaults = append(defaults, name) } } diff --git a/bundle/config/mutator/select_default_target_test.go b/bundle/config/mutator/select_default_target_test.go index 850bfd88546..33ae0c88cb8 100644 --- a/bundle/config/mutator/select_default_target_test.go +++ b/bundle/config/mutator/select_default_target_test.go @@ -63,9 +63,9 @@ func TestSelectDefaultTargetMultipleDefaults(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Targets: map[string]*config.Target{ - "foo": {Default: true}, - "bar": {Default: true}, - "qux": {Default: true}, + "foo": {Default: new(true)}, + "bar": {Default: new(true)}, + "qux": {Default: new(true)}, }, }, } @@ -78,7 +78,7 @@ func TestSelectDefaultTargetSingleDefault(t *testing.T) { Config: config.Root{ Targets: map[string]*config.Target{ "foo": {}, - "bar": {Default: true}, + "bar": {Default: new(true)}, "qux": {}, }, }, diff --git a/bundle/config/optional_bools_test.go b/bundle/config/optional_bools_test.go new file mode 100644 index 00000000000..8da692dd1b8 --- /dev/null +++ b/bundle/config/optional_bools_test.go @@ -0,0 +1,50 @@ +package config + +import ( + "reflect" + "strings" + "testing" + + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/libs/structs/structtag" + "github.com/stretchr/testify/assert" +) + +// An omitempty bool in a CLI-defined config struct must be a *bool, unless the +// struct declares ForceSendFields. Otherwise an explicit false is lost: the +// typed-to-dynamic conversion that runs after every mutator drops zero values, so +// the false never shows in the config and a target cannot override true with false. +func TestOmitemptyBoolsArePointers(t *testing.T) { + // replace_existing is a resource input passed to the API as is; it only + // matters when true. + skip := map[reflect.Type]bool{ + reflect.TypeFor[resources.PostgresEndpointConfig](): true, + reflect.TypeFor[resources.PostgresDatabaseConfig](): true, + reflect.TypeFor[resources.PostgresRoleConfig](): true, + } + checkOmitemptyBools(t, reflect.TypeFor[Root](), skip, map[reflect.Type]bool{}) +} + +func checkOmitemptyBools(t *testing.T, typ reflect.Type, skip, seen map[reflect.Type]bool) { + for typ.Kind() == reflect.Pointer || typ.Kind() == reflect.Slice || typ.Kind() == reflect.Map { + typ = typ.Elem() + } + if typ.Kind() != reflect.Struct || seen[typ] { + return + } + seen[typ] = true + fsf, ok := typ.FieldByName("ForceSendFields") + keepsZeros := ok && len(fsf.Index) == 1 + cliDefined := strings.HasPrefix(typ.PkgPath(), "github.com/databricks/cli/") && !skip[typ] + for f := range typ.Fields() { + f := f + checkOmitemptyBools(t, f.Type, skip, seen) + // Readonly fields are set by the CLI, not by users. + if structtag.BundleTag(f.Tag.Get("bundle")).ReadOnly() { + continue + } + if cliDefined && !keepsZeros && f.Type.Kind() == reflect.Bool && strings.Contains(f.Tag.Get("json"), ",omitempty") { + assert.Fail(t, "omitempty bool", "%s.%s must be a *bool so that an explicit false is kept", typ.Name(), f.Name) + } + } +} diff --git a/bundle/config/target.go b/bundle/config/target.go index fae9c940b3f..fada6b131aa 100644 --- a/bundle/config/target.go +++ b/bundle/config/target.go @@ -13,7 +13,7 @@ type Mode string type Target struct { // Default marks that this target must be used if one isn't specified // by the user (through target variable or command line argument). - Default bool `json:"default,omitempty"` + Default *bool `json:"default,omitempty"` // Determines the mode of the target. // For example, 'mode: development' can be used for deployments for diff --git a/bundle/config/validate/sensitive_variable_references.go b/bundle/config/validate/sensitive_variable_references.go index 4ae8ae9c76d..f9880e8f721 100644 --- a/bundle/config/validate/sensitive_variable_references.go +++ b/bundle/config/validate/sensitive_variable_references.go @@ -7,6 +7,7 @@ import ( "slices" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/direct/dresources" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" @@ -105,7 +106,7 @@ func inferSensitiveVariables(b *bundle.Bundle, refs []variableReference) error { for _, r := range refs { // A reference to an undeclared variable is reported by variable resolution. v, ok := b.Config.Variables[r.name] - if !ok || v.Sensitive || !isSensitiveResourceField(r.path) { + if !ok || config.IsExplicitlyEnabled(v.Sensitive) || !isSensitiveResourceField(r.path) { continue } var err error @@ -120,7 +121,7 @@ func inferSensitiveVariables(b *bundle.Bundle, refs []variableReference) error { func isSensitiveVariable(b *bundle.Bundle, name string) bool { v, ok := b.Config.Variables[name] - return ok && v.Sensitive + return ok && config.IsExplicitlyEnabled(v.Sensitive) } // referencedVariable returns the variable a reference like "var.foo" or diff --git a/bundle/config/validate/sensitive_variable_references_test.go b/bundle/config/validate/sensitive_variable_references_test.go index 73f51c2fc82..1a5b051ebfa 100644 --- a/bundle/config/validate/sensitive_variable_references_test.go +++ b/bundle/config/validate/sensitive_variable_references_test.go @@ -20,8 +20,8 @@ func sensitiveVariablesBundle() *bundle.Bundle { return &bundle.Bundle{ Config: config.Root{ Variables: map[string]*variable.Variable{ - "secret": {Sensitive: true}, - "other_secret": {Sensitive: true, Default: "placeholder"}, + "secret": {Sensitive: new(true)}, + "other_secret": {Sensitive: new(true), Default: "placeholder"}, "plain": {Default: "placeholder"}, }, Resources: config.Resources{ @@ -109,7 +109,7 @@ func TestSensitiveVariableReferencesInfersFromSensitiveResourceField(t *testing. }) assert.Empty(t, SensitiveVariableReferences().Apply(t.Context(), b)) - assert.True(t, b.Config.Variables["plain"].Sensitive) + assert.True(t, config.IsExplicitlyEnabled(b.Config.Variables["plain"].Sensitive)) v, err := dyn.GetByPath(b.Config.Value(), dyn.MustPathFromString("variables.plain.sensitive")) require.NoError(t, err) assert.Equal(t, true, v.AsAny()) diff --git a/bundle/config/variable/variable.go b/bundle/config/variable/variable.go index b6fbf56d260..f5a5fd6ee75 100644 --- a/bundle/config/variable/variable.go +++ b/bundle/config/variable/variable.go @@ -44,7 +44,7 @@ type Variable struct { // redacted in the output of `bundle validate -o json` and `bundle summary -o json`, and // it may only be referenced from sensitive resource fields or from other sensitive // variables, so that the secret never ends up in plain text. - Sensitive bool `json:"sensitive,omitempty"` + Sensitive *bool `json:"sensitive,omitempty"` // This field stores the resolved value for the variable. The variable are // resolved in the following priority order (from highest to lowest) diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index e1448dc7212..bed0d0bf69d 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -38,7 +38,7 @@ type Workspace struct { // Azure specific attributes. AzureResourceID string `json:"azure_workspace_resource_id,omitempty"` - AzureUseMSI bool `json:"azure_use_msi,omitempty"` + AzureUseMSI *bool `json:"azure_use_msi,omitempty"` AzureClientID string `json:"azure_client_id,omitempty"` AzureTenantID string `json:"azure_tenant_id,omitempty"` AzureEnvironment string `json:"azure_environment,omitempty"` @@ -50,7 +50,7 @@ type Workspace struct { // detected automatically from /.well-known/databricks-config. The field is // retained so existing databricks.yml files using it still validate against // the bundle schema. - ExperimentalIsUnifiedHost bool `json:"experimental_is_unified_host,omitempty"` + ExperimentalIsUnifiedHost *bool `json:"experimental_is_unified_host,omitempty"` AccountID string `json:"account_id,omitempty"` WorkspaceID string `json:"workspace_id,omitempty"` @@ -162,7 +162,7 @@ func (w *Workspace) Config(ctx context.Context) *config.Config { // Azure AzureResourceID: w.AzureResourceID, - AzureUseMSI: w.AzureUseMSI, + AzureUseMSI: IsExplicitlyEnabled(w.AzureUseMSI), AzureClientID: w.AzureClientID, AzureTenantID: w.AzureTenantID, AzureEnvironment: w.AzureEnvironment, diff --git a/bundle/deploy/lock/acquire.go b/bundle/deploy/lock/acquire.go index bf8b91212dd..fab822ac2e4 100644 --- a/bundle/deploy/lock/acquire.go +++ b/bundle/deploy/lock/acquire.go @@ -6,6 +6,7 @@ import ( "io/fs" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/permissions" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/locker" @@ -54,7 +55,7 @@ func (m *acquire) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics return diag.FromErr(err) } - force := b.Config.Bundle.Deployment.Lock.Force + force := config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.Lock.Force) log.Infof(ctx, "Acquiring deployment lock (force: %v)", force) err = b.Locker.Lock(ctx, force) if err != nil { diff --git a/bundle/direct/dresources/redact_config.go b/bundle/direct/dresources/redact_config.go index 8812caef453..185713ea830 100644 --- a/bundle/direct/dresources/redact_config.go +++ b/bundle/direct/dresources/redact_config.go @@ -48,7 +48,7 @@ const sensitiveRedactedMarker = "[redacted]" // to the secret by the time the config is rendered. func redactSensitiveVariables(root *config.Root) { for _, variable := range root.Variables { - if variable == nil || !variable.Sensitive { + if variable == nil || !config.IsExplicitlyEnabled(variable.Sensitive) { continue } if variable.Default != nil { diff --git a/bundle/direct/dresources/redact_config_test.go b/bundle/direct/dresources/redact_config_test.go index c4bbabaf5dd..7bd51ace351 100644 --- a/bundle/direct/dresources/redact_config_test.go +++ b/bundle/direct/dresources/redact_config_test.go @@ -27,16 +27,16 @@ func TestRedactSensitiveConfigValues(t *testing.T) { Variables: map[string]*variable.Variable{ "catalog": {Default: "main", Value: "main"}, "count": {Default: 1, Value: 1}, - "secret": {Sensitive: true, Value: "top-secret"}, - "header": {Sensitive: true, Default: "Bearer top-secret", Value: "Bearer top-secret"}, + "secret": {Sensitive: new(true), Value: "top-secret"}, + "header": {Sensitive: new(true), Default: "Bearer top-secret", Value: "Bearer top-secret"}, }, Resources: config.Resources{Secrets: secrets}, }, expected: map[string]*variable.Variable{ "catalog": {Default: "main", Value: "main"}, "count": {Default: 1, Value: 1}, - "secret": {Sensitive: true, Value: "[redacted]"}, - "header": {Sensitive: true, Default: "[redacted]", Value: "[redacted]"}, + "secret": {Sensitive: new(true), Value: "[redacted]"}, + "header": {Sensitive: new(true), Default: "[redacted]", Value: "[redacted]"}, }, }, { @@ -44,12 +44,12 @@ func TestRedactSensitiveConfigValues(t *testing.T) { root: config.Root{ Variables: map[string]*variable.Variable{ "catalog": {Default: "main", Value: "main"}, - "secret": {Sensitive: true, Value: "top-secret"}, + "secret": {Sensitive: new(true), Value: "top-secret"}, }, }, expected: map[string]*variable.Variable{ "catalog": {Default: "main", Value: "main"}, - "secret": {Sensitive: true, Value: "[redacted]"}, + "secret": {Sensitive: new(true), Value: "[redacted]"}, }, }, } diff --git a/bundle/libraries/remote_path.go b/bundle/libraries/remote_path.go index 70e577a1a2a..ff6be05c3b2 100644 --- a/bundle/libraries/remote_path.go +++ b/bundle/libraries/remote_path.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn" "github.com/databricks/cli/libs/patchwheel" @@ -50,7 +51,7 @@ func LocalLibraryPaths(ctx context.Context, b *bundle.Bundle) ([]string, error) if cacheDir != "" { patches := make(map[string]string) // original → patched for artifactName, artifact := range b.Config.Artifacts { - if artifact == nil || artifact.Type != "whl" || !artifact.DynamicVersion { + if artifact == nil || artifact.Type != "whl" || !config.IsExplicitlyEnabled(artifact.DynamicVersion) { continue } for _, f := range artifact.Files { diff --git a/bundle/run/job.go b/bundle/run/job.go index 98f059e5784..185b0bcefd5 100644 --- a/bundle/run/job.go +++ b/bundle/run/job.go @@ -9,6 +9,7 @@ import ( "time" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/resources" "github.com/databricks/cli/bundle/run/output" "github.com/databricks/cli/bundle/run/progress" @@ -207,7 +208,7 @@ func (r *jobRunner) Run(ctx context.Context, opts *Options) (output.RunOutput, e } func (r *jobRunner) convertPythonParams(opts *Options) error { - if r.bundle.Config.Experimental != nil && !r.bundle.Config.Experimental.PythonWheelWrapper { + if r.bundle.Config.Experimental != nil && !config.IsExplicitlyEnabled(r.bundle.Config.Experimental.PythonWheelWrapper) { return nil } diff --git a/bundle/trampoline/conditional_transform_test.go b/bundle/trampoline/conditional_transform_test.go index 928ed578d17..322e9a09a23 100644 --- a/bundle/trampoline/conditional_transform_test.go +++ b/bundle/trampoline/conditional_transform_test.go @@ -96,7 +96,7 @@ func TestTransformWithExperimentalSettingSetToTrue(t *testing.T) { }, }, Experimental: &config.Experimental{ - PythonWheelWrapper: true, + PythonWheelWrapper: new(true), }, }, } diff --git a/bundle/trampoline/python_dbr_warning.go b/bundle/trampoline/python_dbr_warning.go index a6f45e12311..2b097052dcf 100644 --- a/bundle/trampoline/python_dbr_warning.go +++ b/bundle/trampoline/python_dbr_warning.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/libs/diag" "github.com/databricks/cli/libs/dyn/dynvar" @@ -40,7 +41,7 @@ func (m *wrapperWarning) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagn } func isPythonWheelWrapperOn(b *bundle.Bundle) bool { - return b.Config.Experimental != nil && b.Config.Experimental.PythonWheelWrapper + return b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.PythonWheelWrapper) } func hasIncompatibleWheelTasks(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diff --git a/bundle/trampoline/python_dbr_warning_test.go b/bundle/trampoline/python_dbr_warning_test.go index 3f5c35bf7c6..f6ad3ff1cd4 100644 --- a/bundle/trampoline/python_dbr_warning_test.go +++ b/bundle/trampoline/python_dbr_warning_test.go @@ -304,7 +304,7 @@ func TestNoWarningWhenPythonWheelWrapperIsOn(t *testing.T) { b := &bundle.Bundle{ Config: config.Root{ Experimental: &config.Experimental{ - PythonWheelWrapper: true, + PythonWheelWrapper: new(true), }, Resources: config.Resources{ Jobs: map[string]*resources.Job{ diff --git a/bundle/trampoline/python_wheel.go b/bundle/trampoline/python_wheel.go index 722a0b35e6c..e8138612dde 100644 --- a/bundle/trampoline/python_wheel.go +++ b/bundle/trampoline/python_wheel.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/libraries" "github.com/databricks/cli/bundle/metrics" "github.com/databricks/cli/libs/diag" @@ -76,7 +77,7 @@ func TransformWheelTask() bundle.Mutator { } func (transformWheelTask) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - isEnabled := b.Config.Experimental != nil && b.Config.Experimental.PythonWheelWrapper + isEnabled := b.Config.Experimental != nil && config.IsExplicitlyEnabled(b.Config.Experimental.PythonWheelWrapper) b.Metrics.AddBoolValue(metrics.ExperimentalPythonWheelWrapperIsSet, isEnabled) if !isEnabled { return nil diff --git a/cmd/apps/deploy_bundle_test.go b/cmd/apps/deploy_bundle_test.go index 86f0535d99c..36633d6e5c7 100644 --- a/cmd/apps/deploy_bundle_test.go +++ b/cmd/apps/deploy_bundle_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" "github.com/databricks/databricks-sdk-go/service/apps" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -142,15 +143,15 @@ func TestApplyDeployFlags(t *testing.T) { name: "forceLock ignored when force-lock flag unchanged", opts: bundleDeployOptions{forceLock: true}, assertion: func(t *testing.T, b *bundle.Bundle) { - assert.False(t, b.Config.Bundle.Deployment.Lock.Force) + assert.Nil(t, b.Config.Bundle.Deployment.Lock.Force) }, }, { name: "lock.force from databricks.yml survives an unset force-lock flag", opts: bundleDeployOptions{}, - configure: func(b *bundle.Bundle) { b.Config.Bundle.Deployment.Lock.Force = true }, + configure: func(b *bundle.Bundle) { b.Config.Bundle.Deployment.Lock.Force = new(true) }, assertion: func(t *testing.T, b *bundle.Bundle) { - assert.True(t, b.Config.Bundle.Deployment.Lock.Force) + assert.True(t, config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.Lock.Force)) }, }, { @@ -158,7 +159,7 @@ func TestApplyDeployFlags(t *testing.T) { args: []string{"--force-lock"}, opts: bundleDeployOptions{forceLock: true}, assertion: func(t *testing.T, b *bundle.Bundle) { - assert.True(t, b.Config.Bundle.Deployment.Lock.Force) + assert.True(t, config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.Lock.Force)) }, }, { diff --git a/cmd/apps/import.go b/cmd/apps/import.go index f552c3182ce..7df83287871 100644 --- a/cmd/apps/import.go +++ b/cmd/apps/import.go @@ -308,7 +308,7 @@ func runImport(ctx context.Context, w *databricks.WorkspaceClient, appName, outp SkipInitContext: true, AlwaysPull: true, InitFunc: func(b *bundle.Bundle) { - b.Config.Bundle.Deployment.Lock.Force = false + b.Config.Bundle.Deployment.Lock.Force = new(false) }, }) if err != nil { diff --git a/cmd/bundle/debug/list_targets.go b/cmd/bundle/debug/list_targets.go index 613442569c3..2161b80b35d 100644 --- a/cmd/bundle/debug/list_targets.go +++ b/cmd/bundle/debug/list_targets.go @@ -59,7 +59,7 @@ func collectTargets(cfg *config.Root) []targetInfo { result = append(result, info) continue } - info.Default = t.Default + info.Default = config.IsExplicitlyEnabled(t.Default) info.Mode = t.Mode targetEngine := rootEngine if t.Bundle != nil && t.Bundle.Engine != engine.EngineNotSet { diff --git a/cmd/bundle/utils/utils.go b/cmd/bundle/utils/utils.go index 6b7b5cbc6c9..37e9743947e 100644 --- a/cmd/bundle/utils/utils.go +++ b/cmd/bundle/utils/utils.go @@ -22,7 +22,7 @@ func SetForceLock(cmd *cobra.Command, b *bundle.Bundle, forceLock bool) { return } - b.Config.Bundle.Deployment.Lock.Force = forceLock + b.Config.Bundle.Deployment.Lock.Force = new(forceLock) } func configureVariables(cmd *cobra.Command, b *bundle.Bundle, variables []string) { diff --git a/cmd/bundle/utils/utils_test.go b/cmd/bundle/utils/utils_test.go index 3aac3182bc2..6b10845433a 100644 --- a/cmd/bundle/utils/utils_test.go +++ b/cmd/bundle/utils/utils_test.go @@ -61,11 +61,11 @@ func TestSetForceLock(t *testing.T) { require.NoError(t, cmd.ParseFlags(tc.args)) b := &bundle.Bundle{Config: config.Root{}} - b.Config.Bundle.Deployment.Lock.Force = tc.configured + b.Config.Bundle.Deployment.Lock.Force = new(tc.configured) SetForceLock(cmd, b, tc.forceLock) - assert.Equal(t, tc.expected, b.Config.Bundle.Deployment.Lock.Force) + assert.Equal(t, tc.expected, config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.Lock.Force)) }) } } From be44f11cf1ae92a68c8e2884393d246eca92a8e8 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 16:37:50 +0100 Subject: [PATCH 2/5] Add changelog entry Co-authored-by: Isaac --- .nextchanges/bundles/7412.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 .nextchanges/bundles/7412.md diff --git a/.nextchanges/bundles/7412.md b/.nextchanges/bundles/7412.md new file mode 100644 index 00000000000..82c6c6064c6 --- /dev/null +++ b/.nextchanges/bundles/7412.md @@ -0,0 +1 @@ +* Fixed a target not being able to set `artifacts.*.dynamic_version`, `bundle.deployment.lock.force` or `workspace.azure_use_msi` to `false` when the root config sets it to `true`. ([#7412](https://github.com/databricks/cli/pull/7412)) From 61966f083bfbde7cddd5de605a4ef364f548f183 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 16:39:03 +0100 Subject: [PATCH 3/5] Drop a redundant loop variable copy Co-authored-by: Isaac --- bundle/config/optional_bools_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/bundle/config/optional_bools_test.go b/bundle/config/optional_bools_test.go index 8da692dd1b8..4915d8bb9bf 100644 --- a/bundle/config/optional_bools_test.go +++ b/bundle/config/optional_bools_test.go @@ -37,7 +37,6 @@ func checkOmitemptyBools(t *testing.T, typ reflect.Type, skip, seen map[reflect. keepsZeros := ok && len(fsf.Index) == 1 cliDefined := strings.HasPrefix(typ.PkgPath(), "github.com/databricks/cli/") && !skip[typ] for f := range typ.Fields() { - f := f checkOmitemptyBools(t, f.Type, skip, seen) // Readonly fields are set by the CLI, not by users. if structtag.BundleTag(f.Tag.Get("bundle")).ReadOnly() { From a018a918460361862b533e1fa29c6efe663f8b43 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 16:52:59 +0100 Subject: [PATCH 4/5] Require *bool for every bool in CLI config structs; cover all target overrides TestConfigBoolsArePointers now fails for any bool field of a CLI-defined config struct, not only omitempty ones in structs without ForceSendFields. The fields that predate it are listed with the reason they stay bool, and the test also fails if a listed field goes away. The acceptance test now overrides lock.force and azure_use_msi from a target too, and the changelog entry covers every affected field. Co-authored-by: Isaac --- .nextchanges/bundles/7412.md | 2 +- .../validate/explicit_false/databricks.yml | 11 +++- .../bundle/validate/explicit_false/script | 2 +- bundle/config/optional_bools_test.go | 62 ++++++++++++------- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/.nextchanges/bundles/7412.md b/.nextchanges/bundles/7412.md index 82c6c6064c6..884a16358fb 100644 --- a/.nextchanges/bundles/7412.md +++ b/.nextchanges/bundles/7412.md @@ -1 +1 @@ -* Fixed a target not being able to set `artifacts.*.dynamic_version`, `bundle.deployment.lock.force` or `workspace.azure_use_msi` to `false` when the root config sets it to `true`. ([#7412](https://github.com/databricks/cli/pull/7412)) +* Fixed an explicit `false` on optional boolean fields (`variables.*.sensitive`, `artifacts.*.dynamic_version`, `bundle.deployment.lock.force`, `workspace.azure_use_msi` and the `experimental.*` flags) being dropped, which hid it from `bundle validate -o json` and kept a target from overriding `true` with it. ([#7412](https://github.com/databricks/cli/pull/7412)) diff --git a/acceptance/bundle/validate/explicit_false/databricks.yml b/acceptance/bundle/validate/explicit_false/databricks.yml index 5e8f6e657e8..383ec0d49d1 100644 --- a/acceptance/bundle/validate/explicit_false/databricks.yml +++ b/acceptance/bundle/validate/explicit_false/databricks.yml @@ -2,10 +2,10 @@ bundle: name: test-bundle deployment: lock: - force: false + force: true workspace: - azure_use_msi: false + azure_use_msi: true variables: plain: @@ -31,6 +31,13 @@ experimental: targets: dev: default: true + # Each of these turns off what the root config turns on. + bundle: + deployment: + lock: + force: false + workspace: + azure_use_msi: false artifacts: overridden: dynamic_version: false diff --git a/acceptance/bundle/validate/explicit_false/script b/acceptance/bundle/validate/explicit_false/script index 32061afb6c5..832319ec16d 100644 --- a/acceptance/bundle/validate/explicit_false/script +++ b/acceptance/bundle/validate/explicit_false/script @@ -1,5 +1,5 @@ # An explicit false is kept in the config, and a target can override true with false -# (artifacts.overridden.dynamic_version). +# (lock.force, azure_use_msi and artifacts.overridden.dynamic_version). trace $CLI bundle validate -o json | jq '{ lock_force: .bundle.deployment.lock.force, workspace: .workspace | {azure_use_msi}, diff --git a/bundle/config/optional_bools_test.go b/bundle/config/optional_bools_test.go index 4915d8bb9bf..f12e2c40cdf 100644 --- a/bundle/config/optional_bools_test.go +++ b/bundle/config/optional_bools_test.go @@ -5,27 +5,46 @@ import ( "strings" "testing" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/libs/structs/structtag" "github.com/stretchr/testify/assert" ) -// An omitempty bool in a CLI-defined config struct must be a *bool, unless the -// struct declares ForceSendFields. Otherwise an explicit false is lost: the -// typed-to-dynamic conversion that runs after every mutator drops zero values, so -// the false never shows in the config and a target cannot override true with false. -func TestOmitemptyBoolsArePointers(t *testing.T) { - // replace_existing is a resource input passed to the API as is; it only - // matters when true. - skip := map[reflect.Type]bool{ - reflect.TypeFor[resources.PostgresEndpointConfig](): true, - reflect.TypeFor[resources.PostgresDatabaseConfig](): true, - reflect.TypeFor[resources.PostgresRoleConfig](): true, +// plainBools are the bool fields of CLI-defined config structs that predate +// TestConfigBoolsArePointers. Do not add to this list: declare new fields as *bool. +var plainBools = map[string]string{ + "config.Bundle.Force": "readonly: set from --force, not by users", + + "config.Deployment.FailOnActiveRuns": "explicit false kept by ForceSendFields", + "config.Presets.ArtifactsDynamicVersion": "explicit false kept by ForceSendFields", + "resources.Lifecycle.PreventDestroy": "explicit false kept by ForceSendFields", + + "resources.DashboardConfig.EmbedCredentials": "resource input", + "resources.PostgresProjectConfig.PurgeOnDelete": "resource input", + "resources.PostgresBranchConfig.PurgeOnDelete": "resource input", + "resources.PostgresBranchConfig.ReplaceExisting": "resource input", + "resources.PostgresEndpointConfig.ReplaceExisting": "resource input", + "resources.PostgresDatabaseConfig.ReplaceExisting": "resource input", + "resources.PostgresRoleConfig.ReplaceExisting": "resource input", +} + +// Every bool field of a CLI-defined config struct must be a *bool. A plain bool +// loses an explicit false: the typed-to-dynamic conversion that runs after every +// mutator drops zero values, so the false never shows in the config, the bundle +// bitmap cannot record it, and a target cannot override true with false. +func TestConfigBoolsArePointers(t *testing.T) { + found := map[string]bool{} + findPlainBools(reflect.TypeFor[Root](), map[reflect.Type]bool{}, found) + + for name := range found { + _, ok := plainBools[name] + assert.True(t, ok, "%s is a bool; declare it as *bool so that an explicit false is kept", name) + } + for name := range plainBools { + assert.True(t, found[name], "%s is no longer a bool; remove it from plainBools", name) } - checkOmitemptyBools(t, reflect.TypeFor[Root](), skip, map[reflect.Type]bool{}) } -func checkOmitemptyBools(t *testing.T, typ reflect.Type, skip, seen map[reflect.Type]bool) { +// findPlainBools records the bool fields of the CLI-defined structs reachable from typ. +func findPlainBools(typ reflect.Type, seen map[reflect.Type]bool, found map[string]bool) { for typ.Kind() == reflect.Pointer || typ.Kind() == reflect.Slice || typ.Kind() == reflect.Map { typ = typ.Elem() } @@ -33,17 +52,14 @@ func checkOmitemptyBools(t *testing.T, typ reflect.Type, skip, seen map[reflect. return } seen[typ] = true - fsf, ok := typ.FieldByName("ForceSendFields") - keepsZeros := ok && len(fsf.Index) == 1 - cliDefined := strings.HasPrefix(typ.PkgPath(), "github.com/databricks/cli/") && !skip[typ] + cliDefined := strings.HasPrefix(typ.PkgPath(), "github.com/databricks/cli/") for f := range typ.Fields() { - checkOmitemptyBools(t, f.Type, skip, seen) - // Readonly fields are set by the CLI, not by users. - if structtag.BundleTag(f.Tag.Get("bundle")).ReadOnly() { + if !f.IsExported() || f.Tag.Get("json") == "-" { continue } - if cliDefined && !keepsZeros && f.Type.Kind() == reflect.Bool && strings.Contains(f.Tag.Get("json"), ",omitempty") { - assert.Fail(t, "omitempty bool", "%s.%s must be a *bool so that an explicit false is kept", typ.Name(), f.Name) + findPlainBools(f.Type, seen, found) + if cliDefined && f.Type.Kind() == reflect.Bool { + found[typ.String()+"."+f.Name] = true } } } From 9a113f76630e9e8c9b778e02cda94099d7809948 Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 23:56:30 +0100 Subject: [PATCH 5/5] Postgres and deployment bools: *bool instead of ForceSendFields replace_existing on postgres endpoints, databases and roles lost an explicit false like the other plain bools did: their configs declare no ForceSendFields. They become *bool. replace_existing/purge_on_delete on postgres branches and purge_on_delete on postgres projects kept it only through the ForceSendFields those configs declared for these fields; bundle.deployment.fail_on_active_runs likewise (#7117). They become *bool and those ForceSendFields go away; the postgres configs keep their MarshalJSON/UnmarshalJSON, which exist because they embed an SDK spec. The direct engine reads them when building the create and delete requests, and RemapState leaves them unset. Lifecycle, Presets and DashboardConfig keep their ForceSendFields: it covers more than their bool (depends_on, the other presets, the dashboards API). The acceptance test overrides every one of these fields from a target. Co-authored-by: Isaac --- .nextchanges/bundles/7412.md | 2 +- .../refschema/output/postgres_branches.txt | 4 +- .../refschema/output/postgres_databases.txt | 2 +- .../refschema/output/postgres_endpoints.txt | 2 +- .../refschema/output/postgres_projects.txt | 2 +- .../refschema/output/postgres_roles.txt | 2 +- acceptance/bundle/refschema/output/root.txt | 6 +- .../validate/explicit_false/databricks.yml | 64 +++++++++++++++++++ .../bundle/validate/explicit_false/output.txt | 37 +++++++++++ .../bundle/validate/explicit_false/script | 5 +- bundle/config/deployment.go | 7 +- bundle/config/optional_bools_test.go | 16 ++--- bundle/config/resources/postgres_branch.go | 10 +-- .../config/resources/postgres_branch_test.go | 35 +++++++--- bundle/config/resources/postgres_database.go | 2 +- bundle/config/resources/postgres_endpoint.go | 2 +- bundle/config/resources/postgres_project.go | 8 +-- bundle/config/resources/postgres_role.go | 2 +- bundle/direct/dresources/postgres_branch.go | 14 ++-- bundle/direct/dresources/postgres_database.go | 6 +- bundle/direct/dresources/postgres_endpoint.go | 6 +- bundle/direct/dresources/postgres_project.go | 14 ++-- bundle/direct/dresources/postgres_role.go | 8 +-- bundle/statemgmt/check_running_resources.go | 2 +- cmd/apps/deploy_bundle.go | 2 +- cmd/apps/deploy_bundle_test.go | 4 +- cmd/bundle/deploy.go | 2 +- cmd/pipelines/deploy.go | 2 +- libs/structs/structaccess/bundle_test.go | 8 +-- 29 files changed, 185 insertions(+), 91 deletions(-) diff --git a/.nextchanges/bundles/7412.md b/.nextchanges/bundles/7412.md index 884a16358fb..0c586ed4e8b 100644 --- a/.nextchanges/bundles/7412.md +++ b/.nextchanges/bundles/7412.md @@ -1 +1 @@ -* Fixed an explicit `false` on optional boolean fields (`variables.*.sensitive`, `artifacts.*.dynamic_version`, `bundle.deployment.lock.force`, `workspace.azure_use_msi` and the `experimental.*` flags) being dropped, which hid it from `bundle validate -o json` and kept a target from overriding `true` with it. ([#7412](https://github.com/databricks/cli/pull/7412)) +* Fixed an explicit `false` on optional boolean fields (`variables.*.sensitive`, `artifacts.*.dynamic_version`, `bundle.deployment.lock.force`, `workspace.azure_use_msi`, `replace_existing` on postgres endpoints, databases and roles, and the `experimental.*` flags) being dropped, which hid it from `bundle validate -o json` and kept a target from overriding `true` with it. ([#7412](https://github.com/databricks/cli/pull/7412)) diff --git a/acceptance/bundle/refschema/output/postgres_branches.txt b/acceptance/bundle/refschema/output/postgres_branches.txt index 85248e7a93a..abfb99d9c83 100644 --- a/acceptance/bundle/refschema/output/postgres_branches.txt +++ b/acceptance/bundle/refschema/output/postgres_branches.txt @@ -12,8 +12,8 @@ resources.postgres_branches.*.modified_status string INPUT resources.postgres_branches.*.name string REMOTE resources.postgres_branches.*.no_expiry bool ALL resources.postgres_branches.*.parent string ALL -resources.postgres_branches.*.purge_on_delete bool INPUT STATE -resources.postgres_branches.*.replace_existing bool INPUT STATE +resources.postgres_branches.*.purge_on_delete *bool INPUT STATE +resources.postgres_branches.*.replace_existing *bool INPUT STATE resources.postgres_branches.*.source_branch string ALL resources.postgres_branches.*.source_branch_lsn string ALL resources.postgres_branches.*.source_branch_time *time.Time ALL diff --git a/acceptance/bundle/refschema/output/postgres_databases.txt b/acceptance/bundle/refschema/output/postgres_databases.txt index 2ccb3ba2b68..1cb3c2818a0 100644 --- a/acceptance/bundle/refschema/output/postgres_databases.txt +++ b/acceptance/bundle/refschema/output/postgres_databases.txt @@ -10,7 +10,7 @@ resources.postgres_databases.*.modified_status string INPUT resources.postgres_databases.*.name string REMOTE resources.postgres_databases.*.parent string ALL resources.postgres_databases.*.postgres_database string ALL -resources.postgres_databases.*.replace_existing bool INPUT STATE +resources.postgres_databases.*.replace_existing *bool INPUT STATE resources.postgres_databases.*.role string ALL resources.postgres_databases.*.status *postgres.DatabaseDatabaseStatus REMOTE resources.postgres_databases.*.status.database_id string REMOTE diff --git a/acceptance/bundle/refschema/output/postgres_endpoints.txt b/acceptance/bundle/refschema/output/postgres_endpoints.txt index 32f2395228b..befc702f987 100644 --- a/acceptance/bundle/refschema/output/postgres_endpoints.txt +++ b/acceptance/bundle/refschema/output/postgres_endpoints.txt @@ -18,7 +18,7 @@ resources.postgres_endpoints.*.modified_status string INPUT resources.postgres_endpoints.*.name string REMOTE resources.postgres_endpoints.*.no_suspension bool ALL resources.postgres_endpoints.*.parent string ALL -resources.postgres_endpoints.*.replace_existing bool INPUT STATE +resources.postgres_endpoints.*.replace_existing *bool INPUT STATE resources.postgres_endpoints.*.settings *postgres.EndpointSettings ALL resources.postgres_endpoints.*.settings.pg_settings map[string]string ALL resources.postgres_endpoints.*.settings.pg_settings.* string ALL diff --git a/acceptance/bundle/refschema/output/postgres_projects.txt b/acceptance/bundle/refschema/output/postgres_projects.txt index 575b12db52d..11634ec55db 100644 --- a/acceptance/bundle/refschema/output/postgres_projects.txt +++ b/acceptance/bundle/refschema/output/postgres_projects.txt @@ -35,7 +35,7 @@ resources.postgres_projects.*.modified_status string INPUT resources.postgres_projects.*.name string REMOTE resources.postgres_projects.*.pg_version int ALL resources.postgres_projects.*.project_id string ALL -resources.postgres_projects.*.purge_on_delete bool INPUT STATE +resources.postgres_projects.*.purge_on_delete *bool INPUT STATE resources.postgres_projects.*.purge_time *time.Time REMOTE resources.postgres_projects.*.status *postgres.ProjectStatus REMOTE resources.postgres_projects.*.status.branch_logical_size_limit_bytes int64 REMOTE diff --git a/acceptance/bundle/refschema/output/postgres_roles.txt b/acceptance/bundle/refschema/output/postgres_roles.txt index b77c3258488..4cf89374c8e 100644 --- a/acceptance/bundle/refschema/output/postgres_roles.txt +++ b/acceptance/bundle/refschema/output/postgres_roles.txt @@ -17,7 +17,7 @@ resources.postgres_roles.*.modified_status string INPUT resources.postgres_roles.*.name string REMOTE resources.postgres_roles.*.parent string ALL resources.postgres_roles.*.postgres_role string ALL -resources.postgres_roles.*.replace_existing bool INPUT STATE +resources.postgres_roles.*.replace_existing *bool INPUT STATE resources.postgres_roles.*.role_id string ALL resources.postgres_roles.*.status *postgres.RoleRoleStatus REMOTE resources.postgres_roles.*.status.attributes *postgres.RoleAttributes REMOTE diff --git a/acceptance/bundle/refschema/output/root.txt b/acceptance/bundle/refschema/output/root.txt index e3ccaceef5c..e4d905fca0c 100644 --- a/acceptance/bundle/refschema/output/root.txt +++ b/acceptance/bundle/refschema/output/root.txt @@ -29,7 +29,7 @@ bundle.compute_id string INPUT bundle.databricks_cli_version string INPUT bundle.deployment config.Deployment INPUT bundle.deployment.deployment_id string INPUT -bundle.deployment.fail_on_active_runs bool INPUT +bundle.deployment.fail_on_active_runs *bool INPUT bundle.deployment.latest_version_id int INPUT bundle.deployment.lock config.Lock INPUT bundle.deployment.lock.enabled *bool INPUT @@ -75,7 +75,7 @@ environments.*.bundle.compute_id string INPUT environments.*.bundle.databricks_cli_version string INPUT environments.*.bundle.deployment config.Deployment INPUT environments.*.bundle.deployment.deployment_id string INPUT -environments.*.bundle.deployment.fail_on_active_runs bool INPUT +environments.*.bundle.deployment.fail_on_active_runs *bool INPUT environments.*.bundle.deployment.latest_version_id int INPUT environments.*.bundle.deployment.lock config.Lock INPUT environments.*.bundle.deployment.lock.enabled *bool INPUT @@ -292,7 +292,7 @@ targets.*.bundle.compute_id string INPUT targets.*.bundle.databricks_cli_version string INPUT targets.*.bundle.deployment config.Deployment INPUT targets.*.bundle.deployment.deployment_id string INPUT -targets.*.bundle.deployment.fail_on_active_runs bool INPUT +targets.*.bundle.deployment.fail_on_active_runs *bool INPUT targets.*.bundle.deployment.latest_version_id int INPUT targets.*.bundle.deployment.lock config.Lock INPUT targets.*.bundle.deployment.lock.enabled *bool INPUT diff --git a/acceptance/bundle/validate/explicit_false/databricks.yml b/acceptance/bundle/validate/explicit_false/databricks.yml index 383ec0d49d1..ecd9fd56563 100644 --- a/acceptance/bundle/validate/explicit_false/databricks.yml +++ b/acceptance/bundle/validate/explicit_false/databricks.yml @@ -1,6 +1,7 @@ bundle: name: test-bundle deployment: + fail_on_active_runs: true lock: force: true @@ -22,6 +23,46 @@ artifacts: build: "true" dynamic_version: true +presets: + artifacts_dynamic_version: true + +resources: + dashboards: + d: + display_name: d + warehouse_id: w + embed_credentials: true + postgres_projects: + p: + project_id: p + purge_on_delete: true + postgres_branches: + b: + parent: ${resources.postgres_projects.p.name} + branch_id: b + no_expiry: true + purge_on_delete: true + replace_existing: true + postgres_endpoints: + e: + parent: ${resources.postgres_branches.b.name} + endpoint_id: e + endpoint_type: ENDPOINT_TYPE_READ_WRITE + replace_existing: true + postgres_roles: + r: + parent: ${resources.postgres_branches.b.name} + role_id: r + postgres_role: app_role + replace_existing: true + postgres_databases: + db: + parent: ${resources.postgres_branches.b.name} + database_id: db + postgres_database: app_db + role: ${resources.postgres_roles.r.name} + replace_existing: true + experimental: python_wheel_wrapper: false use_legacy_run_as: false @@ -34,6 +75,7 @@ targets: # Each of these turns off what the root config turns on. bundle: deployment: + fail_on_active_runs: false lock: force: false workspace: @@ -41,3 +83,25 @@ targets: artifacts: overridden: dynamic_version: false + presets: + artifacts_dynamic_version: false + resources: + dashboards: + d: + embed_credentials: false + postgres_projects: + p: + purge_on_delete: false + postgres_branches: + b: + purge_on_delete: false + replace_existing: false + postgres_endpoints: + e: + replace_existing: false + postgres_roles: + r: + replace_existing: false + postgres_databases: + db: + replace_existing: false diff --git a/acceptance/bundle/validate/explicit_false/output.txt b/acceptance/bundle/validate/explicit_false/output.txt index 5308e136bc7..223a0304d03 100644 --- a/acceptance/bundle/validate/explicit_false/output.txt +++ b/acceptance/bundle/validate/explicit_false/output.txt @@ -1,10 +1,14 @@ >>> [CLI] bundle validate -o json { + "fail_on_active_runs": false, "lock_force": false, "workspace": { "azure_use_msi": false }, + "presets": { + "artifacts_dynamic_version": false + }, "variables": { "plain": { "sensitive": false @@ -18,6 +22,39 @@ "dynamic_version": false } }, + "resources": { + "dashboards": { + "d": { + "embed_credentials": false + } + }, + "postgres_branches": { + "b": { + "purge_on_delete": false, + "replace_existing": false + } + }, + "postgres_databases": { + "db": { + "replace_existing": false + } + }, + "postgres_endpoints": { + "e": { + "replace_existing": false + } + }, + "postgres_projects": { + "p": { + "purge_on_delete": false + } + }, + "postgres_roles": { + "r": { + "replace_existing": false + } + } + }, "experimental": { "python_wheel_wrapper": false, "skip_artifact_cleanup": false, diff --git a/acceptance/bundle/validate/explicit_false/script b/acceptance/bundle/validate/explicit_false/script index 832319ec16d..4a48350fc3a 100644 --- a/acceptance/bundle/validate/explicit_false/script +++ b/acceptance/bundle/validate/explicit_false/script @@ -1,9 +1,12 @@ # An explicit false is kept in the config, and a target can override true with false -# (lock.force, azure_use_msi and artifacts.overridden.dynamic_version). +# (everything under targets.dev). trace $CLI bundle validate -o json | jq '{ + fail_on_active_runs: .bundle.deployment.fail_on_active_runs, lock_force: .bundle.deployment.lock.force, workspace: .workspace | {azure_use_msi}, + presets: .presets | {artifacts_dynamic_version}, variables: .variables | map_values({sensitive}), artifacts: .artifacts | map_values({dynamic_version}), + resources: .resources | map_values(map_values({embed_credentials, purge_on_delete, replace_existing} | with_entries(select(.value != null)))), experimental }' diff --git a/bundle/config/deployment.go b/bundle/config/deployment.go index 3b3f65608b3..4a9630b4292 100644 --- a/bundle/config/deployment.go +++ b/bundle/config/deployment.go @@ -3,7 +3,7 @@ package config type Deployment struct { // FailOnActiveRuns specifies whether to fail the deployment if there are // running jobs or pipelines in the workspace. Defaults to false. - FailOnActiveRuns bool `json:"fail_on_active_runs,omitempty"` + FailOnActiveRuns *bool `json:"fail_on_active_runs,omitempty"` // Lock configures locking behavior on deployment. Lock Lock `json:"lock,omitempty"` @@ -14,9 +14,4 @@ type Deployment struct { // LatestVersionID is the most recent version recorded for the deployment. It is // zero when the deployment exists but has no version yet. LatestVersionID int `json:"latest_version_id,omitempty"` - - // ForceSendFields records which fields were explicitly set so that zero-value - // overrides can replace non-zero base values (e.g., fail_on_active_runs: false). - // This field is not serialized to JSON. - ForceSendFields []string `json:"-"` } diff --git a/bundle/config/optional_bools_test.go b/bundle/config/optional_bools_test.go index f12e2c40cdf..89b0f5b23a5 100644 --- a/bundle/config/optional_bools_test.go +++ b/bundle/config/optional_bools_test.go @@ -13,17 +13,11 @@ import ( var plainBools = map[string]string{ "config.Bundle.Force": "readonly: set from --force, not by users", - "config.Deployment.FailOnActiveRuns": "explicit false kept by ForceSendFields", - "config.Presets.ArtifactsDynamicVersion": "explicit false kept by ForceSendFields", - "resources.Lifecycle.PreventDestroy": "explicit false kept by ForceSendFields", - - "resources.DashboardConfig.EmbedCredentials": "resource input", - "resources.PostgresProjectConfig.PurgeOnDelete": "resource input", - "resources.PostgresBranchConfig.PurgeOnDelete": "resource input", - "resources.PostgresBranchConfig.ReplaceExisting": "resource input", - "resources.PostgresEndpointConfig.ReplaceExisting": "resource input", - "resources.PostgresDatabaseConfig.ReplaceExisting": "resource input", - "resources.PostgresRoleConfig.ReplaceExisting": "resource input", + // These structs declare ForceSendFields for more than this field, so an + // explicit false is kept through it. + "config.Presets.ArtifactsDynamicVersion": "ForceSendFields also keeps the other presets' zero values", + "resources.Lifecycle.PreventDestroy": "ForceSendFields also keeps an explicit depends_on: []", + "resources.DashboardConfig.EmbedCredentials": "ForceSendFields is passed on to the dashboards API", } // Every bool field of a CLI-defined config struct must be a *bool. A plain bool diff --git a/bundle/config/resources/postgres_branch.go b/bundle/config/resources/postgres_branch.go index a248667a87e..7e20f6001c6 100644 --- a/bundle/config/resources/postgres_branch.go +++ b/bundle/config/resources/postgres_branch.go @@ -23,19 +23,13 @@ type PostgresBranchConfig struct { // ReplaceExisting, when true, takes over an existing branch with the same ID // instead of returning ALREADY_EXISTS. Used to manage the implicitly-created // production branch of a new project. Input-only: not returned by the GET API. - ReplaceExisting bool `json:"replace_existing,omitempty"` + ReplaceExisting *bool `json:"replace_existing,omitempty"` // PurgeOnDelete, when true, hard-deletes the branch on destroy (Purge=true on // DeleteBranch). When false or unset, the backend performs a soft delete that // can be undone within the branch's retention window. Input-only: not // returned by the GET API. - PurgeOnDelete bool `json:"purge_on_delete,omitempty"` - - // ForceSendFields shadows the embedded BranchSpec.ForceSendFields so the - // SDK's marshal package tracks zero-value top-level fields (branch_id, - // parent, replace_existing, purge_on_delete) here instead of polluting - // BranchSpec.ForceSendFields with names that don't exist in that struct. - ForceSendFields []string `json:"-" url:"-"` + PurgeOnDelete *bool `json:"purge_on_delete,omitempty"` } func (c *PostgresBranchConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/postgres_branch_test.go b/bundle/config/resources/postgres_branch_test.go index a9c846c38de..f1d74ed5008 100644 --- a/bundle/config/resources/postgres_branch_test.go +++ b/bundle/config/resources/postgres_branch_test.go @@ -11,18 +11,37 @@ import ( // TestPostgresBranchConfigMarshalHonorsForceSendFields pins the reason // PostgresBranchConfig.MarshalJSON is declared on a value receiver. // -// PurgeOnDelete is omitempty, so its zero value is emitted only because -// ForceSendFields names it, and the SDK marshaler is what honors ForceSendFields. -// With a pointer-receiver MarshalJSON, only *T satisfies json.Marshaler, so -// marshalling a value falls back to plain encoding/json -- which ignores -// ForceSendFields (tagged json:"-") and would drop the field. +// NoExpiry is omitempty, so its zero value is emitted only because the embedded +// BranchSpec.ForceSendFields names it, and the SDK marshaler is what honors +// ForceSendFields. With a pointer-receiver MarshalJSON, only *T satisfies +// json.Marshaler, so marshalling a value falls back to the promoted +// BranchSpec.MarshalJSON, which drops the config's own fields. func TestPostgresBranchConfigMarshalHonorsForceSendFields(t *testing.T) { - c := PostgresBranchConfig{BranchId: "b1", Parent: "projects/p1"} - c.ForceSendFields = []string{"PurgeOnDelete"} + c := PostgresBranchConfig{ + BranchId: "b1", Parent: "projects/p1", + ForceSendFields: []string{"NoExpiry"}, + } b, err := json.Marshal(c) require.NoError(t, err) - assert.JSONEq(t, `{"branch_id":"b1","parent":"projects/p1","purge_on_delete":false}`, string(b)) + assert.JSONEq(t, `{"branch_id":"b1","parent":"projects/p1","no_expiry":false}`, string(b)) +} + +// An explicit false on the config's own *bool fields survives a JSON round trip +// (as the direct engine's state does). These fields are declared outside +// BranchSpec, so they must not end up in BranchSpec.ForceSendFields, which the +// SDK marshaler would reject. +func TestPostgresBranchConfigExplicitFalseRoundTrip(t *testing.T) { + in := `{"branch_id":"b1","parent":"projects/p1","replace_existing":false,"purge_on_delete":false}` + + var c PostgresBranchConfig + require.NoError(t, json.Unmarshal([]byte(in), &c)) + assert.Equal(t, new(false), c.ReplaceExisting) + assert.Equal(t, new(false), c.PurgeOnDelete) + + out, err := json.Marshal(c) + require.NoError(t, err) + assert.JSONEq(t, in, string(out)) } // TestPostgresBranchMarshalValueAndPointerAgree guards the invariant directly: diff --git a/bundle/config/resources/postgres_database.go b/bundle/config/resources/postgres_database.go index efbe5984d5e..43c106ebf7d 100644 --- a/bundle/config/resources/postgres_database.go +++ b/bundle/config/resources/postgres_database.go @@ -24,7 +24,7 @@ type PostgresDatabaseConfig struct { // ReplaceExisting, when true, takes over an existing database with the same ID // instead of returning ALREADY_EXISTS. Used to manage a database that already // exists on the branch. Input-only: not returned by the GET API. - ReplaceExisting bool `json:"replace_existing,omitempty"` + ReplaceExisting *bool `json:"replace_existing,omitempty"` } func (c *PostgresDatabaseConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/postgres_endpoint.go b/bundle/config/resources/postgres_endpoint.go index 0947045a719..09433571c30 100644 --- a/bundle/config/resources/postgres_endpoint.go +++ b/bundle/config/resources/postgres_endpoint.go @@ -23,7 +23,7 @@ type PostgresEndpointConfig struct { // ReplaceExisting, when true, takes over an existing endpoint with the same ID // instead of returning ALREADY_EXISTS. Used to manage the implicitly-created // primary read-write endpoint of a new branch. Input-only: not returned by the GET API. - ReplaceExisting bool `json:"replace_existing,omitempty"` + ReplaceExisting *bool `json:"replace_existing,omitempty"` } func (c *PostgresEndpointConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/postgres_project.go b/bundle/config/resources/postgres_project.go index 0cdfbbcc2af..5843d5e0c69 100644 --- a/bundle/config/resources/postgres_project.go +++ b/bundle/config/resources/postgres_project.go @@ -21,13 +21,7 @@ type PostgresProjectConfig struct { // DeleteProject). When false or unset, the backend performs a soft delete that // can be undone within the project's retention window. Input-only: not // returned by the GET API. - PurgeOnDelete bool `json:"purge_on_delete,omitempty"` - - // ForceSendFields shadows the embedded ProjectSpec.ForceSendFields so the - // SDK's marshal package tracks zero-value top-level fields (project_id, - // purge_on_delete) here instead of polluting ProjectSpec.ForceSendFields - // with names that don't exist in that struct. - ForceSendFields []string `json:"-" url:"-"` + PurgeOnDelete *bool `json:"purge_on_delete,omitempty"` } func (c *PostgresProjectConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/config/resources/postgres_role.go b/bundle/config/resources/postgres_role.go index 7cc30a3ee0a..85244cc92c5 100644 --- a/bundle/config/resources/postgres_role.go +++ b/bundle/config/resources/postgres_role.go @@ -25,7 +25,7 @@ type PostgresRoleConfig struct { // instead of returning ALREADY_EXISTS. Used to manage a role that already // exists on the branch (e.g. inherited from the parent branch). Input-only: // not returned by the GET API. - ReplaceExisting bool `json:"replace_existing,omitempty"` + ReplaceExisting *bool `json:"replace_existing,omitempty"` } func (c *PostgresRoleConfig) UnmarshalJSON(b []byte) error { diff --git a/bundle/direct/dresources/postgres_branch.go b/bundle/direct/dresources/postgres_branch.go index 57bba56707d..19fac982f08 100644 --- a/bundle/direct/dresources/postgres_branch.go +++ b/bundle/direct/dresources/postgres_branch.go @@ -55,7 +55,6 @@ func (*ResourcePostgresBranch) PrepareState(input *resources.PostgresBranch) *Po ReplaceExisting: input.ReplaceExisting, PurgeOnDelete: input.PurgeOnDelete, BranchSpec: input.BranchSpec, - ForceSendFields: input.ForceSendFields, } } @@ -65,13 +64,12 @@ func (*ResourcePostgresBranch) RemapState(remote *PostgresBranchRemote) *Postgre Parent: remote.Parent, // replace_existing is a create-time-only flag; the GET API never returns - // it, so RemapState leaves it false. - ReplaceExisting: false, + // it, so RemapState leaves it unset. + ReplaceExisting: nil, // purge_on_delete is a delete-time query parameter; the GET API never - // returns it, so RemapState leaves it false. - PurgeOnDelete: false, - ForceSendFields: nil, + // returns it, so RemapState leaves it unset. + PurgeOnDelete: nil, BranchSpec: remote.BranchSpec, } @@ -123,7 +121,7 @@ func (r *ResourcePostgresBranch) DoCreate(ctx context.Context, config *PostgresB UpdateTime: nil, ForceSendFields: nil, }, - ReplaceExisting: config.ReplaceExisting, + ReplaceExisting: config.ReplaceExisting != nil && *config.ReplaceExisting, ForceSendFields: nil, }) if err != nil { @@ -203,7 +201,7 @@ func (r *ResourcePostgresBranch) DoUpdate(ctx context.Context, id string, config func (r *ResourcePostgresBranch) DoDelete(ctx context.Context, id string, state *PostgresBranchState) error { waiter, err := r.client.Postgres.DeleteBranch(ctx, postgres.DeleteBranchRequest{ Name: id, - Purge: state.PurgeOnDelete, + Purge: state.PurgeOnDelete != nil && *state.PurgeOnDelete, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/direct/dresources/postgres_database.go b/bundle/direct/dresources/postgres_database.go index 613f5d90270..154163b4c8f 100644 --- a/bundle/direct/dresources/postgres_database.go +++ b/bundle/direct/dresources/postgres_database.go @@ -62,8 +62,8 @@ func (*ResourcePostgresDatabase) RemapState(remote *PostgresDatabaseRemote) *Pos Parent: remote.Parent, // replace_existing is a create-time-only flag; the GET API never returns - // it, so RemapState leaves it false. - ReplaceExisting: false, + // it, so RemapState leaves it unset. + ReplaceExisting: nil, DatabaseDatabaseSpec: remote.DatabaseDatabaseSpec, } @@ -113,7 +113,7 @@ func (r *ResourcePostgresDatabase) DoCreate(ctx context.Context, config *Postgre UpdateTime: nil, ForceSendFields: nil, }, - ReplaceExisting: config.ReplaceExisting, + ReplaceExisting: config.ReplaceExisting != nil && *config.ReplaceExisting, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/direct/dresources/postgres_endpoint.go b/bundle/direct/dresources/postgres_endpoint.go index 786f52b8b21..48ea312005a 100644 --- a/bundle/direct/dresources/postgres_endpoint.go +++ b/bundle/direct/dresources/postgres_endpoint.go @@ -71,8 +71,8 @@ func (*ResourcePostgresEndpoint) RemapState(remote *PostgresEndpointRemote) *Pos Parent: remote.Parent, // replace_existing is a create-time-only flag; the GET API never returns - // it, so RemapState leaves it false. - ReplaceExisting: false, + // it, so RemapState leaves it unset. + ReplaceExisting: nil, EndpointSpec: remote.EndpointSpec, } @@ -154,7 +154,7 @@ func (r *ResourcePostgresEndpoint) DoCreate(ctx context.Context, config *Postgre UpdateTime: nil, ForceSendFields: nil, }, - ReplaceExisting: config.ReplaceExisting, + ReplaceExisting: config.ReplaceExisting != nil && *config.ReplaceExisting, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/direct/dresources/postgres_project.go b/bundle/direct/dresources/postgres_project.go index bdb886e9437..e62b87906cc 100644 --- a/bundle/direct/dresources/postgres_project.go +++ b/bundle/direct/dresources/postgres_project.go @@ -52,10 +52,9 @@ func (*ResourcePostgresProject) New(client *databricks.WorkspaceClient) *Resourc func (*ResourcePostgresProject) PrepareState(input *resources.PostgresProject) *PostgresProjectState { return &PostgresProjectState{ - ProjectId: input.ProjectId, - PurgeOnDelete: input.PurgeOnDelete, - ProjectSpec: input.ProjectSpec, - ForceSendFields: input.ForceSendFields, + ProjectId: input.ProjectId, + PurgeOnDelete: input.PurgeOnDelete, + ProjectSpec: input.ProjectSpec, } } @@ -65,9 +64,8 @@ func (*ResourcePostgresProject) RemapState(remote *PostgresProjectRemote) *Postg ProjectSpec: remote.ProjectSpec, // purge_on_delete is a delete-time query parameter; the GET API never - // returns it, so RemapState leaves it false. - PurgeOnDelete: false, - ForceSendFields: nil, + // returns it, so RemapState leaves it unset. + PurgeOnDelete: nil, } } @@ -200,7 +198,7 @@ func (r *ResourcePostgresProject) DoUpdate(ctx context.Context, id string, confi func (r *ResourcePostgresProject) DoDelete(ctx context.Context, id string, state *PostgresProjectState) error { waiter, err := r.client.Postgres.DeleteProject(ctx, postgres.DeleteProjectRequest{ Name: id, - Purge: state.PurgeOnDelete, + Purge: state.PurgeOnDelete != nil && *state.PurgeOnDelete, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/direct/dresources/postgres_role.go b/bundle/direct/dresources/postgres_role.go index 10ef250e37c..43eef50a657 100644 --- a/bundle/direct/dresources/postgres_role.go +++ b/bundle/direct/dresources/postgres_role.go @@ -32,7 +32,7 @@ type PostgresRoleState struct { // ReplaceExisting takes over an existing role with the same ID on create // instead of returning ALREADY_EXISTS. Input-only: not returned by the GET API. - ReplaceExisting bool `json:"replace_existing,omitempty"` + ReplaceExisting *bool `json:"replace_existing,omitempty"` } // PostgresRoleRemote is the return type for DoRead. It embeds RoleRoleSpec so that @@ -87,8 +87,8 @@ func (*ResourcePostgresRole) RemapState(remote *PostgresRoleRemote) *PostgresRol Parent: remote.Parent, // replace_existing is a create-time-only flag; the GET API never returns - // it, so RemapState leaves it false. - ReplaceExisting: false, + // it, so RemapState leaves it unset. + ReplaceExisting: nil, RoleRoleSpec: remote.RoleRoleSpec, } @@ -138,7 +138,7 @@ func (r *ResourcePostgresRole) DoCreate(ctx context.Context, config *PostgresRol UpdateTime: nil, ForceSendFields: nil, }, - ReplaceExisting: config.ReplaceExisting, + ReplaceExisting: config.ReplaceExisting != nil && *config.ReplaceExisting, ForceSendFields: nil, }) if err != nil { diff --git a/bundle/statemgmt/check_running_resources.go b/bundle/statemgmt/check_running_resources.go index 63681bd5eb9..4c9cae0fc21 100644 --- a/bundle/statemgmt/check_running_resources.go +++ b/bundle/statemgmt/check_running_resources.go @@ -31,7 +31,7 @@ func (l *checkRunningResources) Name() string { } func (l *checkRunningResources) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { - if !b.Config.Bundle.Deployment.FailOnActiveRuns { + if !config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.FailOnActiveRuns) { return nil } diff --git a/cmd/apps/deploy_bundle.go b/cmd/apps/deploy_bundle.go index b390f45e132..50f9cdd5964 100644 --- a/cmd/apps/deploy_bundle.go +++ b/cmd/apps/deploy_bundle.go @@ -60,7 +60,7 @@ func applyDeployFlags(cmd *cobra.Command, b *bundle.Bundle, opts bundleDeployOpt b.Config.Bundle.ClusterId = opts.clusterId } if cmd.Flag("fail-on-active-runs").Changed { - b.Config.Bundle.Deployment.FailOnActiveRuns = opts.failOnActiveRuns + b.Config.Bundle.Deployment.FailOnActiveRuns = new(opts.failOnActiveRuns) } } diff --git a/cmd/apps/deploy_bundle_test.go b/cmd/apps/deploy_bundle_test.go index 36633d6e5c7..311918846cd 100644 --- a/cmd/apps/deploy_bundle_test.go +++ b/cmd/apps/deploy_bundle_test.go @@ -188,14 +188,14 @@ func TestApplyDeployFlags(t *testing.T) { { name: "failOnActiveRuns ignored when flag unchanged", opts: bundleDeployOptions{failOnActiveRuns: true}, - assertion: func(t *testing.T, b *bundle.Bundle) { assert.False(t, b.Config.Bundle.Deployment.FailOnActiveRuns) }, + assertion: func(t *testing.T, b *bundle.Bundle) { assert.Nil(t, b.Config.Bundle.Deployment.FailOnActiveRuns) }, }, { name: "failOnActiveRuns applies when --fail-on-active-runs is set", args: []string{"--fail-on-active-runs"}, opts: bundleDeployOptions{failOnActiveRuns: true}, assertion: func(t *testing.T, b *bundle.Bundle) { - assert.True(t, b.Config.Bundle.Deployment.FailOnActiveRuns) + assert.True(t, config.IsExplicitlyEnabled(b.Config.Bundle.Deployment.FailOnActiveRuns)) }, }, } diff --git a/cmd/bundle/deploy.go b/cmd/bundle/deploy.go index 6f1dce84d32..571b822a192 100644 --- a/cmd/bundle/deploy.go +++ b/cmd/bundle/deploy.go @@ -64,7 +64,7 @@ See https://docs.databricks.com/en/dev-tools/bundles/index.html for more informa b.Config.Bundle.ClusterId = clusterId } if cmd.Flag("fail-on-active-runs").Changed { - b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns + b.Config.Bundle.Deployment.FailOnActiveRuns = new(failOnActiveRuns) } }, Verbose: verbose, diff --git a/cmd/pipelines/deploy.go b/cmd/pipelines/deploy.go index 7275eb246b5..9cd4e3b95ff 100644 --- a/cmd/pipelines/deploy.go +++ b/cmd/pipelines/deploy.go @@ -45,7 +45,7 @@ func deployCommand() *cobra.Command { b.Quiet = bundle.QuietLevel(quiet) if cmd.Flag("fail-on-active-runs").Changed { - b.Config.Bundle.Deployment.FailOnActiveRuns = failOnActiveRuns + b.Config.Bundle.Deployment.FailOnActiveRuns = new(failOnActiveRuns) } }, Verbose: verbose, diff --git a/libs/structs/structaccess/bundle_test.go b/libs/structs/structaccess/bundle_test.go index 4de21ede392..55d08a39b4f 100644 --- a/libs/structs/structaccess/bundle_test.go +++ b/libs/structs/structaccess/bundle_test.go @@ -80,8 +80,7 @@ func TestGet_ConfigRoot_JobTagsAccess(t *testing.T) { // A bundle resource embeds a config struct that embeds the SDK request struct, so its // fields sit two levels down. Get, Set and ValidatePath all have to reach them, and -// ForceSendFields belongs to the struct that declares the field -- not to the outer one -// that shadows the name. +// ForceSendFields belongs to the struct that declares the field. func TestGetSet_DoublyEmbeddedField(t *testing.T) { project := &resources.PostgresProject{} //exhaustruct:ignore project.ProjectId = "p" @@ -97,8 +96,7 @@ func TestGetSet_DoublyEmbeddedField(t *testing.T) { // An explicit empty value is recorded on ProjectSpec, which declares the field. require.NoError(t, SetByString(project, "budget_policy_id", "")) - require.Contains(t, project.ProjectSpec.ForceSendFields, "BudgetPolicyId") - require.NotContains(t, project.ForceSendFields, "BudgetPolicyId") + require.Contains(t, project.ForceSendFields, "BudgetPolicyId") value, err = GetByString(project, "budget_policy_id") require.NoError(t, err) @@ -107,7 +105,7 @@ func TestGetSet_DoublyEmbeddedField(t *testing.T) { // And dropping it again leaves the field absent. require.NoError(t, SetByString(project, "budget_policy_id", nil)) - require.NotContains(t, project.ProjectSpec.ForceSendFields, "BudgetPolicyId") + require.NotContains(t, project.ForceSendFields, "BudgetPolicyId") value, err = GetByString(project, "budget_policy_id") require.NoError(t, err) require.Nil(t, value)