Skip to content

Commit cafa676

Browse files
authored
Fix validations based on fields imported from ECS (#1452)
We inject fields from ECS on different scenarios: when generating documentation, when building packages, or when validating documents in tests. On #1335 we did a refactor around this, to fix some validation issues, in a way that more code is reused between all these uses. After this change, validation used field definitions that include the already resolved external fields, but we weren't including there information that was used by validators, what included validation of expected and allowed values. So these validations haven't been executed since then. Tests have been included to try to avoid regressions related to this in the future.
1 parent 4fb2e26 commit cafa676

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

53 files changed

+73561
-7
lines changed

internal/fields/dependency_manager.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
const (
2525
ecsSchemaName = "ecs"
2626
gitReferencePrefix = "git@"
27+
localFilePrefix = "file://"
2728

2829
ecsSchemaFile = "ecs_nested.yml"
2930
ecsSchemaURL = "https://raw.githubusercontent.com/elastic/ecs/%s/generated/ecs/%s"
@@ -70,6 +71,11 @@ func loadECSFieldsSchema(dep buildmanifest.ECSDependency) ([]FieldDefinition, er
7071
}
7172

7273
func readECSFieldsSchemaFile(dep buildmanifest.ECSDependency) ([]byte, error) {
74+
if strings.HasPrefix(dep.Reference, localFilePrefix) {
75+
path := strings.TrimPrefix(dep.Reference, localFilePrefix)
76+
return os.ReadFile(path)
77+
}
78+
7379
gitReference, err := asGitReference(dep.Reference)
7480
if err != nil {
7581
return nil, fmt.Errorf("can't process the value as Git reference: %w", err)
@@ -152,6 +158,10 @@ type InjectFieldsOptions struct {
152158
// ECS fields at the top level, when they cannot be reused there.
153159
DisallowReusableECSFieldsAtTopLevel bool
154160

161+
// IncludeValidationSettings can be set to enable the injection of settings of imported
162+
// fields that are only used for validation of documents, but are not needed on built packages.
163+
IncludeValidationSettings bool
164+
155165
root string
156166
}
157167

@@ -182,7 +192,7 @@ func (dm *DependencyManager) injectFieldsWithOptions(defs []common.MapStr, optio
182192
return nil, false, fmt.Errorf("field %s cannot be reused at top level", fieldPath)
183193
}
184194

185-
transformed := transformImportedField(imported)
195+
transformed := transformImportedField(imported, options)
186196

187197
// Allow overrides of everything, except the imported type, for consistency.
188198
transformed.DeepUpdate(def)
@@ -295,7 +305,7 @@ func buildFieldPath(root string, field common.MapStr) string {
295305
return path
296306
}
297307

298-
func transformImportedField(fd FieldDefinition) common.MapStr {
308+
func transformImportedField(fd FieldDefinition, options InjectFieldsOptions) common.MapStr {
299309
m := common.MapStr{
300310
"name": fd.Name,
301311
"type": fd.Type,
@@ -318,17 +328,28 @@ func transformImportedField(fd FieldDefinition) common.MapStr {
318328
m["doc_values"] = *fd.DocValues
319329
}
320330

321-
if len(fd.Normalize) > 0 {
322-
m["normalize"] = fd.Normalize
323-
}
324-
325331
if len(fd.MultiFields) > 0 {
326332
var t []common.MapStr
327333
for _, f := range fd.MultiFields {
328-
i := transformImportedField(f)
334+
i := transformImportedField(f, options)
329335
t = append(t, i)
330336
}
331337
m.Put("multi_fields", t)
332338
}
339+
340+
if options.IncludeValidationSettings {
341+
if len(fd.Normalize) > 0 {
342+
m["normalize"] = fd.Normalize
343+
}
344+
345+
if len(fd.AllowedValues) > 0 {
346+
m["allowed_values"] = fd.AllowedValues
347+
}
348+
349+
if len(fd.ExpectedValues) > 0 {
350+
m["expected_values"] = fd.ExpectedValues
351+
}
352+
}
353+
333354
return m
334355
}

internal/fields/dependency_manager_test.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@
55
package fields
66

77
import (
8+
"encoding/json"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
1113

1214
"github.com/elastic/elastic-package/internal/common"
15+
"github.com/elastic/elastic-package/internal/packages/buildmanifest"
1316
)
1417

1518
func TestDependencyManagerInjectExternalFields(t *testing.T) {
@@ -230,6 +233,9 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
230233
"external": "test",
231234
},
232235
},
236+
options: InjectFieldsOptions{
237+
IncludeValidationSettings: true,
238+
},
233239
result: []common.MapStr{
234240
{
235241
"name": "host.ip",
@@ -607,3 +613,116 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
607613
})
608614
}
609615
}
616+
617+
func TestDependencyManagerWithECS(t *testing.T) {
618+
const ecsNestedPath8_10_0 = "./testdata/ecs_nested_v8.10.0.yml"
619+
deps := buildmanifest.Dependencies{
620+
ECS: buildmanifest.ECSDependency{
621+
Reference: "file://" + ecsNestedPath8_10_0,
622+
},
623+
}
624+
dm, err := CreateFieldDependencyManager(deps)
625+
require.NoError(t, err)
626+
627+
cases := []struct {
628+
title string
629+
defs []common.MapStr
630+
result []common.MapStr
631+
options InjectFieldsOptions
632+
checkFn func(*testing.T, []common.MapStr)
633+
valid bool
634+
}{
635+
{
636+
title: "disallowed reusable field at lop level",
637+
defs: []common.MapStr{
638+
{
639+
"name": "geo.city_name",
640+
"external": "ecs",
641+
},
642+
},
643+
options: InjectFieldsOptions{
644+
DisallowReusableECSFieldsAtTopLevel: true,
645+
},
646+
valid: false,
647+
},
648+
{
649+
title: "legacy support to reuse field at lop level",
650+
defs: []common.MapStr{
651+
{
652+
"name": "geo.city_name",
653+
"external": "ecs",
654+
},
655+
},
656+
options: InjectFieldsOptions{
657+
DisallowReusableECSFieldsAtTopLevel: false,
658+
},
659+
result: []common.MapStr{
660+
{
661+
"name": "geo.city_name",
662+
"description": "City name.",
663+
"type": "keyword",
664+
},
665+
},
666+
valid: true,
667+
},
668+
{
669+
title: "allowed values are injected for validation",
670+
defs: []common.MapStr{
671+
{
672+
"name": "event.type",
673+
"external": "ecs",
674+
},
675+
},
676+
options: InjectFieldsOptions{
677+
IncludeValidationSettings: true,
678+
},
679+
valid: true,
680+
checkFn: func(t *testing.T, result []common.MapStr) {
681+
require.Len(t, result, 1)
682+
_, ok := result[0]["allowed_values"]
683+
if !assert.True(t, ok) {
684+
d, _ := json.MarshalIndent(result[0], "", " ")
685+
t.Logf("expected to find allowed_values in %s", string(d))
686+
}
687+
},
688+
},
689+
{
690+
title: "allowed values are not injected when not intended for validation",
691+
defs: []common.MapStr{
692+
{
693+
"name": "event.type",
694+
"external": "ecs",
695+
},
696+
},
697+
options: InjectFieldsOptions{
698+
IncludeValidationSettings: false,
699+
},
700+
valid: true,
701+
checkFn: func(t *testing.T, result []common.MapStr) {
702+
require.Len(t, result, 1)
703+
_, ok := result[0]["allowed_values"]
704+
assert.False(t, ok)
705+
},
706+
},
707+
}
708+
709+
for _, c := range cases {
710+
t.Run(c.title, func(t *testing.T) {
711+
result, _, err := dm.InjectFieldsWithOptions(c.defs, c.options)
712+
if !c.valid {
713+
assert.Error(t, err)
714+
return
715+
}
716+
717+
assert.NoError(t, err)
718+
if len(c.result) > 0 {
719+
assert.EqualValues(t, c.result, result)
720+
}
721+
if c.checkFn != nil {
722+
t.Run("checkFn", func(t *testing.T) {
723+
c.checkFn(t, result)
724+
})
725+
}
726+
})
727+
}
728+
}

0 commit comments

Comments
 (0)