Skip to content

Preserve custom profile fields during 'databricks auth login' (#1897) - #2988

Merged
shreyas-goenka merged 21 commits into
databricks:mainfrom
alyssa-db:cluster-id-config-1897
Jun 26, 2025
Merged

Preserve custom profile fields during 'databricks auth login' (#1897)#2988
shreyas-goenka merged 21 commits into
databricks:mainfrom
alyssa-db:cluster-id-config-1897

Conversation

@alyssa-db

@alyssa-dbalyssa-db commented Jun 4, 2025

Copy link
Copy Markdown
Contributor

Changes

  • Fixed an issue where running databricks auth login would remove the cluster_id field from profiles in .databrickscfg. The login process now preserves the cluster_id field.
  • Added a test to ensure cluster_id is retained after login.

Fixes#1897

Tests

Added TestLoginPreservesClusterID, which tests a new example profile named cluster-profile in .databrickscfg to see if the cluster_id is properly fetched from the config file path.

Reproduction Steps

  1. Create a profile with extra fields (e.g., cluster_id) in .databrickscfg.
  2. Run databricks auth login.
  3. Previously, cluster_id would be deleted. Now, it is preserved.

Comment threadcmd/auth/login.go Outdated
Comment threadcmd/auth/login.go Outdated
@andrewnester

Copy link
Copy Markdown
Contributor

@alyssa-db Could you add an entry to CLI section in NEXT_CHANGELOG.md as part of this PR following the format there with a link to your PR?

Comment threadcmd/auth/login_test.go Outdated
Comment threadNEXT_CHANGELOG.md Outdated
Comment threadcmd/auth/login.go Outdated
@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from 089ab25 to f183cf5CompareJune 13, 2025 01:49
@alyssa-db
alyssa-db requested a review from denikJune 13, 2025 02:10
Comment threadcmd/auth/testdata/.databrickscfg Outdated
host = https://accounts.cloud.databricks.com
account_id = id-from-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.

nit: unnecessary extra new line

Comment threadcmd/auth/login_test.go Outdated
Comment threadcmd/auth/login_test.go Outdated
assert.EqualError(t, err, "the command is being run in a non-interactive environment, please specify an account ID using --account-id")
}

func TestLoginPreservesClusterID(t *testing.T) {

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.

Suggested change
funcTestLoginPreservesClusterID(t*testing.T) {
funcTestLoginGetClusterID(t*testing.T) {

Comment threadcmd/auth/login_test.go Outdated
assert.EqualError(t, err, "the command is being run in a non-interactive environment, please specify an account ID using --account-id")
}

func TestLoginPreservesClusterID(t *testing.T) {

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 three tests have similar assertions, can be converted to a single table test. See TestPrependWorkspacePrefix for an example.

Comment threadcmd/auth/login_test.go Outdated
Comment threadcmd/auth/login.go Outdated
}
cfg.ClusterID = clusterID
} else {
cfg.ClusterID, err = getClusterID(ctx, profileName, defaultConfigPath)

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.

style nit: This can be moved above configureCluster instead of having it in the else block. That way you reduce a level of indentation.

Comment threadcmd/auth/login.go Outdated
}
cfg.ClusterID = clusterID
} else {
cfg.ClusterID, err = getClusterID(ctx, profileName, defaultConfigPath)

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.

Please also add a comment here explaining we read the existing cluster-id to preserve it when overwriting the profile.

@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from 7a7a899 to 8ddc65eCompareJune 17, 2025 23:33
Comment threadcmd/auth/login.go Outdated

@shreyas-goenkashreyas-goenka 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.

Thank you!

@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from e11545d to 36425b4CompareJune 19, 2025 01:25
@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from 36425b4 to f366434CompareJune 19, 2025 01:36

@shreyas-goenkashreyas-goenka 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.

Since we do not have test coverage for this command, could you please manually verify that the auth login command works correctly for these cases?

  1. Both profile and the ~/.databrickscfg file exist.
  2. Both profile and a config file pointed to by DATABRICKS_CONFIG_FILE exist.
  3. The profile does not exist.
  4. Multiple profiles exist with the same name.
  5. ~/.databrickscfg does not exist.
  6. DATABRICKS_CONFIG_FILE is set, but no file exists at that path.

Sorry, this is not ideal but this is a pretty important command that we want to ensure does not break. Meanwhile, we are working on patching the Go SDK so that we can make changes with more confidence in the future: databricks/databricks-sdk-go#1233

Comment threadcmd/auth/login.go Outdated
return split[0]
}

func getClusterID(ctx context.Context, profileName string) (string, error) {

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.

Instead of directly reading from the config file, can you modify the LoadProfiles method directly for the DefaultProfiler?

You could add ClusterId to the Profile struct as well to make that work:

type Profile struct {
Name string
Host string
AccountID string
}

That way, this function can simply call LoadProfiles.

@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from f86a9d4 to d906caeCompareJune 21, 2025 00:30
@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from d906cae to 9b8f930CompareJune 21, 2025 09:33
@alyssa-db
alyssa-dbforce-pushed the cluster-id-config-1897 branch from 9b8f930 to b73caddCompareJune 21, 2025 09:41
github-merge-queueBot pushed a commit to databricks/databricks-sdk-go that referenced this pull request Jun 23, 2025
## What changes are proposed in this pull request?
This PR relaxes the validation performed on the host URL to allow `http`
URLs if the URL points to localhost.
This is needed to add acceptance tests in the Databricks CLI for the
`databricks auth login` command. For context: The acceptance test
framework in the CLI has the ability to spawn a testserver on localhost
and record API requests sent to it. We can use the testserver to
validate that the correct API requests are being sent during `auth
login`, and that the command does not fail.
Prompted by
databricks/cli#2988 (comment) where
I almost let a big regression pass.
NO_CHANGELOG=true
## How is this tested?
Unit test

@shreyas-goenkashreyas-goenka 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.

Thank you, looks good to me. The comments are minor.

Comment threadlibs/databrickscfg/profile/profile.go Outdated
Comment threadcmd/auth/login_test.go Outdated

@shreyas-goenkashreyas-goenka 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.

Thanks!

Comment threadcmd/auth/login_test.go Outdated
Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 2988
  • Commit SHA: a53376da126c56f9d4f3935f8428b56ea8b4ad1d

Checks will be approved automatically on success.

@shreyas-goenka

Copy link
Copy Markdown
Contributor

Integration tests are green.

@shreyas-goenka
shreyas-goenka merged commit 0113ee1 into databricks:mainJun 26, 2025
deco-sdk-taggingBot added a commit that referenced this pull request Jul 2, 2025
## Release v0.258.0
### Notable Changes
* Switch default-python template to use pyproject.toml + dynamic\_version in dev target. uv is now required. ([#3042](#3042))
### Dependency updates
* Upgraded TF provider to 1.84.0 ([#3151](#3151))
### CLI
* Fixed an issue where running `databricks auth login` would remove the `cluster_id` field from profiles in `.databrickscfg`. The login process now preserves the `cluster_id` field. ([#2988](#2988))
### Bundles
* Added support for pipeline environment field ([#3153](#3153))
* "bundle summary" now prints diagnostic warnings to stderr ([#3123](#3123))
* "bundle open" will print the URL before opening the browser ([#3168](#3168))
denik pushed a commit that referenced this pull request May 20, 2026
…#2988)
### Changes
- Fixed an issue where running `databricks auth login` would remove the
`cluster_id` field from profiles in `.databrickscfg`. The login process
now preserves the `cluster_id` field.
- Added a test to ensure `cluster_id` is retained after login.
Fixes#1897
## Tests
Added `TestLoginPreservesClusterID`, which tests a new example profile
named `cluster-profile` in `.databrickscfg` to see if the `cluster_id`
is properly fetched from the config file path.
### Reproduction Steps
1. Create a profile with extra fields (e.g., `cluster_id`) in
`.databrickscfg`.
2. Run `databricks auth login`.
3. Previously, `cluster_id` would be deleted. Now, it is preserved.
---------
Co-authored-by: shreyas-goenka <88374338+shreyas-goenka@users.noreply.github.com>
Co-authored-by: Andrew Nester <andrew.nester.dev@gmail.com>
denik pushed a commit that referenced this pull request May 20, 2026
## Release v0.258.0
### Notable Changes
* Switch default-python template to use pyproject.toml + dynamic\_version in dev target. uv is now required. ([#3042](#3042))
### Dependency updates
* Upgraded TF provider to 1.84.0 ([#3151](#3151))
### CLI
* Fixed an issue where running `databricks auth login` would remove the `cluster_id` field from profiles in `.databrickscfg`. The login process now preserves the `cluster_id` field. ([#2988](#2988))
### Bundles
* Added support for pipeline environment field ([#3153](#3153))
* "bundle summary" now prints diagnostic warnings to stderr ([#3123](#3123))
* "bundle open" will print the URL before opening the browser ([#3168](#3168))
Sign up for freeto 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.

Databricks CLI auth login command removes cluster id

4 participants

@alyssa-db@andrewnester@shreyas-goenka@denik