Skip to content

Commit de0754d

Browse files
simonfaltumsimonfaltum
committed
Fix config resolution, error handling, and test isolation
1 parent a377aca commit de0754d

2 files changed

Lines changed: 112 additions & 34 deletions

File tree

‎cmd/doctor/checks.go‎

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"io"
77
"net/http"
8+
"path/filepath"
89
"time"
910

1011
"github.com/databricks/cli/internal/build"
@@ -26,13 +27,15 @@ const (
2627

2728
// runChecks runs all diagnostic checks and returns the results.
2829
func runChecks(cmd *cobra.Command) []CheckResult {
30+
cfg, err := resolveConfig(cmd)
31+
2932
var results []CheckResult
3033

3134
results = append(results, checkCLIVersion())
3235
results = append(results, checkConfigFile(cmd))
3336
results = append(results, checkCurrentProfile(cmd))
3437

35-
authResult, w := checkAuth(cmd)
38+
authResult, w := checkAuth(cmd, cfg, err)
3639
results = append(results, authResult)
3740

3841
if w != nil {
@@ -45,7 +48,7 @@ func runChecks(cmd *cobra.Command) []CheckResult {
4548
})
4649
}
4750

48-
results = append(results, checkNetwork(cmd, w))
51+
results = append(results, checkNetwork(cmd, cfg, err, w))
4952
return results
5053
}
5154

@@ -124,17 +127,42 @@ func checkCurrentProfile(cmd *cobra.Command) CheckResult {
124127
}
125128
}
126129

