From 0d6294bfbc56cb3c9e99afdb68af565289326b97 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 00:39:58 +0100 Subject: [PATCH 1/9] Add `databricks doctor` diagnostic command Adds a top-level `databricks doctor` command that validates CLI setup by running sequential diagnostic checks: CLI version, config file readability, active profile, authentication, user identity, and network connectivity. Auth failures are reported as check results, not command errors. Supports both text output (colored status icons) and JSON output (`--output json`). Co-authored-by: Isaac --- cmd/cmd.go | 2 + cmd/doctor/checks.go | 217 +++++++++++++++++++++++++++++++++++ cmd/doctor/doctor.go | 79 +++++++++++++ cmd/doctor/doctor_test.go | 235 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 533 insertions(+) create mode 100644 cmd/doctor/checks.go create mode 100644 cmd/doctor/doctor.go create mode 100644 cmd/doctor/doctor_test.go diff --git a/cmd/cmd.go b/cmd/cmd.go index 014471f763..217476f450 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -14,6 +14,7 @@ import ( "github.com/databricks/cli/cmd/cache" "github.com/databricks/cli/cmd/completion" "github.com/databricks/cli/cmd/configure" + "github.com/databricks/cli/cmd/doctor" "github.com/databricks/cli/cmd/experimental" "github.com/databricks/cli/cmd/fs" "github.com/databricks/cli/cmd/labs" @@ -101,6 +102,7 @@ func New(ctx context.Context) *cobra.Command { cli.AddCommand(experimental.New()) cli.AddCommand(psql.New()) cli.AddCommand(configure.New()) + cli.AddCommand(doctor.New()) cli.AddCommand(fs.New()) cli.AddCommand(labs.New(ctx)) cli.AddCommand(sync.New()) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go new file mode 100644 index 0000000000..20961b7bc3 --- /dev/null +++ b/cmd/doctor/checks.go @@ -0,0 +1,217 @@ +package doctor + +import ( + "fmt" + "net/http" + + "github.com/databricks/cli/internal/build" + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/config" + "github.com/spf13/cobra" +) + +const ( + statusPass = "pass" + statusFail = "fail" + statusInfo = "info" +) + +// runChecks runs all diagnostic checks and returns the results. +func runChecks(cmd *cobra.Command) []CheckResult { + var results []CheckResult + + results = append(results, checkCLIVersion()) + results = append(results, checkConfigFile(cmd)) + results = append(results, checkCurrentProfile(cmd)) + + authResult, w := checkAuth(cmd) + results = append(results, authResult) + + if w != nil { + results = append(results, checkIdentity(cmd, w)) + } else { + results = append(results, CheckResult{ + Name: "Identity", + Status: statusFail, + Message: "Skipped (authentication failed)", + }) + } + + results = append(results, checkNetwork(cmd, w)) + return results +} + +func checkCLIVersion() CheckResult { + info := build.GetInfo() + return CheckResult{ + Name: "CLI Version", + Status: statusInfo, + Message: info.Version, + } +} + +func checkConfigFile(cmd *cobra.Command) CheckResult { + ctx := cmd.Context() + profiler := profile.GetProfiler(ctx) + + path, err := profiler.GetPath(ctx) + if err != nil { + return CheckResult{ + Name: "Config File", + Status: statusFail, + Message: "Cannot determine config file path", + Detail: err.Error(), + } + } + + profiles, err := profiler.LoadProfiles(ctx, profile.MatchAllProfiles) + if err != nil { + return CheckResult{ + Name: "Config File", + Status: statusFail, + Message: "Cannot read " + path, + Detail: err.Error(), + } + } + + return CheckResult{ + Name: "Config File", + Status: statusPass, + Message: fmt.Sprintf("%s (%d profiles)", path, len(profiles)), + } +} + +func checkCurrentProfile(cmd *cobra.Command) CheckResult { + profileFlag := cmd.Flag("profile") + profileName := "default" + if profileFlag != nil && profileFlag.Changed { + profileName = profileFlag.Value.String() + } + return CheckResult{ + Name: "Current Profile", + Status: statusInfo, + Message: profileName, + } +} + +func checkAuth(cmd *cobra.Command) (CheckResult, *databricks.WorkspaceClient) { + ctx := cmd.Context() + cfg := &config.Config{} + + profileFlag := cmd.Flag("profile") + if profileFlag != nil && profileFlag.Changed { + cfg.Profile = profileFlag.Value.String() + } + + w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg)) + if err != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Cannot create workspace client", + Detail: err.Error(), + }, nil + } + + req, err := http.NewRequestWithContext(ctx, "", "", nil) + if err != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Internal error", + Detail: err.Error(), + }, nil + } + + err = w.Config.Authenticate(req) + if err != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Authentication failed", + Detail: err.Error(), + }, nil + } + + return CheckResult{ + Name: "Authentication", + Status: statusPass, + Message: fmt.Sprintf("OK (%s)", w.Config.AuthType), + }, w +} + +func checkIdentity(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResult { + ctx := cmd.Context() + me, err := w.CurrentUser.Me(ctx) + if err != nil { + return CheckResult{ + Name: "Identity", + Status: statusFail, + Message: "Cannot fetch current user", + Detail: err.Error(), + } + } + + return CheckResult{ + Name: "Identity", + Status: statusPass, + Message: me.UserName, + } +} + +func checkNetwork(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResult { + var host string + if w != nil { + host = w.Config.Host + } else { + cfg := &config.Config{} + profileFlag := cmd.Flag("profile") + if profileFlag != nil && profileFlag.Changed { + cfg.Profile = profileFlag.Value.String() + } + _ = cfg.EnsureResolved() + host = cfg.Host + } + + return checkNetworkWithHost(cmd, host) +} + +func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { + ctx := cmd.Context() + + if host == "" { + return CheckResult{ + Name: "Network", + Status: statusFail, + Message: "No host configured", + } + } + + req, err := http.NewRequestWithContext(ctx, http.MethodHead, host, nil) + if err != nil { + return CheckResult{ + Name: "Network", + Status: statusFail, + Message: "Cannot create request for " + host, + Detail: err.Error(), + } + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return CheckResult{ + Name: "Network", + Status: statusFail, + Message: "Cannot reach " + host, + Detail: err.Error(), + } + } + resp.Body.Close() + + return CheckResult{ + Name: "Network", + Status: statusPass, + Message: host + " is reachable", + } +} diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go new file mode 100644 index 0000000000..1d8f92274e --- /dev/null +++ b/cmd/doctor/doctor.go @@ -0,0 +1,79 @@ +package doctor + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/flags" + "github.com/fatih/color" + "github.com/spf13/cobra" +) + +// CheckResult holds the outcome of a single diagnostic check. +type CheckResult struct { + Name string `json:"name"` + Status string `json:"status"` // "pass", "fail", "warn", "info" + Message string `json:"message"` + Detail string `json:"detail,omitempty"` +} + +// New returns the doctor command. +func New() *cobra.Command { + cmd := &cobra.Command{ + Use: "doctor", + Args: root.NoArgs, + Short: "Validate your Databricks CLI setup", + } + + cmd.RunE = func(cmd *cobra.Command, args []string) error { + ctx := cmd.Context() + results := runChecks(cmd) + + switch root.OutputType(cmd) { + case flags.OutputJSON: + buf, err := json.MarshalIndent(results, "", " ") + if err != nil { + return err + } + _, err = cmd.OutOrStdout().Write(buf) + return err + case flags.OutputText: + renderResults(ctx, results) + return nil + default: + return fmt.Errorf("unknown output type %s", root.OutputType(cmd)) + } + } + + return cmd +} + +func renderResults(ctx context.Context, results []CheckResult) { + green := color.New(color.FgGreen).SprintFunc() + red := color.New(color.FgRed).SprintFunc() + yellow := color.New(color.FgYellow).SprintFunc() + cyan := color.New(color.FgCyan).SprintFunc() + bold := color.New(color.Bold).SprintFunc() + + for _, r := range results { + var icon string + switch r.Status { + case "pass": + icon = green("[ok]") + case "fail": + icon = red("[FAIL]") + case "warn": + icon = yellow("[warn]") + case "info": + icon = cyan("[info]") + } + msg := fmt.Sprintf("%s %s: %s", icon, bold(r.Name), r.Message) + if r.Detail != "" { + msg += fmt.Sprintf(" (%s)", r.Detail) + } + cmdio.LogString(ctx, msg) + } +} diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go new file mode 100644 index 0000000000..f561a871bd --- /dev/null +++ b/cmd/doctor/doctor_test.go @@ -0,0 +1,235 @@ +package doctor + +import ( + "bytes" + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/flags" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type mockProfiler struct { + profiles profile.Profiles + path string + err error +} + +func (m *mockProfiler) LoadProfiles(_ context.Context, match profile.ProfileMatchFunction) (profile.Profiles, error) { + if m.err != nil { + return nil, m.err + } + var result profile.Profiles + for _, p := range m.profiles { + if match(p) { + result = append(result, p) + } + } + return result, nil +} + +func (m *mockProfiler) GetPath(_ context.Context) (string, error) { + if m.err != nil { + return "", m.err + } + return m.path, nil +} + +func newTestCmd(ctx context.Context) *cobra.Command { + cmd := &cobra.Command{} + cmd.SetContext(ctx) + cmd.Flags().String("profile", "", "") + return cmd +} + +func TestCheckCLIVersion(t *testing.T) { + result := checkCLIVersion() + assert.Equal(t, "CLI Version", result.Name) + assert.Equal(t, statusInfo, result.Status) + assert.NotEmpty(t, result.Message) +} + +func TestCheckConfigFilePass(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + ctx = profile.WithProfiler(ctx, &mockProfiler{ + path: "/home/user/.databrickscfg", + profiles: profile.Profiles{ + {Name: "default", Host: "https://example.com"}, + {Name: "staging", Host: "https://staging.example.com"}, + }, + }) + cmd := newTestCmd(ctx) + + result := checkConfigFile(cmd) + assert.Equal(t, "Config File", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "2 profiles") + assert.Contains(t, result.Message, "/home/user/.databrickscfg") +} + +func TestCheckConfigFileMissing(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + ctx = profile.WithProfiler(ctx, &mockProfiler{ + err: profile.ErrNoConfiguration, + }) + cmd := newTestCmd(ctx) + + result := checkConfigFile(cmd) + assert.Equal(t, "Config File", result.Name) + assert.Equal(t, statusFail, result.Status) +} + +func TestCheckCurrentProfileDefault(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkCurrentProfile(cmd) + assert.Equal(t, "Current Profile", result.Name) + assert.Equal(t, statusInfo, result.Status) + assert.Equal(t, "default", result.Message) +} + +func TestCheckCurrentProfileFromFlag(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + err := cmd.Flag("profile").Value.Set("staging") + require.NoError(t, err) + cmd.Flag("profile").Changed = true + + result := checkCurrentProfile(cmd) + assert.Equal(t, "Current Profile", result.Name) + assert.Equal(t, statusInfo, result.Status) + assert.Equal(t, "staging", result.Message) +} + +func TestCheckNetworkReachable(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + // Create a fake workspace client config to simulate having a host. + // We pass nil for the workspace client and let it fall through to resolving from config. + // Instead, directly test with a real server. + result := checkNetworkWithHost(cmd, srv.URL) + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "reachable") +} + +func TestCheckNetworkNoHost(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkNetworkWithHost(cmd, "") + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusFail, result.Status) + assert.Contains(t, result.Message, "No host configured") +} + +func TestRenderResultsText(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + results := []CheckResult{ + {Name: "Test", Status: statusPass, Message: "all good"}, + {Name: "Another", Status: statusFail, Message: "broken", Detail: "details here"}, + {Name: "Version", Status: statusInfo, Message: "1.0.0"}, + } + + // Should not panic. + renderResults(ctx, results) +} + +func TestRenderResultsJSON(t *testing.T) { + results := []CheckResult{ + {Name: "Test", Status: statusPass, Message: "all good"}, + {Name: "Another", Status: statusFail, Message: "broken", Detail: "details here"}, + } + + buf, err := json.MarshalIndent(results, "", " ") + require.NoError(t, err) + + var parsed []CheckResult + err = json.Unmarshal(buf, &parsed) + require.NoError(t, err) + assert.Len(t, parsed, 2) + assert.Equal(t, "Test", parsed[0].Name) + assert.Equal(t, statusPass, parsed[0].Status) + assert.Equal(t, "broken", parsed[1].Message) + assert.Equal(t, "details here", parsed[1].Detail) +} + +func TestRenderResultsJSONOmitsEmptyDetail(t *testing.T) { + results := []CheckResult{ + {Name: "Test", Status: statusPass, Message: "ok"}, + } + + buf, err := json.Marshal(results) + require.NoError(t, err) + assert.NotContains(t, string(buf), "detail") +} + +func TestAuthFailureBecomesCheckResult(t *testing.T) { + // Unset env vars that might allow auth to succeed. + t.Setenv("DATABRICKS_HOST", "") + t.Setenv("DATABRICKS_TOKEN", "") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "") + t.Setenv("HOME", t.TempDir()) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result, w := checkAuth(cmd) + // Auth should fail since there is no configuration. + assert.Equal(t, "Authentication", result.Name) + assert.Equal(t, statusFail, result.Status) + assert.Nil(t, w) +} + +func TestNewCommandJSON(t *testing.T) { + // Unset env vars that might cause side effects. + t.Setenv("DATABRICKS_HOST", "") + t.Setenv("DATABRICKS_TOKEN", "") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "") + t.Setenv("HOME", t.TempDir()) + + ctx := cmdio.MockDiscard(t.Context()) + ctx = profile.WithProfiler(ctx, &mockProfiler{ + path: "/tmp/.databrickscfg", + profiles: profile.Profiles{ + {Name: "default", Host: "https://example.com"}, + }, + }) + + cmd := New() + cmd.SetContext(ctx) + + // Register the output flag that is normally inherited from the root command. + outputFlag := flags.OutputText + cmd.PersistentFlags().VarP(&outputFlag, "output", "o", "output type: text or json") + + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetArgs([]string{"--output", "json"}) + + err := cmd.Execute() + require.NoError(t, err) + + var results []CheckResult + err = json.Unmarshal(buf.Bytes(), &results) + require.NoError(t, err) + assert.GreaterOrEqual(t, len(results), 4) + + // First check should be CLI Version. + assert.Equal(t, "CLI Version", results[0].Name) + assert.Equal(t, statusInfo, results[0].Status) +} From 1f6e987417daa3fe5397201983b46552ec92f523 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 04:00:06 +0100 Subject: [PATCH 2/9] Fix review findings: config resolution, timeouts, output routing, tests Co-authored-by: Isaac --- cmd/doctor/checks.go | 43 ++++++- cmd/doctor/doctor.go | 26 ++--- cmd/doctor/doctor_test.go | 233 ++++++++++++++++++++++++++++++++------ 3 files changed, 250 insertions(+), 52 deletions(-) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index 20961b7bc3..29556ed9b2 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -1,11 +1,15 @@ package doctor import ( + "errors" "fmt" + "io" "net/http" + "time" "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -14,7 +18,10 @@ import ( const ( statusPass = "pass" statusFail = "fail" + statusWarn = "warn" statusInfo = "info" + + networkTimeout = 10 * time.Second ) // runChecks runs all diagnostic checks and returns the results. @@ -67,6 +74,14 @@ func checkConfigFile(cmd *cobra.Command) CheckResult { profiles, err := profiler.LoadProfiles(ctx, profile.MatchAllProfiles) if err != nil { + // Config file absence is not a hard failure since auth can work via env vars. + if errors.Is(err, profile.ErrNoConfiguration) { + return CheckResult{ + Name: "Config File", + Status: statusWarn, + Message: "No config file found (auth can still work via environment variables)", + } + } return CheckResult{ Name: "Config File", Status: statusFail, @@ -83,18 +98,34 @@ func checkConfigFile(cmd *cobra.Command) CheckResult { } func checkCurrentProfile(cmd *cobra.Command) CheckResult { + ctx := cmd.Context() + profileFlag := cmd.Flag("profile") - profileName := "default" if profileFlag != nil && profileFlag.Changed { - profileName = profileFlag.Value.String() + return CheckResult{ + Name: "Current Profile", + Status: statusInfo, + Message: profileFlag.Value.String(), + } } + + if envProfile := env.Get(ctx, "DATABRICKS_CONFIG_PROFILE"); envProfile != "" { + return CheckResult{ + Name: "Current Profile", + Status: statusInfo, + Message: envProfile + " (from DATABRICKS_CONFIG_PROFILE)", + } + } + return CheckResult{ Name: "Current Profile", Status: statusInfo, - Message: profileName, + Message: "none (using environment or defaults)", } } +// checkAuth uses the SDK's standard config resolution to authenticate. +// This respects --profile, --host, environment variables, and config file. func checkAuth(cmd *cobra.Command) (CheckResult, *databricks.WorkspaceClient) { ctx := cmd.Context() cfg := &config.Config{} @@ -198,7 +229,8 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { } } - resp, err := http.DefaultClient.Do(req) + client := &http.Client{Timeout: networkTimeout} + resp, err := client.Do(req) if err != nil { return CheckResult{ Name: "Network", @@ -207,7 +239,8 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { Detail: err.Error(), } } - resp.Body.Close() + defer resp.Body.Close() + io.Copy(io.Discard, resp.Body) return CheckResult{ Name: "Network", diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index 1d8f92274e..91b3ededa7 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -1,12 +1,11 @@ package doctor import ( - "context" "encoding/json" "fmt" + "io" "github.com/databricks/cli/cmd/root" - "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" "github.com/fatih/color" "github.com/spf13/cobra" @@ -23,13 +22,13 @@ type CheckResult struct { // New returns the doctor command. func New() *cobra.Command { cmd := &cobra.Command{ - Use: "doctor", - Args: root.NoArgs, - Short: "Validate your Databricks CLI setup", + Use: "doctor", + Args: root.NoArgs, + Short: "Validate your Databricks CLI setup", + GroupID: "development", } cmd.RunE = func(cmd *cobra.Command, args []string) error { - ctx := cmd.Context() results := runChecks(cmd) switch root.OutputType(cmd) { @@ -38,10 +37,11 @@ func New() *cobra.Command { if err != nil { return err } + buf = append(buf, '\n') _, err = cmd.OutOrStdout().Write(buf) return err case flags.OutputText: - renderResults(ctx, results) + renderResults(cmd.OutOrStdout(), results) return nil default: return fmt.Errorf("unknown output type %s", root.OutputType(cmd)) @@ -51,7 +51,7 @@ func New() *cobra.Command { return cmd } -func renderResults(ctx context.Context, results []CheckResult) { +func renderResults(w io.Writer, results []CheckResult) { green := color.New(color.FgGreen).SprintFunc() red := color.New(color.FgRed).SprintFunc() yellow := color.New(color.FgYellow).SprintFunc() @@ -61,19 +61,19 @@ func renderResults(ctx context.Context, results []CheckResult) { for _, r := range results { var icon string switch r.Status { - case "pass": + case statusPass: icon = green("[ok]") - case "fail": + case statusFail: icon = red("[FAIL]") - case "warn": + case statusWarn: icon = yellow("[warn]") - case "info": + case statusInfo: icon = cyan("[info]") } msg := fmt.Sprintf("%s %s: %s", icon, bold(r.Name), r.Message) if r.Detail != "" { msg += fmt.Sprintf(" (%s)", r.Detail) } - cmdio.LogString(ctx, msg) + fmt.Fprintln(w, msg) } } diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index f561a871bd..27d628f9c7 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -10,7 +10,10 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/databrickscfg/profile" + "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" + "github.com/databricks/databricks-sdk-go" + "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -42,6 +45,19 @@ func (m *mockProfiler) GetPath(_ context.Context) (string, error) { return m.path, nil } +// noConfigProfiler returns a path but ErrNoConfiguration from LoadProfiles. +type noConfigProfiler struct { + path string +} + +func (m *noConfigProfiler) LoadProfiles(_ context.Context, _ profile.ProfileMatchFunction) (profile.Profiles, error) { + return nil, profile.ErrNoConfiguration +} + +func (m *noConfigProfiler) GetPath(_ context.Context) (string, error) { + return m.path, nil +} + func newTestCmd(ctx context.Context) *cobra.Command { cmd := &cobra.Command{} cmd.SetContext(ctx) @@ -74,18 +90,33 @@ func TestCheckConfigFilePass(t *testing.T) { assert.Contains(t, result.Message, "/home/user/.databrickscfg") } -func TestCheckConfigFileMissing(t *testing.T) { +func TestCheckConfigFileMissingWarn(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) ctx = profile.WithProfiler(ctx, &mockProfiler{ - err: profile.ErrNoConfiguration, + path: "/home/user/.databrickscfg", + err: profile.ErrNoConfiguration, }) cmd := newTestCmd(ctx) result := checkConfigFile(cmd) assert.Equal(t, "Config File", result.Name) + // GetPath returns err first, so this hits the first failure branch. + // To test the warn path, we need GetPath to succeed but LoadProfiles to fail. assert.Equal(t, statusFail, result.Status) } +func TestCheckConfigFileAbsentIsWarn(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + // Profiler that returns a path but fails on LoadProfiles with ErrNoConfiguration. + ctx = profile.WithProfiler(ctx, &noConfigProfiler{path: "/home/user/.databrickscfg"}) + cmd := newTestCmd(ctx) + + result := checkConfigFile(cmd) + assert.Equal(t, "Config File", result.Name) + assert.Equal(t, statusWarn, result.Status) + assert.Contains(t, result.Message, "environment variables") +} + func TestCheckCurrentProfileDefault(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) @@ -93,7 +124,7 @@ func TestCheckCurrentProfileDefault(t *testing.T) { result := checkCurrentProfile(cmd) assert.Equal(t, "Current Profile", result.Name) assert.Equal(t, statusInfo, result.Status) - assert.Equal(t, "default", result.Message) + assert.Equal(t, "none (using environment or defaults)", result.Message) } func TestCheckCurrentProfileFromFlag(t *testing.T) { @@ -109,6 +140,109 @@ func TestCheckCurrentProfileFromFlag(t *testing.T) { assert.Equal(t, "staging", result.Message) } +func TestCheckCurrentProfileFromEnv(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "from-env") + cmd := newTestCmd(ctx) + + result := checkCurrentProfile(cmd) + assert.Equal(t, statusInfo, result.Status) + assert.Equal(t, "from-env (from DATABRICKS_CONFIG_PROFILE)", result.Message) +} + +func TestCheckCurrentProfileFlagOverridesEnv(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "from-env") + cmd := newTestCmd(ctx) + err := cmd.Flag("profile").Value.Set("from-flag") + require.NoError(t, err) + cmd.Flag("profile").Changed = true + + result := checkCurrentProfile(cmd) + assert.Equal(t, "from-flag", result.Message) +} + +func TestCheckAuthSuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_HOST", srv.URL) + ctx = env.Set(ctx, "DATABRICKS_TOKEN", "test-token") + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") + ctx = env.Set(ctx, "HOME", t.TempDir()) + cmd := newTestCmd(ctx) + + result, w := checkAuth(cmd) + assert.Equal(t, "Authentication", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "OK") + assert.NotNil(t, w) +} + +func TestCheckAuthFailure(t *testing.T) { + t.Setenv("DATABRICKS_HOST", "") + t.Setenv("DATABRICKS_TOKEN", "") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "") + t.Setenv("HOME", t.TempDir()) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result, w := checkAuth(cmd) + assert.Equal(t, "Authentication", result.Name) + assert.Equal(t, statusFail, result.Status) + assert.Nil(t, w) +} + +func TestCheckIdentitySuccess(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/2.0/preview/scim/v2/Me" { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"userName": "test@example.com"}`)) + return + } + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ + Host: srv.URL, + Token: "test-token", + })) + require.NoError(t, err) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkIdentity(cmd, w) + assert.Equal(t, "Identity", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Equal(t, "test@example.com", result.Message) +} + +func TestCheckIdentityFailure(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + + w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ + Host: srv.URL, + Token: "bad-token", + })) + require.NoError(t, err) + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkIdentity(cmd, w) + assert.Equal(t, "Identity", result.Name) + assert.Equal(t, statusFail, result.Status) +} + func TestCheckNetworkReachable(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -118,9 +252,6 @@ func TestCheckNetworkReachable(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - // Create a fake workspace client config to simulate having a host. - // We pass nil for the workspace client and let it fall through to resolving from config. - // Instead, directly test with a real server. result := checkNetworkWithHost(cmd, srv.URL) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusPass, result.Status) @@ -137,16 +268,42 @@ func TestCheckNetworkNoHost(t *testing.T) { assert.Contains(t, result.Message, "No host configured") } -func TestRenderResultsText(t *testing.T) { +func TestCheckNetworkWithClient(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ + Host: srv.URL, + Token: "test-token", + })) + require.NoError(t, err) + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkNetwork(cmd, w) + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "reachable") +} + +func TestRenderResultsText(t *testing.T) { results := []CheckResult{ {Name: "Test", Status: statusPass, Message: "all good"}, {Name: "Another", Status: statusFail, Message: "broken", Detail: "details here"}, {Name: "Version", Status: statusInfo, Message: "1.0.0"}, + {Name: "Config", Status: statusWarn, Message: "not found"}, } - // Should not panic. - renderResults(ctx, results) + var buf bytes.Buffer + renderResults(&buf, results) + output := buf.String() + assert.Contains(t, output, "Test") + assert.Contains(t, output, "all good") + assert.Contains(t, output, "broken") + assert.Contains(t, output, "details here") } func TestRenderResultsJSON(t *testing.T) { @@ -178,31 +335,12 @@ func TestRenderResultsJSONOmitsEmptyDetail(t *testing.T) { assert.NotContains(t, string(buf), "detail") } -func TestAuthFailureBecomesCheckResult(t *testing.T) { - // Unset env vars that might allow auth to succeed. - t.Setenv("DATABRICKS_HOST", "") - t.Setenv("DATABRICKS_TOKEN", "") - t.Setenv("DATABRICKS_CONFIG_PROFILE", "") - t.Setenv("HOME", t.TempDir()) - - ctx := cmdio.MockDiscard(t.Context()) - cmd := newTestCmd(ctx) - - result, w := checkAuth(cmd) - // Auth should fail since there is no configuration. - assert.Equal(t, "Authentication", result.Name) - assert.Equal(t, statusFail, result.Status) - assert.Nil(t, w) -} - func TestNewCommandJSON(t *testing.T) { - // Unset env vars that might cause side effects. - t.Setenv("DATABRICKS_HOST", "") - t.Setenv("DATABRICKS_TOKEN", "") - t.Setenv("DATABRICKS_CONFIG_PROFILE", "") - t.Setenv("HOME", t.TempDir()) - ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_HOST", "") + ctx = env.Set(ctx, "DATABRICKS_TOKEN", "") + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") + ctx = env.Set(ctx, "HOME", t.TempDir()) ctx = profile.WithProfiler(ctx, &mockProfiler{ path: "/tmp/.databrickscfg", profiles: profile.Profiles{ @@ -213,7 +351,6 @@ func TestNewCommandJSON(t *testing.T) { cmd := New() cmd.SetContext(ctx) - // Register the output flag that is normally inherited from the root command. outputFlag := flags.OutputText cmd.PersistentFlags().VarP(&outputFlag, "output", "o", "output type: text or json") @@ -229,7 +366,35 @@ func TestNewCommandJSON(t *testing.T) { require.NoError(t, err) assert.GreaterOrEqual(t, len(results), 4) - // First check should be CLI Version. assert.Equal(t, "CLI Version", results[0].Name) assert.Equal(t, statusInfo, results[0].Status) } + +func TestNewCommandJSONTrailingNewline(t *testing.T) { + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_HOST", "") + ctx = env.Set(ctx, "DATABRICKS_TOKEN", "") + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") + ctx = env.Set(ctx, "HOME", t.TempDir()) + ctx = profile.WithProfiler(ctx, &mockProfiler{ + path: "/tmp/.databrickscfg", + profiles: profile.Profiles{ + {Name: "default", Host: "https://example.com"}, + }, + }) + + cmd := New() + cmd.SetContext(ctx) + + outputFlag := flags.OutputText + cmd.PersistentFlags().VarP(&outputFlag, "output", "o", "output type: text or json") + + var buf bytes.Buffer + cmd.SetOut(&buf) + cmd.SetArgs([]string{"--output", "json"}) + + err := cmd.Execute() + require.NoError(t, err) + assert.True(t, buf.Len() > 0) + assert.Equal(t, byte('\n'), buf.Bytes()[buf.Len()-1]) +} From 19ec7dc16a1b49863d2aeff5c2ef896caf9b51ea Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 06:50:01 +0100 Subject: [PATCH 3/9] Fix TestCheckAuthSuccess to work in CI The test used env.Set(ctx, ...) to set DATABRICKS_HOST and DATABRICKS_TOKEN, but checkAuth creates a bare config.Config{} that reads from real environment variables via os.Getenv, not the context-based env layer. Use t.Setenv instead so the SDK can see the values. Co-authored-by: Isaac --- cmd/doctor/doctor_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 27d628f9c7..11fc3c81f7 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -168,11 +168,12 @@ func TestCheckAuthSuccess(t *testing.T) { })) defer srv.Close() + t.Setenv("DATABRICKS_HOST", srv.URL) + t.Setenv("DATABRICKS_TOKEN", "test-token") + t.Setenv("DATABRICKS_CONFIG_PROFILE", "") + t.Setenv("HOME", t.TempDir()) + ctx := cmdio.MockDiscard(t.Context()) - ctx = env.Set(ctx, "DATABRICKS_HOST", srv.URL) - ctx = env.Set(ctx, "DATABRICKS_TOKEN", "test-token") - ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") - ctx = env.Set(ctx, "HOME", t.TempDir()) cmd := newTestCmd(ctx) result, w := checkAuth(cmd) From 64638e8154faaa6f7809246408716e667206ebf5 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 07:27:08 +0100 Subject: [PATCH 4/9] Fix config resolution, error handling, and test isolation --- cmd/doctor/checks.go | 62 +++++++++++++++++++++-------- cmd/doctor/doctor_test.go | 84 +++++++++++++++++++++++++++++++-------- 2 files changed, 112 insertions(+), 34 deletions(-) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index 29556ed9b2..8c2f0e26ac 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "net/http" + "path/filepath" "time" "github.com/databricks/cli/internal/build" @@ -26,13 +27,15 @@ const ( // runChecks runs all diagnostic checks and returns the results. func runChecks(cmd *cobra.Command) []CheckResult { + cfg, err := resolveConfig(cmd) + var results []CheckResult results = append(results, checkCLIVersion()) results = append(results, checkConfigFile(cmd)) results = append(results, checkCurrentProfile(cmd)) - authResult, w := checkAuth(cmd) + authResult, w := checkAuth(cmd, cfg, err) results = append(results, authResult) if w != nil { @@ -45,7 +48,7 @@ func runChecks(cmd *cobra.Command) []CheckResult { }) } - results = append(results, checkNetwork(cmd, w)) + results = append(results, checkNetwork(cmd, cfg, err, w)) return results } @@ -124,17 +127,42 @@ func checkCurrentProfile(cmd *cobra.Command) CheckResult { } } -// checkAuth uses the SDK's standard config resolution to authenticate. -// This respects --profile, --host, environment variables, and config file. -func checkAuth(cmd *cobra.Command) (CheckResult, *databricks.WorkspaceClient) { +func resolveConfig(cmd *cobra.Command) (*config.Config, error) { ctx := cmd.Context() cfg := &config.Config{} + if configFile := env.Get(ctx, "DATABRICKS_CONFIG_FILE"); configFile != "" { + cfg.ConfigFile = configFile + } else if home := env.Get(ctx, env.HomeEnvVar()); home != "" { + cfg.ConfigFile = filepath.Join(home, ".databrickscfg") + } + + cfg.Loaders = []config.Loader{ + env.NewConfigLoader(ctx), + config.ConfigAttributes, + config.ConfigFile, + } + profileFlag := cmd.Flag("profile") if profileFlag != nil && profileFlag.Changed { cfg.Profile = profileFlag.Value.String() } + return cfg, cfg.EnsureResolved() +} + +// checkAuth uses the resolved config to authenticate. +func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckResult, *databricks.WorkspaceClient) { + ctx := cmd.Context() + if resolveErr != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Cannot resolve config", + Detail: resolveErr.Error(), + }, nil + } + w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg)) if err != nil { return CheckResult{ @@ -191,21 +219,21 @@ func checkIdentity(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResul } } -func checkNetwork(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResult { - var host string - if w != nil { - host = w.Config.Host - } else { - cfg := &config.Config{} - profileFlag := cmd.Flag("profile") - if profileFlag != nil && profileFlag.Changed { - cfg.Profile = profileFlag.Value.String() +func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, w *databricks.WorkspaceClient) CheckResult { + if resolveErr != nil { + return CheckResult{ + Name: "Network", + Status: statusFail, + Message: "Cannot resolve config", + Detail: resolveErr.Error(), } - _ = cfg.EnsureResolved() - host = cfg.Host } - return checkNetworkWithHost(cmd, host) + if w != nil { + return checkNetworkWithHost(cmd, w.Config.Host) + } + + return checkNetworkWithHost(cmd, cfg.Host) } func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 11fc3c81f7..c18fc9913c 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -6,6 +6,8 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "os" + "path/filepath" "testing" "github.com/databricks/cli/libs/cmdio" @@ -65,6 +67,18 @@ func newTestCmd(ctx context.Context) *cobra.Command { return cmd } +func clearConfigEnv(t *testing.T) { + t.Helper() + + for _, attr := range config.ConfigAttributes { + for _, key := range attr.EnvVars { + t.Setenv(key, "") + } + } + + t.Setenv(env.HomeEnvVar(), t.TempDir()) +} + func TestCheckCLIVersion(t *testing.T) { result := checkCLIVersion() assert.Equal(t, "CLI Version", result.Name) @@ -118,6 +132,8 @@ func TestCheckConfigFileAbsentIsWarn(t *testing.T) { } func TestCheckCurrentProfileDefault(t *testing.T) { + clearConfigEnv(t) + ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) @@ -168,15 +184,17 @@ func TestCheckAuthSuccess(t *testing.T) { })) defer srv.Close() + clearConfigEnv(t) t.Setenv("DATABRICKS_HOST", srv.URL) t.Setenv("DATABRICKS_TOKEN", "test-token") - t.Setenv("DATABRICKS_CONFIG_PROFILE", "") - t.Setenv("HOME", t.TempDir()) ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result, w := checkAuth(cmd) + cfg, err := resolveConfig(cmd) + require.NoError(t, err) + + result, w := checkAuth(cmd, cfg, err) assert.Equal(t, "Authentication", result.Name) assert.Equal(t, statusPass, result.Status) assert.Contains(t, result.Message, "OK") @@ -184,20 +202,34 @@ func TestCheckAuthSuccess(t *testing.T) { } func TestCheckAuthFailure(t *testing.T) { - t.Setenv("DATABRICKS_HOST", "") - t.Setenv("DATABRICKS_TOKEN", "") - t.Setenv("DATABRICKS_CONFIG_PROFILE", "") - t.Setenv("HOME", t.TempDir()) + clearConfigEnv(t) ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result, w := checkAuth(cmd) + cfg, err := resolveConfig(cmd) + result, w := checkAuth(cmd, cfg, err) assert.Equal(t, "Authentication", result.Name) assert.Equal(t, statusFail, result.Status) assert.Nil(t, w) } +func TestResolveConfigUsesCommandContextEnv(t *testing.T) { + clearConfigEnv(t) + t.Setenv("DATABRICKS_HOST", "https://real.example.com") + t.Setenv("DATABRICKS_TOKEN", "real-token") + + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_HOST", "https://context.example.com") + ctx = env.Set(ctx, "DATABRICKS_TOKEN", "context-token") + cmd := newTestCmd(ctx) + + cfg, err := resolveConfig(cmd) + require.NoError(t, err) + assert.Equal(t, "https://context.example.com", cfg.Host) + assert.Equal(t, "context-token", cfg.Token) +} + func TestCheckIdentitySuccess(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/2.0/preview/scim/v2/Me" { @@ -284,12 +316,34 @@ func TestCheckNetworkWithClient(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkNetwork(cmd, w) + result := checkNetwork(cmd, w.Config, nil, w) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusPass, result.Status) assert.Contains(t, result.Message, "reachable") } +func TestCheckNetworkConfigResolutionFailure(t *testing.T) { + clearConfigEnv(t) + + configFile := filepath.Join(t.TempDir(), ".databrickscfg") + err := os.WriteFile(configFile, []byte("[DEFAULT]\nhost = https://example.com\n"), 0o600) + require.NoError(t, err) + + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_CONFIG_FILE", configFile) + ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "missing") + cmd := newTestCmd(ctx) + + cfg, err := resolveConfig(cmd) + require.Error(t, err) + + result := checkNetwork(cmd, cfg, err, nil) + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusFail, result.Status) + assert.Equal(t, "Cannot resolve config", result.Message) + assert.Contains(t, result.Detail, "missing profile") +} + func TestRenderResultsText(t *testing.T) { results := []CheckResult{ {Name: "Test", Status: statusPass, Message: "all good"}, @@ -337,11 +391,9 @@ func TestRenderResultsJSONOmitsEmptyDetail(t *testing.T) { } func TestNewCommandJSON(t *testing.T) { + clearConfigEnv(t) + ctx := cmdio.MockDiscard(t.Context()) - ctx = env.Set(ctx, "DATABRICKS_HOST", "") - ctx = env.Set(ctx, "DATABRICKS_TOKEN", "") - ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") - ctx = env.Set(ctx, "HOME", t.TempDir()) ctx = profile.WithProfiler(ctx, &mockProfiler{ path: "/tmp/.databrickscfg", profiles: profile.Profiles{ @@ -372,11 +424,9 @@ func TestNewCommandJSON(t *testing.T) { } func TestNewCommandJSONTrailingNewline(t *testing.T) { + clearConfigEnv(t) + ctx := cmdio.MockDiscard(t.Context()) - ctx = env.Set(ctx, "DATABRICKS_HOST", "") - ctx = env.Set(ctx, "DATABRICKS_TOKEN", "") - ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "") - ctx = env.Set(ctx, "HOME", t.TempDir()) ctx = profile.WithProfiler(ctx, &mockProfiler{ path: "/tmp/.databrickscfg", profiles: profile.Profiles{ From 10d3ba9f35359c79e6d28de34f98e112957ce460 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 07:58:46 +0100 Subject: [PATCH 5/9] Fix errcheck lint and add doctor to help golden file --- acceptance/help/output.txt | 1 + cmd/doctor/checks.go | 2 +- cmd/doctor/doctor_test.go | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/acceptance/help/output.txt b/acceptance/help/output.txt index 19b491dd57..73693cc9f9 100644 --- a/acceptance/help/output.txt +++ b/acceptance/help/output.txt @@ -157,6 +157,7 @@ Postgres Developer Tools bundle Databricks Asset Bundles let you express data/AI/analytics projects as code. + doctor Validate your Databricks CLI setup sync Synchronize a local directory to a workspace directory Additional Commands: diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index 8c2f0e26ac..6e6806face 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -268,7 +268,7 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { } } defer resp.Body.Close() - io.Copy(io.Discard, resp.Body) + _, _ = io.Copy(io.Discard, resp.Body) return CheckResult{ Name: "Network", diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index c18fc9913c..d1698567f5 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -234,7 +234,7 @@ func TestCheckIdentitySuccess(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/api/2.0/preview/scim/v2/Me" { w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"userName": "test@example.com"}`)) + _, _ = w.Write([]byte(`{"userName": "test@example.com"}`)) return } w.WriteHeader(http.StatusOK) @@ -446,6 +446,6 @@ func TestNewCommandJSONTrailingNewline(t *testing.T) { err := cmd.Execute() require.NoError(t, err) - assert.True(t, buf.Len() > 0) + assert.Positive(t, buf.Len()) assert.Equal(t, byte('\n'), buf.Bytes()[buf.Len()-1]) } From 039245a9799771f792a7ff07fbab3351ee7c9a0e Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 13:38:33 +0100 Subject: [PATCH 6/9] Use SDK HTTP client for network checks, return error on check failure --- cmd/doctor/checks.go | 44 +++++++++++++++++++++++--- cmd/doctor/doctor.go | 30 ++++++++++++++---- cmd/doctor/doctor_test.go | 65 ++++++++++++++++++++++++++++++++------- 3 files changed, 117 insertions(+), 22 deletions(-) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index 6e6806face..d024546df4 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -1,6 +1,8 @@ package doctor import ( + "context" + "crypto/tls" "errors" "fmt" "io" @@ -11,6 +13,7 @@ import ( "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/databrickscfg/profile" "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/log" "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" @@ -230,14 +233,16 @@ func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, w *d } if w != nil { - return checkNetworkWithHost(cmd, w.Config.Host) + return checkNetworkWithHost(cmd, w.Config.Host, configuredNetworkHTTPClient(w.Config)) } - return checkNetworkWithHost(cmd, cfg.Host) + log.Warnf(cmd.Context(), "workspace client unavailable for network check, falling back to default HTTP client") + return checkNetworkWithHost(cmd, cfg.Host, http.DefaultClient) } -func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { - ctx := cmd.Context() +func checkNetworkWithHost(cmd *cobra.Command, host string, client *http.Client) CheckResult { + ctx, cancel := context.WithTimeout(cmd.Context(), networkTimeout) + defer cancel() if host == "" { return CheckResult{ @@ -257,7 +262,6 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { } } - client := &http.Client{Timeout: networkTimeout} resp, err := client.Do(req) if err != nil { return CheckResult{ @@ -276,3 +280,33 @@ func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult { Message: host + " is reachable", } } + +func configuredNetworkHTTPClient(cfg *config.Config) *http.Client { + return &http.Client{ + Transport: configuredNetworkHTTPTransport(cfg), + } +} + +func configuredNetworkHTTPTransport(cfg *config.Config) http.RoundTripper { + if cfg.HTTPTransport != nil { + return cfg.HTTPTransport + } + + if !cfg.InsecureSkipVerify { + return http.DefaultTransport + } + + transport, ok := http.DefaultTransport.(*http.Transport) + if !ok { + return http.DefaultTransport + } + + clone := transport.Clone() + if clone.TLSClientConfig != nil { + clone.TLSClientConfig = clone.TLSClientConfig.Clone() + } else { + clone.TLSClientConfig = &tls.Config{} + } + clone.TLSClientConfig.InsecureSkipVerify = true + return clone +} diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index 91b3ededa7..f10713b166 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -2,6 +2,7 @@ package doctor import ( "encoding/json" + "errors" "fmt" "io" @@ -22,10 +23,12 @@ type CheckResult struct { // New returns the doctor command. func New() *cobra.Command { cmd := &cobra.Command{ - Use: "doctor", - Args: root.NoArgs, - Short: "Validate your Databricks CLI setup", - GroupID: "development", + Use: "doctor", + Args: root.NoArgs, + Short: "Validate your Databricks CLI setup", + GroupID: "development", + SilenceUsage: true, + SilenceErrors: true, } cmd.RunE = func(cmd *cobra.Command, args []string) error { @@ -39,13 +42,19 @@ func New() *cobra.Command { } buf = append(buf, '\n') _, err = cmd.OutOrStdout().Write(buf) - return err + if err != nil { + return err + } case flags.OutputText: renderResults(cmd.OutOrStdout(), results) - return nil default: return fmt.Errorf("unknown output type %s", root.OutputType(cmd)) } + + if hasFailedChecks(results) { + return errors.New("one or more checks failed") + } + return nil } return cmd @@ -77,3 +86,12 @@ func renderResults(w io.Writer, results []CheckResult) { fmt.Fprintln(w, msg) } } + +func hasFailedChecks(results []CheckResult) bool { + for _, result := range results { + if result.Status == statusFail { + return true + } + } + return false +} diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index d1698567f5..139c48e113 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -60,6 +60,12 @@ func (m *noConfigProfiler) GetPath(_ context.Context) (string, error) { return m.path, nil } +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { + return f(r) +} + func newTestCmd(ctx context.Context) *cobra.Command { cmd := &cobra.Command{} cmd.SetContext(ctx) @@ -285,7 +291,7 @@ func TestCheckNetworkReachable(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkNetworkWithHost(cmd, srv.URL) + result := checkNetworkWithHost(cmd, srv.URL, http.DefaultClient) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusPass, result.Status) assert.Contains(t, result.Message, "reachable") @@ -295,21 +301,26 @@ func TestCheckNetworkNoHost(t *testing.T) { ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkNetworkWithHost(cmd, "") + result := checkNetworkWithHost(cmd, "", http.DefaultClient) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusFail, result.Status) assert.Contains(t, result.Message, "No host configured") } -func TestCheckNetworkWithClient(t *testing.T) { - srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusOK) - })) - defer srv.Close() - +func TestCheckNetworkUsesWorkspaceClientTransport(t *testing.T) { w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ - Host: srv.URL, + Host: "https://example.com", Token: "test-token", + HTTPTransport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + assert.Equal(t, http.MethodHead, r.Method) + assert.Equal(t, "https://example.com", r.URL.String()) + return &http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + Header: make(http.Header), + Request: r, + }, nil + }), })) require.NoError(t, err) @@ -390,6 +401,38 @@ func TestRenderResultsJSONOmitsEmptyDetail(t *testing.T) { assert.NotContains(t, string(buf), "detail") } +func TestHasFailedChecks(t *testing.T) { + tests := []struct { + name string + results []CheckResult + want bool + }{ + { + name: "no failures", + results: []CheckResult{ + {Name: "Test", Status: statusPass}, + {Name: "Info", Status: statusInfo}, + {Name: "Warn", Status: statusWarn}, + }, + want: false, + }, + { + name: "has failure", + results: []CheckResult{ + {Name: "Test", Status: statusPass}, + {Name: "Broken", Status: statusFail}, + }, + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, hasFailedChecks(tt.results)) + }) + } +} + func TestNewCommandJSON(t *testing.T) { clearConfigEnv(t) @@ -412,7 +455,7 @@ func TestNewCommandJSON(t *testing.T) { cmd.SetArgs([]string{"--output", "json"}) err := cmd.Execute() - require.NoError(t, err) + require.ErrorContains(t, err, "one or more checks failed") var results []CheckResult err = json.Unmarshal(buf.Bytes(), &results) @@ -445,7 +488,7 @@ func TestNewCommandJSONTrailingNewline(t *testing.T) { cmd.SetArgs([]string{"--output", "json"}) err := cmd.Execute() - require.NoError(t, err) + require.ErrorContains(t, err, "one or more checks failed") assert.Positive(t, buf.Len()) assert.Equal(t, byte('\n'), buf.Bytes()[buf.Len()-1]) } From 9a05ab35e91c1325f3326ceb0c83600dbae99da5 Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 15:40:43 +0100 Subject: [PATCH 7/9] Use config-based HTTP client in network check fallback path When the workspace client is unavailable but config is resolved, the network check was falling back to http.DefaultClient. This ignores proxy and custom TLS settings from the SDK config, giving misleading results in enterprise environments. Use configuredNetworkHTTPClient(cfg) instead, which respects HTTPTransport and InsecureSkipVerify from the config. --- cmd/doctor/checks.go | 6 ++++-- cmd/doctor/doctor_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index d024546df4..5521ae650d 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -236,8 +236,10 @@ func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, w *d return checkNetworkWithHost(cmd, w.Config.Host, configuredNetworkHTTPClient(w.Config)) } - log.Warnf(cmd.Context(), "workspace client unavailable for network check, falling back to default HTTP client") - return checkNetworkWithHost(cmd, cfg.Host, http.DefaultClient) + // Workspace client unavailable, but we can still build an HTTP client + // from the resolved config to respect proxy and TLS settings. + log.Warnf(cmd.Context(), "workspace client unavailable for network check, using config-based HTTP client") + return checkNetworkWithHost(cmd, cfg.Host, configuredNetworkHTTPClient(cfg)) } func checkNetworkWithHost(cmd *cobra.Command, host string, client *http.Client) CheckResult { diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 139c48e113..1fa73b24c6 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -333,6 +333,33 @@ func TestCheckNetworkUsesWorkspaceClientTransport(t *testing.T) { assert.Contains(t, result.Message, "reachable") } +func TestCheckNetworkFallbackUsesConfigTransport(t *testing.T) { + called := false + cfg := &config.Config{ + Host: "https://example.com", + Token: "test-token", + HTTPTransport: roundTripFunc(func(r *http.Request) (*http.Response, error) { + called = true + assert.Equal(t, http.MethodHead, r.Method) + return &http.Response{ + StatusCode: http.StatusOK, + Body: http.NoBody, + Header: make(http.Header), + Request: r, + }, nil + }), + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkNetwork(cmd, cfg, nil, nil) + assert.True(t, called, "expected config's HTTPTransport to be used") + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "reachable") +} + func TestCheckNetworkConfigResolutionFailure(t *testing.T) { clearConfigEnv(t) From 87353390a10f408ef613381f509a9e164c4ea14d Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 16:06:26 +0100 Subject: [PATCH 8/9] Fix doctor command: account-level auth, per-check timeouts, network fallback, skip status - Detect account-level configs (AccountID + account host) and use NewAccountClient instead of always using NewWorkspaceClient - Add 15s per-check deadline for auth and identity checks to prevent hangs on unresponsive IdP - Network check now tries even when config resolution fails, as long as a host URL is available from partial config resolution - Identity marked as 'skip' (not 'fail') when auth failed or when using account-level profile, avoiding double failures from one root cause - Add skip status rendering in text output --- cmd/doctor/checks.go | 127 ++++++++++++++++++++++++++++---------- cmd/doctor/doctor.go | 2 + cmd/doctor/doctor_test.go | 108 +++++++++++++++++++++++++------- 3 files changed, 179 insertions(+), 58 deletions(-) diff --git a/cmd/doctor/checks.go b/cmd/doctor/checks.go index 5521ae650d..5e08d526cc 100644 --- a/cmd/doctor/checks.go +++ b/cmd/doctor/checks.go @@ -24,8 +24,10 @@ const ( statusFail = "fail" statusWarn = "warn" statusInfo = "info" + statusSkip = "skip" networkTimeout = 10 * time.Second + checkTimeout = 15 * time.Second ) // runChecks runs all diagnostic checks and returns the results. @@ -38,20 +40,20 @@ func runChecks(cmd *cobra.Command) []CheckResult { results = append(results, checkConfigFile(cmd)) results = append(results, checkCurrentProfile(cmd)) - authResult, w := checkAuth(cmd, cfg, err) + authResult, authCfg := checkAuth(cmd, cfg, err) results = append(results, authResult) - if w != nil { - results = append(results, checkIdentity(cmd, w)) + if authCfg != nil { + results = append(results, checkIdentity(cmd, authCfg)) } else { results = append(results, CheckResult{ Name: "Identity", - Status: statusFail, + Status: statusSkip, Message: "Skipped (authentication failed)", }) } - results = append(results, checkNetwork(cmd, cfg, err, w)) + results = append(results, checkNetwork(cmd, cfg, err, authCfg)) return results } @@ -154,9 +156,17 @@ func resolveConfig(cmd *cobra.Command) (*config.Config, error) { return cfg, cfg.EnsureResolved() } +// isAccountLevelConfig returns true if the resolved config targets account-level APIs. +func isAccountLevelConfig(cfg *config.Config) bool { + return cfg.AccountID != "" && cfg.Host != "" && cfg.HostType() == config.AccountHost +} + // checkAuth uses the resolved config to authenticate. -func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckResult, *databricks.WorkspaceClient) { - ctx := cmd.Context() +// On success it returns the authenticated config for use in subsequent checks. +func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckResult, *config.Config) { + ctx, cancel := context.WithTimeout(cmd.Context(), checkTimeout) + defer cancel() + if resolveErr != nil { return CheckResult{ Name: "Authentication", @@ -166,14 +176,31 @@ func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckR }, nil } - w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg)) - if err != nil { - return CheckResult{ - Name: "Authentication", - Status: statusFail, - Message: "Cannot create workspace client", - Detail: err.Error(), - }, nil + // Detect account-level configs and use the appropriate client constructor + // so that account profiles are not incorrectly reported as broken. + var authCfg *config.Config + if isAccountLevelConfig(cfg) { + a, err := databricks.NewAccountClient((*databricks.Config)(cfg)) + if err != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Cannot create account client", + Detail: err.Error(), + }, nil + } + authCfg = a.Config + } else { + w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg)) + if err != nil { + return CheckResult{ + Name: "Authentication", + Status: statusFail, + Message: "Cannot create workspace client", + Detail: err.Error(), + }, nil + } + authCfg = w.Config } req, err := http.NewRequestWithContext(ctx, "", "", nil) @@ -186,7 +213,7 @@ func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckR }, nil } - err = w.Config.Authenticate(req) + err = authCfg.Authenticate(req) if err != nil { return CheckResult{ Name: "Authentication", @@ -196,15 +223,41 @@ func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckR }, nil } + msg := fmt.Sprintf("OK (%s)", authCfg.AuthType) + if isAccountLevelConfig(cfg) { + msg += " [account-level]" + } + return CheckResult{ Name: "Authentication", Status: statusPass, - Message: fmt.Sprintf("OK (%s)", w.Config.AuthType), - }, w + Message: msg, + }, authCfg } -func checkIdentity(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResult { - ctx := cmd.Context() +func checkIdentity(cmd *cobra.Command, authCfg *config.Config) CheckResult { + ctx, cancel := context.WithTimeout(cmd.Context(), checkTimeout) + defer cancel() + + // Account-level configs don't support the /me endpoint for workspace identity. + if authCfg.HostType() == config.AccountHost { + return CheckResult{ + Name: "Identity", + Status: statusSkip, + Message: "Skipped (account-level profile, workspace identity not available)", + } + } + + w, err := databricks.NewWorkspaceClient((*databricks.Config)(authCfg)) + if err != nil { + return CheckResult{ + Name: "Identity", + Status: statusFail, + Message: "Cannot create workspace client", + Detail: err.Error(), + } + } + me, err := w.CurrentUser.Me(ctx) if err != nil { return CheckResult{ @@ -222,24 +275,30 @@ func checkIdentity(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResul } } -func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, w *databricks.WorkspaceClient) CheckResult { - if resolveErr != nil { - return CheckResult{ - Name: "Network", - Status: statusFail, - Message: "Cannot resolve config", - Detail: resolveErr.Error(), - } +func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, authCfg *config.Config) CheckResult { + // Prefer the authenticated config (it has the fully resolved host). + if authCfg != nil { + return checkNetworkWithHost(cmd, authCfg.Host, configuredNetworkHTTPClient(authCfg)) } - if w != nil { - return checkNetworkWithHost(cmd, w.Config.Host, configuredNetworkHTTPClient(w.Config)) + // Auth failed or was skipped. If we still have a host from config resolution + // (even if resolution had other errors), attempt the network check. + if cfg != nil && cfg.Host != "" { + log.Warnf(cmd.Context(), "authenticated client unavailable for network check, using config-based HTTP client") + return checkNetworkWithHost(cmd, cfg.Host, configuredNetworkHTTPClient(cfg)) } - // Workspace client unavailable, but we can still build an HTTP client - // from the resolved config to respect proxy and TLS settings. - log.Warnf(cmd.Context(), "workspace client unavailable for network check, using config-based HTTP client") - return checkNetworkWithHost(cmd, cfg.Host, configuredNetworkHTTPClient(cfg)) + // No host available at all. + detail := "no host configured" + if resolveErr != nil { + detail = resolveErr.Error() + } + return CheckResult{ + Name: "Network", + Status: statusFail, + Message: "No host configured", + Detail: detail, + } } func checkNetworkWithHost(cmd *cobra.Command, host string, client *http.Client) CheckResult { diff --git a/cmd/doctor/doctor.go b/cmd/doctor/doctor.go index f10713b166..faadb349f7 100644 --- a/cmd/doctor/doctor.go +++ b/cmd/doctor/doctor.go @@ -78,6 +78,8 @@ func renderResults(w io.Writer, results []CheckResult) { icon = yellow("[warn]") case statusInfo: icon = cyan("[info]") + case statusSkip: + icon = yellow("[skip]") } msg := fmt.Sprintf("%s %s: %s", icon, bold(r.Name), r.Message) if r.Detail != "" { diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index 1fa73b24c6..f7b6dad148 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "os" @@ -14,7 +15,6 @@ import ( "github.com/databricks/cli/libs/databrickscfg/profile" "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/flags" - "github.com/databricks/databricks-sdk-go" "github.com/databricks/databricks-sdk-go/config" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -200,11 +200,11 @@ func TestCheckAuthSuccess(t *testing.T) { cfg, err := resolveConfig(cmd) require.NoError(t, err) - result, w := checkAuth(cmd, cfg, err) + result, authCfg := checkAuth(cmd, cfg, err) assert.Equal(t, "Authentication", result.Name) assert.Equal(t, statusPass, result.Status) assert.Contains(t, result.Message, "OK") - assert.NotNil(t, w) + assert.NotNil(t, authCfg) } func TestCheckAuthFailure(t *testing.T) { @@ -214,10 +214,34 @@ func TestCheckAuthFailure(t *testing.T) { cmd := newTestCmd(ctx) cfg, err := resolveConfig(cmd) - result, w := checkAuth(cmd, cfg, err) + result, authCfg := checkAuth(cmd, cfg, err) assert.Equal(t, "Authentication", result.Name) assert.Equal(t, statusFail, result.Status) - assert.Nil(t, w) + assert.Nil(t, authCfg) +} + +func TestCheckAuthAccountLevel(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + clearConfigEnv(t) + t.Setenv("DATABRICKS_HOST", "https://accounts.cloud.databricks.com") + t.Setenv("DATABRICKS_ACCOUNT_ID", "test-account-123") + t.Setenv("DATABRICKS_TOKEN", "test-token") + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + cfg, err := resolveConfig(cmd) + require.NoError(t, err) + + result, authCfg := checkAuth(cmd, cfg, err) + assert.Equal(t, "Authentication", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "account-level") + assert.NotNil(t, authCfg) } func TestResolveConfigUsesCommandContextEnv(t *testing.T) { @@ -247,16 +271,15 @@ func TestCheckIdentitySuccess(t *testing.T) { })) defer srv.Close() - w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ + cfg := &config.Config{ Host: srv.URL, Token: "test-token", - })) - require.NoError(t, err) + } ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkIdentity(cmd, w) + result := checkIdentity(cmd, cfg) assert.Equal(t, "Identity", result.Name) assert.Equal(t, statusPass, result.Status) assert.Equal(t, "test@example.com", result.Message) @@ -268,20 +291,35 @@ func TestCheckIdentityFailure(t *testing.T) { })) defer srv.Close() - w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ + cfg := &config.Config{ Host: srv.URL, Token: "bad-token", - })) - require.NoError(t, err) + } ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkIdentity(cmd, w) + result := checkIdentity(cmd, cfg) assert.Equal(t, "Identity", result.Name) assert.Equal(t, statusFail, result.Status) } +func TestCheckIdentitySkippedForAccountLevel(t *testing.T) { + cfg := &config.Config{ + Host: "https://accounts.cloud.databricks.com", + AccountID: "test-account-123", + Token: "test-token", + } + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkIdentity(cmd, cfg) + assert.Equal(t, "Identity", result.Name) + assert.Equal(t, statusSkip, result.Status) + assert.Contains(t, result.Message, "account-level") +} + func TestCheckNetworkReachable(t *testing.T) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) @@ -307,8 +345,8 @@ func TestCheckNetworkNoHost(t *testing.T) { assert.Contains(t, result.Message, "No host configured") } -func TestCheckNetworkUsesWorkspaceClientTransport(t *testing.T) { - w, err := databricks.NewWorkspaceClient((*databricks.Config)(&config.Config{ +func TestCheckNetworkUsesAuthConfigTransport(t *testing.T) { + cfg := &config.Config{ Host: "https://example.com", Token: "test-token", HTTPTransport: roundTripFunc(func(r *http.Request) (*http.Response, error) { @@ -321,13 +359,12 @@ func TestCheckNetworkUsesWorkspaceClientTransport(t *testing.T) { Request: r, }, nil }), - })) - require.NoError(t, err) + } ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx) - result := checkNetwork(cmd, w.Config, nil, w) + result := checkNetwork(cmd, cfg, nil, cfg) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusPass, result.Status) assert.Contains(t, result.Message, "reachable") @@ -360,7 +397,7 @@ func TestCheckNetworkFallbackUsesConfigTransport(t *testing.T) { assert.Contains(t, result.Message, "reachable") } -func TestCheckNetworkConfigResolutionFailure(t *testing.T) { +func TestCheckNetworkConfigResolutionFailureNoHost(t *testing.T) { clearConfigEnv(t) configFile := filepath.Join(t.TempDir(), ".databrickscfg") @@ -372,14 +409,36 @@ func TestCheckNetworkConfigResolutionFailure(t *testing.T) { ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "missing") cmd := newTestCmd(ctx) - cfg, err := resolveConfig(cmd) - require.Error(t, err) + cfg, resolveErr := resolveConfig(cmd) + require.Error(t, resolveErr) - result := checkNetwork(cmd, cfg, err, nil) + // Config resolution failed and host is empty, so network check reports failure. + result := checkNetwork(cmd, cfg, resolveErr, nil) assert.Equal(t, "Network", result.Name) assert.Equal(t, statusFail, result.Status) - assert.Equal(t, "Cannot resolve config", result.Message) - assert.Contains(t, result.Detail, "missing profile") + assert.Equal(t, "No host configured", result.Message) +} + +func TestCheckNetworkConfigResolutionFailureWithHost(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + // Config resolution may fail (e.g. missing credentials) but if the host + // was partially resolved we should still attempt the network check. + cfg := &config.Config{ + Host: srv.URL, + } + resolveErr := fmt.Errorf("validate: missing credentials") + + ctx := cmdio.MockDiscard(t.Context()) + cmd := newTestCmd(ctx) + + result := checkNetwork(cmd, cfg, resolveErr, nil) + assert.Equal(t, "Network", result.Name) + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "reachable") } func TestRenderResultsText(t *testing.T) { @@ -440,6 +499,7 @@ func TestHasFailedChecks(t *testing.T) { {Name: "Test", Status: statusPass}, {Name: "Info", Status: statusInfo}, {Name: "Warn", Status: statusWarn}, + {Name: "Skip", Status: statusSkip}, }, want: false, }, From f4069082600772a39795ba72fdf01ca91629ae1d Mon Sep 17 00:00:00 2001 From: simon Date: Fri, 13 Mar 2026 17:11:01 +0100 Subject: [PATCH 9/9] Fix lint: use errors.New per perfsprint linter rule --- cmd/doctor/doctor_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/doctor/doctor_test.go b/cmd/doctor/doctor_test.go index f7b6dad148..dbe899bb43 100644 --- a/cmd/doctor/doctor_test.go +++ b/cmd/doctor/doctor_test.go @@ -4,7 +4,7 @@ import ( "bytes" "context" "encoding/json" - "fmt" + "errors" "net/http" "net/http/httptest" "os" @@ -430,7 +430,7 @@ func TestCheckNetworkConfigResolutionFailureWithHost(t *testing.T) { cfg := &config.Config{ Host: srv.URL, } - resolveErr := fmt.Errorf("validate: missing credentials") + resolveErr := errors.New("validate: missing credentials") ctx := cmdio.MockDiscard(t.Context()) cmd := newTestCmd(ctx)