Skip to content

postgres: add --json body example to create-role help#5111

Merged
simonfaltum merged 8 commits into
mainfrom
jb/postgres-create-role-help
May 20, 2026
Merged

postgres: add --json body example to create-role help#5111
simonfaltum merged 8 commits into
mainfrom
jb/postgres-create-role-help

Conversation

@jamesbroadhead

Copy link
Copy Markdown
Contributor

Summary

databricks postgres create-role's --json flag binds to the inner Role object (CreateRoleRequest.Role, JSON-tagged "role"), so users must supply spec / name / etc. directly. Without an example this isn't obvious — the auto-generated help leaves the spec fields unflagged (// TODO: complex arg: spec in the generator), and the server's error when the body is wrong is vague:

Field 'role' is required and must contain at least one subfield with a non-default value

That fires whenever the inner Role has no recognized fields, which most commonly happens when a user wraps the body in {"role": ...} (matching the wire format the SDK marshals to). The CLI strips the unknown outer key with Warning: unknown field: role and ships an empty body. Walking out of that loop currently requires reading the SDK source.

This adds a curated override (cmd/workspace/postgres/overrides.go) that appends a concrete service-principal-role example to cmd.Long, plus a short note on the wrapping pitfall.

Help output (after)

Arguments:
  PARENT: The Branch where this Role is created. Format:
    projects/{project_id}/branches/{branch_id}

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/<PROJECT_ID>/branches/<BRANCH_ID> \
    --role-id <SP_CLIENT_ID> \
    --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "<SP_CLIENT_ID>", "auth_method": "LAKEBASE_OAUTH_V1", "membership_roles": ["DATABRICKS_SUPERUSER"]}}'

Scope

