-
Notifications
You must be signed in to change notification settings - Fork 116
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
Disable normalization validating fields in transforms if synthetics is enabled #2011
Disable normalization validating fields in transforms if synthetics is enabled #2011
Conversation
@@ -1462,7 +1462,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); err != nil { | |||
if err := r.checkTransforms(ctx, config, r.pkgManifest, scenario.kibanaDataStream, scenario.dataStream, scenario.syntheticEnabled); err != nil { |
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.
Use the same value as in the main data stream.
Would that assumption be right?
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.
Not sure what Fleet does with transform indexes. Could we add a test case that would fail without this change?
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'll check to run a test package for that use case (with logsdb enabled).
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.
Added a new test package ti_anomali_logsdb
copied from ti_anomali
. This allows to test the package with both logsdb enabled and disabled.
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10752 |
Failure store is failing when synthetics is enabled when unmarshalling the response (error in CI link):
|
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10752 |
case string: | ||
*p = append(*p, v) | ||
case []any: | ||
// asume it is going to be an array of strings | ||
for _, value := range v { | ||
*p = append(*p, fmt.Sprint(value)) | ||
} |
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.
Support string and slice of strings.
Examples of the response:
- logsdb disabled in the stack:
{ "type": "illegal_argument_exception", "message": "unable to convert [eight] to integer", "stack_trace": "...", "pipeline_trace": [ "logs-failure_store.test-0.0.1" ], "pipeline": "logs-failure_store.test-0.0.1", "processor_type": "convert" }
- logsdb enabled in the stack:
{ "message": "unable to convert [eight] to integer", "pipeline": "logs-failure_store.test-0.0.1", "pipeline_trace": "logs-failure_store.test-0.0.1", "processor_type": "convert", "stack_trace": "...", "type": "illegal_argument_exception" }
Integrations build running with Elastic stack 8.15.0-SNAPSHOT and logsdb enabled: https://buildkite.com/elastic/integrations/builds/14572 In that build, |
This reverts commit ab88d3d.
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10752 |
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.
Changes make sense to me, but let's try to find a test case that can help to confirm if transforms have the same index mode as the data streams of the package.
@@ -1462,7 +1462,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); err != nil { | |||
if err := r.checkTransforms(ctx, config, r.pkgManifest, scenario.kibanaDataStream, scenario.dataStream, scenario.syntheticEnabled); err != nil { |
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.
Not sure what Fleet does with transform indexes. Could we add a test case that would fail without this change?
…mplate test package" This reverts commit 708f4c8.
💚 Build Succeeded
History
cc @mrodm |
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.
Added a new file to be able "source" it in the script in charge of testing false positives (test-check-false-positives.sh
) and the generic one (test-check-packages.sh
).
stack_args=$(stack_version_args) # --version <version> | ||
|
||
# Update the stack | ||
elastic-package stack update -v ${stack_args} |
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.
elastic-package stack update
command does not allow the -U
parameter.
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.
Updated test package with the latest contents from integrations repository.
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.
In order to be able to test ti_anomali
with both logsdb enabled and disabled, a new package ti_anomali_logsdb
has been added (copied from ti_anomali
).
This new package is tested using a Elastic stack with the option -U stack.logsdb_enabled=true
, thanks to the file test/packages/parallel/ti_anomali_logsdb.stack_provider_settings
test integrations |
Created or updated PR in integrations repository to test this version. Check elastic/integrations#10752 |
Relates #1971
Fields in transforms were always validated using normalization.
This PR allows to disable normalization when validating those fields (from transforms) when synthetic is enabled in the data stream created in testing.
Example of errors found in
ti_anomali
package:Validation for
event.type
andevent.category
fields is happening when checking transforms. IfcheckTransforms
method is commented out, validation for those fields run successfully.As a note,
labels.is_ioc_transform_source
fields still fails even with this fix. Probably, some missing mapping/field definition? why is it not failing with logsDB disabled ?Author's checklist
ti_anomali_logsdb
package has the validation errors if normalization is not disabled.labels.is_ioc_transform_source
field definition in transform.