Preserve profile fields on re-login instead of wiping all keys (#4597)

simonfaltum · web-flow · commit d00476d68c27 · 2026-02-27T12:31:11.000Z
## Why `SaveToProfile` deleted **every key** in a `.databrickscfg` profile section before writing, then only wrote back the small subset of fields the caller explicitly set. This meant every `databricks auth login` or `databricks configure` silently destroyed fields like `cluster_id`, `warehouse_id`, `scopes`, `azure_environment`, and any custom keys the user had added to their profile. This is especially problematic as profiles carry more explicit fields in the host-agnostic auth work (`workspace_id`, `account_id`, `azure_environment`). Users shouldn't have to re-specify everything every time they re-login. ## Changes ### Core: `SaveToProfile` merge semantics (`libs/databrickscfg/ops.go`) **Before:** Delete all keys in the section, then write only non-zero fields from the new config. **After:** Existing keys not mentioned in the new config are preserved. Non-zero fields from the new config overwrite existing values. A new `clearKeys ...string` variadic parameter lets callers explicitly remove specific keys. ### `auth login` (`cmd/auth/login.go`) **Before:** Re-login wiped everything. `cluster_id` and `serverless_compute_id` were manually read back from the existing profile in the default case (no `--configure-cluster`/`--configure-serverless`), but `warehouse_id`, `azure_environment`, custom keys, etc. were always lost. **After:** - All non-auth fields are preserved automatically via merge semantics (no manual read-back needed). - Incompatible auth credentials (PAT token, basic auth, M2M client secrets, Azure/GCP credentials, metadata service URL, OIDC tokens) are explicitly cleared when switching to OAuth. - `--configure-cluster` explicitly clears `serverless_compute_id` (and vice versa) for mutual exclusion. - `experimental_is_unified_host` is explicitly cleared when `false` (since `false` is a zero value and would otherwise be skipped by the merge, leaving a stale `true` from a previous login). - **Scopes are preserved:** when `--scopes` is not passed, existing scopes from the profile are read back and used for the OAuth challenge. This means the minted token matches the profile's scope configuration. Previously, scopes were always wiped and the default `all-apis` was used. ### Inline login in `auth token` (`cmd/auth/token.go`) **Before:** `runInlineLogin` (the "create new profile" path in the interactive `auth token` flow) saved a minimal set of fields and wiped everything else. Did not handle scopes or `experimental_is_unified_host` clearing. **After:** - Same auth credential clearing as `auth login`. - `experimental_is_unified_host` explicitly cleared when `false`. - Existing profile scopes are read back and used for the OAuth challenge (same as `auth login`). ### `databricks configure` (`cmd/configure/configure.go`) **Before:** PAT configure wiped all keys including `auth_type`, `scopes`, `azure_environment`, etc. — which was correct for `auth_type`/`scopes` but destroyed useful non-auth fields. **After:** - Non-auth fields (`cluster_id`, `warehouse_id`, `azure_environment`, `account_id`, `workspace_id`, custom keys) are preserved. - OAuth metadata (`auth_type`, `scopes`, `databricks_cli_path`) is explicitly cleared — a PAT profile shouldn't keep OAuth artifacts. - All non-PAT auth credentials (basic auth, M2M, Azure, GCP, OIDC, metadata service) are explicitly cleared to prevent multi-auth conflicts. - `experimental_is_unified_host` is always cleared (PAT profiles don't use unified hosts). - `serverless_compute_id` is cleared when `cluster_id` is set — whether via `--configure-cluster` flag **or** via `DATABRICKS_CLUSTER_ID` env var (previously only the flag was checked). ### Profile struct (`libs/databrickscfg/profile/`) - Added `Scopes` field to `profile.Profile` struct and read it from the INI file in `LoadProfiles`. This allows `auth login` and `auth token` to read existing scopes back from the profile. ## Test plan **Unit tests (`libs/databrickscfg/ops_test.go`):** - [x] `TestSaveToProfile_MergePreservesExistingKeys` — token survives when only auth_type is written - [x] `TestSaveToProfile_ClearKeysRemovesSpecifiedKeys` — token and cluster_id cleared, serverless_compute_id added - [x] `TestSaveToProfile_OverwritesExistingValues` — host updated from old to new - [x] `TestSaveToProfile_ClearKeysOnNonExistentKeyIsNoop` — clearing nonexistent keys doesn't error **Unit tests (`cmd/configure/configure_test.go`):** - [x] `TestConfigureClearsOAuthAuthType` — PAT configure clears `auth_type` and `scopes` from a previously OAuth-configured profile - [x] `TestConfigureClearsUnifiedHostMetadata` — PAT configure clears `experimental_is_unified_host` while preserving `account_id`/`workspace_id` - [x] `TestConfigureClearsServerlessWhenClusterFromEnv` — `serverless_compute_id` cleared when `DATABRICKS_CLUSTER_ID` env var provides cluster_id **Acceptance test (`acceptance/cmd/auth/login/preserve-fields/`):** - [x] Profile with `cluster_id`, `warehouse_id`, `azure_environment`, and `custom_key` → all four survive `auth login` re-login without `--configure-cluster`/`--configure-serverless` **Existing tests:** - [x] All 7 `auth login` acceptance tests pass (including `configure-serverless` which verifies `cluster_id` is still cleared) - [x] All `cmd/auth/` unit tests pass - [x] All `cmd/configure/` unit tests pass - [x] `make checks` clean - [x] `make lintfull` clean (0 issues) 🤖 Generated with [Claude Code](https://claude.com/claude-code)
diff --git a/acceptance/cmd/auth/login/preserve-fields/out.test.toml b/acceptance/cmd/auth/login/preserve-fields/out.test.toml
diff --git a/acceptance/cmd/auth/login/preserve-fields/output.txt b/acceptance/cmd/auth/login/preserve-fields/output.txt
@@ -0,0 +1,21 @@
+
+=== Initial profile
+[DEFAULT]
+host = [DATABRICKS_URL]
+cluster_id = existing-cluster-123
+warehouse_id = warehouse-456
+azure_environment = USGOVERNMENT
+custom_key = my-custom-value
+
+=== Run auth login (no --configure-cluster or --configure-serverless)
+>>> [CLI] auth login --host [DATABRICKS_URL] --profile DEFAULT
+Profile DEFAULT was successfully saved
+
+=== Profile after auth login — all non-auth fields should be preserved
+[DEFAULT]
+host              = [DATABRICKS_URL]
+cluster_id        = existing-cluster-123
+warehouse_id      = warehouse-456
+azure_environment = USGOVERNMENT
+custom_key        = my-custom-value
+auth_type         = databricks-cli
diff --git a/acceptance/cmd/auth/login/preserve-fields/script b/acceptance/cmd/auth/login/preserve-fields/script
@@ -0,0 +1,25 @@
+sethome "./home"
+
+# Create an initial profile with cluster_id, warehouse_id, azure_environment,
+# and a custom key that is not a recognized SDK config attribute.
+cat > "./home/.databrickscfg" <<EOF
+[DEFAULT]
+host = $DATABRICKS_HOST
+cluster_id = existing-cluster-123
+warehouse_id = warehouse-456
+azure_environment = USGOVERNMENT
+custom_key = my-custom-value
+EOF
+
+title "Initial profile\n"
+cat "./home/.databrickscfg"
+
+# Use a fake browser that performs a GET on the authorization URL
+# and follows the redirect back to localhost.
+export BROWSER="browser.py"
+
+title "Run auth login (no --configure-cluster or --configure-serverless)"
+trace $CLI auth login --host $DATABRICKS_HOST --profile DEFAULT
+
+title "Profile after auth login — all non-auth fields should be preserved\n"
+cat "./home/.databrickscfg"
diff --git a/acceptance/cmd/auth/login/preserve-fields/test.toml b/acceptance/cmd/auth/login/preserve-fields/test.toml
@@ -0,0 +1,3 @@
+Ignore = [
+    "home"
+]
diff --git a/acceptance/cmd/configure/clears-oauth-on-pat/out.test.toml b/acceptance/cmd/configure/clears-oauth-on-pat/out.test.toml
diff --git a/acceptance/cmd/configure/clears-oauth-on-pat/output.txt b/acceptance/cmd/configure/clears-oauth-on-pat/output.txt
@@ -0,0 +1,19 @@
+
+=== Initial profile (OAuth with unified host)
+[DEFAULT]
+host                         = https://host
+auth_type                    = databricks-cli
+scopes                       = all-apis
+experimental_is_unified_host = true
+account_id                   = acc-123
+workspace_id                 = ws-456
+
+=== Run configure with PAT token
+>>> [CLI] configure --token --host https://host
+
+=== Profile after PAT configure
+[DEFAULT]
+host         = https://host
+account_id   = acc-123
+workspace_id = ws-456
+token        = [DATABRICKS_TOKEN]
diff --git a/acceptance/cmd/configure/clears-oauth-on-pat/script b/acceptance/cmd/configure/clears-oauth-on-pat/script
@@ -0,0 +1,22 @@
+sethome "./home"
+
+# Pre-populate a profile with OAuth metadata and unified host fields
+# (as if `auth login` was previously run against a unified host).
+cat > "./home/.databrickscfg" <<EOF
+[DEFAULT]
+host                         = https://host
+auth_type                    = databricks-cli
+scopes                       = all-apis
+experimental_is_unified_host = true
+account_id                   = acc-123
+workspace_id                 = ws-456
+EOF
+
+title "Initial profile (OAuth with unified host)\n"
+cat "./home/.databrickscfg"
+
+title "Run configure with PAT token"
+echo "new-token" | trace $CLI configure --token --host https://host
+
+title "Profile after PAT configure\n"
+cat "./home/.databrickscfg"
diff --git a/acceptance/cmd/configure/clears-oauth-on-pat/test.toml b/acceptance/cmd/configure/clears-oauth-on-pat/test.toml
@@ -0,0 +1,3 @@
+Ignore = [
+    "home"
+]
diff --git a/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/out.test.toml b/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/out.test.toml
diff --git a/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/output.txt b/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/output.txt
@@ -0,0 +1,14 @@
+
+=== Initial profile with serverless_compute_id
+[DEFAULT]
+host                  = https://host
+serverless_compute_id = auto
+
+=== Run configure with cluster from env var
+>>> [CLI] configure --token --host https://host
+
+=== Profile after configure (serverless should be cleared)
+[DEFAULT]
+host       = https://host
+cluster_id = env-cluster-789
+token      = [DATABRICKS_TOKEN]
diff --git a/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/script b/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/script
@@ -0,0 +1,20 @@
+sethome "./home"
+
+# Pre-populate a profile with serverless_compute_id.
+cat > "./home/.databrickscfg" <<EOF
+[DEFAULT]
+host                  = https://host
+serverless_compute_id = auto
+EOF
+
+title "Initial profile with serverless_compute_id\n"
+cat "./home/.databrickscfg"
+
+# Set cluster ID via env var (not via --configure-cluster flag).
+export DATABRICKS_CLUSTER_ID=env-cluster-789
+
+title "Run configure with cluster from env var"
+echo "new-token" | trace $CLI configure --token --host https://host
+
+title "Profile after configure (serverless should be cleared)\n"
+cat "./home/.databrickscfg"
diff --git a/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/test.toml b/acceptance/cmd/configure/clears-serverless-when-cluster-from-env/test.toml
@@ -0,0 +1,3 @@
+Ignore = [
+    "home"
+]
diff --git a/cmd/auth/login.go b/cmd/auth/login.go
@@ -155,10 +155,18 @@ depends on the existing profiles you have set in your configuration file
 		authArguments.Profile = profileName
 
 		var scopesList []string
-		if scopes != "" {
+		switch {
+		case scopes != "":
+			// Explicit --scopes flag takes precedence.
 			for _, s := range strings.Split(scopes, ",") {
 				scopesList = append(scopesList, strings.TrimSpace(s))
 			}
+		case existingProfile != nil && existingProfile.Scopes != "":
+			// Preserve scopes from the existing profile so re-login
+			// uses the same scopes the user previously configured.
+			for _, s := range strings.Split(existingProfile.Scopes, ",") {
+				scopesList = append(scopesList, strings.TrimSpace(s))
+			}
 		}
 
 		oauthArgument, err := authArguments.ToOAuthArgument()
@@ -190,6 +198,18 @@ depends on the existing profiles you have set in your configuration file
 		// 2. Saving the profile.
 
 		var clusterID, serverlessComputeID string
+
+		// Keys to explicitly remove from the profile. OAuth login always
+		// clears incompatible credential fields (PAT, basic auth, M2M).
+		clearKeys := oauthLoginClearKeys()
+
+		// Boolean false is zero-valued and skipped by SaveToProfile's IsZero
+		// check. Explicitly clear experimental_is_unified_host when false so
+		// it doesn't remain sticky from a previous login.
+		if !authArguments.IsUnifiedHost {
+			clearKeys = append(clearKeys, "experimental_is_unified_host")
+		}
+
 		switch {
 		case configureCluster:
 			// Create a workspace client to list clusters for interactive selection.
@@ -210,20 +230,14 @@ depends on the existing profiles you have set in your configuration file
 			if err != nil {
 				return err
 			}
+			// Cluster and serverless are mutually exclusive.
+			clearKeys = append(clearKeys, "serverless_compute_id")
 		case configureServerless:
 			serverlessComputeID = "auto"
+			// Cluster and serverless are mutually exclusive.
+			clearKeys = append(clearKeys, "cluster_id")
 		default:
-			// Respect the existing profile if it exists, even if it has
-			// both cluster and serverless configured. Tools relying on
-			// these fields from the profile will need to handle this case.
-			//
-			// TODO: consider whether we should use this an an opportunity
-			// to clean up the profile under the assumption that serverless
-			// is the preferred option.
-			if existingProfile != nil {
-				clusterID = existingProfile.ClusterID
-				serverlessComputeID = existingProfile.ServerlessComputeID
-			}
+			// Neither flag: preserve both from existing profile via merge semantics.
 		}
 
 		if profileName != "" {
@@ -238,7 +252,7 @@ depends on the existing profiles you have set in your configuration file
 				ConfigFile:                 os.Getenv("DATABRICKS_CONFIG_FILE"),
 				ServerlessComputeID:        serverlessComputeID,
 				Scopes:                     scopesList,
-			})
+			}, clearKeys...)
 			if err != nil {
 				return err
 			}