127-
// checkAuth uses the SDK's standard config resolution to authenticate.
128-
// This respects --profile, --host, environment variables, and config file.
129-
func checkAuth(cmd *cobra.Command) (CheckResult, *databricks.WorkspaceClient) {
130+
func resolveConfig(cmd *cobra.Command) (*config.Config, error) {
130131
ctx := cmd.Context()
131132
cfg := &config.Config{}
132133

134+
if configFile := env.Get(ctx, "DATABRICKS_CONFIG_FILE"); configFile != "" {
135+
cfg.ConfigFile = configFile
136+
} else if home := env.Get(ctx, env.HomeEnvVar()); home != "" {
137+
cfg.ConfigFile = filepath.Join(home, ".databrickscfg")
138+
}
139+
140+
cfg.Loaders = []config.Loader{
141+
env.NewConfigLoader(ctx),
142+
config.ConfigAttributes,
143+
config.ConfigFile,
144+
}
145+
133146
profileFlag := cmd.Flag("profile")
134147
if profileFlag != nil && profileFlag.Changed {
135148
cfg.Profile = profileFlag.Value.String()
136149
}
137150

151+
return cfg, cfg.EnsureResolved()
152+
}
153+
154+
// checkAuth uses the resolved config to authenticate.
155+
func checkAuth(cmd *cobra.Command, cfg *config.Config, resolveErr error) (CheckResult, *databricks.WorkspaceClient) {
156+
ctx := cmd.Context()
157+
if resolveErr != nil {
158+
return CheckResult{
159+
Name: "Authentication",
160+
Status: statusFail,
161+
Message: "Cannot resolve config",
162+
Detail: resolveErr.Error(),
163+
}, nil
164+
}
165+
138166
w, err := databricks.NewWorkspaceClient((*databricks.Config)(cfg))
139167
if err != nil {
140168
return CheckResult{
@@ -191,21 +219,21 @@ func checkIdentity(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResul
191219
}
192220
}
193221

194-
func checkNetwork(cmd *cobra.Command, w *databricks.WorkspaceClient) CheckResult {
195-
var host string
196-
if w != nil {
197-
host = w.Config.Host
198-
} else {
199-
cfg := &config.Config{}
200-
profileFlag := cmd.Flag("profile")
201-
if profileFlag != nil && profileFlag.Changed {
202-
cfg.Profile = profileFlag.Value.String()
222+
func checkNetwork(cmd *cobra.Command, cfg *config.Config, resolveErr error, w *databricks.WorkspaceClient) CheckResult {
223+
if resolveErr != nil {
224+
return CheckResult{
225+
Name: "Network",
226+
Status: statusFail,
227+
Message: "Cannot resolve config",
228+
Detail: resolveErr.Error(),
203229
}
204-
_ = cfg.EnsureResolved()
205-
host = cfg.Host
206230
}
207231

208-
return checkNetworkWithHost(cmd, host)
232+
if w != nil {
233+
return checkNetworkWithHost(cmd, w.Config.Host)
234+
}
235+
236+
return checkNetworkWithHost(cmd, cfg.Host)
209237
}
210238

211239
func checkNetworkWithHost(cmd *cobra.Command, host string) CheckResult {

‎cmd/doctor/doctor_test.go‎

Lines changed: 67 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"encoding/json"
77
"net/http"
88
"net/http/httptest"
9+
"os"
10+
"path/filepath"
911
"testing"
1012

1113
"github.com/databricks/cli/libs/cmdio"
@@ -65,6 +67,18 @@ func newTestCmd(ctx context.Context) *cobra.Command {
6567
return cmd
6668
}
6769

70+
func clearConfigEnv(t *testing.T) {
71+
t.Helper()
72+
73+
for _, attr := range config.ConfigAttributes {
74+
for _, key := range attr.EnvVars {
75+
t.Setenv(key, "")
76+
}
77+
}
78+
79+
t.Setenv(env.HomeEnvVar(), t.TempDir())
80+
}
81+
6882
func TestCheckCLIVersion(t *testing.T) {
6983
result := checkCLIVersion()
7084
assert.Equal(t, "CLI Version", result.Name)
@@ -118,6 +132,8 @@ func TestCheckConfigFileAbsentIsWarn(t *testing.T) {
118132
}
119133

120134
func TestCheckCurrentProfileDefault(t *testing.T) {
135+
clearConfigEnv(t)
136+
121137
ctx := cmdio.MockDiscard(t.Context())
122138
cmd := newTestCmd(ctx)
123139

@@ -168,36 +184,52 @@ func TestCheckAuthSuccess(t *testing.T) {
168184
}))
169185
defer srv.Close()
170186

187+
clearConfigEnv(t)
171188
t.Setenv("DATABRICKS_HOST", srv.URL)
172189
t.Setenv("DATABRICKS_TOKEN", "test-token")
173-
t.Setenv("DATABRICKS_CONFIG_PROFILE", "")
174-
t.Setenv("HOME", t.TempDir())
175190

176191
ctx := cmdio.MockDiscard(t.Context())
177192
cmd := newTestCmd(ctx)
178193

179-
result, w := checkAuth(cmd)
194+
cfg, err := resolveConfig(cmd)
195+
require.NoError(t, err)
196+
197+
result, w := checkAuth(cmd, cfg, err)
180198
assert.Equal(t, "Authentication", result.Name)
181199
assert.Equal(t, statusPass, result.Status)
182200
assert.Contains(t, result.Message, "OK")
183201
assert.NotNil(t, w)
184202
}
185203

186204
func TestCheckAuthFailure(t *testing.T) {
187-
t.Setenv("DATABRICKS_HOST", "")
188-
t.Setenv("DATABRICKS_TOKEN", "")
189-
t.Setenv("DATABRICKS_CONFIG_PROFILE", "")
190-
t.Setenv("HOME", t.TempDir())
205+
clearConfigEnv(t)
191206

192207
ctx := cmdio.MockDiscard(t.Context())
193208
cmd := newTestCmd(ctx)
194209

195-
result, w := checkAuth(cmd)
210+
cfg, err := resolveConfig(cmd)
211+
result, w := checkAuth(cmd, cfg, err)
196212
assert.Equal(t, "Authentication", result.Name)
197213
assert.Equal(t, statusFail, result.Status)
198214
assert.Nil(t, w)
199215
}
200216

217+
func TestResolveConfigUsesCommandContextEnv(t *testing.T) {
218+
clearConfigEnv(t)
219+
t.Setenv("DATABRICKS_HOST", "https://real.example.com")
220+
t.Setenv("DATABRICKS_TOKEN", "real-token")
221+
222+
ctx := cmdio.MockDiscard(t.Context())
223+
ctx = env.Set(ctx, "DATABRICKS_HOST", "https://context.example.com")
224+
ctx = env.Set(ctx, "DATABRICKS_TOKEN", "context-token")
225+
cmd := newTestCmd(ctx)
226+
227+
cfg, err := resolveConfig(cmd)
228+
require.NoError(t, err)
229+
assert.Equal(t, "https://context.example.com", cfg.Host)
230+
assert.Equal(t, "context-token", cfg.Token)
231+
}
232+
201233
func TestCheckIdentitySuccess(t *testing.T) {
202234
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
203235
if r.URL.Path == "/api/2.0/preview/scim/v2/Me" {
@@ -284,12 +316,34 @@ func TestCheckNetworkWithClient(t *testing.T) {
284316
ctx := cmdio.MockDiscard(t.Context())
285317
cmd := newTestCmd(ctx)
286318

287-
result := checkNetwork(cmd, w)
319+
result := checkNetwork(cmd, w.Config, nil, w)
288320
assert.Equal(t, "Network", result.Name)
289321
assert.Equal(t, statusPass, result.Status)
290322
assert.Contains(t, result.Message, "reachable")
291323
}
292324

325+
func TestCheckNetworkConfigResolutionFailure(t *testing.T) {
326+
clearConfigEnv(t)
327+
328+
configFile := filepath.Join(t.TempDir(), ".databrickscfg")
329+
err := os.WriteFile(configFile, []byte("[DEFAULT]\nhost = https://example.com\n"), 0o600)
330+
require.NoError(t, err)
331+
332+
ctx := cmdio.MockDiscard(t.Context())
333+
ctx = env.Set(ctx, "DATABRICKS_CONFIG_FILE", configFile)
334+
ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "missing")
335+
cmd := newTestCmd(ctx)
336+
337+
cfg, err := resolveConfig(cmd)
338+
require.Error(t, err)
339+
340+
result := checkNetwork(cmd, cfg, err, nil)
341+
assert.Equal(t, "Network", result.Name)
342+
assert.Equal(t, statusFail, result.Status)
343+
assert.Equal(t, "Cannot resolve config", result.Message)
344+
assert.Contains(t, result.Detail, "missing profile")
345+
}
346+
293347
func TestRenderResultsText(t *testing.T) {
294348
results := []CheckResult{
295349
{Name: "Test", Status: statusPass, Message: "all good"},
@@ -337,11 +391,9 @@ func TestRenderResultsJSONOmitsEmptyDetail(t *testing.T) {
337391
}
338392

339393
func TestNewCommandJSON(t *testing.T) {
394+
clearConfigEnv(t)
395+
340396
ctx := cmdio.MockDiscard(t.Context())
341-
ctx = env.Set(ctx, "DATABRICKS_HOST", "")
342-
ctx = env.Set(ctx, "DATABRICKS_TOKEN", "")
343-
ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "")
344-
ctx = env.Set(ctx, "HOME", t.TempDir())
345397
ctx = profile.WithProfiler(ctx, &mockProfiler{
346398
path: "/tmp/.databrickscfg",
347399
profiles: profile.Profiles{
@@ -372,11 +424,9 @@ func TestNewCommandJSON(t *testing.T) {
372424
}
373425

374426
func TestNewCommandJSONTrailingNewline(t *testing.T) {
427+
clearConfigEnv(t)
428+
375429
ctx := cmdio.MockDiscard(t.Context())
376-
ctx = env.Set(ctx, "DATABRICKS_HOST", "")
377-
ctx = env.Set(ctx, "DATABRICKS_TOKEN", "")
378-
ctx = env.Set(ctx, "DATABRICKS_CONFIG_PROFILE", "")
379-
ctx = env.Set(ctx, "HOME", t.TempDir())
380430
ctx = profile.WithProfiler(ctx, &mockProfiler{
381431
path: "/tmp/.databrickscfg",
382432
profiles: profile.Profiles{

0 commit comments

Comments
 (0)