Properly deal with nil values in convert.FromTyped

bundle/config/mutator/process_target_mode.go

@@ -9,7 +9,6 @@ import (
 	"github.com/databricks/cli/bundle/config"
 	"github.com/databricks/cli/libs/auth"
 	"github.com/databricks/cli/libs/diag"
-	"github.com/databricks/cli/libs/dyn"
 	"github.com/databricks/cli/libs/log"
 	"github.com/databricks/databricks-sdk-go/service/catalog"
 	"github.com/databricks/databricks-sdk-go/service/jobs"
@@ -34,10 +33,8 @@ func (m *processTargetMode) Name() string {
 func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
 	if !b.Config.Bundle.Deployment.Lock.IsExplicitlyEnabled() {
 		log.Infof(ctx, "Development mode: disabling deployment lock since bundle.deployment.lock.enabled is not set to true")
-		err := disableDeploymentLock(b)
-		if err != nil {
-			return diag.FromErr(err)
-		}
+		disable := false
+		b.Config.Bundle.Deployment.Lock.Enabled = &disable
 	}
 
 	r := b.Config.Resources
@@ -118,14 +115,6 @@ func transformDevelopmentMode(ctx context.Context, b *bundle.Bundle) diag.Diagno
 	return nil
 }
 
-func disableDeploymentLock(b *bundle.Bundle) error {
-	return b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
-		return dyn.Map(v, "bundle.deployment.lock", func(_ dyn.Path, v dyn.Value) (dyn.Value, error) {
-			return dyn.Set(v, "enabled", dyn.V(false))
-		})
-	})
-}
-
 func validateDevelopmentMode(b *bundle.Bundle) diag.Diagnostics {
 	if path := findNonUserPath(b); path != "" {
 		return diag.Errorf("%s must start with '~/' or contain the current username when using 'mode: development'", path)

bundle/config/mutator/run_as_test.go

@@ -18,7 +18,7 @@ import (
 
 func allResourceTypes(t *testing.T) []string {
 	// Compute supported resource types based on the `Resources{}` struct.
-	r := config.Resources{}
+	r := &config.Resources{}
 	rv, err := convert.FromTyped(r, dyn.NilValue)
 	require.NoError(t, err)
 	normalized, _ := convert.Normalize(r, rv, convert.IncludeMissingFields)
@@ -154,6 +154,11 @@ func TestRunAsErrorForUnsupportedResources(t *testing.T) {
 	v, err := convert.FromTyped(base, dyn.NilValue)
 	require.NoError(t, err)
 
+	// Define top level resources key in the bundle configuration.
+	// This is not part of the typed configuration, so we need to add it manually.
+	v, err = dyn.Set(v, "resources", dyn.V(map[string]dyn.Value{}))
+	require.NoError(t, err)
+
 	for _, rt := range allResourceTypes(t) {
 		// Skip allowed resources
 		if slices.Contains(allowList, rt) {

bundle/deploy/terraform/convert_test.go

@@ -455,6 +455,20 @@ func TestBundleToTerraformModelServingPermissions(t *testing.T) {
 	var src = resources.ModelServingEndpoint{
 		CreateServingEndpoint: &serving.CreateServingEndpoint{
 			Name: "name",
+
+			// Need to specify this to satisfy the equivalence test:
+			// The previous method of generation includes the "create" field
+			// because it is required (not marked as `omitempty`).
+			Config: serving.EndpointCoreConfigInput{
+				ServedModels: []serving.ServedModelInput{
+					{
+						ModelName:          "model_name",
+						ModelVersion:       "1",
+						ScaleToZeroEnabled: true,
+						WorkloadSize:       "Small",
+					},
+				},
+			},
 		},
 		Permissions: []resources.Permission{
 			{

bundle/permissions/filter.go

@@ -66,6 +66,11 @@ func (m *filterCurrentUser) Apply(ctx context.Context, b *bundle.Bundle) diag.Di
 	err := b.Config.Mutate(func(v dyn.Value) (dyn.Value, error) {
 		rv, err := dyn.Get(v, "resources")
 		if err != nil {
+			// If the resources key is not found, we can skip this mutator.
+			if dyn.IsNoSuchKeyError(err) {
+				return v, nil
+			}
+
 			return dyn.InvalidValue, err
 		}
 

libs/dyn/convert/from_typed.go

@@ -59,7 +59,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
 
 	switch srcv.Kind() {
 	case reflect.Struct:
-		return fromTypedStruct(srcv, ref)
+		return fromTypedStruct(srcv, ref, options...)
 	case reflect.Map:
 		return fromTypedMap(srcv, ref)
 	case reflect.Slice:
@@ -77,7 +77,7 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
 	return dyn.InvalidValue, fmt.Errorf("unsupported type: %s", srcv.Kind())
 }
 
-func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
+func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptions) (dyn.Value, error) {
 	// Check that the reference value is compatible or nil.
 	switch ref.Kind() {
 	case dyn.KindMap, dyn.KindNil:
@@ -105,12 +105,22 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
 			return dyn.InvalidValue, err
 		}
 
-		if nv != dyn.NilValue {
+		// Either if the key was set in the reference or the field is not zero-valued, we include it.
+		if ok || nv != dyn.NilValue {
 			out.Set(refk, nv)
 		}
 	}
 
-	return dyn.NewValue(out, ref.Location()), nil
+	// Return the new mapping if:
+	// 1. The mapping has entries (i.e. the struct was not empty).
+	// 2. The reference is a map (i.e. the struct was and still is empty).
+	// 3. The "includeZeroValuedScalars" option is set (i.e. the struct is a non-nil pointer).
+	if out.Len() > 0 || ref.Kind() == dyn.KindMap || slices.Contains(options, includeZeroValuedScalars) {
+		return dyn.NewValue(out, ref.Location()), nil
+	}
+
+	// Otherwise, return nil.
+	return dyn.NilValue, nil
 }
 
 func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {

libs/dyn/convert/from_typed_test.go

@@ -15,9 +15,14 @@ func TestFromTypedStructZeroFields(t *testing.T) {
 	}
 
 	src := Tmp{}
-	ref := dyn.NilValue
 
-	nv, err := FromTyped(src, ref)
+	// For an empty struct with a nil reference we expect a nil.
+	nv, err := FromTyped(src, dyn.NilValue)
+	require.NoError(t, err)
+	assert.Equal(t, dyn.NilValue, nv)
+
+	// For an empty struct with a non-nil reference we expect an empty map.
+	nv, err = FromTyped(src, dyn.V(map[string]dyn.Value{}))
 	require.NoError(t, err)
 	assert.Equal(t, dyn.V(map[string]dyn.Value{}), nv)
 }
@@ -28,17 +33,54 @@ func TestFromTypedStructPointerZeroFields(t *testing.T) {
 		Bar string `json:"bar"`
 	}
 
-	// For an initialized pointer we expect an empty map.
-	src := &Tmp{}
-	nv, err := FromTyped(src, dyn.NilValue)
-	require.NoError(t, err)
-	assert.Equal(t, dyn.V(map[string]dyn.Value{}), nv)
+	var src *Tmp
+	var nv dyn.Value
+	var err error
 
-	// For a nil pointer we expect nil.
+	// For a nil pointer with a nil reference we expect a nil.
 	src = nil
 	nv, err = FromTyped(src, dyn.NilValue)
 	require.NoError(t, err)
 	assert.Equal(t, dyn.NilValue, nv)
+
+	// For a nil pointer with a non-nil reference we expect a nil.
+	src = nil
+	nv, err = FromTyped(src, dyn.V(map[string]dyn.Value{}))
+	require.NoError(t, err)
+	assert.Equal(t, dyn.NilValue, nv)
+
+	// For an initialized pointer with a nil reference we expect a nil.
+	src = &Tmp{}
+	nv, err = FromTyped(src, dyn.NilValue)
+	require.NoError(t, err)
+	assert.Equal(t, dyn.V(map[string]dyn.Value{}), nv)
+
+	// For an initialized pointer with a non-nil reference we expect an empty map.
+	src = &Tmp{}
+	nv, err = FromTyped(src, dyn.V(map[string]dyn.Value{}))
+	require.NoError(t, err)
+	assert.Equal(t, dyn.V(map[string]dyn.Value{}), nv)
+}
+
+func TestFromTypedStructNilFields(t *testing.T) {
+	type Tmp struct {
+		Foo string `json:"foo"`
+		Bar string `json:"bar"`
+	}
+
+	// For a zero value struct with a reference containing nil fields we expect the nils to be retained.
+	src := Tmp{}
+	ref := dyn.V(map[string]dyn.Value{
+		"foo": dyn.NilValue,
+		"bar": dyn.NilValue,
+	})
+
+	nv, err := FromTyped(src, ref)
+	require.NoError(t, err)
+	assert.Equal(t, dyn.V(map[string]dyn.Value{
+		"foo": dyn.NilValue,
+		"bar": dyn.NilValue,
+	}), nv)
 }
 
 func TestFromTypedStructSetFields(t *testing.T) {