@@ -401,6 +415,13 @@ func openURLSuppressingStderr(url string) error {
 	return browserpkg.OpenURL(url)
 }
 
+// oauthLoginClearKeys returns profile keys that should be explicitly removed
+// when performing an OAuth login. Derives auth credential fields dynamically
+// from the SDK's ConfigAttributes to stay in sync as new auth methods are added.
+func oauthLoginClearKeys() []string {
+	return databrickscfg.AuthCredentialKeys()
+}
+
 // getBrowserFunc returns a function that opens the given URL in the browser.
 // It respects the BROWSER environment variable:
 // - empty string: uses the default browser
diff --git a/cmd/auth/token.go b/cmd/auth/token.go
@@ -423,14 +423,27 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr
 
 	loginArgs.Profile = profileName
 
+	// Preserve scopes from the existing profile so the inline login
+	// uses the same scopes the user previously configured.
+	var scopesList []string
+	if existingProfile != nil && existingProfile.Scopes != "" {
+		for _, s := range strings.Split(existingProfile.Scopes, ",") {
+			scopesList = append(scopesList, strings.TrimSpace(s))
+		}
+	}
+
 	oauthArgument, err := loginArgs.ToOAuthArgument()
 	if err != nil {
 		return "", nil, err
 	}
-	persistentAuth, err := u2m.NewPersistentAuth(ctx,
+	persistentAuthOpts := []u2m.PersistentAuthOption{
 		u2m.WithOAuthArgument(oauthArgument),
 		u2m.WithBrowser(openURLSuppressingStderr),
-	)
+	}
+	if len(scopesList) > 0 {
+		persistentAuthOpts = append(persistentAuthOpts, u2m.WithScopes(scopesList))
+	}
+	persistentAuth, err := u2m.NewPersistentAuth(ctx, persistentAuthOpts...)
 	if err != nil {
 		return "", nil, err
 	}
