diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 00152d550e..f8dbd68987 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, `--workspace-id` to override the workspace routing identifier per call, and `{account_id}` substitution from the active profile's `account_id`. + ### Bundles ### Dependency updates diff --git a/acceptance/cmd/api/account-flag/out.requests.txt b/acceptance/cmd/api/account-flag/out.requests.txt new file mode 100644 index 0000000000..0dd6d7022f --- /dev/null +++ b/acceptance/cmd/api/account-flag/out.requests.txt @@ -0,0 +1,21 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin 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/out.test.toml b/acceptance/cmd/api/account-flag/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/account-flag/out.test.toml @@ -0,0 +1,5 @@ +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 0000000000..0967ef424b --- /dev/null +++ b/acceptance/cmd/api/account-flag/output.txt @@ -0,0 +1 @@ +{} diff --git a/acceptance/cmd/api/account-flag/script b/acceptance/cmd/api/account-flag/script new file mode 100644 index 0000000000..ed4dd74b0c --- /dev/null +++ b/acceptance/cmd/api/account-flag/script @@ -0,0 +1 @@ +$CLI api get /api/2.0/clusters/list --account diff --git a/acceptance/cmd/api/account-id-missing/out.requests.txt b/acceptance/cmd/api/account-id-missing/out.requests.txt new file mode 100644 index 0000000000..64b313d01e --- /dev/null +++ b/acceptance/cmd/api/account-id-missing/out.requests.txt @@ -0,0 +1,9 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} diff --git a/acceptance/cmd/api/account-id-missing/out.test.toml b/acceptance/cmd/api/account-id-missing/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/account-id-missing/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/account-id-missing/output.txt b/acceptance/cmd/api/account-id-missing/output.txt new file mode 100644 index 0000000000..44af079ab0 --- /dev/null +++ b/acceptance/cmd/api/account-id-missing/output.txt @@ -0,0 +1,5 @@ + +>>> errcode [CLI] api get /api/2.0/accounts/{account_id}/oauth2/published-app-integrations +Error: path contains {account_id} but no account_id is set on profile "" (set account_id in ~/.databrickscfg, export DATABRICKS_ACCOUNT_ID, or replace {account_id} in the path) + +Exit code: 1 diff --git a/acceptance/cmd/api/account-id-missing/script b/acceptance/cmd/api/account-id-missing/script new file mode 100644 index 0000000000..eba2e79868 --- /dev/null +++ b/acceptance/cmd/api/account-id-missing/script @@ -0,0 +1,5 @@ +# No DATABRICKS_ACCOUNT_ID; the testserver also doesn't set account_id in +# .well-known. The CLI must fail with the actionable error before any target +# API request is sent. (Config resolution still hits .well-known earlier in +# the lifecycle, which is expected.) +trace errcode $CLI api get '/api/2.0/accounts/{account_id}/oauth2/published-app-integrations' diff --git a/acceptance/cmd/api/account-id-substitution/out.requests.txt b/acceptance/cmd/api/account-id-substitution/out.requests.txt new file mode 100644 index 0000000000..0d779ee1b5 --- /dev/null +++ b/acceptance/cmd/api/account-id-substitution/out.requests.txt @@ -0,0 +1,21 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin cmd/api_get cmd-exec-id/[UUID] interactive/none auth/pat" + ] + }, + "method": "GET", + "path": "/api/2.0/accounts/abc-123/oauth2/published-app-integrations" +} diff --git a/acceptance/cmd/api/account-id-substitution/out.test.toml b/acceptance/cmd/api/account-id-substitution/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/account-id-substitution/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/api/account-id-substitution/output.txt b/acceptance/cmd/api/account-id-substitution/output.txt new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/acceptance/cmd/api/account-id-substitution/output.txt @@ -0,0 +1 @@ +{} diff --git a/acceptance/cmd/api/account-id-substitution/script b/acceptance/cmd/api/account-id-substitution/script new file mode 100644 index 0000000000..8e509e7408 --- /dev/null +++ b/acceptance/cmd/api/account-id-substitution/script @@ -0,0 +1,2 @@ +export DATABRICKS_ACCOUNT_ID=abc-123 +$CLI api get '/api/2.0/accounts/{account_id}/oauth2/published-app-integrations' diff --git a/acceptance/cmd/api/account-path/out.requests.txt b/acceptance/cmd/api/account-path/out.requests.txt new file mode 100644 index 0000000000..d143b64be9 --- /dev/null +++ b/acceptance/cmd/api/account-path/out.requests.txt @@ -0,0 +1,21 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin 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/out.test.toml b/acceptance/cmd/api/account-path/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/account-path/out.test.toml @@ -0,0 +1,5 @@ +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 0000000000..0967ef424b --- /dev/null +++ b/acceptance/cmd/api/account-path/output.txt @@ -0,0 +1 @@ +{} diff --git a/acceptance/cmd/api/account-path/script b/acceptance/cmd/api/account-path/script new file mode 100644 index 0000000000..67e46484e4 --- /dev/null +++ b/acceptance/cmd/api/account-path/script @@ -0,0 +1 @@ +$CLI api get /api/2.0/accounts/abc-123/network-policies diff --git a/acceptance/cmd/api/test.toml b/acceptance/cmd/api/test.toml new file mode 100644 index 0000000000..69e31b128f --- /dev/null +++ b/acceptance/cmd/api/test.toml @@ -0,0 +1,22 @@ +RecordRequests = true +IncludeRequestHeaders = ["Authorization", "User-Agent", "X-Databricks-Org-Id"] + +# 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.requests.txt b/acceptance/cmd/api/workspace-id-flag/out.requests.txt new file mode 100644 index 0000000000..7036250369 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/out.requests.txt @@ -0,0 +1,24 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin 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/out.test.toml b/acceptance/cmd/api/workspace-id-flag/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/out.test.toml @@ -0,0 +1,5 @@ +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 0000000000..0967ef424b --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/output.txt @@ -0,0 +1 @@ +{} diff --git a/acceptance/cmd/api/workspace-id-flag/script b/acceptance/cmd/api/workspace-id-flag/script new file mode 100644 index 0000000000..741c4048c8 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-flag/script @@ -0,0 +1 @@ +$CLI api get /api/2.0/clusters/list --workspace-id 999 diff --git a/acceptance/cmd/api/workspace-id-none/out.requests.txt b/acceptance/cmd/api/workspace-id-none/out.requests.txt new file mode 100644 index 0000000000..0dd6d7022f --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/out.requests.txt @@ -0,0 +1,21 @@ +{ + "headers": { + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin" + ] + }, + "method": "GET", + "path": "/.well-known/databricks-config" +} +{ + "headers": { + "Authorization": [ + "Bearer [DATABRICKS_TOKEN]" + ], + "User-Agent": [ + "cli/[DEV_VERSION] databricks-sdk-go/[SDK_VERSION] go/[GO_VERSION] os/darwin 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/out.test.toml b/acceptance/cmd/api/workspace-id-none/out.test.toml new file mode 100644 index 0000000000..d560f1de04 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/out.test.toml @@ -0,0 +1,5 @@ +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 0000000000..0967ef424b --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/output.txt @@ -0,0 +1 @@ +{} diff --git a/acceptance/cmd/api/workspace-id-none/script b/acceptance/cmd/api/workspace-id-none/script new file mode 100644 index 0000000000..692f06ba97 --- /dev/null +++ b/acceptance/cmd/api/workspace-id-none/script @@ -0,0 +1,12 @@ +# 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" < 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", + }, + } + 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)) + }) + } +} + +func TestSubstituteAccountID(t *testing.T) { + cases := []struct { + name string + path string + accountID string + profile string + want string + wantErrSubstring string + }{ + { + name: "placeholder absent leaves path unchanged", + path: "/api/2.0/clusters/list", + accountID: "abc-123", + profile: "DEFAULT", + want: "/api/2.0/clusters/list", + }, + { + name: "placeholder present + account_id set", + path: "/api/2.0/accounts/{account_id}/oauth2/published-app-integrations", + accountID: "abc-123", + profile: "DEFAULT", + want: "/api/2.0/accounts/abc-123/oauth2/published-app-integrations", + }, + { + name: "multiple placeholders all replaced", + path: "/api/2.0/accounts/{account_id}/workspaces/123/foo?ref=accounts/{account_id}", + accountID: "abc-123", + profile: "DEFAULT", + want: "/api/2.0/accounts/abc-123/workspaces/123/foo?ref=accounts/abc-123", + }, + { + name: "placeholder present + account_id empty -> error", + path: "/api/2.0/accounts/{account_id}/oauth2/published-app-integrations", + accountID: "", + profile: "DEFAULT", + wantErrSubstring: `{account_id}`, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, err := substituteAccountID(c.path, c.accountID, c.profile) + if c.wantErrSubstring != "" { + require.Error(t, err) + assert.Contains(t, err.Error(), c.wantErrSubstring) + assert.Contains(t, err.Error(), `profile "`+c.profile+`"`) + assert.Contains(t, err.Error(), "DATABRICKS_ACCOUNT_ID") + return + } + require.NoError(t, err) + assert.Equal(t, c.want, got) + }) + } +} diff --git a/cmd/api/internal/genpaths/classify.go b/cmd/api/internal/genpaths/classify.go index 142ab5b63b..10cf30472b 100644 --- a/cmd/api/internal/genpaths/classify.go +++ b/cmd/api/internal/genpaths/classify.go @@ -5,6 +5,7 @@ import ( "fmt" "go/ast" "go/token" + "slices" "strconv" "strings" ) @@ -86,15 +87,13 @@ func classifySprintf(call *ast.CallExpr) (classification, error) { return classification{class: classNotAccount}, nil } - for _, a := range call.Args[1:] { - if isAccountIDSource(a) { - return classification{class: classAccountAPI}, nil - } + if slices.ContainsFunc(call.Args[1:], isAccountIDSource) { + return classification{class: classAccountAPI}, nil } prefix := prefixUpToFirstVerb(template) if prefix == "" { - return classification{}, fmt.Errorf("Sprintf template %q has no format verb", template) + return classification{}, fmt.Errorf("fmt.Sprintf template %q has no format verb", template) } // Guard: a prefix ending exactly at "accounts/" would match every account // API under that family if it leaked into the deny-list. Refuse to emit diff --git a/cmd/api/internal/genpaths/main.go b/cmd/api/internal/genpaths/main.go index 0e963076ae..5dd797d06a 100644 --- a/cmd/api/internal/genpaths/main.go +++ b/cmd/api/internal/genpaths/main.go @@ -18,7 +18,6 @@ import ( "go/format" "go/parser" "go/token" - "log" "os" "os/exec" "path/filepath" @@ -31,7 +30,8 @@ const sdkModule = "github.com/databricks/databricks-sdk-go" func main() { if err := run(os.Stdout); err != nil { - log.Fatalf("genpaths: %v", err) + fmt.Fprintf(os.Stderr, "genpaths: %v\n", err) + os.Exit(1) } } @@ -112,6 +112,8 @@ func scanAST(fset *token.FileSet, f *ast.File, prefixSet, exactSet map[string]st exactSet[res.value] = struct{}{} case classWorkspaceProxyPrefix: prefixSet[res.value] = struct{}{} + case classNotAccount, classAccountAPI: + // nothing to record; not part of the deny-list } return true }