Skip to content

Use profile information when getting a token using the CLI#855

Merged
hectorcast-db merged 3 commits into
mainfrom
token-profile-flag
Oct 11, 2023
Merged

Use profile information when getting a token using the CLI#855
hectorcast-db merged 3 commits into
mainfrom
token-profile-flag

Conversation

@hectorcast-db
Copy link
Copy Markdown
Contributor

@hectorcast-db hectorcast-db commented Oct 10, 2023

Changes

Use stored profile information when the user provides the profile flag when using the databricks auth token command.

Tests

Run the command with and without the profile flag

./cli auth token
Databricks Host: https://e2-dogfood.staging.cloud.databricks.com/
{
  "access_token": "****",
  "token_type": "Bearer",
  "expiry": "2023-10-10T14:24:11.85617+02:00"
}%

./cli auth token --profile DEFAULT
{
  "access_token": "*****",
  "token_type": "Bearer",
  "expiry": "2023-10-10T14:24:11.85617+02:00"
}%

./cli auth token https://e2-dogfood.staging.cloud.databricks.com/
{
  "access_token": "*****",
  "token_type": "Bearer",
  "expiry": "2023-10-11T09:24:55.046029+02:00"
}%   

./cli auth token --profile DEFAULT https://e2-dogfood.staging.cloud.databricks.com/
Error: providing both a profile and a host parameters is not supported

@andrewnester
Copy link
Copy Markdown
Contributor

Fixes #746

