diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a07b8820902..7dcae4c7b12 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -13,7 +13,6 @@ * `databricks auth profiles --skip-validate` no longer makes any network calls; the host metadata fetch is skipped along with validation ([#5530](https://github.com/databricks/cli/pull/5530)). - ### Bundles * Set the default `data_security_mode` to `DATA_SECURITY_MODE_AUTO` in bundle templates ([#5452](https://github.com/databricks/cli/pull/5452)). * Mark vector search index index_subtype as backend_default to prevent drift after deployment ([#5454](https://github.com/databricks/cli/pull/5454)). diff --git a/cmd/root/auth.go b/cmd/root/auth.go index 6a85b52713b..fb766e926bc 100644 --- a/cmd/root/auth.go +++ b/cmd/root/auth.go @@ -79,16 +79,26 @@ func patSPOGNoWorkspaceIDError(profileName string) error { // client so account-only profiles still describe cleanly. type ErrAccountOnlyProfile struct { profileName string + // host is the profile's canonical host name (config.CanonicalHostName); + // IsClassicAccountHost only matches the canonical form. + host string } func (e ErrAccountOnlyProfile) Error() string { + // A classic account console host serves no workspace APIs at all, so the + // generic "set workspace_id" advice below can never make workspace + // commands work there; following it is how broken profiles get created + // (https://github.com/databricks/cli/issues/5479). + if auth.IsClassicAccountHost(e.host) { + return fmt.Sprintf("profile %q points to a Databricks account console host (%s), which serves only account-level APIs; this command requires a workspace. Run `databricks auth login --host https://` to create a workspace profile, or use `databricks account ...` commands with this profile", e.profileName, e.host) + } return fmt.Sprintf("profile %q 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", e.profileName) } // accountOnlyProfileError describes why a workspace command can't run against // a profile that has an account_id but no workspace_id. -func accountOnlyProfileError(profileName string) error { - return ErrAccountOnlyProfile{profileName: profileName} +func accountOnlyProfileError(cfg *config.Config) error { + return ErrAccountOnlyProfile{profileName: cfg.Profile, host: cfg.CanonicalHostName()} } func profileFlagValue(cmd *cobra.Command) (string, bool) { @@ -245,7 +255,7 @@ func workspaceClientOrPrompt(ctx context.Context, cfg *config.Config, allowPromp // can recognize ErrAccountOnlyProfile and fall through to the account // client; the PAT-on-SPOG check below handles the remaining cases // (env-var-only configs and profiles without account_id resolved). - return nil, accountOnlyProfileError(cfg.Profile) + return nil, accountOnlyProfileError(cfg) } if err == nil && isPATOnSPOGWithoutWorkspaceID(cfg) { // PATs are workspace-scoped. On a SPOG host without workspace_id the diff --git a/cmd/root/auth_test.go b/cmd/root/auth_test.go index 4187b6525d1..e2c86786523 100644 --- a/cmd/root/auth_test.go +++ b/cmd/root/auth_test.go @@ -549,6 +549,57 @@ func TestWorkspaceClientOrPromptRejectsAccountOnlyProfile(t *testing.T) { } } +func TestErrAccountOnlyProfileMessage(t *testing.T) { + tests := []struct { + name string + err ErrAccountOnlyProfile + want string + }{ + { + name: "account console host", + err: ErrAccountOnlyProfile{profileName: "acc", host: "https://accounts.test"}, + want: "profile \"acc\" points to a Databricks account console host (https://accounts.test), which serves only account-level APIs; " + + "this command requires a workspace. Run `databricks auth login --host https://` to create a workspace profile, " + + "or use `databricks account ...` commands with this profile", + }, + { + // On non-account-console hosts (SPOG/unified) workspace APIs are + // served, so setting workspace_id is still the right fix. + name: "other host keeps workspace_id advice", + err: ErrAccountOnlyProfile{profileName: "spog", host: "https://unified.test"}, + want: "profile \"spog\" 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", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, tt.err.Error()) + }) + } +} + +func TestWorkspaceClientOrPromptAccountOnlyProfileOnAccountConsoleHost(t *testing.T) { + testutil.CleanupEnvironment(t) + t.Setenv("PATH", "") + + cfg := &config.Config{ + Host: "https://accounts.test/", + AccountID: "abc-123", + Token: "foobar", + Profile: "acc", + HTTPTransport: noNetworkTransport, + } + + w, err := workspaceClientOrPrompt(t.Context(), cfg, false) + assert.Nil(t, w) + require.Error(t, err) + var accountOnly ErrAccountOnlyProfile + require.ErrorAs(t, err, &accountOnly) + assert.Contains(t, err.Error(), "account console host (https://accounts.test)") + assert.Contains(t, err.Error(), "databricks auth login --host") + assert.NotContains(t, err.Error(), "set workspace_id to a real ID") +} + func TestWorkspaceClientOrPromptRejectsPATOnSPOGWithoutWorkspaceID(t *testing.T) { testutil.CleanupEnvironment(t) t.Setenv("PATH", "") diff --git a/cmd/root/root.go b/cmd/root/root.go index 6b6de2a9baa..d1b12ac7f69 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -149,6 +149,12 @@ Stack Trace: cfg := cmdctx.ConfigUsed(cmd.Context()) err = auth.EnrichAuthError(cmd.Context(), cfg, err) } + // A workspace client on the context means the command operates against + // a workspace; see AppendAccountHostHint for why every error from such + // commands gets the account-console-host note. + if cmdctx.HasWorkspaceClient(cmd.Context()) { + err = auth.AppendAccountHostHint(cmdctx.WorkspaceClient(cmd.Context()).Config, err) + } fmt.Fprintf(cmd.ErrOrStderr(), "Error: %s\n", err.Error()) } diff --git a/cmd/root/root_test.go b/cmd/root/root_test.go index 44a07073984..ce1584eacd4 100644 --- a/cmd/root/root_test.go +++ b/cmd/root/root_test.go @@ -2,9 +2,12 @@ package root import ( "bytes" + "context" + "errors" "testing" "github.com/databricks/cli/libs/cmdctx" + "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/apierr" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -77,6 +80,85 @@ func TestExecuteNoEnrichmentWithoutConfigUsed(t *testing.T) { assert.NotContains(t, output, "Next steps:") } +func TestExecuteAppendsAccountHostHint(t *testing.T) { + tests := []struct { + name string + setup func(ctx context.Context) context.Context + wantHint bool + }{ + { + name: "workspace client on account console host", + setup: func(ctx context.Context) context.Context { + cfg := &config.Config{Host: "https://accounts.test", Profile: "acc"} + ctx = cmdctx.SetConfigUsed(ctx, cfg) + return cmdctx.SetWorkspaceClient(ctx, &databricks.WorkspaceClient{Config: cfg}) + }, + wantHint: true, + }, + { + name: "workspace client on workspace host", + setup: func(ctx context.Context) context.Context { + cfg := &config.Config{Host: "https://adb-123.test", Profile: "ws"} + ctx = cmdctx.SetConfigUsed(ctx, cfg) + return cmdctx.SetWorkspaceClient(ctx, &databricks.WorkspaceClient{Config: cfg}) + }, + wantHint: false, + }, + { + // `databricks account ...` commands configure an account client, + // not a workspace client, so the hint must stay silent. + name: "account client on account console host", + setup: func(ctx context.Context) context.Context { + cfg := &config.Config{Host: "https://accounts.test", Profile: "acc"} + ctx = cmdctx.SetConfigUsed(ctx, cfg) + return cmdctx.SetAccountClient(ctx, &databricks.AccountClient{Config: cfg}) + }, + wantHint: false, + }, + { + name: "no client on account console host", + setup: func(ctx context.Context) context.Context { + cfg := &config.Config{Host: "https://accounts.test", Profile: "acc"} + return cmdctx.SetConfigUsed(ctx, cfg) + }, + wantHint: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stderr := &bytes.Buffer{} + cmd := &cobra.Command{ + Use: "test", + SilenceUsage: true, + SilenceErrors: true, + RunE: func(cmd *cobra.Command, args []string) error { + // The account console returns unstructured junk for + // workspace API paths; this is one real example. + return errors.New("received HTML response instead of JSON") + }, + } + cmd.SetErr(stderr) + cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + cmd.SetContext(tt.setup(cmd.Context())) + return nil + } + + err := Execute(t.Context(), cmd) + require.Error(t, err) + + output := stderr.String() + assert.Contains(t, output, "received HTML response instead of JSON") + if tt.wantHint { + assert.Contains(t, output, "account console host") + assert.Contains(t, output, "databricks auth login --host") + } else { + assert.NotContains(t, output, "account console host") + } + }) + } +} + func TestExecuteErrAlreadyPrintedNotEnriched(t *testing.T) { ctx := t.Context() stderr := &bytes.Buffer{} diff --git a/libs/auth/error.go b/libs/auth/error.go index 7cfb6b6fc76..5bd5d3d43c6 100644 --- a/libs/auth/error.go +++ b/libs/auth/error.go @@ -112,6 +112,27 @@ func EnrichAuthError(ctx context.Context, cfg *config.Config, err error) error { return fmt.Errorf("%w\n%s", err, b.String()) } +// AppendAccountHostHint appends a note to errors from commands that ran with a +// workspace client configured against a classic account console host. Such +// hosts serve only account-level APIs, so a workspace command can never +// succeed there and the note is relevant for any error. The console's +// responses to workspace API paths are unstructured (HTML pages, bare-string +// 400s), so there is no reliable way to detect "this API is not served here" +// from the error itself; callers gate on the command type instead. +func AppendAccountHostHint(cfg *config.Config, err error) error { + host := cfg.CanonicalHostName() + if !IsClassicAccountHost(host) { + return err + } + subject := "this configuration" + scope := "" + if cfg.Profile != "" { + subject = fmt.Sprintf("profile %q", cfg.Profile) + scope = " with this profile" + } + return fmt.Errorf("%w\n\nNote: %s points to a Databricks account console host (%s), which serves only account-level APIs.\nWorkspace commands need a workspace host: run `databricks auth login --host https://`, or use `databricks account ...` commands%s", err, subject, host, scope) +} + // writeReauthSteps writes auth-type-aware re-authentication suggestions for 401 errors. func writeReauthSteps(ctx context.Context, cfg *config.Config, b *strings.Builder) { switch strings.ToLower(cfg.AuthType) { diff --git a/libs/auth/error_test.go b/libs/auth/error_test.go index 1e724a4b251..ce8f0df1b63 100644 --- a/libs/auth/error_test.go +++ b/libs/auth/error_test.go @@ -2,6 +2,7 @@ package auth import ( "errors" + "strings" "testing" "github.com/databricks/databricks-sdk-go/apierr" @@ -260,3 +261,67 @@ func TestEnrichAuthError(t *testing.T) { }) } } + +func TestAppendAccountHostHint(t *testing.T) { + tests := []struct { + name string + cfg *config.Config + want string + }{ + { + name: "account console host with profile", + cfg: &config.Config{Host: "https://accounts.test", Profile: "acc"}, + want: "base error\n\n" + + "Note: profile \"acc\" points to a Databricks account console host (https://accounts.test), which serves only account-level APIs.\n" + + "Workspace commands need a workspace host: run `databricks auth login --host https://`, or use `databricks account ...` commands with this profile", + }, + { + name: "account console host without profile", + cfg: &config.Config{Host: "https://accounts.test"}, + want: "base error\n\n" + + "Note: this configuration points to a Databricks account console host (https://accounts.test), which serves only account-level APIs.\n" + + "Workspace commands need a workspace host: run `databricks auth login --host https://`, or use `databricks account ...` commands", + }, + { + name: "workspace host is left unchanged", + cfg: &config.Config{Host: "https://adb-123.test", Profile: "ws"}, + want: "base error", + }, + { + name: "empty host is left unchanged", + cfg: &config.Config{}, + want: "base error", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := AppendAccountHostHint(tt.cfg, errors.New("base error")) + assert.Equal(t, tt.want, result.Error()) + }) + } +} + +func TestAppendAccountHostHintPreservesErrorChain(t *testing.T) { + cfg := &config.Config{Host: "https://accounts.test", Profile: "acc"} + original := &apierr.APIError{StatusCode: 400, Message: "Unable to load OAuth Config"} + + result := AppendAccountHostHint(cfg, original) + + var unwrapped *apierr.APIError + require.ErrorAs(t, result, &unwrapped) + assert.Equal(t, 400, unwrapped.StatusCode) +} + +func TestAppendAccountHostHintComposesWithEnrichAuthError(t *testing.T) { + cfg := &config.Config{Host: "https://accounts.test", Profile: "acc", AuthType: AuthTypePat} + original := &apierr.APIError{StatusCode: 403, Message: "permission denied"} + + // Same composition order as cmd/root.Execute: enrichment first, hint last. + result := AppendAccountHostHint(cfg, EnrichAuthError(t.Context(), cfg, original)) + + msg := result.Error() + assert.Contains(t, msg, "permission denied") + assert.Contains(t, msg, "Next steps:") + assert.Contains(t, msg, "Note: profile \"acc\" points to a Databricks account console host") + assert.Less(t, strings.Index(msg, "Next steps:"), strings.Index(msg, "Note:")) +} diff --git a/libs/cmdctx/workspace_client.go b/libs/cmdctx/workspace_client.go index 194beee5f66..915ea0337c7 100644 --- a/libs/cmdctx/workspace_client.go +++ b/libs/cmdctx/workspace_client.go @@ -20,3 +20,9 @@ func WorkspaceClient(ctx context.Context) *databricks.WorkspaceClient { } return v.(*databricks.WorkspaceClient) } + +// HasWorkspaceClient reports whether a workspace client was configured on the +// context via SetWorkspaceClient. +func HasWorkspaceClient(ctx context.Context) bool { + return ctx.Value(workspaceClientKey) != nil +} diff --git a/libs/cmdctx/workspace_client_test.go b/libs/cmdctx/workspace_client_test.go index cd3959602f5..3f2942d8e81 100644 --- a/libs/cmdctx/workspace_client_test.go +++ b/libs/cmdctx/workspace_client_test.go @@ -36,3 +36,16 @@ func TestCommandWorkspaceClient(t *testing.T) { cmdctx.SetWorkspaceClient(ctx, client) }) } + +func TestHasWorkspaceClient(t *testing.T) { + ctx := t.Context() + assert.False(t, cmdctx.HasWorkspaceClient(ctx)) + + client := &databricks.WorkspaceClient{ + Config: &config.Config{ + Host: "https://test.test", + }, + } + ctx = cmdctx.SetWorkspaceClient(ctx, client) + assert.True(t, cmdctx.HasWorkspaceClient(ctx)) +}