Skip to content

Commit

Permalink
[system tests] Add exceptions for leaf of objects if required for map…
Browse files Browse the repository at this point in the history
…ping bases validation (#2454)

This PR adds the same exception for leaf of objects that was set in
place for validation based on fields found in the documents. These
validations are based on the spec version defined in the package manifest.
  • Loading branch information
mrodm authored Mar 5, 2025
1 parent b64f374 commit 183024e
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 17 deletions.
8 changes: 5 additions & 3 deletions cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func RootCmd() *cobra.Command {
)
},
}
rootCmd.PersistentFlags().BoolP(cobraext.VerboseFlagName, cobraext.VerboseFlagShorthand, false, cobraext.VerboseFlagDescription)
rootCmd.PersistentFlags().CountP(cobraext.VerboseFlagName, cobraext.VerboseFlagShorthand, cobraext.VerboseFlagDescription)
rootCmd.PersistentFlags().StringP(cobraext.ChangeDirectoryFlagName, cobraext.ChangeDirectoryFlagShorthand, "", cobraext.ChangeDirectoryFlagDescription)

for _, cmd := range commands {
Expand All @@ -71,12 +71,14 @@ func Commands() []*cobraext.Command {
}

func processPersistentFlags(cmd *cobra.Command, args []string) error {
verbose, err := cmd.Flags().GetBool(cobraext.VerboseFlagName)
verbose, err := cmd.Flags().GetCount(cobraext.VerboseFlagName)
if err != nil {
return cobraext.FlagParsingError(err, cobraext.VerboseFlagName)
}
if verbose {
if verbose == 1 {
logger.EnableDebugMode()
} else if verbose > 1 {
logger.EnableTraceMode()
}

changeDirectory, err := cmd.Flags().GetString(cobraext.ChangeDirectoryFlagName)
Expand Down
15 changes: 10 additions & 5 deletions internal/fields/exceptionfields.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package fields

import (
"slices"
"strings"

"github.com/elastic/elastic-package/internal/common"
Expand Down Expand Up @@ -37,6 +38,8 @@ func (v *Validator) listExceptionFieldsMapElement(root string, elem common.MapSt
all = append(all, fields...)
default:
if skipLeafOfObject(root, name, v.specVersion, v.Schema) {
logger.Tracef("Skip validating leaf of object (spec %q): %q", v.specVersion, key)
all = append(all, key)
// Till some versions we skip some validations on leaf of objects, check if it is the case.
break
}
Expand All @@ -45,6 +48,8 @@ func (v *Validator) listExceptionFieldsMapElement(root string, elem common.MapSt
all = append(all, fields...)
}
}
slices.Sort(all)
all = slices.Compact(all)
return all
}

Expand Down Expand Up @@ -86,7 +91,7 @@ func (v *Validator) parseExceptionField(key string, definition FieldDefinition,
case "ip":
case "array":
if v.specVersion.LessThan(semver2_0_0) {
logger.Warnf("Skip validating field of type array with spec < 2.0.0 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
logger.Tracef("Skip validating field of type array with spec < 2.0.0 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
return []string{key}
}
return nil
Expand All @@ -97,21 +102,21 @@ func (v *Validator) parseExceptionField(key string, definition FieldDefinition,
// This is probably an element from an array of objects,
// even if not recommended, it should be validated.
if v.specVersion.LessThan(semver3_0_1) {
logger.Warnf("Skip validating object (map[string]any) in package spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
logger.Tracef("Skip validating object (map[string]any) in package spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
return []string{key}
}
return nil
case []any:
// This can be an array of array of objects. Elasticsearh will probably
// flatten this. So even if this is quite unexpected, let's try to handle it.
if v.specVersion.LessThan(semver3_0_1) {
logger.Warnf("Skip validating object ([]any) because spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
logger.Tracef("Skip validating object ([]any) because spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
return []string{key}
}
return nil
case nil:
// The document contains a null, let's consider this like an empty array.
logger.Warnf("Skip validating object empty array because spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
logger.Tracef("Skip validating object empty array because spec < 3.0.1 (key %q type %q spec %q)", key, definition.Type, v.specVersion)
return []string{key}
default:
switch {
Expand All @@ -123,7 +128,7 @@ func (v *Validator) parseExceptionField(key string, definition FieldDefinition,
return v.parseExceptionField(key, definition, val)
case definition.Type == "object" && definition.ObjectType == "":
// Legacy mapping, ambiguous definition not allowed by recent versions of the spec, ignore it.
logger.Warnf("Skip legacy mapping: object field without \"object_type\" parameter: %q", key)
logger.Tracef("Skip legacy mapping: object field without \"object_type\" parameter: %q", key)
return []string{key}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/fields/exceptionfields_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func TestFindExceptionElements(t *testing.T) {
Type: "array",
},
},
expected: []string{"remote_ip_list", "remote_ip_list"},
expected: []string{"remote_ip_list"},
specVersion: *semver.MustParse("1.5.0"),
},
{
Expand Down Expand Up @@ -68,7 +68,7 @@ func TestFindExceptionElements(t *testing.T) {
Type: "nested",
},
},
expected: []string{"ip_port_info", "ip_port_info"},
expected: []string{"ip_port_info"},
specVersion: *semver.MustParse("1.5.0"),
},
{
Expand Down Expand Up @@ -118,7 +118,7 @@ func TestFindExceptionElements(t *testing.T) {
Type: "group",
},
},
expected: []string{"answers", "answers"},
expected: []string{"answers"},
specVersion: *semver.MustParse("1.5.0"),
},
{
Expand Down
8 changes: 4 additions & 4 deletions internal/fields/mappings.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit
}

if slices.Contains(v.exceptionFields, path) {
logger.Warnf("Found exception field, skip its validation: %q", path)
logger.Tracef("Found exception field, skip its validation: %q", path)
return nil
}

Expand All @@ -457,7 +457,7 @@ func (v *MappingValidator) compareMappings(path string, couldBeParametersDefinit
}
return nil
} else if !isObject(preview) {
errs = append(errs, fmt.Errorf("not found properties in preview mappings for path: %q", path))
errs = append(errs, fmt.Errorf("undefined field mappings found in path: %q", path))
return errs.Unique()
}
previewProperties, err := getMappingDefinitionsField("properties", preview)
Expand Down Expand Up @@ -561,7 +561,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 (not present in preview): %q", fieldPath)
logger.Tracef("Found exception field, skip its validation (not present in preview): %q", fieldPath)
return nil
}

Expand All @@ -572,7 +572,7 @@ func (v *MappingValidator) validateMappingsNotInPreview(currentPath string, chil
}

if isEmptyObject(def) {
logger.Debugf("Skip field which value is an empty object: %q", fieldPath)
logger.Tracef("Skip field which value is an empty object: %q", fieldPath)
continue
}

Expand Down
2 changes: 1 addition & 1 deletion internal/fields/mappings_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ func TestComparingMappings(t *testing.T) {
exceptionFields: []string{},
schema: []FieldDefinition{},
expectedErrors: []string{
`not found properties in preview mappings for path: "foo"`,
`undefined field mappings found in path: "foo"`,
},
},
{
Expand Down
32 changes: 31 additions & 1 deletion internal/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
)

var isDebugMode bool
var isTraceMode bool

// EnableDebugMode method enables verbose logging.
func EnableDebugMode() {
Expand All @@ -18,6 +19,14 @@ func EnableDebugMode() {
Debug("Enable verbose logging")
}

// EnableTraceMode method enables trace verbose logging.
func EnableTraceMode() {
isDebugMode = true
isTraceMode = true

Debug("Enable trace verbose logging")
}

// Debug method logs message with "debug" level.
func Debug(a ...interface{}) {
if !IsDebugMode() {
Expand All @@ -36,7 +45,28 @@ func Debugf(format string, a ...interface{}) {

// IsDebugMode method checks if the debug mode is enabled.
func IsDebugMode() bool {
return isDebugMode
return isDebugMode || isTraceMode
}

// Trace method logs message with "trace" level.
func Trace(a ...interface{}) {
if !IsTraceMode() {
return
}
logMessage("TRACE", a...)
}

// Tracef method logs message with "trace" level and formats it.
func Tracef(format string, a ...interface{}) {
if !IsTraceMode() {
return
}
logMessagef("TRACE", format, a...)
}

// IsTraceMode method checks if the trace mode is enabled.
func IsTraceMode() bool {
return isTraceMode
}

// Info method logs message with "info" level.
Expand Down
1 change: 1 addition & 0 deletions internal/testrunner/runners/system/tester.go
Original file line number Diff line number Diff line change
Expand Up @@ -2395,6 +2395,7 @@ func listExceptionFields(docs []common.MapStr, fieldsValidator *fields.Validator
}
}

logger.Tracef("Fields to be skipped validation: %s", strings.Join(allFields, ","))
return allFields
}

Expand Down

0 comments on commit 183024e

Please sign in to comment.