From f90f6f9dccb24dcb45e03d529e51f15b34a3dbea Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Mon, 9 Nov 2026 16:28:35 +0100 Subject: [PATCH 1/4] 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/4] 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/4] 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/4] 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 } } }