From bb45195fcc68dc7069aa3d840411e4c395521626 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 31 Mar 2022 17:42:44 +0200 Subject: [PATCH 1/3] Check allowed values for field --- internal/fields/model.go | 58 +++++++++++--- internal/fields/validate.go | 128 +++++++++++++++++++------------ internal/fields/validate_test.go | 42 +++++++++- 3 files changed, 164 insertions(+), 64 deletions(-) diff --git a/internal/fields/model.go b/internal/fields/model.go index 9b6a3d2cca..b56095684a 100644 --- a/internal/fields/model.go +++ b/internal/fields/model.go @@ -9,22 +9,25 @@ import ( "strings" "gopkg.in/yaml.v3" + + "github.com/elastic/elastic-package/internal/common" ) // FieldDefinition describes a single field with its properties. type FieldDefinition struct { - Name string `yaml:"name"` - Description string `yaml:"description"` - Type string `yaml:"type"` - Value string `yaml:"value"` // The value to associate with a constant_keyword field. - Pattern string `yaml:"pattern"` - Unit string `yaml:"unit"` - MetricType string `yaml:"metric_type"` - External string `yaml:"external"` - Index *bool `yaml:"index"` - DocValues *bool `yaml:"doc_values"` - Fields FieldDefinitions `yaml:"fields,omitempty"` - MultiFields []FieldDefinition `yaml:"multi_fields,omitempty"` + Name string `yaml:"name"` + Description string `yaml:"description"` + Type string `yaml:"type"` + Value string `yaml:"value"` // The value to associate with a constant_keyword field. + AllowedValues AllowedValues `yaml:"allowed_values"` + Pattern string `yaml:"pattern"` + Unit string `yaml:"unit"` + MetricType string `yaml:"metric_type"` + External string `yaml:"external"` + Index *bool `yaml:"index"` + DocValues *bool `yaml:"doc_values"` + Fields FieldDefinitions `yaml:"fields,omitempty"` + MultiFields []FieldDefinition `yaml:"multi_fields,omitempty"` } func (orig *FieldDefinition) Update(fd FieldDefinition) { @@ -40,6 +43,9 @@ func (orig *FieldDefinition) Update(fd FieldDefinition) { if fd.Value != "" { orig.Value = fd.Value } + if len(fd.AllowedValues) > 0 { + orig.AllowedValues = fd.AllowedValues + } if fd.Pattern != "" { orig.Pattern = fd.Pattern } @@ -156,3 +162,31 @@ func cleanNestedNames(parent string, fields []FieldDefinition) { } } } + +// AllowedValues is the list of allowed values for a field. +type AllowedValues []AllowedValue + +// Allowed returns true if a given value is allowed. +func (avs AllowedValues) IsAllowed(value string) bool { + if len(avs) == 0 { + // No configured allowed values, any value is allowed. + return true + } + return common.StringSliceContains(avs.Values(), value) +} + +// Values returns the list of allowed values. +func (avs AllowedValues) Values() []string { + var values []string + for _, v := range avs { + values = append(values, v.Name) + } + return values +} + +// AllowedValue is one of the allowed values for a field. +type AllowedValue struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + ExpectedEventTypes []string `yaml:"expected_event_types"` +} diff --git a/internal/fields/validate.go b/internal/fields/validate.go index 7a8ac84f29..5738c0379f 100644 --- a/internal/fields/validate.go +++ b/internal/fields/validate.go @@ -354,60 +354,74 @@ func compareKeys(key string, def FieldDefinition, searchedKey string) bool { } func (v *Validator) parseElementValue(key string, definition FieldDefinition, val interface{}) error { - val, ok := ensureSingleElementValue(val) - if !ok { - return nil // it's an array, but it's not possible to extract the single value. - } + return forEachElementValue(val, func(val interface{}) error { + var valid bool + switch definition.Type { + case "constant_keyword": + var valStr string + valStr, valid = val.(string) + if !valid { + break + } - var valid bool - switch definition.Type { - case "constant_keyword": - var valStr string - valStr, valid = val.(string) - if !valid { - break - } + if err := ensureConstantKeywordValueMatches(key, valStr, definition.Value); err != nil { + return err + } + if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { + return err + } + if err := ensureAllowedValues(key, valStr, definition.AllowedValues); err != nil { + return err + } + case "keyword", "text": + var valStr string + valStr, valid = val.(string) + if !valid { + break + } - if err := ensureConstantKeywordValueMatches(key, valStr, definition.Value); err != nil { - return err - } - if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { - return err - } - case "date", "keyword", "text": - var valStr string - valStr, valid = val.(string) - if !valid { - break - } + if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { + return err + } + if err := ensureAllowedValues(key, valStr, definition.AllowedValues); err != nil { + return err + } + case "date": + var valStr string + valStr, valid = val.(string) + if !valid { + break + } - if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { - return err - } - case "ip": - var valStr string - valStr, valid = val.(string) - if !valid { - break - } + if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { + return err + } + case "ip": + var valStr string + valStr, valid = val.(string) + if !valid { + break + } - if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { - return err + if err := ensurePatternMatches(key, valStr, definition.Pattern); err != nil { + return err + } + + if v.enabledAllowedIPCheck && !v.isAllowedIPValue(valStr) { + 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) + } + case "float", "long", "double": + _, valid = val.(float64) + default: + valid = true // all other types are considered valid not blocking validation } - if v.enabledAllowedIPCheck && !v.isAllowedIPValue(valStr) { - 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) + if !valid { + return fmt.Errorf("field %q's Go type, %T, does not match the expected field type: %s (field value: %v)", key, val, definition.Type, val) } - case "float", "long", "double": - _, valid = val.(float64) - default: - valid = true // all other types are considered valid not blocking validation - } - if !valid { - return fmt.Errorf("field %q's Go type, %T, does not match the expected field type: %s (field value: %v)", key, val, definition.Type, val) - } - return nil + return nil + }) } // isAllowedIPValue checks if the provided IP is allowed for testing @@ -436,15 +450,18 @@ func (v *Validator) isAllowedIPValue(s string) bool { // ensureSingleElementValue extracts single entity from a potential array, which is a valid field representation // in Elasticsearch. For type assertion we need a single value. -func ensureSingleElementValue(val interface{}) (interface{}, bool) { +func forEachElementValue(val interface{}, fn func(interface{}) error) error { arr, isArray := val.([]interface{}) if !isArray { - return val, true + return fn(val) } - if len(arr) > 0 { - return arr[0], true + for _, element := range arr { + err := fn(element) + if err != nil { + return err + } } - return nil, false // false: empty array, can't deduce single value type + return nil } // ensurePatternMatches validates the document's field value matches the field @@ -474,3 +491,12 @@ func ensureConstantKeywordValueMatches(key, value, constantKeywordValue string) } return nil } + +// ensureAllowedValues validates that the document's field value +// is one of the allowed values. +func ensureAllowedValues(key, value string, allowedValues AllowedValues) error { + if !allowedValues.IsAllowed(value) { + return fmt.Errorf("field %q's value %q is not one of the allowed values (%s)", key, value, strings.Join(allowedValues.Values(), ", ")) + } + return nil +} diff --git a/internal/fields/validate_test.go b/internal/fields/validate_test.go index 1e142927c5..1e9f5f5b77 100644 --- a/internal/fields/validate_test.go +++ b/internal/fields/validate_test.go @@ -98,7 +98,7 @@ func Test_parseElementValue(t *testing.T) { definition FieldDefinition fail bool }{ - // Arrays (only first value checked) + // Arrays { key: "string array to keyword", value: []interface{}{"hello", "world"}, @@ -114,6 +114,14 @@ func Test_parseElementValue(t *testing.T) { }, fail: true, }, + { + key: "mixed numbers and strings in number array", + value: []interface{}{123, "hi"}, + definition: FieldDefinition{ + Type: "long", + }, + fail: true, + }, // keyword and constant_keyword (string) { @@ -212,6 +220,38 @@ func Test_parseElementValue(t *testing.T) { }, fail: true, }, + // allowed values + { + key: "allowed values", + value: "configuration", + definition: FieldDefinition{ + Type: "keyword", + AllowedValues: AllowedValues{ + { + Name: "configuration", + }, + { + Name: "network", + }, + }, + }, + }, + { + key: "not allowed value", + value: "display", + definition: FieldDefinition{ + Type: "keyword", + AllowedValues: AllowedValues{ + { + Name: "configuration", + }, + { + Name: "network", + }, + }, + }, + fail: true, + }, } { v := Validator{disabledDependencyManagement: true} t.Run(test.key, func(t *testing.T) { From 6044bd61d0870c26cd5fa3567e4ed61829cdce7a Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 31 Mar 2022 17:57:14 +0200 Subject: [PATCH 2/3] Update comment of replaced function --- internal/fields/validate.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/fields/validate.go b/internal/fields/validate.go index 5738c0379f..aff8793894 100644 --- a/internal/fields/validate.go +++ b/internal/fields/validate.go @@ -448,8 +448,8 @@ func (v *Validator) isAllowedIPValue(s string) bool { return false } -// ensureSingleElementValue extracts single entity from a potential array, which is a valid field representation -// in Elasticsearch. For type assertion we need a single value. +// forEachElementValue visits a function for each element in the given value if +// it is an array. If it is not an array, it calls the function with it. func forEachElementValue(val interface{}, fn func(interface{}) error) error { arr, isArray := val.([]interface{}) if !isArray { From 0b36b50d328db1a7af7470fc8717a8b0c07202c6 Mon Sep 17 00:00:00 2001 From: Jaime Soriano Pastor Date: Thu, 31 Mar 2022 17:59:27 +0200 Subject: [PATCH 3/3] Add test case with array of values --- internal/fields/validate_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/internal/fields/validate_test.go b/internal/fields/validate_test.go index 1e9f5f5b77..b0b2009b5a 100644 --- a/internal/fields/validate_test.go +++ b/internal/fields/validate_test.go @@ -252,6 +252,22 @@ func Test_parseElementValue(t *testing.T) { }, fail: true, }, + { + key: "not allowed value in array", + value: []string{"configuration", "display"}, + definition: FieldDefinition{ + Type: "keyword", + AllowedValues: AllowedValues{ + { + Name: "configuration", + }, + { + Name: "network", + }, + }, + }, + fail: true, + }, } { v := Validator{disabledDependencyManagement: true} t.Run(test.key, func(t *testing.T) {