config: pin cfg.Profile on the legacy [DEFAULT] fallback#1690
Open
simonfaltum wants to merge 1 commit into
Open
config: pin cfg.Profile on the legacy [DEFAULT] fallback#1690simonfaltum wants to merge 1 commit into
simonfaltum wants to merge 1 commit into
Conversation
The config-file loader's resolveProfile distinguishes "the caller named
this profile" from "we silently fell back to [DEFAULT]" via the
isFallback return. On a fallback, the loader loaded the section's
values into cfg but deliberately left cfg.Profile empty (the
`if !isFallback { cfg.Profile = profile }` gate). The intent was
preserving the caller's "I didn't pick a profile" signal.
That signal turns out to be a footgun for consumers that derive a
per-profile identifier from cfg.Profile. The Databricks CLI's U2M
OAuth cache, for example, uses cfg.Profile as the cache key. When the
CLI calls `auth login --profile DEFAULT --host ...` it writes a token
under cache key "DEFAULT". When the user later runs `auth describe`
with no flag, the loader silently falls back to [DEFAULT] but
cfg.Profile stays empty, so the OAuthArgument's cache key drifts to
the host URL and the lookup misses. The bug is masked when the CLI
also dual-writes to a host-keyed entry (its legacy plaintext mode);
under secure storage it surfaces as "cache: token not found" or, when
a stale host-key entry exists, InvalidRefreshTokenError.
Drop the !isFallback gate so cfg.Profile always reflects the section
the loader actually read. isFallback is still load-bearing earlier in
Configure: a missing section is tolerated only on fallback.
This is a self-consistency fix for the SDK and a defense-in-depth
followup to the CLI's own resolveDefaultProfile change. After the SDK
bump, the CLI's pre-population becomes redundant but harmless.
Co-authored-by: Isaac
Signed-off-by: simon <simon.faltum@databricks.com>
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
simonfaltum
temporarily deployed
to
test-trigger-is
GitHub Actions
Inactive
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
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
1 participant
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
The config-file loader's
resolveProfiledistinguishes "the caller named this profile" from "we silently fell back to[DEFAULT]" via theisFallbackreturn. On a fallback,Configureloaded the section's values intocfgbut deliberately leftcfg.Profileempty (theif !isFallback { cfg.Profile = profile }gate atconfig/config_file.go:103-105). The intent was preserving the caller's "I didn't pick a profile" signal.That signal turns out to be a footgun for consumers that derive a per-profile identifier from
cfg.Profile. The Databricks CLI's U2M OAuth cache, for example, usescfg.Profileas the cache key. When the CLI runsdatabricks auth login --profile DEFAULT --host ...it writes a token under cache key"DEFAULT". When the user later runsdatabricks auth describewith no flag, the loader silently falls back to[DEFAULT]butcfg.Profilestays empty, so the OAuthArgument's cache key drifts to the host URL and the lookup misses. The bug is masked when the CLI also dual-writes to a host-keyed entry (its legacy plaintext mode); under secure storage it surfaces ascache: token not foundor, when a stale host-key entry exists,InvalidRefreshTokenError.This PR drops the
!isFallbackgate socfg.Profilealways reflects the section the loader actually read.isFallbackis still load-bearing earlier inConfigure(lines 91-95): a missing[DEFAULT]section is tolerated only on fallback, and that behavior is preserved.This is a self-consistency fix for the SDK and a defense-in-depth followup to the CLI's own
resolveDefaultProfilechange (databricks/cli#5280). After the SDK bump, the CLI's pre-population becomes redundant but harmless.Linked: databricks/cli#5280 (the CLI-side fix that lands ahead of this).
Tests
TestConfigFile_LegacyFallbackSetsResolvedProfileNameassertscfg.Profile == "DEFAULT"on the legacy fallback path.cfg.Profilevalue on the legacy fallback was the bug.make fmt,make lint,make testclean locally.This pull request and its description were written by Isaac.
Signed-off-by: Isaac