Skip to content

Commit ca6bf4e

Browse files
radakamradakam
committed
Guard default_profile against env auth in databricks api
The no---profile branch in `databricks api` pinned [__settings__].default_profile unconditionally, so a conflicting default profile could be merged with env auth (DATABRICKS_HOST/DATABRICKS_TOKEN), failing with "more than one authorization method configured" (the #5616 class already fixed for MustWorkspaceClient). Share the guard: export resolveDefaultProfile as ResolveDefaultProfile and call it from cmd/api, so the default profile is skipped when DATABRICKS_HOST is set. Adds an acceptance case with env PAT + conflicting default_profile.
1 parent e16df4f commit ca6bf4e

7 files changed

Lines changed: 63 additions & 23 deletions

File tree

‎acceptance/cmd/api/default-profile-vs-env/out.test.toml‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎acceptance/cmd/api/default-profile-vs-env/output.txt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
2+
=== api without --profile uses env auth, ignoring default_profile (#5616)
3+
4+
>>> [CLI] api get /api/2.0/clusters/list
5+
{}
6+
7+
>>> print_requests.py --get //api/2.0/clusters/list
8+
{
9+
"headers": {
10+
"Authorization": [
11+
"Bearer [DATABRICKS_TOKEN]"
12+
],
13+
"User-Agent": [
14+
"cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat"
15+
],
16+
"X-Databricks-Workspace-Id": [
17+
"[NUMID]"
18+
]
19+
},
20+
"method": "GET",
21+
"path": "/api/2.0/clusters/list"
22+
}

‎acceptance/cmd/api/default-profile-vs-env/script‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
sethome "./home"
2+
3+
# A default profile with conflicting (basic) auth on a different host. The env
4+
# below still points a PAT at the test server. Without the #5616 guard, the
5+
# default profile would be pinned and merged with the env PAT, failing with
6+
# "more than one authorization method configured".
7+
cat > "./home/.databrickscfg" <<EOF
8+
[__settings__]
9+
default_profile = other
10+
11+
[other]
12+
host = https://other.cloud.databricks.test
13+
username = user
14+
password = pass
15+
EOF
16+
17+
title "api without --profile uses env auth, ignoring default_profile (#5616)\n"
18+
MSYS_NO_PATHCONV=1 trace $CLI api get /api/2.0/clusters/list
19+
trace print_requests.py --get //api/2.0/clusters/list

‎acceptance/cmd/api/default-profile-vs-env/test.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Ignore = [
2+
"home",
3+
]

‎cmd/api/api.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,9 @@ func makeCommand(method string) *cobra.Command {
8080

8181
cfg := &config.Config{}
8282

83-
// Resolve the profile mirroring MustWorkspaceClient precedence:
84-
// 1. --profile flag, 2. DATABRICKS_CONFIG_PROFILE env var (the SDK
85-
// also reads it, but setting cfg.Profile here keeps any error
86-
// messages we render referring to the same name), 3.
87-
// [__settings__].default_profile in the config file.
83+
// Profile precedence, mirroring MustWorkspaceClient: --profile flag,
84+
// then DATABRICKS_CONFIG_PROFILE (set here so our error messages name
85+
// the same profile the SDK loads), then the default profile below.
8886
profileFlag := cmd.Flag("profile")
8987
hasProfileFlag := profileFlag != nil && profileFlag.Value.String() != ""
9088
if hasProfileFlag {
@@ -93,17 +91,16 @@ func makeCommand(method string) *cobra.Command {
9391
if cfg.Profile == "" {
9492
cfg.Profile = env.Get(cmd.Context(), "DATABRICKS_CONFIG_PROFILE")
9593
}
96-
if cfg.Profile == "" {
97-
cfg.Profile = databrickscfg.ResolveDefaultProfile(cmd.Context())
98-
}
9994

10095
if hasProfileFlag {
101-
// An explicit --profile wins over auth env vars (#5096); the host
102-
// comes from the profile, so skip env host normalization.
96+
// An explicit --profile wins over auth env vars; its host makes env
97+
// host normalization moot (#5096).
10398
cfg.Loaders = databrickscfg.ProfileAuthLoaders
10499
} else {
105100
auth.NormalizeDatabricksConfigFromEnv(cmd.Context(), cfg)
106101
}
102+
// Skips the default profile when DATABRICKS_HOST is set (#5616).
103+
root.ResolveDefaultProfile(cmd.Context(), cfg)
107104

108105
api, err := client.New(cfg)
109106
if err != nil {

‎cmd/root/auth.go‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func MustAccountClient(cmd *cobra.Command, args []string) error {
220220

221221
profiler := profile.GetProfiler(ctx)
222222

223-
resolveDefaultProfile(ctx, cfg)
223+
ResolveDefaultProfile(ctx, cfg)
224224

225225
if cfg.Profile == "" {
226226
// account-level CLI was not really done before, so here are the assumptions:
@@ -337,7 +337,7 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
337337
cfg.Profile = profile
338338
}
339339
applyProfileAuthPrecedence(ctx, cfg, hasProfileFlag)
340-
resolveDefaultProfile(ctx, cfg)
340+
ResolveDefaultProfile(ctx, cfg)
341341

342342
_, isTargetFlagSet := targetFlagValue(cmd)
343343
// If the profile flag is set but the target flag is not, we should skip loading the bundle configuration.
@@ -379,17 +379,13 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
379379
return nil
380380
}
381381

382-
// resolveDefaultProfile applies the [__settings__].default_profile setting
383-
// when no profile is specified via --profile flag or DATABRICKS_CONFIG_PROFILE.
382+
// ResolveDefaultProfile applies [__settings__].default_profile when no profile
383+
// is set via --profile or DATABRICKS_CONFIG_PROFILE.
384384
//
385-
// It does nothing when a host is configured directly via the environment
386-
// (DATABRICKS_HOST). Authentication is then fully determined by the environment,
387-
// and the SDK already ignores .databrickscfg in that case, but only while
388-
// cfg.Profile is empty (see configFileLoader.Configure in databricks-sdk-go).
389-
// Pinning a default profile would defeat that skip and merge the profile's
390-
// credentials with the environment config, failing with "more than one
391-
// authorization method configured".
392-
func resolveDefaultProfile(ctx context.Context, cfg *config.Config) {
385+
// It skips when DATABRICKS_HOST is set: the SDK ignores .databrickscfg while
386+
// cfg.Profile is empty, so pinning a default profile would merge it with the env
387+
// config and fail with "more than one authorization method configured" (#5616).
388+
func ResolveDefaultProfile(ctx context.Context, cfg *config.Config) {
393389
if cfg.Profile != "" || envlib.Get(ctx, "DATABRICKS_CONFIG_PROFILE") != "" {
394390
return
395391
}

‎cmd/root/auth_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -929,7 +929,7 @@ password = pass
929929
w := cmdctx.WorkspaceClient(cmd.Context())
930930
require.NotNil(t, w)
931931
// Host and token in the environment fully determine auth, so the default
932-
// profile must not be pinned (see resolveDefaultProfile).
932+
// profile must not be pinned (see ResolveDefaultProfile).
933933
assert.Empty(t, w.Config.Profile)
934934
assert.Equal(t, "https://env.test", w.Config.Host)
935935
assert.Equal(t, "env-token", w.Config.Token)

0 commit comments

Comments
 (0)