Skip to content

Commit fe17837

Browse files
committed
Address review: extract profile precedence helper, cover bundle host+profile
Dedupe the --profile precedence block shared by MustWorkspaceClient and MustAccountClient into applyProfileAuthPrecedence so the rationale lives in one place. Add a bundle test for workspace.host + workspace.profile with conflicting auth env vars.
1 parent 3a1bb89 commit fe17837

2 files changed

Lines changed: 49 additions & 16 deletions

File tree

bundle/config/workspace_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,34 @@ func TestWorkspaceClientProfileFillsAuthFromEnv(t *testing.T) {
201201
assert.Equal(t, "env-token", client.Config.Token)
202202
}
203203

204+
func TestWorkspaceClientHostAndProfileOverridesAuthEnv(t *testing.T) {
205+
// A bundle that pins both workspace.host and workspace.profile: the profile
206+
// still wins for auth over the env vars, the bundle host is honored, and
207+
// ValidateConfigAndProfileHost passes because they agree (#5096).
208+
setupWorkspaceTest(t)
209+
210+
err := databrickscfg.SaveToProfile(t.Context(), &config.Config{
211+
Profile: "tst",
212+
Host: "https://tst.cloud.databricks.test",
213+
Token: "tst-token",
214+
})
215+
require.NoError(t, err)
216+
217+
// direnv-style auth env vars pointing at a different (dev) workspace.
218+
t.Setenv("DATABRICKS_HOST", "https://dev.cloud.databricks.test")
219+
t.Setenv("DATABRICKS_TOKEN", "dev-token")
220+
221+
w := Workspace{
222+
Host: "https://tst.cloud.databricks.test",
223+
Profile: "tst",
224+
}
225+
client, err := w.Client(t.Context())
226+
require.NoError(t, err)
227+
assert.Equal(t, "tst", client.Config.Profile)
228+
assert.Equal(t, "https://tst.cloud.databricks.test", client.Config.Host)
229+
assert.Equal(t, "tst-token", client.Config.Token)
230+
}
231+
204232
func TestWorkspaceConfigHTTPTimeout(t *testing.T) {
205233
for _, tc := range []struct {
206234
envVal string

cmd/root/auth.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,25 @@ func profileFlagValue(cmd *cobra.Command) (string, bool) {
110110
return value, value != ""
111111
}
112112

113+
// applyProfileAuthPrecedence makes an explicitly selected --profile take
114+
// precedence over auth environment variables instead of being shadowed by them.
115+
//
116+
// With a profile flag we install databrickscfg.ProfileAuthLoaders (the single
117+
// source of truth for this precedence rule; see its doc and #5096) and skip
118+
// NormalizeDatabricksConfigFromEnv: the host comes from the profile, not
119+
// DATABRICKS_HOST, so promoting that env host's ?o=/?a= query params would be
120+
// wrong. Trade-off: a host-less profile combined with a SPOG-style
121+
// DATABRICKS_HOST (?o=123) no longer extracts the workspace_id from the query.
122+
//
123+
// Without a profile flag we keep the SDK's env-first behavior.
124+
func applyProfileAuthPrecedence(ctx context.Context, cfg *config.Config, hasProfileFlag bool) {
125+
if hasProfileFlag {
126+
cfg.Loaders = databrickscfg.ProfileAuthLoaders
127+
return
128+
}
129+
auth.NormalizeDatabricksConfigFromEnv(ctx, cfg)
130+
}
131+
113132
// Helper function to create an account client or prompt once if the given configuration is not valid.
114133
func accountClientOrPrompt(ctx context.Context, cfg *config.Config, allowPrompt bool) (*databricks.AccountClient, error) {
115134
a, err := databricks.NewAccountClient((*databricks.Config)(cfg))
@@ -201,14 +220,8 @@ func MustAccountClient(cmd *cobra.Command, args []string) error {
201220
pr, hasProfileFlag := profileFlagValue(cmd)
202221
if hasProfileFlag {
203222
cfg.Profile = pr
204-
// An explicit --profile takes precedence over auth env vars; see
205-
// databrickscfg.ProfileAuthLoaders (#5096). NormalizeDatabricksConfigFromEnv
206-
// is skipped too: the host comes from the profile, not DATABRICKS_HOST, so
207-
// promoting its ?o=/?a= query params would be wrong.
208-
cfg.Loaders = databrickscfg.ProfileAuthLoaders
209-
} else {
210-
auth.NormalizeDatabricksConfigFromEnv(ctx, cfg)
211223
}
224+
applyProfileAuthPrecedence(ctx, cfg, hasProfileFlag)
212225

213226
ctx = cmdctx.SetConfigUsed(ctx, cfg)
214227
cmd.SetContext(ctx)
@@ -330,16 +343,8 @@ func MustWorkspaceClient(cmd *cobra.Command, args []string) error {
330343
profile, hasProfileFlag := profileFlagValue(cmd)
331344
if hasProfileFlag {
332345
cfg.Profile = profile
333-
// An explicit --profile takes precedence over auth env vars; see
334-
// databrickscfg.ProfileAuthLoaders (#5096). NormalizeDatabricksConfigFromEnv
335-
// is skipped too: the host comes from the profile, not DATABRICKS_HOST, so
336-
// promoting its ?o=/?a= query params would be wrong. Trade-off: a host-less
337-
// profile + SPOG-style DATABRICKS_HOST (?o=123) no longer extracts the
338-
// workspace_id from the query.
339-
cfg.Loaders = databrickscfg.ProfileAuthLoaders
340-
} else {
341-
auth.NormalizeDatabricksConfigFromEnv(ctx, cfg)
342346
}
347+
applyProfileAuthPrecedence(ctx, cfg, hasProfileFlag)
343348
resolveDefaultProfile(ctx, cfg)
344349

345350
_, isTargetFlagSet := targetFlagValue(cmd)

0 commit comments

Comments
 (0)