Comment thread cmd/auth/token.go
cmd.RunE = func(cmd *cobra.Command, args []string) error {
ctx := cmd.Context()

var profileName 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.

What would happen if we run the command with host positional argument and profile flag? Like

databricks auth token https://foo.com --profile bar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It would be ignored. The profile flag is used only to get the host from the config. I can't see any valid use-case of passing both as parameters.

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.

The confusing point here is when there's a mismatch between profile host and host in argument. We either should not allow to provide both at the same time or error out if there's host mismatch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we throw an error if the url is provided on top of the profile?

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.

I'd say we throw an error when host from profile and host from args do not match. @pietern wdyt?

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.

I don't see the value of doing that over simply erroring because both a profile name and a host are specified.

Are there valid cases where you specify both?

If not we should make these mutually exclusive.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We only use the profile to select the host. So currently there is no use case for passing both. I added an error if they are both specified.

Comment thread cmd/auth/token.go
@hectorcast-db hectorcast-db added this pull request to the merge queue Oct 11, 2023
Merged via the queue into main with commit 77101c9 Oct 11, 2023
@hectorcast-db hectorcast-db deleted the token-profile-flag branch October 11, 2023 11:17
andrewnester added a commit that referenced this pull request Oct 11, 2023
CLI:
 * Improve `workspace import` command by allowing references to local files for content ([#793](#793)).
 * Add `--file` flag to workspace export command ([#794](#794)).
 * Ensure profile flag is respected for sync command ([#837](#837)).
 * Add hint to delete sync snapshot if parsing fails ([#853](#853)).
 * Use profile information when getting a token using the CLI ([#855](#855)).

Bundles:
 * Minor template tweaks ([#832](#832)).
 * Fixed using repo files as pipeline libraries ([#847](#847)).
 * Support .gitignore syntax in sync section and make sure it works recursively ([#854](#854)).
 * Allow target overrides for sync section ([#856](#856)).

Internal:
 * Fix import export integration tests on windows ([#842](#842)).
 * Fix workspace import test ([#844](#844)).
 * Automatically create a release PR in homebrew-tap repo ([#841](#841)).

Dependency updates:
 * Bump golang.org/x/term from 0.12.0 to 0.13.0 ([#852](#852)).
 * Bump golang.org/x/mod from 0.12.0 to 0.13.0 ([#851](#851)).
 * Bump golang.org/x/sync from 0.3.0 to 0.4.0 ([#849](#849)).
 * Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0 ([#850](#850)).
@andrewnester andrewnester mentioned this pull request Oct 11, 2023
github-merge-queue Bot pushed a commit that referenced this pull request Oct 11, 2023
CLI:
* Improve `workspace import` command by allowing references to local
files for content ([#793](#793)).
* Add `--file` flag to workspace export command
([#794](#794)).
* Ensure profile flag is respected for sync command
([#837](#837)).
* Add hint to delete sync snapshot if parsing fails
([#853](#853)).
* Use profile information when getting a token using the CLI
([#855](#855)).

Bundles:
* Minor template tweaks
([#832](#832)).
* Fixed using repo files as pipeline libraries
([#847](#847)).
* Support .gitignore syntax in sync section and make sure it works
recursively ([#854](#854)).
* Allow target overrides for sync section
([#856](#856)).

Internal:
* Fix import export integration tests on windows
([#842](#842)).
* Fix workspace import test
([#844](#844)).
* Automatically create a release PR in homebrew-tap repo
([#841](#841)).


Dependency updates:
* Bump golang.org/x/term from 0.12.0 to 0.13.0
([#852](#852)).
* Bump golang.org/x/mod from 0.12.0 to 0.13.0
([#851](#851)).
* Bump golang.org/x/sync from 0.3.0 to 0.4.0
([#849](#849)).
* Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0
([#850](#850)).
hectorcast-db added a commit that referenced this pull request Oct 13, 2023
## Changes
Use stored profile information when the user provides the profile flag
when using the `databricks auth token` command.

## Tests
Run the command with and without the profile flag

```
./cli auth token
Databricks Host: https://e2-dogfood.staging.cloud.databricks.com/
{
  "access_token": "****",
  "token_type": "Bearer",
  "expiry": "2023-10-10T14:24:11.85617+02:00"
}%

./cli auth token --profile DEFAULT
{
  "access_token": "*****",
  "token_type": "Bearer",
  "expiry": "2023-10-10T14:24:11.85617+02:00"
}%

./cli auth token https://e2-dogfood.staging.cloud.databricks.com/
{
  "access_token": "*****",
  "token_type": "Bearer",
  "expiry": "2023-10-11T09:24:55.046029+02:00"
}%   

./cli auth token --profile DEFAULT https://e2-dogfood.staging.cloud.databricks.com/
Error: providing both a profile and a host parameters is not supported
```
hectorcast-db pushed a commit that referenced this pull request Oct 13, 2023
CLI:
* Improve `workspace import` command by allowing references to local
files for content ([#793](#793)).
* Add `--file` flag to workspace export command
([#794](#794)).
* Ensure profile flag is respected for sync command
([#837](#837)).
* Add hint to delete sync snapshot if parsing fails
([#853](#853)).
* Use profile information when getting a token using the CLI
([#855](#855)).

Bundles:
* Minor template tweaks
([#832](#832)).
* Fixed using repo files as pipeline libraries
([#847](#847)).
* Support .gitignore syntax in sync section and make sure it works
recursively ([#854](#854)).
* Allow target overrides for sync section
([#856](#856)).

Internal:
* Fix import export integration tests on windows
([#842](#842)).
* Fix workspace import test
([#844](#844)).
* Automatically create a release PR in homebrew-tap repo
([#841](#841)).


Dependency updates:
* Bump golang.org/x/term from 0.12.0 to 0.13.0
([#852](#852)).
* Bump golang.org/x/mod from 0.12.0 to 0.13.0
([#851](#851)).
* Bump golang.org/x/sync from 0.3.0 to 0.4.0
([#849](#849)).
* Bump golang.org/x/oauth2 from 0.12.0 to 0.13.0
([#850](#850)).
alexott pushed a commit to alexott/databricks-sdk-go that referenced this pull request May 17, 2026
…ricks#1605)

## 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](https://github.com/databricks/cli/blob/77101c9b8567/cmd/root/root.go)
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](https://github.com/databricks/databricks-sdk-go/blob/main/config/cli_token_source.go#L120))
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 workspace` → `Error: 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](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

- `--profile` support added in CLI v0.207.1:
[databricks/cli#855](databricks/cli#855) (Oct
2023)

## 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.

---------

Signed-off-by: Mihai Mitrea <mihai.mitrea@databricks.com>
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.

3 participants