Skip to content

Commit

Permalink
Validate expected fields normalization (#765)
Browse files Browse the repository at this point in the history
ECS fields can include normalization rules, indicated by the `normalize` parameter.

By now only the `array` rule exist, that indicates that values should be included in the
field as an array, even if there is a single element.

Validate this on test results for packages with `format_version` >= 2.0.0.
  • Loading branch information
jsoriano authored Sep 7, 2022
1 parent 20e8351 commit c64ba8d
Show file tree
Hide file tree
Showing 36 changed files with 1,377 additions and 1,062 deletions.
14 changes: 14 additions & 0 deletions internal/common/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,17 @@ func StringSliceContains(slice []string, s string) bool {
}
return false
}

// StringSlicesUnion joins multiple slices and returns an slice with the distinct
// elements of all of them.
func StringSlicesUnion(slices ...[]string) (result []string) {
for _, slice := range slices {
for _, elem := range slice {
if StringSliceContains(result, elem) {
continue
}
result = append(result, elem)
}
}
return
}
19 changes: 19 additions & 0 deletions internal/common/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,22 @@ func TestStringSliceContains(t *testing.T) {
assert.Equalf(t, c.expected, found, "checking if slice %v contains '%s'", c.slice, c.s)
}
}

func TestStringSlicesUnion(t *testing.T) {
cases := []struct {
slices [][]string
expected []string
}{
{nil, nil},
{[][]string{{"foo", "bar"}, nil}, []string{"foo", "bar"}},
{[][]string{nil, {"foo", "bar"}}, []string{"foo", "bar"}},
{[][]string{{"foo", "bar"}, {"foo", "bar"}}, []string{"foo", "bar"}},
{[][]string{{"foo", "baz"}, {"foo", "bar"}}, []string{"foo", "bar", "baz"}},
{[][]string{{"foo", "bar"}, {"foo", "baz"}}, []string{"foo", "bar", "baz"}},
}

for _, c := range cases {
result := StringSlicesUnion(c.slices...)
assert.ElementsMatch(t, c.expected, result)
}
}
2 changes: 1 addition & 1 deletion internal/configuration/locations/locations.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ var (
dockerCustomAgentDeployerDir = filepath.Join(deployerDir, "docker_custom_agent")
)