@@ -443,6 +456,10 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr
 		return "", nil, err
 	}
 
+	clearKeys := oauthLoginClearKeys()
+	if !loginArgs.IsUnifiedHost {
+		clearKeys = append(clearKeys, "experimental_is_unified_host")
+	}
 	err = databrickscfg.SaveToProfile(ctx, &config.Config{
 		Profile:                    profileName,
 		Host:                       loginArgs.Host,
@@ -451,7 +468,8 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr
 		WorkspaceID:                loginArgs.WorkspaceID,
 		Experimental_IsUnifiedHost: loginArgs.IsUnifiedHost,
 		ConfigFile:                 os.Getenv("DATABRICKS_CONFIG_FILE"),
-	})
+		Scopes:                     scopesList,
+	}, clearKeys...)
 	if err != nil {
 		return "", nil, err
 	}
diff --git a/cmd/configure/configure.go b/cmd/configure/configure.go
@@ -12,6 +12,15 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// patConfigureExtraClearKeys lists non-credential profile keys that should also
+// be cleared when saving a PAT-based profile. Auth credential keys are derived
+// dynamically from config.ConfigAttributes via databrickscfg.AuthCredentialKeys().
+var patConfigureExtraClearKeys = []string{
+	"auth_type",
+	"scopes",
+	"databricks_cli_path",
+}
+
 func configureInteractive(cmd *cobra.Command, flags *configureFlags, cfg *config.Config) error {
 	ctx := cmd.Context()
 
@@ -141,14 +150,28 @@ The host must be specified with the --host flag or the DATABRICKS_HOST environme
 		// This is relevant for OAuth only.
 		cfg.DatabricksCliPath = ""
 
-		// Save profile to config file.
+		// Save profile to config file. PAT-based configure clears all
+		// non-PAT auth credentials and OAuth metadata to prevent
+		// multi-auth conflicts in the profile.
+		clearKeys := append(databrickscfg.AuthCredentialKeys(), patConfigureExtraClearKeys...)
+
+		// Cluster and serverless are mutually exclusive. Clear serverless
+		// when a cluster is being set (via flag or env var).
+		if cfg.ClusterID != "" {
+			clearKeys = append(clearKeys, "serverless_compute_id")
+		}
+
+		// Clear stale unified-host metadata — PAT profiles don't use it,
+		// and leaving it can change HostType() routing.
+		clearKeys = append(clearKeys, "experimental_is_unified_host")
+
 		return databrickscfg.SaveToProfile(ctx, &config.Config{
 			Profile:    cfg.Profile,
 			Host:       cfg.Host,
 			Token:      cfg.Token,
 			ClusterID:  cfg.ClusterID,
 			ConfigFile: cfg.ConfigFile,
-		})
+		}, clearKeys...)
 	}
 
 	return cmd
