From 56b1b516b4424b0a3b22d35a65de548da406f960 Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Wed, 15 Jul 2026 10:07:22 +0200 Subject: [PATCH 1/2] 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") +} From d202457ede628c1dbd4ddaac7ff541ac6a830f1d Mon Sep 17 00:00:00 2001 From: Jan Rose Date: Tue, 21 Jul 2026 12:20:43 +0200 Subject: [PATCH 2/2] Use 5s timeout; Pass const to function instead of var that we rewrite for tests only --- cmd/auth/profiles.go | 11 +++++------ cmd/auth/profiles_test.go | 18 +++++++----------- 2 files changed, 12 insertions(+), 17 deletions(-) diff --git a/cmd/auth/profiles.go b/cmd/auth/profiles.go index d015510ffd4..e65b11371b7 100644 --- a/cmd/auth/profiles.go +++ b/cmd/auth/profiles.go @@ -26,8 +26,7 @@ import ( // 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 +const profileValidationTimeout = 5 * time.Second type profileMetadata struct { Name string `json:"name"` @@ -44,8 +43,8 @@ func (c *profileMetadata) IsEmpty() bool { return c.Host == "" && c.AccountID == "" } -func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipValidate bool) { - timeoutSeconds := int(profileValidationTimeout / time.Second) +func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipValidate bool, timeout time.Duration) { + timeoutSeconds := int(timeout / time.Second) cfg := &config.Config{ Loaders: []config.Loader{config.ConfigFile}, ConfigFile: configFilePath, @@ -90,7 +89,7 @@ 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) + callCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() switch configType { @@ -169,7 +168,7 @@ func newProfilesCommand() *cobra.Command { wg.Go(func() { ctx := cmd.Context() t := time.Now() - profile.Load(ctx, iniFile.Path(), skipValidate) + profile.Load(ctx, iniFile.Path(), skipValidate, profileValidationTimeout) log.Debugf(ctx, "Profile %q took %s to load", profile.Name, time.Since(t)) }) profiles = append(profiles, profile) diff --git a/cmd/auth/profiles_test.go b/cmd/auth/profiles_test.go index 1061b4b6d88..90efa2a99bd 100644 --- a/cmd/auth/profiles_test.go +++ b/cmd/auth/profiles_test.go @@ -41,7 +41,7 @@ func TestProfiles(t *testing.T) { // Load the profile profile := &profileMetadata{Name: "profile1"} - profile.Load(ctx, configFile, true) + profile.Load(ctx, configFile, true, profileValidationTimeout) // Check the profile assert.Equal(t, "profile1", profile.Name) @@ -72,7 +72,7 @@ func TestProfileLoadSkipValidateMakesNoRequests(t *testing.T) { require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600)) p := &profileMetadata{Name: "offline-profile", Host: server.URL} - p.Load(t.Context(), configFile, true) + p.Load(t.Context(), configFile, true, profileValidationTimeout) assert.Zero(t, requests.Load(), "expected no network calls with skipValidate") assert.Equal(t, server.URL, p.Host) @@ -223,7 +223,7 @@ func TestProfileLoadSPOGConfigType(t *testing.T) { Host: tc.host, AccountID: tc.accountID, } - p.Load(t.Context(), configFile, false) + p.Load(t.Context(), configFile, false, profileValidationTimeout) assert.Equal(t, tc.wantValid, p.Valid, "Valid mismatch") assert.NotEmpty(t, p.Host, "Host should be set") @@ -279,7 +279,7 @@ func TestProfileLoadNoDiscoveryStaysWorkspace(t *testing.T) { Host: server.URL, AccountID: "some-acct", } - p.Load(t.Context(), configFile, false) + p.Load(t.Context(), configFile, false, profileValidationTimeout) assert.True(t, p.Valid, "should validate as workspace when discovery is unavailable") assert.NotEmpty(t, p.Host) @@ -289,18 +289,14 @@ func TestProfileLoadNoDiscoveryStaysWorkspace(t *testing.T) { // 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 +// `auth profiles` listing. The timeout passed to Load 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 +// A 2s timeout is passed 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 @@ -321,7 +317,7 @@ func TestProfileLoadTimesOutOnUnresponsiveHost(t *testing.T) { p := &profileMetadata{Name: "hung", Host: server.URL} start := time.Now() - p.Load(t.Context(), configFile, false) + p.Load(t.Context(), configFile, false, 2*time.Second) elapsed := time.Since(start) assert.False(t, p.Valid, "an unresponsive host must not validate")