//LocationManager maintains an instance of a config path location
// LocationManager maintains an instance of a config path location
type LocationManager struct {
stackPath string
}
Expand Down
4 changes: 4 additions & 0 deletions internal/fields/dependency_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ 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 {
Expand Down
53 changes: 53 additions & 0 deletions internal/fields/dependency_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,51 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
changed: true,
valid: true,
},
{
title: "array field",
defs: []common.MapStr{
{
"name": "host.ip",
"external": "test",
},
},
result: []common.MapStr{
{
"name": "host.ip",
"type": "ip",
"description": "Host ip addresses.",
"normalize": []string{
"array",
},
},
},
changed: true,
valid: true,
},
{
title: "array field override",
defs: []common.MapStr{
{
"name": "container.id",
"external": "test",
"normalize": []string{
"array",
},
},
},
result: []common.MapStr{
{
"name": "container.id",
"type": "keyword",
"description": "Container identifier.",
"normalize": []string{
"array",
},
},
},
changed: true,
valid: true,
},
{
title: "unknown field",
defs: []common.MapStr{
Expand Down Expand Up @@ -335,6 +380,14 @@ func TestDependencyManagerInjectExternalFields(t *testing.T) {
Index: &indexFalse,
DocValues: &indexFalse,
},
{
Name: "host.ip",
Description: "Host ip addresses.",
Type: "ip",
Normalize: []string{
"array",
},
},
{
Name: "source.mac",
Description: "MAC address of the source.",
Expand Down
5 changes: 5 additions & 0 deletions internal/fields/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ type FieldDefinition struct {
External string `yaml:"external"`
Index *bool `yaml:"index"`
DocValues *bool `yaml:"doc_values"`
Normalize []string `yaml:"normalize,omitempty"`
Fields FieldDefinitions `yaml:"fields,omitempty"`
MultiFields []FieldDefinition `yaml:"multi_fields,omitempty"`
}
Expand Down Expand Up @@ -73,6 +74,10 @@ func (orig *FieldDefinition) Update(fd FieldDefinition) {
orig.DocValues = fd.DocValues
}

if len(fd.Normalize) > 0 {
orig.Normalize = common.StringSlicesUnion(orig.Normalize, fd.Normalize)
}

if len(fd.Fields) > 0 {
orig.Fields = updateFields(orig.Fields, fd.Fields)
}
Expand Down
4 changes: 4 additions & 0 deletions internal/fields/testdata/fields/fields.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,7 @@
value: correct
- name: ip_address
type: ip
- name: container.image.tag
type: keyword
normalize:
- array
3 changes: 3 additions & 0 deletions internal/fields/testdata/invalid-array-normalization.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"container.image.tag": "sometag"
}
3 changes: 3 additions & 0 deletions internal/fields/testdata/valid-array-normalization.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"container.image.tag": ["sometag"]
}
39 changes: 38 additions & 1 deletion internal/fields/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"regexp"
"strings"

"github.com/Masterminds/semver"
"github.com/pkg/errors"
"gopkg.in/yaml.v3"

Expand All @@ -32,6 +33,9 @@ type Validator struct {
// FieldDependencyManager resolves references to external fields
FieldDependencyManager *DependencyManager

// SpecVersion contains the version of the spec used by the package.
specVersion semver.Version

defaultNumericConversion bool
numericKeywordFields map[string]struct{}

Expand All @@ -44,6 +48,18 @@ type Validator struct {
// ValidatorOption represents an optional flag that can be passed to CreateValidatorForDirectory.
type ValidatorOption func(*Validator) error

// WithSpecVersion enables validation dependant of the spec version used by the package.
func WithSpecVersion(version string) ValidatorOption {
return func(v *Validator) error {
sv, err := semver.NewVersion(version)
if err != nil {
return fmt.Errorf("invalid version %q: %v", version, err)
}
v.specVersion = *sv
return nil
}
}

// WithDefaultNumericConversion configures the validator to accept defined keyword (or constant_keyword) fields as numeric-type.
func WithDefaultNumericConversion() ValidatorOption {
return func(v *Validator) error {
Expand Down Expand Up @@ -255,7 +271,12 @@ func (v *Validator) validateScalarElement(key string, val interface{}) error {
val = fmt.Sprintf("%q", val)
}

err := v.parseElementValue(key, *definition, val)
err := v.validateExpectedNormalization(*definition, val)
if err != nil {
return errors.Wrapf(err, "field %q is not normalized as expected", key)
}

err = v.parseElementValue(key, *definition, val)
if err != nil {
return errors.Wrap(err, "parsing field value failed")
}
Expand Down Expand Up @@ -361,6 +382,22 @@ func compareKeys(key string, def FieldDefinition, searchedKey string) bool {
return false
}

func (v *Validator) validateExpectedNormalization(definition FieldDefinition, val interface{}) error {
// Validate expected normalization starting with packages following spec v2 format.
if v.specVersion.LessThan(semver.MustParse("2.0.0")) {
return nil
}
for _, normalize := range definition.Normalize {
switch normalize {
case "array":
if _, isArray := val.([]interface{}); val != nil && !isArray {
return fmt.Errorf("expected array, found %q (%T)", val, val)
}
}
}
return nil
}

// validSubField checks if the extra part that didn't match with any field definition,
// matches with the possible sub field of complex fields like geo_point or histogram.
func validSubField(def FieldDefinition, extraPart string) bool {
Expand Down
22 changes: 22 additions & 0 deletions internal/fields/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,28 @@ func TestValidate_ipAddress(t *testing.T) {
require.Empty(t, errs)
}

func TestValidate_WithSpecVersion(t *testing.T) {
validator, err := CreateValidatorForDirectory("testdata", WithSpecVersion("2.0.0"))
require.NoError(t, err)

e := readSampleEvent(t, "testdata/invalid-array-normalization.json")
errs := validator.ValidateDocumentBody(e)
require.Len(t, errs, 1)
require.Contains(t, errs[0].Error(), `field "container.image.tag" is not normalized as expected`)

e = readSampleEvent(t, "testdata/valid-array-normalization.json")
errs = validator.ValidateDocumentBody(e)
require.Empty(t, errs)

// Check now that this validation was only enabled for 2.0.0.
validator, err = CreateValidatorForDirectory("testdata", WithSpecVersion("1.99.99"))
require.NoError(t, err)

e = readSampleEvent(t, "testdata/invalid-array-normalization.json")
errs = validator.ValidateDocumentBody(e)
require.Empty(t, errs)
}

func Test_parseElementValue(t *testing.T) {
for _, test := range []struct {
key string
Expand Down
1 change: 1 addition & 0 deletions internal/packages/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ type Owner struct {

// PackageManifest represents the basic structure of a package's manifest
type PackageManifest struct {
SpecVersion string `config:"format_version" json:"format_version" yaml:"format_version"`
Name string `config:"name" json:"name" yaml:"name"`
Title string `config:"title" json:"title" yaml:"title"`
Type string `config:"type" json:"type" yaml:"type"`
Expand Down
6 changes: 6 additions & 0 deletions internal/testrunner/runners/pipeline/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ func (r *runner) run() ([]testrunner.TestResult, error) {
return nil, errors.Wrap(err, "installing ingest pipelines failed")
}

pkgManifest, err := packages.ReadPackageManifestFromPackageRoot(r.options.PackageRootPath)
if err != nil {
return nil, errors.Wrap(err, "failed to read manifest")
}

results := make([]testrunner.TestResult, 0)
for _, testCaseFile := range testCaseFiles {
tr := testrunner.TestResult{
Expand Down Expand Up @@ -135,6 +140,7 @@ func (r *runner) run() ([]testrunner.TestResult, error) {

tr.TimeElapsed = time.Since(startTime)
fieldsValidator, err := fields.CreateValidatorForDirectory(dataStreamPath,
fields.WithSpecVersion(pkgManifest.SpecVersion),
fields.WithNumericKeywordFields(tc.config.NumericKeywordFields),
// explicitly enabled for pipeline tests only
// since system tests can have dynamic public IPs
Expand Down
14 changes: 10 additions & 4 deletions internal/testrunner/runners/static/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/elastic/elastic-package/internal/fields"
"github.com/elastic/elastic-package/internal/logger"
"github.com/elastic/elastic-package/internal/packages"
"github.com/elastic/elastic-package/internal/testrunner"
)

Expand Down Expand Up @@ -64,12 +65,17 @@ func (r runner) run() ([]testrunner.TestResult, error) {
return result.WithSkip(testConfig.Skip)
}

pkgManifest, err := packages.ReadPackageManifestFromPackageRoot(r.options.PackageRootPath)
if err != nil {
return result.WithError(errors.Wrap(err, "failed to read manifest"))
}

var results []testrunner.TestResult
results = append(results, r.verifySampleEvent()...)
results = append(results, r.verifySampleEvent(pkgManifest)...)
return results, nil
}

func (r runner) verifySampleEvent() []testrunner.TestResult {
func (r runner) verifySampleEvent(pkgManifest *packages.PackageManifest) []testrunner.TestResult {
dataStreamPath := filepath.Join(r.options.PackageRootPath, "data_stream", r.options.TestFolder.DataStream)
sampleEventPath := filepath.Join(dataStreamPath, sampleEventJSON)
_, err := os.Stat(sampleEventPath)
Expand All @@ -89,8 +95,8 @@ func (r runner) verifySampleEvent() []testrunner.TestResult {
return results
}

fieldsValidator, err := fields.CreateValidatorForDirectory(
dataStreamPath,
fieldsValidator, err := fields.CreateValidatorForDirectory(dataStreamPath,
fields.WithSpecVersion(pkgManifest.SpecVersion),
fields.WithDefaultNumericConversion())
if err != nil {
results, _ := resultComposer.WithError(errors.Wrap(err, "creating fields validator for data stream failed"))
Expand Down
1 change: 1 addition & 0 deletions internal/testrunner/runners/system/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,7 @@ func (r *runner) runTest(config *testConfig, ctxt servicedeployer.ServiceContext

// Validate fields in docs
fieldsValidator, err := fields.CreateValidatorForDirectory(serviceOptions.DataStreamRootPath,
fields.WithSpecVersion(pkgManifest.SpecVersion),
fields.WithNumericKeywordFields(config.NumericKeywordFields))
if err != nil {
return result.WithError(errors.Wrapf(err, "creating fields validator for data stream failed (path: %s)", serviceOptions.DataStreamRootPath))
Expand Down
2 changes: 1 addition & 1 deletion test/packages/parallel/apache/_dev/build/build.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
dependencies:
ecs:
reference: git@1.12
reference: git@8.1
Loading

0 comments on commit c64ba8d

Please sign in to comment.