Deduplication of entries and items before sending to endpoint #71297

alexk307 · 2020-07-09T19:02:06Z

Summary

Implements deduplication of entries and items in exception lists before they are converted and ultimately made available to the endpoint.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…ptions-dedupe

madirey · 2020-07-09T19:16:44Z

👀

madirey · 2020-07-09T19:33:08Z

x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts

@@ -97,10 +97,18 @@ export function translateToEndpointExceptions(
 exc: FoundExceptionListItemSchema,
 schemaVersion: string
 ): TranslatedExceptionListItem[] {
+ const entrySet = new Set();


I think this would be cleaner using reduce with an accumulator that contains both the set and the filtered list... and then just return the list at the end. Or you could use filter, but using reduce could avoid the external state.

I was going to use filter but avoided it because I thought it would just make things worse since I have to add to the set as well after checking if it exists.

I think filter or reduce would be preferred over forEach here. My guess is that reduce will be the best fit, but again, I'm still learning.

I agree with @madirey here. Using reduce as she describes would be cleaner. However, I think it would be better if we pulled out the super duper deduping logic out of this function and into a separate function. That way we could test that logic independently. translateToEndpointExceptions and translateItem should do only that, translation. Deduping might be better handled elsewhere.

Not sure if I agree, pulling this out would still force us to keep some sort of state within the translation functions, and just move the Set methods elsewhere. Not much use in testing that separately I think.

I'll look into filter or reduce

We talked and now we like this. 💯

madirey · 2020-07-09T19:34:11Z

x-pack/plugins/security_solution/server/endpoint/lib/artifacts/lists.ts

@@ -124,12 +132,17 @@ function translateItem(
 schemaVersion: string,
 item: ExceptionListItemSchema
 ): TranslatedExceptionListItem {
+ const itemSet = new Set();


Again you could use a custom accumulator here to avoid the global state... not sure how much of a problem it is, maybe get another opinion on it. Looks like it will work though.

Not too well versed on this. Does this provide us with benefits other than syntactic sugar/style?

@alexk307 I'm not the best authority on javascript, but my understanding is that it's best practice to avoid mutable state and mixed scope. This might not be so bad because itemSet is a const, but it's introducing a block scope variable that's (probably unnecessarily?) external to the scope of the reduce function.

And this too.

madirey · 2020-07-09T19:35:42Z

Looks great! Good tests... I think I'd avoid the global state, but happy to hear other opinions on it.

kibanamachine · 2020-07-09T20:59:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 5ef2354

Build metrics

✅ unchanged

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

#71349) * Deduplication of entries and items before sending to endpoint * Renaming

…11y-overlay * 'master' of github.com:elastic/kibana: (33 commits) address index templates feedback (elastic#71353) Upgrade EUI to v26.3.1 (elastic#70243) [build] Creates Linux aarch64 archive (elastic#69165) [SIEM][Detection Engine] Fixes skipped tests (elastic#71347) [SIEM][Detection Engine][Lists] Adds read_privileges route for lists and list items [kbn/optimizer] implement "requiredBundles" property of KP plugins (elastic#70911) [Security Solution][Exceptions] - Exceptions modal pt 2 (elastic#70886) [ML] DF Analytics: stop status polling when job stopped (elastic#71159) [SIEM][CASE] IBM Resilient Connector (elastic#66385) jenkins_xpack_saved_objects_field_metrics.sh expects to be run from the KIBANA_DIR in CI Deduplication of entries and items before sending to endpoint (elastic#71297) [services/remote/webdriver] fix eslint error (elastic#71346) send slack notifications on visual baseline failures fix visual regression job (elastic#70999) [Ingest Manager] Add schema to usageCollector. (elastic#71219) [ftr] use typed chromeOptions object, adding TEST_BROWSER_BINARY_PATH (elastic#71279) [Ingest Manager] Fix limited packages incorrect response (elastic#71292) Support multiple features declaring same properties (elastic#71106) [Security_Solution][Resolver]Add beta badge to Resolver panel (elastic#71183) [DOCS] Clarify trial subscription levels (elastic#70636) ...

elasticmachine · 2020-07-16T18:11:38Z

Pinging @elastic/siem (Team:SIEM)

elasticmachine · 2021-09-23T14:31:13Z

Pinging @elastic/security-solution (Team: SecuritySolution)

Deduplication of entries and items before sending to endpoint

bfa70cc

alexk307 requested review from a team as code owners July 9, 2020 19:02

Alex Kahan added 2 commits July 9, 2020 15:04

Renaming

d001454

Merge branch 'master' of github.com:elastic/kibana into endpoint-exce…

5ef2354

…ptions-dedupe

madirey reviewed Jul 9, 2020

View reviewed changes

alexk307 added v7.9.0 release_note:enhancement release_note:fix and removed release_note:enhancement labels Jul 9, 2020

madirey approved these changes Jul 9, 2020

View reviewed changes

alexk307 merged commit c9e8650 into elastic:master Jul 9, 2020

alexk307 deleted the endpoint-exceptions-dedupe branch July 9, 2020 22:57

alexk307 mentioned this pull request Jul 9, 2020

[7.x] Deduplication of entries and items before sending to endpoint (#71297) #71349

Merged

alexk307 pushed a commit that referenced this pull request Jul 10, 2020

Deduplication of entries and items before sending to endpoint (#71297) (

693ddbb

#71349) * Deduplication of entries and items before sending to endpoint * Renaming

LeeDr added the Team:SIEM label Jul 16, 2020

MindyRS added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Sep 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Deduplication of entries and items before sending to endpoint #71297

Deduplication of entries and items before sending to endpoint #71297

alexk307 commented Jul 9, 2020 •

edited

Loading

madirey commented Jul 9, 2020

madirey Jul 9, 2020 •

edited

Loading

alexk307 Jul 9, 2020

madirey Jul 9, 2020

peluja1012 Jul 9, 2020

alexk307 Jul 9, 2020

madirey Jul 9, 2020

madirey Jul 9, 2020

alexk307 Jul 9, 2020

madirey Jul 9, 2020

madirey Jul 9, 2020

madirey commented Jul 9, 2020

kibanamachine commented Jul 9, 2020

elasticmachine commented Jul 16, 2020

elasticmachine commented Sep 23, 2021

Deduplication of entries and items before sending to endpoint #71297

Deduplication of entries and items before sending to endpoint #71297

Conversation

alexk307 commented Jul 9, 2020 • edited Loading

Summary

Checklist

For maintainers

madirey commented Jul 9, 2020

madirey Jul 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

madirey commented Jul 9, 2020

kibanamachine commented Jul 9, 2020

💚 Build Succeeded

Build metrics

elasticmachine commented Jul 16, 2020

elasticmachine commented Sep 23, 2021

alexk307 commented Jul 9, 2020 •

edited

Loading

madirey Jul 9, 2020 •

edited

Loading