From ceb79b5f0da9e62e7b31baea7d3ff0617cd0228d Mon Sep 17 00:00:00 2001 From: James Broadhead Date: Tue, 28 Apr 2026 15:33:40 +0000 Subject: [PATCH 1/3] postgres: add --json body example to create-role help MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `databricks postgres create-role`'s `--json` flag binds to the inner Role object (CreateRoleRequest.Role, JSON-tagged "role"), so users supply `spec`/`name`/etc. directly. Without an example this is non-obvious: - Auto-generated help leaves `// TODO: complex arg: spec` with no flag hint, so the only way to set spec fields is through `--json`. - If a user wraps the body in `{"role": ...}` (matching the wire format the SDK marshals to), the CLI strips `role` as unknown and ships an empty body. The server then returns a generic `Field 'role' is required and must contain at least one subfield with a non-default value` — which is hard to act on. Adds a curated override that appends a concrete service-principal-role example to `cmd.Long`, plus a short note on the wrapping pitfall. Same pattern (auto-gen TODO `spec`/`status`, opaque error on bad body) exists for create-endpoint, create-branch, create-project, and create-database. Holding off on those until this approach is approved. Co-authored-by: Isaac --- cmd/workspace/postgres/overrides.go | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 cmd/workspace/postgres/overrides.go diff --git a/cmd/workspace/postgres/overrides.go b/cmd/workspace/postgres/overrides.go new file mode 100644 index 00000000000..0ec8bff8807 --- /dev/null +++ b/cmd/workspace/postgres/overrides.go @@ -0,0 +1,34 @@ +package postgres + +import ( + "github.com/databricks/databricks-sdk-go/service/postgres" + "github.com/spf13/cobra" +) + +// createRoleOverride appends an example body to the auto-generated help. +// The --json flag binds to the inner Role object (CreateRoleRequest.Role, +// JSON-tagged "role"), so users supply spec/name/etc. directly. Without an +// example, the auto-generated `// TODO: complex arg: spec` flags leave no +// hint about the body shape and the API's "Field 'role' is required" error +// is unhelpful when the request body is wrong. +func createRoleOverride(createRoleCmd *cobra.Command, _ *postgres.CreateRoleRequest) { + createRoleCmd.Long += ` + + Body shape (passed via --json): fields go directly on the Role object. + Do not wrap them in '{"role": ...}' — the CLI will strip the unknown + outer key and the server will reject the empty body with "Field 'role' + is required". + + Example — create a service-principal-backed role: + + databricks postgres create-role projects//branches/ \ + --role-id \ + --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "", "auth_method": "LAKEBASE_OAUTH_V1", "membership_roles": ["DATABRICKS_SUPERUSER"]}}' + + See databricks-sdk-go/service/postgres.RoleRoleSpec for the full set of + spec fields.` +} + +func init() { + createRoleOverrides = append(createRoleOverrides, createRoleOverride) +} From 5fbd72462ddb4872e9a3396f67ccfad7f5069112 Mon Sep 17 00:00:00 2001 From: James Broadhead Date: Tue, 28 Apr 2026 16:33:17 +0000 Subject: [PATCH 2/3] docs(postgres): omit DATABRICKS_SUPERUSER from create-role help example The previous example granted DATABRICKS_SUPERUSER, normalizing over-privileged service-principal roles. Drop membership_roles from the example so the default copy-paste path follows least privilege; add a note pointing to separate SQL grants and noting when DATABRICKS_SUPERUSER is appropriate. Co-authored-by: Isaac --- cmd/workspace/postgres/overrides.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cmd/workspace/postgres/overrides.go b/cmd/workspace/postgres/overrides.go index 0ec8bff8807..7ea8cbd102e 100644 --- a/cmd/workspace/postgres/overrides.go +++ b/cmd/workspace/postgres/overrides.go @@ -23,7 +23,13 @@ func createRoleOverride(createRoleCmd *cobra.Command, _ *postgres.CreateRoleRequ databricks postgres create-role projects//branches/ \ --role-id \ - --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "", "auth_method": "LAKEBASE_OAUTH_V1", "membership_roles": ["DATABRICKS_SUPERUSER"]}}' + --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "", "auth_method": "LAKEBASE_OAUTH_V1"}}' + + The example omits 'membership_roles' so the role starts with default + privileges only — grant database/schema/table access separately via + SQL, following least privilege. Set 'membership_roles' (e.g. + ["DATABRICKS_SUPERUSER"]) only when broad administrative access is + intentional. See databricks-sdk-go/service/postgres.RoleRoleSpec for the full set of spec fields.` From 4ff71ddb41a4cff7ebdaa35eab80c833f7dc5575 Mon Sep 17 00:00:00 2001 From: James Broadhead Date: Tue, 28 Apr 2026 17:29:19 +0000 Subject: [PATCH 3/3] fix(postgres): reject wrapped {"role":...} JSON client-side on create-role The --json flag binds to the inner Role object, so a top-level {"role": {...}} wrapper produced an "unknown field: role" warning, an empty body, and a confusing server error: "Field 'role' is required". Add a PreRunE on create-role that peeks at the raw --json bytes via a new flags.JsonFlag.Raw() accessor and rejects any object with a top- level "role" key, returning an actionable error that points to the correct shape. Update the help text to reflect the new behavior and add unit-tests covering wrapped, unwrapped, empty, non-object, and no-flag cases. Co-authored-by: Isaac --- cmd/workspace/postgres/overrides.go | 61 ++++++++++++++++++++++-- cmd/workspace/postgres/overrides_test.go | 50 +++++++++++++++++++ libs/flags/json_flag.go | 7 +++ 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 cmd/workspace/postgres/overrides_test.go diff --git a/cmd/workspace/postgres/overrides.go b/cmd/workspace/postgres/overrides.go index 7ea8cbd102e..26bd0152145 100644 --- a/cmd/workspace/postgres/overrides.go +++ b/cmd/workspace/postgres/overrides.go @@ -1,23 +1,39 @@ package postgres import ( + "encoding/json" + "errors" + "strings" + + "github.com/databricks/cli/libs/flags" "github.com/databricks/databricks-sdk-go/service/postgres" "github.com/spf13/cobra" ) -// createRoleOverride appends an example body to the auto-generated help. +// createRoleOverride appends an example body to the auto-generated help and +// rejects wrapped {"role": ...} bodies with a clear client-side error. // The --json flag binds to the inner Role object (CreateRoleRequest.Role, // JSON-tagged "role"), so users supply spec/name/etc. directly. Without an // example, the auto-generated `// TODO: complex arg: spec` flags leave no // hint about the body shape and the API's "Field 'role' is required" error // is unhelpful when the request body is wrong. func createRoleOverride(createRoleCmd *cobra.Command, _ *postgres.CreateRoleRequest) { + prevPreRunE := createRoleCmd.PreRunE + createRoleCmd.PreRunE = func(cmd *cobra.Command, args []string) error { + if err := rejectWrappedRoleJSON(cmd); err != nil { + return err + } + if prevPreRunE != nil { + return prevPreRunE(cmd, args) + } + return nil + } + createRoleCmd.Long += ` Body shape (passed via --json): fields go directly on the Role object. - Do not wrap them in '{"role": ...}' — the CLI will strip the unknown - outer key and the server will reject the empty body with "Field 'role' - is required". + Do not wrap them in '{"role": ...}' — the CLI rejects wrapped bodies + client-side with a hint pointing to the right shape. Example — create a service-principal-backed role: @@ -35,6 +51,43 @@ func createRoleOverride(createRoleCmd *cobra.Command, _ *postgres.CreateRoleRequ spec fields.` } +// rejectWrappedRoleJSON returns a clear error when --json is a top-level +// object containing a "role" key. Without this guard the generated unmarshal +// strips the unknown outer "role" field with a warning and ships an empty +// body, and the server rejects with a confusing "Field 'role' is required" +// message. +func rejectWrappedRoleJSON(cmd *cobra.Command) error { + flag := cmd.Flags().Lookup("json") + if flag == nil { + return nil + } + jf, ok := flag.Value.(*flags.JsonFlag) + if !ok { + return nil + } + raw := jf.Raw() + if len(raw) == 0 { + return nil + } + var top map[string]json.RawMessage + if err := json.Unmarshal(raw, &top); err != nil { + return nil //nolint:nilerr // defer non-object inputs to the generated unmarshal so its diagnostics render + } + if _, hasRole := top["role"]; hasRole { + return errors.New(strings.TrimSpace(` +--json should NOT be wrapped in '{"role": ...}'. + +The flag binds to the inner Role object — supply 'spec'/'name'/etc. +directly. Example: + + databricks postgres create-role projects//branches/ \ + --role-id \ + --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "", "auth_method": "LAKEBASE_OAUTH_V1"}}' +`)) + } + return nil +} + func init() { createRoleOverrides = append(createRoleOverrides, createRoleOverride) } diff --git a/cmd/workspace/postgres/overrides_test.go b/cmd/workspace/postgres/overrides_test.go new file mode 100644 index 00000000000..d94aefb40a7 --- /dev/null +++ b/cmd/workspace/postgres/overrides_test.go @@ -0,0 +1,50 @@ +package postgres + +import ( + "testing" + + "github.com/databricks/cli/libs/flags" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func cmdWithJSON(t *testing.T, raw string) *cobra.Command { + t.Helper() + cmd := &cobra.Command{} + var jf flags.JsonFlag + cmd.Flags().Var(&jf, "json", "JSON body") + if raw != "" { + require.NoError(t, jf.Set(raw)) + } + return cmd +} + +func TestRejectWrappedRoleJSON(t *testing.T) { + t.Run("rejects wrapped {role: ...}", func(t *testing.T) { + cmd := cmdWithJSON(t, `{"role":{"spec":{"identity_type":"SERVICE_PRINCIPAL"}}}`) + err := rejectWrappedRoleJSON(cmd) + require.Error(t, err) + assert.Contains(t, err.Error(), "should NOT be wrapped") + assert.Contains(t, err.Error(), `databricks postgres create-role`) + }) + + t.Run("passes when body has spec at top level", func(t *testing.T) { + cmd := cmdWithJSON(t, `{"spec":{"identity_type":"SERVICE_PRINCIPAL"}}`) + assert.NoError(t, rejectWrappedRoleJSON(cmd)) + }) + + t.Run("passes when --json was not provided", func(t *testing.T) { + cmd := cmdWithJSON(t, "") + assert.NoError(t, rejectWrappedRoleJSON(cmd)) + }) + + t.Run("passes through non-object JSON to the generated diagnostics path", func(t *testing.T) { + cmd := cmdWithJSON(t, `"not-an-object"`) + assert.NoError(t, rejectWrappedRoleJSON(cmd)) + }) + + t.Run("passes when --json flag is absent on the command", func(t *testing.T) { + assert.NoError(t, rejectWrappedRoleJSON(&cobra.Command{})) + }) +} diff --git a/libs/flags/json_flag.go b/libs/flags/json_flag.go index d5f143c5654..5e239a1f7ec 100644 --- a/libs/flags/json_flag.go +++ b/libs/flags/json_flag.go @@ -113,3 +113,10 @@ func (j *JsonFlag) Unmarshal(v any) diag.Diagnostics { func (j *JsonFlag) Type() string { return "JSON" } + +// Raw returns the raw JSON bytes the flag was set to (or nil when the flag +// was not provided). Exposed so command overrides can do shape-level +// validation before the generated Unmarshal call. +func (j *JsonFlag) Raw() []byte { + return j.raw +}