Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andrewnester merged 5 commits into from
Feb 26, 2025
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57c721e
Do not load host from bundle for CLI commands when profile flag is used
andrewnester Feb 11, 2025
a79504e
update test
andrewnester Feb 11, 2025
051d36e
make it local only
andrewnester Feb 12, 2025
f08770a
added comment
andrewnester Feb 25, 2025
df4e032
added bundle command tests
andrewnester Feb 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 61 additions & 2 deletions acceptance/auth/bundle_and_profile/output.txt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -11,9 +11,9 @@
>>> errcode [CLI] current-user me -t dev -p DEFAULT
"[USERNAME]"

=== Inside the bundle, profile flag not matching bundle host. Badness: should use profile from flag instead and not fail
=== Inside the bundle, profile flag not matching bundle host. Should use profile from the flag and not the bundle.
>>> errcode [CLI] 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 [DATABRICKS_TARGET]
Error: Get "https://non-existing-subdomain.databricks.com/api/2.0/preview/scim/v2/Me": (redacted)

Exit code: 1

Expand All@@ -23,6 +23,65 @@ Error: cannot resolve bundle auth configuration: config host mismatch: profile u

Exit code: 1

=== Bundle commands load bundle configuration when no flags, validation OK
>>> errcode [CLI] bundle validate
Name: test-auth
Target: dev
Workspace:
Host: [DATABRICKS_TARGET]
User: [USERNAME]
Path: /Workspace/Users/[USERNAME]/.bundle/test-auth/dev

Validation OK!

=== Bundle commands load bundle configuration with -t flag, validation OK
>>> errcode [CLI] bundle validate -t dev
Name: test-auth
Target: dev
Workspace:
Host: [DATABRICKS_TARGET]
User: [USERNAME]
Path: /Workspace/Users/[USERNAME]/.bundle/test-auth/dev

Validation OK!

=== Bundle commands load bundle configuration with -p flag, validation not OK (profile host don't match bundle host)
>>> errcode [CLI] bundle validate -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 [DATABRICKS_TARGET]

Name: test-auth
Target: dev
Workspace:
Host: [DATABRICKS_TARGET]

Found 1 error

Exit code: 1

=== Bundle commands load bundle configuration with -t and -p flag, validation OK (profile host match bundle host)
>>> errcode [CLI] bundle validate -t dev -p DEFAULT
Name: test-auth
Target: dev
Workspace:
Host: [DATABRICKS_TARGET]
User: [USERNAME]
Path: /Workspace/Users/[USERNAME]/.bundle/test-auth/dev

Validation OK!

=== Bundle commands load bundle configuration with -t and -p flag, validation not OK (profile host don't match bundle host)
>>> errcode [CLI] bundle validate -t prod -p DEFAULT
Error: cannot resolve bundle auth configuration: config host mismatch: profile uses host [DATABRICKS_TARGET], but CLI configured to use https://bar.com

Name: test-auth
Target: prod
Workspace:
Host: https://bar.com

Found 1 error

Exit code: 1

=== Outside the bundle, no flags
>>> errcode [CLI] current-user me
"[USERNAME]"
Expand Down
17 changes: 16 additions & 1 deletion acceptance/auth/bundle_and_profile/script
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -15,12 +15,27 @@ 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."

@denikdenikFeb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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'.

trace errcode $CLI current-user me -p profile_name | jq .userName

title "Inside the bundle, target and not matching profile"
trace errcode $CLI current-user me -t dev -p profile_name

title "Bundle commands load bundle configuration when no flags, validation OK"
trace errcode $CLI bundle validate

title "Bundle commands load bundle configuration with -t flag, validation OK"
trace errcode $CLI bundle validate -t dev

title "Bundle commands load bundle configuration with -p flag, validation not OK (profile host don't match bundle host)"
trace errcode $CLI bundle validate -p profile_name

title "Bundle commands load bundle configuration with -t and -p flag, validation OK (profile host match bundle host)"
trace errcode $CLI bundle validate -t dev -p DEFAULT

title "Bundle commands load bundle configuration with -t and -p flag, validation not OK (profile host don't match bundle host)"
trace errcode $CLI bundle validate -t prod -p DEFAULT

cd ..
export DATABRICKS_HOST=$host
title "Outside the bundle, no flags"
Expand Down
6 changes: 5 additions & 1 deletion acceptance/auth/bundle_and_profile/test.toml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
Badness = "When -p flag is used inside the bundle folder for any CLI commands, CLI use bundle host anyway instead of profile one"
LocalOnly=true

# Some of the clouds have DATABRICKS_HOST variable setup without https:// prefix
# In the result, output is replaced with DATABRICKS_URL variable instead of DATABRICKS_HOST
Expand All@@ -10,3 +10,7 @@ New='DATABRICKS_TARGET'
[[Repls]]
Old='DATABRICKS_URL'
New='DATABRICKS_TARGET'

[[Repls]]
Old='Get "https://non-existing-subdomain.databricks.com/api/2.0/preview/scim/v2/Me": .*'
New='Get "https://non-existing-subdomain.databricks.com/api/2.0/preview/scim/v2/Me": (redacted)'
6 changes: 6 additions & 0 deletions cmd/root/auth.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -195,6 +195,12 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
cfg.Profile = profile
}

_, isTargetFlagSet := targetFlagValue(cmd)
// If the profile flag is set but the target flag is not, we should skip loading the bundle configuration.
if !isTargetFlagSet && hasProfileFlag {

@denikdenikFeb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@andrewnesterandrewnesterFeb 25, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@denikdenikFeb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if hasProfileFlag {?

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

@andrewnesterandrewnesterFeb 25, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

cmd.SetContext(SkipLoadBundle(cmd.Context()))
Comment thread
shreyas-goenka marked this conversation as resolved.
Comment thread
andrewnester marked this conversation as resolved.
}

ctx := cmd.Context()
ctx = context.WithValue(ctx, &configUsed, cfg)
cmd.SetContext(ctx)
Expand Down
23 changes: 16 additions & 7 deletions cmd/root/bundle.go
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -14,26 +14,35 @@ import (

// getTarget returns the name of the target to operate in.
func getTarget(cmd *cobra.Command) (value string) {
target, isFlagSet := targetFlagValue(cmd)
if isFlagSet {
return target
}

// If it's not set, use the environment variable.
target, _ = env.Target(cmd.Context())
return target
}

func targetFlagValue(cmd *cobra.Command) (string, bool) {

@denikdenikFeb 25, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@andrewnesterandrewnesterFeb 25, 2025

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

// The command line flag takes precedence.
flag := cmd.Flag("target")
if flag != nil {
value = flag.Value.String()
value := flag.Value.String()
if value != "" {
return
return value, true
}
}

oldFlag := cmd.Flag("environment")
if oldFlag != nil {
value = oldFlag.Value.String()
value := oldFlag.Value.String()
if value != "" {
return
return value, true
}
}

// If it's not set, use the environment variable.
target, _ := env.Target(cmd.Context())
return target
return "", false
}

func getProfile(cmd *cobra.Command) (value string) {
Expand Down