Add interactive profile picker to auth logout#4616
Add interactive profile picker to auth logout#4616mihaimitrea-db wants to merge 25 commits intomainfrom
Conversation
|
Commit: e5a1d88
19 interesting tests: 7 SKIP, 6 RECOVERED, 5 flaky, 1 FAIL
Top 50 slowest tests (at least 2 minutes):
|
simonfaltum
left a comment
There was a problem hiding this comment.
Looks good! I left some comments
f374e5a to
c903ed8
Compare
c903ed8 to
889b7ca
Compare
889b7ca to
7a39778
Compare
7a39778 to
67eff3f
Compare
cmd/auth/logout.go
Outdated
| slices.SortFunc(allProfiles, func(a, b profile.Profile) int { | ||
| return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name)) | ||
| }) |
There was a problem hiding this comment.
do we always sort profiles alphabetically? I'd think we just want to show them in the order they were added (or alternatively last used or most used but we don't log that anywhere..)
There was a problem hiding this comment.
My personal preference is to have lists sorted, but you are right, no other picker in the codebase does this so for consistency let's go wth the order in which they were added.
| // promptForLogoutProfile shows an interactive profile picker for logout. | ||
| // Account profiles are displayed as "name (account: id)", workspace profiles | ||
| // as "name (host)". | ||
| func promptForLogoutProfile(ctx context.Context, profiler profile.Profiler) (string, error) { |
There was a problem hiding this comment.
I think this is good work but did you look through the codebase for similar patterns?
I think the closest existing pattern is promptForProfileSelection in cmd/auth/token.go (line ~368), which already handles an "all profiles" picker with search by name/host and the StartInSearchMode: len(profiles) > 5 pattern.
I think we are re-implementing much of this logic, which maybe is justified but it might be worth it to make a component we could re-use broadly as selecting profile interactively is something we will have to do multiple times. I think in auth token it lets you create a new profile also, which is probably not something we want to do in logout 😃
There was a problem hiding this comment.
I tried fixing that in the next PR on this stack. Check that out: #4647
a87d5a2 to
bcf6e70
Compare
bcf6e70 to
e536cc8
Compare
Resolve config path from the profiler instead of hardcoding fallbacks. Delete the profile before clearing the token cache so a config write failure does not leave tokens removed. Fix token cleanup for account and unified profiles by computing the correct OIDC cache key (host/oidc/accounts/<account_id>). Drop the nil profiler guard, add a success message on logout, and extract backupConfigFile in ops.go to remove duplication. Consolidate token cleanup tests into a table-driven test covering shared hosts, unique hosts, account, and unified profiles.
Merge shared-host token deletion verification into one main parametrized test by addding the hostBasedKey and isSharedKey parameters to each case. This replaces the TestLogoutTokenCacheCleanup test with an assertion: host-based keys are preserved when another profile shares the same host, and deleted otherwise.
Rewrite the test to use inline config seeds and explicit expected state. Add cases for deleting the last non-default profile, deleting a unified host profile with multiple keys, and deleting the DEFAULT section.
- Use profiler.GetPath() to resolve config path instead of hardcoding platform-specific defaults for the help text. - Read DATABRICKS_CONFIG_FILE via env.Get(ctx, ...) instead of os.Getenv to respect context-level env overrides. - Add abort message when user declines the confirmation prompt. - Guard DeleteProfile against non-existent profiles to avoid creating unnecessary backup files. - Add TestDeleteProfile_NotFound for the error path.
Cover four scenarios: profile ordering and comments are preserved after deletion, deleting the last non-default profile leaves an empty DEFAULT section, deleting the DEFAULT profile itself clears its keys and restores the default comment, and error paths for non-existent profiles and missing --profile in non-interactive mode.
By default, Authentication related commands. For more information regarding how authentication for the Databricks CLI and SDKs work please refer to the documentation linked below. AWS: https://docs.databricks.com/dev-tools/auth/index.html Azure: https://learn.microsoft.com/azure/databricks/dev-tools/auth GCP: https://docs.gcp.databricks.com/dev-tools/auth/index.html Usage: databricks auth [command] Available Commands: describe Describes the credentials and the source of those credentials, being used by the CLI to authenticate env Get env login Log into a Databricks workspace or account profiles Lists profiles from ~/.databrickscfg token Get authentication token Flags: --account-id string Databricks Account ID --experimental-is-unified-host Flag to indicate if the host is a unified host -h, --help help for auth --host string Databricks Host --workspace-id string Databricks Workspace ID Global Flags: --debug enable debug logging -o, --output type output type: text or json (default text) -p, --profile string ~/.databrickscfg profile -t, --target string bundle target to use (if applicable) Use "databricks auth [command] --help" for more information about a command. now only clears cached OAuth tokens without removing the profile from ~/.databrickscfg. Pass --delete to also remove the profile entry from the config file.
Existing acceptance tests that verify profile deletion now use --delete since profile removal is opt-in. Two new acceptance tests verify token-only logout: one for a unique host (both cache entries cleared) and one for a shared host (host-keyed token preserved).
Replace manual strings.TrimRight(host, /) with the SDK's config.Config.CanonicalHostName(), which handles scheme normalization, trailing slashes, and empty hosts consistently with how the SDK itself computes token cache keys.
- Make Long description static to avoid calling logger and GetPath at command construction time before the logger is initialized. - Remove empty test.toml files from acceptance tests. - Add \n to error-case titles so errors appear on a separate line. - Use .tokens | keys in jq queries for token cache to reduce verbosity. - Switch test profiles from PAT to auth_type=databricks-cli (U2M) so token cache tests exercise a realistic OAuth logout flow. - Add AuthType field to profile.Profile to detect non-U2M profiles; skip token cache cleanup and adjust success message accordingly. - Add delete-m2m-profiles acceptance test covering PAT profile logout with and without --delete. - Fix DeleteProfile to clear DEFAULT section keys instead of deleting and recreating it, preserving its position in the file.
- Rename isU2MProfile to isCreatedByLogin to accurately reflect that the check is specific to profiles created by . - Tighten success and error messages: drop "Successfully", add actionable suggestions (e.g. "Use --delete to also remove it"), and include retry guidance on partial failures. - Return errors instead of logging on DeleteProfile failure so callers see a non-zero exit code. - Fix token-only acceptance test: use OIDC-style cache key (host/oidc/accounts/<id>) for account profiles so both token cache entries are correctly cleaned up.
4bd2e44 to
d8fe900
Compare
When --profile is not specified in an interactive terminal, show a searchable prompt listing all configured profiles. Profiles are sorted alphabetically and displayed with their host or account ID. The picker supports fuzzy search by name, host, or account ID.
Document the four interaction modes (explicit profile, interactive picker, non-interactive error, and --force) in the command's long help text.
d8fe900 to
e5a1d88
Compare
…ckers (#4647) ## 🥞 Stacked PR Use this [link](https://github.com/databricks/cli/pull/4647/files/e5a1d8842ee9a458d4ffb3cea672a0f88077356f..51f14b5c7ea409cffdc78a3025a70de5c674fa9a) to review incremental changes. - [stack/auth_logout](#4613) [[Files changed](https://github.com/databricks/cli/pull/4613/files)] - [stack/auth_logout_profile_picker](#4616) [[Files changed](https://github.com/databricks/cli/pull/4616/files/c9e8d79e276d6c33faa1e859ca20cc5136f9efb7..e5a1d8842ee9a458d4ffb3cea672a0f88077356f)] - [**stack/auth_logout_deduplication**](#4647) [[Files changed](https://github.com/databricks/cli/pull/4647/files/e5a1d8842ee9a458d4ffb3cea672a0f88077356f..51f14b5c7ea409cffdc78a3025a70de5c674fa9a)] --------- Four places built nearly identical `promptui.Select` prompts for interactive profile selection (`auth logout`, `auth token`, `cmd/root/auth.go`, `cmd/root/bundle.go`). This PR extracts a reusable `profile.SelectProfile` function that accepts a declarative `SelectConfig` with label, profiles, and template strings, replacing all four implementations. ## Changes - Add `profile.SelectProfile` in `libs/databrickscfg/profile/select.go` — a shared interactive profile picker that accepts a `SelectConfig` (label, profiles, template strings) and returns the selected profile name. - Replace the four inline `promptui.Select` implementations in `cmd/auth/logout.go`, `cmd/auth/token.go`, `cmd/root/auth.go`, and `cmd/root/bundle.go` with calls to `SelectProfile`. - Add `AccountID` to `Profiles.SearchCaseInsensitive` so all pickers support searching by account ID, not just name and host. - Extract `writeConfigFile` helper in `libs/databrickscfg/ops.go` to consolidate the repeated default-comment / backup / save sequence shared by `SaveToProfile` and `DeleteProfile`. ## Why The four profile pickers each duplicated the same prompt setup, searcher wiring, and result extraction. This made it easy for behavior to diverge (e.g., only the logout picker searched by account ID). A single shared helper keeps the UX consistent and reduces the surface area for future changes. ## Tests - Existing unit and acceptance tests for `auth logout`, `auth token`, workspace/account profile selection, and bundle profile resolution continue to pass — the refactor is behavior-preserving. - The `SelectProfile` helper is exercised indirectly through all existing callers. --------- Co-authored-by: simon <simon.faltum@databricks.com>
🥞 Stacked PR
Use this link to review incremental changes.
When --profile is not specified in an interactive terminal, show a searchable prompt listing all configured profiles. Profiles are sorted alphabetically and displayed with their host or account ID. The picker supports fuzzy search by name, host, or account ID.
Changes
Tests