Skip to content

Commit 8ef7b6a

Browse files
committed
Skip auth-steering env vars gap-filling a selected profile
The final loader in ProfileAuthLoaders was the SDK's config.ConfigAttributes, which re-reads every still-empty attribute from the environment after the profile is loaded. That let host/routing/auth-steering env vars (auth_type, discovery_url, audience, cloud, workspace_id, account_id) shadow an explicitly selected profile, e.g. DATABRICKS_AUTH_TYPE=basic forcing basic auth over a profile's PAT (#5096). Replace it with resolveAuthGapFromEnv, a filtered env loader that skips nonAuthEnvSkipAttrs but still gap-fills real auth attrs, so a complete conflicting env auth method surfaces as an error rather than being silently dropped. Add an end-to-end ProfileAuthLoaders test covering the steering-env repro.
1 parent a8e961c commit 8ef7b6a

2 files changed

Lines changed: 60 additions & 9 deletions

File tree

libs/databrickscfg/loader.go

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,31 @@ var ResolveProfileFromHost = profileFromHostLoader{}
1616

1717
// ResolveNonAuthFromEnv reads non-auth, non-host config from environment
1818
// variables. See ProfileAuthLoaders for how it fits into the chain.
19-
var ResolveNonAuthFromEnv = nonAuthEnvLoader{}
19+
var ResolveNonAuthFromEnv = envLoader{name: "environment (excluding auth)", skipAuth: true}
20+
21+
// resolveAuthGapFromEnv gap-fills auth attributes the profile left empty from
22+
// environment variables. See ProfileAuthLoaders for how it fits into the chain.
23+
var resolveAuthGapFromEnv = envLoader{name: "environment (auth gap-fill)", skipAuth: false}
2024

2125
// ProfileAuthLoaders is the loader chain for an explicitly selected profile
2226
// (--profile or a bundle's workspace.profile). Unlike the SDK's default
2327
// env-first chain, the profile wins over auth env vars (#5096):
2428
//
2529
// 1. ResolveNonAuthFromEnv: non-auth env attrs (e.g. cluster_id), env-wins.
2630
// 2. ConfigFile: the selected profile (host, routing, auth).
27-
// 3. ConfigAttributes: gap-fills only fields the profile left empty.
31+
// 3. resolveAuthGapFromEnv: gap-fills auth attrs the profile left empty from
32+
// env, so a complete conflicting env auth method still surfaces as an error
33+
// rather than being silently dropped. It skips nonAuthEnvSkipAttrs (like the
34+
// env-first pass) so host/routing/auth-steering attrs come from the profile
35+
// only; using the SDK's ConfigAttributes here would let those env vars shadow
36+
// the profile (e.g. DATABRICKS_AUTH_TYPE=basic over a PAT profile).
2837
//
2938
// A profile from DATABRICKS_CONFIG_PROFILE keeps env-first precedence; only an
3039
// out-of-band profile name triggers this chain.
3140
var ProfileAuthLoaders = []config.Loader{
3241
ResolveNonAuthFromEnv,
3342
config.ConfigFile,
34-
config.ConfigAttributes,
43+
resolveAuthGapFromEnv,
3544
}
3645

3746
var errNoMatchingProfiles = errors.New("no matching config profiles found")
@@ -96,16 +105,28 @@ var nonAuthEnvSkipAttrs = map[string]bool{
96105
"cloud": true,
97106
}
98107

99-
type nonAuthEnvLoader struct{}
108+
// envLoader reads config attributes from environment variables. It always skips
109+
// nonAuthEnvSkipAttrs so host/routing/auth-steering attrs come from the selected
110+
// profile only (#5096). When skipAuth is true it additionally skips the SDK's
111+
// auth attributes, for the env-first pass that runs before the profile.
112+
type envLoader struct {
113+
name string
114+
skipAuth bool
115+
}
100116

101-
func (nonAuthEnvLoader) Name() string {
102-
return "environment (excluding auth)"
117+
func (l envLoader) Name() string {
118+
return l.name
103119
}
104120

105-
func (nonAuthEnvLoader) Configure(cfg *config.Config) error {
121+
func (l envLoader) Configure(cfg *config.Config) error {
106122
for _, attr := range config.ConfigAttributes {
107-
// Host and auth come from the profile (config file), not env.
108-
if nonAuthEnvSkipAttrs[attr.Name] || attr.HasAuthAttribute() {
123+
// Host/routing/auth-steering come from the profile (config file), not env.
124+
if nonAuthEnvSkipAttrs[attr.Name] {
125+
continue
126+
}
127+
// The env-first pass leaves auth for the profile; the gap-fill pass runs
128+
// after the profile and fills only auth attrs the profile didn't set.
129+
if l.skipAuth && attr.HasAuthAttribute() {
109130
continue
110131
}
111132
// Don't overwrite an already-set value (SDK loader semantics).

libs/databrickscfg/loader_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,36 @@ func TestResolveNonAuthFromEnvSkipsHostAndAuth(t *testing.T) {
6565
assert.Equal(t, "env-cluster", cfg.ClusterID)
6666
}
6767

68+
func TestProfileAuthLoadersProfileWinsOverSteeringEnv(t *testing.T) {
69+
// A PAT profile plus auth-steering/routing env vars. The profile owns
70+
// host/routing/auth-steering, so these env vars must not shadow it (#5096).
71+
// In particular DATABRICKS_AUTH_TYPE=basic must not force basic auth over the
72+
// profile's PAT.
73+
t.Setenv("DATABRICKS_AUTH_TYPE", "basic")
74+
t.Setenv("DATABRICKS_DISCOVERY_URL", "https://discovery.env.test")
75+
t.Setenv("DATABRICKS_CLOUD", "azure")
76+
t.Setenv("DATABRICKS_WORKSPACE_ID", "env-workspace")
77+
t.Setenv("DATABRICKS_ACCOUNT_ID", "env-account")
78+
79+
cfg := config.Config{
80+
Loaders: ProfileAuthLoaders,
81+
ConfigFile: "profile/testdata/databrickscfg",
82+
Profile: "DEFAULT",
83+
}
84+
85+
err := cfg.EnsureResolved()
86+
require.NoError(t, err)
87+
88+
assert.Equal(t, "https://default.test", cfg.Host)
89+
assert.Equal(t, "default", cfg.Token)
90+
// Steering/routing env vars did not shadow the profile.
91+
assert.Empty(t, cfg.AuthType)
92+
assert.Empty(t, cfg.DiscoveryURL)
93+
assert.Empty(t, cfg.Cloud)
94+
assert.Empty(t, cfg.WorkspaceID)
95+
assert.Empty(t, cfg.AccountID)
96+
}
97+
6898
func TestProfileAuthLoadersConflictingEnvAuthMethodErrors(t *testing.T) {
6999
// Profile has a PAT; env gap-fills a different complete method (OAuth M2M).
70100
// The config then carries two auth methods, which the SDK rejects rather

0 commit comments

Comments
 (0)