From 56b1b516b4424b0a3b22d35a65de548da406f960 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 10:07:22 +0200 Subject: [PATCH] auth profiles: bound per-profile validation with a 10s timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `databricks auth profiles` validates each profile with a live API call (Workspaces.List or CurrentUser.Me). The SDK retries transient network failures — connection refused, connect/TLS timeout, retriable 5xx — for its default RetryTimeoutSeconds (~5 minutes), so a single unreachable workspace stalls the entire listing. (Hosts that fail DNS are not retriable and already fail fast; those never stalled.) Bound each validation with a 10s context timeout, and set the same value on HTTPTimeoutSeconds/RetryTimeoutSeconds so the host-metadata fetch in EnsureResolved is bounded too — it runs on context.Background internally, so the context.WithTimeout on the validation call cannot reach it. Adds a regression test that points a profile at a server which hangs until the client cancels and asserts Load returns bounded rather than retrying to the SDK default. profileValidationTimeout is a var so the test can shrink it. Co-authored-by: Isaac --- .nextchanges/cli/auth-profiles-timeout.md | 1 + cmd/auth/profiles.go | 25 +++++++++++- cmd/auth/profiles_test.go | 47 +++++++++++++++++++++++ 3 files changed, 71 insertions(+), 2 deletions(-) create mode 100644 .nextchanges/cli/auth-profiles-timeout.md diff --git a/.nextchanges/cli/auth-profiles-timeout.md b/.nextchanges/cli/auth-profiles-timeout.md new file mode 100644 index 00000000000..d521b3aaff5 --- /dev/null +++ b/.nextchanges/cli/auth-profiles-timeout.md @@ -0,0 +1 @@ +* `databricks auth profiles` no longer stalls on an unreachable workspace. Each profile is now validated with a 10s timeout (also applied to the host-metadata fetch in `EnsureResolved`), so a host the SDK would otherwise retry — connection refused, connect/TLS timeout, or a retriable 5xx — can't block the whole listing for the SDK's default ~5-minute retry budget. diff --git a/cmd/auth/profiles.go b/cmd/auth/profiles.go index 8c317293322..d015510ffd4 100644 --- a/cmd/auth/profiles.go +++ b/cmd/auth/profiles.go @@ -21,6 +21,14 @@ import ( "gopkg.in/ini.v1" ) +// profileValidationTimeout bounds each per-profile validation so a host the SDK +// retries — connection refused, connect/TLS timeout, retriable 5xx — cannot +// stall the whole listing. Without it a single such host blocks `auth profiles` +// for the SDK's default retry budget (~5 minutes). Hosts that fail DNS are not +// retriable and already fail fast, so this only bounds the retriable cases. +// A var (not const) so tests can shrink it. +var profileValidationTimeout = 10 * time.Second + type profileMetadata struct { Name string `json:"name"` Host string `json:"host,omitempty"` @@ -37,11 +45,21 @@ func (c *profileMetadata) IsEmpty() bool { } func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipValidate bool) { + timeoutSeconds := int(profileValidationTimeout / time.Second) cfg := &config.Config{ Loaders: []config.Loader{config.ConfigFile}, ConfigFile: configFilePath, Profile: c.Name, DatabricksCliPath: env.Get(ctx, "DATABRICKS_CLI_PATH"), + + // Bound the SDK's per-request and total-retry budgets to the same + // per-profile ceiling. EnsureResolved fetches host metadata via the + // SDK's retrier, which defaults to 5 minutes — and it runs on + // context.Background internally, so the context.WithTimeout below on the + // validation call cannot reach it. Without these a single unreachable + // host stalls the listing well past the validation timeout. + HTTPTimeoutSeconds: timeoutSeconds, + RetryTimeoutSeconds: timeoutSeconds, } if skipValidate { // EnsureResolved fetches /.well-known/databricks-config to enrich @@ -72,13 +90,16 @@ func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipV log.Debugf(ctx, "Profile %q: overrode config type from %s to %s (SPOG host)", c.Name, cfg.ConfigType(), configType) } + callCtx, cancel := context.WithTimeout(ctx, profileValidationTimeout) + defer cancel() + switch configType { case config.AccountConfig: a, err := databricks.NewAccountClient((*databricks.Config)(cfg)) if err != nil { return } - _, err = a.Workspaces.List(ctx) + _, err = a.Workspaces.List(callCtx) c.Host = cfg.Host c.AuthType = cfg.AuthType if err != nil { @@ -90,7 +111,7 @@ func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipV if err != nil { return } - _, err = w.CurrentUser.Me(ctx, iam.MeRequest{}) + _, err = w.CurrentUser.Me(callCtx, iam.MeRequest{}) c.Host = cfg.Host c.AuthType = cfg.AuthType if err != nil { diff --git a/cmd/auth/profiles_test.go b/cmd/auth/profiles_test.go index 2ebbeed0e60..1061b4b6d88 100644 --- a/cmd/auth/profiles_test.go +++ b/cmd/auth/profiles_test.go @@ -9,6 +9,7 @@ import ( "runtime" "sync/atomic" "testing" + "time" "github.com/databricks/cli/libs/databrickscfg" "github.com/databricks/databricks-sdk-go/config" @@ -284,3 +285,49 @@ func TestProfileLoadNoDiscoveryStaysWorkspace(t *testing.T) { assert.NotEmpty(t, p.Host) assert.Equal(t, "pat", p.AuthType) } + +// TestProfileLoadTimesOutOnUnresponsiveHost is a regression test for a hang: +// the SDK retries transient network failures for ~5 minutes by default, so a +// host that accepts the connection but never responds would stall the whole +// `auth profiles` listing. profileValidationTimeout must bound the wait +// (including the EnsureResolved metadata fetch, which runs on context.Background +// and so is only reachable via HTTPTimeoutSeconds/RetryTimeoutSeconds). +// +// The timeout is shrunk to 2s here — kept >=1s because Load derives the SDK's +// integer-second budgets from it, and a sub-second value would floor to 0 +// (which the SDK reads as "use the default", defeating the test). +func TestProfileLoadTimesOutOnUnresponsiveHost(t *testing.T) { + prev := profileValidationTimeout + profileValidationTimeout = 2 * time.Second + t.Cleanup(func() { profileValidationTimeout = prev }) + + // A server that hangs every request until the client gives up. Waiting on + // the request context (rather than a private channel) means the handler + // returns as soon as our timeout cancels the call, so server.Close doesn't + // block on a leaked connection during cleanup. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-r.Context().Done() + })) + t.Cleanup(server.Close) + + dir := t.TempDir() + configFile := filepath.Join(dir, ".databrickscfg") + t.Setenv("HOME", dir) + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", dir) + } + content := "[hung]\nhost = " + server.URL + "\ntoken = test-token\n" + require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600)) + + p := &profileMetadata{Name: "hung", Host: server.URL} + start := time.Now() + p.Load(t.Context(), configFile, false) + elapsed := time.Since(start) + + assert.False(t, p.Valid, "an unresponsive host must not validate") + // Generously above the 2s bound (metadata fetch + validation call, each + // bounded) but far below the SDK's ~5m default that this timeout prevents. + // A regression that drops the bound would blow past this and the whole + // package would then hit the go test -timeout instead. + assert.Less(t, elapsed, 60*time.Second, "Load must be bounded by profileValidationTimeout, not the SDK default") +}