diff --git a/acceptance/cmd/workspace/database/update-database-instance/output.txt b/acceptance/cmd/workspace/database/update-database-instance/output.txt index 463b128eaf4..3bebd059802 100644 --- a/acceptance/cmd/workspace/database/update-database-instance/output.txt +++ b/acceptance/cmd/workspace/database/update-database-instance/output.txt @@ -27,6 +27,5 @@ Exit code: 1 >>> [CLI] database update-database-instance test-db * --json {"stopped": true} { - "name": "test-db", - "stopped": true + "name": "test-db" } diff --git a/cmd/account/disaster-recovery/disaster-recovery.go b/cmd/account/disaster-recovery/disaster-recovery.go index 156d3baea4a..abdefd4680e 100644 --- a/cmd/account/disaster-recovery/disaster-recovery.go +++ b/cmd/account/disaster-recovery/disaster-recovery.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" "github.com/databricks/databricks-sdk-go/service/disasterrecovery" "github.com/spf13/cobra" @@ -151,7 +152,11 @@ func newCreateFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -245,7 +250,11 @@ func newCreateStableUrl() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -464,7 +473,11 @@ func newFailoverFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -524,7 +537,11 @@ func newGetFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -584,7 +601,11 @@ func newGetStableUrl() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -847,7 +868,11 @@ func newUpdateFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/clean-rooms/clean-rooms.go b/cmd/workspace/clean-rooms/clean-rooms.go index 6ce4f30def1..803dfca4635 100644 --- a/cmd/workspace/clean-rooms/clean-rooms.go +++ b/cmd/workspace/clean-rooms/clean-rooms.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/cleanrooms" "github.com/spf13/cobra" ) @@ -331,7 +332,11 @@ func newGet() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"remote_detailed_info.creator.invite_recipient_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -481,7 +486,11 @@ func newUpdate() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"remote_detailed_info.creator.invite_recipient_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/database/database.go b/cmd/workspace/database/database.go index 92aa1408bfd..b723135f1af 100644 --- a/cmd/workspace/database/database.go +++ b/cmd/workspace/database/database.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/database" "github.com/spf13/cobra" ) @@ -146,7 +147,11 @@ Create a Database Catalog. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -535,7 +540,11 @@ Create a Synced Database Table. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -881,7 +890,11 @@ Find a Database Instance by uid.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1011,7 +1024,11 @@ Get a Database Catalog.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1070,7 +1087,11 @@ Get a Database Instance. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1242,7 +1263,11 @@ Get a Synced Database Table.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1644,7 +1669,11 @@ func newUpdateDatabaseCatalog() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1735,7 +1764,11 @@ Update a Database Instance. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1820,7 +1853,11 @@ func newUpdateSyncedDatabaseTable() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/postgres/postgres.go b/cmd/workspace/postgres/postgres.go index 9522a6d7745..8d1b4360c80 100644 --- a/cmd/workspace/postgres/postgres.go +++ b/cmd/workspace/postgres/postgres.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" "github.com/databricks/databricks-sdk-go/experimental/api" "github.com/databricks/databricks-sdk-go/service/postgres" @@ -1851,7 +1852,11 @@ Get a Branch. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1912,7 +1917,11 @@ Get a Database Catalog. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1972,7 +1981,11 @@ Get a Database. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2035,7 +2048,11 @@ Get an Endpoint. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2158,7 +2175,11 @@ Get a Project. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_endpoint_spec", "spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2221,7 +2242,11 @@ Get a Postgres Role for a Branch. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2284,7 +2309,11 @@ Get a Synced Database Table. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go b/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go index 8290fbf92ac..45d79951947 100644 --- a/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go +++ b/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/qualitymonitorv2" "github.com/spf13/cobra" ) @@ -128,7 +129,11 @@ Create a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -255,7 +260,11 @@ Read a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -427,7 +436,11 @@ Update a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/secrets-uc/secrets-uc.go b/cmd/workspace/secrets-uc/secrets-uc.go index 006a5f5d33d..6fdcc9c711f 100644 --- a/cmd/workspace/secrets-uc/secrets-uc.go +++ b/cmd/workspace/secrets-uc/secrets-uc.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" sdktime "github.com/databricks/databricks-sdk-go/common/types/time" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -164,7 +165,11 @@ func newCreateSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -294,7 +299,11 @@ func newGetSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -505,7 +514,11 @@ func newUpdateSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/internal/cligen/cligen.go b/internal/cligen/cligen.go index 200392f8b68..a536f593676 100644 --- a/internal/cligen/cligen.go +++ b/internal/cligen/cligen.go @@ -71,6 +71,7 @@ func Generate(jsonPath, targetDir string) ([]string, error) { if err := batch.Resolve(); err != nil { return nil, fmt.Errorf("%s: %w", jsonPath, err) } + populateInputOnlyPaths(batch, doc.Schemas) var filenames []string diff --git a/internal/cligen/input_only.go b/internal/cligen/input_only.go new file mode 100644 index 00000000000..02e009b31b8 --- /dev/null +++ b/internal/cligen/input_only.go @@ -0,0 +1,105 @@ +package main + +import ( + "slices" + "strings" + + "github.com/databricks/cli/internal/clijson" +) + +// inputOnlyBehavior is the field-behavior token in cli.json that marks a field +// the server reads on writes but never returns on responses. The constant lives +// here rather than as an enum because cli.json carries Behaviors as []string; +// matching by value keeps the contract dependency one-way. +const inputOnlyBehavior = "INPUT_ONLY" + +// populateInputOnlyPaths fills MethodJSON.InputOnlyPaths for every method +// whose response is a singleton message and that takes the normal sync render +// path (no pagination / LRO / wait / byte stream). The paths drive an +// inputonly.Strip call emitted by service.go.tmpl before cmdio.Render so the +// SDK transport struct's INPUT_ONLY fields don't leak into JSON output. +// +// Methods on the LRO / wait / pagination / byte-stream branches still render +// directly via cmdio.Render — extending those is a follow-up because each +// branch's rendered shape differs (operation type, iterator element, byte +// stream) and cli.json does not yet surface iterator element types. +func populateInputOnlyPaths(batch *CommandsBlock, schemas map[string]*clijson.SchemaJSON) { + if len(schemas) == 0 { + return + } + for _, s := range batch.Services { + if s.Package == nil { + continue + } + for _, m := range s.Methods { + if !eligibleForInputOnly(m) { + continue + } + rootName := s.Package.Name + "." + m.Response.PascalName + m.InputOnlyPaths = inputOnlyPaths(schemas, rootName) + } + } +} + +// eligibleForInputOnly picks the methods whose render site is the standard +// `cmdio.Render(ctx, response)` call in the method-call template. +func eligibleForInputOnly(m *MethodJSON) bool { + if m.Response == nil || m.Response.PascalName == "" || m.Response.IsEmptyResponse { + return false + } + if m.IsResponseByteStream { + return false + } + if m.Pagination != nil || m.Wait != nil || m.LongRunningOperation != nil { + return false + } + return true +} + +// inputOnlyPaths walks the schema graph rooted at rootName and returns the +// sorted dotted JSON paths of every INPUT_ONLY field reachable via direct +// message-typed refs. +// +// Array and map element types are not followed: cli.json's SchemaFieldJSON +// carries one ref slot, populated for singleton message-typed fields only — +// `collaborators` (an array of CleanRoomCollaborator) has no ref, so a field +// like `invite_recipient_workspace_id` reachable only via that array is not +// caught here. A field that is itself INPUT_ONLY at the container level is +// still emitted as a single path; the runtime in libs/inputonly traverses +// arrays and maps transparently. +// +// Cycles in the schema graph are pruned via a per-descent set: the same +// schema can still be reached from multiple parent paths and emit different +// paths for each, but recursion never loops. +func inputOnlyPaths(schemas map[string]*clijson.SchemaJSON, rootName string) []string { + var paths []string + onPath := map[string]bool{} + var walk func(prefix []string, schemaName string) + walk = func(prefix []string, schemaName string) { + s := schemas[schemaName] + if s == nil || onPath[schemaName] { + return + } + onPath[schemaName] = true + defer delete(onPath, schemaName) + names := make([]string, 0, len(s.Fields)) + for name := range s.Fields { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + f := s.Fields[name] + childPath := append(slices.Clone(prefix), name) + if slices.Contains(f.Behaviors, inputOnlyBehavior) { + paths = append(paths, strings.Join(childPath, ".")) + continue + } + if f.Ref != "" { + walk(childPath, f.Ref) + } + } + } + walk(nil, rootName) + slices.Sort(paths) + return paths +} diff --git a/internal/cligen/input_only_test.go b/internal/cligen/input_only_test.go new file mode 100644 index 00000000000..3a4bf7a004d --- /dev/null +++ b/internal/cligen/input_only_test.go @@ -0,0 +1,126 @@ +package main + +import ( + "testing" + + "github.com/databricks/cli/internal/clijson" + "github.com/stretchr/testify/assert" +) + +func TestInputOnlyPathsFlat(t *testing.T) { + schemas := map[string]*clijson.SchemaJSON{ + "dr.StableUrl": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "name": {}, + "initial_workspace_id": {Behaviors: []string{"IMMUTABLE", "INPUT_ONLY"}}, + "url": {Behaviors: []string{"OUTPUT_ONLY"}}, + }, + }, + } + assert.Equal(t, []string{"initial_workspace_id"}, inputOnlyPaths(schemas, "dr.StableUrl")) +} + +func TestInputOnlyPathsNestedMessage(t *testing.T) { + // A singleton message ref is followed; an INPUT_ONLY field on the + // referenced type emits a dotted path. + schemas := map[string]*clijson.SchemaJSON{ + "x.Wrapper": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "inner": {Ref: "x.Inner"}, + }, + }, + "x.Inner": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "secret": {Behaviors: []string{"INPUT_ONLY"}}, + "name": {}, + }, + }, + } + assert.Equal(t, []string{"inner.secret"}, inputOnlyPaths(schemas, "x.Wrapper")) +} + +func TestInputOnlyPathsInputOnlyMessageNotRecursedInto(t *testing.T) { + // A field whose type is itself INPUT_ONLY emits a single path; the + // whole subtree is stripped at runtime so its inner fields don't need + // their own paths. + schemas := map[string]*clijson.SchemaJSON{ + "x.Outer": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "creds": {Ref: "x.Creds", Behaviors: []string{"INPUT_ONLY"}}, + }, + }, + "x.Creds": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "password": {Behaviors: []string{"INPUT_ONLY"}}, + }, + }, + } + assert.Equal(t, []string{"creds"}, inputOnlyPaths(schemas, "x.Outer")) +} + +func TestInputOnlyPathsCycle(t *testing.T) { + // A tree-shaped resource that references itself: the walker doesn't + // loop and still picks up the INPUT_ONLY field at this level. + schemas := map[string]*clijson.SchemaJSON{ + "x.Tree": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "secret": {Behaviors: []string{"INPUT_ONLY"}}, + "parent": {Ref: "x.Tree"}, + }, + }, + } + assert.Equal(t, []string{"secret"}, inputOnlyPaths(schemas, "x.Tree")) +} + +func TestInputOnlyPathsUnknownRoot(t *testing.T) { + assert.Nil(t, inputOnlyPaths(map[string]*clijson.SchemaJSON{}, "x.Missing")) +} + +func TestEligibleForInputOnly(t *testing.T) { + cases := []struct { + name string + method *MethodJSON + want bool + }{ + { + name: "standard sync method", + method: &MethodJSON{Response: &EntityJSON{PascalName: "StableUrl"}}, + want: true, + }, + { + name: "empty response", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Empty", IsEmptyResponse: true}}, + want: false, + }, + { + name: "no response", + method: &MethodJSON{}, + want: false, + }, + { + name: "paginated", + method: &MethodJSON{Response: &EntityJSON{PascalName: "ListResponse"}, Pagination: &PaginationJSON{}}, + want: false, + }, + { + name: "byte stream", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Body"}, IsResponseByteStream: true}, + want: false, + }, + { + name: "long-running", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Operation"}, LongRunningOperation: &LROJSON{}}, + want: false, + }, + { + name: "wait", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Resp"}, Wait: &WaitJSON{}}, + want: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, eligibleForInputOnly(c.method)) + }) + } +} diff --git a/internal/cligen/model.go b/internal/cligen/model.go index ae5ce44dd63..3b6dc12ab75 100644 --- a/internal/cligen/model.go +++ b/internal/cligen/model.go @@ -126,6 +126,19 @@ func (s *ServiceJSON) Comment(prefix string, maxLen int) string { return commentWrap(s.Description, prefix, maxLen) } +// HasInputOnlyPaths is true when any of the service's methods has a non-empty +// InputOnlyPaths. service.go.tmpl uses it to gate the libs/inputonly import: +// importing it unconditionally would leave it unused on services that don't +// strip anything. +func (s *ServiceJSON) HasInputOnlyPaths() bool { + for _, m := range s.Methods { + if len(m.InputOnlyPaths) > 0 { + return true + } + } + return false +} + // MethodJSON is the render context for one command (a method of a service). type MethodJSON struct { Name string `json:"name,omitempty"` @@ -163,6 +176,12 @@ type MethodJSON struct { Pagination *PaginationJSON `json:"pagination,omitempty"` Wait *WaitJSON `json:"wait,omitempty"` LongRunningOperation *LROJSON `json:"long_running_operation,omitempty"` + + // InputOnlyPaths is the sorted set of dotted JSON paths in the method's + // response type that are annotated INPUT_ONLY in cli.json's schemas + // block. Populated by populateInputOnlyPaths after Resolve(); empty for + // methods whose render site is not the standard sync path. + InputOnlyPaths []string `json:"-"` } func (m *MethodJSON) KebabName() string { return kebabName(m.Name) } diff --git a/internal/cligen/templates/service.go.tmpl b/internal/cligen/templates/service.go.tmpl index 21b634f4578..c2aa04b5b18 100644 --- a/internal/cligen/templates/service.go.tmpl +++ b/internal/cligen/templates/service.go.tmpl @@ -11,6 +11,9 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/flags" + {{ if .HasInputOnlyPaths -}} + "github.com/databricks/cli/libs/inputonly" + {{ end -}} "github.com/databricks/cli/cmd/root" "github.com/databricks/databricks-sdk-go/experimental/api" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" @@ -545,6 +548,12 @@ func new{{.PascalName}}() *cobra.Command { {{ if $method.IsResponseByteStream -}} defer response.{{$method.ResponseBodyField.PascalName}}.Close() return cmdio.Render{{ if $method.Pagination}}Iterator{{end}}(ctx, response.{{$method.ResponseBodyField.PascalName}}) + {{- else if $method.InputOnlyPaths }} + masked, err := inputonly.Strip(response, []string{ {{- range $i, $p := $method.InputOnlyPaths}}{{if $i}}, {{end}}"{{$p}}"{{end -}} }) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) {{- else }} return cmdio.Render{{ if $method.Pagination}}Iterator{{end}}(ctx, response) {{- end -}} diff --git a/libs/inputonly/inputonly.go b/libs/inputonly/inputonly.go new file mode 100644 index 00000000000..be77557afc0 --- /dev/null +++ b/libs/inputonly/inputonly.go @@ -0,0 +1,102 @@ +// Package inputonly drops fields marked INPUT_ONLY in the OpenAPI spec from +// SDK response values before they are rendered to the user. +// +// The Databricks Go SDK uses a single struct per resource for both request and +// response (transport-layer pattern). Some fields are REQUIRED on the request +// side — so their JSON tags have no omitempty — but INPUT_ONLY on the response +// side, meaning the server never populates them. When the CLI hands such a +// struct straight to encoding/json it emits the zero value (`"foo": ""`), +// which leaks API surface that isn't meant to round-trip. +// +// This package is consumed by the generated CLI command code in cmd/account/** +// and cmd/workspace/**: cligen reads the schemas in .codegen/cli.json, walks +// the response type, and emits a Strip call before the existing cmdio.Render. +package inputonly + +import ( + "bytes" + "encoding/json" + "fmt" + "strings" +) + +// Strip returns v with the listed dotted paths removed. If paths is empty, v +// is returned unchanged. Otherwise v is round-tripped through JSON into a +// generic representation, the listed paths are deleted, and the masked value +// is returned for the caller to marshal in its preferred format. +// +// Paths use dotted notation (e.g. "stable_url.initial_workspace_id") and are +// matched literally at every segment. Arrays are traversed transparently: a +// path is applied to every element of any []any encountered along the way. +// Maps are not — see deletePath for the reasoning. +// +// Numbers are decoded via json.Number rather than float64 so values above +// 2^53 (e.g. SDK fields typed int64 like spark_context_id) re-marshal +// verbatim instead of silently losing precision. +// +// Side note: encoding/json marshals struct fields in declaration order but +// map[string]any keys alphabetically. Filtered responses therefore render +// with sorted JSON keys, while unfiltered ones keep SDK-struct order. +// Acceptance fixtures and downstream consumers should be tolerant of that. +func Strip(v any, paths []string) (any, error) { + if len(paths) == 0 { + return v, nil + } + b, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("inputonly: marshal: %w", err) + } + dec := json.NewDecoder(bytes.NewReader(b)) + dec.UseNumber() + var out any + if err := dec.Decode(&out); err != nil { + return nil, fmt.Errorf("inputonly: unmarshal: %w", err) + } + for _, p := range paths { + deletePath(out, strings.Split(p, ".")) + } + return out, nil +} + +// deletePath walks v according to keys and removes the leaf key from the +// object it lands on. Each segment is matched literally; if no literal match +// exists at a given level the path simply does not apply, and we return. +// +// Arrays are traversed transparently. After json.Unmarshal a JSON array is +// always []any, type-distinguishable from map[string]any, so descending into +// every element with the same key list is unambiguous. +// +// Maps are not traversed transparently, even though proto map +// surfaces as map[string]any just like a struct does. Falling back to +// match-anywhere when the literal misses turns an anchored path into a +// match-anywhere expression: e.g. path "name" against +// {"id":"123","details":{"name":"x"}} would strip "details.name" instead of +// no-oping. Since INPUT_ONLY fields are always omitted by the server, the +// literal miss is the normal case and the fallback would fire on every +// Strip call; the failure mode is silent over-stripping. cligen emits +// paths anchored to specific schema locations and does not currently emit +// paths that descend through proto maps (cli.json carries one ref slot, +// populated for singleton message fields only). If that contract grows +// map value refs later, the path language should grow an explicit map +// marker (e.g. "*") rather than reintroducing implicit fallback. +func deletePath(v any, keys []string) { + if len(keys) == 0 { + return + } + switch t := v.(type) { + case map[string]any: + child, ok := t[keys[0]] + if !ok { + return + } + if len(keys) == 1 { + delete(t, keys[0]) + return + } + deletePath(child, keys[1:]) + case []any: + for _, el := range t { + deletePath(el, keys) + } + } +} diff --git a/libs/inputonly/inputonly_test.go b/libs/inputonly/inputonly_test.go new file mode 100644 index 00000000000..ff1af4fc2a3 --- /dev/null +++ b/libs/inputonly/inputonly_test.go @@ -0,0 +1,155 @@ +package inputonly + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stableURL struct { + Name string `json:"name"` + InitialWorkspaceID string `json:"initial_workspace_id"` + URL string `json:"url,omitempty"` +} + +type stableURLList struct { + StableURLs []stableURL `json:"stable_urls"` +} + +type wrapper struct { + StableURL stableURL `json:"stable_url"` +} + +type taggedStableURLs struct { + Tags map[string]stableURL `json:"tags"` +} + +func TestStripEmptyPathsReturnsValueUnchanged(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w"} + out, err := Strip(in, nil) + require.NoError(t, err) + assert.Equal(t, in, out) +} + +func TestStripFlatField(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} + out, err := Strip(in, []string{"initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"n","url":"u"}`, string(b)) +} + +func TestStripFieldAbsentIsNoop(t *testing.T) { + in := stableURL{Name: "n"} + out, err := Strip(in, []string{"missing_field"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + // Name retained; missing path silently ignored. InitialWorkspaceID stays + // present at "" because the struct tag has no omitempty. + assert.JSONEq(t, `{"name":"n","initial_workspace_id":""}`, string(b)) +} + +func TestStripNested(t *testing.T) { + in := wrapper{StableURL: stableURL{Name: "n", InitialWorkspaceID: "w"}} + out, err := Strip(in, []string{"stable_url.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"stable_url":{"name":"n"}}`, string(b)) +} + +func TestStripSliceElements(t *testing.T) { + in := stableURLList{StableURLs: []stableURL{ + {Name: "a", InitialWorkspaceID: "1"}, + {Name: "b", InitialWorkspaceID: "2"}, + }} + out, err := Strip(in, []string{"stable_urls.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"stable_urls":[{"name":"a"},{"name":"b"}]}`, string(b)) +} + +func TestStripMultiplePaths(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} + out, err := Strip(in, []string{"initial_workspace_id", "url"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"n"}`, string(b)) +} + +// TestStripPreservesLargeInt64 guards against the float64 round-trip pitfall: +// encoding/json decodes JSON numbers into float64 (53-bit mantissa) when the +// destination is `any`, which silently loses precision for SDK fields like +// spark_context_id (int64) above 2^53. Strip uses json.Number to side-step +// that. +func TestStripPreservesLargeInt64(t *testing.T) { + type clusterResponse struct { + ClusterName string `json:"cluster_name"` + SparkContextID int64 `json:"spark_context_id"` + InitialWorkspaceID string `json:"initial_workspace_id"` + } + in := clusterResponse{ + ClusterName: "c", + SparkContextID: 9007199254740993, // 2^53 + 1, unrepresentable as float64 + InitialWorkspaceID: "ws-1", + } + out, err := Strip(in, []string{"initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.Contains(t, string(b), `"spark_context_id":9007199254740993`) + assert.NotContains(t, string(b), "initial_workspace_id") +} + +// TestStripDoesNotMatchAnywhere guards against the implicit-fallback pitfall: +// an anchored path that doesn't match at the level it targets must NOT +// silently descend into other objects and strip same-named fields elsewhere. +// Since INPUT_ONLY fields are always omitted by the server, the literal miss +// at the anchored level is the common case — a fallback descent would fire +// every time and quietly strip legitimate fields with the same leaf name. +func TestStripDoesNotMatchAnywhere(t *testing.T) { + type details struct { + Name string `json:"name"` + Size string `json:"size"` + } + type response struct { + ID string `json:"id"` + Details details `json:"details"` + } + in := response{ID: "123", Details: details{Name: "keep-me", Size: "L"}} + out, err := Strip(in, []string{"name"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"id":"123","details":{"name":"keep-me","size":"L"}}`, string(b)) +} + +// TestStripDoesNotDescendIntoMaps documents the strict-map behavior: a path +// that doesn't literally match a map key is a no-op, never a fallback into +// every value. cligen does not emit paths that descend through proto maps +// today (cli.json carries no map value refs); the runtime matches that +// contract instead of guessing. +func TestStripDoesNotDescendIntoMaps(t *testing.T) { + in := taggedStableURLs{Tags: map[string]stableURL{ + "env": {Name: "a", InitialWorkspaceID: "1"}, + "prod": {Name: "b", InitialWorkspaceID: "2"}, + }} + out, err := Strip(in, []string{"tags.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + + var got struct { + Tags map[string]map[string]any `json:"tags"` + } + require.NoError(t, json.Unmarshal(b, &got)) + require.Len(t, got.Tags, 2) + assert.Equal(t, "1", got.Tags["env"]["initial_workspace_id"]) + assert.Equal(t, "2", got.Tags["prod"]["initial_workspace_id"]) +}