From 393c15efe38779b7797738805fd21bfe3322ea75 Mon Sep 17 00:00:00 2001 From: simon Date: Tue, 21 Apr 2026 12:30:48 +0200 Subject: [PATCH 1/2] auth: decouple CLI from SDK's Experimental_IsUnifiedHost field Prepare for SDK PR databricks/databricks-sdk-go#1641 which removes the field, its env var, and the UnifiedHost case in HostType(). Thread the unified-host signal through CLI-owned types via a new HasUnifiedHostSignal helper and an explicit fallback parameter on IsSPOG. New profiles no longer persist experimental_is_unified_host; existing profiles still read the key for back-compat. The --experimental-is-unified-host flag and DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST env var are deprecated no-ops for this release. Co-authored-by: Isaac --- NEXT_CHANGELOG.md | 1 + .../credentials/unified-host/out.requests.txt | 48 --------- .../credentials/unified-host/out.test.toml | 5 - .../auth/credentials/unified-host/output.txt | 12 --- .../auth/credentials/unified-host/script | 12 --- .../auth/credentials/unified-host/test.toml | 3 - bundle/config/workspace.go | 10 +- cmd/auth/auth.go | 2 +- cmd/auth/login.go | 102 +++++++----------- cmd/auth/login_test.go | 25 +++++ cmd/auth/profiles.go | 18 ++-- cmd/auth/profiles_test.go | 41 +------ cmd/auth/token.go | 22 ++-- libs/auth/arguments.go | 13 ++- libs/auth/config_type.go | 35 ++++-- libs/auth/config_type_test.go | 54 +++++++--- libs/auth/credentials.go | 14 +-- libs/auth/credentials_test.go | 20 ++-- libs/auth/error.go | 15 ++- libs/auth/error_test.go | 14 +-- 20 files changed, 195 insertions(+), 271 deletions(-) delete mode 100644 acceptance/auth/credentials/unified-host/out.requests.txt delete mode 100644 acceptance/auth/credentials/unified-host/out.test.toml delete mode 100644 acceptance/auth/credentials/unified-host/output.txt delete mode 100644 acceptance/auth/credentials/unified-host/script delete mode 100644 acceptance/auth/credentials/unified-host/test.toml diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index ede82f7779..3412a8f787 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,6 +8,7 @@ * Added `--limit` flag to all paginated list commands for client-side result capping ([#4984](https://github.com/databricks/cli/pull/4984)). * Accept `yes` in addition to `y` for confirmation prompts, and show `[y/N]` to indicate that no is the default. +* Stop persisting `experimental_is_unified_host` to new profiles and ignore the `DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST` env var. Unified hosts are now detected automatically from `/.well-known/databricks-config`. Existing profiles with the key set continue to work; `--experimental-is-unified-host` remains as a deprecated no-op for this release. ### Bundles * Remove `experimental-jobs-as-code` template, superseded by `pydabs` ([#4999](https://github.com/databricks/cli/pull/4999)). diff --git a/acceptance/auth/credentials/unified-host/out.requests.txt b/acceptance/auth/credentials/unified-host/out.requests.txt deleted file mode 100644 index e94814526d..0000000000 --- a/acceptance/auth/credentials/unified-host/out.requests.txt +++ /dev/null @@ -1,48 +0,0 @@ -{ - "headers": { - "User-Agent": [ - "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]" - ] - }, - "method": "GET", - "path": "/.well-known/databricks-config" -} -{ - "headers": { - "Authorization": [ - "Bearer dapi-unified-token" - ], - "User-Agent": [ - "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/current-user_me cmd-exec-id/[UUID] interactive/none auth/pat" - ], - "X-Databricks-Org-Id": [ - "[NUMID]" - ] - }, - "method": "GET", - "path": "/api/2.0/preview/scim/v2/Me" -} -{ - "headers": { - "User-Agent": [ - "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS]" - ] - }, - "method": "GET", - "path": "/.well-known/databricks-config" -} -{ - "headers": { - "Authorization": [ - "Bearer dapi-unified-token" - ], - "User-Agent": [ - "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/current-user_me cmd-exec-id/[UUID] interactive/none auth/pat" - ], - "X-Databricks-Org-Id": [ - "[NUMID]" - ] - }, - "method": "GET", - "path": "/api/2.0/preview/scim/v2/Me" -} diff --git a/acceptance/auth/credentials/unified-host/out.test.toml b/acceptance/auth/credentials/unified-host/out.test.toml deleted file mode 100644 index d560f1de04..0000000000 --- a/acceptance/auth/credentials/unified-host/out.test.toml +++ /dev/null @@ -1,5 +0,0 @@ -Local = true -Cloud = false - -[EnvMatrix] - DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/auth/credentials/unified-host/output.txt b/acceptance/auth/credentials/unified-host/output.txt deleted file mode 100644 index af071887d0..0000000000 --- a/acceptance/auth/credentials/unified-host/output.txt +++ /dev/null @@ -1,12 +0,0 @@ - -=== With workspace_id -{ - "id":"[USERID]", - "userName":"[USERNAME]" -} - -=== Without workspace_id (should error) -{ - "id":"[USERID]", - "userName":"[USERNAME]" -} diff --git a/acceptance/auth/credentials/unified-host/script b/acceptance/auth/credentials/unified-host/script deleted file mode 100644 index f785987219..0000000000 --- a/acceptance/auth/credentials/unified-host/script +++ /dev/null @@ -1,12 +0,0 @@ -# Test unified host authentication with PAT token -export DATABRICKS_TOKEN=dapi-unified-token -export DATABRICKS_ACCOUNT_ID=test-account-123 -export DATABRICKS_WORKSPACE_ID=1234567890 -export DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST=true - -title "With workspace_id\n" -$CLI current-user me - -title "Without workspace_id (should error)\n" -unset DATABRICKS_WORKSPACE_ID -errcode $CLI current-user me diff --git a/acceptance/auth/credentials/unified-host/test.toml b/acceptance/auth/credentials/unified-host/test.toml deleted file mode 100644 index fd0cd96421..0000000000 --- a/acceptance/auth/credentials/unified-host/test.toml +++ /dev/null @@ -1,3 +0,0 @@ -# Test unified host authentication with PAT tokens -# Include X-Databricks-Org-Id header to verify workspace_id is sent -IncludeRequestHeaders = ["Authorization", "User-Agent", "X-Databricks-Org-Id"] diff --git a/bundle/config/workspace.go b/bundle/config/workspace.go index 325e7cbd55..9cd397f13a 100644 --- a/bundle/config/workspace.go +++ b/bundle/config/workspace.go @@ -46,6 +46,11 @@ type Workspace struct { AzureLoginAppID string `json:"azure_login_app_id,omitempty"` // Unified host specific attributes. + // + // ExperimentalIsUnifiedHost is a deprecated no-op. Unified hosts are now + // detected automatically from /.well-known/databricks-config. The field is + // retained so existing databricks.yml files using it still validate against + // the bundle schema. ExperimentalIsUnifiedHost bool `json:"experimental_is_unified_host,omitempty"` AccountID string `json:"account_id,omitempty"` WorkspaceID string `json:"workspace_id,omitempty"` @@ -135,9 +140,8 @@ func (w *Workspace) Config(ctx context.Context) *config.Config { AzureLoginAppID: w.AzureLoginAppID, // Unified host - Experimental_IsUnifiedHost: w.ExperimentalIsUnifiedHost, - AccountID: w.AccountID, - WorkspaceID: w.WorkspaceID, + AccountID: w.AccountID, + WorkspaceID: w.WorkspaceID, } for k := range config.ConfigAttributes { diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index 6bca3f5962..b1f35e1ac6 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -28,7 +28,7 @@ GCP: https://docs.gcp.databricks.com/dev-tools/auth/index.html`, var authArguments auth.AuthArguments cmd.PersistentFlags().StringVar(&authArguments.Host, "host", "", "Databricks Host") cmd.PersistentFlags().StringVar(&authArguments.AccountID, "account-id", "", "Databricks Account ID") - cmd.PersistentFlags().BoolVar(&authArguments.IsUnifiedHost, "experimental-is-unified-host", false, "Flag to indicate if the host is a unified host") + cmd.PersistentFlags().BoolVar(&authArguments.IsUnifiedHost, "experimental-is-unified-host", false, "Deprecated: unified hosts are now detected automatically from /.well-known/databricks-config") cmd.PersistentFlags().StringVar(&authArguments.WorkspaceID, "workspace-id", "", "Databricks Workspace ID") cmd.AddCommand(newEnvCommand()) diff --git a/cmd/auth/login.go b/cmd/auth/login.go index 7157f797b7..93d96cb939 100644 --- a/cmd/auth/login.go +++ b/cmd/auth/login.go @@ -279,15 +279,11 @@ a new profile is created. var clusterID, serverlessComputeID string // Keys to explicitly remove from the profile. OAuth login always - // clears incompatible credential fields (PAT, basic auth, M2M). + // clears incompatible credential fields (PAT, basic auth, M2M) and + // the deprecated experimental_is_unified_host key (routing now comes + // from .well-known discovery, so stale values would be misleading). 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") - } + clearKeys = append(clearKeys, "experimental_is_unified_host") switch { case configureCluster: @@ -295,11 +291,10 @@ a new profile is created. // We use a custom CredentialsStrategy that wraps the token we just minted, // avoiding the need to spawn a child CLI process (which AuthType "databricks-cli" does). w, err := databricks.NewWorkspaceClient(&databricks.Config{ - Host: authArguments.Host, - AccountID: authArguments.AccountID, - WorkspaceID: authArguments.WorkspaceID, - Experimental_IsUnifiedHost: authArguments.IsUnifiedHost, - Credentials: config.NewTokenSourceStrategy("login-token", authconv.AuthTokenSource(persistentAuth)), + Host: authArguments.Host, + AccountID: authArguments.AccountID, + WorkspaceID: authArguments.WorkspaceID, + Credentials: config.NewTokenSourceStrategy("login-token", authconv.AuthTokenSource(persistentAuth)), }) if err != nil { return err @@ -320,17 +315,19 @@ a new profile is created. } if profileName != "" { + // experimental_is_unified_host is no longer written to new profiles. + // Routing now comes from .well-known discovery; stale keys on existing + // profiles are cleaned up via clearKeys above. err := databrickscfg.SaveToProfile(ctx, &config.Config{ - Profile: profileName, - Host: authArguments.Host, - AuthType: authTypeDatabricksCLI, - AccountID: authArguments.AccountID, - WorkspaceID: authArguments.WorkspaceID, - Experimental_IsUnifiedHost: authArguments.IsUnifiedHost, - ClusterID: clusterID, - ConfigFile: env.Get(ctx, "DATABRICKS_CONFIG_FILE"), - ServerlessComputeID: serverlessComputeID, - Scopes: scopesList, + Profile: profileName, + Host: authArguments.Host, + AuthType: authTypeDatabricksCLI, + AccountID: authArguments.AccountID, + WorkspaceID: authArguments.WorkspaceID, + ClusterID: clusterID, + ConfigFile: env.Get(ctx, "DATABRICKS_CONFIG_FILE"), + ServerlessComputeID: serverlessComputeID, + Scopes: scopesList, }, clearKeys...) if err != nil { return err @@ -407,52 +404,33 @@ func setHostAndAccountId(ctx context.Context, existingProfile *profile.Profile, // are logged as warnings and never block login. runHostDiscovery(ctx, authArguments) - // Determine the host type and handle account ID / workspace ID accordingly - cfg := &config.Config{ - Host: authArguments.Host, - AccountID: authArguments.AccountID, - WorkspaceID: authArguments.WorkspaceID, - Experimental_IsUnifiedHost: authArguments.IsUnifiedHost, - } - - switch cfg.HostType() { //nolint:staticcheck // HostType() deprecated in SDK v0.127.0; SDK moving to host-agnostic behavior. - case config.AccountHost: - // Account host: prompt for account ID if not provided - if authArguments.AccountID == "" { - if existingProfile != nil && existingProfile.AccountID != "" { - authArguments.AccountID = existingProfile.AccountID - } else { - accountId, err := promptForAccountID(ctx) - if err != nil { - return err - } - authArguments.AccountID = accountId - } - } - case config.UnifiedHost: - // Unified host requires an account ID for OAuth URL construction. - // Workspace selection happens post-OAuth via promptForWorkspaceSelection. - if authArguments.AccountID == "" { - if existingProfile != nil && existingProfile.AccountID != "" { - authArguments.AccountID = existingProfile.AccountID - } else { - accountId, err := promptForAccountID(ctx) - if err != nil { - return err - } - authArguments.AccountID = accountId + if needsAccountIDPrompt(authArguments.Host, authArguments.IsUnifiedHost, authArguments.DiscoveryURL) && authArguments.AccountID == "" { + if existingProfile != nil && existingProfile.AccountID != "" { + authArguments.AccountID = existingProfile.AccountID + } else { + accountId, err := promptForAccountID(ctx) + if err != nil { + return err } + authArguments.AccountID = accountId } - case config.WorkspaceHost: - // Regular workspace host: no additional prompts needed. - // If discovery already populated account_id/workspace_id, those are kept. - default: - return fmt.Errorf("unknown host type: %v", cfg.HostType()) //nolint:staticcheck // HostType() deprecated in SDK v0.127.0; SDK moving to host-agnostic behavior. } return nil } +// needsAccountIDPrompt reports whether the target host requires an account ID +// for OAuth URL construction. True for classic account hosts (accounts.*) and +// for unified hosts (either legacy flag or account-scoped DiscoveryURL). +func needsAccountIDPrompt(host string, isUnifiedHost bool, discoveryURL string) bool { + canonicalHost := (&config.Config{Host: host}).CanonicalHostName() + if strings.HasPrefix(canonicalHost, "https://accounts.") || + strings.HasPrefix(canonicalHost, "https://accounts-dod.") { + return true + } + return auth.HasUnifiedHostSignal(discoveryURL, isUnifiedHost) +} + // runHostDiscovery calls EnsureResolved() with a temporary config to fetch // .well-known/databricks-config from the host. Populates account_id and // workspace_id from discovery if not already set. diff --git a/cmd/auth/login_test.go b/cmd/auth/login_test.go index 81924f027a..2b6cc4b642 100644 --- a/cmd/auth/login_test.go +++ b/cmd/auth/login_test.go @@ -392,6 +392,31 @@ func TestShouldUseDiscovery(t *testing.T) { } } +func TestNeedsAccountIDPrompt(t *testing.T) { + cases := []struct { + name string + host string + isUnifiedHost bool + discoveryURL string + want bool + }{ + {name: "classic accounts host", host: "https://accounts.cloud.databricks.com", want: true}, + {name: "accounts-dod host", host: "https://accounts-dod.databricks.com", want: true}, + {name: "accounts host with path", host: "https://accounts.cloud.databricks.com/some/path", want: true}, + {name: "plain workspace host", host: "https://workspace.cloud.databricks.com"}, + {name: "unified flag set", host: "https://spog.cloud.databricks.com", isUnifiedHost: true, want: true}, + {name: "account-scoped DiscoveryURL", host: "https://spog.cloud.databricks.com", discoveryURL: "https://spog.cloud.databricks.com/oidc/accounts/acct-123/.well-known/oauth-authorization-server", want: true}, + {name: "workspace-scoped DiscoveryURL", host: "https://workspace.cloud.databricks.com", discoveryURL: "https://workspace.cloud.databricks.com/oidc/.well-known/oauth-authorization-server"}, + {name: "workspace host no signals", host: "https://workspace.cloud.databricks.com"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := needsAccountIDPrompt(tc.host, tc.isUnifiedHost, tc.discoveryURL) + assert.Equal(t, tc.want, got) + }) + } +} + func TestSplitScopes(t *testing.T) { tests := []struct { name string diff --git a/cmd/auth/profiles.go b/cmd/auth/profiles.go index 51c397a9ea..6e926ab22b 100644 --- a/cmd/auth/profiles.go +++ b/cmd/auth/profiles.go @@ -29,6 +29,11 @@ type profileMetadata struct { AuthType string `json:"auth_type"` Valid bool `json:"valid"` Default bool `json:"default,omitempty"` + + // isUnifiedHost carries the legacy experimental_is_unified_host value so we + // can route unified-host profiles without the SDK field (which is being + // removed). Not serialized. + isUnifiedHost bool } func (c *profileMetadata) IsEmpty() bool { @@ -57,7 +62,7 @@ func (c *profileMetadata) Load(ctx context.Context, configFilePath string, skipV return } - configType := auth.ResolveConfigType(cfg) + configType := auth.ResolveConfigType(cfg, c.isUnifiedHost) if configType != cfg.ConfigType() { log.Debugf(ctx, "Profile %q: overrode config type from %s to %s (SPOG host)", c.Name, cfg.ConfigType(), configType) } @@ -126,11 +131,12 @@ func newProfilesCommand() *cobra.Command { for _, v := range iniFile.Sections() { hash := v.KeysHash() profile := &profileMetadata{ - Name: v.Name(), - Host: hash["host"], - AccountID: hash["account_id"], - WorkspaceID: hash["workspace_id"], - Default: v.Name() == defaultProfile, + Name: v.Name(), + Host: hash["host"], + AccountID: hash["account_id"], + WorkspaceID: hash["workspace_id"], + Default: v.Name() == defaultProfile, + isUnifiedHost: hash["experimental_is_unified_host"] == "true", } if profile.IsEmpty() { continue diff --git a/cmd/auth/profiles_test.go b/cmd/auth/profiles_test.go index a0792344ae..59803e210c 100644 --- a/cmd/auth/profiles_test.go +++ b/cmd/auth/profiles_test.go @@ -201,45 +201,6 @@ func TestProfileLoadSPOGConfigType(t *testing.T) { } } -func TestProfileLoadUnifiedHostFallback(t *testing.T) { - // When Experimental_IsUnifiedHost is set but .well-known is unreachable, - // ConfigType() returns InvalidConfig. The fallback should reclassify as - // AccountConfig so the profile is still validated. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/json") - switch r.URL.Path { - case "/.well-known/databricks-config": - w.WriteHeader(http.StatusNotFound) - case "/api/2.0/accounts/unified-acct/workspaces": - _ = json.NewEncoder(w).Encode([]map[string]any{}) - default: - w.WriteHeader(http.StatusNotFound) - } - })) - 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 := "[unified-profile]\nhost = " + server.URL + "\ntoken = test-token\naccount_id = unified-acct\nexperimental_is_unified_host = true\n" - require.NoError(t, os.WriteFile(configFile, []byte(content), 0o600)) - - p := &profileMetadata{ - Name: "unified-profile", - Host: server.URL, - AccountID: "unified-acct", - } - p.Load(t.Context(), configFile, false) - - assert.True(t, p.Valid, "unified host profile should be valid via fallback") - assert.NotEmpty(t, p.Host) - assert.NotEmpty(t, p.AuthType) -} - func TestClassicAccountsHostConfigType(t *testing.T) { // Classic accounts.* hosts can't be tested through Load() because httptest // generates 127.0.0.1 URLs. Verify directly that ConfigType() classifies @@ -256,7 +217,7 @@ func TestClassicAccountsHostConfigType(t *testing.T) { } func TestProfileLoadNoDiscoveryStaysWorkspace(t *testing.T) { - // When .well-known returns 404 and Experimental_IsUnifiedHost is false, + // When .well-known returns 404 and the unified-host fallback is false, // the SPOG override should NOT trigger even if account_id is set. The // profile should stay WorkspaceConfig and validate via CurrentUser.Me. server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/cmd/auth/token.go b/cmd/auth/token.go index fbdd8811e8..0b814b418b 100644 --- a/cmd/auth/token.go +++ b/cmd/auth/token.go @@ -324,9 +324,6 @@ func resolveNoArgsToken(ctx context.Context, profiler profile.Profiler, authArgs if v := env.Get(ctx, "DATABRICKS_WORKSPACE_ID"); v != "" { authArgs.WorkspaceID = v } - if ok, _ := env.GetBool(ctx, "DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST"); ok { - authArgs.IsUnifiedHost = true - } return "", nil, nil } @@ -480,19 +477,16 @@ func runInlineLogin(ctx context.Context, profiler profile.Profiler) (string, *pr } clearKeys := oauthLoginClearKeys() - if !loginArgs.IsUnifiedHost { - clearKeys = append(clearKeys, "experimental_is_unified_host") - } + clearKeys = append(clearKeys, "experimental_is_unified_host") err = databrickscfg.SaveToProfile(ctx, &config.Config{ - Profile: profileName, - Host: loginArgs.Host, - AuthType: authTypeDatabricksCLI, - AccountID: loginArgs.AccountID, - WorkspaceID: loginArgs.WorkspaceID, - Experimental_IsUnifiedHost: loginArgs.IsUnifiedHost, - ConfigFile: env.Get(ctx, "DATABRICKS_CONFIG_FILE"), - Scopes: scopesList, + Profile: profileName, + Host: loginArgs.Host, + AuthType: authTypeDatabricksCLI, + AccountID: loginArgs.AccountID, + WorkspaceID: loginArgs.WorkspaceID, + ConfigFile: env.Get(ctx, "DATABRICKS_CONFIG_FILE"), + Scopes: scopesList, }, clearKeys...) if err != nil { return "", nil, err diff --git a/libs/auth/arguments.go b/libs/auth/arguments.go index 4f724cc801..e66fc30d72 100644 --- a/libs/auth/arguments.go +++ b/libs/auth/arguments.go @@ -30,7 +30,7 @@ type AuthArguments struct { // ToOAuthArgument converts the AuthArguments to an OAuthArgument from the Go SDK. // It calls EnsureResolved() to run host metadata discovery and routes based on -// the resolved DiscoveryURL rather than the Experimental_IsUnifiedHost flag. +// the resolved DiscoveryURL, with a.IsUnifiedHost as a legacy fallback. func (a AuthArguments) ToOAuthArgument() (u2m.OAuthArgument, error) { // Strip the "none" sentinel so it is never passed to the SDK. workspaceID := a.WorkspaceID @@ -39,11 +39,10 @@ func (a AuthArguments) ToOAuthArgument() (u2m.OAuthArgument, error) { } cfg := &config.Config{ - Host: a.Host, - AccountID: a.AccountID, - WorkspaceID: workspaceID, - Experimental_IsUnifiedHost: a.IsUnifiedHost, - HTTPTimeoutSeconds: 5, + Host: a.Host, + AccountID: a.AccountID, + WorkspaceID: workspaceID, + HTTPTimeoutSeconds: 5, // Skip config file loading. We only want host metadata resolution // based on the explicit fields provided. Loaders: []config.Loader{config.ConfigAttributes}, @@ -69,7 +68,7 @@ func (a AuthArguments) ToOAuthArgument() (u2m.OAuthArgument, error) { // discovery (which returns account_id for every host since PR #4809). // Using cfg.AccountID would cause IsSPOG to misroute plain workspace // hosts as SPOG simply because their metadata includes an account_id. - if IsSPOG(cfg, a.AccountID) { + if IsSPOG(cfg, a.AccountID, a.IsUnifiedHost) { return u2m.NewProfileUnifiedOAuthArgument(host, cfg.AccountID, a.Profile) } diff --git a/libs/auth/config_type.go b/libs/auth/config_type.go index 520b6864cd..4cd5a8523d 100644 --- a/libs/auth/config_type.go +++ b/libs/auth/config_type.go @@ -6,23 +6,35 @@ import ( "github.com/databricks/databricks-sdk-go/config" ) +// HasUnifiedHostSignal reports whether a host has been identified as unified, +// either by a resolved DiscoveryURL pointing at an account-scoped OIDC endpoint +// or by the caller-provided legacy fallback. Extracted so callers that don't +// (yet) have an account ID can check the signal without tripping IsSPOG's guard. +// +// fallback replaces a former read of cfg.Experimental_IsUnifiedHost. Callers +// thread the CLI-side signal in (e.g. AuthArguments.IsUnifiedHost, +// Profile.IsUnifiedHost) because the SDK field is being removed. +func HasUnifiedHostSignal(discoveryURL string, fallback bool) bool { + if discoveryURL != "" && strings.Contains(discoveryURL, "/oidc/accounts/") { + return true + } + return fallback +} + // IsSPOG returns true if the config represents a SPOG (Single Pane of Glass) -// host with account-scoped OIDC. Detection is based on: -// 1. The resolved DiscoveryURL containing /oidc/accounts/ (from .well-known). -// 2. The Experimental_IsUnifiedHost flag as a legacy fallback. +// host with account-scoped OIDC. Detection layers HasUnifiedHostSignal on top +// of an accountID guard: SPOG routing requires an account ID to construct the +// OAuth URL, so a nil or empty accountID always returns false. // // The accountID parameter is separate from cfg.AccountID so that callers can // control the source: ResolveConfigType passes cfg.AccountID (from config file), // while ToOAuthArgument passes the caller-provided value to avoid env var // contamination (DATABRICKS_ACCOUNT_ID or .well-known back-fill). -func IsSPOG(cfg *config.Config, accountID string) bool { +func IsSPOG(cfg *config.Config, accountID string, unifiedHostFallback bool) bool { if accountID == "" { return false } - if cfg.DiscoveryURL != "" && strings.Contains(cfg.DiscoveryURL, "/oidc/accounts/") { - return true - } - return cfg.Experimental_IsUnifiedHost + return HasUnifiedHostSignal(cfg.DiscoveryURL, unifiedHostFallback) } // ResolveConfigType determines the effective ConfigType for a resolved config. @@ -31,18 +43,19 @@ func IsSPOG(cfg *config.Config, accountID string) bool { // function additionally uses IsSPOG to detect SPOG hosts. // // The cfg must already be resolved (via EnsureResolved) before calling this. -func ResolveConfigType(cfg *config.Config) config.ConfigType { +// unifiedHostFallback is threaded through to IsSPOG; see its docstring. +func ResolveConfigType(cfg *config.Config, unifiedHostFallback bool) config.ConfigType { configType := cfg.ConfigType() if configType == config.AccountConfig { return configType } - if !IsSPOG(cfg, cfg.AccountID) { + if !IsSPOG(cfg, cfg.AccountID, unifiedHostFallback) { return configType } // The WorkspaceConfig return is a no-op when configType is already - // WorkspaceConfig, but is needed for InvalidConfig (legacy IsUnifiedHost + // WorkspaceConfig, but is needed for InvalidConfig (legacy unified-host // profiles where the SDK dropped the UnifiedHost case in v0.126.0). if cfg.WorkspaceID != "" && cfg.WorkspaceID != WorkspaceIDNone { return config.WorkspaceConfig diff --git a/libs/auth/config_type_test.go b/libs/auth/config_type_test.go index 0ce3b6d410..823a396ebd 100644 --- a/libs/auth/config_type_test.go +++ b/libs/auth/config_type_test.go @@ -7,11 +7,33 @@ import ( "github.com/stretchr/testify/assert" ) +func TestHasUnifiedHostSignal(t *testing.T) { + cases := []struct { + name string + discoveryURL string + fallback bool + want bool + }{ + {name: "no signal", want: false}, + {name: "account-scoped OIDC", discoveryURL: "https://spog.databricks.com/oidc/accounts/acct-123/.well-known/oauth-authorization-server", want: true}, + {name: "workspace-scoped OIDC", discoveryURL: "https://workspace.databricks.com/oidc/.well-known/oauth-authorization-server", want: false}, + {name: "fallback only", fallback: true, want: true}, + {name: "both set", discoveryURL: "https://spog.databricks.com/oidc/accounts/acct-123", fallback: true, want: true}, + {name: "workspace OIDC with fallback", discoveryURL: "https://workspace.databricks.com/oidc/.well-known", fallback: true, want: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, HasUnifiedHostSignal(tc.discoveryURL, tc.fallback)) + }) + } +} + func TestResolveConfigType(t *testing.T) { cases := []struct { - name string - cfg *config.Config - want config.ConfigType + name string + cfg *config.Config + unifiedHostFallback bool + want config.ConfigType }{ { name: "classic accounts host stays AccountConfig", @@ -60,26 +82,26 @@ func TestResolveConfigType(t *testing.T) { want: config.WorkspaceConfig, }, { - name: "IsUnifiedHost fallback without discovery routes to AccountConfig", + name: "unifiedHostFallback without discovery routes to AccountConfig", cfg: &config.Config{ - Host: "https://spog.databricks.com", - AccountID: "acct-123", - Experimental_IsUnifiedHost: true, + Host: "https://spog.databricks.com", + AccountID: "acct-123", }, - want: config.AccountConfig, + unifiedHostFallback: true, + want: config.AccountConfig, }, { - name: "IsUnifiedHost fallback with workspace routes to WorkspaceConfig", + name: "unifiedHostFallback with workspace routes to WorkspaceConfig", cfg: &config.Config{ - Host: "https://spog.databricks.com", - AccountID: "acct-123", - WorkspaceID: "ws-456", - Experimental_IsUnifiedHost: true, + Host: "https://spog.databricks.com", + AccountID: "acct-123", + WorkspaceID: "ws-456", }, - want: config.WorkspaceConfig, + unifiedHostFallback: true, + want: config.WorkspaceConfig, }, { - name: "no discovery and no IsUnifiedHost stays WorkspaceConfig", + name: "no discovery and no fallback stays WorkspaceConfig", cfg: &config.Config{ Host: "https://workspace.databricks.com", AccountID: "acct-123", @@ -97,7 +119,7 @@ func TestResolveConfigType(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - got := ResolveConfigType(tc.cfg) + got := ResolveConfigType(tc.cfg, tc.unifiedHostFallback) assert.Equal(t, tc.want, got) }) } diff --git a/libs/auth/credentials.go b/libs/auth/credentials.go index 7ab6eb2a85..10b5e40467 100644 --- a/libs/auth/credentials.go +++ b/libs/auth/credentials.go @@ -122,13 +122,15 @@ func (c CLICredentials) persistentAuth(ctx context.Context, opts ...u2m.Persiste } // authArgumentsFromConfig converts an SDK config to AuthArguments. +// The legacy IsUnifiedHost signal is no longer read from the SDK config +// (the field is being removed); DiscoveryURL is the primary signal, and +// cfg reaches us after EnsureResolved() so it's populated from .well-known. func authArgumentsFromConfig(cfg *config.Config) AuthArguments { return AuthArguments{ - Host: cfg.Host, - AccountID: cfg.AccountID, - WorkspaceID: cfg.WorkspaceID, - IsUnifiedHost: cfg.Experimental_IsUnifiedHost, - Profile: cfg.Profile, - DiscoveryURL: cfg.DiscoveryURL, + Host: cfg.Host, + AccountID: cfg.AccountID, + WorkspaceID: cfg.WorkspaceID, + Profile: cfg.Profile, + DiscoveryURL: cfg.DiscoveryURL, } } diff --git a/libs/auth/credentials_test.go b/libs/auth/credentials_test.go index 1bc70b63ab..7bee98c0e9 100644 --- a/libs/auth/credentials_test.go +++ b/libs/auth/credentials_test.go @@ -83,18 +83,18 @@ func TestAuthArgumentsFromConfig(t *testing.T) { { name: "all fields", cfg: &config.Config{ - Host: "https://myhost.com", - AccountID: "acc-123", - WorkspaceID: "ws-456", - Profile: "my-profile", - Experimental_IsUnifiedHost: true, + Host: "https://myhost.com", + AccountID: "acc-123", + WorkspaceID: "ws-456", + Profile: "my-profile", + DiscoveryURL: "https://myhost.com/oidc/accounts/acc-123/.well-known/oauth-authorization-server", }, want: AuthArguments{ - Host: "https://myhost.com", - AccountID: "acc-123", - WorkspaceID: "ws-456", - Profile: "my-profile", - IsUnifiedHost: true, + Host: "https://myhost.com", + AccountID: "acc-123", + WorkspaceID: "ws-456", + Profile: "my-profile", + DiscoveryURL: "https://myhost.com/oidc/accounts/acc-123/.well-known/oauth-authorization-server", }, }, } diff --git a/libs/auth/error.go b/libs/auth/error.go index 60c2e5ce7b..2ca8aa5f80 100644 --- a/libs/auth/error.go +++ b/libs/auth/error.go @@ -124,10 +124,10 @@ func writeReauthSteps(ctx context.Context, cfg *config.Config, b *strings.Builde return } oauthArg, argErr := AuthArguments{ - Host: cfg.Host, - AccountID: cfg.AccountID, - WorkspaceID: cfg.WorkspaceID, - IsUnifiedHost: cfg.Experimental_IsUnifiedHost, + Host: cfg.Host, + AccountID: cfg.AccountID, + WorkspaceID: cfg.WorkspaceID, + DiscoveryURL: cfg.DiscoveryURL, }.ToOAuthArgument() if argErr != nil { fmt.Fprint(b, "\n - Re-authenticate: databricks auth login") @@ -172,10 +172,9 @@ func BuildLoginCommand(ctx context.Context, profile string, arg u2m.OAuthArgumen } else { switch arg := arg.(type) { case u2m.UnifiedOAuthArgument: - // The --experimental-is-unified-host flag is redundant now that - // discovery handles routing, but kept for backward compatibility - // until the flag is fully removed. - cmd = append(cmd, "--host", arg.GetHost(), "--account-id", arg.GetAccountId(), "--experimental-is-unified-host") + // Discovery handles unified-host routing from --host + --account-id, + // so we no longer suggest --experimental-is-unified-host here. + cmd = append(cmd, "--host", arg.GetHost(), "--account-id", arg.GetAccountId()) case u2m.AccountOAuthArgument: cmd = append(cmd, "--host", arg.GetAccountHost(), "--account-id", arg.GetAccountId()) case u2m.WorkspaceOAuthArgument: diff --git a/libs/auth/error_test.go b/libs/auth/error_test.go index 52c739294b..1e724a4b25 100644 --- a/libs/auth/error_test.go +++ b/libs/auth/error_test.go @@ -228,20 +228,20 @@ func TestEnrichAuthError(t *testing.T) { "\n - Consider setting up a profile: databricks auth login --profile ", }, { - name: "401 with unified host and no profile", + name: "401 with unified host (resolved DiscoveryURL) and no profile", cfg: &config.Config{ - Host: "https://unified.cloud.databricks.com", - AccountID: "acc-123", - WorkspaceID: "ws-456", - AuthType: AuthTypeDatabricksCli, - Experimental_IsUnifiedHost: true, + Host: "https://unified.cloud.databricks.com", + AccountID: "acc-123", + WorkspaceID: "ws-456", + AuthType: AuthTypeDatabricksCli, + DiscoveryURL: "https://unified.cloud.databricks.com/oidc/accounts/acc-123/.well-known/oauth-authorization-server", }, statusCode: 401, wantMsg: "test error message\n" + "\nHost: https://unified.cloud.databricks.com" + "\nAuth type: OAuth (databricks-cli)" + "\n\nNext steps:" + - "\n - Re-authenticate: databricks auth login --host https://unified.cloud.databricks.com --account-id acc-123 --experimental-is-unified-host" + + "\n - Re-authenticate: databricks auth login --host https://unified.cloud.databricks.com --account-id acc-123" + "\n - Check your identity: databricks auth describe" + "\n - Consider setting up a profile: databricks auth login --profile ", }, From 66cd1969f63d5bdd71c7fb3e25b94847a8e070bd Mon Sep 17 00:00:00 2001 From: simon Date: Tue, 21 Apr 2026 12:44:42 +0200 Subject: [PATCH 2/2] auth: restore legacy unified-host fallback in credentials + reauth Addresses codex review findings on PR #5047. After dropping cfg.Experimental_IsUnifiedHost reads, CLICredentials.Configure and writeReauthSteps stopped honoring the INI fallback for profiles where .well-known is unreachable. This reads experimental_is_unified_host from the resolved profile's .databrickscfg section and threads it through AuthArguments.IsUnifiedHost so token cache keys and reauth suggestions continue to match. Also rewords the changelog and flag description from "no-op" to "deprecated but still honored as a routing fallback", which is what the code actually does. Co-authored-by: Isaac --- NEXT_CHANGELOG.md | 2 +- cmd/auth/auth.go | 2 +- libs/auth/credentials.go | 62 +++++++++++++++++++++++++++++------ libs/auth/credentials_test.go | 48 ++++++++++++++++++++++++++- libs/auth/error.go | 9 ++--- 5 files changed, 106 insertions(+), 17 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 3412a8f787..2381355147 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,7 +8,7 @@ * Added `--limit` flag to all paginated list commands for client-side result capping ([#4984](https://github.com/databricks/cli/pull/4984)). * Accept `yes` in addition to `y` for confirmation prompts, and show `[y/N]` to indicate that no is the default. -* Stop persisting `experimental_is_unified_host` to new profiles and ignore the `DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST` env var. Unified hosts are now detected automatically from `/.well-known/databricks-config`. Existing profiles with the key set continue to work; `--experimental-is-unified-host` remains as a deprecated no-op for this release. +* Stop persisting `experimental_is_unified_host` to new profiles and ignore the `DATABRICKS_EXPERIMENTAL_IS_UNIFIED_HOST` env var. Unified hosts are now detected automatically from `/.well-known/databricks-config`. Existing profiles with the key set continue to work via a legacy fallback; `--experimental-is-unified-host` is deprecated but still honored as a routing fallback for this release. ### Bundles * Remove `experimental-jobs-as-code` template, superseded by `pydabs` ([#4999](https://github.com/databricks/cli/pull/4999)). diff --git a/cmd/auth/auth.go b/cmd/auth/auth.go index b1f35e1ac6..a686786e18 100644 --- a/cmd/auth/auth.go +++ b/cmd/auth/auth.go @@ -28,7 +28,7 @@ GCP: https://docs.gcp.databricks.com/dev-tools/auth/index.html`, var authArguments auth.AuthArguments cmd.PersistentFlags().StringVar(&authArguments.Host, "host", "", "Databricks Host") cmd.PersistentFlags().StringVar(&authArguments.AccountID, "account-id", "", "Databricks Account ID") - cmd.PersistentFlags().BoolVar(&authArguments.IsUnifiedHost, "experimental-is-unified-host", false, "Deprecated: unified hosts are now detected automatically from /.well-known/databricks-config") + cmd.PersistentFlags().BoolVar(&authArguments.IsUnifiedHost, "experimental-is-unified-host", false, "Deprecated: unified hosts are now detected automatically from /.well-known/databricks-config. Still honored as a routing fallback.") cmd.PersistentFlags().StringVar(&authArguments.WorkspaceID, "workspace-id", "", "Databricks Workspace ID") cmd.AddCommand(newEnvCommand()) diff --git a/libs/auth/credentials.go b/libs/auth/credentials.go index 10b5e40467..32e41738ac 100644 --- a/libs/auth/credentials.go +++ b/libs/auth/credentials.go @@ -3,7 +3,10 @@ package auth import ( "context" "errors" + "path/filepath" + "strings" + "github.com/databricks/cli/libs/env" "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/config/credentials" "github.com/databricks/databricks-sdk-go/config/experimental/auth" @@ -93,7 +96,7 @@ func (c CLICredentials) Configure(ctx context.Context, cfg *config.Config) (cred if cfg.Host == "" { return nil, errNoHost } - oauthArg, err := authArgumentsFromConfig(cfg).ToOAuthArgument() + oauthArg, err := authArgumentsFromConfig(ctx, cfg).ToOAuthArgument() if err != nil { return nil, err } @@ -122,15 +125,54 @@ func (c CLICredentials) persistentAuth(ctx context.Context, opts ...u2m.Persiste } // authArgumentsFromConfig converts an SDK config to AuthArguments. -// The legacy IsUnifiedHost signal is no longer read from the SDK config -// (the field is being removed); DiscoveryURL is the primary signal, and -// cfg reaches us after EnsureResolved() so it's populated from .well-known. -func authArgumentsFromConfig(cfg *config.Config) AuthArguments { +// The SDK config no longer carries Experimental_IsUnifiedHost (the field is +// being removed). DiscoveryURL is the primary unified-host signal, populated +// by EnsureResolved() before this runs. For users on hosts where .well-known +// is unreachable, the signal is recovered from the legacy INI key by +// legacyUnifiedHostFromProfile so token cache keys continue to match. +func authArgumentsFromConfig(ctx context.Context, cfg *config.Config) AuthArguments { return AuthArguments{ - Host: cfg.Host, - AccountID: cfg.AccountID, - WorkspaceID: cfg.WorkspaceID, - Profile: cfg.Profile, - DiscoveryURL: cfg.DiscoveryURL, + Host: cfg.Host, + AccountID: cfg.AccountID, + WorkspaceID: cfg.WorkspaceID, + Profile: cfg.Profile, + DiscoveryURL: cfg.DiscoveryURL, + IsUnifiedHost: legacyUnifiedHostFromProfile(ctx, cfg), } } + +// legacyUnifiedHostFromProfile reads experimental_is_unified_host from the +// profile section of the resolved .databrickscfg. Best-effort: returns false +// on any error (missing config file, missing section, parse failure). +// +// This exists to carry the legacy unified-host signal forward after the SDK +// stopped populating cfg.Experimental_IsUnifiedHost from the INI key. Without +// it, OAuth cache-key generation regresses for profiles that set the key but +// sit behind a host where .well-known/databricks-config is unreachable. +func legacyUnifiedHostFromProfile(ctx context.Context, cfg *config.Config) bool { + if cfg.Profile == "" { + return false + } + path := cfg.ConfigFile + if path == "" { + path = env.Get(ctx, "DATABRICKS_CONFIG_FILE") + } + if path == "" { + home, err := env.UserHomeDir(ctx) + if err != nil { + return false + } + path = filepath.Join(home, ".databrickscfg") + } else if strings.HasPrefix(path, "~") { + home, err := env.UserHomeDir(ctx) + if err != nil { + return false + } + path = filepath.Join(home, path[1:]) + } + f, err := config.LoadFile(path) + if err != nil { + return false + } + return f.Section(cfg.Profile).Key("experimental_is_unified_host").MustBool(false) +} diff --git a/libs/auth/credentials_test.go b/libs/auth/credentials_test.go index 7bee98c0e9..322302e17e 100644 --- a/libs/auth/credentials_test.go +++ b/libs/auth/credentials_test.go @@ -4,12 +4,16 @@ import ( "context" "errors" "net/http" + "os" + "path/filepath" "slices" "testing" "github.com/databricks/databricks-sdk-go/config" "github.com/databricks/databricks-sdk-go/config/experimental/auth" "github.com/databricks/databricks-sdk-go/credentials/u2m" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "golang.org/x/oauth2" ) @@ -50,6 +54,10 @@ func TestCLICredentialsName(t *testing.T) { } func TestAuthArgumentsFromConfig(t *testing.T) { + // Point config file at a nonexistent path so legacyUnifiedHostFromProfile + // doesn't read the developer's real ~/.databrickscfg. + t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(t.TempDir(), "nonexistent.cfg")) + tests := []struct { name string cfg *config.Config @@ -101,7 +109,7 @@ func TestAuthArgumentsFromConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := authArgumentsFromConfig(tt.cfg) + got := authArgumentsFromConfig(t.Context(), tt.cfg) if got != tt.want { t.Errorf("want %v, got %v", tt.want, got) } @@ -109,6 +117,44 @@ func TestAuthArgumentsFromConfig(t *testing.T) { } } +func TestLegacyUnifiedHostFromProfile(t *testing.T) { + dir := t.TempDir() + cfgPath := filepath.Join(dir, ".databrickscfg") + require.NoError(t, os.WriteFile(cfgPath, []byte(` +[unified] +host = https://unified.example.com +account_id = acc-1 +experimental_is_unified_host = true + +[plain] +host = https://plain.example.com +`), 0o600)) + + cases := []struct { + name string + cfg *config.Config + envFile string + want bool + }{ + {name: "no profile", cfg: &config.Config{ConfigFile: cfgPath}, want: false}, + {name: "unified profile", cfg: &config.Config{Profile: "unified", ConfigFile: cfgPath}, want: true}, + {name: "plain profile", cfg: &config.Config{Profile: "plain", ConfigFile: cfgPath}, want: false}, + {name: "missing profile", cfg: &config.Config{Profile: "nope", ConfigFile: cfgPath}, want: false}, + {name: "unreadable file", cfg: &config.Config{Profile: "unified", ConfigFile: filepath.Join(dir, "nope.cfg")}, want: false}, + {name: "picks up DATABRICKS_CONFIG_FILE env", cfg: &config.Config{Profile: "unified"}, envFile: cfgPath, want: true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if tc.envFile != "" { + t.Setenv("DATABRICKS_CONFIG_FILE", tc.envFile) + } else { + t.Setenv("DATABRICKS_CONFIG_FILE", filepath.Join(dir, "nonexistent.cfg")) + } + assert.Equal(t, tc.want, legacyUnifiedHostFromProfile(t.Context(), tc.cfg)) + }) + } +} + func TestCLICredentialsConfigure(t *testing.T) { testErr := errors.New("test error") diff --git a/libs/auth/error.go b/libs/auth/error.go index 2ca8aa5f80..8c2cd09104 100644 --- a/libs/auth/error.go +++ b/libs/auth/error.go @@ -124,10 +124,11 @@ func writeReauthSteps(ctx context.Context, cfg *config.Config, b *strings.Builde return } oauthArg, argErr := AuthArguments{ - Host: cfg.Host, - AccountID: cfg.AccountID, - WorkspaceID: cfg.WorkspaceID, - DiscoveryURL: cfg.DiscoveryURL, + Host: cfg.Host, + AccountID: cfg.AccountID, + WorkspaceID: cfg.WorkspaceID, + DiscoveryURL: cfg.DiscoveryURL, + IsUnifiedHost: legacyUnifiedHostFromProfile(ctx, cfg), }.ToOAuthArgument() if argErr != nil { fmt.Fprint(b, "\n - Re-authenticate: databricks auth login")