Make --profile take precedence over auth environment variables#5702
Draft
radakam wants to merge 4 commits into
Draft
Make --profile take precedence over auth environment variables#5702radakam wants to merge 4 commits into
radakam wants to merge 4 commits into
Conversation
When --profile is set explicitly, host and auth credentials from the profile now win over DATABRICKS_HOST/DATABRICKS_TOKEN and other auth env vars. Previously the SDK's env-first loader order silently shadowed the selected profile (#5096).
Collaborator
Integration test reportCommit: a673c11
23 interesting tests: 13 SKIP, 7 KNOWN, 2 FAIL, 1 flaky
Top 5 slowest tests (at least 2 minutes):
|
Extend the --profile precedence fix (#5096): - ResolveNonAuthFromEnv now also skips auth_type and discovery_url, which are tagged auth:"-" in the SDK and so are invisible to HasAuthAttribute, letting DATABRICKS_AUTH_TYPE/DATABRICKS_DISCOVERY_URL shadow the profile. It also records the env source so `auth describe` and debug output match the SDK loader. - Workspace.Client uses ResolveNonAuthFromEnv when a profile is set (from --profile or workspace.profile) so env auth vars no longer shadow the profile for bundle commands. - Use the reserved .test TLD for new test fixture hosts so the SDK's well-known host metadata resolver fast-fails instead of stalling on a live network lookup.
A host-only profile combined with DATABRICKS_TOKEN previously failed because the profile loader chain stopped at the config file. Append config.ConfigAttributes after the profile so the environment can fill auth fields the profile does not provide, while the profile still wins for any field it sets (#5096).
Consolidate the profile-precedence loader chain into a single shared databrickscfg.ProfileAuthLoaders, used by both cmd/root and bundle config so the two copies can't drift. Add audience and cloud to the env skip list (both steer auth but are tagged auth:"-" so HasAuthAttribute misses them) and document the inclusion criterion. Clarify that the gap-fill of auth fields from the environment and the env-first precedence of DATABRICKS_CONFIG_PROFILE are intentional, and that NormalizeDatabricksConfigFromEnv is deliberately skipped under --profile. Add an acceptance test covering --profile winning over DATABRICKS_HOST/ DATABRICKS_TOKEN and a host-only profile filling its token from the environment (#5096).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
When
--profileis passed explicitly (orworkspace.profileis set in a bundle), the selected profile's host and authentication credentials now take precedence over authentication environment variables (DATABRICKS_HOST,DATABRICKS_TOKEN, and the other auth/credential vars).databrickscfg.ResolveNonAuthFromEnv, an SDK config loader that reads all config from the environment except the host and auth credentials.[ResolveNonAuthFromEnv, ConfigFile, ConfigAttributes]and skipNormalizeDatabricksConfigFromEnv(which would otherwise promote the env host). This is wired throughMustWorkspaceClient,MustAccountClient, andbundle/config.Workspace.Clientvia a sharedprofileAuthLoaderschain.Why
The SDK's default loader order is environment first, then config file, and a loader never overwrites an already-set field. So with
DATABRICKS_HOST/DATABRICKS_TOKENset (e.g. viadirenvin a project dir), they were loaded first and the profile selected with--profilecould not overwrite them — the command silently ran against the env's workspace instead of the profile's (#5096).Swapping the loader order only works if the host and auth fields are left empty for the config-file loader to fill, hence the dedicated non-auth env loader. The fix is intentionally scoped to host + authentication: non-auth attributes (e.g.
cluster_id,workspace_id) keep their existing env precedence.A trailing
config.ConfigAttributesloader runs after the profile so the environment can still fill auth fields the profile leaves empty. This supports the common CI pattern where the host lives in.databrickscfgbutDATABRICKS_TOKENis injected by the runner: the profile wins for any field it sets, and the env fills only what the profile omits.Tests
TestMustWorkspaceClientProfileFlagOverridesAuthEnv: the exact CLI profile not respected #5096 scenario (env points at dev,--profileselects another workspace); asserts the profile's host and token win.TestMustWorkspaceClientProfileFlagFillsAuthFromEnv: a host-only profile combined withDATABRICKS_TOKEN; asserts the profile's host wins and the env fills the missing token.TestMustAccountClientProfileFlagOverridesAuthEnv: same override behavior for the account client path.TestWorkspaceClientProfileOverridesAuthEnv/TestWorkspaceClientProfileFillsAuthFromEnv: cover theworkspace.profile(bundle) code path for both override and env-fill behavior.TestResolveNonAuthFromEnvSkipsHostAndAuth: unit test confirming the loader skips host/token but still applies non-auth env vars.--profile>DATABRICKS_CONFIG_PROFILE, env-host-skips-default-profile) still pass.go build ./...,go vet, and thecmd/root+libs/databrickscfg+bundle/configpackages pass.