packetbeat/pb/event.go - generate correct ECS mappings (#42116) (#42283)

This fixes the behavior of Packetbeat's MarshalMapStr() so that it will
properly recurse through nested structs when turning the ecs.Fields object into
a map. This might technically be a breaking change, since the process.parent
field will now produce a different (but correct) map.

Fixes #42108

(cherry picked from commit 525ed30)

Co-authored-by: Alex K. <8418476+fearful-symmetry@users.noreply.github.com>

CHANGELOG.next.asciidoc

@@ -232,6 +232,8 @@ otherwise no tag is added. {issue}42208[42208] {pull}42403[42403]
 
 *Packetbeat*
 
+- Properly marshal nested structs in ECS fields, fixing issues with mixed cases in field names {pull}42116[42116]
+
 
 *Winlogbeat*
 

packetbeat/pb/event.go

@@ -416,8 +416,23 @@ func marshalStruct(m mapstr.M, key string, val reflect.Value) error {
 	}
 
 	typ := val.Type()
+	// pre-emptively handle time
+	if reflect.TypeOf(time.Time{}) == typ {
+		_, err := m.Put(key, val.Interface())
+		if err != nil {
+			return fmt.Errorf("error creating time value: %w", err)
+		}
+		return nil
+	}
+
+	// NumField() will panic if we don't have a struct
+	if val.Type().Kind() != reflect.Struct {
+		return fmt.Errorf("value must be a struct or a pointer to a struct, but got %v at key %s", val.Type(), key)
+	}
+
 	for i := 0; i < typ.NumField(); i++ {
 		structField := typ.Field(i)
+
 		tag := getTag(structField)
 		if tag == "" {
 			continue
@@ -431,7 +446,7 @@ func marshalStruct(m mapstr.M, key string, val reflect.Value) error {
 				case "inline":
 					inline = true
 				default:
-					return fmt.Errorf("Unsupported flag %q in tag %q of type %s", flag, tag, typ)
+					return fmt.Errorf("unsupported flag %q in tag %q of type %s", flag, tag, typ)
 				}
 			}
 			tag = tags[0]
@@ -446,6 +461,13 @@ func marshalStruct(m mapstr.M, key string, val reflect.Value) error {
 			if err := marshalStruct(m, key, fieldValue); err != nil {
 				return err
 			}
+			// look for a struct or pointer to a struct
+			// that reflect.Ptr check is needed so Elem() doesn't panic
+		} else if (structField.Type.Kind() == reflect.Ptr && fieldValue.Elem().Kind() == reflect.Struct) ||
+			structField.Type.Kind() == reflect.Struct {
+			if err := marshalStruct(m, key+"."+tag, fieldValue); err != nil {
+				return err
+			}
 		} else {
 			if _, err := m.Put(key+"."+tag, fieldValue.Interface()); err != nil {
 				return err

packetbeat/pb/event_test.go

@@ -24,14 +24,80 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/beats/v7/libbeat/ecs"
 	"github.com/elastic/elastic-agent-libs/mapstr"
 )
 
+func TestTimeMarshal(t *testing.T) {
+	testTime := time.Now()
+	f := NewFields()
+
+	f.Process = &ecs.Process{
+		Start: testTime,
+		Parent: &ecs.Process{
+			Start: testTime,
+		},
+	}
+
+	m := mapstr.M{}
+	err := f.MarshalMapStr(m)
+	require.NoError(t, err)
+	procData := m["process"]
+	assert.Equal(t, testTime, procData.(mapstr.M)["start"])
+	assert.Equal(t, testTime, procData.(mapstr.M)["parent"].(mapstr.M)["start"])
+
+}
+
+func TestPointerHandling(t *testing.T) {
+	testInt := 10
+	testStr := "test"
+	// test to make to sure we correctly handle pointers that aren't structs
+	// mostly checking to make sure we don't panic due to pointer/reflect bugs
+	testStruct := struct {
+		PointerInt       *int         `ecs:"one"`
+		SecondPointerInt *int         `ecs:"two"`
+		TestStruct       *ecs.Process `ecs:"struct"`
+		StrPointer       *string      `ecs:"string"`
+	}{
+		PointerInt:       nil,
+		SecondPointerInt: &testInt,
+		StrPointer:       &testStr,
+		TestStruct: &ecs.Process{
+			Name: "Test",
+		},
+	}
+
+	out := mapstr.M{}
+	err := MarshalStruct(out, "test", testStruct)
+	require.NoError(t, err)
+
+	want := mapstr.M{
+		"test": mapstr.M{
+			"struct": mapstr.M{
+				"name": "Test",
+			},
+			"two":    &testInt,
+			"string": &testStr,
+		},
+	}
+
+	require.Equal(t, want, out)
+}
+
 func TestMarshalMapStr(t *testing.T) {
 	f := NewFields()
 	f.Source = &ecs.Source{IP: "127.0.0.1"}
+	// make sure recursion works properly
+	f.Process = &ecs.Process{
+		Parent: &ecs.Process{
+			Name: "Foo",
+			Parent: &ecs.Process{
+				Name: "Bar",
+			},
+		},
+	}
 
 	m := mapstr.M{}
 	if err := f.MarshalMapStr(m); err != nil {
@@ -45,6 +111,14 @@ func TestMarshalMapStr(t *testing.T) {
 			"type":     []string{"connection", "protocol"},
 		},
 		"source": mapstr.M{"ip": "127.0.0.1"},
+		"process": mapstr.M{
+			"parent": mapstr.M{
+				"name": "Foo",
+				"parent": mapstr.M{
+					"name": "Bar",
+				},
+			},
+		},
 	}, m)
 }