Skip to content

Commit

Permalink
Fix validations based on fields imported from ECS (#1452)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jsoriano authored Sep 18, 2023
1 parent 4fb2e26 commit cafa676
Show file tree
Hide file tree
Showing 53 changed files with 73,561 additions and 7 deletions.
35 changes: 28 additions & 7 deletions internal/fields/dependency_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
const (
ecsSchemaName = "ecs"
gitReferencePrefix = "git@"
localFilePrefix = "file://"

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

func readECSFieldsSchemaFile(dep buildmanifest.ECSDependency) ([]byte, error) {
if strings.HasPrefix(dep.Reference, localFilePrefix) {
path := strings.TrimPrefix(dep.Reference, localFilePrefix)
return os.ReadFile(path)
}

gitReference, err := asGitReference(dep.Reference)
if err != nil {
return nil, fmt.Errorf("can't process the value as Git reference: %w", err)
Expand Down Expand Up @@ -152,6 +158,10 @@ type InjectFieldsOptions struct {
// ECS fields at the top level, when they cannot be reused there.
DisallowReusableECSFieldsAtTopLevel bool

// IncludeValidationSettings can be set to enable the injection of settings of imported
// fields that are only used for validation of documents, but are not needed on built packages.
IncludeValidationSettings bool

root string
}

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

transformed := transformImportedField(imported)
transformed := transformImportedField(imported, options)

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

func transformImportedField(fd FieldDefinition) common.MapStr {
func transformImportedField(fd FieldDefinition, options InjectFieldsOptions) common.MapStr {
m := common.MapStr{
"name": fd.Name,
"type": fd.Type,
Expand All @@ -318,17 +328,28 @@ func transformImportedField(fd FieldDefinition) common.MapStr {
m["doc_values"] = *fd.DocValues
}

if len(fd.Normalize) > 0 {
m["normalize"] = fd.Normalize
}

if len(fd.MultiFields) > 0 {
var t []common.MapStr
for _, f := range fd.MultiFields {
i := transformImportedField(f)
i := transformImportedField(f, options)
t = append(t, i)
}
m.Put("multi_fields", t)
}

if options.IncludeValidationSettings {
if len(fd.Normalize) > 0 {
m["normalize"] = fd.Normalize
}

if len(fd.AllowedValues) > 0 {
m["allowed_values"] = fd.AllowedValues
}

if len(fd.ExpectedValues) > 0 {
m["expected_values"] = fd.ExpectedValues
}
}

return m
}
119 changes: 119 additions & 0 deletions internal/fields/dependency_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,14 @@
package fields

import (
"encoding/json"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/elastic/elastic-package/internal/common"
"github.com/elastic/elastic-package/internal/packages/buildmanifest"
)

func TestDependencyManagerInjectExternalFields(t *testing.T) {
Expand Down Expand Up @@ -230,6 +233,9 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
"external": "test",
},
},
options: InjectFieldsOptions{
IncludeValidationSettings: true,
},
result: []common.MapStr{
{
"name": "host.ip",
Expand Down Expand Up @@ -607,3 +613,116 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
})
}
}

func TestDependencyManagerWithECS(t *testing.T) {
const ecsNestedPath8_10_0 = "./testdata/ecs_nested_v8.10.0.yml"
deps := buildmanifest.Dependencies{
ECS: buildmanifest.ECSDependency{
Reference: "file://" + ecsNestedPath8_10_0,
},
}
dm, err := CreateFieldDependencyManager(deps)
require.NoError(t, err)

cases := []struct {
title string
defs []common.MapStr
result []common.MapStr
options InjectFieldsOptions
checkFn func(*testing.T, []common.MapStr)
valid bool
}{
{
title: "disallowed reusable field at lop level",
defs: []common.MapStr{
{
"name": "geo.city_name",
"external": "ecs",
},
},
options: InjectFieldsOptions{
DisallowReusableECSFieldsAtTopLevel: true,
},
valid: false,
},
{
title: "legacy support to reuse field at lop level",
defs: []common.MapStr{
{
"name": "geo.city_name",
"external": "ecs",
},
},
options: InjectFieldsOptions{
DisallowReusableECSFieldsAtTopLevel: false,
},
result: []common.MapStr{
{
"name": "geo.city_name",
"description": "City name.",
"type": "keyword",
},
},
valid: true,
},
{
title: "allowed values are injected for validation",
defs: []common.MapStr{
{
"name": "event.type",
"external": "ecs",
},
},
options: InjectFieldsOptions{
IncludeValidationSettings: true,
},
valid: true,
checkFn: func(t *testing.T, result []common.MapStr) {
require.Len(t, result, 1)
_, ok := result[0]["allowed_values"]
if !assert.True(t, ok) {
d, _ := json.MarshalIndent(result[0], "", " ")
t.Logf("expected to find allowed_values in %s", string(d))
}
},
},
{
title: "allowed values are not injected when not intended for validation",
defs: []common.MapStr{
{
"name": "event.type",
"external": "ecs",
},
},
options: InjectFieldsOptions{
IncludeValidationSettings: false,
},
valid: true,
checkFn: func(t *testing.T, result []common.MapStr) {
require.Len(t, result, 1)
_, ok := result[0]["allowed_values"]
assert.False(t, ok)
},
},
}

for _, c := range cases {
t.Run(c.title, func(t *testing.T) {
result, _, err := dm.InjectFieldsWithOptions(c.defs, c.options)
if !c.valid {
assert.Error(t, err)
return
}

assert.NoError(t, err)
if len(c.result) > 0 {
assert.EqualValues(t, c.result, result)
}
if c.checkFn != nil {
t.Run("checkFn", func(t *testing.T) {
c.checkFn(t, result)
})
}
})
}
}
Loading

0 comments on commit cafa676

Please sign in to comment.