This PR only touches create-role. The same shape gap (// TODO: complex arg: spec + opaque error) exists for create-endpoint, create-branch, create-project, and create-database. Happy to extend if the approach is right; left them out so reviewers can decide on the pattern first.

Test plan

  • go build ./cmd/workspace/postgres/...
  • databricks postgres create-role --help shows the new section (output above)
  • make fmt clean
  • Reproduced the original confusion with a service-principal payload before the change; with this PR the example would have led me straight to the working body shape

This pull request and its description were written by Isaac.

@simonfaltum

Copy link
Copy Markdown
Member

I took a look. I think the underlying problem is real: create-role --json binds to the inner Role, and the SDK sends request.Role as the body, so {"role": ...} is genuinely the wrong CLI shape. The help text plus early error are useful.

A few things I would want addressed or discussed before merge:

  • Design / scope concern: cmd/workspace/postgres/overrides.go hardcodes a one-off role wrapper check, but the same inner-body --json pattern exists for sibling Postgres commands (branch, database, endpoint, project, etc.). If we add JsonFlag.Raw() for this, I would rather see a small reusable helper or generator-level pattern than command-specific JSON parsing that will spread by copy-paste.

  • Should fix: rejectWrappedRoleJSON silently returns nil if the json flag is not *flags.JsonFlag. That is an internal invariant for this generated command; it should fail loudly with %T so a future generator/refactor does not silently disable the validation.

  • Should fix: this changes user-facing help and adds a new client-side error, but there is no NEXT_CHANGELOG.md entry.

Test/CI note: lint, generated validation, and unit test jobs are passing on GitHub; the integration test check is currently failing.

@jamesbroadhead jamesbroadhead force-pushed the jb/postgres-create-role-help branch from 4ff71dd to 2238c40 Compare May 18, 2026 22:30
@jamesbroadhead

Copy link
Copy Markdown
Contributor Author

👋 @simonfaltum @andrewnester — Claude here on James's behalf.

Rebased onto current main. Diff unchanged from the prior shape: 3 files, +150 / -0 (new overrides.go + overrides_test.go + a 7-line addition in libs/flags/json_flag.go). Build clean, unit tests pass locally.

Ready for review whenever you've got a moment.

(comment posted by Claude)

jamesbroadhead and others added 4 commits May 20, 2026 13:15
`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
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
…-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
- Generalize the wrapped-JSON rejection into a reusable
  `(*flags.JsonFlag).RejectWrappedJSON(outerKey, example)` helper in
  libs/flags/json_flag.go. Sibling postgres commands (create-branch,
  create-database, create-endpoint, create-project) can adopt the same
  guard with a one-line call, avoiding the copy-paste-per-command shape
  Simon flagged.
- Harden the postgres override's flag-lookup and type-assertion to fail
  loud with `%T` instead of silently returning nil. create-role is a
  generated command and the --json flag invariant should produce a
  visible regression if codegen ever drops it.
- Add a NEXT_CHANGELOG entry for the user-visible help/error addition.

Co-authored-by: Isaac
@jamesbroadhead jamesbroadhead force-pushed the jb/postgres-create-role-help branch from 2238c40 to 76a5978 Compare May 20, 2026 13:19
@jamesbroadhead

Copy link
Copy Markdown
Contributor Author

👋 @simonfaltum @andrewnester — Claude here on James's behalf.

Apologies for the long delay — I missed Simon's May 7 review when I rebased and pinged on May 18. Catching up now. All three of Simon's items are addressed in 76a59789c:

1. Design / scope — Generalized the wrapped-body rejection. New method (*flags.JsonFlag).RejectWrappedJSON(outerKey, example) error in libs/flags/json_flag.go does the detection + error rendering. The postgres override is now a one-liner:

return jf.RejectWrappedJSON("role", `databricks postgres create-role projects/<PROJECT_ID>/branches/<BRANCH_ID> \
    --role-id <SP_CLIENT_ID> \
    --json '{"spec": {...}}'`)

Sibling postgres commands (create-branch, create-database, create-endpoint, create-project) can adopt the same guard in a one-line override each — happy to follow up with those in a small separate PR if the helper shape lands well here.

2. Should-fix (loud type-assertion)rejectWrappedRoleJSON now returns fmt.Errorf("internal: ... got %T", flag.Value) if the --json flag is missing or has an unexpected type. Comment notes this is a generator/refactor tripwire, not a user-reachable path. Test case updated (was previously asserting the old silent-nil; now asserts the loud error).

3. Should-fix (NEXT_CHANGELOG) — Entry added under ### CLI.

Diff: 5 files, +93/-27. Added a TestRejectWrappedJSON table to libs/flags/json_flag_test.go covering empty body, non-object body, object without/with outer key, sibling keys, mismatched key, plus a separate test for the Example: hint formatting.

Re the CI integration-test failure you flagged: passing CI now; the rebase pulled in fixes since.

(comment posted by Claude)

@simonfaltum simonfaltum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the changelog regression is fixed now.

@jamesbroadhead jamesbroadhead enabled auto-merge May 20, 2026 14:34
@simonfaltum simonfaltum disabled auto-merge May 20, 2026 15:29
@simonfaltum simonfaltum merged commit f8f0230 into main May 20, 2026
22 of 23 checks passed
@simonfaltum simonfaltum deleted the jb/postgres-create-role-help branch May 20, 2026 15:29
deco-sdk-tagging Bot added a commit that referenced this pull request May 21, 2026
## Release v1.0.0

### Notable Changes

* The Databricks CLI is now generally available with version v1.0.0 as the first major release 🚀. From this version on, the CLI follows semantic versioning (see [README](README.md)). This change does not impact DABs or other existing commands beyond the changes listed below.
* The 0.299.x line continues to receive security-critical patches through May 20, 2027; see [SECURITY](SECURITY.md) for the support policy.
* Starting with v1.0.0, the CLI will use [immutable release tags](https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases) to increase security against supply chain attacks.
* Breaking change: OAuth tokens for interactive logins (`auth_type = databricks-cli`) are now stored in the OS-native secure store by default (Keychain on macOS, Credential Manager on Windows, Secret Service on Linux) instead of `~/.databricks/token-cache.json`. After upgrading, run `databricks auth login` once per profile to re-authenticate; cached tokens from older versions are not migrated. To keep the previous file-backed storage, set `DATABRICKS_AUTH_STORAGE=plaintext` or add `auth_storage = plaintext` under `[__settings__]` in `~/.databrickscfg` (the env var takes precedence over the config setting), then re-run `databricks auth login`. On systems where the OS keyring is not reachable (e.g. Linux containers without a D-Bus session bus), the CLI transparently falls back to the file cache when reading tokens so legacy `token-cache.json` entries remain accessible without manual configuration.

### CLI

* Added `databricks aitools` command group for installing Databricks skills into your coding agents (Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity). Skills are fetched from [github.com/databricks/databricks-agent-skills](https://github.com/databricks/databricks-agent-skills) and either symlinked into each agent's skills directory or copied into the current project. Use `databricks aitools install` to set up, `update` to pull newer versions, `list` to see what's available, and `uninstall` to remove them. Pick where they go with `--scope=project|global` (`--scope=both` is accepted on `update` and `list`).
* `[__settings__].default_profile` is now consulted as a fallback by `databricks api`, `databricks auth token`, and bundle commands when neither `--profile` nor `DATABRICKS_CONFIG_PROFILE` is set. `databricks auth token` continues to give precedence to `DATABRICKS_HOST` over `default_profile`. For bundle commands, `default_profile` only applies when the bundle does not pin its own `workspace.host`.
* Fixed bug where auth commands did not load the DEFAULT profile properly during auth where type is `databricks-cli`.
* `databricks workspace import-dir` now skips `.git`, `.databricks`, and `node_modules` directories during recursive imports. To import one of these directories deliberately, pass it as `SOURCE_PATH` ([#5118](#5118)).
* `databricks postgres create-role --help` now documents the `--json` body shape and rejects the common mistake of wrapping the body in `{"role": ...}` client-side with a hint pointing at the correct shape ([#5111](#5111)).
* `databricks aitools list` honors `--output json`, emitting a structured `{release, skills[...], summary{}}` document so coding agents and CI can consume the skill/version/installation matrix without scraping the tabular text output ([#5233](#5233)).

### Bundles
* Make sure warnings asking for approval are understood by agents ([#5239](#5239))
* Support `replace_existing: true` on `postgres_branches` and `postgres_endpoints` so bundles can manage the implicitly-created production branch and primary read-write endpoint of a Lakebase project.
* Add `postgres_catalogs` resource to bind a Unity Catalog catalog to a Postgres database on a Lakebase Autoscaling branch ([#5265](#5265)).
* Add `postgres_synced_tables` resource to sync a Unity Catalog Delta table into a Postgres table on a Lakebase Autoscaling branch ([#5268](#5268)).
* engine/direct: Changes to state file now persisted to .wal file right away instead of being saved in the end ([#5149](#5149))
TanishqDatabricks pushed a commit to TanishqDatabricks/cli that referenced this pull request May 22, 2026
)

## Summary

`databricks postgres create-role`'s `--json` flag binds to the inner
`Role` object (`CreateRoleRequest.Role`, JSON-tagged `"role"`), so users
must supply `spec` / `name` / etc. directly. Without an example this
isn't obvious — the auto-generated help leaves the spec fields unflagged
(`// TODO: complex arg: spec` in the generator), and the server's error
when the body is wrong is vague:

```
Field 'role' is required and must contain at least one subfield with a non-default value
```

That fires whenever the inner `Role` has no recognized fields, which
most commonly happens when a user wraps the body in `{"role": ...}`
(matching the wire format the SDK marshals to). The CLI strips the
unknown outer key with `Warning: unknown field: role` and ships an empty
body. Walking out of that loop currently requires reading the SDK
source.

This adds a curated override (`cmd/workspace/postgres/overrides.go`)
that appends a concrete service-principal-role example to `cmd.Long`,
plus a short note on the wrapping pitfall.

### Help output (after)

```
Arguments:
  PARENT: The Branch where this Role is created. Format:
    projects/{project_id}/branches/{branch_id}

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/<PROJECT_ID>/branches/<BRANCH_ID> \
    --role-id <SP_CLIENT_ID> \
    --json '{"spec": {"identity_type": "SERVICE_PRINCIPAL", "postgres_role": "<SP_CLIENT_ID>", "auth_method": "LAKEBASE_OAUTH_V1", "membership_roles": ["DATABRICKS_SUPERUSER"]}}'
```

### Scope

This PR only touches `create-role`. The same shape gap (`// TODO:
complex arg: spec` + opaque error) exists for `create-endpoint`,
`create-branch`, `create-project`, and `create-database`. Happy to
extend if the approach is right; left them out so reviewers can decide
on the pattern first.

## Test plan
- [x] `go build ./cmd/workspace/postgres/...`
- [x] `databricks postgres create-role --help` shows the new section
(output above)
- [x] `make fmt` clean
- [x] Reproduced the original confusion with a service-principal payload
before the change; with this PR the example would have led me straight
to the working body shape

This pull request and its description were written by Isaac.

---------

Co-authored-by: simon <4305831+simonfaltum@users.noreply.github.com>
Co-authored-by: simon <simon.faltum@databricks.com>
TanishqDatabricks pushed a commit to TanishqDatabricks/cli that referenced this pull request May 22, 2026
## Release v1.0.0

### Notable Changes

* The Databricks CLI is now generally available with version v1.0.0 as the first major release 🚀. From this version on, the CLI follows semantic versioning (see [README](README.md)). This change does not impact DABs or other existing commands beyond the changes listed below.
* The 0.299.x line continues to receive security-critical patches through May 20, 2027; see [SECURITY](SECURITY.md) for the support policy.
* Starting with v1.0.0, the CLI will use [immutable release tags](https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases) to increase security against supply chain attacks.
* Breaking change: OAuth tokens for interactive logins (`auth_type = databricks-cli`) are now stored in the OS-native secure store by default (Keychain on macOS, Credential Manager on Windows, Secret Service on Linux) instead of `~/.databricks/token-cache.json`. After upgrading, run `databricks auth login` once per profile to re-authenticate; cached tokens from older versions are not migrated. To keep the previous file-backed storage, set `DATABRICKS_AUTH_STORAGE=plaintext` or add `auth_storage = plaintext` under `[__settings__]` in `~/.databrickscfg` (the env var takes precedence over the config setting), then re-run `databricks auth login`. On systems where the OS keyring is not reachable (e.g. Linux containers without a D-Bus session bus), the CLI transparently falls back to the file cache when reading tokens so legacy `token-cache.json` entries remain accessible without manual configuration.

### CLI

* Added `databricks aitools` command group for installing Databricks skills into your coding agents (Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity). Skills are fetched from [github.com/databricks/databricks-agent-skills](https://github.com/databricks/databricks-agent-skills) and either symlinked into each agent's skills directory or copied into the current project. Use `databricks aitools install` to set up, `update` to pull newer versions, `list` to see what's available, and `uninstall` to remove them. Pick where they go with `--scope=project|global` (`--scope=both` is accepted on `update` and `list`).
* `[__settings__].default_profile` is now consulted as a fallback by `databricks api`, `databricks auth token`, and bundle commands when neither `--profile` nor `DATABRICKS_CONFIG_PROFILE` is set. `databricks auth token` continues to give precedence to `DATABRICKS_HOST` over `default_profile`. For bundle commands, `default_profile` only applies when the bundle does not pin its own `workspace.host`.
* Fixed bug where auth commands did not load the DEFAULT profile properly during auth where type is `databricks-cli`.
* `databricks workspace import-dir` now skips `.git`, `.databricks`, and `node_modules` directories during recursive imports. To import one of these directories deliberately, pass it as `SOURCE_PATH` ([databricks#5118](databricks#5118)).
* `databricks postgres create-role --help` now documents the `--json` body shape and rejects the common mistake of wrapping the body in `{"role": ...}` client-side with a hint pointing at the correct shape ([databricks#5111](databricks#5111)).
* `databricks aitools list` honors `--output json`, emitting a structured `{release, skills[...], summary{}}` document so coding agents and CI can consume the skill/version/installation matrix without scraping the tabular text output ([databricks#5233](databricks#5233)).

### Bundles
* Make sure warnings asking for approval are understood by agents ([databricks#5239](databricks#5239))
* Support `replace_existing: true` on `postgres_branches` and `postgres_endpoints` so bundles can manage the implicitly-created production branch and primary read-write endpoint of a Lakebase project.
* Add `postgres_catalogs` resource to bind a Unity Catalog catalog to a Postgres database on a Lakebase Autoscaling branch ([databricks#5265](databricks#5265)).
* Add `postgres_synced_tables` resource to sync a Unity Catalog Delta table into a Postgres table on a Lakebase Autoscaling branch ([databricks#5268](databricks#5268)).
* engine/direct: Changes to state file now persisted to .wal file right away instead of being saved in the end ([databricks#5149](databricks#5149))
denik pushed a commit that referenced this pull request May 28, 2026
## Release v1.0.0

### Notable Changes

* The Databricks CLI is now generally available with version v1.0.0 as the first major release 🚀. From this version on, the CLI follows semantic versioning (see [README](README.md)). This change does not impact DABs or other existing commands beyond the changes listed below.
* The 0.299.x line continues to receive security-critical patches through May 20, 2027; see [SECURITY](SECURITY.md) for the support policy.
* Starting with v1.0.0, the CLI will use [immutable release tags](https://docs.github.com/en/code-security/concepts/supply-chain-security/immutable-releases) to increase security against supply chain attacks.
* Breaking change: OAuth tokens for interactive logins (`auth_type = databricks-cli`) are now stored in the OS-native secure store by default (Keychain on macOS, Credential Manager on Windows, Secret Service on Linux) instead of `~/.databricks/token-cache.json`. After upgrading, run `databricks auth login` once per profile to re-authenticate; cached tokens from older versions are not migrated. To keep the previous file-backed storage, set `DATABRICKS_AUTH_STORAGE=plaintext` or add `auth_storage = plaintext` under `[__settings__]` in `~/.databrickscfg` (the env var takes precedence over the config setting), then re-run `databricks auth login`. On systems where the OS keyring is not reachable (e.g. Linux containers without a D-Bus session bus), the CLI transparently falls back to the file cache when reading tokens so legacy `token-cache.json` entries remain accessible without manual configuration.

### CLI

* Added `databricks aitools` command group for installing Databricks skills into your coding agents (Claude Code, Cursor, Codex CLI, OpenCode, GitHub Copilot, Antigravity). Skills are fetched from [github.com/databricks/databricks-agent-skills](https://github.com/databricks/databricks-agent-skills) and either symlinked into each agent's skills directory or copied into the current project. Use `databricks aitools install` to set up, `update` to pull newer versions, `list` to see what's available, and `uninstall` to remove them. Pick where they go with `--scope=project|global` (`--scope=both` is accepted on `update` and `list`).
* `[__settings__].default_profile` is now consulted as a fallback by `databricks api`, `databricks auth token`, and bundle commands when neither `--profile` nor `DATABRICKS_CONFIG_PROFILE` is set. `databricks auth token` continues to give precedence to `DATABRICKS_HOST` over `default_profile`. For bundle commands, `default_profile` only applies when the bundle does not pin its own `workspace.host`.
* Fixed bug where auth commands did not load the DEFAULT profile properly during auth where type is `databricks-cli`.
* `databricks workspace import-dir` now skips `.git`, `.databricks`, and `node_modules` directories during recursive imports. To import one of these directories deliberately, pass it as `SOURCE_PATH` ([#5118](#5118)).
* `databricks postgres create-role --help` now documents the `--json` body shape and rejects the common mistake of wrapping the body in `{"role": ...}` client-side with a hint pointing at the correct shape ([#5111](#5111)).
* `databricks aitools list` honors `--output json`, emitting a structured `{release, skills[...], summary{}}` document so coding agents and CI can consume the skill/version/installation matrix without scraping the tabular text output ([#5233](#5233)).

### Bundles
* Make sure warnings asking for approval are understood by agents ([#5239](#5239))
* Support `replace_existing: true` on `postgres_branches` and `postgres_endpoints` so bundles can manage the implicitly-created production branch and primary read-write endpoint of a Lakebase project.
* Add `postgres_catalogs` resource to bind a Unity Catalog catalog to a Postgres database on a Lakebase Autoscaling branch ([#5265](#5265)).
* Add `postgres_synced_tables` resource to sync a Unity Catalog Delta table into a Postgres table on a Lakebase Autoscaling branch ([#5268](#5268)).
* engine/direct: Changes to state file now persisted to .wal file right away instead of being saved in the end ([#5149](#5149))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants