Skip to content

Fix CLI token source --profile fallback with version detection - #1605

Merged
mihaimitrea-db merged 7 commits into
mainfrom
mihaimitrea-db/stack/generalize-cli-commands
Apr 23, 2026
Merged

Fix CLI token source --profile fallback with version detection#1605
mihaimitrea-db merged 7 commits into
mainfrom
mihaimitrea-db/stack/generalize-cli-commands

Conversation

@mihaimitrea-db

@mihaimitrea-db mihaimitrea-db commented Mar 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix the broken --profile fallback in CliTokenSource by replacing error-based detection with version-based CLI detection at init time.

Why

The --profile flag on databricks auth token is a global Cobra flag (defined as a persistent flag on the root command). Old CLIs (< v0.207.1) silently accept it — they don't report "unknown flag: --profile" but instead fail later with "cannot fetch credentials". This means the existing isUnknownFlagError check (config/cli_token_source.go:120) never matches, and the --host fallback is dead code.

This was verified by testing against CLI v0.207.0 vs v0.207.1:

  • v0.207.0: databricks auth token --profile workspaceError: init: cannot fetch credentials (not "unknown flag")
  • v0.207.1: databricks auth token --profile workspace → returns a valid token

Approaches considered

Three approaches were evaluated for detecting whether the installed CLI supports --profile:

  • Error-based detection (try-and-retry) — the current approach on main. Run databricks auth token --profile <name> and check whether the error contains "unknown flag: --profile". This is broken: because --profile is a global Cobra flag, old CLIs accept it silently and fail with a different error ("cannot fetch credentials"), so the fallback to --host never triggers.

  • --help flag parsing (databricks auth token --help + substring matching) was rejected because the --help output format is not a stable API. More importantly, --profile would appear in --help output even on old CLIs that don't actually implement profile-based token lookup — it shows up because it's a global persistent flag, not because the auth token subcommand uses it. This approach has the same fundamental flaw as error-based detection.

  • Version detection (databricks version + semver comparison) — the approach taken here. Run databricks version at init time, parse the semver (e.g., "Databricks CLI v0.207.1"), and compare against known minimum versions for each flag. This is reliable because the version string is a stable output format, and the mapping between flags and CLI versions is well-defined (databricks/cli#855 for --profile in v0.207.1). If version detection fails, the SDK falls back to the most conservative command (--host only).

References

What changed

Interface changes

None. CliTokenSource is not part of the public API surface.

NewCliTokenSource now takes context.Context as its first parameter, needed for exec.CommandContext when running databricks version. This is consistent with every CredentialsStrategy.Configure method in the codebase, and the single caller (auth_u2m.go) already has ctx in scope.

Behavioral changes

  • When cfg.Profile is set but the CLI is too old (< v0.207.1), the SDK now correctly falls back to --host. Previously this fallback was dead code.
  • A warning is logged when the --profile flag is not supported and the SDK falls back to --host.

Internal changes

  • cliVersion type: semver parsing with AtLeast() comparison and String() formatting.
  • getCliVersion(ctx, cliPath): runs databricks version and parses the output.
  • parseCliVersion: parses "Databricks CLI v0.207.1"cliVersion{0, 207, 1}.
  • resolveCliCommand: bridges version detection and command building. Falls back to zero version on detection failure.
  • buildCliCommand: pure function — takes a version, returns a single resolved command. No exec calls, easy to test.
  • CliTokenSource simplified to a single cmd []string field. No hostCmd, no runtime fallback.
  • Token() is now one line: return c.execCliCommand(ctx, c.cmd).
  • Removed: isUnknownFlagError, buildCliCommands (plural), buildHostCommand, hostCmd field.

How is this tested?

Manual tests on versions 0.207.0 and 0.207.1

Unit tests in config/cli_token_source_test.go:

  • TestParseCliVersion — standard versions, patch versions, malformed output, empty string, missing prefix.
  • TestCliVersion_AtLeast — equal, higher/lower patch/minor/major, zero vs zero, zero vs nonzero.
  • TestBuildCliCommand — table-driven: version x config → expected command. Covers: host-only, account host, profile+new CLI (uses --profile), profile+old CLI (falls back to --host), profile-only+old CLI (nil), zero version (detection failed, falls back to --host), neither profile nor host (nil).
  • TestNewCliTokenSource — success with host, success with profile, CLI not found, neither profile nor host.
  • TestCliTokenSource_Token — success, CLI error, invalid JSON.

@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (4cb4c26 -> 82d9599)
NEXT_CHANGELOG.md
@@ -6,5 +6,5 @@
  
 + * Generalize CLI token source into a progressive command list for forward-compatible flag support.
   * Normalize internal token sources on `auth.TokenSource` for proper context propagation ([#1577](https://github.com/databricks/databricks-sdk-go/pull/1577)).
-  * Bump golang.org/x/crypto from 0.21.0 to 0.45.0 in /examples/slog ([#1566](https://github.com/databricks/databricks-sdk-go/pull/1566)).
-  * Bump golang.org/x/net from 0.23.0 to 0.33.0 in /examples/slog ([#1127](https://github.com/databricks/databricks-sdk-go/pull/1127)).
\ No newline at end of file
+  * Fix `TestAzureGithubOIDCCredentials` hang caused by missing `HTTPTransport` stub: `EnsureResolved` now calls `resolveHostMetadata`, which makes a real network request when no transport is set ([#1550](https://github.com/databricks/databricks-sdk-go/pull/1550)).
+  * Bump golang.org/x/crypto from 0.21.0 to 0.45.0 in /examples/slog ([#1566](https://github.com/databricks/databricks-sdk-go/pull/1566)).
\ No newline at end of file
config/cli_token_source.go
@@ -18,13 +18,13 @@
 +	flag           string
 +	warningMessage string
 +}
-+
+ 
+-	// profileCmd uses --profile for token lookup. Nil when cfg.Profile is empty.
+-	profileCmd []string
 +var cliFeatureFlags = []cliFeatureFlag{
 +	{"--force-refresh", "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version."},
 +}
- 
--	// profileCmd uses --profile for token lookup. Nil when cfg.Profile is empty.
--	profileCmd []string
++
 +const profileFlagWarning = "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version."
  
 -	// hostCmd uses --host as a fallback for CLIs that predate --profile support.
@@ -83,7 +83,7 @@
 +				warning = cliFeatureFlags[i].warningMessage
 +			}
 +			commands = append(commands, cliCommand{args: args, warningMessage: warning})
-+			}
++		}
  	}
 -	if cfg.Host != "" {
 -		hostCmd = buildHostCommand(cliPath, cfg)

Reproduce locally: git range-diff f626fed..4cb4c26 67f79d8..82d9599 | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from 82d9599 to 68d45f4 Compare March 30, 2026 13:56
@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (82d9599 -> 68d45f4)
config/cli_token_source.go
@@ -18,13 +18,13 @@
 +	flag           string
 +	warningMessage string
 +}
- 
--	// profileCmd uses --profile for token lookup. Nil when cfg.Profile is empty.
--	profileCmd []string
++
 +var cliFeatureFlags = []cliFeatureFlag{
 +	{"--force-refresh", "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version."},
 +}
-+
+ 
+-	// profileCmd uses --profile for token lookup. Nil when cfg.Profile is empty.
+-	profileCmd []string
 +const profileFlagWarning = "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version."
  
 -	// hostCmd uses --host as a fallback for CLIs that predate --profile support.
@@ -143,10 +143,8 @@
 -		logger.Warnf(ctx, "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version.")
 -	}
 -
--	if c.hostCmd != nil {
--		return c.execCliCommand(ctx, c.hostCmd)
--	}
--
- 	return nil, fmt.Errorf("no CLI command configured")
+-	return c.execCliCommand(ctx, c.hostCmd)
++	return nil, fmt.Errorf("cannot get access token: no CLI commands configured")
  }
- 
\ No newline at end of file
+ 
+ func (c *CliTokenSource) execCliCommand(ctx context.Context, args []string) (*oauth2.Token, error) {
\ No newline at end of file

Reproduce locally: git range-diff 67f79d8..82d9599 db4df21..68d45f4 | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from 68d45f4 to 4a5079f Compare March 30, 2026 15:14
@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (68d45f4 -> 4a5079f)
config/cli_token_source.go
@@ -116,6 +116,17 @@
  func (c *CliTokenSource) Token(ctx context.Context) (*oauth2.Token, error) {
 -	if c.forceCmd != nil {
 -		tok, err := c.execCliCommand(ctx, c.forceCmd)
+-		if err == nil {
+-			return tok, nil
+-		}
+-		if !isUnknownFlagError(err, "--force-refresh") && !isUnknownFlagError(err, "--profile") {
+-			return nil, err
+-		}
+-		logger.Warnf(ctx, "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.")
+-	}
+-
+-	if c.profileCmd != nil {
+-		tok, err := c.execCliCommand(ctx, c.profileCmd)
 +	for i := c.activeCommandIndex; i < len(c.commands); i++ {
 +		cmd := c.commands[i]
 +		tok, err := c.execCliCommand(ctx, cmd.args)
@@ -123,26 +134,18 @@
 +			c.activeCommandIndex = i
  			return tok, nil
  		}
--		if !isUnknownFlagError(err, "") {
+-		if !isUnknownFlagError(err, "--profile") {
 +		lastCommand := i == len(c.commands)-1
 +		if lastCommand || !isUnknownFlagError(err, "") {
  			return nil, err
  		}
--		logger.Warnf(ctx, "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.")
-+		logger.Warnf(ctx, cmd.warningMessage)
- 	}
--
--	if c.profileCmd != nil {
--		tok, err := c.execCliCommand(ctx, c.profileCmd)
--		if err == nil {
--			return tok, nil
--		}
--		if !isUnknownFlagError(err, "--profile") {
--			return nil, err
--		}
 -		logger.Warnf(ctx, "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version.")
 -	}
 -
+-	if c.hostCmd == nil {
+-		return nil, fmt.Errorf("cannot get access token: no CLI commands available")
++		logger.Warnf(ctx, cmd.warningMessage)
+ 	}
 -	return c.execCliCommand(ctx, c.hostCmd)
 +	return nil, fmt.Errorf("cannot get access token: no CLI commands configured")
  }
config/cli_token_source_test.go
@@ -76,13 +76,13 @@
 -			gotForceCmd, gotProfileCmd, gotHostCmd := buildCliCommands(cliPath, tc.cfg)
 -			if !slices.Equal(gotForceCmd, tc.wantForceCmd) {
 -				t.Errorf("force cmd = %v, want %v", gotForceCmd, tc.wantForceCmd)
--			}
--			if !slices.Equal(gotProfileCmd, tc.wantProfileCmd) {
--				t.Errorf("profile cmd = %v, want %v", gotProfileCmd, tc.wantProfileCmd)
 +			got := buildCliCommands(cliPath, tc.cfg)
 +			if len(got) != len(tc.wantCommands) {
 +				t.Fatalf("got %d commands, want %d", len(got), len(tc.wantCommands))
  			}
+-			if !slices.Equal(gotProfileCmd, tc.wantProfileCmd) {
+-				t.Errorf("profile cmd = %v, want %v", gotProfileCmd, tc.wantProfileCmd)
+-			}
 -			if !slices.Equal(gotHostCmd, tc.wantHostCmd) {
 -				t.Errorf("host cmd = %v, want %v", gotHostCmd, tc.wantHostCmd)
 +			for i, want := range tc.wantCommands {
@@ -208,15 +208,15 @@
  	_, err := ts.Token(context.Background())
  	if err == nil {
  		t.Fatal("Token() error = nil, want error")
- 		t.Errorf("Token() error = %v, want error containing original auth failure", err)
  	}
  }
-+
+ 
+-func TestCliTokenSource_Token_NilHostCmdReturnsError(t *testing.T) {
 +func TestCliTokenSource_Token_ActiveCommandIndexPersists(t *testing.T) {
-+	if runtime.GOOS == "windows" {
-+		t.Skip("Skipping shell script test on Windows")
-+	}
-+
+ 	if runtime.GOOS == "windows" {
+ 		t.Skip("Skipping shell script test on Windows")
+ 	}
+ 
 +	expiry := time.Now().Add(1 * time.Hour).Format(time.RFC3339)
 +	validResponse, _ := json.Marshal(cliTokenResponse{
 +		AccessToken: "host-token",
@@ -224,18 +224,25 @@
 +		Expiry:      expiry,
 +	})
 +
-+	tempDir := t.TempDir()
-+
-+	forceScript := filepath.Join(tempDir, "force_cli.sh")
+ 	tempDir := t.TempDir()
+ 
+ 	forceScript := filepath.Join(tempDir, "force_cli.sh")
+-	if err := os.WriteFile(forceScript, []byte("#!/bin/sh\necho 'Error: unknown flag: --profile' >&2\nexit 1"), 0755); err != nil {
 +	if err := os.WriteFile(forceScript, []byte("#!/bin/sh\necho 'Error: unknown flag: --force-refresh' >&2\nexit 1"), 0755); err != nil {
-+		t.Fatalf("failed to create force script: %v", err)
-+	}
-+
+ 		t.Fatalf("failed to create force script: %v", err)
+ 	}
+ 
+-	profileScript := filepath.Join(tempDir, "profile_cli.sh")
+-	if err := os.WriteFile(profileScript, []byte("#!/bin/sh\necho 'Error: unknown flag: --profile' >&2\nexit 1"), 0755); err != nil {
+-		t.Fatalf("failed to create profile script: %v", err)
 +	hostScript := filepath.Join(tempDir, "host_cli.sh")
 +	if err := os.WriteFile(hostScript, []byte("#!/bin/sh\necho '"+string(validResponse)+"'"), 0755); err != nil {
 +		t.Fatalf("failed to create host script: %v", err)
-+	}
-+
+ 	}
+ 
+-	ts := &CliTokenSource{
+-		forceCmd:   []string{forceScript},
+-		profileCmd: []string{profileScript},
 +	ts := &CliTokenSource{commands: []cliCommand{
 +		{args: []string{forceScript}, warningMessage: "force-refresh not supported"},
 +		{args: []string{hostScript}},
@@ -245,13 +252,18 @@
 +	token, err := ts.Token(context.Background())
 +	if err != nil {
 +		t.Fatalf("first Token() error = %v", err)
-+	}
+ 	}
+-	_, err := ts.Token(context.Background())
+-	if err == nil {
+-		t.Fatal("Token() error = nil, want error")
 +	if token.AccessToken != "host-token" {
 +		t.Errorf("first AccessToken = %q, want %q", token.AccessToken, "host-token")
 +	}
 +	if ts.activeCommandIndex != 1 {
 +		t.Errorf("activeCommandIndex = %d, want 1", ts.activeCommandIndex)
-+	}
+ 	}
+-	if !strings.Contains(err.Error(), "no CLI commands available") {
+-		t.Errorf("Token() error = %v, want error containing %q", err, "no CLI commands available")
 +
 +	// Second call: starts at activeCommandIndex, skipping the force command.
 +	token, err = ts.Token(context.Background())
@@ -260,5 +272,5 @@
 +	}
 +	if token.AccessToken != "host-token" {
 +		t.Errorf("second AccessToken = %q, want %q", token.AccessToken, "host-token")
-+	}
-+}
\ No newline at end of file
+ 	}
+ }
\ No newline at end of file

Reproduce locally: git range-diff db4df21..68d45f4 97a1007..4a5079f | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from 4a5079f to 6f4fead Compare March 30, 2026 16:27
@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (4a5079f -> 6f4fead)
config/cli_token_source.go
@@ -1,6 +1,13 @@
 diff --git a/config/cli_token_source.go b/config/cli_token_source.go
 --- a/config/cli_token_source.go
 +++ b/config/cli_token_source.go
+ 	"path/filepath"
+ 	"runtime"
+ 	"strings"
++	"sync/atomic"
+ 	"time"
+ 
+ 	"github.com/databricks/databricks-sdk-go/logger"
  	Expiry      string `json:"expiry"`
  }
  
@@ -42,7 +49,7 @@
 +// falling back progressively for older CLI versions that lack newer flags.
 +type CliTokenSource struct {
 +	commands           []cliCommand
-+	activeCommandIndex int
++	activeCommandIndex atomic.Int32
  }
  
  func NewCliTokenSource(cfg *Config) (*CliTokenSource, error) {
@@ -127,11 +134,11 @@
 -
 -	if c.profileCmd != nil {
 -		tok, err := c.execCliCommand(ctx, c.profileCmd)
-+	for i := c.activeCommandIndex; i < len(c.commands); i++ {
++	for i := int(c.activeCommandIndex.Load()); i < len(c.commands); i++ {
 +		cmd := c.commands[i]
 +		tok, err := c.execCliCommand(ctx, cmd.args)
  		if err == nil {
-+			c.activeCommandIndex = i
++			c.activeCommandIndex.Store(int32(i))
  			return tok, nil
  		}
 -		if !isUnknownFlagError(err, "--profile") {
config/cli_token_source_test.go
@@ -259,8 +259,8 @@
 +	if token.AccessToken != "host-token" {
 +		t.Errorf("first AccessToken = %q, want %q", token.AccessToken, "host-token")
 +	}
-+	if ts.activeCommandIndex != 1 {
-+		t.Errorf("activeCommandIndex = %d, want 1", ts.activeCommandIndex)
++	if ts.activeCommandIndex.Load() != 1 {
++		t.Errorf("activeCommandIndex = %d, want 1", ts.activeCommandIndex.Load())
  	}
 -	if !strings.Contains(err.Error(), "no CLI commands available") {
 -		t.Errorf("Token() error = %v, want error containing %q", err, "no CLI commands available")

Reproduce locally: git range-diff 97a1007..4a5079f 69a7c95..6f4fead | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from 6f4fead to 2218270 Compare March 31, 2026 08:07
@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (6f4fead -> 2218270)
config/cli_token_source.go
@@ -11,43 +11,29 @@
  	Expiry      string `json:"expiry"`
  }
  
--// CliTokenSource fetches OAuth tokens by shelling out to the Databricks CLI.
++// cliCommand is a single CLI invocation with an associated warning message
++// that is logged when this command fails and we fall back to the next one.
++type cliCommand struct {
++	args           []string
++	warningMessage string
++}
++
+ // CliTokenSource fetches OAuth tokens by shelling out to the Databricks CLI.
 -// Commands are tried in order: forceCmd -> profileCmd -> hostCmd, progressively
 -// falling back to simpler invocations for older CLI versions.
--type CliTokenSource struct {
++// It holds a list of commands ordered from most feature-rich to simplest,
++// falling back progressively for older CLI versions that lack newer flags.
+ type CliTokenSource struct {
 -	// forceCmd uses --profile with --force-refresh to bypass the CLI's token cache.
 -	// Nil when cfg.Profile is empty (--force-refresh requires --profile support).
 -	forceCmd []string
-+// cliFeatureFlag defines a CLI feature flag and the warning to log when
-+// falling back because the CLI does not support it. Ordered newest-first:
-+// commands are built by progressively stripping these flags for older CLIs.
-+type cliFeatureFlag struct {
-+	flag           string
-+	warningMessage string
-+}
-+
-+var cliFeatureFlags = []cliFeatureFlag{
-+	{"--force-refresh", "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version."},
-+}
- 
+-
 -	// profileCmd uses --profile for token lookup. Nil when cfg.Profile is empty.
 -	profileCmd []string
-+const profileFlagWarning = "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version."
- 
+-
 -	// hostCmd uses --host as a fallback for CLIs that predate --profile support.
 -	// Nil when cfg.Host is empty.
 -	hostCmd []string
-+// cliCommand is a single CLI invocation with an associated warning message
-+// that is logged when this command fails and we fall back to the next one.
-+type cliCommand struct {
-+	args           []string
-+	warningMessage string
-+}
-+
-+// CliTokenSource fetches OAuth tokens by shelling out to the Databricks CLI.
-+// It holds a list of commands ordered from most feature-rich to simplest,
-+// falling back progressively for older CLI versions that lack newer flags.
-+type CliTokenSource struct {
 +	commands           []cliCommand
 +	activeCommandIndex atomic.Int32
  }
@@ -69,48 +55,47 @@
 -func buildCliCommands(cliPath string, cfg *Config) ([]string, []string, []string) {
 -	var forceCmd, profileCmd, hostCmd []string
 +// buildCliCommands constructs the list of CLI commands for fetching an auth
-+// token, ordered from most feature-rich to simplest. When cfg.Profile is set,
-+// commands include feature flags from [cliFeatureFlags] (stripped progressively)
-+// plus a --host fallback when host is available. When cfg.Profile is empty,
-+// only --host is returned — the CLI must support --profile before any feature
-+// flags can be used (monotonic feature assumption).
++// token, ordered from most feature-rich to simplest. The order defines the
++// fallback chain: when a command fails with an unknown flag error, the next
++// one is tried.
 +func buildCliCommands(cliPath string, cfg *Config) []cliCommand {
 +	var commands []cliCommand
  	if cfg.Profile != "" {
 -		profileCmd = []string{cliPath, "auth", "token", "--profile", cfg.Profile}
 -		forceCmd = append(profileCmd, "--force-refresh")
-+		baseArgs := []string{cliPath, "auth", "token", "--profile", cfg.Profile}
-+		for i := 0; i <= len(cliFeatureFlags); i++ {
-+			args := append([]string{}, baseArgs...)
-+			for _, f := range cliFeatureFlags[i:] {
-+				args = append(args, f.flag)
-+			}
-+			warning := profileFlagWarning
-+			if i < len(cliFeatureFlags) {
-+				warning = cliFeatureFlags[i].warningMessage
-+			}
-+			commands = append(commands, cliCommand{args: args, warningMessage: warning})
-+		}
- 	}
+-	}
 -	if cfg.Host != "" {
 -		hostCmd = buildHostCommand(cliPath, cfg)
-+	hostArgs := buildHostCommand(cliPath, cfg)
-+	if hostArgs != nil {
-+		commands = append(commands, cliCommand{args: hostArgs})
- 	}
+-	}
 -	return forceCmd, profileCmd, hostCmd
++		commands = append(commands, cliCommand{
++			args:           []string{cliPath, "auth", "token", "--profile", cfg.Profile, "--force-refresh"},
++			warningMessage: "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.",
++		})
++		commands = append(commands, cliCommand{
++			args:           []string{cliPath, "auth", "token", "--profile", cfg.Profile},
++			warningMessage: "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version.",
++		})
++	}
++	commands = appendHostCommand(commands, cliPath, cfg)
 +	return commands
  }
  
- // buildHostCommand constructs the legacy --host based CLI command.
-+// Returns nil when cfg.Host is empty.
- func buildHostCommand(cliPath string, cfg *Config) []string {
+-// buildHostCommand constructs the legacy --host based CLI command.
+-func buildHostCommand(cliPath string, cfg *Config) []string {
+-	cmd := []string{cliPath, "auth", "token", "--host", cfg.Host}
++func appendHostCommand(commands []cliCommand, cliPath string, cfg *Config) []cliCommand {
 +	if cfg.Host == "" {
-+		return nil
++		return commands
 +	}
- 	cmd := []string{cliPath, "auth", "token", "--host", cfg.Host}
++	args := []string{cliPath, "auth", "token", "--host", cfg.Host}
  	switch cfg.HostType() {
  	case AccountHost:
+-		cmd = append(cmd, "--account-id", cfg.AccountID)
++		args = append(args, "--account-id", cfg.AccountID)
+ 	}
+-	return cmd
++	return append(commands, cliCommand{args: args})
  }
  
  // Token fetches an OAuth token by shelling out to the Databricks CLI.

Reproduce locally: git range-diff 69a7c95..6f4fead 69a7c95..2218270 | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from 2218270 to 76e74ca Compare March 31, 2026 09:06
@mihaimitrea-db

Copy link
Copy Markdown
Contributor Author
Range-diff: stack/force-refresh-flag (2218270 -> 76e74ca)
config/cli_token_source.go
@@ -15,6 +15,7 @@
 +// that is logged when this command fails and we fall back to the next one.
 +type cliCommand struct {
 +	args           []string
++	flags          []string
 +	warningMessage string
 +}
 +
@@ -70,10 +71,12 @@
 -	return forceCmd, profileCmd, hostCmd
 +		commands = append(commands, cliCommand{
 +			args:           []string{cliPath, "auth", "token", "--profile", cfg.Profile, "--force-refresh"},
++			flags:          []string{"--force-refresh", "--profile"},
 +			warningMessage: "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.",
 +		})
 +		commands = append(commands, cliCommand{
 +			args:           []string{cliPath, "auth", "token", "--profile", cfg.Profile},
++			flags:          []string{"--profile"},
 +			warningMessage: "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version.",
 +		})
 +	}
@@ -108,17 +111,6 @@
  func (c *CliTokenSource) Token(ctx context.Context) (*oauth2.Token, error) {
 -	if c.forceCmd != nil {
 -		tok, err := c.execCliCommand(ctx, c.forceCmd)
--		if err == nil {
--			return tok, nil
--		}
--		if !isUnknownFlagError(err, "--force-refresh") && !isUnknownFlagError(err, "--profile") {
--			return nil, err
--		}
--		logger.Warnf(ctx, "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.")
--	}
--
--	if c.profileCmd != nil {
--		tok, err := c.execCliCommand(ctx, c.profileCmd)
 +	for i := int(c.activeCommandIndex.Load()); i < len(c.commands); i++ {
 +		cmd := c.commands[i]
 +		tok, err := c.execCliCommand(ctx, cmd.args)
@@ -126,20 +118,52 @@
 +			c.activeCommandIndex.Store(int32(i))
  			return tok, nil
  		}
--		if !isUnknownFlagError(err, "--profile") {
+-		if !isUnknownFlagError(err, "--force-refresh") && !isUnknownFlagError(err, "--profile") {
 +		lastCommand := i == len(c.commands)-1
-+		if lastCommand || !isUnknownFlagError(err, "") {
++		if lastCommand || !isUnknownFlagError(err, cmd.flags) {
  			return nil, err
  		}
+-		logger.Warnf(ctx, "Databricks CLI does not support --force-refresh flag. The CLI's token cache may provide stale tokens. Please upgrade your CLI to the latest version.")
++		logger.Warnf(ctx, cmd.warningMessage)
+ 	}
+-
+-	if c.profileCmd != nil {
+-		tok, err := c.execCliCommand(ctx, c.profileCmd)
+-		if err == nil {
+-			return tok, nil
+-		}
+-		if !isUnknownFlagError(err, "--profile") {
+-			return nil, err
+-		}
 -		logger.Warnf(ctx, "Databricks CLI does not support --profile flag. Falling back to --host. Please upgrade your CLI to the latest version.")
 -	}
 -
 -	if c.hostCmd == nil {
 -		return nil, fmt.Errorf("cannot get access token: no CLI commands available")
-+		logger.Warnf(ctx, cmd.warningMessage)
- 	}
+-	}
 -	return c.execCliCommand(ctx, c.hostCmd)
 +	return nil, fmt.Errorf("cannot get access token: no CLI commands configured")
  }
  
- func (c *CliTokenSource) execCliCommand(ctx context.Context, args []string) (*oauth2.Token, error) {
\ No newline at end of file
+ func (c *CliTokenSource) execCliCommand(ctx context.Context, args []string) (*oauth2.Token, error) {
+ }
+ 
+ // isUnknownFlagError returns true if the error indicates the CLI does not
+-// recognize a flag. Pass a specific flag (e.g. "--profile") to check for that
+-// flag, or pass "" to match any "unknown flag:" error.
+-func isUnknownFlagError(err error, flag string) bool {
+-	if flag == "" {
+-		return strings.Contains(err.Error(), "unknown flag:")
++// recognize one of the given flags.
++func isUnknownFlagError(err error, flags []string) bool {
++	msg := err.Error()
++	for _, flag := range flags {
++		if strings.Contains(msg, "unknown flag: "+flag) {
++			return true
++		}
+ 	}
+-	return strings.Contains(err.Error(), "unknown flag: "+flag)
++	return false
+ }
+ 
+ // parseExpiry parses an expiry time string in multiple formats for cross-SDK compatibility.
\ No newline at end of file
config/cli_token_source_test.go
@@ -121,7 +121,7 @@
 -		profileCmd: []string{profileScript},
 -	}
 +	ts := &CliTokenSource{commands: []cliCommand{
-+		{args: []string{forceScript}, warningMessage: "force-refresh not supported"},
++		{args: []string{forceScript}, flags: []string{"--force-refresh"}, warningMessage: "force-refresh not supported"},
 +		{args: []string{profileScript}},
 +	}}
  	token, err := ts.Token(context.Background())
@@ -135,7 +135,7 @@
 -		hostCmd:    []string{hostScript},
 -	}
 +	ts := &CliTokenSource{commands: []cliCommand{
-+		{args: []string{profileScript}, warningMessage: "profile not supported"},
++		{args: []string{profileScript}, flags: []string{"--profile"}, warningMessage: "profile not supported"},
 +		{args: []string{hostScript}},
 +	}}
  	token, err := ts.Token(context.Background())
@@ -171,8 +171,8 @@
 -		hostCmd:    []string{hostScript},
 -	}
 +	ts := &CliTokenSource{commands: []cliCommand{
-+		{args: []string{forceScript}, warningMessage: "force-refresh not supported"},
-+		{args: []string{profileScript}, warningMessage: "profile not supported"},
++		{args: []string{forceScript}, flags: []string{"--force-refresh", "--profile"}, warningMessage: "force-refresh not supported"},
++		{args: []string{profileScript}, flags: []string{"--profile"}, warningMessage: "profile not supported"},
 +		{args: []string{hostScript}},
 +	}}
  	token, err := ts.Token(context.Background())
@@ -201,8 +201,8 @@
 -		hostCmd:    []string{hostScript},
 -	}
 +	ts := &CliTokenSource{commands: []cliCommand{
-+		{args: []string{forceScript}, warningMessage: "force-refresh not supported"},
-+		{args: []string{profileScript}, warningMessage: "profile not supported"},
++		{args: []string{forceScript}, flags: []string{"--force-refresh", "--profile"}, warningMessage: "force-refresh not supported"},
++		{args: []string{profileScript}, flags: []string{"--profile"}, warningMessage: "profile not supported"},
 +		{args: []string{hostScript}},
 +	}}
  	_, err := ts.Token(context.Background())
@@ -244,7 +244,7 @@
 -		forceCmd:   []string{forceScript},
 -		profileCmd: []string{profileScript},
 +	ts := &CliTokenSource{commands: []cliCommand{
-+		{args: []string{forceScript}, warningMessage: "force-refresh not supported"},
++		{args: []string{forceScript}, flags: []string{"--force-refresh"}, warningMessage: "force-refresh not supported"},
 +		{args: []string{hostScript}},
 +	}}
 +

Reproduce locally: git range-diff 69a7c95..2218270 69a7c95..76e74ca | Disable: git config gitstack.push-range-diff false

@mihaimitrea-db
mihaimitrea-db marked this pull request as ready for review March 31, 2026 09:53
- Replace custom cliVersion struct with golang.org/x/mod/semver for
  robust version parsing and comparison.
- Use displayVersion helper instead of a String() method on a struct,
  so the empty (unknown) case is handled explicitly.
- Clarify --profile global-flag comment to explain why version
  detection (not runtime probing) is needed.
- Tighten the --profile fallback warning and use len(cmd) == 0 instead
  of comparing against nil.
- Fix the misleading exec.ExitError comment so it describes why we
  prefer stderr over the wrapped error.
- Consolidate TestNewCliTokenSource subtests into a table-driven form
  consistent with the rest of the file.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
Resolves conflict in NEXT_CHANGELOG.md: v0.127.0 and v0.128.0 have
already shipped, so their entries have moved to CHANGELOG.md. Only
the Layer 1 bug-fix entry remains under v0.129.0 > Bug Fixes.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
Dev builds of the Databricks CLI (plain `go build` without the goreleaser
ldflags) report their version as `v0.0.0-dev[+<commit>]`. That sentinel
compares less than every real release in semver, so capability checks like
`--profile` (>= v0.207.1) fail closed and the SDK silently drops back to
the conservative flag set.

This is the safe default — we'd rather under-claim than over-claim
capabilities — but leaves CLI contributors wondering why their locally
built binary "doesn't support" features that clearly exist in main.

Detect the `v0.0.0-dev` prefix in resolveCliCommand and emit an info-level
log pointing to the exact ldflags workaround that matches the CLI's own
goreleaser config. Behavior is unchanged; only observability improves.

Covered by TestNewCliTokenSource_DevBuildInfoLog and two new cases in
TestParseCliVersion.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
Reorganize buildCliCommand so the happy path (use `--profile` on a
supported CLI) sits at the lowest indentation and every branch is
terminal — each `if` ends in a `return` instead of mutating a shared
`cmd` variable.

Extract the --host fallback into buildHostCommand, so the reader of
buildCliCommand doesn't need to understand host construction to follow
the decision tree. buildHostCommand returns nil when cfg.Host is unset,
which naturally propagates the "neither profile nor host usable" case
back up to resolveCliCommand.

Per review feedback on #1605 (r3106582935). No behavior change; all
existing TestBuildCliCommand cases pass unchanged, plus a new
TestBuildHostCommand for the extracted helper.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
The global-flag comment explains why buildCliCommand must use version
detection to decide whether to emit --profile. Placing it at the top of
the function reads as if it applies to the empty-profile guard just
below. Moving it to sit directly above the semver.Compare check (with
surrounding blank lines for breathing room) makes it clear the comment
describes that specific decision.

Pure whitespace/comment move. No behavior change, no test changes.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
Comment thread config/cli_token_source.go Outdated
Comment on lines +41 to +43
// Such builds always compare less than any real release, which would disable
// every feature gate; we surface an informational hint instead.
// See https://github.com/databricks/cli/blob/main/internal/build/info.go.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this part of the comment next to the logic that processes the value.

Comment thread config/cli_token_source.go Outdated
// buildCliCommand constructs the CLI command for fetching an auth token.
// The CLI version determines which flags are used. Returns nil when neither
// profile nor host is usable.
func buildCliCommand(ctx context.Context, cliPath string, cfg *Config, ver string) []string {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's pipe the error so that we can show a precise message (is it the host? is it the profile?). This is also more extendable.

Suggested change
func buildCliCommand(ctx context.Context, cliPath string, cfg *Config, ver string) []string {
func buildCliCommand(ctx context.Context, cliPath string, cfg *Config, ver string) ([]string, error) {

Comment thread config/cli_token_source.go

@renaudhartert-db renaudhartert-db left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo resolution of open comments.

mihaimitrea-db added a commit that referenced this pull request Apr 22, 2026
Three fixes per Renaud's review on #1605:

1. Move the dev-build behavior description from the devBuildVersionPrefix
   const doc to resolveCliCommand where the handling actually happens.
   The const doc now only describes what the sentinel is; the consequence
   of encountering it lives next to the logger.Infof call.

2. Change buildCliCommand to return ([]string, error) so callers get a
   precise reason when no command can be built:
   - "neither profile nor host is configured" when both are empty.
   - "Databricks CLI X does not support --profile (requires >= Y) and no
     host fallback is configured" when profile is set on an old CLI and
     there's nothing to fall back to.
   Also skip the misleading "Falling back to --host" warning when there
   is no host to fall back to. buildHostCommand now assumes its caller
   has verified cfg.Host is non-empty.

3. Guard against empty exitErr.Stderr in execCliCommand so we don't
   render "cannot get access token: : exit status N" with a double
   colon when the CLI dies without writing to stderr (e.g. killed by a
   signal). When stderr is non-empty we still include it in the message
   and wrap exitErr with %w to preserve it for errors.As.

Tests updated: TestBuildCliCommand now covers both new error paths,
TestBuildHostCommand drops its "no host — nil" case (the caller now
guards), and TestCliTokenSource_Token covers the empty-stderr case.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
mihaimitrea-db added a commit that referenced this pull request Apr 22, 2026
Three fixes per Renaud's review on #1605:

1. Move the dev-build behavior description from the devBuildVersionPrefix
   const doc to resolveCliCommand where the handling actually happens.
   The const doc now only describes what the sentinel is; the consequence
   of encountering it lives next to the logger.Infof call.

2. Change buildCliCommand to return ([]string, error) so callers get a
   precise reason when no command can be built:
   - "neither profile nor host is configured" when both are empty.
   - "Databricks CLI X does not support --profile (requires >= Y) and no
     host fallback is configured" when profile is set on an old CLI and
     there's nothing to fall back to.
   Also skip the misleading "Falling back to --host" warning when there
   is no host to fall back to. buildHostCommand now assumes its caller
   has verified cfg.Host is non-empty.

3. Guard against empty exitErr.Stderr in execCliCommand so we don't
   render "cannot get access token: : exit status N" with a double
   colon when the CLI dies without writing to stderr (e.g. killed by a
   signal). When stderr is non-empty we still include it in the message
   and wrap exitErr with %w to preserve it for errors.As.

Tests updated: TestBuildCliCommand now covers both new error paths,
TestBuildHostCommand drops its "no host — nil" case (the caller now
guards), and TestCliTokenSource_Token covers the empty-stderr case.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from a90b93b to a76e875 Compare April 22, 2026 11:23
Three fixes per Renaud's review on #1605:

1. Move the dev-build behavior description from the devBuildVersionPrefix
   const doc to resolveCliCommand where the handling actually happens.
   The const doc now only describes what the sentinel is; the consequence
   of encountering it lives next to the logger.Infof call.

2. Change buildCliCommand to return ([]string, error) so callers get a
   precise reason when no command can be built:
   - "neither profile nor host is configured" when both are empty.
   - "Databricks CLI X does not support --profile (requires >= Y) and no
     host fallback is configured" when profile is set on an old CLI and
     there's nothing to fall back to.
   Also skip the misleading "Falling back to --host" warning when there
   is no host to fall back to. buildHostCommand now assumes its caller
   has verified cfg.Host is non-empty.

3. Guard against empty exitErr.Stderr in execCliCommand so we don't
   render "cannot get access token: : exit status N" with a double
   colon when the CLI dies without writing to stderr (e.g. killed by a
   signal). When stderr is non-empty we still include it in the message
   and wrap exitErr with %w to preserve it for errors.As.

Tests updated: TestBuildCliCommand now covers both new error paths,
TestBuildHostCommand drops its "no host — nil" case (the caller now
guards), and TestCliTokenSource_Token covers the empty-stderr case.

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
@mihaimitrea-db
mihaimitrea-db force-pushed the mihaimitrea-db/stack/generalize-cli-commands branch from a76e875 to 1a52600 Compare April 22, 2026 11:32
@github-actions

Copy link
Copy Markdown

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/sdk-go

Inputs:

  • PR number: 1605
  • Commit SHA: 1a5260054d279add637f3250ae500d3dec9f275d

Checks will be approved automatically on success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants