Skip to content

Commit beabd87

Browse files
authored
Validate arrays of objects (#1489)
Add validation for arrays of objects, considering them as valid when they are inside nested or group. When group is used, a filterable error is given, mentioning that nested should be used instead on this case.
1 parent 29ded9d commit beabd87

File tree

2 files changed

+93
-12
lines changed

2 files changed

+93
-12
lines changed

internal/fields/validate.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ import (
3232
var (
3333
semver2_0_0 = semver.MustParse("2.0.0")
3434
semver2_3_0 = semver.MustParse("2.3.0")
35+
semver3_0_0 = semver.MustParse("3.0.0")
3536

3637
defaultExternal = "ecs"
38+
39+
errArrayOfObjects = errors.New("array of objects not used as nested type can lead to unexpected results")
3740
)
3841

3942
// Validator is responsible for fields validation.
@@ -783,11 +786,22 @@ func (v *Validator) parseSingleElementValue(key string, definition FieldDefiniti
783786
return fmt.Errorf("the IP %q is not one of the allowed test IPs (see: https://github.com/elastic/elastic-package/blob/main/internal/fields/_static/allowed_geo_ips.txt)", valStr)
784787
}
785788
// Groups should only contain nested fields, not single values.
786-
case "group":
787-
switch val.(type) {
789+
case "group", "nested":
790+
switch val := val.(type) {
788791
case map[string]interface{}:
789-
// TODO: This is probably an element from an array of objects,
792+
// This is probably an element from an array of objects,
790793
// even if not recommended, it should be validated.
794+
if v.specVersion.LessThan(semver3_0_0) {
795+
break
796+
}
797+
errs := v.validateMapElement(key, common.MapStr(val), doc)
798+
if definition.Type == "group" {
799+
errs = append(errs, errArrayOfObjects)
800+
}
801+
if len(errs) == 0 {
802+
return nil
803+
}
804+
return errs
791805
default:
792806
return fmt.Errorf("field %q is a group of fields, it cannot store values", key)
793807
}

internal/fields/validate_test.go

Lines changed: 76 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,12 @@ import (
1111
"sort"
1212
"testing"
1313

14+
"github.com/Masterminds/semver/v3"
1415
"github.com/stretchr/testify/assert"
1516
"github.com/stretchr/testify/require"
1617

1718
"github.com/elastic/elastic-package/internal/common"
19+
"github.com/elastic/elastic-package/internal/multierror"
1820
)
1921

2022
type results struct {
@@ -309,10 +311,12 @@ func TestValidate_ExpectedDatasets(t *testing.T) {
309311

310312
func Test_parseElementValue(t *testing.T) {
311313
for _, test := range []struct {
312-
key string
313-
value interface{}
314-
definition FieldDefinition
315-
fail bool
314+
key string
315+
value interface{}
316+
definition FieldDefinition
317+
fail bool
318+
assertError func(t *testing.T, err error)
319+
specVersion semver.Version
316320
}{
317321
// Arrays
318322
{
@@ -619,17 +623,80 @@ func Test_parseElementValue(t *testing.T) {
619623
},
620624
},
621625
},
626+
// elements in arrays of objects should be validated
627+
{
628+
key: "details",
629+
value: []interface{}{
630+
map[string]interface{}{
631+
"id": "somehost-id",
632+
"hostname": "somehost",
633+
},
634+
},
635+
definition: FieldDefinition{
636+
Name: "details",
637+
Type: "group",
638+
Fields: []FieldDefinition{
639+
{
640+
Name: "id",
641+
Type: "keyword",
642+
},
643+
},
644+
},
645+
specVersion: *semver3_0_0,
646+
fail: true,
647+
assertError: func(t *testing.T, err error) {
648+
errs := err.(multierror.Error)
649+
if assert.Len(t, errs, 2) {
650+
assert.Contains(t, errs[0].Error(), `"details.hostname" is undefined`)
651+
assert.ErrorIs(t, errs[1], errArrayOfObjects)
652+
}
653+
},
654+
},
655+
// elements in nested objects
656+
{
657+
key: "nested",
658+
value: []interface{}{
659+
map[string]interface{}{
660+
"id": "somehost-id",
661+
"hostname": "somehost",
662+
},
663+
},
664+
definition: FieldDefinition{
665+
Name: "nested",
666+
Type: "nested",
667+
Fields: []FieldDefinition{
668+
{
669+
Name: "id",
670+
Type: "keyword",
671+
},
672+
},
673+
},
674+
specVersion: *semver3_0_0,
675+
fail: true,
676+
assertError: func(t *testing.T, err error) {
677+
errs := err.(multierror.Error)
678+
if assert.Len(t, errs, 1) {
679+
assert.Contains(t, errs[0].Error(), `"nested.hostname" is undefined`)
680+
}
681+
},
682+
},
622683
} {
623-
v := Validator{
624-
disabledDependencyManagement: true,
625-
enabledAllowedIPCheck: true,
626-
allowedCIDRs: initializeAllowedCIDRsList(),
627-
}
628684

629685
t.Run(test.key, func(t *testing.T) {
686+
v := Validator{
687+
Schema: []FieldDefinition{test.definition},
688+
disabledDependencyManagement: true,
689+
enabledAllowedIPCheck: true,
690+
allowedCIDRs: initializeAllowedCIDRsList(),
691+
specVersion: test.specVersion,
692+
}
693+
630694
err := v.parseElementValue(test.key, test.definition, test.value, common.MapStr{})
631695
if test.fail {
632696
require.Error(t, err)
697+
if test.assertError != nil {
698+
test.assertError(t, err)
699+
}
633700
} else {
634701
require.NoError(t, err)
635702
}

0 commit comments

Comments
 (0)