Do not load host from bundle for CLI commands when profile flag is used #2335

andrewnester · 2025-02-11T13:03:33Z

Changes

Now when profile flag is used we won't pick up host from bundle anymore and use the one provided by -p flag

Previous behaviour in the context of bundle

databricks current-user me -p profile_name
Error: cannot resolve bundle auth configuration: config host mismatch: profile uses host https://non-existing-subdomain.databricks.com, but CLI configured to use https://foo.com

New behaviour (make an api call)

databricks current-user me -p profile_name
{
  email: "[email protected]"
  ...
}

We still load bundle configuration when -t flag provide because we want to load host information from the target.

Fixes #1358

Tests

Added acceptance test

cmd/root/auth.go

acceptance/auth/bundle_and_profile/output.txt

denik · 2025-02-25T11:44:05Z

acceptance/auth/bundle_and_profile/script

@@ -15,7 +15,7 @@ trace errcode $CLI current-user me -t dev | jq .userName
 title "Inside the bundle, target and matching profile"
 trace errcode $CLI current-user me -t dev -p DEFAULT | jq .userName

-title "Inside the bundle, profile flag not matching bundle host. Badness: should use profile from flag instead and not fail"
+title "Inside the bundle, profile flag not matching bundle host. Should use profile from the flag and not the bundle."


nit: I'd replace "Should use profile from the flag" with 'Prefers profile passed via "-p profile_name" over the one specified by bundle host'.

denik · 2025-02-25T11:46:09Z

cmd/root/auth.go

@@ -195,6 +195,12 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
 		cfg.Profile = profile
 	}

