From 2c3f76efffadeb4d80773c6c51705b4705f7e049 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 12 Dec 2024 13:09:38 +0100 Subject: [PATCH 01/51] Add dynamic templates as parameter --- internal/fields/mappings.go | 29 ++++++++++++++++++----------- internal/fields/mappings_test.go | 4 +++- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 474a986b7..4662937ef 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -241,7 +241,14 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs.Unique() } - mappingErrs := v.compareMappings("", rawPreview, rawActual) + var rawDynamicTemplates map[string]any + err = json.Unmarshal(actualDynamicTemplates, &rawDynamicTemplates) + if err != nil { + errs = append(errs, fmt.Errorf("failed to unmarshal actual dynamic templates (data stream %s): %w", v.dataStreamName, err)) + return errs.Unique() + } + + mappingErrs := v.compareMappings("", rawPreview, rawActual, rawDynamicTemplates) errs = append(errs, mappingErrs...) if len(errs) > 0 { @@ -481,7 +488,7 @@ func validateConstantKeywordField(path string, preview, actual map[string]any) ( return isConstantKeyword, nil } -func (v *MappingValidator) compareMappings(path string, preview, actual map[string]any) multierror.Error { +func (v *MappingValidator) compareMappings(path string, preview, actual, dynamicTemplates map[string]any) multierror.Error { var errs multierror.Error isConstantKeywordType, err := validateConstantKeywordField(path, preview, actual) @@ -519,7 +526,7 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri if err != nil { errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err)) } - compareErrors := v.compareMappings(path, previewProperties, actualProperties) + compareErrors := v.compareMappings(path, previewProperties, actualProperties, dynamicTemplates) errs = append(errs, compareErrors...) if len(errs) == 0 { @@ -542,13 +549,13 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri if err != nil { errs = append(errs, fmt.Errorf("found invalid multi_fields type in actual mappings for path %q: %w", path, err)) } - compareErrors := v.compareMappings(path, previewFields, actualFields) + compareErrors := v.compareMappings(path, previewFields, actualFields, dynamicTemplates) errs = append(errs, compareErrors...) // not returning here to keep validating the other fields of this object if any } // Compare and validate the elements under "properties": objects or fields and its parameters - propertiesErrs := v.validateObjectProperties(path, containsMultifield, actual, preview) + propertiesErrs := v.validateObjectProperties(path, containsMultifield, actual, preview, dynamicTemplates) errs = append(errs, propertiesErrs...) if len(errs) == 0 { return nil @@ -556,7 +563,7 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri return errs.Unique() } -func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview map[string]any) multierror.Error { +func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview, dynamicTemplates map[string]any) multierror.Error { var errs multierror.Error for key, value := range actual { if containsMultifield && key == "fields" { @@ -576,14 +583,14 @@ func (v *MappingValidator) validateObjectProperties(path string, containsMultifi logger.Debugf("field %q is an empty object and it does not exist in the preview", currentPath) continue } - ecsErrors := v.validateMappingsNotInPreview(currentPath, childField) + ecsErrors := v.validateMappingsNotInPreview(currentPath, childField, dynamicTemplates) errs = append(errs, ecsErrors...) } continue } - fieldErrs := v.validateObjectMappingAndParameters(preview[key], value, currentPath) + fieldErrs := v.validateObjectMappingAndParameters(preview[key], value, currentPath, dynamicTemplates) errs = append(errs, fieldErrs...) } if len(errs) == 0 { @@ -594,7 +601,7 @@ func (v *MappingValidator) validateObjectProperties(path string, containsMultifi // validateMappingsNotInPreview validates the object and the nested objects in the current path with other resources // like ECS schema, dynamic templates or local fields defined in the package (type array). -func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField map[string]any) multierror.Error { +func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField, dynamicTemplates map[string]any) multierror.Error { var errs multierror.Error flattenFields, err := flattenMappings(currentPath, childField) if err != nil { @@ -634,7 +641,7 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values // in the actual mapping with the values in the preview mapping. -func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string) multierror.Error { +func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates map[string]any) multierror.Error { var errs multierror.Error switch actualValue.(type) { case map[string]any: @@ -647,7 +654,7 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu if !ok { errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath)) } - errs = append(errs, v.compareMappings(currentPath, previewField, actualField)...) + errs = append(errs, v.compareMappings(currentPath, previewField, actualField, dynamicTemplates)...) case any: // Validate each setting/parameter of the mapping // If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 334fc4d6b..cc06b762e 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -634,6 +634,8 @@ func TestComparingMappings(t *testing.T) { for _, c := range cases { t.Run(c.title, func(t *testing.T) { + dynamicTemplates := map[string]any{} + logger.EnableDebugMode() specVersion := defaultSpecVersion if c.spec != "" { @@ -647,7 +649,7 @@ func TestComparingMappings(t *testing.T) { ) require.NoError(t, err) - errs := v.compareMappings("", c.preview, c.actual) + errs := v.compareMappings("", c.preview, c.actual, dynamicTemplates) if len(c.expectedErrors) > 0 { assert.Len(t, errs, len(c.expectedErrors)) for _, err := range errs { From 41d7b8878f42cd0cca57f256cccc2f95fc7718eb Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 16 Dec 2024 17:44:15 +0100 Subject: [PATCH 02/51] Add validation for each dynamic template depending on the parameters --- internal/fields/mappings.go | 232 ++++++++++++++++++++++++++++++- internal/fields/mappings_test.go | 2 +- 2 files changed, 227 insertions(+), 7 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 4662937ef..be954bdaf 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "path/filepath" + "regexp" "slices" "strings" @@ -241,7 +242,7 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs.Unique() } - var rawDynamicTemplates map[string]any + var rawDynamicTemplates []map[string]any err = json.Unmarshal(actualDynamicTemplates, &rawDynamicTemplates) if err != nil { errs = append(errs, fmt.Errorf("failed to unmarshal actual dynamic templates (data stream %s): %w", v.dataStreamName, err)) @@ -488,7 +489,7 @@ func validateConstantKeywordField(path string, preview, actual map[string]any) ( return isConstantKeyword, nil } -func (v *MappingValidator) compareMappings(path string, preview, actual, dynamicTemplates map[string]any) multierror.Error { +func (v *MappingValidator) compareMappings(path string, preview, actual map[string]any, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error isConstantKeywordType, err := validateConstantKeywordField(path, preview, actual) @@ -563,7 +564,7 @@ func (v *MappingValidator) compareMappings(path string, preview, actual, dynamic return errs.Unique() } -func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview, dynamicTemplates map[string]any) multierror.Error { +func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview map[string]any, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error for key, value := range actual { if containsMultifield && key == "fields" { @@ -601,7 +602,7 @@ func (v *MappingValidator) validateObjectProperties(path string, containsMultifi // validateMappingsNotInPreview validates the object and the nested objects in the current path with other resources // like ECS schema, dynamic templates or local fields defined in the package (type array). -func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField, dynamicTemplates map[string]any) multierror.Error { +func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField map[string]any, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error flattenFields, err := flattenMappings(currentPath, childField) if err != nil { @@ -611,7 +612,7 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil for fieldPath, object := range flattenFields { if slices.Contains(v.exceptionFields, fieldPath) { - logger.Warnf("Found exception field, skip its validation: %q", fieldPath) + logger.Warnf("Found exception field, skip its validation (not in preview): %q", fieldPath) return nil } @@ -628,6 +629,12 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil // TODO: validate mapping with dynamic templates first than validating with ECS // just raise an error if both validation processes fail + matched, err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) + if err == nil { + // TODO + logger.Debugf("Matched: %t", matched) + continue + } // are all fields under this key defined in ECS? err = v.validateMappingInECSSchema(fieldPath, def) @@ -639,9 +646,222 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil return errs.Unique() } +func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []map[string]any) (bool, error) { + + parseSetting := func(value any) ([]string, error) { + all := []string{} + switch v := value.(type) { + case []any: + for _, elem := range v { + s, ok := elem.(string) + if !ok { + return nil, fmt.Errorf("failed to cast to string: %s", elem) + } + all = append(all, s) + } + case any: + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("failed to cast to string: %s", v) + } + all = append(all, s) + default: + return nil, fmt.Errorf("unexpected type for setting: %T", value) + + } + return all, nil + } + + fieldName := func(path string) string { + if !strings.Contains(path, ".") { + return path + } + + elems := strings.Split(path, ".") + return elems[len(elems)-1] + } + + stringMatchesPatterns := func(regexes []string, elem string, fullRegex bool) (bool, error) { + applies := false + for _, v := range regexes { + if !strings.Contains(v, "*") { + // not a regex + continue + } + + var r string + if fullRegex { + r = v + } else { + r = strings.ReplaceAll(v, ".", "\\.") + r = strings.ReplaceAll(r, "*", ".*") + } + + match, err := regexp.MatchString(r, elem) + if err != nil { + return false, fmt.Errorf("failed to build regex %s: %w", r, err) + } + if match { + applies = true + break + } + } + return applies, nil + } + + fieldType := mappingParameter("type", definition) + if fieldType == "" { + return false, fmt.Errorf("missing type for the field %s", currentPath) + } + + for _, template := range dynamicTemplates { + if len(template) != 1 { + return false, fmt.Errorf("unexpected number of dynamic template definitions found") + } + + templateName := "" + var rawContents any + for key, value := range template { + templateName = key + rawContents = value + } + logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) + + contents, ok := rawContents.(map[string]any) + if !ok { + return false, fmt.Errorf("unexpected dynamic template format found for %s", templateName) + } + + fullRegex := false + if v, ok := contents["match_pattern"]; ok { + s, ok := v.(string) + if !ok { + return false, fmt.Errorf("invalid type for \"match_pattern\": %T", v) + } + if s == "regex" { + logger.Debugf("Use full regex in dynamic templates (match_pattern: regex)") + fullRegex = true + } + } + + // matches with the current definitions and path + // https://www.elastic.co/guide/en/elasticsearch/reference/current/dynamic-templates.html + matched := true + for setting, value := range contents { + switch setting { + case "mapping": + // Skip + case "match_pattern": + // Skip + case "match": + name := fieldName(currentPath) + logger.Debugf("> Check match: %q (key %q)", currentPath, name) + values, err := parseSetting(value) + if err != nil { + return false, fmt.Errorf("failed to check match setting: %w", err) + } + if !slices.Contains(values, name) { + matched = false + break + } + matches, err := stringMatchesPatterns(values, name, fullRegex) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + } + if !matches { + logger.Debugf(">> Issue: not matches") + matched = false + break + } + case "unmatch": + name := fieldName(currentPath) + logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) + values, err := parseSetting(value) + if err != nil { + return false, fmt.Errorf("failed to check match setting: %w", err) + } + if slices.Contains(values, name) { + matched = false + break + } + matches, err := stringMatchesPatterns(values, name, fullRegex) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + } + if matches { + logger.Debugf(">> Issue: matches") + matched = false + break + } + case "path_match": + logger.Debugf("> Check path_match: %q", currentPath) + values, err := parseSetting(value) + matches, err := stringMatchesPatterns(values, currentPath, false) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + } + if !matches { + logger.Debugf(">> Issue: not matches") + matched = false + break + } + case "path_unmatch": + logger.Debugf("> Check path_unmatch: %q", currentPath) + values, err := parseSetting(value) + if err != nil { + return false, fmt.Errorf("failed to check path_unmatch setting: %w", err) + } + matches, err := stringMatchesPatterns(values, currentPath, false) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + } + if matches { + logger.Debugf(">> Issue: matches") + matched = false + break + } + case "match_mapping_type": + logger.Debugf("> Check match_mapping_type: %q (type %s vs mapping type %q)", currentPath, definition, fieldType) + values, err := parseSetting(value) + if err != nil { + return false, fmt.Errorf("failed to check match_mapping_type setting: %w", err) + } + if slices.Contains(values, "*") { + continue + } + if !slices.Contains(values, fieldType) { + logger.Debugf(">> Issue: not matches") + matched = false + break + } + case "unmatch_mapping_type": + logger.Debugf("> Check unmatch_mapping_type: %q (type %s vs mapping type %q)", currentPath, definition, fieldType) + values, err := parseSetting(value) + if err != nil { + return false, fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) + } + if slices.Contains(values, fieldType) { + logger.Debugf(">> Issue: matches") + matched = false + break + } + default: + return false, fmt.Errorf("unexpected setting found in dynamic template") + } + } + if matched { + logger.Debugf("> Found dynamic template matched: %s", templateName) + return true, nil + } + } + + logger.Debugf(">>> No template matching") + return false, nil +} + // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values // in the actual mapping with the values in the preview mapping. -func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates map[string]any) multierror.Error { +func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error switch actualValue.(type) { case map[string]any: diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index cc06b762e..2f98211dd 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -634,7 +634,7 @@ func TestComparingMappings(t *testing.T) { for _, c := range cases { t.Run(c.title, func(t *testing.T) { - dynamicTemplates := map[string]any{} + dynamicTemplates := []map[string]any{} logger.EnableDebugMode() specVersion := defaultSpecVersion From ff5298ed2e123bf723020f0c37a15654c2363ed5 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 16 Dec 2024 19:13:48 +0100 Subject: [PATCH 03/51] Fixes for dynamic templates --- internal/fields/mappings.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index be954bdaf..7b78f8bd3 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -391,6 +391,7 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini if found.Type == actualType { return nil } + // TODO: Compare all parameters and multifieds // exceptions related to numbers if isNumberTypeField(found.Type, actualType) { @@ -695,6 +696,8 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } else { r = strings.ReplaceAll(v, ".", "\\.") r = strings.ReplaceAll(r, "*", ".*") + // Force to match beginning and ending + r = fmt.Sprintf("^%s$", r) } match, err := regexp.MatchString(r, elem) @@ -746,8 +749,9 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi // matches with the current definitions and path // https://www.elastic.co/guide/en/elasticsearch/reference/current/dynamic-templates.html - matched := true + allMatched := true for setting, value := range contents { + matched := true switch setting { case "mapping": // Skip @@ -821,11 +825,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi break } case "match_mapping_type": - logger.Debugf("> Check match_mapping_type: %q (type %s vs mapping type %q)", currentPath, definition, fieldType) + logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) values, err := parseSetting(value) if err != nil { return false, fmt.Errorf("failed to check match_mapping_type setting: %w", err) } + logger.Debugf(">> Comparing to values: %s", values) if slices.Contains(values, "*") { continue } @@ -835,11 +840,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi break } case "unmatch_mapping_type": - logger.Debugf("> Check unmatch_mapping_type: %q (type %s vs mapping type %q)", currentPath, definition, fieldType) + logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) values, err := parseSetting(value) if err != nil { return false, fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) } + logger.Debugf(">> Comparing to values: %s", values) if slices.Contains(values, fieldType) { logger.Debugf(">> Issue: matches") matched = false @@ -848,8 +854,13 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi default: return false, fmt.Errorf("unexpected setting found in dynamic template") } + if !matched { + // If just one parameter does not match, this dynamic template can be skipped + allMatched = false + break + } } - if matched { + if allMatched { logger.Debugf("> Found dynamic template matched: %s", templateName) return true, nil } From af04d665eaeffbf0a7571e373d5f94a251052dbd Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 17 Dec 2024 17:22:32 +0100 Subject: [PATCH 04/51] Compare fields with dynamic templates --- internal/fields/mappings.go | 95 ++++++++++++++++++++++--------------- 1 file changed, 58 insertions(+), 37 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 7b78f8bd3..f063aa23d 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -457,7 +457,10 @@ func flattenMappings(path string, definition map[string]any) (map[string]any, er } func getMappingDefinitionsField(field string, definition map[string]any) (map[string]any, error) { - anyValue := definition[field] + anyValue, ok := definition[field] + if !ok { + return nil, fmt.Errorf("not found field: %q", field) + } object, ok := anyValue.(map[string]any) if !ok { return nil, fmt.Errorf("unexpected type found for %q: %T ", field, anyValue) @@ -628,26 +631,22 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil continue } - // TODO: validate mapping with dynamic templates first than validating with ECS - // just raise an error if both validation processes fail - matched, err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) + // validate whether or not the field has a corresponding dynamic template + err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) if err == nil { - // TODO - logger.Debugf("Matched: %t", matched) continue } - // are all fields under this key defined in ECS? + // validate whether or not all fields under this key are defined in ECS err = v.validateMappingInECSSchema(fieldPath, def) if err != nil { - logger.Warnf("undefined path %q (pending to check dynamic templates)", fieldPath) errs = append(errs, fmt.Errorf("field %q is undefined: %w", fieldPath, err)) } } return errs.Unique() } -func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []map[string]any) (bool, error) { +func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []map[string]any) error { parseSetting := func(value any) ([]string, error) { all := []string{} @@ -714,32 +713,44 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi fieldType := mappingParameter("type", definition) if fieldType == "" { - return false, fmt.Errorf("missing type for the field %s", currentPath) + return fmt.Errorf("missing type parameter for field: %q", currentPath) } for _, template := range dynamicTemplates { if len(template) != 1 { - return false, fmt.Errorf("unexpected number of dynamic template definitions found") + return fmt.Errorf("unexpected number of dynamic template definitions found") } + // there is just one dynamic template per object templateName := "" var rawContents any for key, value := range template { templateName = key rawContents = value } + + // if strings.HasPrefix(templateName, "_embedded_ecs-") { + // continue + // } + // if strings.HasPrefix(templateName, "ecs_") { + // continue + // } + // if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + // continue + // } + logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) contents, ok := rawContents.(map[string]any) if !ok { - return false, fmt.Errorf("unexpected dynamic template format found for %s", templateName) + return fmt.Errorf("unexpected dynamic template format found for %q", templateName) } fullRegex := false if v, ok := contents["match_pattern"]; ok { s, ok := v.(string) if !ok { - return false, fmt.Errorf("invalid type for \"match_pattern\": %T", v) + return fmt.Errorf("invalid type for \"match_pattern\": %T", v) } if s == "regex" { logger.Debugf("Use full regex in dynamic templates (match_pattern: regex)") @@ -753,16 +764,14 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi for setting, value := range contents { matched := true switch setting { - case "mapping": - // Skip - case "match_pattern": - // Skip + case "mapping", "match_pattern": + // Do nothing case "match": name := fieldName(currentPath) logger.Debugf("> Check match: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { - return false, fmt.Errorf("failed to check match setting: %w", err) + return fmt.Errorf("failed to check match setting: %w", err) } if !slices.Contains(values, name) { matched = false @@ -770,19 +779,18 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } matches, err := stringMatchesPatterns(values, name, fullRegex) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if !matches { logger.Debugf(">> Issue: not matches") matched = false - break } case "unmatch": name := fieldName(currentPath) logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { - return false, fmt.Errorf("failed to check match setting: %w", err) + return fmt.Errorf("failed to check unmatch setting: %w", err) } if slices.Contains(values, name) { matched = false @@ -790,45 +798,45 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } matches, err := stringMatchesPatterns(values, name, fullRegex) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if matches { logger.Debugf(">> Issue: matches") matched = false - break } case "path_match": logger.Debugf("> Check path_match: %q", currentPath) values, err := parseSetting(value) + if err != nil { + return fmt.Errorf("failed to check path_match setting: %w", err) + } matches, err := stringMatchesPatterns(values, currentPath, false) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if !matches { logger.Debugf(">> Issue: not matches") matched = false - break } case "path_unmatch": logger.Debugf("> Check path_unmatch: %q", currentPath) values, err := parseSetting(value) if err != nil { - return false, fmt.Errorf("failed to check path_unmatch setting: %w", err) + return fmt.Errorf("failed to check path_unmatch setting: %w", err) } matches, err := stringMatchesPatterns(values, currentPath, false) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) + return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if matches { logger.Debugf(">> Issue: matches") matched = false - break } case "match_mapping_type": logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) values, err := parseSetting(value) if err != nil { - return false, fmt.Errorf("failed to check match_mapping_type setting: %w", err) + return fmt.Errorf("failed to check match_mapping_type setting: %w", err) } logger.Debugf(">> Comparing to values: %s", values) if slices.Contains(values, "*") { @@ -837,22 +845,20 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi if !slices.Contains(values, fieldType) { logger.Debugf(">> Issue: not matches") matched = false - break } case "unmatch_mapping_type": logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) values, err := parseSetting(value) if err != nil { - return false, fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) + return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) } logger.Debugf(">> Comparing to values: %s", values) if slices.Contains(values, fieldType) { logger.Debugf(">> Issue: matches") matched = false - break } default: - return false, fmt.Errorf("unexpected setting found in dynamic template") + return fmt.Errorf("unexpected setting found in dynamic template") } if !matched { // If just one parameter does not match, this dynamic template can be skipped @@ -860,14 +866,29 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi break } } - if allMatched { - logger.Debugf("> Found dynamic template matched: %s", templateName) - return true, nil + if !allMatched { + // Look for another dynamic template + continue + } + + logger.Debugf("> Found dynamic template matched: %s", templateName) + mappingParameter, ok := contents["mapping"] + if !ok { + logger.Warnf(">> Missing \"mapping\" field: %s", templateName) + return fmt.Errorf("missing mapping parameter in %s", templateName) + } + + logger.Debugf(">> Check all the other parameters (%q): %q", templateName, currentPath) + errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}) + if errs != nil { + return fmt.Errorf("invalid mapping found for %q: %w", currentPath, errs.Unique()) } + + return nil } logger.Debugf(">>> No template matching") - return false, nil + return fmt.Errorf("no template matching for %q", currentPath) } // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values From f21c7124ef9cd768d4c37245d5f11216334cccd3 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 17 Dec 2024 19:08:27 +0100 Subject: [PATCH 05/51] Remove multi_fields from flattened fields multi_fields are now compared directly with dynamic templates or ECS fields. It was required to know whether or not that field was a multi_field or not to apply some specific validations. --- internal/fields/mappings.go | 168 +++++++++++++++++++++++++----------- internal/fields/validate.go | 4 +- 2 files changed, 120 insertions(+), 52 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index f063aa23d..9d67931af 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -387,19 +387,72 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini return fmt.Errorf("missing definition for path (not in ECS)") } - actualType := mappingParameter("type", definition) - if found.Type == actualType { - return nil + err := compareFieldDefinitionWithECS(currentPath, found, definition) + if err != nil { + return err + } + + // Compare multifields + var ecsMultiFields []FieldDefinition + // Filter multi-fields added by appendECSMappingMultifields + for _, f := range found.MultiFields { + if f.External != externalFieldAppendedTag { + ecsMultiFields = append(ecsMultiFields, f) + } + } + + if isMultiFields(definition) != (len(ecsMultiFields) > 0) { + return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields)) } - // TODO: Compare all parameters and multifieds - // exceptions related to numbers - if isNumberTypeField(found.Type, actualType) { - logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(found.Type), string(actualType)) + // if there are no multifieds in ECS, nothing to compare + if len(ecsMultiFields) == 0 { return nil } - // any other field to validate here? - return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, found.Type) + + actualMultiFields, err := getMappingDefinitionsField("fields", definition) + if err != nil { + return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err) + } + + for _, ecsMultiField := range ecsMultiFields { + multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) + actualMultiField, ok := actualMultiFields[ecsMultiField.Name] + if !ok { + return fmt.Errorf("missing multi_field definition for %q", multiFieldPath) + } + + def, ok := actualMultiField.(map[string]any) + if !ok { + return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) + } + + err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) + if err != nil { + return err + } + } + + return nil +} + +func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error { + actualType := mappingParameter("type", actual) + if ecs.Type != actualType { + // exceptions related to numbers + if !isNumberTypeField(ecs.Type, actualType) { + return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type) + } else { + logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType)) + } + } + + // Compare other parameters + metricType := mappingParameter("time_series_metric", actual) + if ecs.MetricType != metricType { + return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType) + } + return nil } // flattenMappings returns all the mapping definitions found at "path" flattened including @@ -407,22 +460,9 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini func flattenMappings(path string, definition map[string]any) (map[string]any, error) { newDefs := map[string]any{} if isMultiFields(definition) { - multifields, err := getMappingDefinitionsField("fields", definition) - if err != nil { - return nil, multierror.Error{fmt.Errorf("invalid multi_field mapping %q: %w", path, err)} - } - - // Include also the definition itself newDefs[path] = definition - - for key, object := range multifields { - currentPath := currentMappingPath(path, key) - def, ok := object.(map[string]any) - if !ok { - return nil, multierror.Error{fmt.Errorf("invalid multi_field mapping type: %q", path)} - } - newDefs[currentPath] = def - } + // multi_fields are going to be validated directly with the dynamic templates + // or with ECS fields return newDefs, nil } @@ -646,6 +686,8 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil return errs.Unique() } +// matchingWithDynamicTemplates validates a given definition (currentPath) with a set of dynamic templates. +// The dynamic templates parameters are based on https://www.elastic.co/guide/en/elasticsearch/reference/8.17/dynamic-templates.html func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []map[string]any) error { parseSetting := func(value any) ([]string, error) { @@ -681,7 +723,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return elems[len(elems)-1] } - stringMatchesPatterns := func(regexes []string, elem string, fullRegex bool) (bool, error) { + stringMatchesPatterns := func(regexes []string, elem string) (bool, error) { applies := false for _, v := range regexes { if !strings.Contains(v, "*") { @@ -689,19 +731,9 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi continue } - var r string - if fullRegex { - r = v - } else { - r = strings.ReplaceAll(v, ".", "\\.") - r = strings.ReplaceAll(r, "*", ".*") - // Force to match beginning and ending - r = fmt.Sprintf("^%s$", r) - } - - match, err := regexp.MatchString(r, elem) + match, err := regexp.MatchString(v, elem) if err != nil { - return false, fmt.Errorf("failed to build regex %s: %w", r, err) + return false, fmt.Errorf("failed to build regex %s: %w", v, err) } if match { applies = true @@ -711,6 +743,18 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return applies, nil } + stringMatchesWildcards := func(regexes []string, elem string) (bool, error) { + for i, v := range regexes { + r := strings.ReplaceAll(v, ".", "\\.") + r = strings.ReplaceAll(r, "*", ".*") + // Force to match beginning and ending + r = fmt.Sprintf("^%s$", r) + + regexes[i] = r + } + return stringMatchesPatterns(regexes, elem) + } + fieldType := mappingParameter("type", definition) if fieldType == "" { return fmt.Errorf("missing type parameter for field: %q", currentPath) @@ -729,15 +773,17 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi rawContents = value } - // if strings.HasPrefix(templateName, "_embedded_ecs-") { - // continue - // } - // if strings.HasPrefix(templateName, "ecs_") { - // continue - // } - // if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { - // continue - // } + // Filter out dynamic templates created by elastic-package (import_mappings) + // or added automatically by ecs@mappings component template + if strings.HasPrefix(templateName, "_embedded_ecs-") { + continue + } + if strings.HasPrefix(templateName, "ecs_") { + continue + } + if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + continue + } logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) @@ -777,10 +823,17 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi matched = false break } - matches, err := stringMatchesPatterns(values, name, fullRegex) + + var matches bool + if fullRegex { + matches, err = stringMatchesPatterns(values, name) + } else { + matches, err = stringMatchesWildcards(values, name) + } if err != nil { return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } + if !matches { logger.Debugf(">> Issue: not matches") matched = false @@ -796,10 +849,17 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi matched = false break } - matches, err := stringMatchesPatterns(values, name, fullRegex) + + var matches bool + if fullRegex { + matches, err = stringMatchesPatterns(values, name) + } else { + matches, err = stringMatchesWildcards(values, name) + } if err != nil { return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } + if matches { logger.Debugf(">> Issue: matches") matched = false @@ -810,7 +870,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi if err != nil { return fmt.Errorf("failed to check path_match setting: %w", err) } - matches, err := stringMatchesPatterns(values, currentPath, false) + matches, err := stringMatchesWildcards(values, currentPath) if err != nil { return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } @@ -824,7 +884,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi if err != nil { return fmt.Errorf("failed to check path_unmatch setting: %w", err) } - matches, err := stringMatchesPatterns(values, currentPath, false) + matches, err := stringMatchesWildcards(values, currentPath) if err != nil { return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } @@ -888,7 +948,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } logger.Debugf(">>> No template matching") - return fmt.Errorf("no template matching for %q", currentPath) + return fmt.Errorf("no template matching for path: %q", currentPath) } // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values @@ -906,8 +966,14 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu if !ok { errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath)) } + if len(dynamicTemplates) == 0 { + logger.Debugf(">>> Compare mappings map[string]any %s", currentPath) + } errs = append(errs, v.compareMappings(currentPath, previewField, actualField, dynamicTemplates)...) case any: + if len(dynamicTemplates) == 0 { + logger.Debugf(">>> Compare mappings any %s", currentPath) + } // Validate each setting/parameter of the mapping // If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case if previewValue == actualValue { diff --git a/internal/fields/validate.go b/internal/fields/validate.go index 978f96781..62fe01252 100644 --- a/internal/fields/validate.go +++ b/internal/fields/validate.go @@ -30,6 +30,8 @@ import ( "github.com/elastic/elastic-package/internal/packages/buildmanifest" ) +const externalFieldAppendedTag = "ecs_component" + var ( semver2_0_0 = semver.MustParse("2.0.0") semver2_3_0 = semver.MustParse("2.3.0") @@ -448,7 +450,7 @@ func appendECSMappingMultifields(schema []FieldDefinition, prefix string) []Fiel { Name: "text", Type: "match_only_text", - External: "ecs", + External: externalFieldAppendedTag, }, }, }, From aa28db6615341e76edc8552405d1a4d25b75ca8d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 17 Dec 2024 20:09:54 +0100 Subject: [PATCH 06/51] Test without filtering dynamic templates --- internal/fields/mappings.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 9d67931af..4bf1eebb1 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -775,15 +775,15 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi // Filter out dynamic templates created by elastic-package (import_mappings) // or added automatically by ecs@mappings component template - if strings.HasPrefix(templateName, "_embedded_ecs-") { - continue - } - if strings.HasPrefix(templateName, "ecs_") { - continue - } - if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { - continue - } + // if strings.HasPrefix(templateName, "_embedded_ecs-") { + // continue + // } + // if strings.HasPrefix(templateName, "ecs_") { + // continue + // } + // if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + // continue + // } logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) From 3767111c0e44001b16a723336c2b60ed06324c40 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 18 Dec 2024 17:54:07 +0100 Subject: [PATCH 07/51] Ensure properties subfields are validated accordingly --- internal/fields/mappings.go | 29 ++++++----- internal/fields/mappings_test.go | 82 ++++++++++++++++++++++++++++++-- 2 files changed, 96 insertions(+), 15 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 4bf1eebb1..b1eaf6b6c 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -249,7 +249,7 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs.Unique() } - mappingErrs := v.compareMappings("", rawPreview, rawActual, rawDynamicTemplates) + mappingErrs := v.compareMappings("", false, rawPreview, rawActual, rawDynamicTemplates) errs = append(errs, mappingErrs...) if len(errs) > 0 { @@ -533,7 +533,7 @@ func validateConstantKeywordField(path string, preview, actual map[string]any) ( return isConstantKeyword, nil } -func (v *MappingValidator) compareMappings(path string, preview, actual map[string]any, dynamicTemplates []map[string]any) multierror.Error { +func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinition bool, preview, actual map[string]any, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error isConstantKeywordType, err := validateConstantKeywordField(path, preview, actual) @@ -554,7 +554,9 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri return nil } - if isObject(actual) { + // Ensure to validate properties from an object (subfields) in the right location of the mappings + // there could be "sub-fields" with name "properties" too + if couldBeParametersDefinition && isObject(actual) { if isObjectFullyDynamic(preview) { // TODO: Skip for now, it should be required to compare with dynamic templates logger.Debugf("Pending to validate with the dynamic templates defined the path: %q", path) @@ -571,7 +573,7 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri if err != nil { errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err)) } - compareErrors := v.compareMappings(path, previewProperties, actualProperties, dynamicTemplates) + compareErrors := v.compareMappings(path, false, previewProperties, actualProperties, dynamicTemplates) errs = append(errs, compareErrors...) if len(errs) == 0 { @@ -594,13 +596,13 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri if err != nil { errs = append(errs, fmt.Errorf("found invalid multi_fields type in actual mappings for path %q: %w", path, err)) } - compareErrors := v.compareMappings(path, previewFields, actualFields, dynamicTemplates) + compareErrors := v.compareMappings(path, false, previewFields, actualFields, dynamicTemplates) errs = append(errs, compareErrors...) // not returning here to keep validating the other fields of this object if any } // Compare and validate the elements under "properties": objects or fields and its parameters - propertiesErrs := v.validateObjectProperties(path, containsMultifield, actual, preview, dynamicTemplates) + propertiesErrs := v.validateObjectProperties(path, false, containsMultifield, actual, preview, dynamicTemplates) errs = append(errs, propertiesErrs...) if len(errs) == 0 { return nil @@ -608,13 +610,14 @@ func (v *MappingValidator) compareMappings(path string, preview, actual map[stri return errs.Unique() } -func (v *MappingValidator) validateObjectProperties(path string, containsMultifield bool, actual, preview map[string]any, dynamicTemplates []map[string]any) multierror.Error { +func (v *MappingValidator) validateObjectProperties(path string, couldBeParametersDefinition, containsMultifield bool, actual, preview map[string]any, dynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error for key, value := range actual { if containsMultifield && key == "fields" { // already checked continue } + currentPath := currentMappingPath(path, key) if skipValidationForField(currentPath) { continue @@ -630,12 +633,14 @@ func (v *MappingValidator) validateObjectProperties(path string, containsMultifi } ecsErrors := v.validateMappingsNotInPreview(currentPath, childField, dynamicTemplates) errs = append(errs, ecsErrors...) + continue } - + // Parameter not defined + errs = append(errs, fmt.Errorf("field %q is undefined", currentPath)) continue } - fieldErrs := v.validateObjectMappingAndParameters(preview[key], value, currentPath, dynamicTemplates) + fieldErrs := v.validateObjectMappingAndParameters(preview[key], value, currentPath, dynamicTemplates, true) errs = append(errs, fieldErrs...) } if len(errs) == 0 { @@ -939,7 +944,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } logger.Debugf(">> Check all the other parameters (%q): %q", templateName, currentPath) - errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}) + errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { return fmt.Errorf("invalid mapping found for %q: %w", currentPath, errs.Unique()) } @@ -953,7 +958,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values // in the actual mapping with the values in the preview mapping. -func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any) multierror.Error { +func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any, couldBeObjectDefinition bool) multierror.Error { var errs multierror.Error switch actualValue.(type) { case map[string]any: @@ -969,7 +974,7 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu if len(dynamicTemplates) == 0 { logger.Debugf(">>> Compare mappings map[string]any %s", currentPath) } - errs = append(errs, v.compareMappings(currentPath, previewField, actualField, dynamicTemplates)...) + errs = append(errs, v.compareMappings(currentPath, couldBeObjectDefinition, previewField, actualField, dynamicTemplates)...) case any: if len(dynamicTemplates) == 0 { logger.Debugf(">>> Compare mappings any %s", currentPath) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 2f98211dd..5df591b0d 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -538,6 +538,16 @@ func TestComparingMappings(t *testing.T) { }, }, }, + "bar": map[string]any{ + "properties": map[string]any{ + "type": map[string]any{ + "type": "keyword", + }, + "properties": map[string]any{ + "type": "keyword", + }, + }, + }, }, actual: map[string]any{ "foo": map[string]any{ @@ -552,9 +562,24 @@ func TestComparingMappings(t *testing.T) { }, }, }, + "bar": map[string]any{ + "properties": map[string]any{ + "type": map[string]any{ + "type": "keyword", + "ignore_above": 1024, + }, + "properties": map[string]any{ + "type": "keyword", + "ignore_above": 1024, + }, + }, + }, + }, + schema: []FieldDefinition{}, + expectedErrors: []string{ + `field "bar.type.ignore_above" is undefined`, + `field "bar.properties.ignore_above" is undefined`, }, - schema: []FieldDefinition{}, - expectedErrors: []string{}, }, { title: "different parameter values within an object", @@ -584,6 +609,32 @@ func TestComparingMappings(t *testing.T) { `unexpected value found in mapping for field "foo.type.ignore_above": preview mappings value (1024) different from the actual mappings value (2048)`, }, }, + { + title: "undefined parameter values within an object", + preview: map[string]any{ + "foo": map[string]any{ + "properties": map[string]any{ + "type": map[string]any{ + "type": "keyword", + }, + }, + }, + }, + actual: map[string]any{ + "foo": map[string]any{ + "properties": map[string]any{ + "type": map[string]any{ + "type": "keyword", + "time_series_matric": "counter", + }, + }, + }, + }, + schema: []FieldDefinition{}, + expectedErrors: []string{ + `field "foo.type.time_series_matric" is undefined`, + }, + }, { title: "different number types", preview: map[string]any{ @@ -625,11 +676,36 @@ func TestComparingMappings(t *testing.T) { }, }, }, + // foo is added to the exception list because it is type nested exceptionFields: []string{"foo"}, spec: "3.0.0", schema: []FieldDefinition{}, expectedErrors: []string{}, }, + { + title: "validate nested types starting spec 3.0.1", + preview: map[string]any{ + "foo": map[string]any{ + "type": "nested", + }, + }, + actual: map[string]any{ + "foo": map[string]any{ + "type": "nested", + "properties": map[string]any{ + "bar": map[string]any{ + "type": "long", + }, + }, + }, + }, + exceptionFields: []string{}, + spec: "3.0.1", + schema: []FieldDefinition{}, + expectedErrors: []string{ + `not found properties in preview mappings for path: "foo"`, + }, + }, } for _, c := range cases { @@ -649,7 +725,7 @@ func TestComparingMappings(t *testing.T) { ) require.NoError(t, err) - errs := v.compareMappings("", c.preview, c.actual, dynamicTemplates) + errs := v.compareMappings("", false, c.preview, c.actual, dynamicTemplates) if len(c.expectedErrors) > 0 { assert.Len(t, errs, len(c.expectedErrors)) for _, err := range errs { From 9fb80b4570ee4fdb85deca5ceaa24aa4ebdd635b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 18 Dec 2024 19:56:55 +0100 Subject: [PATCH 08/51] Restore filtering and add tests --- internal/fields/mappings.go | 66 ++++++++++++---------- internal/fields/mappings_test.go | 94 ++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 38 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index b1eaf6b6c..8a1d00750 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -401,15 +401,15 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini } } - if isMultiFields(definition) != (len(ecsMultiFields) > 0) { - return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields)) - } - // if there are no multifieds in ECS, nothing to compare if len(ecsMultiFields) == 0 { return nil } + if isMultiFields(definition) != (len(ecsMultiFields) > 0) { + return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields)) + } + actualMultiFields, err := getMappingDefinitionsField("fields", definition) if err != nil { return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err) @@ -573,6 +573,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit if err != nil { errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err)) } + // logger.Debugf(">> Compare object properties (mappings) - length %d: %q", len(actualProperties), path) compareErrors := v.compareMappings(path, false, previewProperties, actualProperties, dynamicTemplates) errs = append(errs, compareErrors...) @@ -596,11 +597,17 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit if err != nil { errs = append(errs, fmt.Errorf("found invalid multi_fields type in actual mappings for path %q: %w", path, err)) } + // if len(dynamicTemplates) == 0 { + // logger.Debugf(">>> Compare multi-fields %q", path) + // } compareErrors := v.compareMappings(path, false, previewFields, actualFields, dynamicTemplates) errs = append(errs, compareErrors...) // not returning here to keep validating the other fields of this object if any } + // if len(dynamicTemplates) == 0 { + // logger.Debugf(">>> Compare object properties %q", path) + // } // Compare and validate the elements under "properties": objects or fields and its parameters propertiesErrs := v.validateObjectProperties(path, false, containsMultifield, actual, preview, dynamicTemplates) errs = append(errs, propertiesErrs...) @@ -677,9 +684,11 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil } // validate whether or not the field has a corresponding dynamic template - err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) - if err == nil { - continue + if len(dynamicTemplates) > 0 { + err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) + if err == nil { + continue + } } // validate whether or not all fields under this key are defined in ECS @@ -780,15 +789,15 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi // Filter out dynamic templates created by elastic-package (import_mappings) // or added automatically by ecs@mappings component template - // if strings.HasPrefix(templateName, "_embedded_ecs-") { - // continue - // } - // if strings.HasPrefix(templateName, "ecs_") { - // continue - // } - // if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { - // continue - // } + if strings.HasPrefix(templateName, "_embedded_ecs-") { + continue + } + if strings.HasPrefix(templateName, "ecs_") { + continue + } + if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + continue + } logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) @@ -822,11 +831,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi logger.Debugf("> Check match: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { + logger.Warnf("failed to check match setting: %s", err) return fmt.Errorf("failed to check match setting: %w", err) } - if !slices.Contains(values, name) { - matched = false - break + if slices.Contains(values, name) { + logger.Warnf(">>>> no contained %s: %s", values, name) + continue } var matches bool @@ -939,20 +949,20 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi logger.Debugf("> Found dynamic template matched: %s", templateName) mappingParameter, ok := contents["mapping"] if !ok { - logger.Warnf(">> Missing \"mapping\" field: %s", templateName) return fmt.Errorf("missing mapping parameter in %s", templateName) } - logger.Debugf(">> Check all the other parameters (%q): %q", templateName, currentPath) + logger.Debugf(">> Check parameters (%q): %q", templateName, currentPath) errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { + logger.Warnf("invalid mapping found for %q:\n%s", currentPath, errs.Unique()) return fmt.Errorf("invalid mapping found for %q: %w", currentPath, errs.Unique()) } return nil } - logger.Debugf(">>> No template matching") + logger.Debugf(">>> No template matching for path: %q", currentPath) return fmt.Errorf("no template matching for path: %q", currentPath) } @@ -971,14 +981,14 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu if !ok { errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath)) } - if len(dynamicTemplates) == 0 { - logger.Debugf(">>> Compare mappings map[string]any %s", currentPath) - } + // if len(dynamicTemplates) == 0 { + // logger.Debugf(">>> Compare mappings map[string]any %s - length %d", currentPath, len(actualField)) + // } errs = append(errs, v.compareMappings(currentPath, couldBeObjectDefinition, previewField, actualField, dynamicTemplates)...) case any: - if len(dynamicTemplates) == 0 { - logger.Debugf(">>> Compare mappings any %s", currentPath) - } + // if len(dynamicTemplates) == 0 { + // logger.Debugf(">>> Compare mappings any %s", currentPath) + // } // Validate each setting/parameter of the mapping // If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case if previewValue == actualValue { diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 5df591b0d..3ba8ba1f7 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -16,13 +16,14 @@ import ( func TestComparingMappings(t *testing.T) { defaultSpecVersion := "3.3.0" cases := []struct { - title string - preview map[string]any - actual map[string]any - schema []FieldDefinition - spec string - exceptionFields []string - expectedErrors []string + title string + preview map[string]any + actual map[string]any + schema []FieldDefinition + dynamicTemplates []map[string]any + spec string + exceptionFields []string + expectedErrors []string }{ { title: "same mappings", @@ -706,12 +707,85 @@ func TestComparingMappings(t *testing.T) { `not found properties in preview mappings for path: "foo"`, }, }, + { + title: "fields matching dynamic templates", + preview: map[string]any{}, + actual: map[string]any{ + "foo": map[string]any{ + "type": "keyword", + }, + "foa": map[string]any{ + "type": "long", + }, + "fob": map[string]any{ + "type": "double", + "time_series_metric": "gauge", + }, + "bar": map[string]any{ + "type": "text", + "fields": map[string]any{ + "type": "keyword", + }, + }, + "bar_double": map[string]any{ + "type": "double", + }, + }, + dynamicTemplates: []map[string]any{ + { + "fo*_keyword": map[string]any{ + "path_match": "fo*", + "unmatch_mapping_type": []any{"long", "double"}, + "mapping": map[string]any{ + "type": "keyword", + }, + }, + }, + { + "fo*_number": map[string]any{ + "path_match": "fo*", + "match_mapping_type": []any{"long", "double"}, + "mapping": map[string]any{ + "type": "double", + "time_series_metric": "counter", + }, + }, + }, + { + "bar_match": map[string]any{ + "unmatch": []any{"foo", "foo42", "*42"}, + "match": []any{"*ar", "bar42"}, + "match_mapping_type": "text", + "mapping": map[string]any{ + "type": "text", + "fields": map[string]any{ + "type": "keyword", + }, + }, + }, + }, + { + "bar_star_double": map[string]any{ + "match": "*", + "unmatch_mapping_type": []any{"text"}, + "mapping": map[string]any{ + "type": "double", + }, + }, + }, + }, + exceptionFields: []string{}, + spec: "3.0.0", + schema: []FieldDefinition{}, + expectedErrors: []string{ + // Should it be considered this error in "foa" "missing time_series_metric bar", + `field "fob" is undefined: missing definition for path`, + }, + }, } for _, c := range cases { t.Run(c.title, func(t *testing.T) { - dynamicTemplates := []map[string]any{} - logger.EnableDebugMode() specVersion := defaultSpecVersion if c.spec != "" { @@ -725,7 +799,7 @@ func TestComparingMappings(t *testing.T) { ) require.NoError(t, err) - errs := v.compareMappings("", false, c.preview, c.actual, dynamicTemplates) + errs := v.compareMappings("", false, c.preview, c.actual, c.dynamicTemplates) if len(c.expectedErrors) > 0 { assert.Len(t, errs, len(c.expectedErrors)) for _, err := range errs { From 2ad28ac1c76f72209c7797fc6da8385946d1813a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 19 Dec 2024 11:33:24 +0100 Subject: [PATCH 09/51] Disable unmatch_mapping_type and match_mapping_type and continue looking for other dynamic templates if parameters do not match --- internal/fields/mappings.go | 56 +++++++++++++++++--------------- internal/fields/mappings_test.go | 4 ++- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 8a1d00750..508ed655d 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -907,31 +907,35 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi logger.Debugf(">> Issue: matches") matched = false } - case "match_mapping_type": - logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) - values, err := parseSetting(value) - if err != nil { - return fmt.Errorf("failed to check match_mapping_type setting: %w", err) - } - logger.Debugf(">> Comparing to values: %s", values) - if slices.Contains(values, "*") { - continue - } - if !slices.Contains(values, fieldType) { - logger.Debugf(">> Issue: not matches") - matched = false - } - case "unmatch_mapping_type": - logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) - values, err := parseSetting(value) - if err != nil { - return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) - } - logger.Debugf(">> Comparing to values: %s", values) - if slices.Contains(values, fieldType) { - logger.Debugf(">> Issue: matches") - matched = false - } + case "match_mapping_type", "unmatch_mapping_type": + // Do nothing + // These comparisons are done with the original data, and it's likely that the + // resulting mapping does not have the same type since it could change by the `mapping` field + // case "match_mapping_type": + // logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) + // values, err := parseSetting(value) + // if err != nil { + // return fmt.Errorf("failed to check match_mapping_type setting: %w", err) + // } + // logger.Debugf(">> Comparing to values: %s", values) + // if slices.Contains(values, "*") { + // continue + // } + // if !slices.Contains(values, fieldType) { + // logger.Debugf(">> Issue: not matches") + // matched = false + // } + // case "unmatch_mapping_type": + // logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) + // values, err := parseSetting(value) + // if err != nil { + // return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) + // } + // logger.Debugf(">> Comparing to values: %s", values) + // if slices.Contains(values, fieldType) { + // logger.Debugf(">> Issue: matches") + // matched = false + // } default: return fmt.Errorf("unexpected setting found in dynamic template") } @@ -956,7 +960,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { logger.Warnf("invalid mapping found for %q:\n%s", currentPath, errs.Unique()) - return fmt.Errorf("invalid mapping found for %q: %w", currentPath, errs.Unique()) + continue } return nil diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 3ba8ba1f7..5b27da5d9 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -715,7 +715,7 @@ func TestComparingMappings(t *testing.T) { "type": "keyword", }, "foa": map[string]any{ - "type": "long", + "type": "double", }, "fob": map[string]any{ "type": "double", @@ -735,6 +735,7 @@ func TestComparingMappings(t *testing.T) { { "fo*_keyword": map[string]any{ "path_match": "fo*", + "path_unmatch": []any{"foa", "fob"}, "unmatch_mapping_type": []any{"long", "double"}, "mapping": map[string]any{ "type": "keyword", @@ -744,6 +745,7 @@ func TestComparingMappings(t *testing.T) { { "fo*_number": map[string]any{ "path_match": "fo*", + "path_unmatch": "foo", "match_mapping_type": []any{"long", "double"}, "mapping": map[string]any{ "type": "double", From 151e59e8a8d4b5b8a658095363bac7c57a1f9b08 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 19 Dec 2024 17:20:07 +0100 Subject: [PATCH 10/51] Refactors and remove log statements --- internal/fields/mappings.go | 130 +++++++++++++++++------------------- 1 file changed, 63 insertions(+), 67 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index f3b8ddd4e..cb1036c5e 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -153,6 +153,7 @@ func createValidatorForMappingsAndPackageRoot(fieldsParentDir string, finder pac return v, nil } + // TODO: Should we remove this code to load external and local fields? fieldsDir := filepath.Join(fieldsParentDir, "fields") var fdm *DependencyManager @@ -266,6 +267,15 @@ func currentMappingPath(path, key string) string { return fmt.Sprintf("%s.%s", path, key) } +func fieldNameFromPath(path string) string { + if !strings.Contains(path, ".") { + return path + } + + elems := strings.Split(path, ".") + return elems[len(elems)-1] +} + func mappingParameter(field string, definition map[string]any) string { fieldValue, ok := definition[field] if !ok { @@ -396,6 +406,7 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini var ecsMultiFields []FieldDefinition // Filter multi-fields added by appendECSMappingMultifields for _, f := range found.MultiFields { + // TODO: Should we use another way to filter these fields? if f.External != externalFieldAppendedTag { ecsMultiFields = append(ecsMultiFields, f) } @@ -573,7 +584,6 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit if err != nil { errs = append(errs, fmt.Errorf("found invalid properties type in actual mappings for path %q: %w", path, err)) } - // logger.Debugf(">> Compare object properties (mappings) - length %d: %q", len(actualProperties), path) compareErrors := v.compareMappings(path, false, previewProperties, actualProperties, dynamicTemplates) errs = append(errs, compareErrors...) @@ -597,17 +607,11 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit if err != nil { errs = append(errs, fmt.Errorf("found invalid multi_fields type in actual mappings for path %q: %w", path, err)) } - // if len(dynamicTemplates) == 0 { - // logger.Debugf(">>> Compare multi-fields %q", path) - // } compareErrors := v.compareMappings(path, false, previewFields, actualFields, dynamicTemplates) errs = append(errs, compareErrors...) // not returning here to keep validating the other fields of this object if any } - // if len(dynamicTemplates) == 0 { - // logger.Debugf(">>> Compare object properties %q", path) - // } // Compare and validate the elements under "properties": objects or fields and its parameters propertiesErrs := v.validateObjectProperties(path, false, containsMultifield, preview, actual, dynamicTemplates) errs = append(errs, propertiesErrs...) @@ -728,15 +732,6 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return all, nil } - fieldName := func(path string) string { - if !strings.Contains(path, ".") { - return path - } - - elems := strings.Split(path, ".") - return elems[len(elems)-1] - } - stringMatchesPatterns := func(regexes []string, elem string) (bool, error) { applies := false for _, v := range regexes { @@ -787,20 +782,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi rawContents = value } - // Filter out dynamic templates created by elastic-package (import_mappings) - // or added automatically by ecs@mappings component template - if strings.HasPrefix(templateName, "_embedded_ecs-") { - continue - } - if strings.HasPrefix(templateName, "ecs_") { - continue - } - if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + if shouldSkipDynamicTemplate(templateName) { continue } - logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) - + // logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) contents, ok := rawContents.(map[string]any) if !ok { return fmt.Errorf("unexpected dynamic template format found for %q", templateName) @@ -828,7 +814,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi // Do nothing case "match": name := fieldName(currentPath) - logger.Debugf("> Check match: %q (key %q)", currentPath, name) + // logger.Debugf("> Check match: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { logger.Warnf("failed to check match setting: %s", err) @@ -850,12 +836,12 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } if !matches { - logger.Debugf(">> Issue: not matches") + // logger.Debugf(">> Issue: not matches") matched = false } case "unmatch": name := fieldName(currentPath) - logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) + // logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { return fmt.Errorf("failed to check unmatch setting: %w", err) @@ -876,11 +862,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi } if matches { - logger.Debugf(">> Issue: matches") + // logger.Debugf(">> Issue: matches") matched = false } case "path_match": - logger.Debugf("> Check path_match: %q", currentPath) + // logger.Debugf("> Check path_match: %q", currentPath) values, err := parseSetting(value) if err != nil { return fmt.Errorf("failed to check path_match setting: %w", err) @@ -890,11 +876,11 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if !matches { - logger.Debugf(">> Issue: not matches") + // logger.Debugf(">> Issue: not matches") matched = false } case "path_unmatch": - logger.Debugf("> Check path_unmatch: %q", currentPath) + // logger.Debugf("> Check path_unmatch: %q", currentPath) values, err := parseSetting(value) if err != nil { return fmt.Errorf("failed to check path_unmatch setting: %w", err) @@ -904,38 +890,38 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) } if matches { - logger.Debugf(">> Issue: matches") + // logger.Debugf(">> Issue: matches") matched = false } case "match_mapping_type", "unmatch_mapping_type": // Do nothing // These comparisons are done with the original data, and it's likely that the // resulting mapping does not have the same type since it could change by the `mapping` field - // case "match_mapping_type": - // logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) - // values, err := parseSetting(value) - // if err != nil { - // return fmt.Errorf("failed to check match_mapping_type setting: %w", err) - // } - // logger.Debugf(">> Comparing to values: %s", values) - // if slices.Contains(values, "*") { - // continue - // } - // if !slices.Contains(values, fieldType) { - // logger.Debugf(">> Issue: not matches") - // matched = false - // } - // case "unmatch_mapping_type": - // logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) - // values, err := parseSetting(value) - // if err != nil { - // return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) - // } - // logger.Debugf(">> Comparing to values: %s", values) - // if slices.Contains(values, fieldType) { - // logger.Debugf(">> Issue: matches") - // matched = false - // } + // case "match_mapping_type": + // logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) + // values, err := parseSetting(value) + // if err != nil { + // return fmt.Errorf("failed to check match_mapping_type setting: %w", err) + // } + // logger.Debugf(">> Comparing to values: %s", values) + // if slices.Contains(values, "*") { + // continue + // } + // if !slices.Contains(values, fieldType) { + // logger.Debugf(">> Issue: not matches") + // matched = false + // } + // case "unmatch_mapping_type": + // logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) + // values, err := parseSetting(value) + // if err != nil { + // return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) + // } + // logger.Debugf(">> Comparing to values: %s", values) + // if slices.Contains(values, fieldType) { + // logger.Debugf(">> Issue: matches") + // matched = false + // } default: return fmt.Errorf("unexpected setting found in dynamic template") } @@ -959,7 +945,8 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi logger.Debugf(">> Check parameters (%q): %q", templateName, currentPath) errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { - logger.Warnf("invalid mapping found for %q:\n%s", currentPath, errs.Unique()) + // Look for another dynamic template + logger.Debugf("invalid mapping found for %q:\n%s", currentPath, errs.Unique()) continue } @@ -970,6 +957,21 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return fmt.Errorf("no template matching for path: %q", currentPath) } +func shouldSkipDynamicTemplate(templateName string) bool { + // Filter out dynamic templates created by elastic-package (import_mappings) + // or added automatically by ecs@mappings component template + if strings.HasPrefix(templateName, "_embedded_ecs-") { + return true + } + if strings.HasPrefix(templateName, "ecs_") { + return true + } + if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + return true + } + return false +} + // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values // in the actual mapping with the values in the preview mapping. func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any, couldBeParametersDefinition bool) multierror.Error { @@ -985,14 +987,8 @@ func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actu if !ok { errs = append(errs, fmt.Errorf("unexpected type in actual mappings for path: %q", currentPath)) } - // if len(dynamicTemplates) == 0 { - // logger.Debugf(">>> Compare mappings map[string]any %s - length %d", currentPath, len(actualField)) - // } errs = append(errs, v.compareMappings(currentPath, couldBeParametersDefinition, previewField, actualField, dynamicTemplates)...) case any: - // if len(dynamicTemplates) == 0 { - // logger.Debugf(">>> Compare mappings any %s", currentPath) - // } // Validate each setting/parameter of the mapping // If a mapping exist in both preview and actual, they should be the same. But forcing to compare each parameter just in case if previewValue == actualValue { From d0ff6b91ec67c2cb053700773a3243a5c469f31d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 19 Dec 2024 17:26:23 +0100 Subject: [PATCH 11/51] Fix function naming --- internal/fields/mappings.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index cb1036c5e..186b2a9e1 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -813,7 +813,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi case "mapping", "match_pattern": // Do nothing case "match": - name := fieldName(currentPath) + name := fieldNameFromPath(currentPath) // logger.Debugf("> Check match: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { @@ -840,7 +840,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi matched = false } case "unmatch": - name := fieldName(currentPath) + name := fieldNameFromPath(currentPath) // logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) values, err := parseSetting(value) if err != nil { @@ -936,13 +936,13 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi continue } - logger.Debugf("> Found dynamic template matched: %s", templateName) + logger.Debugf("Found dynamic template matched: %s", templateName) mappingParameter, ok := contents["mapping"] if !ok { return fmt.Errorf("missing mapping parameter in %s", templateName) } - logger.Debugf(">> Check parameters (%q): %q", templateName, currentPath) + logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { // Look for another dynamic template @@ -953,7 +953,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return nil } - logger.Debugf(">>> No template matching for path: %q", currentPath) + logger.Debugf(">> No template matching for path: %q", currentPath) return fmt.Errorf("no template matching for path: %q", currentPath) } From b474b4b82a906340fa56906daa946bf4387695f8 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 19 Dec 2024 17:39:27 +0100 Subject: [PATCH 12/51] Add test with match_pattern regex --- internal/fields/mappings.go | 5 +++-- internal/fields/mappings_test.go | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 186b2a9e1..b36aa5e38 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -942,14 +942,15 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return fmt.Errorf("missing mapping parameter in %s", templateName) } - logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) + // logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) if errs != nil { // Look for another dynamic template - logger.Debugf("invalid mapping found for %q:\n%s", currentPath, errs.Unique()) + logger.Debugf("invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) continue } + logger.Debugf("Valid template for path %q: %q", currentPath, templateName) return nil } diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 5b27da5d9..48d6f9b5e 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -730,6 +730,9 @@ func TestComparingMappings(t *testing.T) { "bar_double": map[string]any{ "type": "double", }, + "full_regex_1": map[string]any{ + "type": "double", + }, }, dynamicTemplates: []map[string]any{ { @@ -769,12 +772,22 @@ func TestComparingMappings(t *testing.T) { { "bar_star_double": map[string]any{ "match": "*", + "unmatch": "full*", "unmatch_mapping_type": []any{"text"}, "mapping": map[string]any{ "type": "double", }, }, }, + { + "full_regex_1": map[string]any{ + "match_pattern": "regex", + "match": "^full_.*\\d$", + "mapping": map[string]any{ + "type": "double", + }, + }, + }, }, exceptionFields: []string{}, spec: "3.0.0", From 066b5a025fbd45c1aa3e5f19b492144b1794d793 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 19 Dec 2024 18:45:57 +0100 Subject: [PATCH 13/51] Add comment in tests --- internal/fields/mappings_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 48d6f9b5e..b9ec60475 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -793,7 +793,8 @@ func TestComparingMappings(t *testing.T) { spec: "3.0.0", schema: []FieldDefinition{}, expectedErrors: []string{ - // Should it be considered this error in "foa" "missing time_series_metric bar", + // Should it be considered this error in "foa" "missing time_series_metric bar"? + // "fob" is failing because it does not have the expected value for the "time_series_metric" field `field "fob" is undefined: missing definition for path`, }, }, From a7eb191bdddda72817af9c833ff40ccbb662320f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 7 Jan 2025 16:06:27 +0100 Subject: [PATCH 14/51] Remove loading schema from create validator for mappings method --- internal/fields/mappings.go | 92 +------------------- internal/fields/mappings_test.go | 14 +-- internal/testrunner/runners/system/tester.go | 4 +- 3 files changed, 5 insertions(+), 105 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index b36aa5e38..ed0f14eec 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -7,14 +7,11 @@ package fields import ( "context" "encoding/json" - "errors" "fmt" - "path/filepath" "regexp" "slices" "strings" - "github.com/Masterminds/semver/v3" "github.com/google/go-cmp/cmp" "github.com/elastic/elastic-package/internal/elasticsearch" @@ -27,17 +24,6 @@ type MappingValidator struct { // Schema contains definition records. Schema []FieldDefinition - // SpecVersion contains the version of the spec used by the package. - specVersion semver.Version - - disabledDependencyManagement bool - - enabledImportAllECSSchema bool - - disabledNormalization bool - - injectFieldsOptions InjectFieldsOptions - esClient *elasticsearch.Client indexTemplateName string @@ -50,50 +36,6 @@ type MappingValidator struct { // MappingValidatorOption represents an optional flag that can be passed to CreateValidatorForMappings. type MappingValidatorOption func(*MappingValidator) error -// WithMappingValidatorSpecVersion enables validation dependant of the spec version used by the package. -func WithMappingValidatorSpecVersion(version string) MappingValidatorOption { - return func(v *MappingValidator) error { - sv, err := semver.NewVersion(version) - if err != nil { - return fmt.Errorf("invalid version %q: %v", version, err) - } - v.specVersion = *sv - return nil - } -} - -// WithMappingValidatorDisabledDependencyManagement configures the validator to ignore external fields and won't follow dependencies. -func WithMappingValidatorDisabledDependencyManagement() MappingValidatorOption { - return func(v *MappingValidator) error { - v.disabledDependencyManagement = true - return nil - } -} - -// WithMappingValidatorEnabledImportAllECSSchema configures the validator to check or not the fields with the complete ECS schema. -func WithMappingValidatorEnabledImportAllECSSChema(importSchema bool) MappingValidatorOption { - return func(v *MappingValidator) error { - v.enabledImportAllECSSchema = importSchema - return nil - } -} - -// WithMappingValidatorDisableNormalization configures the validator to disable normalization. -func WithMappingValidatorDisableNormalization(disabledNormalization bool) MappingValidatorOption { - return func(v *MappingValidator) error { - v.disabledNormalization = disabledNormalization - return nil - } -} - -// WithMappingValidatorInjectFieldsOptions configures fields injection. -func WithMappingValidatorInjectFieldsOptions(options InjectFieldsOptions) MappingValidatorOption { - return func(v *MappingValidator) error { - v.injectFieldsOptions = options - return nil - } -} - // WithMappingValidatorElasticsearchClient configures the Elasticsearch client. func WithMappingValidatorElasticsearchClient(esClient *elasticsearch.Client) MappingValidatorOption { return func(v *MappingValidator) error { @@ -135,13 +77,12 @@ func WithMappingValidatorExceptionFields(fields []string) MappingValidatorOption } // CreateValidatorForMappings function creates a validator for the mappings. -func CreateValidatorForMappings(fieldsParentDir string, esClient *elasticsearch.Client, opts ...MappingValidatorOption) (v *MappingValidator, err error) { - p := packageRoot{} +func CreateValidatorForMappings(esClient *elasticsearch.Client, opts ...MappingValidatorOption) (v *MappingValidator, err error) { opts = append(opts, WithMappingValidatorElasticsearchClient(esClient)) - return createValidatorForMappingsAndPackageRoot(fieldsParentDir, p, opts...) + return createValidatorForMappingsAndPackageRoot(opts...) } -func createValidatorForMappingsAndPackageRoot(fieldsParentDir string, finder packageRootFinder, opts ...MappingValidatorOption) (v *MappingValidator, err error) { +func createValidatorForMappingsAndPackageRoot(opts ...MappingValidatorOption) (v *MappingValidator, err error) { v = new(MappingValidator) for _, opt := range opts { if err := opt(v); err != nil { @@ -149,33 +90,6 @@ func createValidatorForMappingsAndPackageRoot(fieldsParentDir string, finder pac } } - if len(v.Schema) > 0 { - return v, nil - } - - // TODO: Should we remove this code to load external and local fields? - fieldsDir := filepath.Join(fieldsParentDir, "fields") - - var fdm *DependencyManager - if !v.disabledDependencyManagement { - packageRoot, found, err := finder.FindPackageRoot() - if err != nil { - return nil, fmt.Errorf("can't find package root: %w", err) - } - if !found { - return nil, errors.New("package root not found and dependency management is enabled") - } - fdm, v.Schema, err = initDependencyManagement(packageRoot, v.specVersion, v.enabledImportAllECSSchema) - if err != nil { - return nil, fmt.Errorf("failed to initialize dependency management: %w", err) - } - } - fields, err := loadFieldsFromDir(fieldsDir, fdm, v.injectFieldsOptions) - if err != nil { - return nil, fmt.Errorf("can't load fields from directory (path: %s): %w", fieldsDir, err) - } - - v.Schema = append(fields, v.Schema...) return v, nil } diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index b9ec60475..a108d12ad 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -14,14 +14,12 @@ import ( ) func TestComparingMappings(t *testing.T) { - defaultSpecVersion := "3.3.0" cases := []struct { title string preview map[string]any actual map[string]any schema []FieldDefinition dynamicTemplates []map[string]any - spec string exceptionFields []string expectedErrors []string }{ @@ -517,7 +515,6 @@ func TestComparingMappings(t *testing.T) { }, }, exceptionFields: []string{"access.field"}, - spec: "1.0.0", expectedErrors: []string{ `field "error.field" is undefined: missing definition for path`, // should status.field return error ? or should it be ignored? @@ -679,7 +676,6 @@ func TestComparingMappings(t *testing.T) { }, // foo is added to the exception list because it is type nested exceptionFields: []string{"foo"}, - spec: "3.0.0", schema: []FieldDefinition{}, expectedErrors: []string{}, }, @@ -701,7 +697,6 @@ func TestComparingMappings(t *testing.T) { }, }, exceptionFields: []string{}, - spec: "3.0.1", schema: []FieldDefinition{}, expectedErrors: []string{ `not found properties in preview mappings for path: "foo"`, @@ -790,7 +785,6 @@ func TestComparingMappings(t *testing.T) { }, }, exceptionFields: []string{}, - spec: "3.0.0", schema: []FieldDefinition{}, expectedErrors: []string{ // Should it be considered this error in "foa" "missing time_series_metric bar"? @@ -803,14 +797,8 @@ func TestComparingMappings(t *testing.T) { for _, c := range cases { t.Run(c.title, func(t *testing.T) { logger.EnableDebugMode() - specVersion := defaultSpecVersion - if c.spec != "" { - specVersion = c.spec - } - v, err := CreateValidatorForMappings("", nil, - WithMappingValidatorSpecVersion(specVersion), + v, err := CreateValidatorForMappings(nil, WithMappingValidatorFallbackSchema(c.schema), - WithMappingValidatorDisabledDependencyManagement(), WithMappingValidatorExceptionFields(c.exceptionFields), ) require.NoError(t, err) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 356d54f34..2fa1d4b1e 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1528,12 +1528,10 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re logger.Warn("Validate mappings found (technical preview)") exceptionFields := listExceptionFields(scenario.docs, fieldsValidator) - mappingsValidator, err := fields.CreateValidatorForMappings(r.dataStreamPath, r.esClient, + mappingsValidator, err := fields.CreateValidatorForMappings(r.esClient, fields.WithMappingValidatorFallbackSchema(fieldsValidator.Schema), fields.WithMappingValidatorIndexTemplate(scenario.indexTemplateName), fields.WithMappingValidatorDataStream(scenario.dataStream), - fields.WithMappingValidatorSpecVersion(r.pkgManifest.SpecVersion), - fields.WithMappingValidatorEnabledImportAllECSSChema(true), fields.WithMappingValidatorExceptionFields(exceptionFields), ) if err != nil { From 7666ac2afc6242c40c789c0a501976164acb2aaf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 7 Jan 2025 18:07:33 +0100 Subject: [PATCH 15/51] Separate parsing and validation of dynamic templates --- internal/fields/dynamic_template.go | 255 +++++++++++++++++++++++++++ internal/fields/mappings.go | 264 ++-------------------------- 2 files changed, 271 insertions(+), 248 deletions(-) create mode 100644 internal/fields/dynamic_template.go diff --git a/internal/fields/dynamic_template.go b/internal/fields/dynamic_template.go new file mode 100644 index 000000000..fafe16f68 --- /dev/null +++ b/internal/fields/dynamic_template.go @@ -0,0 +1,255 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package fields + +import ( + "fmt" + "regexp" + "slices" + "strings" + + "github.com/elastic/elastic-package/internal/logger" +) + +type dynamicTemplate struct { + name string + matchPattern string + match []string + unmatch []string + pathMatch []string + unpathMatch []string + mapping any +} + +func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) (bool, error) { + fullRegex := d.matchPattern == "regex" + var err error + + if len(d.match) > 0 { + name := fieldNameFromPath(currentPath) + if !slices.Contains(d.match, name) { + // If there is no an exact match, it is compared with patterns/wildcards + + // logger.Warnf(">>>> no contained %s: %s", d.match, name) + var matches bool + if fullRegex { + matches, err = stringMatchesPatterns(d.match, name) + } else { + matches, err = stringMatchesWildcards(d.match, name) + } + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + } + + if !matches { + // logger.Debugf(">> Issue match: not matches") + return false, nil + } + } + } + + if len(d.unmatch) > 0 { + name := fieldNameFromPath(currentPath) + if slices.Contains(d.unmatch, name) { + return false, nil + } + + var matches bool + if fullRegex { + matches, err = stringMatchesPatterns(d.unmatch, name) + } else { + matches, err = stringMatchesWildcards(d.unmatch, name) + } + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + } + + if matches { + // logger.Debugf(">> Issue unmatch: matches") + return false, nil + } + } + + if len(d.pathMatch) > 0 { + // logger.Debugf("path_match -> Comparing %s to %q", strings.Join(d.pathMatch, ";"), currentPath) + matches, err := stringMatchesWildcards(d.pathMatch, currentPath) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + } + if !matches { + logger.Debugf(">> Issue path_match: not matches (currentPath %s)", currentPath) + return false, nil + } + } + + if len(d.unpathMatch) > 0 { + matches, err := stringMatchesWildcards(d.unpathMatch, currentPath) + if err != nil { + return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + } + if matches { + // logger.Debugf(">> Issue unpath_match: matches") + return false, nil + } + } + return true, nil +} + +func stringMatchesPatterns(regexes []string, elem string) (bool, error) { + applies := false + for _, v := range regexes { + if !strings.Contains(v, "*") { + // not a regex + continue + } + + match, err := regexp.MatchString(v, elem) + if err != nil { + return false, fmt.Errorf("failed to build regex %s: %w", v, err) + } + if match { + applies = true + break + } + } + return applies, nil +} + +func stringMatchesWildcards(regexes []string, elem string) (bool, error) { + updatedRegexes := []string{} + for _, v := range regexes { + r := strings.ReplaceAll(v, ".", "\\.") + r = strings.ReplaceAll(r, "*", ".*") + + // Force to match the beginning and ending of the given path + r = fmt.Sprintf("^%s$", r) + + updatedRegexes = append(updatedRegexes, r) + } + return stringMatchesPatterns(updatedRegexes, elem) +} + +func parseDynamicTemplates(rawDynamicTemplates []map[string]any) ([]dynamicTemplate, error) { + dynamicTemplates := []dynamicTemplate{} + + for _, template := range rawDynamicTemplates { + if len(template) != 1 { + return nil, fmt.Errorf("unexpected number of dynamic template definitions found") + } + + // there is just one dynamic template per object + templateName := "" + var rawContents any + for key, value := range template { + templateName = key + rawContents = value + } + + if shouldSkipDynamicTemplate(templateName) { + continue + } + + aDynamicTemplate := dynamicTemplate{ + name: templateName, + } + + // logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) + contents, ok := rawContents.(map[string]any) + if !ok { + return nil, fmt.Errorf("unexpected dynamic template format found for %q", templateName) + } + + for setting, value := range contents { + switch setting { + case "mapping": + aDynamicTemplate.mapping = value + case "match_pattern": + s, ok := value.(string) + if !ok { + return nil, fmt.Errorf("invalid type for \"match_pattern\": %T", value) + } + aDynamicTemplate.matchPattern = s + case "match": + // logger.Debugf("> Check match: %q (key %q)", currentPath, name) + values, err := parseDynamicTemplateParameter(value) + if err != nil { + logger.Warnf("failed to check match setting: %s", err) + return nil, fmt.Errorf("failed to check match setting: %w", err) + } + aDynamicTemplate.match = values + case "unmatch": + // logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) + values, err := parseDynamicTemplateParameter(value) + if err != nil { + return nil, fmt.Errorf("failed to check unmatch setting: %w", err) + } + aDynamicTemplate.unmatch = values + case "path_match": + // logger.Debugf("> Check path_match: %q", currentPath) + values, err := parseDynamicTemplateParameter(value) + if err != nil { + return nil, fmt.Errorf("failed to check path_match setting: %w", err) + } + aDynamicTemplate.pathMatch = values + case "path_unmatch": + // logger.Debugf("> Check path_unmatch: %q", currentPath) + values, err := parseDynamicTemplateParameter(value) + if err != nil { + return nil, fmt.Errorf("failed to check path_unmatch setting: %w", err) + } + aDynamicTemplate.unpathMatch = values + case "match_mapping_type", "unmatch_mapping_type": + // Do nothing + // These parameters require to check the original type (before the document is ingested) + // but the dynamic template just contains the type from the `mapping` field + default: + return nil, fmt.Errorf("unexpected setting found in dynamic template") + } + } + + dynamicTemplates = append(dynamicTemplates, aDynamicTemplate) + } + + return dynamicTemplates, nil +} + +func shouldSkipDynamicTemplate(templateName string) bool { + // Filter out dynamic templates created by elastic-package (import_mappings) + // or added automatically by ecs@mappings component template + if strings.HasPrefix(templateName, "_embedded_ecs-") { + return true + } + if strings.HasPrefix(templateName, "ecs_") { + return true + } + if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { + return true + } + return false +} + +func parseDynamicTemplateParameter(value any) ([]string, error) { + all := []string{} + switch v := value.(type) { + case []any: + for _, elem := range v { + s, ok := elem.(string) + if !ok { + return nil, fmt.Errorf("failed to cast to string: %s", elem) + } + all = append(all, s) + } + case any: + s, ok := v.(string) + if !ok { + return nil, fmt.Errorf("failed to cast to string: %s", v) + } + all = append(all, s) + default: + return nil, fmt.Errorf("unexpected type for setting: %T", value) + + } + return all, nil +} diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index ed0f14eec..280071274 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -8,7 +8,6 @@ import ( "context" "encoding/json" "fmt" - "regexp" "slices" "strings" @@ -576,7 +575,7 @@ func (v *MappingValidator) validateObjectProperties(path string, couldBeParamete // validateMappingsNotInPreview validates the object and the nested objects in the current path with other resources // like ECS schema, dynamic templates or local fields defined in the package (type array). -func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField map[string]any, dynamicTemplates []map[string]any) multierror.Error { +func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, childField map[string]any, rawDynamicTemplates []map[string]any) multierror.Error { var errs multierror.Error flattenFields, err := flattenMappings(currentPath, childField) if err != nil { @@ -584,6 +583,11 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil return errs } + dynamicTemplates, err := parseDynamicTemplates(rawDynamicTemplates) + if err != nil { + return multierror.Error{fmt.Errorf("failed to parse dynamic templates: %w", err)} + } + for fieldPath, object := range flattenFields { if slices.Contains(v.exceptionFields, fieldPath) { logger.Warnf("Found exception field, skip its validation (not in preview): %q", fieldPath) @@ -602,7 +606,7 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil } // validate whether or not the field has a corresponding dynamic template - if len(dynamicTemplates) > 0 { + if len(rawDynamicTemplates) > 0 { err := v.matchingWithDynamicTemplates(fieldPath, def, dynamicTemplates) if err == nil { continue @@ -620,251 +624,30 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil // matchingWithDynamicTemplates validates a given definition (currentPath) with a set of dynamic templates. // The dynamic templates parameters are based on https://www.elastic.co/guide/en/elasticsearch/reference/8.17/dynamic-templates.html -func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []map[string]any) error { - - parseSetting := func(value any) ([]string, error) { - all := []string{} - switch v := value.(type) { - case []any: - for _, elem := range v { - s, ok := elem.(string) - if !ok { - return nil, fmt.Errorf("failed to cast to string: %s", elem) - } - all = append(all, s) - } - case any: - s, ok := v.(string) - if !ok { - return nil, fmt.Errorf("failed to cast to string: %s", v) - } - all = append(all, s) - default: - return nil, fmt.Errorf("unexpected type for setting: %T", value) - - } - return all, nil - } - - stringMatchesPatterns := func(regexes []string, elem string) (bool, error) { - applies := false - for _, v := range regexes { - if !strings.Contains(v, "*") { - // not a regex - continue - } - - match, err := regexp.MatchString(v, elem) - if err != nil { - return false, fmt.Errorf("failed to build regex %s: %w", v, err) - } - if match { - applies = true - break - } - } - return applies, nil - } - - stringMatchesWildcards := func(regexes []string, elem string) (bool, error) { - for i, v := range regexes { - r := strings.ReplaceAll(v, ".", "\\.") - r = strings.ReplaceAll(r, "*", ".*") - // Force to match beginning and ending - r = fmt.Sprintf("^%s$", r) - - regexes[i] = r - } - return stringMatchesPatterns(regexes, elem) - } - - fieldType := mappingParameter("type", definition) - if fieldType == "" { - return fmt.Errorf("missing type parameter for field: %q", currentPath) - } - +func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, definition map[string]any, dynamicTemplates []dynamicTemplate) error { for _, template := range dynamicTemplates { - if len(template) != 1 { - return fmt.Errorf("unexpected number of dynamic template definitions found") - } - - // there is just one dynamic template per object - templateName := "" - var rawContents any - for key, value := range template { - templateName = key - rawContents = value - } - - if shouldSkipDynamicTemplate(templateName) { - continue - } - - // logger.Debugf("Checking dynamic template for %q: %q", currentPath, templateName) - contents, ok := rawContents.(map[string]any) - if !ok { - return fmt.Errorf("unexpected dynamic template format found for %q", templateName) - } - - fullRegex := false - if v, ok := contents["match_pattern"]; ok { - s, ok := v.(string) - if !ok { - return fmt.Errorf("invalid type for \"match_pattern\": %T", v) - } - if s == "regex" { - logger.Debugf("Use full regex in dynamic templates (match_pattern: regex)") - fullRegex = true - } + matches, err := template.Matches(currentPath, definition) + if err != nil { + return fmt.Errorf("failed to validate %q with dynamic template %q: %w", currentPath, template.name, err) } - // matches with the current definitions and path - // https://www.elastic.co/guide/en/elasticsearch/reference/current/dynamic-templates.html - allMatched := true - for setting, value := range contents { - matched := true - switch setting { - case "mapping", "match_pattern": - // Do nothing - case "match": - name := fieldNameFromPath(currentPath) - // logger.Debugf("> Check match: %q (key %q)", currentPath, name) - values, err := parseSetting(value) - if err != nil { - logger.Warnf("failed to check match setting: %s", err) - return fmt.Errorf("failed to check match setting: %w", err) - } - if slices.Contains(values, name) { - logger.Warnf(">>>> no contained %s: %s", values, name) - continue - } - - var matches bool - if fullRegex { - matches, err = stringMatchesPatterns(values, name) - } else { - matches, err = stringMatchesWildcards(values, name) - } - if err != nil { - return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) - } - - if !matches { - // logger.Debugf(">> Issue: not matches") - matched = false - } - case "unmatch": - name := fieldNameFromPath(currentPath) - // logger.Debugf("> Check unmatch: %q (key %q)", currentPath, name) - values, err := parseSetting(value) - if err != nil { - return fmt.Errorf("failed to check unmatch setting: %w", err) - } - if slices.Contains(values, name) { - matched = false - break - } - - var matches bool - if fullRegex { - matches, err = stringMatchesPatterns(values, name) - } else { - matches, err = stringMatchesWildcards(values, name) - } - if err != nil { - return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) - } - - if matches { - // logger.Debugf(">> Issue: matches") - matched = false - } - case "path_match": - // logger.Debugf("> Check path_match: %q", currentPath) - values, err := parseSetting(value) - if err != nil { - return fmt.Errorf("failed to check path_match setting: %w", err) - } - matches, err := stringMatchesWildcards(values, currentPath) - if err != nil { - return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) - } - if !matches { - // logger.Debugf(">> Issue: not matches") - matched = false - } - case "path_unmatch": - // logger.Debugf("> Check path_unmatch: %q", currentPath) - values, err := parseSetting(value) - if err != nil { - return fmt.Errorf("failed to check path_unmatch setting: %w", err) - } - matches, err := stringMatchesWildcards(values, currentPath) - if err != nil { - return fmt.Errorf("failed to parse dynamic template %s: %w", templateName, err) - } - if matches { - // logger.Debugf(">> Issue: matches") - matched = false - } - case "match_mapping_type", "unmatch_mapping_type": - // Do nothing - // These comparisons are done with the original data, and it's likely that the - // resulting mapping does not have the same type since it could change by the `mapping` field - // case "match_mapping_type": - // logger.Debugf("> Check match_mapping_type: %q (type %s)", currentPath, fieldType) - // values, err := parseSetting(value) - // if err != nil { - // return fmt.Errorf("failed to check match_mapping_type setting: %w", err) - // } - // logger.Debugf(">> Comparing to values: %s", values) - // if slices.Contains(values, "*") { - // continue - // } - // if !slices.Contains(values, fieldType) { - // logger.Debugf(">> Issue: not matches") - // matched = false - // } - // case "unmatch_mapping_type": - // logger.Debugf("> Check unmatch_mapping_type: %q (type %s)", currentPath, fieldType) - // values, err := parseSetting(value) - // if err != nil { - // return fmt.Errorf("failed to check unmatch_mapping_type setting: %w", err) - // } - // logger.Debugf(">> Comparing to values: %s", values) - // if slices.Contains(values, fieldType) { - // logger.Debugf(">> Issue: matches") - // matched = false - // } - default: - return fmt.Errorf("unexpected setting found in dynamic template") - } - if !matched { - // If just one parameter does not match, this dynamic template can be skipped - allMatched = false - break - } - } - if !allMatched { + if !matches { // Look for another dynamic template continue } - logger.Debugf("Found dynamic template matched: %s", templateName) - mappingParameter, ok := contents["mapping"] - if !ok { - return fmt.Errorf("missing mapping parameter in %s", templateName) - } + logger.Debugf("Found dynamic template matched: %s", template.name) // logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) - errs := v.validateObjectMappingAndParameters(mappingParameter, definition, currentPath, []map[string]any{}, true) + // Check that all parameters match (setting no dynamic templates to avoid recursion) + errs := v.validateObjectMappingAndParameters(template.mapping, definition, currentPath, []map[string]any{}, true) if errs != nil { // Look for another dynamic template logger.Debugf("invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) continue } - logger.Debugf("Valid template for path %q: %q", currentPath, templateName) + logger.Debugf("Valid template for path %q: %q", currentPath, template.name) return nil } @@ -872,21 +655,6 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return fmt.Errorf("no template matching for path: %q", currentPath) } -func shouldSkipDynamicTemplate(templateName string) bool { - // Filter out dynamic templates created by elastic-package (import_mappings) - // or added automatically by ecs@mappings component template - if strings.HasPrefix(templateName, "_embedded_ecs-") { - return true - } - if strings.HasPrefix(templateName, "ecs_") { - return true - } - if slices.Contains([]string{"all_strings_to_keywords", "strings_as_keyword"}, templateName) { - return true - } - return false -} - // validateObjectMappingAndParameters validates the current object or field parameter (currentPath) comparing the values // in the actual mapping with the values in the preview mapping. func (v *MappingValidator) validateObjectMappingAndParameters(previewValue, actualValue any, currentPath string, dynamicTemplates []map[string]any, couldBeParametersDefinition bool) multierror.Error { From a718796431faedeefe457e525de4ae5f4ecb612f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 7 Jan 2025 20:10:47 +0100 Subject: [PATCH 16/51] Support match_pattern for the other settings --- internal/fields/dynamic_template.go | 30 ++++++++++++----------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/internal/fields/dynamic_template.go b/internal/fields/dynamic_template.go index fafe16f68..4af966e6d 100644 --- a/internal/fields/dynamic_template.go +++ b/internal/fields/dynamic_template.go @@ -25,7 +25,6 @@ type dynamicTemplate struct { func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) (bool, error) { fullRegex := d.matchPattern == "regex" - var err error if len(d.match) > 0 { name := fieldNameFromPath(currentPath) @@ -33,12 +32,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) // If there is no an exact match, it is compared with patterns/wildcards // logger.Warnf(">>>> no contained %s: %s", d.match, name) - var matches bool - if fullRegex { - matches, err = stringMatchesPatterns(d.match, name) - } else { - matches, err = stringMatchesWildcards(d.match, name) - } + matches, err := stringMatchesPatterns(d.match, name, fullRegex) if err != nil { return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) } @@ -56,12 +50,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) return false, nil } - var matches bool - if fullRegex { - matches, err = stringMatchesPatterns(d.unmatch, name) - } else { - matches, err = stringMatchesWildcards(d.unmatch, name) - } + matches, err := stringMatchesPatterns(d.unmatch, name, fullRegex) if err != nil { return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) } @@ -74,7 +63,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) if len(d.pathMatch) > 0 { // logger.Debugf("path_match -> Comparing %s to %q", strings.Join(d.pathMatch, ";"), currentPath) - matches, err := stringMatchesWildcards(d.pathMatch, currentPath) + matches, err := stringMatchesPatterns(d.pathMatch, currentPath, fullRegex) if err != nil { return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) } @@ -85,7 +74,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) } if len(d.unpathMatch) > 0 { - matches, err := stringMatchesWildcards(d.unpathMatch, currentPath) + matches, err := stringMatchesPatterns(d.unpathMatch, currentPath, fullRegex) if err != nil { return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) } @@ -97,7 +86,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) return true, nil } -func stringMatchesPatterns(regexes []string, elem string) (bool, error) { +func stringMatchesRegex(regexes []string, elem string) (bool, error) { applies := false for _, v := range regexes { if !strings.Contains(v, "*") { @@ -117,7 +106,12 @@ func stringMatchesPatterns(regexes []string, elem string) (bool, error) { return applies, nil } -func stringMatchesWildcards(regexes []string, elem string) (bool, error) { +func stringMatchesPatterns(regexes []string, elem string, fullRegex bool) (bool, error) { + if fullRegex { + return stringMatchesRegex(regexes, elem) + } + + // transform wildcards to valid regexes updatedRegexes := []string{} for _, v := range regexes { r := strings.ReplaceAll(v, ".", "\\.") @@ -128,7 +122,7 @@ func stringMatchesWildcards(regexes []string, elem string) (bool, error) { updatedRegexes = append(updatedRegexes, r) } - return stringMatchesPatterns(updatedRegexes, elem) + return stringMatchesRegex(updatedRegexes, elem) } func parseDynamicTemplates(rawDynamicTemplates []map[string]any) ([]dynamicTemplate, error) { From 0a5e7752d0de6157ad0ea9430945005b017a97b6 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 11:44:25 +0100 Subject: [PATCH 17/51] fix test --- internal/fields/mappings_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index a108d12ad..dc198f289 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -719,7 +719,9 @@ func TestComparingMappings(t *testing.T) { "bar": map[string]any{ "type": "text", "fields": map[string]any{ - "type": "keyword", + "text": map[string]any{ + "type": "keyword", + }, }, }, "bar_double": map[string]any{ @@ -759,7 +761,9 @@ func TestComparingMappings(t *testing.T) { "mapping": map[string]any{ "type": "text", "fields": map[string]any{ - "type": "keyword", + "text": map[string]any{ + "type": "keyword", + }, }, }, }, From 131cc9ceadb55dd09c93cd309ce66b28afd54c6c Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 12:52:26 +0100 Subject: [PATCH 18/51] Update tests for multi-fields --- internal/fields/mappings_test.go | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index dc198f289..b9d02688f 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -241,7 +241,7 @@ func TestComparingMappings(t *testing.T) { "bar": map[string]any{ "properties": map[string]any{ "type": map[string]any{ - "type": "constant_keyword", + "type": "keyword", }, "fields": map[string]any{ "type": "text", @@ -269,7 +269,12 @@ func TestComparingMappings(t *testing.T) { "bar": map[string]any{ "properties": map[string]any{ "type": map[string]any{ - "type": "constant_keyword", + "type": "keyword", + "fields": map[string]any{ + "text": map[string]any{ + "type": "match_only_text", + }, + }, }, "fields": map[string]any{ "type": "text", @@ -285,6 +290,7 @@ func TestComparingMappings(t *testing.T) { schema: []FieldDefinition{}, expectedErrors: []string{ `field "foo.text" is undefined: missing definition for path`, + `not found multi_fields in preview mappings for path: "bar.type"`, }, }, { @@ -799,6 +805,9 @@ func TestComparingMappings(t *testing.T) { } for _, c := range cases { + if c.title != "validate multifields failure" { + continue + } t.Run(c.title, func(t *testing.T) { logger.EnableDebugMode() v, err := CreateValidatorForMappings(nil, From 38db8dfd24274c425dbff647eddca0d48b4b9e3f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 13:01:45 +0100 Subject: [PATCH 19/51] Review multi-fields logic --- internal/fields/mappings.go | 51 ++++++++++++++++++++++---------- internal/fields/mappings_test.go | 3 -- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 280071274..28d82fa41 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -325,13 +325,16 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini } } - // if there are no multifieds in ECS, nothing to compare - if len(ecsMultiFields) == 0 { + // if there are no multifieds in the actual definition, nothing to compare + if !isMultiFields(definition) { return nil } + // if len(ecsMultiFields) == 0 { + // return nil + // } - if isMultiFields(definition) != (len(ecsMultiFields) > 0) { - return fmt.Errorf("not matched definitions for multifields for %q: actual multi_fields in mappings %t - ECS multi_fields length %d", currentPath, isMultiFields(definition), len(ecsMultiFields)) + if len(ecsMultiFields) == 0 { + return fmt.Errorf("not matched definitions for multifields for %q: no multi-fields found in ECS", currentPath) } actualMultiFields, err := getMappingDefinitionsField("fields", definition) @@ -339,25 +342,41 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err) } - for _, ecsMultiField := range ecsMultiFields { - multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) - actualMultiField, ok := actualMultiFields[ecsMultiField.Name] - if !ok { - return fmt.Errorf("missing multi_field definition for %q", multiFieldPath) - } + for key, value := range actualMultiFields { + multiFieldPath := currentMappingPath(currentPath, key) - def, ok := actualMultiField.(map[string]any) + def, ok := value.(map[string]any) if !ok { return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) } - - err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) - if err != nil { - return err + for _, ecsMultiField := range ecsMultiFields { + err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) + if err == nil { + return nil + } } } + return fmt.Errorf("not found multi-field definition in ECS for %q", currentPath) - return nil + // for _, ecsMultiField := range ecsMultiFields { + // multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) + // actualMultiField, ok := actualMultiFields[ecsMultiField.Name] + // if !ok { + // return fmt.Errorf("missing multi_field definition for %q", multiFieldPath) + // } + + // def, ok := actualMultiField.(map[string]any) + // if !ok { + // return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) + // } + + // err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) + // if err != nil { + // return err + // } + // } + + // return nil } func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error { diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index b9d02688f..e517b2590 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -805,9 +805,6 @@ func TestComparingMappings(t *testing.T) { } for _, c := range cases { - if c.title != "validate multifields failure" { - continue - } t.Run(c.title, func(t *testing.T) { logger.EnableDebugMode() v, err := CreateValidatorForMappings(nil, From 9ff3d0cf145ed8cd6c4563519d7bde7cfce7d926 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 14:09:14 +0100 Subject: [PATCH 20/51] Revisit multi-fields logic in ECS --- internal/fields/mappings.go | 62 ++++++++++++++++++++------------ internal/fields/mappings_test.go | 56 +++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 23 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 28d82fa41..d3556938c 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -315,7 +315,29 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini return err } - // Compare multifields + return compareMultiFieldsInECSSchema(currentPath, found, definition) +} + +func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error { + actualType := mappingParameter("type", actual) + if ecs.Type != actualType { + // exceptions related to numbers + if !isNumberTypeField(ecs.Type, actualType) { + return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type) + } else { + logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType)) + } + } + + // Compare other parameters + metricType := mappingParameter("time_series_metric", actual) + if ecs.MetricType != metricType { + return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType) + } + return nil +} + +func compareMultiFieldsInECSSchema(currentPath string, found *FieldDefinition, definition map[string]any) error { var ecsMultiFields []FieldDefinition // Filter multi-fields added by appendECSMappingMultifields for _, f := range found.MultiFields { @@ -342,6 +364,7 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err) } + missingMultiFields := []string{} for key, value := range actualMultiFields { multiFieldPath := currentMappingPath(currentPath, key) @@ -349,14 +372,26 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini if !ok { return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) } + found := false for _, ecsMultiField := range ecsMultiFields { + if ecsMultiField.Name != key { + continue + } err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) if err == nil { - return nil + found = true + break } } + if !found { + missingMultiFields = append(missingMultiFields, key) + } + } + if len(missingMultiFields) > 0 { + return fmt.Errorf("missing definition for multi-fields in ECS: %q", strings.Join(missingMultiFields, ", ")) } - return fmt.Errorf("not found multi-field definition in ECS for %q", currentPath) + + return nil // for _, ecsMultiField := range ecsMultiFields { // multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) @@ -379,25 +414,6 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini // return nil } -func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error { - actualType := mappingParameter("type", actual) - if ecs.Type != actualType { - // exceptions related to numbers - if !isNumberTypeField(ecs.Type, actualType) { - return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type) - } else { - logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType)) - } - } - - // Compare other parameters - metricType := mappingParameter("time_series_metric", actual) - if ecs.MetricType != metricType { - return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType) - } - return nil -} - // flattenMappings returns all the mapping definitions found at "path" flattened including // specific entries for multi fields too. func flattenMappings(path string, definition map[string]any) (map[string]any, error) { @@ -670,7 +686,7 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi return nil } - logger.Debugf(">> No template matching for path: %q", currentPath) + // logger.Debugf(">> No template matching for path: %q", currentPath) return fmt.Errorf("no template matching for path: %q", currentPath) } diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index e517b2590..d002aa9c5 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -99,6 +99,24 @@ func TestComparingMappings(t *testing.T) { "foo": map[string]any{ "type": "keyword", }, + "user": map[string]any{ + "type": "keyword", + }, + "time": map[string]any{ + "type": "keyword", + "fields": map[string]any{ + "text": map[string]any{ + "type": "match_only_text", + }, + "other": map[string]any{ + "type": "match_only_text", + }, + }, + }, + // Should this fail since it has no multi-fields as in the ECS definition? + "name": map[string]any{ + "type": "keyword", + }, }, schema: []FieldDefinition{ { @@ -116,9 +134,35 @@ func TestComparingMappings(t *testing.T) { Type: "keyword", External: "", }, + { + Name: "time", + Type: "keyword", + External: "ecs", + MultiFields: []FieldDefinition{ + { + Name: "text", + Type: "match_only_text", + External: "ecs", + }, + }, + }, + { + Name: "name", + Type: "keyword", + External: "ecs", + MultiFields: []FieldDefinition{ + { + Name: "text", + Type: "match_only_text", + External: "ecs", + }, + }, + }, }, expectedErrors: []string{ `field "metrics" is undefined: actual mapping type (long) does not match with ECS definition type: keyword`, + `field "user" is undefined: missing definition for path (not in ECS)`, + `field "time" is undefined: missing definition for multi-fields in ECS: "other"`, }, }, { @@ -230,6 +274,14 @@ func TestComparingMappings(t *testing.T) { { title: "validate multifields failure", preview: map[string]any{ + "time": map[string]any{ + "type": "keyword", + "fields": map[string]any{ + "text": map[string]any{ + "type": "match_only_text", + }, + }, + }, "foo": map[string]any{ "type": "keyword", "fields": map[string]any{ @@ -255,6 +307,10 @@ func TestComparingMappings(t *testing.T) { }, }, actual: map[string]any{ + // Should this fail since it has no multi-fields as in the preview? + "time": map[string]any{ + "type": "keyword", + }, "foo": map[string]any{ "type": "keyword", "fields": map[string]any{ From 51656120125f6c1d2a126739c554e399e162a836 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 18:38:52 +0100 Subject: [PATCH 21/51] Report all errors related to multi-fields comparing with ECS --- internal/fields/mappings.go | 105 +++++++++++++++---------------- internal/fields/mappings_test.go | 2 +- 2 files changed, 50 insertions(+), 57 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index d3556938c..2f0d4aa47 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -300,30 +300,31 @@ func isNumberTypeField(previewType, actualType string) bool { return false } -func (v *MappingValidator) validateMappingInECSSchema(currentPath string, definition map[string]any) error { +func (v *MappingValidator) validateMappingInECSSchema(currentPath string, definition map[string]any) multierror.Error { found := FindElementDefinition(currentPath, v.Schema) if found == nil { - return fmt.Errorf("missing definition for path") + return multierror.Error{fmt.Errorf("missing definition for path")} } if found.External != "ecs" { - return fmt.Errorf("missing definition for path (not in ECS)") + return multierror.Error{fmt.Errorf("missing definition for path (not in ECS)")} } - err := compareFieldDefinitionWithECS(currentPath, found, definition) - if err != nil { - return err + errs := compareFieldDefinitionWithECS(currentPath, found, definition) + if len(errs) > 0 { + return errs } return compareMultiFieldsInECSSchema(currentPath, found, definition) } -func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) error { +func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) multierror.Error { + var errs multierror.Error actualType := mappingParameter("type", actual) if ecs.Type != actualType { // exceptions related to numbers if !isNumberTypeField(ecs.Type, actualType) { - return fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type) + errs = append(errs, fmt.Errorf("actual mapping type (%s) does not match with ECS definition type: %s", actualType, ecs.Type)) } else { logger.Debugf("Allowed number fields with different types (ECS %s - actual %s)", string(ecs.Type), string(actualType)) } @@ -332,86 +333,76 @@ func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, act // Compare other parameters metricType := mappingParameter("time_series_metric", actual) if ecs.MetricType != metricType { - return fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType) - } - return nil -} - -func compareMultiFieldsInECSSchema(currentPath string, found *FieldDefinition, definition map[string]any) error { - var ecsMultiFields []FieldDefinition - // Filter multi-fields added by appendECSMappingMultifields - for _, f := range found.MultiFields { - // TODO: Should we use another way to filter these fields? - if f.External != externalFieldAppendedTag { - ecsMultiFields = append(ecsMultiFields, f) - } + errs = append(errs, fmt.Errorf("actual mapping \"time_series_metric\" (%s) does not match with ECS definition value: %s", metricType, ecs.MetricType)) } - // if there are no multifieds in the actual definition, nothing to compare - if !isMultiFields(definition) { - return nil + if len(errs) > 0 { + return errs } - // if len(ecsMultiFields) == 0 { - // return nil - // } + return nil +} - if len(ecsMultiFields) == 0 { - return fmt.Errorf("not matched definitions for multifields for %q: no multi-fields found in ECS", currentPath) - } +func compareMultiFieldsInECSSchema(currentPath string, found *FieldDefinition, definition map[string]any) multierror.Error { + var errs multierror.Error - actualMultiFields, err := getMappingDefinitionsField("fields", definition) - if err != nil { - return fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err) + actualMultiFields := map[string]any{} + if isMultiFields(definition) { + var err error + actualMultiFields, err = getMappingDefinitionsField("fields", definition) + if err != nil { + return multierror.Error{fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err)} + } } - missingMultiFields := []string{} + // Check whehter or not all the multi-fields in the actual mapping are in ECS for key, value := range actualMultiFields { multiFieldPath := currentMappingPath(currentPath, key) def, ok := value.(map[string]any) if !ok { - return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) + errs = append(errs, fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath)) + return errs } - found := false - for _, ecsMultiField := range ecsMultiFields { + foundMultiField := false + for _, ecsMultiField := range found.MultiFields { if ecsMultiField.Name != key { continue } err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) if err == nil { - found = true + foundMultiField = true break } } - if !found { - missingMultiFields = append(missingMultiFields, key) + if !foundMultiField { + errs = append(errs, fmt.Errorf("missing definition for multi-field in ECS: %q", key)) } } - if len(missingMultiFields) > 0 { - return fmt.Errorf("missing definition for multi-fields in ECS: %q", strings.Join(missingMultiFields, ", ")) - } - return nil - - // for _, ecsMultiField := range ecsMultiFields { + // // Check whehter or not all the multi-fields in ECS are present in the actual mapping + // for _, ecsMultiField := range found.MultiFields { // multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) // actualMultiField, ok := actualMultiFields[ecsMultiField.Name] // if !ok { - // return fmt.Errorf("missing multi_field definition for %q", multiFieldPath) + // errs = append(errs, fmt.Errorf("missing ECS multi_field definition for %q", multiFieldPath)) + // continue // } // def, ok := actualMultiField.(map[string]any) // if !ok { - // return fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath) + // return multierror.Error{fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath)} // } // err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) // if err != nil { - // return err + // errs = append(errs, fmt.Errorf("missing ECS definition for multi-field in actual mapping: %q", ecsMultiField.Name)) // } // } + if len(errs) > 0 { + return errs.Unique() + } - // return nil + return nil } // flattenMappings returns all the mapping definitions found at "path" flattened including @@ -649,9 +640,11 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil } // validate whether or not all fields under this key are defined in ECS - err = v.validateMappingInECSSchema(fieldPath, def) - if err != nil { - errs = append(errs, fmt.Errorf("field %q is undefined: %w", fieldPath, err)) + ecsErrs := v.validateMappingInECSSchema(fieldPath, def) + if len(ecsErrs) > 0 { + for _, e := range ecsErrs { + errs = append(errs, fmt.Errorf("field %q is undefined: %w", fieldPath, e)) + } } } return errs.Unique() @@ -671,18 +664,18 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi continue } - logger.Debugf("Found dynamic template matched: %s", template.name) + // logger.Debugf("Found dynamic template matched: %s", template.name) // logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) // Check that all parameters match (setting no dynamic templates to avoid recursion) errs := v.validateObjectMappingAndParameters(template.mapping, definition, currentPath, []map[string]any{}, true) if errs != nil { // Look for another dynamic template - logger.Debugf("invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) + // logger.Debugf("invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) continue } - logger.Debugf("Valid template for path %q: %q", currentPath, template.name) + // logger.Debugf("Valid template for path %q: %q", currentPath, template.name) return nil } diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index d002aa9c5..df4cc8cdc 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -162,7 +162,7 @@ func TestComparingMappings(t *testing.T) { expectedErrors: []string{ `field "metrics" is undefined: actual mapping type (long) does not match with ECS definition type: keyword`, `field "user" is undefined: missing definition for path (not in ECS)`, - `field "time" is undefined: missing definition for multi-fields in ECS: "other"`, + `field "time" is undefined: missing definition for multi-field in ECS: "other"`, }, }, { From ce557c9a341b0ca983ba17af076757770e199dff Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 18:48:38 +0100 Subject: [PATCH 22/51] Ignored validation of multi-fields with ECS --- internal/fields/mappings.go | 70 +++----------------------------- internal/fields/mappings_test.go | 3 +- 2 files changed, 8 insertions(+), 65 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 2f0d4aa47..46e1ed360 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -315,7 +315,12 @@ func (v *MappingValidator) validateMappingInECSSchema(currentPath string, defini return errs } - return compareMultiFieldsInECSSchema(currentPath, found, definition) + // Currently, validation of multi-fields with ECS is skipped + // Any multi-field found in the actual mapping must come from a definition from the preview or + // a dynamic template, therefore there is a definition for it. + // Example of this validation can be found at: + // https://github.com/elastic/elastic-package/pull/2285/commits/51656120 + return nil } func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, actual map[string]any) multierror.Error { @@ -342,69 +347,6 @@ func compareFieldDefinitionWithECS(currentPath string, ecs *FieldDefinition, act return nil } -func compareMultiFieldsInECSSchema(currentPath string, found *FieldDefinition, definition map[string]any) multierror.Error { - var errs multierror.Error - - actualMultiFields := map[string]any{} - if isMultiFields(definition) { - var err error - actualMultiFields, err = getMappingDefinitionsField("fields", definition) - if err != nil { - return multierror.Error{fmt.Errorf("invalid multi_field mapping %q: %w", currentPath, err)} - } - } - - // Check whehter or not all the multi-fields in the actual mapping are in ECS - for key, value := range actualMultiFields { - multiFieldPath := currentMappingPath(currentPath, key) - - def, ok := value.(map[string]any) - if !ok { - errs = append(errs, fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath)) - return errs - } - foundMultiField := false - for _, ecsMultiField := range found.MultiFields { - if ecsMultiField.Name != key { - continue - } - err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) - if err == nil { - foundMultiField = true - break - } - } - if !foundMultiField { - errs = append(errs, fmt.Errorf("missing definition for multi-field in ECS: %q", key)) - } - } - - // // Check whehter or not all the multi-fields in ECS are present in the actual mapping - // for _, ecsMultiField := range found.MultiFields { - // multiFieldPath := currentMappingPath(currentPath, ecsMultiField.Name) - // actualMultiField, ok := actualMultiFields[ecsMultiField.Name] - // if !ok { - // errs = append(errs, fmt.Errorf("missing ECS multi_field definition for %q", multiFieldPath)) - // continue - // } - - // def, ok := actualMultiField.(map[string]any) - // if !ok { - // return multierror.Error{fmt.Errorf("invalid multi_field mapping type: %q", multiFieldPath)} - // } - - // err := compareFieldDefinitionWithECS(multiFieldPath, &ecsMultiField, def) - // if err != nil { - // errs = append(errs, fmt.Errorf("missing ECS definition for multi-field in actual mapping: %q", ecsMultiField.Name)) - // } - // } - if len(errs) > 0 { - return errs.Unique() - } - - return nil -} - // flattenMappings returns all the mapping definitions found at "path" flattened including // specific entries for multi fields too. func flattenMappings(path string, definition map[string]any) (map[string]any, error) { diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index df4cc8cdc..09fccdd33 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -108,6 +108,7 @@ func TestComparingMappings(t *testing.T) { "text": map[string]any{ "type": "match_only_text", }, + // there should be a dynamic template in order to exist this multi-field "other": map[string]any{ "type": "match_only_text", }, @@ -162,7 +163,7 @@ func TestComparingMappings(t *testing.T) { expectedErrors: []string{ `field "metrics" is undefined: actual mapping type (long) does not match with ECS definition type: keyword`, `field "user" is undefined: missing definition for path (not in ECS)`, - `field "time" is undefined: missing definition for multi-field in ECS: "other"`, + // `field "time" is undefined: missing definition for multi-field in ECS: "other"`, // multi-fields are skipped }, }, { From ebda8599dab64514208de7aab38ef45fd0870314 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 8 Jan 2025 18:51:13 +0100 Subject: [PATCH 23/51] Fix multi-field test --- internal/fields/mappings_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index 09fccdd33..c2ff7fb6c 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -83,7 +83,7 @@ func TestComparingMappings(t *testing.T) { expectedErrors: []string{}, }, { - title: "validate field with ECS", + title: "validate fields with ECS", preview: map[string]any{ "foo": map[string]any{ "type": "keyword", @@ -308,9 +308,13 @@ func TestComparingMappings(t *testing.T) { }, }, actual: map[string]any{ - // Should this fail since it has no multi-fields as in the preview? "time": map[string]any{ "type": "keyword", + "fields": map[string]any{ + "text": map[string]any{ + "type": "match_only_text", + }, + }, }, "foo": map[string]any{ "type": "keyword", From 019ffd48549dbffac8cff942792048375dd76d13 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 9 Jan 2025 12:12:36 +0100 Subject: [PATCH 24/51] Rephrase errors --- internal/fields/mappings.go | 16 ++++++++-------- internal/fields/mappings_test.go | 24 ++++++++++++------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 46e1ed360..4cd6be2fa 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -303,11 +303,11 @@ func isNumberTypeField(previewType, actualType string) bool { func (v *MappingValidator) validateMappingInECSSchema(currentPath string, definition map[string]any) multierror.Error { found := FindElementDefinition(currentPath, v.Schema) if found == nil { - return multierror.Error{fmt.Errorf("missing definition for path")} + return multierror.Error{fmt.Errorf("field definition not found")} } if found.External != "ecs" { - return multierror.Error{fmt.Errorf("missing definition for path (not in ECS)")} + return multierror.Error{fmt.Errorf("field definition not found")} } errs := compareFieldDefinitionWithECS(currentPath, found, definition) @@ -391,11 +391,11 @@ func flattenMappings(path string, definition map[string]any) (map[string]any, er func getMappingDefinitionsField(field string, definition map[string]any) (map[string]any, error) { anyValue, ok := definition[field] if !ok { - return nil, fmt.Errorf("not found field: %q", field) + return nil, fmt.Errorf("field not found: %q", field) } object, ok := anyValue.(map[string]any) if !ok { - return nil, fmt.Errorf("unexpected type found for %q: %T ", field, anyValue) + return nil, fmt.Errorf("unexpected type found for field %q: %T ", field, anyValue) } return object, nil } @@ -420,7 +420,7 @@ func validateConstantKeywordField(path string, preview, actual map[string]any) ( if previewValue != actualValue { // This should also be detected by the failure storage (if available) // or no documents being ingested - return isConstantKeyword, fmt.Errorf("constant_keyword value in preview %q does not match the actual mapping value %q for path: %q", previewValue, actualValue, path) + return isConstantKeyword, fmt.Errorf("invalid value in field %q: constant_keyword value in preview %q does not match the actual mapping value %q", path, previewValue, actualValue) } return isConstantKeyword, nil } @@ -442,7 +442,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit } if isObjectFullyDynamic(actual) { - logger.Debugf("Dynamic object found but no fields ingested under path: \"%s.*\"", path) + logger.Warnf("Dynamic object found but no fields ingested under path: \"%s.*\"", path) return nil } @@ -477,7 +477,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit containsMultifield := isMultiFields(actual) if containsMultifield { if !isMultiFields(preview) { - errs = append(errs, fmt.Errorf("not found multi_fields in preview mappings for path: %q", path)) + errs = append(errs, fmt.Errorf("field %q is undefined: not found multi_fields definitions in preview mapping", path)) return errs.Unique() } previewFields, err := getMappingDefinitionsField("fields", preview) @@ -527,7 +527,7 @@ func (v *MappingValidator) validateObjectProperties(path string, couldBeParamete errs = append(errs, ecsErrors...) continue } - // Parameter not defined + // Field or Parameter not defined errs = append(errs, fmt.Errorf("field %q is undefined", currentPath)) continue } diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index c2ff7fb6c..ccf2aa46f 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -162,7 +162,7 @@ func TestComparingMappings(t *testing.T) { }, expectedErrors: []string{ `field "metrics" is undefined: actual mapping type (long) does not match with ECS definition type: keyword`, - `field "user" is undefined: missing definition for path (not in ECS)`, + `field "user" is undefined: field definition not found`, // `field "time" is undefined: missing definition for multi-field in ECS: "other"`, // multi-fields are skipped }, }, @@ -216,7 +216,7 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `field "foo" is undefined: missing definition for path`, + `field "foo" is undefined: field definition not found`, }, }, { @@ -235,7 +235,7 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `constant_keyword value in preview "example" does not match the actual mapping value "bar" for path: "foo"`, + `invalid value in field "foo": constant_keyword value in preview "example" does not match the actual mapping value "bar"`, }, }, { @@ -350,8 +350,8 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `field "foo.text" is undefined: missing definition for path`, - `not found multi_fields in preview mappings for path: "bar.type"`, + `field "foo.text" is undefined: field definition not found`, + `field "bar.type" is undefined: not found multi_fields definitions in preview mapping`, }, }, { @@ -373,7 +373,7 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `not found multi_fields in preview mappings for path: "foo"`, + `field "foo" is undefined: not found multi_fields definitions in preview mapping`, }, }, { @@ -404,8 +404,8 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `field "file.path" is undefined: missing definition for path`, - `field "bar" is undefined: missing definition for path`, + `field "file.path" is undefined: field definition not found`, + `field "bar" is undefined: field definition not found`, }, }, { @@ -433,7 +433,7 @@ func TestComparingMappings(t *testing.T) { schema: []FieldDefinition{}, expectedErrors: []string{ // TODO: there is an exception in the logic to not raise this error - // `field "_tmp" is undefined: missing definition for path`, + // `field "_tmp" is undefined: field definition not found`, }, }, { @@ -528,7 +528,7 @@ func TestComparingMappings(t *testing.T) { }, schema: []FieldDefinition{}, expectedErrors: []string{ - `field "sql.metrics.example" is undefined: missing definition for path`, + `field "sql.metrics.example" is undefined: field definition not found`, }, }, { @@ -583,7 +583,7 @@ func TestComparingMappings(t *testing.T) { }, exceptionFields: []string{"access.field"}, expectedErrors: []string{ - `field "error.field" is undefined: missing definition for path`, + `field "error.field" is undefined: field definition not found`, // should status.field return error ? or should it be ignored? `field "status.field" is undefined: actual mapping type (keyword) does not match with ECS definition type: array`, }, @@ -860,7 +860,7 @@ func TestComparingMappings(t *testing.T) { expectedErrors: []string{ // Should it be considered this error in "foa" "missing time_series_metric bar"? // "fob" is failing because it does not have the expected value for the "time_series_metric" field - `field "fob" is undefined: missing definition for path`, + `field "fob" is undefined: field definition not found`, }, }, } From 8f36c189acbd6ff0328b2154aec9168f9652f640 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 9 Jan 2025 13:08:14 +0100 Subject: [PATCH 25/51] Validate fully dynamic objects (preview) with dynamic templates --- internal/fields/dynamic_template.go | 16 ++++---- internal/fields/mappings.go | 7 +++- internal/fields/mappings_test.go | 64 +++++++++++------------------ 3 files changed, 36 insertions(+), 51 deletions(-) diff --git a/internal/fields/dynamic_template.go b/internal/fields/dynamic_template.go index 4af966e6d..46166fe00 100644 --- a/internal/fields/dynamic_template.go +++ b/internal/fields/dynamic_template.go @@ -30,15 +30,13 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) name := fieldNameFromPath(currentPath) if !slices.Contains(d.match, name) { // If there is no an exact match, it is compared with patterns/wildcards - - // logger.Warnf(">>>> no contained %s: %s", d.match, name) matches, err := stringMatchesPatterns(d.match, name, fullRegex) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + return false, fmt.Errorf("failed to parse dynamic template %q: %w", d.name, err) } if !matches { - // logger.Debugf(">> Issue match: not matches") + // logger.Debugf(">> Issue matchi %q: not matches", d.name) return false, nil } } @@ -52,11 +50,11 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) matches, err := stringMatchesPatterns(d.unmatch, name, fullRegex) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + return false, fmt.Errorf("failed to parse dynamic template %q: %w", d.name, err) } if matches { - // logger.Debugf(">> Issue unmatch: matches") + // logger.Debugf(">> Issue unmatchi %q: matches", d.name) return false, nil } } @@ -68,7 +66,7 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) } if !matches { - logger.Debugf(">> Issue path_match: not matches (currentPath %s)", currentPath) + // logger.Debugf(">> Issue path_match %q: not matches (currentPath %s)", d.name, currentPath) return false, nil } } @@ -76,10 +74,10 @@ func (d *dynamicTemplate) Matches(currentPath string, definition map[string]any) if len(d.unpathMatch) > 0 { matches, err := stringMatchesPatterns(d.unpathMatch, currentPath, fullRegex) if err != nil { - return false, fmt.Errorf("failed to parse dynamic template %s: %w", d.name, err) + return false, fmt.Errorf("failed to parse dynamic template %q: %w", d.name, err) } if matches { - // logger.Debugf(">> Issue unpath_match: matches") + // logger.Debugf(">> Issue unpath_matchi %q: matches", d.name) return false, nil } } diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 4cd6be2fa..e8ed4222e 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -450,8 +450,11 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit // there could be "sub-fields" with name "properties" too if couldBeParametersDefinition && isObject(actual) { if isObjectFullyDynamic(preview) { - // TODO: Skip for now, it should be required to compare with dynamic templates - logger.Debugf("Pending to validate with the dynamic templates defined the path: %q", path) + dynamicErrors := v.validateMappingsNotInPreview(path, actual, dynamicTemplates) + errs = append(errs, dynamicErrors...) + if len(errs) > 0 { + return errs.Unique() + } return nil } else if !isObject(preview) { errs = append(errs, fmt.Errorf("not found properties in preview mappings for path: %q", path)) diff --git a/internal/fields/mappings_test.go b/internal/fields/mappings_test.go index ccf2aa46f..9a1fb3bd8 100644 --- a/internal/fields/mappings_test.go +++ b/internal/fields/mappings_test.go @@ -437,7 +437,7 @@ func TestComparingMappings(t *testing.T) { }, }, { - title: "skip dynamic objects", // TODO: should this be checked using dynamic templates? + title: "validate fully dynamic objects in preview", preview: map[string]any{ "@timestamp": map[string]any{ "type": "keyword", @@ -451,6 +451,13 @@ func TestComparingMappings(t *testing.T) { "type": "object", "dynamic": "true", }, + "string": map[string]any{ + "type": "object", + "dynamic": "true", + }, + "foo": map[string]any{ + "type": "keyword", + }, }, }, }, @@ -473,61 +480,38 @@ func TestComparingMappings(t *testing.T) { }, }, }, - }, - }, - }, - }, - }, - schema: []FieldDefinition{}, - expectedErrors: []string{}, - }, - { - title: "compare all objects even dynamic true", // TODO: should this be checked using dynamic templates? - preview: map[string]any{ - "@timestamp": map[string]any{ - "type": "keyword", - }, - "sql": map[string]any{ - "properties": map[string]any{ - "metrics": map[string]any{ - "properties": map[string]any{ - "dynamic": "true", - "numeric": map[string]any{ - "type": "object", - "dynamic": "true", - }, - }, - }, - }, - }, - }, - actual: map[string]any{ - "@timestamp": map[string]any{ - "type": "keyword", - }, - "sql": map[string]any{ - "properties": map[string]any{ - "metrics": map[string]any{ - "properties": map[string]any{ - "dynamic": "true", - "numeric": map[string]any{ + "string": map[string]any{ "dynamic": "true", "properties": map[string]any{ "innodb_data_fsyncs": map[string]any{ - "type": "long", + "type": "keyword", }, }, }, "example": map[string]any{ "type": "keyword", }, + "foo": map[string]any{ + "type": "keyword", + }, }, }, }, }, }, + dynamicTemplates: []map[string]any{ + { + "sql.metrics.string.*": map[string]any{ + "path_match": "sql.metrics.string.*", + "mapping": map[string]any{ + "type": "keyword", + }, + }, + }, + }, schema: []FieldDefinition{}, expectedErrors: []string{ + `field "sql.metrics.numeric.innodb_data_fsyncs" is undefined: field definition not found`, `field "sql.metrics.example" is undefined: field definition not found`, }, }, From d96ffbf5be4fa80cf5e357ce212c0086a59f292d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 9 Jan 2025 13:41:28 +0100 Subject: [PATCH 26/51] Rephrase debug message --- internal/fields/mappings.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index e8ed4222e..3273cd7c1 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -523,7 +523,7 @@ func (v *MappingValidator) validateObjectProperties(path string, couldBeParamete if childField, ok := value.(map[string]any); ok { if isEmptyObject(childField) { // TODO: Should this be raised as an error instead? - logger.Debugf("field %q is an empty object and it does not exist in the preview", currentPath) + logger.Debugf("field %q skipped: empty object wihtout definition in the preview", currentPath) continue } ecsErrors := v.validateMappingsNotInPreview(currentPath, childField, dynamicTemplates) From dbca4feac02cb94243a6bbb0eb8bbd8889bafc01 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 9 Jan 2025 16:19:16 +0100 Subject: [PATCH 27/51] Add logging - to be removed --- internal/fields/mappings.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 3273cd7c1..adde04b83 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -609,22 +609,22 @@ func (v *MappingValidator) matchingWithDynamicTemplates(currentPath string, defi continue } - // logger.Debugf("Found dynamic template matched: %s", template.name) + logger.Warnf("> Found dynamic template matched: %s", template.name) - // logger.Debugf("> Check parameters (%q): %q", templateName, currentPath) + logger.Debugf("> Check parameters (%q): %q", template.name, currentPath) // Check that all parameters match (setting no dynamic templates to avoid recursion) errs := v.validateObjectMappingAndParameters(template.mapping, definition, currentPath, []map[string]any{}, true) if errs != nil { // Look for another dynamic template - // logger.Debugf("invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) + logger.Warnf(">> Invalid dynamic template for %q:\n%s", currentPath, errs.Unique()) continue } - // logger.Debugf("Valid template for path %q: %q", currentPath, template.name) + logger.Warnf("> Valid template for path %q: %q", currentPath, template.name) return nil } - // logger.Debugf(">> No template matching for path: %q", currentPath) + logger.Warnf(">> No template matching for path: %q", currentPath) return fmt.Errorf("no template matching for path: %q", currentPath) } From aa5674fc715a585274ceb282279724a6e2669435 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 15 Jan 2025 19:42:43 +0100 Subject: [PATCH 28/51] Validate transforms with mappings getting docs --- internal/packages/packages.go | 3 + internal/testrunner/runners/system/tester.go | 182 +++++++++++++------ 2 files changed, 128 insertions(+), 57 deletions(-) diff --git a/internal/packages/packages.go b/internal/packages/packages.go index cd628e81f..7ab1791e2 100644 --- a/internal/packages/packages.go +++ b/internal/packages/packages.go @@ -199,6 +199,9 @@ type TransformDefinition struct { Source struct { Index []string `config:"index" yaml:"index"` } `config:"source" yaml:"source"` + Dest struct { + Index string `config:"index" yaml:"index"` + } `config:"dest" yaml:"dest"` Meta struct { FleetTransformVersion string `config:"fleet_transform_version" yaml:"fleet_transform_version"` } `config:"_meta" yaml:"_meta"` diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 7f82c13fd..d8a418df4 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -740,7 +740,7 @@ func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) r.esAPI.Search.WithIgnoreUnavailable(true), ) if err != nil { - return nil, fmt.Errorf("could not search data stream: %w", err) + return nil, fmt.Errorf("could not search index or data stream: %w", err) } defer resp.Body.Close() @@ -750,7 +750,7 @@ func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) return &hits{}, nil } if resp.IsError() { - return nil, fmt.Errorf("failed to search docs for data stream %s: %s", dataStream, resp.String()) + return nil, fmt.Errorf("failed to search docs for index or data stream %s: %s", dataStream, resp.String()) } var results struct { @@ -791,10 +791,10 @@ func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) numHits := results.Hits.Total.Value if results.Error != nil { - logger.Debugf("found %d hits in %s data stream: %s: %s Status=%d", + logger.Debugf("found %d hits in %s index or data stream: %s: %s Status=%d", numHits, dataStream, results.Error.Type, results.Error.Reason, results.Status) } else { - logger.Debugf("found %d hits in %s data stream", numHits, dataStream) + logger.Debugf("found %d hits in %s index or data stream", numHits, dataStream) } var hits hits @@ -1243,52 +1243,10 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return &scenario, nil } - // Use custom timeout if the service can't collect data immediately. - waitForDataTimeout := waitForDataDefaultTimeout - if config.WaitForDataTimeout > 0 { - waitForDataTimeout = config.WaitForDataTimeout - } - - // (TODO in future) Optionally exercise service to generate load. - logger.Debugf("checking for expected data in data stream (%s)...", waitForDataTimeout) - var hits *hits - oldHits := 0 - passed, waitErr := wait.UntilTrue(ctx, func(ctx context.Context) (bool, error) { - var err error - hits, err = r.getDocs(ctx, scenario.dataStream) - if err != nil { - return false, err - } - - if r.checkFailureStore { - failureStore, err := r.getFailureStoreDocs(ctx, scenario.dataStream) - if err != nil { - return false, fmt.Errorf("failed to check failure store: %w", err) - } - if n := len(failureStore); n > 0 { - // Interrupt loop earlier if there are failures in the document store. - logger.Debugf("Found %d hits in the failure store for %s", len(failureStore), scenario.dataStream) - return true, nil - } - } - - if config.Assert.HitCount > 0 { - if hits.size() < config.Assert.HitCount { - return false, nil - } - - ret := hits.size() == oldHits - if !ret { - oldHits = hits.size() - time.Sleep(4 * time.Second) - } - - return ret, nil - } - - return hits.size() > 0, nil - }, 1*time.Second, waitForDataTimeout) + hits, waitErr := r.waitForDocs(ctx, config, scenario.dataStream, 1*time.Second, true) + // before checking "waitErr" error , it is necessary to check if the service has finished with error + // to report as a test case failed if service != nil && config.Service != "" && !config.IgnoreServiceError { exited, code, err := service.ExitCode(ctx, config.Service) if err != nil && !errors.Is(err, servicedeployer.ErrNotSupported) { @@ -1303,10 +1261,6 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil, waitErr } - if !passed { - return nil, testrunner.ErrTestCaseFailed{Reason: fmt.Sprintf("could not find hits in %s data stream", scenario.dataStream)} - } - logger.Debugf("Check whether or not synthetic source mode is enabled (data stream %s)...", scenario.dataStream) scenario.syntheticEnabled, err = isSyntheticSourceModeEnabled(ctx, r.esAPI, scenario.dataStream) if err != nil { @@ -1987,10 +1941,62 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan if err != nil { return fmt.Errorf("creating fields validator for data stream failed (path: %s): %w", transformRootPath, err) } - if errs := validateFields(transformDocs, fieldsValidator); len(errs) > 0 { - return testrunner.ErrTestCaseFailed{ - Reason: fmt.Sprintf("errors found in documents of preview for transform %s for data stream %s", transformId, dataStream), - Details: errs.Error(), + if r.fieldValidationMethod == allMethods || r.fieldValidationMethod == fieldsMethod { + if errs := validateFields(transformDocs, fieldsValidator); len(errs) > 0 { + return testrunner.ErrTestCaseFailed{ + Reason: fmt.Sprintf("errors found in documents of preview for transform %s for data stream %s", transformId, dataStream), + Details: errs.Error(), + } + } + } + + if r.fieldValidationMethod == allMethods || r.fieldValidationMethod == mappingsMethod { + destIndexTransform := transform.Definition.Dest.Index + // IDs format is: "-.-template" + // For instance: "logs-ti_anomali.latest_intelligence-template" + indexTemplateTransform := fmt.Sprintf("%s-%s.%s-template", + ds.Inputs[0].Streams[0].DataStream.Type, + pkgManifest.Name, + transform.Name, + ) + + // TODO: use the docs obtained here for the fields validator too + // In order to compare the mappings, it is required to wait until the documents has been + // ingested in the given transform index + // It looks like that not all documents ingested previously in the main data stream are going + // to be ingested in the destination index of the transform. That's the reason to disable + // the asserts + hits, waitErr := r.waitForDocs(ctx, config, destIndexTransform, 5*time.Second, false) + if waitErr != nil { + return waitErr + } + logger.Debugf("Check whether or not synthetic source mode is enabled (transform index %s)...", destIndexTransform) + syntheticEnabled, err = isSyntheticSourceModeEnabled(ctx, r.esAPI, destIndexTransform) + if err != nil { + return fmt.Errorf("failed to check if synthetic source mode is enabled for transform index %s: %w", destIndexTransform, err) + } + logger.Debugf("Index %s has synthetic source mode enabled: %t", destIndexTransform, syntheticEnabled) + + transformDocs := hits.getDocs(syntheticEnabled) + + logger.Warn("Validate mappings found in transform (technical preview)") + exceptionFields := listExceptionFields(transformDocs, fieldsValidator) + + mappingsValidator, err := fields.CreateValidatorForMappings(r.esClient, + fields.WithMappingValidatorFallbackSchema(fieldsValidator.Schema), + fields.WithMappingValidatorIndexTemplate(indexTemplateTransform), + fields.WithMappingValidatorDataStream(destIndexTransform), + fields.WithMappingValidatorExceptionFields(exceptionFields), + ) + if err != nil { + return fmt.Errorf("creating mappings validator for the transform index failed (index: %s): %w", destIndexTransform, err) + } + + if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { + return testrunner.ErrTestCaseFailed{ + Reason: fmt.Sprintf("one or more errors found in mappings in the transform (index %s)", indexTemplateTransform, destIndexTransform), + Details: errs.Error(), + } } } } @@ -2401,3 +2407,65 @@ func (r *tester) generateCoverageReport(pkgName string) (testrunner.CoverageRepo return testrunner.GenerateBaseFileCoverageReportGlob(pkgName, patterns, r.coverageType, true) } + +func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream string, period time.Duration, checkAsserts bool) (*hits, error) { + // Use custom timeout if the service can't collect data immediately. + waitForDataTimeout := waitForDataDefaultTimeout + if config.WaitForDataTimeout > 0 { + waitForDataTimeout = config.WaitForDataTimeout + } + + // (TODO in future) Optionally exercise service to generate load. + logger.Debugf("checking for expected data in data stream or index (%s)...", waitForDataTimeout) + var hits *hits + oldHits := 0 + passed, waitErr := wait.UntilTrue(ctx, func(ctx context.Context) (bool, error) { + var err error + hits, err = r.getDocs(ctx, dataStream) + if err != nil { + return false, err + } + + if r.checkFailureStore { + failureStore, err := r.getFailureStoreDocs(ctx, dataStream) + if err != nil { + return false, fmt.Errorf("failed to check failure store: %w", err) + } + if n := len(failureStore); n > 0 { + // Interrupt loop earlier if there are failures in the document store. + logger.Debugf("Found %d hits in the failure store for %s", len(failureStore), dataStream) + return true, nil + } + } + + if !checkAsserts { + return hits.size() > 0, nil + } + + if config.Assert.HitCount > 0 { + if hits.size() < config.Assert.HitCount { + return false, nil + } + + ret := hits.size() == oldHits + if !ret { + oldHits = hits.size() + time.Sleep(4 * time.Second) + } + + return ret, nil + } + + return hits.size() > 0, nil + }, period, waitForDataTimeout) + + if waitErr != nil { + return nil, waitErr + } + + if !passed { + return nil, testrunner.ErrTestCaseFailed{Reason: fmt.Sprintf("could not find hits in data stream or index: %s", dataStream)} + } + + return hits, nil +} From 4bf6104144672c5d8160380cfdcaffc59dabc0f9 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 16 Jan 2025 11:39:51 +0100 Subject: [PATCH 29/51] Take into account deleted docs for transforms --- internal/testrunner/runners/system/tester.go | 104 ++++++++++++++----- 1 file changed, 77 insertions(+), 27 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index d8a418df4..995c84fbf 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -729,6 +729,39 @@ func (h hits) size() int { return len(h.Source) } +func (r *tester) getDeletedDocs(ctx context.Context, dataStream string) (int, error) { + resp, err := r.esAPI.Indices.Stats( + r.esAPI.Indices.Stats.WithContext(ctx), + r.esAPI.Indices.Stats.WithIndex(dataStream), + r.esAPI.Indices.Stats.WithMetric("docs"), + ) + if err != nil { + return 0, fmt.Errorf("could not get stats for index: %w", err) + } + defer resp.Body.Close() + + if resp.IsError() { + return 0, fmt.Errorf("failed to get stats for index %s: %s", dataStream, resp.String()) + } + + var results struct { + All struct { + Total struct { + Docs struct { + Count int `json:"count"` + Deleted int `json:"deleted"` + } `json:"docs"` + } `json:"total"` + } `json:"_all"` + } + + if err := json.NewDecoder(resp.Body).Decode(&results); err != nil { + return 0, fmt.Errorf("could not decode index stats results response: %w", err) + } + + return results.All.Total.Docs.Deleted, nil +} + func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) { resp, err := r.esAPI.Search( r.esAPI.Search.WithContext(ctx), @@ -1243,7 +1276,23 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return &scenario, nil } - hits, waitErr := r.waitForDocs(ctx, config, scenario.dataStream, 1*time.Second, true) + hits, waitErr := r.waitForDocs(ctx, config, scenario.dataStream, 1*time.Second, func(hits *hits, oldHits int) (bool, error) { + if config.Assert.HitCount > 0 { + if hits.size() < config.Assert.HitCount { + return false, nil + } + + logger.Debugf("Hits size %d oldHits %d", hits.size(), oldHits) + ret := hits.size() == oldHits + if !ret { + time.Sleep(4 * time.Second) + } + + return ret, nil + } + + return hits.size() > 0, nil + }) // before checking "waitErr" error , it is necessary to check if the service has finished with error // to report as a test case failed @@ -1952,7 +2001,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan if r.fieldValidationMethod == allMethods || r.fieldValidationMethod == mappingsMethod { destIndexTransform := transform.Definition.Dest.Index - // IDs format is: "-.-template" + // Index Template format is: "-.-template" // For instance: "logs-ti_anomali.latest_intelligence-template" indexTemplateTransform := fmt.Sprintf("%s-%s.%s-template", ds.Inputs[0].Streams[0].DataStream.Type, @@ -1966,19 +2015,27 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan // It looks like that not all documents ingested previously in the main data stream are going // to be ingested in the destination index of the transform. That's the reason to disable // the asserts - hits, waitErr := r.waitForDocs(ctx, config, destIndexTransform, 5*time.Second, false) + _, waitErr := r.waitForDocs(ctx, config, destIndexTransform, 5*time.Second, func(hits *hits, _ int) (bool, error) { + deleted, err := r.getDeletedDocs(ctx, destIndexTransform) + if err != nil { + return false, err + } + foundDeleted := deleted > 0 + foundHits := hits.size() > 0 + if foundDeleted { + logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) + } + if foundHits { + logger.Debugf("Found %s hits in %s index", destIndexTransform) + } + return foundDeleted || foundHits, nil + }) if waitErr != nil { return waitErr } - logger.Debugf("Check whether or not synthetic source mode is enabled (transform index %s)...", destIndexTransform) - syntheticEnabled, err = isSyntheticSourceModeEnabled(ctx, r.esAPI, destIndexTransform) - if err != nil { - return fmt.Errorf("failed to check if synthetic source mode is enabled for transform index %s: %w", destIndexTransform, err) - } - logger.Debugf("Index %s has synthetic source mode enabled: %t", destIndexTransform, syntheticEnabled) - - transformDocs := hits.getDocs(syntheticEnabled) + // As it could happen that there are no hits found , just deleted docs + // Here it is used the docs found in the preview API response logger.Warn("Validate mappings found in transform (technical preview)") exceptionFields := listExceptionFields(transformDocs, fieldsValidator) @@ -1994,7 +2051,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { return testrunner.ErrTestCaseFailed{ - Reason: fmt.Sprintf("one or more errors found in mappings in the transform (index %s)", indexTemplateTransform, destIndexTransform), + Reason: fmt.Sprintf("one or more errors found in mappings in the transform (index %s)", destIndexTransform), Details: errs.Error(), } } @@ -2408,7 +2465,7 @@ func (r *tester) generateCoverageReport(pkgName string) (testrunner.CoverageRepo return testrunner.GenerateBaseFileCoverageReportGlob(pkgName, patterns, r.coverageType, true) } -func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream string, period time.Duration, checkAsserts bool) (*hits, error) { +func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream string, period time.Duration, stopCondition func(*hits, int) (bool, error)) (*hits, error) { // Use custom timeout if the service can't collect data immediately. waitForDataTimeout := waitForDataDefaultTimeout if config.WaitForDataTimeout > 0 { @@ -2437,22 +2494,15 @@ func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream return true, nil } } + defer func() { + oldHits = hits.size() + }() - if !checkAsserts { - return hits.size() > 0, nil - } - - if config.Assert.HitCount > 0 { - if hits.size() < config.Assert.HitCount { - return false, nil - } - - ret := hits.size() == oldHits - if !ret { - oldHits = hits.size() - time.Sleep(4 * time.Second) + if stopCondition != nil { + ret, err := stopCondition(hits, oldHits) + if err != nil { + return false, err } - return ret, nil } From 9acd95c17edac6a27b12e4b8252b82bc0a0faef8 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 16 Jan 2025 16:15:54 +0100 Subject: [PATCH 30/51] Update logs --- internal/testrunner/runners/system/tester.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 995c84fbf..ddbdddb60 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1282,7 +1282,6 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return false, nil } - logger.Debugf("Hits size %d oldHits %d", hits.size(), oldHits) ret := hits.size() == oldHits if !ret { time.Sleep(4 * time.Second) @@ -2026,7 +2025,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) } if foundHits { - logger.Debugf("Found %s hits in %s index", destIndexTransform) + logger.Debugf("Found %s hits in %s index", hits.size(), destIndexTransform) } return foundDeleted || foundHits, nil }) From a5b635d17f9d8dabcc2c9de22defe282f326e604 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 16 Jan 2025 16:23:19 +0100 Subject: [PATCH 31/51] Force to update timestamp in ti_anomali test package --- .../_dev/test/system/test-webhook-http-config.yml | 6 ++++++ .../_dev/test/system/test-webhook-https-config.yml | 6 ++++++ .../_dev/test/system/test-webhook-http-config.yml | 6 ++++++ .../_dev/test/system/test-webhook-https-config.yml | 6 ++++++ .../threatstream/elasticsearch/ingest_pipeline/default.yml | 7 +++++++ 5 files changed, 31 insertions(+) diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml index ec6e03df0..22179ed92 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml @@ -8,5 +8,11 @@ data_stream: listen_port: 9080 ioc_expiration_duration: 5d preserve_original_event: true + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci assert: hit_count: 100 diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml index 3ab3a7c14..6a266df08 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml @@ -8,6 +8,12 @@ data_stream: listen_port: 7443 preserve_original_event: true ioc_expiration_duration: 5d + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci ssl: |- certificate: | -----BEGIN CERTIFICATE----- diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml index ec6e03df0..22179ed92 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml @@ -8,5 +8,11 @@ data_stream: listen_port: 9080 ioc_expiration_duration: 5d preserve_original_event: true + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci assert: hit_count: 100 diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml index 3ab3a7c14..6a266df08 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml @@ -8,6 +8,12 @@ data_stream: listen_port: 7443 preserve_original_event: true ioc_expiration_duration: 5d + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci ssl: |- certificate: | -----BEGIN CERTIFICATE----- diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index baad31861..12a2f2387 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -239,6 +239,13 @@ processors: field: "@timestamp" copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null + + # Processor added just to update timestamp for testing purposes + - set: + tag: set_timestamp + field: "@timestamp" + value: "{{{_ingest.timestamp}}}" + if: 'ctx.tags != null && ctx.tags.contains("force_update_elastic_package_ci")' # # Map IP geolocation fields. # From 6fc1c2eaecfa9ebe4b7dc527df0f22762bd5c99e Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 16 Jan 2025 17:58:53 +0100 Subject: [PATCH 32/51] Add options struct for waitForDocs --- internal/testrunner/runners/system/tester.go | 88 ++++++++++++-------- 1 file changed, 55 insertions(+), 33 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index ddbdddb60..328ebfe80 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1276,22 +1276,27 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return &scenario, nil } - hits, waitErr := r.waitForDocs(ctx, config, scenario.dataStream, 1*time.Second, func(hits *hits, oldHits int) (bool, error) { - if config.Assert.HitCount > 0 { - if hits.size() < config.Assert.HitCount { - return false, nil - } + waitOpts := waitForDocsOptions{ + timeout: config.WaitForDataTimeout, + stopCondition: func(hits *hits, oldHits int) (bool, error) { + if config.Assert.HitCount > 0 { + if hits.size() < config.Assert.HitCount { + return false, nil + } + + ret := hits.size() == oldHits + if !ret { + time.Sleep(4 * time.Second) + } - ret := hits.size() == oldHits - if !ret { - time.Sleep(4 * time.Second) + return ret, nil } - return ret, nil - } + return hits.size() > 0, nil + }, + } - return hits.size() > 0, nil - }) + hits, waitErr := r.waitForDocs(ctx, scenario.dataStream, waitOpts) // before checking "waitErr" error , it is necessary to check if the service has finished with error // to report as a test case failed @@ -2014,21 +2019,27 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan // It looks like that not all documents ingested previously in the main data stream are going // to be ingested in the destination index of the transform. That's the reason to disable // the asserts - _, waitErr := r.waitForDocs(ctx, config, destIndexTransform, 5*time.Second, func(hits *hits, _ int) (bool, error) { - deleted, err := r.getDeletedDocs(ctx, destIndexTransform) - if err != nil { - return false, err - } - foundDeleted := deleted > 0 - foundHits := hits.size() > 0 - if foundDeleted { - logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) - } - if foundHits { - logger.Debugf("Found %s hits in %s index", hits.size(), destIndexTransform) - } - return foundDeleted || foundHits, nil - }) + waitOpts := waitForDocsOptions{ + period: 5 * time.Second, + timeout: 10 * time.Minute, + stopCondition: func(hits *hits, oldHits int) (bool, error) { + deleted, err := r.getDeletedDocs(ctx, destIndexTransform) + if err != nil { + return false, err + } + foundDeleted := deleted > 0 + foundHits := hits.size() > 0 + if foundDeleted { + logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) + } + if foundHits { + logger.Debugf("Found %d hits in %s index", hits.size(), destIndexTransform) + } + return foundDeleted || foundHits, nil + }, + } + + _, waitErr := r.waitForDocs(ctx, destIndexTransform, waitOpts) if waitErr != nil { return waitErr } @@ -2464,11 +2475,22 @@ func (r *tester) generateCoverageReport(pkgName string) (testrunner.CoverageRepo return testrunner.GenerateBaseFileCoverageReportGlob(pkgName, patterns, r.coverageType, true) } -func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream string, period time.Duration, stopCondition func(*hits, int) (bool, error)) (*hits, error) { +type waitForDocsOptions struct { + period time.Duration + timeout time.Duration + stopCondition func(*hits, int) (bool, error) +} + +func (r *tester) waitForDocs(ctx context.Context, dataStream string, opts waitForDocsOptions) (*hits, error) { // Use custom timeout if the service can't collect data immediately. waitForDataTimeout := waitForDataDefaultTimeout - if config.WaitForDataTimeout > 0 { - waitForDataTimeout = config.WaitForDataTimeout + if opts.timeout > 0 { + waitForDataTimeout = opts.timeout + } + + periodTime := 1 * time.Second + if opts.period > 0 { + periodTime = opts.period } // (TODO in future) Optionally exercise service to generate load. @@ -2497,8 +2519,8 @@ func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream oldHits = hits.size() }() - if stopCondition != nil { - ret, err := stopCondition(hits, oldHits) + if opts.stopCondition != nil { + ret, err := opts.stopCondition(hits, oldHits) if err != nil { return false, err } @@ -2506,7 +2528,7 @@ func (r *tester) waitForDocs(ctx context.Context, config *testConfig, dataStream } return hits.size() > 0, nil - }, period, waitForDataTimeout) + }, periodTime, waitForDataTimeout) if waitErr != nil { return nil, waitErr From bcca5895c125636c84626471501b252b882ece2b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 16 Jan 2025 18:31:05 +0100 Subject: [PATCH 33/51] Update missing system test config with tags --- .../_dev/test/system/test-integrator-http-config.yml | 6 ++++++ .../_dev/test/system/test-integrator-http-config.yml | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml index 212232f37..e43bdd56c 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml @@ -9,5 +9,11 @@ data_stream: secret: TheSecret preserve_original_event: true ioc_expiration_duration: 5d + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci assert: hit_count: 1 diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml index 212232f37..e43bdd56c 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml @@ -9,5 +9,11 @@ data_stream: secret: TheSecret preserve_original_event: true ioc_expiration_duration: 5d + # Default tags plus the tag to force updating timestamp fields in order to be processed + # documents by the transform + tags: + - forwarded + - anomali-threatstream + - force_update_elastic_package_ci assert: hit_count: 1 From 0a241a982f0a6eed8f24c8d25b09486b423c9d55 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 17 Jan 2025 10:33:06 +0100 Subject: [PATCH 34/51] Update ingest pipeline for ti_anomali --- .../threatstream/elasticsearch/ingest_pipeline/default.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index baad31861..12a2f2387 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -239,6 +239,13 @@ processors: field: "@timestamp" copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null + + # Processor added just to update timestamp for testing purposes + - set: + tag: set_timestamp + field: "@timestamp" + value: "{{{_ingest.timestamp}}}" + if: 'ctx.tags != null && ctx.tags.contains("force_update_elastic_package_ci")' # # Map IP geolocation fields. # From 9a0b677a79f1349a229f4db47149a6aa9ec6b23f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 17 Jan 2025 23:22:23 +0100 Subject: [PATCH 35/51] Sanitize values read for dynamic templates --- internal/elasticsearch/client.go | 12 ++++++++++++ internal/fields/mappings.go | 10 ++++++++++ 2 files changed, 22 insertions(+) diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index 748edf0eb..010d0db8c 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -330,6 +330,12 @@ func (c *Client) SimulateIndexTemplate(ctx context.Context, indexTemplateName st return nil, nil, fmt.Errorf("error unmarshaling mappings: %w", err) } + // TODO + // // In case there are no dynamic templates, set an empty array + // if string(preview.Template.Mappings.DynamicTemplates) == "" { + // preview.Template.Mappings.DynamicTemplates = []byte("[]") + // } + return preview.Template.Mappings.DynamicTemplates, preview.Template.Mappings.Properties, nil } @@ -372,5 +378,11 @@ func (c *Client) DataStreamMappings(ctx context.Context, dataStreamName string) mappingsDefinition = v.Mappings } + // TODO + // // In case there are no dynamic templates, set an empty array + // if string(mappingsDefinition.DynamicTemplates) == "" { + // mappingsDefinition.DynamicTemplates = []byte("[]") + // } + return mappingsDefinition.DynamicTemplates, mappingsDefinition.Properties, nil } diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index adde04b83..8c20ebf35 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -101,6 +101,11 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs } + // In case there are no dynamic templates, set an empty array + if string(actualDynamicTemplates) == "" { + actualDynamicTemplates = []byte("[]") + } + logger.Debugf("Simulate Index Template (%s)", v.indexTemplateName) previewDynamicTemplates, previewMappings, err := v.esClient.SimulateIndexTemplate(ctx, v.indexTemplateName) if err != nil { @@ -108,6 +113,11 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs } + // In case there are no dynamic templates, set an empty array + if string(previewDynamicTemplates) == "" { + previewDynamicTemplates = []byte("[]") + } + // Code from comment posted in https://github.com/google/go-cmp/issues/224 transformJSON := cmp.FilterValues(func(x, y []byte) bool { return json.Valid(x) && json.Valid(y) From ebdd98c12ad07b8f6d087ef92ad9e09c85b32573 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 Jan 2025 16:38:30 +0100 Subject: [PATCH 36/51] Move check for empty strings to elasticsearch functions --- internal/elasticsearch/client.go | 28 ++++++++++++++++++---------- internal/fields/mappings.go | 10 ---------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/internal/elasticsearch/client.go b/internal/elasticsearch/client.go index 010d0db8c..1f6156363 100644 --- a/internal/elasticsearch/client.go +++ b/internal/elasticsearch/client.go @@ -330,11 +330,15 @@ func (c *Client) SimulateIndexTemplate(ctx context.Context, indexTemplateName st return nil, nil, fmt.Errorf("error unmarshaling mappings: %w", err) } - // TODO - // // In case there are no dynamic templates, set an empty array - // if string(preview.Template.Mappings.DynamicTemplates) == "" { - // preview.Template.Mappings.DynamicTemplates = []byte("[]") - // } + // In case there are no dynamic templates, set an empty array + if string(preview.Template.Mappings.DynamicTemplates) == "" { + preview.Template.Mappings.DynamicTemplates = []byte("[]") + } + + // In case there are no mappings defined, set an empty map + if string(preview.Template.Mappings.Properties) == "" { + preview.Template.Mappings.Properties = []byte("{}") + } return preview.Template.Mappings.DynamicTemplates, preview.Template.Mappings.Properties, nil } @@ -378,11 +382,15 @@ func (c *Client) DataStreamMappings(ctx context.Context, dataStreamName string) mappingsDefinition = v.Mappings } - // TODO - // // In case there are no dynamic templates, set an empty array - // if string(mappingsDefinition.DynamicTemplates) == "" { - // mappingsDefinition.DynamicTemplates = []byte("[]") - // } + // In case there are no dynamic templates, set an empty array + if string(mappingsDefinition.DynamicTemplates) == "" { + mappingsDefinition.DynamicTemplates = []byte("[]") + } + + // In case there are no mappings defined, set an empty map + if string(mappingsDefinition.Properties) == "" { + mappingsDefinition.Properties = []byte("{}") + } return mappingsDefinition.DynamicTemplates, mappingsDefinition.Properties, nil } diff --git a/internal/fields/mappings.go b/internal/fields/mappings.go index 8c20ebf35..adde04b83 100644 --- a/internal/fields/mappings.go +++ b/internal/fields/mappings.go @@ -101,11 +101,6 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs } - // In case there are no dynamic templates, set an empty array - if string(actualDynamicTemplates) == "" { - actualDynamicTemplates = []byte("[]") - } - logger.Debugf("Simulate Index Template (%s)", v.indexTemplateName) previewDynamicTemplates, previewMappings, err := v.esClient.SimulateIndexTemplate(ctx, v.indexTemplateName) if err != nil { @@ -113,11 +108,6 @@ func (v *MappingValidator) ValidateIndexMappings(ctx context.Context) multierror return errs } - // In case there are no dynamic templates, set an empty array - if string(previewDynamicTemplates) == "" { - previewDynamicTemplates = []byte("[]") - } - // Code from comment posted in https://github.com/google/go-cmp/issues/224 transformJSON := cmp.FilterValues(func(x, y []byte) bool { return json.Valid(x) && json.Valid(y) From b9917fda686b80a573e9c71405a9948ac8c39cab Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 Jan 2025 10:41:00 +0100 Subject: [PATCH 37/51] Remove unused field --- internal/testrunner/runners/system/tester.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 328ebfe80..57a2f8ddd 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -748,7 +748,6 @@ func (r *tester) getDeletedDocs(ctx context.Context, dataStream string) (int, er All struct { Total struct { Docs struct { - Count int `json:"count"` Deleted int `json:"deleted"` } `json:"docs"` } `json:"total"` From 9251224bbbe945fcd91de8d9f3b49cabffe6b8a7 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 Jan 2025 15:02:24 +0100 Subject: [PATCH 38/51] Update stop condition in transforms --- internal/testrunner/runners/system/tester.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 57a2f8ddd..07301576b 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1983,6 +1983,8 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan return fmt.Errorf("no documents found in preview for transform %q", transformId) } + logger.Debugf("Found %d documents in preview for transform %q", len(transformDocs), transformId) + transformRootPath := filepath.Dir(transform.Path) fieldsValidator, err := fields.CreateValidatorForDirectory(transformRootPath, fields.WithSpecVersion(pkgManifest.SpecVersion), @@ -2019,7 +2021,8 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan // to be ingested in the destination index of the transform. That's the reason to disable // the asserts waitOpts := waitForDocsOptions{ - period: 5 * time.Second, + period: 5 * time.Second, + // Add specific timeout to ensure transform processes the documents (depends on "delay" field?) timeout: 10 * time.Minute, stopCondition: func(hits *hits, oldHits int) (bool, error) { deleted, err := r.getDeletedDocs(ctx, destIndexTransform) @@ -2027,7 +2030,8 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan return false, err } foundDeleted := deleted > 0 - foundHits := hits.size() > 0 + // Wait at least for the number of documents found after running the transform preview + foundHits := hits.size() > len(transformDocs) if foundDeleted { logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) } From c50b94d687713d845ecdf144f10849b131af0e1c Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 Jan 2025 19:20:18 +0100 Subject: [PATCH 39/51] Update sync.time.field setting in transform - ti_anomali --- .../_dev/test/system/test-integrator-http-config.yml | 6 ------ .../_dev/test/system/test-webhook-http-config.yml | 6 ------ .../_dev/test/system/test-webhook-https-config.yml | 6 ------ .../threatstream/elasticsearch/ingest_pipeline/default.yml | 6 ------ .../elasticsearch/transform/latest_ioc/transform.yml | 3 ++- .../_dev/test/system/test-integrator-http-config.yml | 6 ------ .../_dev/test/system/test-webhook-http-config.yml | 6 ------ .../_dev/test/system/test-webhook-https-config.yml | 6 ------ .../threatstream/elasticsearch/ingest_pipeline/default.yml | 6 ------ .../elasticsearch/transform/latest_ioc/transform.yml | 3 ++- 10 files changed, 4 insertions(+), 50 deletions(-) diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml index e43bdd56c..212232f37 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml @@ -9,11 +9,5 @@ data_stream: secret: TheSecret preserve_original_event: true ioc_expiration_duration: 5d - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci assert: hit_count: 1 diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml index 22179ed92..ec6e03df0 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml @@ -8,11 +8,5 @@ data_stream: listen_port: 9080 ioc_expiration_duration: 5d preserve_original_event: true - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci assert: hit_count: 100 diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml index 6a266df08..3ab3a7c14 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml @@ -8,12 +8,6 @@ data_stream: listen_port: 7443 preserve_original_event: true ioc_expiration_duration: 5d - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci ssl: |- certificate: | -----BEGIN CERTIFICATE----- diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index 12a2f2387..6af9dccee 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -240,12 +240,6 @@ processors: copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null - # Processor added just to update timestamp for testing purposes - - set: - tag: set_timestamp - field: "@timestamp" - value: "{{{_ingest.timestamp}}}" - if: 'ctx.tags != null && ctx.tags.contains("force_update_elastic_package_ci")' # # Map IP geolocation fields. # diff --git a/test/packages/parallel/ti_anomali/elasticsearch/transform/latest_ioc/transform.yml b/test/packages/parallel/ti_anomali/elasticsearch/transform/latest_ioc/transform.yml index a0a971684..a30f56e4c 100644 --- a/test/packages/parallel/ti_anomali/elasticsearch/transform/latest_ioc/transform.yml +++ b/test/packages/parallel/ti_anomali/elasticsearch/transform/latest_ioc/transform.yml @@ -22,7 +22,8 @@ description: Latest Anomali IoC data frequency: 30s sync: time: - field: "@timestamp" + # ensure that the field used to synchronize uses the current timestamp so it is processed + field: "event.ingested" # Updated to 120s because of refresh delay in Serverless. With default 60s, sometimes transform wouldn't process all documents. delay: 120s retention_policy: diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml index e43bdd56c..212232f37 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-integrator-http-config.yml @@ -9,11 +9,5 @@ data_stream: secret: TheSecret preserve_original_event: true ioc_expiration_duration: 5d - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci assert: hit_count: 1 diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml index 22179ed92..ec6e03df0 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-http-config.yml @@ -8,11 +8,5 @@ data_stream: listen_port: 9080 ioc_expiration_duration: 5d preserve_original_event: true - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci assert: hit_count: 100 diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml index 6a266df08..3ab3a7c14 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/_dev/test/system/test-webhook-https-config.yml @@ -8,12 +8,6 @@ data_stream: listen_port: 7443 preserve_original_event: true ioc_expiration_duration: 5d - # Default tags plus the tag to force updating timestamp fields in order to be processed - # documents by the transform - tags: - - forwarded - - anomali-threatstream - - force_update_elastic_package_ci ssl: |- certificate: | -----BEGIN CERTIFICATE----- diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index 12a2f2387..6af9dccee 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -240,12 +240,6 @@ processors: copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null - # Processor added just to update timestamp for testing purposes - - set: - tag: set_timestamp - field: "@timestamp" - value: "{{{_ingest.timestamp}}}" - if: 'ctx.tags != null && ctx.tags.contains("force_update_elastic_package_ci")' # # Map IP geolocation fields. # diff --git a/test/packages/parallel/ti_anomali_logsdb/elasticsearch/transform/latest_ioc/transform.yml b/test/packages/parallel/ti_anomali_logsdb/elasticsearch/transform/latest_ioc/transform.yml index 65f3add2d..526383edb 100644 --- a/test/packages/parallel/ti_anomali_logsdb/elasticsearch/transform/latest_ioc/transform.yml +++ b/test/packages/parallel/ti_anomali_logsdb/elasticsearch/transform/latest_ioc/transform.yml @@ -22,7 +22,8 @@ description: Latest Anomali IoC data frequency: 30s sync: time: - field: "@timestamp" + # ensure that the field used to synchronize uses the current timestamp so it is processed + field: "event.ingested" # Updated to 120s because of refresh delay in Serverless. With default 60s, sometimes transform wouldn't process all documents. delay: 120s retention_policy: From ffcad915f58ad0782f5d5abd6b94d9f9f8fe32cf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 Jan 2025 19:22:13 +0100 Subject: [PATCH 40/51] Deleted undesired empty lines --- .../threatstream/elasticsearch/ingest_pipeline/default.yml | 1 - .../threatstream/elasticsearch/ingest_pipeline/default.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index 6af9dccee..baad31861 100644 --- a/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -239,7 +239,6 @@ processors: field: "@timestamp" copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null - # # Map IP geolocation fields. # diff --git a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml index 6af9dccee..baad31861 100644 --- a/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml +++ b/test/packages/parallel/ti_anomali_logsdb/data_stream/threatstream/elasticsearch/ingest_pipeline/default.yml @@ -239,7 +239,6 @@ processors: field: "@timestamp" copy_from: _temp_.timestamp if: ctx._temp_?.timestamp != null - # # Map IP geolocation fields. # From 460b42027261ce8a6dfc0890732aa1df085e792a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 22 Jan 2025 12:12:42 +0100 Subject: [PATCH 41/51] Fix stop conditions to wait for docs in transforms --- internal/testrunner/runners/system/tester.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 07301576b..ade0ea0ad 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2031,7 +2031,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan } foundDeleted := deleted > 0 // Wait at least for the number of documents found after running the transform preview - foundHits := hits.size() > len(transformDocs) + foundHits := hits.size() >= len(transformDocs) if foundDeleted { logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) } From 1cbe763ad3ffe02b75ba013535e606d9372bedea Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 Jan 2025 11:20:19 +0100 Subject: [PATCH 42/51] Add transform name to test case --- internal/testrunner/runners/system/tester.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index ade0ea0ad..93cb61e01 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2059,12 +2059,12 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan fields.WithMappingValidatorExceptionFields(exceptionFields), ) if err != nil { - return fmt.Errorf("creating mappings validator for the transform index failed (index: %s): %w", destIndexTransform, err) + return fmt.Errorf("creating mappings validator for the %q transform index failed (index: %s): %w", transform.Name, destIndexTransform, err) } if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { return testrunner.ErrTestCaseFailed{ - Reason: fmt.Sprintf("one or more errors found in mappings in the transform (index %s)", destIndexTransform), + Reason: fmt.Sprintf("one or more errors found in mappings in the transform %q (index %s)", transform.Name, destIndexTransform), Details: errs.Error(), } } From 930bbbefed5bd7726f37690ad7b412b3296cdef2 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 Jan 2025 13:44:38 +0100 Subject: [PATCH 43/51] Extract function to validate transforms based on mappings --- internal/testrunner/runners/system/tester.go | 105 ++++++++++--------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 93cb61e01..67c20a09a 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2014,63 +2014,70 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan transform.Name, ) - // TODO: use the docs obtained here for the fields validator too - // In order to compare the mappings, it is required to wait until the documents has been - // ingested in the given transform index - // It looks like that not all documents ingested previously in the main data stream are going - // to be ingested in the destination index of the transform. That's the reason to disable - // the asserts - waitOpts := waitForDocsOptions{ - period: 5 * time.Second, - // Add specific timeout to ensure transform processes the documents (depends on "delay" field?) - timeout: 10 * time.Minute, - stopCondition: func(hits *hits, oldHits int) (bool, error) { - deleted, err := r.getDeletedDocs(ctx, destIndexTransform) - if err != nil { - return false, err - } - foundDeleted := deleted > 0 - // Wait at least for the number of documents found after running the transform preview - foundHits := hits.size() >= len(transformDocs) - if foundDeleted { - logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) - } - if foundHits { - logger.Debugf("Found %d hits in %s index", hits.size(), destIndexTransform) - } - return foundDeleted || foundHits, nil - }, - } - - _, waitErr := r.waitForDocs(ctx, destIndexTransform, waitOpts) - if waitErr != nil { - return waitErr + err := r.validateTransformsWithMappings(ctx, transform.Name, destIndexTransform, indexTemplateTransform, transformDocs, fieldsValidator) + if err != nil { + return err } + } + } - // As it could happen that there are no hits found , just deleted docs - // Here it is used the docs found in the preview API response - logger.Warn("Validate mappings found in transform (technical preview)") - exceptionFields := listExceptionFields(transformDocs, fieldsValidator) + return nil +} - mappingsValidator, err := fields.CreateValidatorForMappings(r.esClient, - fields.WithMappingValidatorFallbackSchema(fieldsValidator.Schema), - fields.WithMappingValidatorIndexTemplate(indexTemplateTransform), - fields.WithMappingValidatorDataStream(destIndexTransform), - fields.WithMappingValidatorExceptionFields(exceptionFields), - ) +func (r *tester) validateTransformsWithMappings(ctx context.Context, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { + // TODO: use the docs obtained here for the fields validator too + // In order to compare the mappings, it is required to wait until the documents has been + // ingested in the given transform index + // It looks like that not all documents ingested previously in the main data stream are going + // to be ingested in the destination index of the transform. That's the reason to disable + // the asserts + waitOpts := waitForDocsOptions{ + period: 5 * time.Second, + // Add specific timeout to ensure transform processes the documents (depends on "delay" field?) + timeout: 10 * time.Minute, + stopCondition: func(hits *hits, oldHits int) (bool, error) { + deleted, err := r.getDeletedDocs(ctx, destIndexTransform) if err != nil { - return fmt.Errorf("creating mappings validator for the %q transform index failed (index: %s): %w", transform.Name, destIndexTransform, err) + return false, err } - - if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { - return testrunner.ErrTestCaseFailed{ - Reason: fmt.Sprintf("one or more errors found in mappings in the transform %q (index %s)", transform.Name, destIndexTransform), - Details: errs.Error(), - } + foundDeleted := deleted > 0 + // Wait at least for the number of documents found after running the transform preview + foundHits := hits.size() >= len(transformDocs) + if foundDeleted { + logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) } - } + if foundHits { + logger.Debugf("Found %d hits in %s index", hits.size(), destIndexTransform) + } + return foundDeleted || foundHits, nil + }, } + if _, err := r.waitForDocs(ctx, destIndexTransform, waitOpts); err != nil { + return err + } + + // As it could happen that there are no hits found , just deleted docs + // Here it is used the docs found in the preview API response + logger.Warn("Validate mappings found in transform (technical preview)") + exceptionFields := listExceptionFields(transformDocs, fieldsValidator) + + mappingsValidator, err := fields.CreateValidatorForMappings(r.esClient, + fields.WithMappingValidatorFallbackSchema(fieldsValidator.Schema), + fields.WithMappingValidatorIndexTemplate(indexTemplateTransform), + fields.WithMappingValidatorDataStream(destIndexTransform), + fields.WithMappingValidatorExceptionFields(exceptionFields), + ) + if err != nil { + return fmt.Errorf("creating mappings validator for the %q transform index failed (index: %s): %w", transformName, destIndexTransform, err) + } + + if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { + return testrunner.ErrTestCaseFailed{ + Reason: fmt.Sprintf("one or more errors found in mappings in the transform %q (index %s)", transformName, destIndexTransform), + Details: errs.Error(), + } + } return nil } From e2eee9330562660b8c99ad53a93391a891dfd655 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 Jan 2025 16:14:38 +0100 Subject: [PATCH 44/51] Remove TODO --- internal/testrunner/runners/system/tester.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 67c20a09a..1fd73f486 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2025,7 +2025,6 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan } func (r *tester) validateTransformsWithMappings(ctx context.Context, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { - // TODO: use the docs obtained here for the fields validator too // In order to compare the mappings, it is required to wait until the documents has been // ingested in the given transform index // It looks like that not all documents ingested previously in the main data stream are going From a178a68349ce9f1b3d36a54cb1ca5124178c7417 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 Jan 2025 18:16:37 +0100 Subject: [PATCH 45/51] Use processed docs as stop condition in transforms --- internal/testrunner/runners/system/tester.go | 54 +++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 1fd73f486..e975daf73 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2014,7 +2014,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan transform.Name, ) - err := r.validateTransformsWithMappings(ctx, transform.Name, destIndexTransform, indexTemplateTransform, transformDocs, fieldsValidator) + err := r.validateTransformsWithMappings(ctx, transformId, transform.Name, destIndexTransform, indexTemplateTransform, transformDocs, fieldsValidator) if err != nil { return err } @@ -2024,7 +2024,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan return nil } -func (r *tester) validateTransformsWithMappings(ctx context.Context, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { +func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { // In order to compare the mappings, it is required to wait until the documents has been // ingested in the given transform index // It looks like that not all documents ingested previously in the main data stream are going @@ -2034,21 +2034,15 @@ func (r *tester) validateTransformsWithMappings(ctx context.Context, transformNa period: 5 * time.Second, // Add specific timeout to ensure transform processes the documents (depends on "delay" field?) timeout: 10 * time.Minute, - stopCondition: func(hits *hits, oldHits int) (bool, error) { - deleted, err := r.getDeletedDocs(ctx, destIndexTransform) + stopCondition: func(_ *hits, _ int) (bool, error) { + processed, err := r.processedTransformDocs(ctx, transformId) if err != nil { return false, err } - foundDeleted := deleted > 0 - // Wait at least for the number of documents found after running the transform preview - foundHits := hits.size() >= len(transformDocs) - if foundDeleted { - logger.Debugf("Found %d deleted docs in %s index", deleted, destIndexTransform) - } - if foundHits { - logger.Debugf("Found %d hits in %s index", hits.size(), destIndexTransform) + if processed > 0 { + logger.Debugf("Documents processed by transform %q: %d", transformId, processed) } - return foundDeleted || foundHits, nil + return processed >= len(transformDocs), nil }, } @@ -2141,6 +2135,40 @@ func (r *tester) previewTransform(ctx context.Context, transformId string) ([]co return preview.Documents, nil } +func (r *tester) processedTransformDocs(ctx context.Context, transformId string) (int, error) { + resp, err := r.esAPI.TransformGetTransformStats(transformId, + r.esAPI.TransformGetTransformStats.WithContext(ctx), + ) + if err != nil { + return 0, err + } + defer resp.Body.Close() + + if resp.IsError() { + return 0, fmt.Errorf("failed to get stats for transform %q: %s", transformId, resp.String()) + } + + var stats struct { + Transforms []struct { + Stats struct { + DocumentsProcessed int `json:"documents_processed"` + } `json:"stats"` + } `json:"transforms"` + } + err = json.NewDecoder(resp.Body).Decode(&stats) + if err != nil { + return 0, fmt.Errorf("failed to decode response: %w", err) + } + + if len(stats.Transforms) == 0 { + logger.Debugf("No stats information for transform %q", transformId) + return 0, nil + } + + // As it is requested one stransform, there must be just one element in the Transform array + return stats.Transforms[0].Stats.DocumentsProcessed, nil +} + func filterAgents(allAgents []kibana.Agent, svcInfo servicedeployer.ServiceInfo) []kibana.Agent { if svcInfo.Agent.Host.NamePrefix != "" { logger.Debugf("filter agents using criteria: NamePrefix=%s", svcInfo.Agent.Host.NamePrefix) From aff903b769164e998f0c9d380de98a1397a5991f Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 Jan 2025 18:23:36 +0100 Subject: [PATCH 46/51] Remove unused function --- internal/testrunner/runners/system/tester.go | 32 -------------------- 1 file changed, 32 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index e975daf73..9550afaa3 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -729,38 +729,6 @@ func (h hits) size() int { return len(h.Source) } -func (r *tester) getDeletedDocs(ctx context.Context, dataStream string) (int, error) { - resp, err := r.esAPI.Indices.Stats( - r.esAPI.Indices.Stats.WithContext(ctx), - r.esAPI.Indices.Stats.WithIndex(dataStream), - r.esAPI.Indices.Stats.WithMetric("docs"), - ) - if err != nil { - return 0, fmt.Errorf("could not get stats for index: %w", err) - } - defer resp.Body.Close() - - if resp.IsError() { - return 0, fmt.Errorf("failed to get stats for index %s: %s", dataStream, resp.String()) - } - - var results struct { - All struct { - Total struct { - Docs struct { - Deleted int `json:"deleted"` - } `json:"docs"` - } `json:"total"` - } `json:"_all"` - } - - if err := json.NewDecoder(resp.Body).Decode(&results); err != nil { - return 0, fmt.Errorf("could not decode index stats results response: %w", err) - } - - return results.All.Total.Docs.Deleted, nil -} - func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) { resp, err := r.esAPI.Search( r.esAPI.Search.WithContext(ctx), From c385cb24631350d6f9d8668fa66cd0a024bb5203 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 24 Jan 2025 15:37:06 +0100 Subject: [PATCH 47/51] Add exception for transforms in stacks < 8.14.0 Do not validate transform fields based on mappings for those packages where the kibana version is lower than 8.14.0. There is an known issue related to how mappings are generated. Related issue: https://github.com/elastic/kibana/issues/175331 --- internal/testrunner/runners/system/tester.go | 26 ++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 9550afaa3..cbf6ac1fa 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -136,6 +136,8 @@ var ( enableIndependentAgentsEnv = environment.WithElasticPackagePrefix("TEST_ENABLE_INDEPENDENT_AGENT") dumpScenarioDocsEnv = environment.WithElasticPackagePrefix("TEST_DUMP_SCENARIO_DOCS") fieldValidationTestMethodEnv = environment.WithElasticPackagePrefix("FIELD_VALIDATION_TEST_METHOD") + + semver_8_14_0 = semver.MustParse("8.14.0") ) type fieldValidationMethod int @@ -1915,6 +1917,25 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan if err != nil { return fmt.Errorf("loading transforms for package failed (root: %s): %w", r.packageRootPath, err) } + + // Before stack version 8.14.0, there are some issues generating the corresponding + // mappings for the fields defined in the transforms + // Forced to not use mappings to validate transforms before 8.14.0 + // Related issue: https://github.com/elastic/kibana/issues/175331 + stackVersion, err := semver.NewVersion(r.stackVersion.Number) + if err != nil { + return fmt.Errorf("invalid stack version: %w", err) + } + + validationMethod := r.fieldValidationMethod + if stackVersion.LessThan(semver_8_14_0) { + logger.Debugf("Forced to validate transforms based on fields, not available for stack versions < 8.14.0") + validationMethod = fieldsMethod + } + + fieldsValidationBased := validationMethod == allMethods || validationMethod == fieldsMethod + mappingsValidationBased := validationMethod == allMethods || validationMethod == mappingsMethod + for _, transform := range transforms { hasSource, err := transform.HasSource(dataStream) if err != nil { @@ -1963,7 +1984,8 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan if err != nil { return fmt.Errorf("creating fields validator for data stream failed (path: %s): %w", transformRootPath, err) } - if r.fieldValidationMethod == allMethods || r.fieldValidationMethod == fieldsMethod { + + if fieldsValidationBased { if errs := validateFields(transformDocs, fieldsValidator); len(errs) > 0 { return testrunner.ErrTestCaseFailed{ Reason: fmt.Sprintf("errors found in documents of preview for transform %s for data stream %s", transformId, dataStream), @@ -1972,7 +1994,7 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan } } - if r.fieldValidationMethod == allMethods || r.fieldValidationMethod == mappingsMethod { + if mappingsValidationBased { destIndexTransform := transform.Definition.Dest.Index // Index Template format is: "-.-template" // For instance: "logs-ti_anomali.latest_intelligence-template" From a65efc3156a098b1efe7aea5e28662082e3e2ac9 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 27 Jan 2025 13:28:36 +0100 Subject: [PATCH 48/51] Check for errors in docs when validation based on mappings is used --- internal/testrunner/runners/system/tester.go | 34 +++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 10d2ec0fa..d35d34c07 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1641,7 +1641,11 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re return result.WithErrorf("creating mappings validator for data stream failed (data stream: %s): %w", scenario.dataStream, err) } - if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { + // any error found in ingested docs + errs := ensureNoErrorsInDocs(scenario.docs) + + if mappingsErrs := validateMappings(ctx, mappingsValidator); len(mappingsErrs) > 0 { + errs = append(errs, mappingsErrs...) return result.WithError(testrunner.ErrTestCaseFailed{ Reason: fmt.Sprintf("one or more errors found in mappings in %s index template", scenario.indexTemplateName), Details: errs.Error(), @@ -2179,7 +2183,10 @@ func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId return fmt.Errorf("creating mappings validator for the %q transform index failed (index: %s): %w", transformName, destIndexTransform, err) } - if errs := validateMappings(ctx, mappingsValidator); len(errs) > 0 { + errs := ensureNoErrorsInDocs(transformDocs) + + if mappingErrs := validateMappings(ctx, mappingsValidator); len(mappingErrs) > 0 { + errs = append(errs, mappingErrs...) return testrunner.ErrTestCaseFailed{ Reason: fmt.Sprintf("one or more errors found in mappings in the transform %q (index %s)", transformName, destIndexTransform), Details: errs.Error(), @@ -2364,13 +2371,9 @@ func validateFailureStore(failureStore []failureStoreDocument) error { } func validateFields(docs []common.MapStr, fieldsValidator *fields.Validator) multierror.Error { - var multiErr multierror.Error - for _, doc := range docs { - if message, err := doc.GetValue("error.message"); err != common.ErrKeyNotFound { - multiErr = append(multiErr, fmt.Errorf("found error.message in event: %v", message)) - continue - } + multiErr := ensureNoErrorsInDocs(docs) + for _, doc := range docs { errs := fieldsValidator.ValidateDocumentMap(doc) if errs != nil { multiErr = append(multiErr, errs...) @@ -2687,3 +2690,18 @@ func (r *tester) waitForDocs(ctx context.Context, dataStream string, opts waitFo return hits, nil } + +func ensureNoErrorsInDocs(docs []common.MapStr) multierror.Error { + var multiErr multierror.Error + for _, doc := range docs { + if message, err := doc.GetValue("error.message"); err != common.ErrKeyNotFound { + multiErr = append(multiErr, fmt.Errorf("found error.message in event: %v", message)) + } + } + + if len(multiErr) > 0 { + return multiErr.Unique() + } + + return nil +} From 1e5a7ae6202eb3b5bd03ba4774ba1bd834b61897 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 27 Jan 2025 17:15:17 +0100 Subject: [PATCH 49/51] Rename function --- internal/testrunner/runners/system/tester.go | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index d35d34c07..0f6fda344 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1678,7 +1678,7 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re } // Check transforms if present - if err := r.checkTransforms(ctx, config, r.pkgManifest, scenario.kibanaDataStream, scenario.dataStream, scenario.syntheticEnabled); err != nil { + if err := r.checkTransforms(ctx, stackVersion, config, r.pkgManifest, scenario.kibanaDataStream, scenario.dataStream, scenario.syntheticEnabled); err != nil { results, _ := result.WithError(err) return results, nil } @@ -2040,7 +2040,7 @@ func selectPolicyTemplateByName(policies []packages.PolicyTemplate, name string) return packages.PolicyTemplate{}, fmt.Errorf("policy template %q not found", name) } -func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgManifest *packages.PackageManifest, ds kibana.PackageDataStream, dataStream string, syntheticEnabled bool) error { +func (r *tester) checkTransforms(ctx context.Context, stackVersion *semver.Version, config *testConfig, pkgManifest *packages.PackageManifest, ds kibana.PackageDataStream, dataStream string, syntheticEnabled bool) error { transforms, err := packages.ReadTransformsFromPackageRoot(r.packageRootPath) if err != nil { return fmt.Errorf("loading transforms for package failed (root: %s): %w", r.packageRootPath, err) @@ -2050,11 +2050,6 @@ func (r *tester) checkTransforms(ctx context.Context, config *testConfig, pkgMan // mappings for the fields defined in the transforms // Forced to not use mappings to validate transforms before 8.14.0 // Related issue: https://github.com/elastic/kibana/issues/175331 - stackVersion, err := semver.NewVersion(r.stackVersion.Number) - if err != nil { - return fmt.Errorf("invalid stack version: %w", err) - } - validationMethod := r.fieldValidationMethod if stackVersion.LessThan(semver_8_14_0) { logger.Debugf("Forced to validate transforms based on fields, not available for stack versions < 8.14.0") @@ -2153,7 +2148,7 @@ func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId // Add specific timeout to ensure transform processes the documents (depends on "delay" field?) timeout: 10 * time.Minute, stopCondition: func(_ *hits, _ int) (bool, error) { - processed, err := r.processedTransformDocs(ctx, transformId) + processed, err := r.processedDocsByTransform(ctx, transformId) if err != nil { return false, err } @@ -2256,7 +2251,7 @@ func (r *tester) previewTransform(ctx context.Context, transformId string) ([]co return preview.Documents, nil } -func (r *tester) processedTransformDocs(ctx context.Context, transformId string) (int, error) { +func (r *tester) processedDocsByTransform(ctx context.Context, transformId string) (int, error) { resp, err := r.esAPI.TransformGetTransformStats(transformId, r.esAPI.TransformGetTransformStats.WithContext(ctx), ) From afac6f361e37e282ce0c44be39ca42a7820205bb Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 27 Jan 2025 18:16:37 +0100 Subject: [PATCH 50/51] Report any mapping error or any doc with an error --- internal/testrunner/runners/system/tester.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 0f6fda344..fd25d0167 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1646,6 +1646,8 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re if mappingsErrs := validateMappings(ctx, mappingsValidator); len(mappingsErrs) > 0 { errs = append(errs, mappingsErrs...) + } + if len(errs) > 0 { return result.WithError(testrunner.ErrTestCaseFailed{ Reason: fmt.Sprintf("one or more errors found in mappings in %s index template", scenario.indexTemplateName), Details: errs.Error(), @@ -2179,9 +2181,10 @@ func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId } errs := ensureNoErrorsInDocs(transformDocs) - if mappingErrs := validateMappings(ctx, mappingsValidator); len(mappingErrs) > 0 { errs = append(errs, mappingErrs...) + } + if len(errs) > 0 { return testrunner.ErrTestCaseFailed{ Reason: fmt.Sprintf("one or more errors found in mappings in the transform %q (index %s)", transformName, destIndexTransform), Details: errs.Error(), From 1d539eef6799ea54d21d96e32086c63227c8ca45 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 29 Jan 2025 16:08:11 +0100 Subject: [PATCH 51/51] Use source data stream hits as a reference for transforms --- internal/testrunner/runners/system/tester.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 09827388b..34ce62514 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -2129,7 +2129,7 @@ func (r *tester) checkTransforms(ctx context.Context, stackVersion *semver.Versi transform.Name, ) - err := r.validateTransformsWithMappings(ctx, transformId, transform.Name, destIndexTransform, indexTemplateTransform, transformDocs, fieldsValidator) + err := r.validateTransformsWithMappings(ctx, dataStream, transformId, transform.Name, destIndexTransform, indexTemplateTransform, transformDocs, fieldsValidator) if err != nil { return err } @@ -2139,7 +2139,12 @@ func (r *tester) checkTransforms(ctx context.Context, stackVersion *semver.Versi return nil } -func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { +func (r *tester) validateTransformsWithMappings(ctx context.Context, sourceDataStream, transformId, transformName, destIndexTransform, indexTemplateTransform string, transformDocs []common.MapStr, fieldsValidator *fields.Validator) error { + logger.Debugf("Searching the number of documents found in source data stream %q before validating transform %q", sourceDataStream, transformId) + sourceDataStreamHits, err := r.getDocs(ctx, sourceDataStream) + if err != nil { + return fmt.Errorf("failed to get docs for transform (%q) validation from data stream %q: %w", transformId, sourceDataStream, err) + } // In order to compare the mappings, it is required to wait until the documents has been // ingested in the given transform index // It looks like that not all documents ingested previously in the main data stream are going @@ -2157,7 +2162,7 @@ func (r *tester) validateTransformsWithMappings(ctx context.Context, transformId if processed > 0 { logger.Debugf("Documents processed by transform %q: %d", transformId, processed) } - return processed >= len(transformDocs), nil + return processed >= sourceDataStreamHits.size(), nil }, }