diff --git a/libs/databrickscfg/ops.go b/libs/databrickscfg/ops.go
@@ -80,7 +80,26 @@ func matchOrCreateSection(ctx context.Context, configFile *config.File, cfg *con
 	return section, nil
 }
 
-func SaveToProfile(ctx context.Context, cfg *config.Config) error {
+// AuthCredentialKeys returns the config file key names for all auth credential
+// fields from the SDK's ConfigAttributes. These are fields annotated with an
+// auth type (e.g. pat, basic, oauth, azure, google). Use this to clear stale
+// credentials when switching auth methods.
+func AuthCredentialKeys() []string {
+	var keys []string
+	for _, attr := range config.ConfigAttributes {
+		if attr.HasAuthAttribute() {
+			keys = append(keys, attr.Name)
+		}
+	}
+	return keys
+}
+
+// SaveToProfile merges the provided config into a .databrickscfg profile.
+// Non-zero fields in cfg overwrite existing values. Existing keys not
+// mentioned in cfg are preserved. Keys listed in clearKeys are explicitly
+// removed (use this for mutually exclusive fields like cluster_id vs
+// serverless_compute_id, or to drop stale auth credentials on auth-type switch).
+func SaveToProfile(ctx context.Context, cfg *config.Config, clearKeys ...string) error {
 	configFile, err := loadOrCreateConfigFile(cfg.ConfigFile)
 	if err != nil {
 		return err
@@ -95,11 +114,13 @@ func SaveToProfile(ctx context.Context, cfg *config.Config) error {
 	cfg.Profile = ""
 	cfg.ConfigFile = ""
 
-	// clear old keys in case we're overriding the section
-	for _, oldKey := range section.KeyStrings() {
-		section.DeleteKey(oldKey)
+	// Explicitly remove keys the caller wants cleared.
+	for _, key := range clearKeys {
+		section.DeleteKey(key)
 	}
 
+	// Write non-zero fields from the new config. Iterates ConfigAttributes
+	// in declaration order for deterministic key ordering on new profiles.
 	for _, attr := range config.ConfigAttributes {
 		if attr.IsZero(cfg) {
 			continue
diff --git a/libs/databrickscfg/ops_test.go b/libs/databrickscfg/ops_test.go
diff --git a/libs/databrickscfg/profile/file.go b/libs/databrickscfg/profile/file.go
diff --git a/libs/databrickscfg/profile/profile.go b/libs/databrickscfg/profile/profile.go