+	_, isTargetFlagSet := targetFlagValue(cmd)
+	if !isTargetFlagSet && hasProfileFlag {


What happens if both -p and -t are set? Should that result in an error? or is it a valid use case as well?

This is a valid use case, for example, you want to make a CLI call to -t dev but it requires some different profile so you need to provide -p flag. If the hosts mismatch in such cases it will correct fail

If it's a valid case, should the condition be

if hasProfileFlag {?

so if -p is provided, then we always use that.

if we use this condition we will skip bundle configuration always, even when -t is provided. But we don't want to skip it when -t is provided

denik · 2025-02-25T11:47:37Z

cmd/root/bundle.go

+	return target
+}
+
+func targetFlagValue(cmd *cobra.Command) (string, bool) {


What's the motivation for ignoring env var but taking command line option into account?

Do you mean not having the same behaviour like with flag set?
The motivation that is the usage intention is different. You're likely to set DATABRICKS_CONFIG_PROFILE env for all the commands you use, like databricks bundle deploy and databricks clusters list equally.
While providing a flag to command is an explicit way to want it to be executed against different profile.

denik · 2025-02-25T13:50:30Z

cmd/root/auth.go

@@ -195,6 +195,12 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
 		cfg.Profile = profile
 	}

+	_, isTargetFlagSet := targetFlagValue(cmd)
+	if !isTargetFlagSet && hasProfileFlag {


If it's a valid case, should the condition be

if hasProfileFlag {?

so if -p is provided, then we always use that.

cmd/root/auth.go

Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. CLI: * Do not load host from bundle for CLI commands when profile flag is used ([#2335](#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([#2373](#2373)). Bundles: * Provide instructions for testing in the default-python template ([#2355](#2355)). * Remove `run_as` from the built-in templates ([#2044](#2044)). * Change warning about incomplete permissions section into a recommendation ([#2043](#2043)). * Refine `mode: production` diagnostic output ([#2236](#2236)). * Support serverless mode in default-python template (explicit prompt) ([#2377](#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([#2372](#2372)). * Fixed spark version check for clusters defined in the same bundle ([#2374](#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)

Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. For more details see the related GitHub issue #1358 CLI: * Do not load host from bundle for CLI commands when profile flag is used ([#2335](#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([#2373](#2373)). Bundles: * Provide instructions for testing in the default-python template ([#2355](#2355)). * Remove `run_as` from the built-in templates ([#2044](#2044)). * Change warning about incomplete permissions section into a recommendation ([#2043](#2043)). * Refine `mode: production` diagnostic output ([#2236](#2236)). * Support serverless mode in default-python template (explicit prompt) ([#2377](#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([#2372](#2372)). * Fixed spark version check for clusters defined in the same bundle ([#2374](#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)

…ed (databricks#2335) ## Changes Now when `profile` flag is used we won't pick up host from bundle anymore and use the one provided by -p flag Previous behaviour in the context of bundle ``` databricks current-user me -p profile_name Error: cannot resolve bundle auth configuration: config host mismatch: profile uses host https://non-existing-subdomain.databricks.com, but CLI configured to use https://foo.com ``` New behaviour (make an api call) ``` databricks current-user me -p profile_name { email: "[email protected]" ... } ``` We still load bundle configuration when `-t` flag provide because we want to load host information from the target. Fixes databricks#1358 ## Tests Added acceptance test

Notable changes: Starting this version CLI does not load bundle auth information when CLI command is executed inside the bundle directory with explicitly provided via `-p` flag profile. For more details see the related GitHub issue databricks#1358 CLI: * Do not load host from bundle for CLI commands when profile flag is used ([databricks#2335](databricks#2335)). * Fixed accessing required path parameters in CLI generation when --json flag ([databricks#2373](databricks#2373)). Bundles: * Provide instructions for testing in the default-python template ([databricks#2355](databricks#2355)). * Remove `run_as` from the built-in templates ([databricks#2044](databricks#2044)). * Change warning about incomplete permissions section into a recommendation ([databricks#2043](databricks#2043)). * Refine `mode: production` diagnostic output ([databricks#2236](databricks#2236)). * Support serverless mode in default-python template (explicit prompt) ([databricks#2377](databricks#2377)). * Set default data_security_mode to "SINGLE_USER" in bundle templates ([databricks#2372](databricks#2372)). * Fixed spark version check for clusters defined in the same bundle ([databricks#2374](databricks#2374)). API Changes: * Added `databricks genie get-message-query-result-by-attachment` command. OpenAPI commit 99f644e72261ef5ecf8d74db20f4b7a1e09723cc (2025-02-11)

andrewnester requested review from pietern, shreyas-goenka and denik as code owners February 11, 2025 13:03

andrewnester temporarily deployed to test-trigger-is February 11, 2025 13:03 — with GitHub Actions Inactive

andrewnester temporarily deployed to test-trigger-is February 11, 2025 13:18 — with GitHub Actions Inactive

shreyas-goenka reviewed Feb 14, 2025

View reviewed changes

cmd/root/auth.go Show resolved Hide resolved

andrewnester requested a review from shreyas-goenka February 14, 2025 10:43

shreyas-goenka approved these changes Feb 14, 2025

View reviewed changes

acceptance/auth/bundle_and_profile/output.txt Outdated Show resolved Hide resolved

andrewnester added 4 commits February 25, 2025 12:32

Do not load host from bundle for CLI commands when profile flag is used

57c721e

update test

a79504e

make it local only

051d36e

added comment

f08770a

andrewnester force-pushed the feature/use-profile-flag-in-bundle branch from 38e7d9c to f08770a Compare February 25, 2025 11:36

andrewnester temporarily deployed to test-trigger-is February 25, 2025 11:37 — with GitHub Actions Inactive

denik reviewed Feb 25, 2025

View reviewed changes

andrewnester requested a review from denik February 25, 2025 12:39

denik reviewed Feb 25, 2025

View reviewed changes

added bundle command tests

df4e032

andrewnester temporarily deployed to test-trigger-is February 25, 2025 15:00 — with GitHub Actions Inactive

andrewnester requested a review from denik February 25, 2025 15:01

denik approved these changes Feb 26, 2025

View reviewed changes

andrewnester added this pull request to the merge queue Feb 26, 2025

Merged via the queue into main with commit df001dc Feb 26, 2025
9 checks passed

andrewnester deleted the feature/use-profile-flag-in-bundle branch February 26, 2025 12:36

andrewnester mentioned this pull request Feb 27, 2025

[Release] Release v0.242.0 #2393

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Do not load host from bundle for CLI commands when profile flag is used #2335

Do not load host from bundle for CLI commands when profile flag is used #2335

andrewnester commented Feb 11, 2025 •

edited

Loading

denik Feb 25, 2025

denik Feb 25, 2025

andrewnester Feb 25, 2025

denik Feb 25, 2025

andrewnester Feb 25, 2025

denik Feb 25, 2025

andrewnester Feb 25, 2025

denik Feb 25, 2025

Do not load host from bundle for CLI commands when profile flag is used #2335

Do not load host from bundle for CLI commands when profile flag is used #2335

Conversation

andrewnester commented Feb 11, 2025 • edited Loading

Changes

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andrewnester commented Feb 11, 2025 •

edited

Loading