-
Notifications
You must be signed in to change notification settings - Fork 438
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
Update security service integrations packages mappings #12624
base: main
Are you sure you want to change the base?
Update security service integrations packages mappings #12624
Conversation
🚀 Benchmarks reportTo see the full report comment with |
… - sublime_security
(cherry picked from commit fa96beb)
8ddd8ff
to
1469158
Compare
@@ -10,7 +10,7 @@ source: | |||
# that ability in order to prevent having duplicate data and prevent query | |||
# time field type conflicts. | |||
dest: | |||
index: "logs-github_latest.dest_code_scanning-1" | |||
index: "logs-github_latest.dest_code_scanning-2" |
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.
If a new field definition is added to a transform, should this destination index be updated (increment suffix number)? Or keep that destination index without changes?
Same doubt for the other packages updating field definitions in transforms (tychon and wiz).
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.
/cc @kcreddy
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.
We increment the index to avoid any conflicts due to mapping changes.
I also simulated an integration upgrade from keyword
to match_only_text
on same index. After the upgrade, the type remained to be keyword
and didn't change to match_only_text
.
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.
So, I guess this would mean that the destination index must be updated, am I right ? @kcreddy
In the other transforms (from other packages), there are changes related to field definitions with changes in mappings like:
- keyword to ip
- text to ip
- text to keyword
- text to match_only_text
- keyword to wildcard
- keyword to match_only_text
Should we update also the destination index there?
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.
So, I guess this would mean that the destination index must be updated, am I right ?
@mrodm, yes the destination index version has to be updated
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've bumped the version defined in the fleet_transform_version
setting for all the transforms that I've modified here @kcreddy
packages/github/elasticsearch/transform/latest_code_scanning/fields/ecs.yml
Outdated
Show resolved
Hide resolved
packages/sublime_security/data_stream/email_message/fields/fields.yml
Outdated
Show resolved
Hide resolved
- name: ecs.version | ||
external: ecs |
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.
Error: field "ecs.version" is undefined: actual mapping type (text) does not match with ECS definition type: keyword
This change ensures that ecs.version
is set as keyword
.
- name: message | ||
external: ecs |
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.
Error: field "message" is undefined: actual mapping type (text) does not match with ECS definition type: match_only_text
This change ensures that message
is set as match_only_text
.
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.
Same query as above.
- name: device.id | ||
external: ecs |
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.
Error: field "device.id" is undefined: actual mapping type (text) does not match with ECS definition type: keyword
This change ensures that device.id
is mapped as keyword
.
- name: vulnerability.reference | ||
external: ecs |
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.
Error: field "vulnerability.reference" is undefined: actual mapping type (text) does not match with ECS definition type: keyword
This change ensures that vulnerability.reference
is mapped as keyword
.
- name: tags | ||
external: ecs |
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.
Error: field "tags" is undefined: actual mapping type (text) does not match with ECS definition type: keyword
This change ensures that tags
is mapped as keyword
.
Add missing tychon
I was wondering if it would be worth to move all the changes related to transforms to another Pull Request so all the other packages can be updated (exception for teleport), WDYT ? @jsoriano @efd6 With the latest changes applied in |
That wouldn't make me unhappy. |
The changes of the packages that were updating transform field definitions have been moved to #12841 (github, ti_custom, tychon and wiz). I've tried to keep links to the previous discussions. Updated the description accordingly. |
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.
LGTM after clarification of nits.
Moved |
|
💚 Build Succeeded
History
cc @mrodm |
Proposed commit message
Updated failed mappings found in system tests while validating the documents based on mappings in the data-streams
and the corresponding transforms.Example of Buildkite build where the failures are raised: https://buildkite.com/elastic/integrations/builds/20794
Packages updated here:
Packages with changes related to transforms moved to #12841
Teleport changes moved to #12851
event-groups
)Checklist
changelog.yml
file.Author's Checklist
elastic-package
enabled mappings https://buildkite.com/elastic/integrations/builds/21924.buildkite
folder andgo.mod
/go.sum
files.How to test this PR locally
Run elastic-package with mappings validation enabled and run system tests:
Related issues