auth: handle account-only profiles cleanly for workspace and describe commands#5340
Open
simonfaltum wants to merge 4 commits into
Open
auth: handle account-only profiles cleanly for workspace and describe commands#5340simonfaltum wants to merge 4 commits into
simonfaltum wants to merge 4 commits into
Conversation
Profiles created with `databricks auth login --skip-workspace` are persisted with the CLI-only `workspace_id = none` sentinel. Running a workspace command (e.g. `clusters list`) against such a profile failed with an opaque "Credential was not sent or was of an unsupported type" error from the auth endpoint, because the SDK forwarded the literal string "none" as a routing identifier. Detect the sentinel in `workspaceClientOrPrompt` before any API call and return a message that names the profile and suggests concrete fixes (edit the profile to set a real workspace_id, or pass --profile with a workspace-scoped profile). Co-authored-by: Isaac
Contributor
Waiting for approvalBased on git history, these people are best suited to review:
Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
PR #5338 stops writing workspace_id = none on --skip-workspace. After it lands, new account-only profiles have no workspace_id key at all, so the "= none" check here misses them. Broaden the detection to fire when cfg.Profile is set, cfg.AccountID is non-empty, and cfg.WorkspaceID is either empty or the legacy "none" sentinel. Reword the error to talk about "no workspace_id set" instead of the literal sentinel. The cfg.Profile guard keeps env-var-only configs targeting a unified host out of the rejection path (their workspace APIs are served from the account host and auth resolves end-to-end). Co-authored-by: Isaac
Collaborator
|
Commit: fa7f3a1 |
Without this, `databricks auth describe --profile <account-only>` reached `w.CurrentUser.Me()` in describe.go, which on a SPOG host with workspace_id absent (or the legacy "none" sentinel) hit the account-plane and was rejected by the backend with "Unable to load OAuth Config" — a false negative for a valid profile. - Promote `accountOnlyProfileError` from a plain `errors.Errorf` to a typed `ErrAccountOnlyProfile` so callers can detect it. - Extend MustAnyClient's fall-through check to include the new type, so workspace commands (clusters list, etc.) keep getting the clear actionable error from PR #5340 while MustAnyClient callers (auth describe, the only one today) recover and describe the profile via the account client. Co-authored-by: Isaac
GPT review pointed out the previous TestMustAnyClientFallsThroughOnAccountOnlyProfile only verified workspaceClientOrPrompt returned ErrAccountOnlyProfile — it never called MustAnyClient, so removing the fall-through case from MustAnyClient wouldn't have failed the test. Drive it end-to-end: build the same cobra command with --profile, invoke MustAnyClient, and assert it returns (true, nil) with an AccountClient on the context. Co-authored-by: Isaac
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
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
Two related bugs in a bug bash, both rooted in profiles created with `databricks auth login --skip-workspace` (no `workspace_id`, or the legacy `workspace_id = none` sentinel):
Reproduced both against `db-deco-test.databricks.com` with a real `--skip-workspace` profile.
Changes
After the fix on the same `db-deco-test` profile:
```
$ databricks auth describe --profile bug2-test
Host: https://db-deco-test.databricks.com
Account ID: 968367da-...
Authenticated with: databricks-cli
✓ host: https://db-deco-test.databricks.com (from .databrickscfg)
✓ account_id: 968367da-... (from .databrickscfg)
✓ workspace_id: none (from .databrickscfg)
✓ profile: bug2-test (from --profile flag)
✓ auth_type: databricks-cli
...
$ databricks clusters list --profile bug2-test
Error: profile "bug2-test" has no workspace_id set (account-only); this command requires a workspace.
Edit the profile to set workspace_id to a real ID, or pass --profile with a workspace-scoped profile
```
The env-var-only path (no `Profile` set) is intentionally left alone — unified hosts can serve workspace APIs from the account host with just `DATABRICKS_HOST` + `DATABRICKS_ACCOUNT_ID` + a token, and we don't want to reject those.
Test plan