libs/auth/storage: add dormant secure-storage foundation#5008
Open
simonfaltum wants to merge 13 commits intomainfrom
Open
libs/auth/storage: add dormant secure-storage foundation#5008simonfaltum wants to merge 13 commits intomainfrom
simonfaltum wants to merge 13 commits intomainfrom
Conversation
Reads [__settings__].auth_storage in the same style as GetConfiguredDefaultProfile. Write path is deliberately not provided yet. Unused until a follow-up PR wires it into the storage-mode resolver.
Introduces the secure / legacy / plaintext mode enum and a precedence resolver that reads override -> DATABRICKS_AUTH_STORAGE env -> [__settings__].auth_storage -> Legacy. Nothing imports this yet; a follow-up PR wires it into the auth commands.
Switch to t.Setenv so DATABRICKS_AUTH_STORAGE and DATABRICKS_CONFIG_FILE are hermetically overridden per subtest. The previous test used the context-based env.Set helper, which falls through to os.LookupEnv when the key is absent from the context - that meant a developer or CI lane with a real DATABRICKS_AUTH_STORAGE export could fail the 'env unset' subtests. Also add a doc note on ResolveStorageMode clarifying that an invalid override is a caller bug rather than user input, so the asymmetric error wrapping (source name on env/config; bare on override) is intentional.
…dency Adds github.com/zalando/go-keyring (MIT) and wires up KeyringCache, which satisfies cache.TokenCache. Service name is 'databricks-cli'; account is the SDK-provided cache key. Lookup and Store(nil) are stubbed; follow-up tasks fill them in. Tests use a fake backend injected via an internal interface.
Maps the backend's ErrNotFound onto cache.ErrNotFound. Unmarshal errors surface as wrapped errors rather than ErrNotFound so callers can tell 'missing' apart from 'corrupted'.
Deleting a missing entry is a no-op so that logout is idempotent across re-runs. Other backend errors bubble up unchanged.
Mirrors GitHub CLI's pattern. Protects against Linux D-Bus hangs in containers and SSH sessions without a running Secret Service daemon. TimeoutError is exported so callers can render a tailored message.
Pairs with the zalando/go-keyring dependency added earlier in this branch. Without this block TestNoticeFileCompleteness fails because every direct dep in go.mod must have a matching entry under its SPDX license section in NOTICE.
Linter auto-fix from make lint. Functionally identical; testifylint prefers the dedicated assertion because it gives a clearer failure message when the error type does not match.
The zero value of the exported type was a foot-gun: a bare
&KeyringCache{} has a nil backend and a nil errNotFound sentinel, which
would panic on first Store or Lookup. Since the only valid construction
is via NewKeyringCache, lower the struct to keyringCache and have the
constructor return cache.TokenCache directly. Callers never see the
concrete type.
Drops the explicit if-branch for empty Op in favor of cmp.Or, matching modern Go 1.25 idioms. Same output, one less branch.
Per go-code-structure.md principles 1 and 2, move the precedence logic into a pure resolveStorageMode(override, envValue, configValue) that takes plain strings and has no side effects. ResolveStorageMode stays as a thin wrapper that reads the env var and the .databrickscfg setting and delegates. The pure core is now trivially testable without t.Setenv or TempDir. TestResolveStorageMode covers all 8 precedence cases inline. TestResolveStorageMode_ReadsEnvAndConfig separately verifies the wrapper actually reads from env and config (3 small subcases). Also flips mode_test.go to package storage (internal) so the pure core is reachable without exporting it.
Approval status: pending
|
4 tasks
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.
Why
Groundwork for the CLI GA milestone that introduces OS-native secure token storage behind an experimental opt-in (MS1 of the CLI GA rollout). This PR adds the building blocks without wiring them into any command. A follow-up PR will plug the resolver and cache into login / refresh / token / logout.
Changes
Before: the CLI only has the SDK's file-backed
TokenCacheand has no way to select a different storage backend at runtime.Now: three additive pieces, all dormant. Nothing imports the new
libs/auth/storagepackage from production code yet.libs/auth/storage/mode.go:StorageModeenum (legacy,secure,plaintext) andResolveStorageMode(ctx, override)that resolves precedenceoverride -> DATABRICKS_AUTH_STORAGE env -> [__settings__].auth_storage in .databrickscfg -> Legacy.libs/auth/storage/keyring.go:KeyringCacheimplementation of the SDK'scache.TokenCache, backed bygithub.com/zalando/go-keyring(MIT, same library used by GitHub CLI). Includes a 3-second per-operation timeout that protects against Linux D-Bus hangs, and a pluggable backend interface so tests inject a fake without touching the OS keychain. Service name isdatabricks-cli; the account field carries the cache key the SDK passes through.libs/databrickscfg/ops.go:GetConfiguredAuthStoragereader, mirroring the existingGetConfiguredDefaultProfileshape.go.mod/go.sum/NOTICE/NEXT_CHANGELOG.md: dependency add and required metadata.No command code changes. No user-visible behavior change.
Test plan