-
Notifications
You must be signed in to change notification settings - Fork 455
[Netskope Alerts] Add text multi-field to netskope.alerts.breach.description field #13977
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
[Netskope Alerts] Add text multi-field to netskope.alerts.breach.description field #13977
Conversation
The test cases were generated running |
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
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.
The manifest specifies v8.13.0 as the minimum version, so the test expectations should be rendered with that stack version.
Just did it, using |
The URI parts and user agent processors are a bit different between the two versions. |
/test |
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.
As you note, this is a breaking change. That should be reflected in the changelog version bump. But also, do we want to split this into to parts: one part for the description mapping (which is not breaking and so is just a minor bump); and the other which is arguably a breaking bugfix. This would allow people to pick up the enhancement without any breakage. It would require this being broken into two PRs so that separate versions get published.
🚀 Benchmarks reportTo see the full report comment with |
No problem, I can change this PR to just change the mapping for the description field and then create a new one for the mapping of the date field with the new processor. The change in the mapping on the description field I would consider an enhancement, as the original mapping is not being changed, just a new But regarding to the versions, should both bump the minor version? I don't think that the breaking change on the date filter for the new PR would be enough to justify bumping this to version 2, there is some discussion on how to deal with these changes here: #13861 |
/test |
What would be enough? |
Not exactly sure, something more complex, I guess. The main issue is that bumping major versions on breaking changes can lead to a faster increase of the integration version, which can give the false impression that a lot of things is changing. If for example we find another 2 breaking next week, this could lead the integration from version 1.X to 4.X in a couple of days. This is related to what is mentioned on the discussion linked.
|
|
💚 Build Succeeded
History
|
Yeah, this is the issue. That is a vibe concept, but we need to have a solid construction. That solid construction is provided by semver. This change would break people, so it's a breaking change. Absent a policy that excludes this kind of breakage from the set that we consider to be breaking, I think we need to at least consider it. This is something that we are discussing internally.
I think this is an education issue; major means no more and no less than, "We are breaking our continuation of API/behaviour contract." That people interpret it to mean progress is an issue with their interpretation. |
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.
Suggested commit message body
The value of the field netskope.alerts.breach.description can be a
lengthy description of the breach, but it is mapped only as a keyword.
Since sometimes the user may want to search for strings in this field
it should be a multi-field with a .text field mapped as a
match_only_field.
@efd6 do I need to change anything else here to have this merged? Once this is merged I will make the other PR. |
If you're happy with that commit message, I'll go ahead and merge with it. |
That's ok! We can merge it. |
Package netskope - 1.24.0 containing this change is available at https://epr.elastic.co/package/netskope/1.24.0/ |
* main: (42 commits) [jamf_pro] Fix `flattened` field types for non-object values (elastic#13985) [Netskope Alerts] Add text multi-field to netskope.alerts.breach.description field (elastic#13977) zscaler_zia: add strict field template mode for tcp and http_endpoint input data streams (elastic#13904) apm: Add config for tail-based sampling discard on write (elastic#13950) [CI] Add dev/coverage into backport script (elastic#13987) Update configuration updatecli for 8.x snapshot (elastic#13981) [Prometheus] Add username, password, and SSL related fields for query dataset (elastic#13969) o365: Ignore failures in rename processors for organization fields (elastic#13983) aws.firewall: Document ingested log types of AWS Network Firewall (elastic#13978) mimecast: resolve field data type conflicts between data streams (elastic#13825) [Infoblox NIOS] Handle the parsing of IPv6 address (elastic#13947) [Cribl] Fix handling of metric event type (elastic#13930) zscaler_zpa: fix handling of multiple remote IPs, and event categorisation (elastic#13755) Adding agentless deployment to the sublime security integration (elastic#13963) [integration/system] add use_performance_counters in system integration (elastic#13150) crowdstrike,m365_defender,microsoft_defender_{cloud,endpoint},sentinel_one: normalise severity handling (elastic#13955) [forgerock] Map `forgerock.response.elapsedTime` as a long not a date (elastic#13959) github: squelch errors from pagination ends (elastic#13965) cisco_secure_endpoint: squelch errors from pagination ends (elastic#13964) [Cloud Security] Cloud Asset Inventory: fixed cloud formation URL (elastic#13971) ...
* feature/use-google-secrets: (43 commits) use -ci account [jamf_pro] Fix `flattened` field types for non-object values (#13985) [Netskope Alerts] Add text multi-field to netskope.alerts.breach.description field (#13977) zscaler_zia: add strict field template mode for tcp and http_endpoint input data streams (#13904) apm: Add config for tail-based sampling discard on write (#13950) [CI] Add dev/coverage into backport script (#13987) Update configuration updatecli for 8.x snapshot (#13981) [Prometheus] Add username, password, and SSL related fields for query dataset (#13969) o365: Ignore failures in rename processors for organization fields (#13983) aws.firewall: Document ingested log types of AWS Network Firewall (#13978) mimecast: resolve field data type conflicts between data streams (#13825) [Infoblox NIOS] Handle the parsing of IPv6 address (#13947) [Cribl] Fix handling of metric event type (#13930) zscaler_zpa: fix handling of multiple remote IPs, and event categorisation (#13755) Adding agentless deployment to the sublime security integration (#13963) [integration/system] add use_performance_counters in system integration (#13150) crowdstrike,m365_defender,microsoft_defender_{cloud,endpoint},sentinel_one: normalise severity handling (#13955) [forgerock] Map `forgerock.response.elapsedTime` as a long not a date (#13959) github: squelch errors from pagination ends (#13965) cisco_secure_endpoint: squelch errors from pagination ends (#13964) ...
Proposed commit message
The value of the field
netskope.alerts.breach.date
is a date in unix epoch format, this field is incorrectly mapped as adouble
, but it should be mapped as adate
as it represents the date when the breach was identified.For this to work as a date a new
date
processor is required and added as well.The value of the field
netskope.alerts.breach.description
can also be a lenghty description of the breach, but it is mapped only as akeyword
, since sometimes the user may want to search for strings in this field it should be amulti-field
with a.text
field mapped as amatch_only_field
One thing to add is that this is a breaking change, the mapping of the field
netskope.alerts.breach.date
is being changed fromdouble
todate
and this will lead to a Kibana Conflict, which makes the field unusable until the conflict is solved.I think that is very unlikely that this field is being used with the current mapping.
Also, the changelog specs does not allow to use multiple types, so since this is correcting the mappings and also adding a new processor, I'm tagging as an
enhancement
and bumped the minor version, I don't think this change justify bumping the major version.Checklist
changelog.yml
file.Related issues