Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions NEXT_CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,6 +10,7 @@

* 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.
* `[__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`.
* `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](https://github.com/databricks/cli/pull/5111)).

### Bundles
* Make sure warnings asking for approval are understood by agents ([#5239](https://github.com/databricks/cli/pull/5239))
Expand Down
78 changes: 78 additions & 0 deletions cmd/workspace/postgres/overrides.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
package postgres

import (
"errors"
"fmt"

"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 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 rejects wrapped bodies
client-side with a hint pointing to the right shape.

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"}}'

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.`
}

// 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 {
// These checks are internal invariants — postgres create-role is a
// generated command and always has a *flags.JsonFlag for --json. A
// future codegen/refactor change could break that, and we want loud
// breakage rather than a silently-disabled guard.
flag := cmd.Flags().Lookup("json")
if flag == nil {
return errors.New("internal: postgres create-role expected a --json flag; this override is wired to the wrong command")
}
jf, ok := flag.Value.(*flags.JsonFlag)
if !ok {
return fmt.Errorf("internal: postgres create-role --json flag has unexpected type %T; expected *flags.JsonFlag", flag.Value)
}
return jf.RejectWrappedJSON("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"}}'`)
}

func init() {
createRoleOverrides = append(createRoleOverrides, createRoleOverride)
}
56 changes: 56 additions & 0 deletions cmd/workspace/postgres/overrides_test.go
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
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("fails loudly when --json flag is absent on the command", func(t *testing.T) {
// Internal invariant: postgres create-role is a generated command and
// always has a --json flag. If a future codegen change drops it, this
// override is wired to the wrong command and should fail loudly so the
// regression is caught rather than silently disabling the guard.
err := rejectWrappedRoleJSON(&cobra.Command{})
require.Error(t, err)
assert.Contains(t, err.Error(), "internal:")
})
}
41 changes: 41 additions & 0 deletions libs/flags/json_flag.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -113,3 +113,44 @@ 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
}

// RejectWrappedJSON returns a clear client-side error when the --json body
// is a top-level object containing outerKey. It detects the common mistake
// of wrapping the body in '{"<outerKey>": ...}' when the flag binds to the
// inner object (the SDK request type's JSON-tagged outer field). Returns
// nil when the flag is unset, when the body isn't an object, or when no
// outerKey is present at the top level.
//
// example, if non-empty, is appended to the error as a hint at the correct
// shape. Callers typically construct it as a self-contained command line.
//
// This is the shared helper used by the postgres command overrides; the
// same inner-body --json shape exists across siblings like create-branch,
// create-database, create-endpoint, and create-project.
func (j *JsonFlag) RejectWrappedJSON(outerKey, example string) error {
if len(j.raw) == 0 {
return nil
}
var top map[string]json.RawMessage
if err := json.Unmarshal(j.raw, &top); err != nil {
// Defer non-object inputs to the generated unmarshal so its
// diagnostics render the original parse error.
return nil //nolint:nilerr
}
if _, found := top[outerKey]; !found {
return nil
}
msg := fmt.Sprintf("--json should NOT be wrapped in '{%q: ...}'.\n\n"+
"The flag binds to the inner object — supply its fields directly.", outerKey)
if example != "" {
msg += "\n\nExample:\n\n " + example
}
return fmt.Errorf("%s", msg)
}
41 changes: 41 additions & 0 deletions libs/flags/json_flag_test.go
Original file line numberDiff line numberDiff line change
Expand Up@@ -284,3 +284,44 @@ func TestJsonUnmarshalForRequestWithForceSendFields(t *testing.T) {
assert.NotContains(t, r.NewSettings.NotificationSettings.ForceSendFields, "NoAlertForSkippedRuns")
assert.Contains(t, r.NewSettings.NotificationSettings.ForceSendFields, "NoAlertForCanceledRuns")
}

func TestRejectWrappedJSON(t *testing.T) {
tests := []struct {
name string
raw string
outerKey string
wantErr string
}{
{name: "empty body passes", raw: "", outerKey: "role"},
{name: "non-object body passes", raw: `"not-an-object"`, outerKey: "role"},
{name: "object without outer key passes", raw: `{"spec":{"foo":"bar"}}`, outerKey: "role"},
{name: "object with outer key rejected", raw: `{"role":{"spec":{"foo":"bar"}}}`, outerKey: "role", wantErr: `should NOT be wrapped`},
{name: "object with outer key plus siblings still rejected", raw: `{"role":{},"other":1}`, outerKey: "role", wantErr: `should NOT be wrapped`},
{name: "outer key differs from body's outer", raw: `{"branch":{}}`, outerKey: "role"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var jf JsonFlag
if tt.raw != "" {
require.NoError(t, jf.Set(tt.raw))
}
err := jf.RejectWrappedJSON(tt.outerKey, "")
if tt.wantErr != "" {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.wantErr)
return
}
assert.NoError(t, err)
})
}
}

func TestRejectWrappedJSONIncludesExample(t *testing.T) {
var jf JsonFlag
require.NoError(t, jf.Set(`{"role":{}}`))
err := jf.RejectWrappedJSON("role", "databricks postgres create-role --json '{\"spec\":{}}'")
require.Error(t, err)
assert.Contains(t, err.Error(), "Example:")
assert.Contains(t, err.Error(), `--json '{"spec":{}}'`)
}
Loading