Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v1.4.1
v1.4.2
21 changes: 16 additions & 5 deletions field.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"sort"
"strings"

"github.com/gogo/protobuf/gogoproto"
"github.com/gravitational/trace"
)

Expand Down Expand Up @@ -158,7 +159,7 @@ func BuildFields(m MessageBuildContext) ([]*Field, error) {
}

if f != nil {
fields = append(fields, f)
fields = append(fields, f...)
}
}

Expand All @@ -173,7 +174,7 @@ func BuildFields(m MessageBuildContext) ([]*Field, error) {
}

// BuildField builds Field structure
func BuildField(c *FieldBuildContext) (*Field, error) {
func BuildField(c *FieldBuildContext) ([]*Field, error) {
var err error

if c.IsExcluded() {
Expand Down Expand Up @@ -208,6 +209,11 @@ func BuildField(c *FieldBuildContext) (*Field, error) {
if err != nil {
return nil, trace.Wrap(err)
}

// If this is an embedded field, return message's fields instead of creating another field
if gogoproto.IsEmbed(c.field.FieldDescriptorProto) {
return f.Message.Fields, nil
}
}

if c.IsRepeated() {
Expand All @@ -233,7 +239,7 @@ func BuildField(c *FieldBuildContext) (*Field, error) {
f.OneOfType = c.GetOneOfTypeName()
}

return f, nil
return []*Field{f}, nil
}

// BuildPlaceholderField represents no-field-message single field placeholder
Expand Down Expand Up @@ -270,11 +276,16 @@ func (f *Field) setMapValues(c *FieldBuildContext) error {
var err error

// gogoprotobuf returns incorrect elem type for maps. It always contains "*", we have to override.
typ, f.MapValueField, err = f.getMapValueField(c)
typ, fs, err := f.getMapValueField(c)
if err != nil {
return trace.Wrap(err)
}

if len(fs) == 0 {
return trace.BadParameter("expected at least one field")
}
f.MapValueField = fs[0]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why is only the first field used?

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.

Good question, pinging @marcoandredinis for reply.

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.

It looks like for maps this can only ever be of size 0. When this is an embedded struct, it can return multiple fields. If it's not an embedded struct, then it just returns a single element array.


// Otherwise, that would contain artificial protobuf Map_Entry type information
f.GoType = typ
f.IsNullable = strings.Contains(typ, "*")
Expand Down Expand Up @@ -324,7 +335,7 @@ func (f *Field) getMessage(c *FieldBuildContext) (*Message, error) {
}

// getMapValueField returns map value field for a field
func (f *Field) getMapValueField(c *FieldBuildContext) (string, *Field, error) {
func (f *Field) getMapValueField(c *FieldBuildContext) (string, []*Field, error) {
// For some reason, gogoprotobuf incorrectly treats nullable status when map value is a message.
// We have to override it.
typ, d, err := c.GetMapValueFieldDescriptorAndType()
Expand Down
5 changes: 5 additions & 0 deletions field_build_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ type FieldBuildContext struct {
func NewFieldBuildContext(m MessageBuildContext, field *FieldDescriptorProtoExt, index int) (*FieldBuildContext, error) {
typeName := m.GetName() + "." + field.GetName()
path := m.GetPath() + "." + field.GetName()
// If the field is an embedded field, path should be
// message name, instead of full path to message name.
if gogoproto.IsEmbed(field.FieldDescriptorProto) {
path = m.GetName()
}

var t string

Expand Down
10 changes: 10 additions & 0 deletions test/copy_from_terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,13 @@ func TestCopyFromOneOfObjectNoBranch(t *testing.T) {

require.Equal(t, nil, target.OneOf)
}

func TestCopyFromEmbeddedField(t *testing.T) {
obj := copyFromTerraformObject(t)

target := Test{}
require.False(t, CopyTestFromTerraform(context.Background(), obj, &target).HasError())

require.Equal(t, "embdtest1", target.EmbStr)
require.Equal(t, "embdtest2", target.EmbeddedNestedField.EmbNStr)
}
15 changes: 15 additions & 0 deletions test/copy_to_terraform_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,3 +318,18 @@ func TestCopyToOneOfNoBranch(t *testing.T) {
require.True(t, o.Attrs["branch2"].(types.Object).Null)
require.True(t, o.Attrs["branch3"].(types.String).Null)
}

func TestCopyToEmbeddedField(t *testing.T) {
o := copyToTerraformObject(t)
testObj := createTestObj()

diags := CopyTestToTerraform(context.Background(), testObj, &o)
require.False(t, diags.HasError())

require.Equal(t, "embdtest1", o.Attrs["embedded_string"].(types.String).Value)
require.False(t, o.Attrs["embedded_string"].(types.String).Unknown)
require.False(t, o.Attrs["embedded_string"].(types.String).Null)

require.Equal(t, "embdtest2", o.Attrs["embedded_nested_field"].(types.Object).Attrs["embedded_nested_string"].(types.String).Value)

}
23 changes: 23 additions & 0 deletions test/fixtures.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,13 @@ func createTestObj() Test {
},

Map: map[string]string{"key1": "value1", "key2": "value2"},

EmbeddedField: EmbeddedField{
EmbStr: "embdtest1",
EmbeddedNestedField: &EmbeddedNestedField{
EmbNStr: "embdtest2",
},
},
}
}

Expand Down Expand Up @@ -409,6 +416,22 @@ func copyFromTerraformObject(t *testing.T) types.Object {
"branch3": types.String{Null: true},
"empty_message_branch": types.Object{Null: true},
"string_branch": types.String{Null: true},
"embedded_string": types.String{Value: "embdtest1"},
"embedded_nested_field": types.Object{
Attrs: map[string]attr.Value{
"embedded_nested_string": types.String{Value: "embdtest2"},
},
},
// "emb_field": types.Object{
// Attrs: map[string]attr.Value{
// "EmbStr": types.String{Value: "embdtest1"},
// "embedded_nested_field": types.Object{
// Attrs: map[string]attr.Value{
// "EmbNStr": types.String{Value: "embdtest2"},
// },
// },
// },
// },
Comment thread
flyinghermit marked this conversation as resolved.
Outdated
},
AttrTypes: obj.AttrTypes,
}
Expand Down
Loading