diff --git a/acceptance/cmd/experimental/doctor/out.test.toml b/acceptance/cmd/experimental/doctor/out.test.toml new file mode 100644 index 00000000000..d560f1de043 --- /dev/null +++ b/acceptance/cmd/experimental/doctor/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/experimental/doctor/output.txt b/acceptance/cmd/experimental/doctor/output.txt new file mode 100644 index 00000000000..5c755fb9c35 --- /dev/null +++ b/acceptance/cmd/experimental/doctor/output.txt @@ -0,0 +1,14 @@ + +=== Doctor with --profile flag + +>>> [CLI] experimental doctor --profile my-workspace --output json +CLI Version: info - [DEV_VERSION] +Updates: info - development build ([DEV_VERSION]) +Toolchain: info - [TOOLCHAIN] +Proxy/TLS: info - no proxy or TLS overrides configured +Log File: info - not configured (set DATABRICKS_LOG_FILE or pass --log-file to enable) +Config File: pass - ~/.databrickscfg (1 profiles) +Current Profile: info - my-workspace +Authentication: pass - OK (pat) +Identity: pass - test@example.com +Network: pass - [DATABRICKS_URL] is reachable diff --git a/acceptance/cmd/experimental/doctor/script b/acceptance/cmd/experimental/doctor/script new file mode 100644 index 00000000000..9a74e060b2f --- /dev/null +++ b/acceptance/cmd/experimental/doctor/script @@ -0,0 +1,10 @@ +sethome "./home" + +cat > "./home/.databrickscfg" < "2.42.0", "Terraform v1.5.0" -> "v1.5.0"). +func firstLineVersion(out string) string { + line := strings.TrimSpace(strings.SplitN(out, "\n", 2)[0]) + fields := strings.Fields(line) + for i, f := range fields { + if looksLikeVersion(f) { + return strings.Join(fields[i:], " ") + } + } + return line +} + +func looksLikeVersion(tok string) bool { + if len(tok) == 0 { + return false + } + c := tok[0] + if c >= '0' && c <= '9' { + return true + } + // Accept a leading "v" only when followed by a digit (e.g. v1.5.0). + if c == 'v' && len(tok) >= 2 && tok[1] >= '0' && tok[1] <= '9' { + return true + } + return false +} + +// proxyEnvVars lists the environment variables that influence HTTP/TLS behavior. +// Both upper- and lower-case forms are checked since Go's http.ProxyFromEnvironment +// honors both. +var proxyEnvVars = []string{ + "HTTPS_PROXY", "https_proxy", + "HTTP_PROXY", "http_proxy", + "NO_PROXY", "no_proxy", + "SSL_CERT_FILE", + "REQUESTS_CA_BUNDLE", + "CURL_CA_BUNDLE", +} + +// checkProxy reports proxy/TLS-related environment variables that affect the CLI's +// network stack. These are a common source of enterprise connectivity issues. +func checkProxy(ctx context.Context) CheckResult { + var seen []string + reported := map[string]bool{} + for _, key := range proxyEnvVars { + v := env.Get(ctx, key) + if v == "" { + continue + } + canonical := strings.ToUpper(key) + if reported[canonical] { + continue + } + reported[canonical] = true + seen = append(seen, canonical+"="+maskProxyValue(v)) + } + if len(seen) == 0 { + return info("Proxy/TLS", "no proxy or TLS overrides configured") + } + return info("Proxy/TLS", strings.Join(seen, ", ")) +} + +// maskProxyValue hides credentials in proxy URLs. Any userinfo (user:pass@host, +// token@host, or just user@host) is replaced with "***" since even a bare +// username can be a secret (e.g. http://TOKEN@proxy). +// net/url would percent-encode "***", so the replacement is done via string rewrite +// on the exact userinfo segment. +func maskProxyValue(v string) string { + u, err := url.Parse(v) + if err != nil || u.User == nil { + return v + } + userinfo := u.User.String() + if userinfo == "" { + return v + } + return strings.Replace(v, userinfo, "***", 1) +} + +// checkUpdates fetches the latest CLI release and compares it to the current build. +// Development builds are reported as info; older builds produce a warn status. +func checkUpdates(ctx context.Context, client *http.Client, releaseURL string) CheckResult { + return checkUpdatesWithVersion(ctx, client, releaseURL, build.GetInfo().Version) +} + +// checkUpdatesWithVersion is the testable core of checkUpdates, parameterized on the current version. +func checkUpdatesWithVersion(ctx context.Context, client *http.Client, releaseURL, current string) CheckResult { + if isDevBuild(current) { + return info("Updates", "development build ("+current+")") + } + + ctx, cancel := context.WithTimeout(ctx, updateCheckTimeout) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, http.MethodGet, releaseURL, nil) + if err != nil { + return warn("Updates", "cannot check for updates: "+err.Error()) + } + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := client.Do(req) + if err != nil { + return warn("Updates", "cannot reach "+releaseURL) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return warn("Updates", "cannot read release response: "+err.Error()) + } + if resp.StatusCode != http.StatusOK { + return warn("Updates", fmt.Sprintf("release lookup returned HTTP %d", resp.StatusCode)) + } + + var release struct { + TagName string `json:"tag_name"` + } + if err := json.Unmarshal(body, &release); err != nil { + return warn("Updates", "cannot parse release response: "+err.Error()) + } + + latest := strings.TrimPrefix(release.TagName, "v") + if latest == "" { + return warn("Updates", "empty release tag") + } + if latest == current { + return pass("Updates", "up to date ("+current+")") + } + return warn("Updates", fmt.Sprintf("newer version available: %s (current %s)", latest, current)) +} + +// isDevBuild reports whether the running binary is a local/dev build. +// The build.GetInfo version for dev builds is "0.0.0-dev+". +func isDevBuild(version string) bool { + return version == "" || strings.HasPrefix(version, "0.0.0-dev") +} + +// checkLogFile surfaces where CLI logs are being written, so users can find them for support. +func checkLogFile(ctx context.Context) CheckResult { + path := env.Get(ctx, "DATABRICKS_LOG_FILE") + if path == "" { + return info("Log File", "not configured (set DATABRICKS_LOG_FILE or pass --log-file to enable)") + } + return info("Log File", path) +} diff --git a/experimental/doctor/cmd/extended_test.go b/experimental/doctor/cmd/extended_test.go new file mode 100644 index 00000000000..fd7bd150994 --- /dev/null +++ b/experimental/doctor/cmd/extended_test.go @@ -0,0 +1,279 @@ +package doctor + +import ( + "context" + "errors" + "io" + "net/http" + "net/http/httptest" + "os/exec" + "strings" + "testing" + + "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/env" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFirstLineVersion(t *testing.T) { + tests := []struct { + in, want string + }{ + {"git version 2.42.0\n", "2.42.0"}, + {"Python 3.11.5", "3.11.5"}, + {"uv 0.1.0 (unknown)", "0.1.0 (unknown)"}, + {"Terraform v1.5.0\non darwin", "v1.5.0"}, + {"", ""}, + {"weirdtool", "weirdtool"}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + assert.Equal(t, tt.want, firstLineVersion(tt.in)) + }) + } +} + +func TestCheckToolchain(t *testing.T) { + mockExec := func(versions map[string]string) execFunc { + return func(_ context.Context, name string, _ ...string) (string, error) { + v, ok := versions[name] + if !ok { + // Wrap in the same error that os/exec returns for a missing binary. + return "", &exec.Error{Name: name, Err: exec.ErrNotFound} + } + return v, nil + } + } + + t.Run("all tools present", func(t *testing.T) { + run := mockExec(map[string]string{ + "git": "git version 2.42.0", + "python3": "Python 3.11.5", + "uv": "uv 0.1.0", + "terraform": "Terraform v1.5.0", + }) + result := checkToolchain(t.Context(), run) + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "git 2.42.0") + assert.Contains(t, result.Message, "python 3.11.5") + assert.Contains(t, result.Message, "terraform v1.5.0") + }) + + t.Run("missing tools reported as not found", func(t *testing.T) { + run := mockExec(map[string]string{"git": "git version 2.42.0"}) + result := checkToolchain(t.Context(), run) + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "git 2.42.0") + assert.Contains(t, result.Message, "terraform not found") + assert.Contains(t, result.Message, "python not found") + }) + + t.Run("non-not-found errors are surfaced verbatim", func(t *testing.T) { + run := func(_ context.Context, name string, _ ...string) (string, error) { + if name == "git" { + return "", errors.New("permission denied") + } + return "", &exec.Error{Name: name, Err: exec.ErrNotFound} + } + result := checkToolchain(t.Context(), run) + assert.Contains(t, result.Message, "git error: permission denied") + assert.Contains(t, result.Message, "python not found") + }) +} + +func TestToolchainErrorLabel(t *testing.T) { + tests := []struct { + name string + err error + want string + }{ + {"not found", &exec.Error{Name: "foo", Err: exec.ErrNotFound}, "not found"}, + {"other error", errors.New("permission denied"), "error: permission denied"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, toolchainErrorLabel(tt.err)) + }) + } +} + +func TestCheckProxy(t *testing.T) { + t.Run("none configured", func(t *testing.T) { + clearConfigEnv(t) + for _, v := range proxyEnvVars { + t.Setenv(v, "") + } + ctx := cmdio.MockDiscard(t.Context()) + result := checkProxy(ctx) + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "no proxy") + }) + + t.Run("reports set env vars", func(t *testing.T) { + for _, v := range proxyEnvVars { + t.Setenv(v, "") + } + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "HTTPS_PROXY", "http://proxy.example.com:8080") + ctx = env.Set(ctx, "NO_PROXY", "localhost,127.0.0.1") + + result := checkProxy(ctx) + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "HTTPS_PROXY=http://proxy.example.com:8080") + assert.Contains(t, result.Message, "NO_PROXY=localhost,127.0.0.1") + }) + + t.Run("masks credentials in proxy URL", func(t *testing.T) { + for _, v := range proxyEnvVars { + t.Setenv(v, "") + } + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "HTTPS_PROXY", "http://user:secret@proxy.example.com:8080") + result := checkProxy(ctx) + assert.Contains(t, result.Message, "***@proxy.example.com") + assert.NotContains(t, result.Message, "secret") + assert.NotContains(t, result.Message, "user") + }) + + t.Run("masks token-only userinfo", func(t *testing.T) { + for _, v := range proxyEnvVars { + t.Setenv(v, "") + } + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "HTTPS_PROXY", "http://glpat-SECRETTOKEN@proxy.example.com:8080") + result := checkProxy(ctx) + assert.Contains(t, result.Message, "***@proxy.example.com") + assert.NotContains(t, result.Message, "glpat-SECRETTOKEN") + }) + + t.Run("deduplicates upper and lower case variants", func(t *testing.T) { + for _, v := range proxyEnvVars { + t.Setenv(v, "") + } + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "HTTPS_PROXY", "http://a.example.com") + ctx = env.Set(ctx, "https_proxy", "http://b.example.com") + + result := checkProxy(ctx) + assert.Equal(t, 1, strings.Count(result.Message, "HTTPS_PROXY=")) + }) +} + +func TestMaskProxyValue(t *testing.T) { + tests := []struct { + in, want string + }{ + {"http://proxy.example.com:8080", "http://proxy.example.com:8080"}, + {"http://user:secret@proxy.example.com:8080", "http://***@proxy.example.com:8080"}, + {"http://tokenonly@proxy.example.com:8080", "http://***@proxy.example.com:8080"}, + {"not a url", "not a url"}, + {"localhost,127.0.0.1", "localhost,127.0.0.1"}, + } + for _, tt := range tests { + t.Run(tt.in, func(t *testing.T) { + assert.Equal(t, tt.want, maskProxyValue(tt.in)) + }) + } +} + +func TestCheckUpdates(t *testing.T) { + t.Run("dev build", func(t *testing.T) { + // current version is "0.0.0-dev+" in tests + result := checkUpdates(t.Context(), http.DefaultClient, "http://unused") + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "development build") + }) + + t.Run("up to date", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, `{"tag_name": "v1.0.0"}`) + })) + defer srv.Close() + + result := checkUpdatesWithVersion(t.Context(), http.DefaultClient, srv.URL, "1.0.0") + assert.Equal(t, statusPass, result.Status) + assert.Contains(t, result.Message, "up to date") + }) + + t.Run("newer available", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, `{"tag_name": "v1.2.3"}`) + })) + defer srv.Close() + + result := checkUpdatesWithVersion(t.Context(), http.DefaultClient, srv.URL, "1.0.0") + assert.Equal(t, statusWarn, result.Status) + assert.Contains(t, result.Message, "1.2.3") + assert.Contains(t, result.Message, "1.0.0") + }) + + t.Run("network failure warns", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + result := checkUpdatesWithVersion(t.Context(), http.DefaultClient, srv.URL, "1.0.0") + assert.Equal(t, statusWarn, result.Status) + assert.Contains(t, result.Message, "HTTP 500") + }) + + t.Run("malformed response warns", func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, `not-json`) + })) + defer srv.Close() + + result := checkUpdatesWithVersion(t.Context(), http.DefaultClient, srv.URL, "1.0.0") + assert.Equal(t, statusWarn, result.Status) + }) +} + +func TestCheckLogFile(t *testing.T) { + t.Run("not configured", func(t *testing.T) { + t.Setenv("DATABRICKS_LOG_FILE", "") + ctx := cmdio.MockDiscard(t.Context()) + result := checkLogFile(ctx) + assert.Equal(t, statusInfo, result.Status) + assert.Contains(t, result.Message, "not configured") + }) + + t.Run("configured path", func(t *testing.T) { + t.Setenv("DATABRICKS_LOG_FILE", "") + ctx := cmdio.MockDiscard(t.Context()) + ctx = env.Set(ctx, "DATABRICKS_LOG_FILE", "/var/log/databricks.log") + result := checkLogFile(ctx) + assert.Equal(t, statusInfo, result.Status) + assert.Equal(t, "/var/log/databricks.log", result.Message) + }) +} + +// Extra sanity: the full command includes all new checks. +func TestRunChecksIncludesExtended(t *testing.T) { + clearConfigEnv(t) + ctx := cmdio.MockDiscard(t.Context()) + results := runChecks(ctx, "", false) + + names := map[string]bool{} + for _, r := range results { + names[r.Name] = true + } + for _, want := range []string{"CLI Version", "Updates", "Toolchain", "Proxy/TLS", "Log File", "Config File", "Current Profile", "Authentication", "Identity", "Network"} { + assert.True(t, names[want], "expected check %q in results", want) + } +} + +// require the test server to return JSON (sanity). +func TestUpdatesServerShape(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.WriteString(w, `{"tag_name": "v0.1.0"}`) + })) + defer srv.Close() + + resp, err := http.Get(srv.URL) + require.NoError(t, err) + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + assert.Contains(t, string(body), "tag_name") +} diff --git a/libs/cmdio/color.go b/libs/cmdio/color.go index ffece82e47e..850a3bc4b68 100644 --- a/libs/cmdio/color.go +++ b/libs/cmdio/color.go @@ -44,6 +44,9 @@ func render(ctx context.Context, code, msg string) string { return code + msg + ansiReset } +// Bold renders msg in bold. +func Bold(ctx context.Context, msg string) string { return render(ctx, ansiBold, msg) } + // Red renders msg in red. func Red(ctx context.Context, msg string) string { return render(ctx, ansiRed, msg) } diff --git a/libs/env/loader.go b/libs/env/loader.go index 74c54cee8fa..89b6bdddc9c 100644 --- a/libs/env/loader.go +++ b/libs/env/loader.go @@ -43,7 +43,7 @@ func (le *configLoader) Configure(cfg *config.Config) error { if v == "" { continue } - if err := a.Set(cfg, v); err != nil { + if err := a.SetS(cfg, v); err != nil { return err } }