Skip to content

Commit dd23604

Browse files
authored
auth: stop writing workspace_id = none on --skip-workspace (#5338)
## Why `databricks auth login --skip-workspace` writes a literal `workspace_id = none` into `.databrickscfg` when no workspace can be discovered. That string is a CLI-internal sentinel. ## Changes - Before: `--skip-workspace` (and "Skip" picked in the interactive workspace prompt) set the in-memory `WorkspaceIDNone` sentinel, which was then saved to disk as `workspace_id = none`. - Now: both paths leave `WorkspaceID` empty. `SaveToProfile` already omits empty fields, so account-only profiles land with no `workspace_id` key at all. `MatchAccountProfiles` already treats absent and `none` identically, so legacy profiles that still have `workspace_id = none` keep working. `auth.WorkspaceIDNone` stays defined for backwards-compatible parsing. Re-login regression fix: without the sentinel, an account-only profile loads back with `WorkspaceID == ""`, which has the same shape as a fresh login that still needs a workspace pick. Without further changes, `databricks auth login --profile foo` (no `--skip-workspace`) would prompt again on every run. Extract `shouldPromptWorkspace` and treat "existing profile is already account-only" as honoring the user's prior skip choice. Covers both the new (absent `workspace_id`) and legacy (`workspace_id = none`) shapes. ## Test plan - [x] `go test ./cmd/auth/...` (existing tests, including `TestSetHostAndAccountId_WorkspaceIDNoneSentinelInherited`) - [x] New `TestShouldPromptWorkspace` table-driven test exercises re-login into legacy and new-shape account-only profiles, a workspace profile, `--skip-workspace`, and the no-account-id / known-workspace-id short-circuits. - [x] `go test ./acceptance -run TestAccept/cmd/auth/profiles/spog-account` (legacy `workspace_id = none` profile still parses as account) - [x] `./task checks` - [x] `./task lint-q`
1 parent b999e77 commit dd23604

3 files changed

Lines changed: 99 additions & 16 deletions

File tree

cmd/auth/login.go

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -328,27 +328,17 @@ a new profile is created.
328328
// 2. Configuring cluster and serverless;
329329
// 3. Saving the profile.
330330

331-
// If discovery gave us an account_id but we still have no workspace_id,
332-
// prompt the user to select a workspace. This applies to any host where
333-
// .well-known/databricks-config returned an account_id.
334-
shouldPromptWorkspace := authArguments.AccountID != "" &&
335-
authArguments.WorkspaceID == "" &&
336-
!skipWorkspace
337-
338-
if skipWorkspace && authArguments.WorkspaceID == "" {
339-
authArguments.WorkspaceID = auth.WorkspaceIDNone
340-
}
341-
342-
if shouldPromptWorkspace {
331+
if shouldPromptWorkspace(authArguments, existingProfile, skipWorkspace) {
343332
wsID, wsErr := promptForWorkspaceSelection(ctx, authArguments, persistentAuth)
344333
if wsErr != nil {
345334
log.Warnf(ctx, "Workspace selection failed: %v", wsErr)
346-
} else if wsID == "" {
347-
// User selected "Skip" from the prompt.
348-
authArguments.WorkspaceID = auth.WorkspaceIDNone
349-
} else {
335+
} else if wsID != "" {
350336
authArguments.WorkspaceID = wsID
351337
}
338+
// If wsID is empty, the user picked "Skip" — leave WorkspaceID empty.
339+
// SaveToProfile omits the workspace_id key entirely for account-level
340+
// profiles; MatchAccountProfiles treats absent workspace_id the same
341+
// as the legacy "none" sentinel.
352342
}
353343

354344
var clusterID, serverlessComputeID string
@@ -417,6 +407,26 @@ a new profile is created.
417407
return cmd
418408
}
419409

410+
// shouldPromptWorkspace reports whether the login flow should ask the user to
411+
// pick a workspace. We prompt when we have an account_id but no workspace_id
412+
// and the user did not pass --skip-workspace, with one exception: re-login
413+
// into an existing profile that's already account-only for the SAME account
414+
// (account_id matches and workspace_id is absent or the legacy "none"
415+
// sentinel) honors the user's prior "skip" choice instead of re-prompting on
416+
// every login. We require the account_id to match so reusing a profile name
417+
// against a different account still gets the workspace prompt.
418+
func shouldPromptWorkspace(authArguments *auth.AuthArguments, existingProfile *profile.Profile, skipWorkspace bool) bool {
419+
if authArguments.AccountID == "" || authArguments.WorkspaceID != "" || skipWorkspace {
420+
return false
421+
}
422+
if existingProfile != nil &&
423+
existingProfile.AccountID == authArguments.AccountID &&
424+
(existingProfile.WorkspaceID == "" || existingProfile.WorkspaceID == auth.WorkspaceIDNone) {
425+
return false
426+
}
427+
return true
428+
}
429+
420430
// Sets the host in the persistentAuth object based on the provided arguments and flags.
421431
// Follows the following precedence:
422432
// 1. [HOST] (first positional argument) or --host flag. Error if both are specified.

cmd/auth/login_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,75 @@ func TestSetHostAndAccountId_WorkspaceIDNoneSentinelInherited(t *testing.T) {
546546
assert.Equal(t, auth.WorkspaceIDNone, args.WorkspaceID)
547547
}
548548

549+
func TestShouldPromptWorkspace(t *testing.T) {
550+
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")
551+
ctx, _ := cmdio.SetupTest(t.Context(), cmdio.TestOptions{})
552+
553+
legacyAccountProfile := loadTestProfile(t, ctx, "spog-skip-workspace")
554+
newAccountProfile := loadTestProfile(t, ctx, "spog-skip-workspace-new")
555+
workspaceProfile := loadTestProfile(t, ctx, "unified-workspace")
556+
557+
tests := []struct {
558+
name string
559+
authArguments auth.AuthArguments
560+
existingProfile *profile.Profile
561+
skipWorkspace bool
562+
want bool
563+
}{
564+
{
565+
name: "no profile, account_id set, no workspace_id",
566+
authArguments: auth.AuthArguments{AccountID: "acc"},
567+
want: true,
568+
},
569+
{
570+
name: "re-login into legacy account-only profile (workspace_id = none)",
571+
authArguments: auth.AuthArguments{AccountID: "spog-account"},
572+
existingProfile: legacyAccountProfile,
573+
want: false,
574+
},
575+
{
576+
name: "re-login into new account-only profile (no workspace_id key)",
577+
authArguments: auth.AuthArguments{AccountID: "spog-account"},
578+
existingProfile: newAccountProfile,
579+
want: false,
580+
},
581+
{
582+
name: "re-login into workspace profile prompts when workspace_id missing from args",
583+
authArguments: auth.AuthArguments{AccountID: "test-unified-account"},
584+
existingProfile: workspaceProfile,
585+
want: true,
586+
},
587+
{
588+
name: "account-only profile for a different account still prompts",
589+
authArguments: auth.AuthArguments{AccountID: "different-account"},
590+
existingProfile: legacyAccountProfile,
591+
want: true,
592+
},
593+
{
594+
name: "skipWorkspace suppresses the prompt",
595+
authArguments: auth.AuthArguments{AccountID: "acc"},
596+
skipWorkspace: true,
597+
want: false,
598+
},
599+
{
600+
name: "no account_id means no prompt",
601+
authArguments: auth.AuthArguments{},
602+
want: false,
603+
},
604+
{
605+
name: "workspace_id already known means no prompt",
606+
authArguments: auth.AuthArguments{AccountID: "acc", WorkspaceID: "12345"},
607+
want: false,
608+
},
609+
}
610+
for _, tt := range tests {
611+
t.Run(tt.name, func(t *testing.T) {
612+
got := shouldPromptWorkspace(&tt.authArguments, tt.existingProfile, tt.skipWorkspace)
613+
assert.Equal(t, tt.want, got)
614+
})
615+
}
616+
}
617+
549618
func TestSetHostAndAccountId_URLParamsOverrideProfile(t *testing.T) {
550619
t.Setenv("DATABRICKS_CONFIG_FILE", "./testdata/.databrickscfg")
551620
ctx, _ := cmdio.SetupTest(t.Context(), cmdio.TestOptions{})

cmd/auth/testdata/.databrickscfg

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,3 +31,7 @@ experimental_is_unified_host = true
3131
host = https://spog.example.com
3232
account_id = spog-account
3333
workspace_id = none
34+
35+
[spog-skip-workspace-new]
36+
host = https://spog.example.com
37+
account_id = spog-account

0 commit comments

Comments
 (0)