Properly deal with nil values in convert.FromTyped

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,24 @@ 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`).
+ // The previous method used [json.Marshal] from the standard library
+ // and as such observed the `omitempty` tag.
+ // The new method leverages [dyn.Value] where any field that is not
+ // explicitly set is not part of the value.
+ 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

@@ -12,26 +12,24 @@ import (
 type fromTypedOptions int
 
 const (
-	// If this flag is set, zero values for scalars (strings, bools, ints, floats)
-	// would resolve to corresponding zero values in the dynamic representation.
-	// Otherwise, zero values for scalars resolve to dyn.NilValue.
+	// If this flag is set, zero values in the typed representation are resolved to
+	// the equivalent zero value in the dynamic representation.
+	// If it is not set, zero values resolve to [dyn.NilValue].
 	//
-	// This flag exists to reconcile the default values for scalars in a Go struct
-	// being zero values with zero values in a dynamic representation. In a Go struct,
-	// zero values are the same as the values not being set at all. This is not the case
-	// in the dynamic representation.
-	//
-	// If a scalar value in a typed Go struct is zero, in the dynamic representation
-	// we would set it to dyn.NilValue, i.e. equivalent to the value not being set at all.
-	//
-	// If a scalar value in a Go map, slice or pointer is set to zero, we will set it
-	// to the zero value in the dynamic representation, and not dyn.NilValue. This is
-	// equivalent to the value being intentionally set to zero.
-	includeZeroValuedScalars fromTypedOptions = 1 << iota
+	// This flag exists to reconcile default values in Go being zero values with values
+	// being intentionally set to their zero value. We capture zero values in the dynamic
+	// configuration if they are 1) behind a pointer, 2) a map value, 3) a slice element,
+	// in the typed configuration.
+	includeZeroValues fromTypedOptions = 1 << iota
 )
 
 // FromTyped converts changes made in the typed structure w.r.t. the configuration value
 // back to the configuration value, retaining existing location information where possible.
+//
+// It uses the reference value both for location information and to determine if the typed
+// value was changed or not. For example, if a struct-by-value field is nil in the reference
+// it will be zero-valued in the typed configuration. If it remains zero-valued, this
+// this function will still emit a nil value in the dynamic representation.
 func FromTyped(src any, ref dyn.Value) (dyn.Value, error) {
 	return fromTyped(src, ref)
 }
@@ -48,18 +46,18 @@ func fromTyped(src any, ref dyn.Value, options ...fromTypedOptions) (dyn.Value,
  }
  srcv = srcv.Elem()
 
- // If a pointer to a scalar type points to a zero value, we should include
+ // If a pointer to a type points to a zero value, we should include
  // that zero value in the dynamic representation.
  // This is because by default a pointer is nil in Go, and it not being nil
  // indicates its value was intentionally set to zero.
- if !slices.Contains(options, includeZeroValuedScalars) {
- options = append(options, includeZeroValuedScalars)
+ if !slices.Contains(options, includeZeroValues) {
+ options = append(options, includeZeroValues)
  }
 	}
 
 	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:
@@ -80,7 +78,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:
@@ -108,12 +106,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 "includeZeroValues" option is set (i.e. the struct is a non-nil pointer).
+	if out.Len() > 0 || ref.Kind() == dyn.KindMap || slices.Contains(options, includeZeroValues) {
+ 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) {
@@ -146,7 +154,7 @@ func fromTypedMap(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
  }
 
  // Convert entry taking into account the reference value (may be equal to dyn.NilValue).
- nv, err := fromTyped(v.Interface(), refv, includeZeroValuedScalars)
+ nv, err := fromTyped(v.Interface(), refv, includeZeroValues)
  if err != nil {
  return dyn.InvalidValue, err
  }
@@ -183,7 +191,7 @@ func fromTypedSlice(src reflect.Value, ref dyn.Value) (dyn.Value, error) {
  }
 
  // Convert entry taking into account the reference value (may be equal to dyn.NilValue).
- nv, err := fromTyped(v.Interface(), refv, includeZeroValuedScalars)
+ nv, err := fromTyped(v.Interface(), refv, includeZeroValues)
  if err != nil {
  return dyn.InvalidValue, err
  }
@@ -206,7 +214,7 @@ func fromTypedString(src reflect.Value, ref dyn.Value, options ...fromTypedOptio
 	case dyn.KindNil:
  // This field is not set in the reference. We set it to nil if it's zero
  // valued in the typed representation and the includeZeroValues option is not set.
- if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
+ if src.IsZero() && !slices.Contains(options, includeZeroValues) {
  return dyn.NilValue, nil
  }
  return dyn.V(src.String()), nil
@@ -226,7 +234,7 @@ func fromTypedBool(src reflect.Value, ref dyn.Value, options ...fromTypedOptions
 	case dyn.KindNil:
  // This field is not set in the reference. We set it to nil if it's zero
  // valued in the typed representation and the includeZeroValues option is not set.
- if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
+ if src.IsZero() && !slices.Contains(options, includeZeroValues) {
  return dyn.NilValue, nil
  }
  return dyn.V(src.Bool()), nil
@@ -251,7 +259,7 @@ func fromTypedInt(src reflect.Value, ref dyn.Value, options ...fromTypedOptions)
 	case dyn.KindNil:
  // This field is not set in the reference. We set it to nil if it's zero
  // valued in the typed representation and the includeZeroValues option is not set.
- if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
+ if src.IsZero() && !slices.Contains(options, includeZeroValues) {
  return dyn.NilValue, nil
  }
  return dyn.V(src.Int()), nil
@@ -276,7 +284,7 @@ func fromTypedFloat(src reflect.Value, ref dyn.Value, options ...fromTypedOption
 	case dyn.KindNil:
  // This field is not set in the reference. We set it to nil if it's zero
  // valued in the typed representation and the includeZeroValues option is not set.
- if src.IsZero() && !slices.Contains(options, includeZeroValuedScalars) {
+ if src.IsZero() && !slices.Contains(options, includeZeroValues) {
  return dyn.NilValue, nil
  }
  return dyn.V(src.Float()), nil

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) {