diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 00152d550ea..e992aa7ed9b 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -4,6 +4,8 @@ ### CLI +* `databricks api` now works against unified hosts. Adds `--account` to scope a call to the account API and `--workspace-id` to override the workspace routing identifier per call. A `?o=` query parameter on the path (the SPOG URL convention used by the Databricks UI) is also recognized as a per-call workspace override, so URLs pasted from the browser route correctly. + ### Bundles ### Dependency updates diff --git a/acceptance/cmd/api/account-flag/out.test.toml b/acceptance/cmd/api/account-flag/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/account-flag/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/account-flag/output.txt b/acceptance/cmd/api/account-flag/output.txt new file mode 100644 index 00000000000..c165bf2af88 --- /dev/null +++ b/acceptance/cmd/api/account-flag/output.txt @@ -0,0 +1,15 @@ +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/account-flag/script b/acceptance/cmd/api/account-flag/script new file mode 100644 index 00000000000..de2d4de92b7 --- /dev/null +++ b/acceptance/cmd/api/account-flag/script @@ -0,0 +1,2 @@ +MSYS_NO_PATHCONV=1 $CLI api get /api/2.0/clusters/list --account +trace print_requests.py --get //api/2.0/clusters/list | contains.py "!X-Databricks-Org-Id" diff --git a/acceptance/cmd/api/account-path/out.test.toml b/acceptance/cmd/api/account-path/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/account-path/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/account-path/output.txt b/acceptance/cmd/api/account-path/output.txt new file mode 100644 index 00000000000..1afd36c01d3 --- /dev/null +++ b/acceptance/cmd/api/account-path/output.txt @@ -0,0 +1,15 @@ +{} + +>>> print_requests.py --get //api/2.0/accounts/abc-123/network-policies +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ] + }, + "method": "GET", + "path": "/api/2.0/accounts/abc-123/network-policies" +} diff --git a/acceptance/cmd/api/account-path/script b/acceptance/cmd/api/account-path/script new file mode 100644 index 00000000000..6cc97637695 --- /dev/null +++ b/acceptance/cmd/api/account-path/script @@ -0,0 +1,2 @@ +MSYS_NO_PATHCONV=1 $CLI api get /api/2.0/accounts/abc-123/network-policies +trace print_requests.py --get //api/2.0/accounts/abc-123/network-policies | contains.py "!X-Databricks-Org-Id" diff --git a/acceptance/cmd/api/test.toml b/acceptance/cmd/api/test.toml new file mode 100644 index 00000000000..11d83c3f486 --- /dev/null +++ b/acceptance/cmd/api/test.toml @@ -0,0 +1,40 @@ +RecordRequests = true +IncludeRequestHeaders = ["Authorization", "User-Agent", "X-Databricks-Org-Id"] + +# Normalize OS-dependent and CI-only User-Agent segments so the recorded +# requests are stable across local macOS/Linux runs and GitHub Actions. +[[Repls]] +Old = '(linux|darwin|windows)' +New = '[OS]' + +[[Repls]] +Old = " cicd/[A-Za-z0-9.-]+" +New = "" + +[[Repls]] +Old = " upstream/[A-Za-z0-9.-]+" +New = "" + +[[Repls]] +Old = " upstream-version/[A-Za-z0-9.-]+" +New = "" + +# Common stubs for paths used across variants. Each returns an empty JSON +# object; the variants assert on the *recorded request* (out.requests.txt), +# not on the response body, so any well-formed JSON is fine. + +[[Server]] +Pattern = "GET /api/2.0/clusters/list" +Response.Body = '{}' + +[[Server]] +Pattern = "GET /api/2.0/accounts/abc-123/network-policies" +Response.Body = '{}' + +[[Server]] +Pattern = "GET /api/2.0/accounts/abc-123/oauth2/published-app-integrations" +Response.Body = '{}' + +[[Server]] +Pattern = "GET /api/2.0/preview/accounts/access-control/rule-sets" +Response.Body = '{}' diff --git a/acceptance/cmd/api/workspace-id-flag/out.test.toml b/acceptance/cmd/api/workspace-id-flag/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/workspace-id-flag/output.txt b/acceptance/cmd/api/workspace-id-flag/output.txt new file mode 100644 index 00000000000..5ff264fa554 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/output.txt @@ -0,0 +1,18 @@ +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Org-Id": [ + "999" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/workspace-id-flag/script b/acceptance/cmd/api/workspace-id-flag/script new file mode 100644 index 00000000000..83664ba8662 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/script @@ -0,0 +1,2 @@ +MSYS_NO_PATHCONV=1 $CLI api get /api/2.0/clusters/list --workspace-id 999 +trace print_requests.py --get //api/2.0/clusters/list | contains.py "X-Databricks-Org-Id" "999" diff --git a/acceptance/cmd/api/workspace-id-from-query/out.test.toml b/acceptance/cmd/api/workspace-id-from-query/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-from-query/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/workspace-id-from-query/output.txt b/acceptance/cmd/api/workspace-id-from-query/output.txt new file mode 100644 index 00000000000..7a72f8de473 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-from-query/output.txt @@ -0,0 +1,21 @@ +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Org-Id": [ + "999" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list", + "q": { + "o": "999" + } +} diff --git a/acceptance/cmd/api/workspace-id-from-query/script b/acceptance/cmd/api/workspace-id-from-query/script new file mode 100644 index 00000000000..fd3fa0e151b --- /dev/null +++ b/acceptance/cmd/api/workspace-id-from-query/script @@ -0,0 +1,2 @@ +MSYS_NO_PATHCONV=1 $CLI api get "/api/2.0/clusters/list?o=999" +trace print_requests.py --get //api/2.0/clusters/list | contains.py "X-Databricks-Org-Id" "999" diff --git a/acceptance/cmd/api/workspace-id-none/out.test.toml b/acceptance/cmd/api/workspace-id-none/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/workspace-id-none/output.txt b/acceptance/cmd/api/workspace-id-none/output.txt new file mode 100644 index 00000000000..c165bf2af88 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/output.txt @@ -0,0 +1,15 @@ +{} + +>>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/workspace-id-none/script b/acceptance/cmd/api/workspace-id-none/script new file mode 100644 index 00000000000..4ecd00c6768 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/script @@ -0,0 +1,13 @@ +# Profile with workspace_id = none overrides the host-metadata back-fill. +# The CLI must strip the sentinel before the header decision; the recorded +# request should not carry the routing identifier. +sethome "./home" +cat > "./home/.databrickscfg" <>> print_requests.py --get //api/2.0/clusters/list +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Org-Id": [ + "[NUMID]" + ] + }, + "method": "GET", + "path": "/api/2.0/clusters/list" +} diff --git a/acceptance/cmd/api/workspace-path/script b/acceptance/cmd/api/workspace-path/script new file mode 100644 index 00000000000..4e5bb35c4be --- /dev/null +++ b/acceptance/cmd/api/workspace-path/script @@ -0,0 +1,2 @@ +MSYS_NO_PATHCONV=1 $CLI api get /api/2.0/clusters/list +trace print_requests.py --get //api/2.0/clusters/list | contains.py "X-Databricks-Org-Id" diff --git a/acceptance/cmd/api/workspace-proxy-regression/out.test.toml b/acceptance/cmd/api/workspace-proxy-regression/out.test.toml new file mode 100644 index 00000000000..f784a183258 --- /dev/null +++ b/acceptance/cmd/api/workspace-proxy-regression/out.test.toml @@ -0,0 +1,3 @@ +Local = true +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/workspace-proxy-regression/output.txt b/acceptance/cmd/api/workspace-proxy-regression/output.txt new file mode 100644 index 00000000000..c98486a15e1 --- /dev/null +++ b/acceptance/cmd/api/workspace-proxy-regression/output.txt @@ -0,0 +1,18 @@ +{} + +>>> print_requests.py --get //api/2.0/preview/accounts/access-control/rule-sets +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/[OS] cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ], + "X-Databricks-Org-Id": [ + "[NUMID]" + ] + }, + "method": "GET", + "path": "/api/2.0/preview/accounts/access-control/rule-sets" +} diff --git a/acceptance/cmd/api/workspace-proxy-regression/script b/acceptance/cmd/api/workspace-proxy-regression/script new file mode 100644 index 00000000000..39ab661f4f1 --- /dev/null +++ b/acceptance/cmd/api/workspace-proxy-regression/script @@ -0,0 +1,5 @@ +# Workspace-routed proxy under accounts/. The deny-list must keep this from +# being misclassified as account-scope, so the routing identifier should be +# present on the recorded request. +MSYS_NO_PATHCONV=1 $CLI api get /api/2.0/preview/accounts/access-control/rule-sets +trace print_requests.py --get //api/2.0/preview/accounts/access-control/rule-sets | contains.py "X-Databricks-Org-Id" diff --git a/cmd/api/api.go b/cmd/api/api.go index 057c8f22468..823a6a4b663 100644 --- a/cmd/api/api.go +++ b/cmd/api/api.go @@ -1,11 +1,15 @@ package api import ( + "errors" "fmt" "net/http" + "net/url" + "regexp" "strings" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/auth" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" "github.com/databricks/databricks-sdk-go/client" @@ -13,6 +17,26 @@ import ( "github.com/spf13/cobra" ) +const ( + // orgIDHeader is the workspace routing identifier sent on workspace-scope + // requests against unified hosts. Generated SDK service methods set this + // per-call when cfg.WorkspaceID is populated; we mirror the same idiom. + orgIDHeader = "X-Databricks-Org-Id" + + // orgIDQueryParam is the SPOG (single-page-of-glass) URL convention used + // by the Databricks UI: "?o=" identifies the workspace a URL + // targets. When present on the path, we treat it as a per-call override + // for the workspace routing identifier so that pasted SPOG URLs route + // correctly without requiring --workspace-id. + orgIDQueryParam = "o" +) + +// accountSegmentRe matches a non-empty segment immediately after "accounts/", +// anchored at the start of the path or after a "/". Account-ID shape is +// deliberately opaque; the workspace-proxy list carves out SDK proxies that +// also live under /accounts/. +var accountSegmentRe = regexp.MustCompile(`(^|/)accounts/[^/]+`) + func New() *cobra.Command { cmd := &cobra.Command{ Use: "api", @@ -32,7 +56,11 @@ func New() *cobra.Command { } func makeCommand(method string) *cobra.Command { - var payload flags.JsonFlag + var ( + payload flags.JsonFlag + forceAccount bool + workspaceIDFlag string + ) command := &cobra.Command{ Use: strings.ToLower(method) + " PATH", @@ -60,8 +88,23 @@ func makeCommand(method string) *cobra.Command { return err } - var response any + orgID, err := resolveOrgID( + forceAccount, + workspaceIDFlag, + cmd.Flags().Changed("workspace-id"), + normalizeWorkspaceID(cfg.WorkspaceID), + path, + ) + if err != nil { + return err + } + headers := map[string]string{"Content-Type": "application/json"} + if orgID != "" { + headers[orgIDHeader] = orgID + } + + var response any err = api.Do(cmd.Context(), method, path, headers, nil, request, &response) if err != nil { return err @@ -71,5 +114,87 @@ func makeCommand(method string) *cobra.Command { } command.Flags().Var(&payload, "json", `either inline JSON string or @path/to/file.json with request body`) + command.Flags().BoolVar(&forceAccount, "account", false, + "Treat this call as account-scoped (skip the workspace routing identifier). Mutually exclusive with --workspace-id.") + command.Flags().StringVar(&workspaceIDFlag, "workspace-id", "", + "Override the workspace routing identifier on this call. Mutually exclusive with --account.") return command } + +// normalizeWorkspaceID strips the CLI-only WorkspaceIDNone sentinel so the +// SDK's idiomatic "if cfg.WorkspaceID != \"\"" check produces the right call +// shape. The CLI persists "none" in .databrickscfg to mark profiles where the +// user explicitly skipped workspace selection; the SDK does not know about +// this sentinel and would otherwise send the literal "none" as a routing +// identifier. +func normalizeWorkspaceID(workspaceID string) string { + if workspaceID == auth.WorkspaceIDNone { + return "" + } + return workspaceID +} + +// hasAccountSegment reports whether path is an account-scope API. The match +// runs on URL.Path, so query strings and fragments containing "/accounts/" +// can't trigger a false positive. Returns false for paths that match a known +// workspace-routed proxy from the proxy path tables. +func hasAccountSegment(rawPath string) (bool, error) { + u, err := url.Parse(rawPath) + if err != nil { + return false, fmt.Errorf("parse path: %w", err) + } + p := u.Path + if isWorkspaceProxyPath(p) { + return false, nil + } + return accountSegmentRe.MatchString(p), nil +} + +// extractOrgIDFromQuery returns the value of the "o" query parameter on path +// (the SPOG URL convention), or "" if absent or empty. +func extractOrgIDFromQuery(rawPath string) (string, error) { + u, err := url.Parse(rawPath) + if err != nil { + return "", fmt.Errorf("parse path: %w", err) + } + return u.Query().Get(orgIDQueryParam), nil +} + +// resolveOrgID picks the value (if any) for the workspace routing identifier +// based on flags, the resolved profile, and the path shape. Returns "" when +// no header should be sent. +func resolveOrgID( + forceAccount bool, + workspaceIDFlag string, + workspaceIDFlagSet bool, + cfgWorkspaceID string, + path string, +) (string, error) { + if forceAccount && workspaceIDFlagSet { + return "", errors.New("--account and --workspace-id are mutually exclusive") + } + if forceAccount { + return "", nil + } + if workspaceIDFlagSet { + if workspaceIDFlag == "" { + return "", errors.New("--workspace-id requires a value; use --account to scope this call to the account API") + } + return workspaceIDFlag, nil + } + orgIDFromQuery, err := extractOrgIDFromQuery(path) + if err != nil { + return "", err + } + if orgIDFromQuery != "" { + return orgIDFromQuery, nil + } + isAccount, err := hasAccountSegment(path) + if err != nil { + return "", err + } + if isAccount { + return "", nil + } + return cfgWorkspaceID, nil +} diff --git a/cmd/api/api_test.go b/cmd/api/api_test.go new file mode 100644 index 00000000000..69cd28fe5fe --- /dev/null +++ b/cmd/api/api_test.go @@ -0,0 +1,226 @@ +package api + +import ( + "testing" + + "github.com/databricks/cli/libs/auth" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHasAccountSegment(t *testing.T) { + cases := []struct { + name string + path string + want bool + }{ + {"account UUID", "/api/2.0/accounts/123e4567-e89b-12d3-a456-426614174000/ip-access-lists", true}, + {"AIP resource-name shape", "/api/networking/v1/accounts/123e4567-e89b-12d3-a456-426614174000/endpoints/abc", true}, + {"iamv2 account API", "/api/2.0/identity/accounts/123e4567-e89b-12d3-a456-426614174000/workspaces/123/workspaceAccessDetails/abc", true}, + {"non-UUID account ID", "/api/2.0/accounts/abc/foo", true}, + {"hyphenated short ID", "/api/2.0/accounts/abc-123/network-policies", true}, + {"substituted any-shape ID", "/api/2.0/accounts/some-account/oauth2/published-app-integrations", true}, + + {"deny-listed exact rule-sets", "/api/2.0/preview/accounts/access-control/rule-sets", false}, + {"deny-listed exact assignable-roles", "/api/2.0/preview/accounts/access-control/assignable-roles", false}, + {"exact-list miss falls to regex (rule-sets/foo)", "/api/2.0/preview/accounts/access-control/rule-sets/foo", true}, + {"exact-list miss falls to regex (assignable-roles-extra)", "/api/2.0/preview/accounts/access-control/assignable-roles-extra", true}, + {"deny-listed prefix servicePrincipals", "/api/2.0/accounts/servicePrincipals/abc-123/credentials/secrets", false}, + + {"no accounts segment", "/api/2.0/clusters/list", false}, + {"segment ends in accounts (boundary)", "/api/2.0/some-accounts/abc/foo", false}, + + {"query string preserved on match", "/api/2.0/accounts/abc-123?include=foo", true}, + {"query string with /accounts/ does not match", "/api/2.0/clusters/list?next=/accounts/foo", false}, + {"fragment with accounts/ does not match", "/api/2.0/clusters/list#accounts/foo", false}, + + {"absolute URL, account path", "https://ignored.example.com/api/2.0/accounts/abc/foo?include=x", true}, + {"absolute URL, query-string accounts/ does not match", "https://ignored.example.com/api/2.0/clusters/list?next=/accounts/foo", false}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := hasAccountSegment(c.path) + require.NoError(t, err) + assert.Equal(t, c.want, got) + }) + } +} + +func TestExtractOrgIDFromQuery(t *testing.T) { + cases := []struct { + name string + path string + want string + }{ + {"no query string", "/api/2.0/clusters/list", ""}, + {"o param present", "/api/2.2/jobs/list?o=7474644166319138", "7474644166319138"}, + {"o param empty", "/api/2.0/clusters/list?o=", ""}, + {"o among other params first", "/api/2.0/clusters/list?o=123&foo=bar", "123"}, + {"o among other params last", "/api/2.0/clusters/list?foo=bar&o=123", "123"}, + {"unrelated o-prefixed param ignored", "/api/2.0/clusters/list?other=1", ""}, + {"absolute URL", "https://example.com/api/2.0/clusters/list?o=42", "42"}, + {"first value wins on duplicate", "/api/2.0/clusters/list?o=1&o=2", "1"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := extractOrgIDFromQuery(c.path) + require.NoError(t, err) + assert.Equal(t, c.want, got) + }) + } +} + +func TestResolveOrgID(t *testing.T) { + const ( + workspacePath = "/api/2.0/clusters/list" + accountPath = "/api/2.0/accounts/abc-123/network-policies" + proxyPath = "/api/2.0/preview/accounts/access-control/rule-sets" + spogPath = "/api/2.2/jobs/list?o=7474644166319138" + spogAccountPath = "/api/2.0/accounts/abc-123/network-policies?o=7474644166319138" + spogWorkspaceID = "7474644166319138" + resolvedWSID = "900800700600" + flagWSID = "999" + ) + + cases := []struct { + name string + forceAccount bool + workspaceIDFlag string + flagSet bool + cfgWorkspaceID string + path string + want string + wantErrSubstring string + }{ + { + name: "empty WorkspaceID + workspace path -> no identifier", + cfgWorkspaceID: "", + path: workspacePath, + want: "", + }, + { + name: "WorkspaceID set + workspace path -> sends identifier", + cfgWorkspaceID: resolvedWSID, + path: workspacePath, + want: resolvedWSID, + }, + { + name: "WorkspaceID set + account path -> no identifier (auto-detect)", + cfgWorkspaceID: resolvedWSID, + path: accountPath, + want: "", + }, + { + name: "WorkspaceID set + workspace-routed proxy under accounts/", + cfgWorkspaceID: resolvedWSID, + path: proxyPath, + want: resolvedWSID, + }, + { + name: "--account on workspace path", + forceAccount: true, + cfgWorkspaceID: resolvedWSID, + path: workspacePath, + want: "", + }, + { + name: "--workspace-id overrides resolved value", + workspaceIDFlag: flagWSID, + flagSet: true, + cfgWorkspaceID: resolvedWSID, + path: workspacePath, + want: flagWSID, + }, + { + name: "--workspace-id on account path still overrides", + workspaceIDFlag: flagWSID, + flagSet: true, + cfgWorkspaceID: resolvedWSID, + path: accountPath, + want: flagWSID, + }, + { + name: "--workspace-id empty value -> error", + workspaceIDFlag: "", + flagSet: true, + cfgWorkspaceID: resolvedWSID, + path: workspacePath, + wantErrSubstring: "--workspace-id requires a value", + }, + { + name: "--account and --workspace-id both set -> error", + forceAccount: true, + workspaceIDFlag: flagWSID, + flagSet: true, + cfgWorkspaceID: resolvedWSID, + path: workspacePath, + wantErrSubstring: "mutually exclusive", + }, + { + name: "?o= sets identifier when no flag and no profile WorkspaceID", + cfgWorkspaceID: "", + path: spogPath, + want: spogWorkspaceID, + }, + { + name: "?o= overrides profile WorkspaceID", + cfgWorkspaceID: resolvedWSID, + path: spogPath, + want: spogWorkspaceID, + }, + { + name: "--workspace-id wins over ?o=", + workspaceIDFlag: flagWSID, + flagSet: true, + cfgWorkspaceID: resolvedWSID, + path: spogPath, + want: flagWSID, + }, + { + name: "--account wins over ?o=", + forceAccount: true, + cfgWorkspaceID: resolvedWSID, + path: spogPath, + want: "", + }, + { + name: "?o= on /accounts/ path still routes to that workspace", + cfgWorkspaceID: "", + path: spogAccountPath, + want: spogWorkspaceID, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := resolveOrgID(c.forceAccount, c.workspaceIDFlag, c.flagSet, c.cfgWorkspaceID, c.path) + if c.wantErrSubstring != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), c.wantErrSubstring) + return + } + require.NoError(t, err) + assert.Equal(t, c.want, got) + }) + } +} + +// TestNormalizeWorkspaceID covers the helper that strips the CLI-only +// WorkspaceIDNone sentinel. RunE calls this directly before resolveOrgID, so +// a regression here would surface as the literal "none" being sent on the +// wire. +func TestNormalizeWorkspaceID(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"sentinel stripped to empty", auth.WorkspaceIDNone, ""}, + {"empty passes through", "", ""}, + {"normal value passes through", "900800700600", "900800700600"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, normalizeWorkspaceID(c.in)) + }) + } +} diff --git a/cmd/api/paths.go b/cmd/api/paths.go new file mode 100644 index 00000000000..67b301f84d7 --- /dev/null +++ b/cmd/api/paths.go @@ -0,0 +1,29 @@ +package api + +import "strings" + +// workspaceProxyPrefixes lists SDK endpoints that live under accounts/ but +// route to the workspace gateway. Keep this list in sync with workspace-routed +// proxy APIs in the pinned SDK. +var workspaceProxyPrefixes = []string{ + "/api/2.0/accounts/servicePrincipals/", +} + +// workspaceProxyExact lists literal SDK endpoints that live under accounts/ but +// route to the workspace gateway. +var workspaceProxyExact = map[string]struct{}{ + "/api/2.0/preview/accounts/access-control/assignable-roles": {}, + "/api/2.0/preview/accounts/access-control/rule-sets": {}, +} + +func isWorkspaceProxyPath(path string) bool { + if _, ok := workspaceProxyExact[path]; ok { + return true + } + for _, prefix := range workspaceProxyPrefixes { + if strings.HasPrefix(path, prefix) { + return true + } + } + return false +} diff --git a/cmd/api/paths_test.go b/cmd/api/paths_test.go new file mode 100644 index 00000000000..c14da6b3ccc --- /dev/null +++ b/cmd/api/paths_test.go @@ -0,0 +1,52 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestIsWorkspaceProxyPath(t *testing.T) { + cases := []struct { + name string + path string + want bool + }{ + { + name: "assignable roles proxy", + path: "/api/2.0/preview/accounts/access-control/assignable-roles", + want: true, + }, + { + name: "rule sets proxy", + path: "/api/2.0/preview/accounts/access-control/rule-sets", + want: true, + }, + { + name: "service principal secrets proxy", + path: "/api/2.0/accounts/servicePrincipals/spn-123/credentials/secrets", + want: true, + }, + { + name: "account service principal secrets path has account id segment", + path: "/api/2.0/accounts/abc-123/servicePrincipals/spn-123/credentials/secrets", + want: false, + }, + { + name: "rule sets child is not part of exact proxy entry", + path: "/api/2.0/preview/accounts/access-control/rule-sets/foo", + want: false, + }, + { + name: "workspace path", + path: "/api/2.0/clusters/list", + want: false, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, isWorkspaceProxyPath(c.path)) + }) + } +}