Skip to content
Open
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 number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
* direct: Fix spurious update when `apply_policy_default_values: true` is set on job task, for-each-task, or job cluster new_cluster ([#5731](https://github.com/databricks/cli/pull/5731)). Also fix spurious updates for for-each-task clusters due to missing backend defaults for `data_security_mode`, `node_type_id`, `driver_node_type_id`, `driver_instance_pool_id`, `enable_elastic_disk`, and `enable_local_disk_encryption`.
* direct: Cluster resize now falls back to regular update if resize fails due to `INVALID_STATE` ([#5716](https://github.com/databricks/cli/pull/5716)).
* Fixed `bundle deployment migrate` failing on `model_serving_endpoints`/`database_instances` with permissions (regression since v1.5.0) ([#5775](https://github.com/databricks/cli/pull/5775)).
* direct: Fix deploy bug when a `postgres_projects`, `postgres_branches`, or `postgres_endpoints` field is set to its zero value (e.g. `enable_pg_native_login: false`, `replace_existing: false`).

### Dependency updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ resources:
my_project:
project_id: test-pg-proj-$UNIQUE_NAME
display_name: "Test Postgres Project"
enable_pg_native_login: false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we also add this to invariant tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Included this in the project resource config for the invariant tests.

pg_version: 16
history_retention_duration: "604800s"
default_endpoint_settings:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
"suspend_timeout_duration": "300s"
},
"display_name": "Test Postgres Project",
"enable_pg_native_login": false,
"history_retention_duration": "604800s",
"pg_version": 16
}
Expand Down
88 changes: 88 additions & 0 deletions bundle/config/resources_types_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
package config

import (
"encoding/json"
"reflect"
"slices"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/databricks/cli/bundle/config/resources"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/dyn/convert"
"github.com/databricks/cli/libs/structs/structtag"
)

func TestResourcesTypesMap(t *testing.T) {
Expand All @@ -20,3 +26,85 @@ func TestResourcesTypesMap(t *testing.T) {
assert.True(t, ok, "resources type for 'jobs.permissions' not found in ResourcesTypes map")
assert.Equal(t, reflect.TypeFor[[]resources.JobPermission](), typ, "resources type for 'jobs.permissions' mismatch")
}

// TestResourceTypesZeroValueFieldsSerialize guards against the ForceSendFields
// routing bug fixed in libs/dyn/convert: a field declared in a struct embedded
// more than one level deep (e.g. PostgresProject -> PostgresProjectConfig ->
// ProjectSpec) had its zero value recorded in the wrong struct's ForceSendFields,
// which the SDK marshaler rejects with "field X cannot be found in struct Y".
// The direct engine hits this path when it serializes planned state to JSON.
//
// For every registered resource type it sets every omitempty scalar field (at any
// depth) to its zero value, converts via ToTyped, and marshals - the same round
// trip the direct engine performs. Any newly added resource whose wrapper embeds
// an SDK spec is covered automatically.
func TestResourceTypesZeroValueFieldsSerialize(t *testing.T) {
names := make([]string, 0, len(ResourcesTypes))
for name := range ResourcesTypes {
names = append(names, name)
}
slices.Sort(names)

for _, name := range names {
t.Run(name, func(t *testing.T) {
typ := ResourcesTypes[name]
zeros := zeroValueScalars(typ, 0, map[reflect.Type]bool{})
if zeros.Kind() != dyn.KindMap {
return
}

ptr := reflect.New(typ)
require.NoError(t, convert.ToTyped(ptr.Interface(), zeros))

_, err := json.Marshal(ptr.Interface())
require.NoError(t, err)
})
}
}

// zeroValueScalars builds a [dyn.Value] map that sets every omitempty scalar field
// reachable through embedded anonymous structs to its zero value. Those are exactly
// the fields the convert layer records in ForceSendFields, so they exercise the
// routing logic. depth and seen bound recursion against deep or recursive types.
func zeroValueScalars(t reflect.Type, depth int, seen map[reflect.Type]bool) dyn.Value {
for t.Kind() == reflect.Pointer {
t = t.Elem()
}
if t.Kind() != reflect.Struct || depth > 6 || seen[t] {
return dyn.NilValue
}
seen[t] = true
defer delete(seen, t)

m := dyn.NewMapping()
for f := range t.Fields() {
if f.Anonymous {
if sub := zeroValueScalars(f.Type, depth+1, seen); sub.Kind() == dyn.KindMap {
for _, p := range sub.MustMap().Pairs() {
m.SetLoc(p.Key.MustString(), nil, p.Value)
}
}
continue
}

tag := structtag.JSONTag(f.Tag.Get("json"))
name := tag.Name()
if name == "" || name == "-" || !f.IsExported() || !tag.OmitEmpty() {
continue
}

switch f.Type.Kind() {
case reflect.Bool:
m.SetLoc(name, nil, dyn.V(false))
case reflect.String:
m.SetLoc(name, nil, dyn.V(""))
case reflect.Int, reflect.Int32, reflect.Int64:
m.SetLoc(name, nil, dyn.V(int64(0)))
case reflect.Float32, reflect.Float64:
m.SetLoc(name, nil, dyn.V(float64(0)))
default:
// Only basic types are eligible for ForceSendFields; skip the rest.
}
}
return dyn.V(m)
}
72 changes: 42 additions & 30 deletions libs/dyn/convert/struct_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package convert
import (
"reflect"
"slices"
"strconv"
"strings"
"sync"

"github.com/databricks/cli/libs/dyn"
Expand All @@ -28,9 +30,12 @@ type structInfo struct {
// Maps JSON-name of the field to Golang struct name
GolangNames map[string]string

// ForceSendFieldsStructKey maps the JSON-name of the field to which ForceSendFields slice it belongs to:
// -1 for direct fields, embedded struct index for embedded fields
ForceSendFieldsStructKey map[string]int
// ForceSendFieldsStructKey maps the JSON-name of the field to the struct that
// owns the ForceSendFields slice it belongs to. The value is the index path to
// that struct (see indexPathKey); "" is the top-level struct. The path can be
// more than one element deep because a field's declaring struct may be embedded
// several levels down (e.g. PostgresProject -> PostgresProjectConfig -> ProjectSpec).
ForceSendFieldsStructKey map[string]string
}

// structInfoCache caches type information.
Expand Down Expand Up @@ -61,7 +66,7 @@ func buildStructInfo(typ reflect.Type) structInfo {
Fields: make(map[string][]int),
ForceEmpty: make(map[string]bool),
GolangNames: make(map[string]string),
ForceSendFieldsStructKey: make(map[string]int),
ForceSendFieldsStructKey: make(map[string]string),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why can't we keep precalculated index? the structs are static, so it should always resolve to the same index, right?

}

// Queue holds the indexes of the structs to visit.
Expand Down Expand Up @@ -119,14 +124,9 @@ func buildStructInfo(typ reflect.Type) structInfo {
}
out.GolangNames[name] = sf.Name

// Determine which ForceSendFields this field belongs to
if len(prefix) == 0 {
// Direct field on the main struct
out.ForceSendFieldsStructKey[name] = -1
} else {
// Field on embedded struct
out.ForceSendFieldsStructKey[name] = prefix[0]
}
// The field is declared directly in the struct reached by prefix, so
// its ForceSendFields lives there. prefix is empty for the top-level struct.
out.ForceSendFieldsStructKey[name] = indexPathKey(prefix)
}
}

Expand Down Expand Up @@ -192,33 +192,45 @@ func (s *structInfo) FieldValues(v reflect.Value) []FieldValue {
// Type of [dyn.Value].
var configValueType = reflect.TypeFor[dyn.Value]()

// getForceSendFieldsValues collects ForceSendFields reflect.Values
// Returns map[structKey]reflect.Value where structKey is -1 for direct fields, embedded index for embedded fields
func getForceSendFieldsValues(v reflect.Value) map[int]reflect.Value {
if !v.IsValid() || v.Type().Kind() != reflect.Struct {
return make(map[int]reflect.Value)
}
// getForceSendFieldsValues collects the ForceSendFields slice declared directly
// by the top-level struct and by every embedded struct reachable through anonymous
// fields, at any depth. The result is keyed by the index path to each owning struct
// (see indexPathKey), matching structInfo.ForceSendFieldsStructKey. Embedding can be
// arbitrarily deep (e.g. PostgresProject -> PostgresProjectConfig -> ProjectSpec),
// so each level is recorded under its own path rather than collapsed to one index.
func getForceSendFieldsValues(v reflect.Value) map[string]reflect.Value {
result := make(map[string]reflect.Value)
collectForceSendFieldsValues(v, nil, result)
return result
}

result := make(map[int]reflect.Value)
func collectForceSendFieldsValues(v reflect.Value, path []int, result map[string]reflect.Value) {
v = deref(v)
if !v.IsValid() || v.Kind() != reflect.Struct {
return
}

for i := range v.Type().NumField() {
field := v.Type().Field(i)
fieldValue := v.Field(i)

if field.Name == "ForceSendFields" && !field.Anonymous {
// Direct ForceSendFields (structKey = -1)
result[-1] = fieldValue
} else if field.Anonymous {
// Embedded struct - check for ForceSendFields inside it
if embeddedStruct := deref(fieldValue); embeddedStruct.IsValid() {
if forceSendField := embeddedStruct.FieldByName("ForceSendFields"); forceSendField.IsValid() {
result[i] = forceSendField
}
}
switch {
case field.Name == "ForceSendFields" && !field.Anonymous:
result[indexPathKey(path)] = fieldValue
case field.Anonymous:
collectForceSendFieldsValues(fieldValue, append(path, i), result)
}
}
}

return result
// indexPathKey renders a reflect index path as a stable map key. The empty path
// (the top-level struct) renders as "".
func indexPathKey(path []int) string {
parts := make([]string, len(path))
for i, x := range path {
parts[i] = strconv.Itoa(x)
}
return strings.Join(parts, ".")
}

// deref dereferences a pointer, returning invalid value if nil
Expand Down
13 changes: 4 additions & 9 deletions libs/dyn/convert/to_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ func toTypedStruct(dst reflect.Value, src dyn.Value) error {
info := getStructInfo(dst.Type())

forceSendFieldLocations := getForceSendFieldsValues(dst)
forceSendFieldsMap := make(map[int][]string)
forceSendFieldsMap := make(map[string][]string)

for _, pair := range src.MustMap().Pairs() {
pk := pair.Key
Expand Down Expand Up @@ -111,14 +111,9 @@ func toTypedStruct(dst reflect.Value, src dyn.Value) error {
}

if pv.IsZero() {
// Use first index as key: -1 for direct fields, struct index for embedded fields
var structKey int
if len(index) == 1 {
structKey = -1 // Direct field
} else {
structKey = index[0] // Embedded struct index
}

// The field's zero value must still serialize, so record its Go name
// in the ForceSendFields of the struct that declares it.
structKey := info.ForceSendFieldsStructKey[jsonKey]
forceSendFieldsMap[structKey] = append(forceSendFieldsMap[structKey], info.GolangNames[jsonKey])
}
}
Expand Down
34 changes: 34 additions & 0 deletions libs/dyn/convert/to_typed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,3 +754,37 @@ func TestToTypedFieldByNameBugRegressionTest(t *testing.T) {
assert.Equal(t, "test-job", out.Name)
assert.Empty(t, out.Permissions)
}

func TestToTypedDeeplyEmbeddedStructForceSendFields(t *testing.T) {
// Mirrors resources.PostgresProject -> PostgresProjectConfig -> ProjectSpec:
// Spec is embedded two levels down and Wrapper shadows ForceSendFields to keep
// its own direct field out of Spec's ForceSendFields. A zero-value spec field
// must route to Spec.ForceSendFields, not Wrapper's, otherwise the SDK marshaler
// fails with "field ... cannot be found in struct".
type Spec struct {
SpecField bool `json:"spec_field,omitempty"`
ForceSendFields []string `json:"-"`
}

type Wrapper struct {
Spec
WrapperField string `json:"wrapper_field,omitempty"`
ForceSendFields []string `json:"-"`
}

type Outer struct {
Wrapper
}

var out Outer
m := dyn.Mapping{}
m.SetLoc("spec_field", nil, dyn.V(false))
m.SetLoc("wrapper_field", nil, dyn.V(""))
v := dyn.V(m)

err := ToTyped(&out, v)
require.NoError(t, err)

assert.Equal(t, []string{"SpecField"}, out.Spec.ForceSendFields)
assert.Equal(t, []string{"WrapperField"}, out.ForceSendFields)
}
Loading