[Security Solution][Detection engine] skips geo_point non-ecs validation #163487
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 245ba57.
vitaliidm
changed the title
vitaliidm
added
8.10 candidate
release_note:skip
vitaliidm
added
Team: SecuritySolution
dhurley14
approved these changes
Aug 9, 2023
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]
History
To update your PR or re-run it, just comment with: cc @vitaliidm |
ymao1
added a commit
to ymao1/kibana
that referenced
this pull request
Aug 10, 2023
… validation (elastic#163487)" This reverts commit 48b7acf.
jloleysens
added a commit
to jloleysens/kibana
that referenced
this pull request
Aug 10, 2023
* main: (108 commits) [Telemetry Schema Validation] Allow `null` on `string` (elastic#163499) [Search] Add Slack and Gmail connectors (elastic#163321) [ML] Provide hints for empty fields in dropdown options in Anomaly detection & Transform creation wizards, Change point detection view (elastic#163371) chore(slo): Add response required fields (elastic#163430) [AO] Fix add_to_case functional test (elastic#163155) unskip license type functional test (elastic#163199) fix(NA): yarn env vars for node_modules mirrors (elastic#163549) [Response Ops][Task Manager] Expose SLI metrics in HTTP API (elastic#162178) [Logs UI] Adapt test to ES highlighting changes and unskip (elastic#163592) [Infra UI] Implement Telemetry on 'Show' buttons within Inventory (elastic#163587) [Enterprise Search]Migrate all usages of EuiPage*_Deprecated (elastic#163482) fix(slo): settings and access for serverless (elastic#163514) [Infra UI] Implement telemetry for the asset details flyout (elastic#163078) [Fleet] Add a banner to the top of the Kafka Output UI to say that Elastic Defend integration is not supported (elastic#163579) [Fleet] Re-enable and fix Fleet policy secret integration tests (elastic#163428) [Fleet] add managed to imported saved object (elastic#163526) [Index Management] Disable index actions using contextRef (elastic#163475) [Discover] Inline shard failures warnings (elastic#161271) [Security Solution][Detection engine] skips geo_point non-ecs validation (elastic#163487) Update EUI layout components in bfetch example plugin (elastic#163490) ...
ymao1
added a commit
that referenced
this pull request
Aug 10, 2023
Reverting #163414 and #163487 ## Summary @pmuellr uncovered a bug in ES with `ignore_malformed` and datastreams while working on #154266 > Found what I hope is an ES bug yesterday w/data streams (DS). It doesn’t like ignore_malformed on the @timestamp field :slightly_smiling_face:. I think this is a bug since [the doc says (https://www.elastic.co/guide/en/elasticsearch/reference/current/ignore-malformed.html#ignore-malformed-setting) Mapping types that don’t support the setting will ignore it if set on the index level. I think it’s understandable - the @timestamp field is a key field for DS (can be overridden) - so you’d not be surprised it’s treated specially. But … why not just ignore it in that case, like the other mapping types that are ignored. I tried overriding ignore_malformed for just that field, and it complained that I couldn’t use that option on that field! hahahahah So, we’d be left having to add ignore_malformed to every mapped field in our mappings, except for @timestamp. For the time being, I’ve removed all the ignore_malformed stuff in my AaD DS PR, when using DS, but left it when using alias/index. Unless someone knows more about this special ignored_malformed / @timestamp field / data-stream relationship, I’ll boil down a simple test case and open an issue for ES. In order to avoid having even more divergent code between serverless & serverful, we will revert this change until we can confirm a bug with ES and hopefully get a fix in.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
While reviewing #163414, I have noticed, that
geo_pointtype still gets validated incomputeIsEcsCompliant, that could lead to removing some of the complexgeo_pointtype representations, notably object like ones, for example{ "type": "Point", "coordinates": [-88.34, 20.12], }In this PR, I completely removing validation for this field type (we even have functional test to verify it)
With changes introduced in #163414, alert will be created even with not valid
geo_pointfields, instead of failing (changed e2e test name)Checklist
Delete any items that are not applicable to this PR.