diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index ede82f7779..2381355147 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 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/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..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, "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. Still honored as a routing fallback.") 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..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,13 +125,54 @@ func (c CLICredentials) persistentAuth(ctx context.Context, opts ...u2m.Persiste } // authArgumentsFromConfig converts an SDK config to AuthArguments. -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, - IsUnifiedHost: cfg.Experimental_IsUnifiedHost, 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 1bc70b63ab..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 @@ -83,25 +91,25 @@ 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", }, }, } 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 60c2e5ce7b..8c2cd09104 100644 --- a/libs/auth/error.go +++ b/libs/auth/error.go @@ -127,7 +127,8 @@ func writeReauthSteps(ctx context.Context, cfg *config.Config, b *strings.Builde Host: cfg.Host, AccountID: cfg.AccountID, WorkspaceID: cfg.WorkspaceID, - IsUnifiedHost: cfg.Experimental_IsUnifiedHost, + DiscoveryURL: cfg.DiscoveryURL, + IsUnifiedHost: legacyUnifiedHostFromProfile(ctx, cfg), }.ToOAuthArgument() if argErr != nil { fmt.Fprint(b, "\n - Re-authenticate: databricks auth login") @@ -172,10 +173,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 ", },