-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate arrays of objects #1489
Conversation
internal/fields/validate.go
Outdated
) | ||
|
||
// EPF - Elastic Package Fields [validation] | ||
const ArrayOfObjectsErrorCode = "EPF00001" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mrodm do we have a place where we are already defining Elastic Package errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, there is no definition of errors in Elastic Package. Until now, it was just getting the errors from package-spec validate method and return the errors filtered (if any).
Should this be added into a new internal/errors
package ? Or maybe another naming to not have collisions with standard errors package (e.g. validationerrors? I don't like neither too much this).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed by now.
internal/fields/validate.go
Outdated
) | ||
|
||
// EPF - Elastic Package Fields [validation] | ||
const ArrayOfObjectsErrorCode = "EPF00001" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, there is no definition of errors in Elastic Package. Until now, it was just getting the errors from package-spec validate method and return the errors filtered (if any).
Should this be added into a new internal/errors
package ? Or maybe another naming to not have collisions with standard errors package (e.g. validationerrors? I don't like neither too much this).
internal/fields/validate.go
Outdated
} | ||
errs := v.validateMapElement(key, common.MapStr(val), doc) | ||
if definition.Type == "group" { | ||
errs = append(errs, arrayOfObjectsErr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these errors could be filtered as it is the code.
The methods that get the errors and filter are just for the errors from package-spec. This is in internal/validation
package. For instance
func ValidateAndFilterFromPath(rootPath string) (error, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this would need to be filtered at the moment of testing the package. This makes me wonder if this validation errors filtering fits here, or if this should be configured in tests. What do you think?
I could continue without making this filterable by now. It would only affect packages using spec v3.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed by now.
/test |
💚 Build Succeeded
History
cc @jsoriano |
We added some additional validations for subobjects and arrays of objects in #1498, #1489 and related PRs. These validations only apply to packages with spec starting on 3.0.1. These validations rely on the structure of documents. With the adoption of features like `subobjects: false` or synthetic source the structure is lost, and exceptions based on spec version are not working, so the tests fail for cases where they should not for versions of the spec older than 3.0.1. This happens for example in the `dns` data stream of the `network_traffic` package when LogsDB is enabled.
Add validation for arrays of objects, considering them as valid when they are inside
nested
orgroup
.When
group
is used, a filterable error is given, mentioning thatnested
should be used instead on this case.Fix #1488