[Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression by madirey · Pull Request #70759 · elastic/kibana

madirey · 2020-07-05T17:59:07Z

Summary

Addresses feedback and TODOs from [SIEM][Security Solution][Endpoint] Endpoint Artifact Manifest Management + Artifact Download and Distribution #67707 and [Security Solution][Endpoint] Update to new manifest format (without compression) #70752
Adds compression of artifacts (in progress).

To do in this PR:

compress the artifacts
prefer async/await to then
look for additional opportunities to use array.map, array.reduce, etc instead of for loops
ensure dispatch on callback before commit (https://github.com/elastic/kibana/pull/70752/files#diff-c2c2cdb2c0ae4eecf741c83a240c7654R60-R61)
fix artifact size and sha calculations
ensure that all exceptions are caught in ingest manager callback to address recent bug
fix base64 bug on download

To address in a follow-up PR:

Checklist

Delete any items that are not applicable to this PR.

Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

…t-artifacts-pt2

elasticmachine · 2020-07-05T18:01:28Z

Pinging @elastic/endpoint-app-team (Feature:Endpoint)

elasticmachine · 2020-07-05T18:01:30Z

Pinging @elastic/endpoint-response (Team:Endpoint Response)

alexk307 · 2020-07-06T15:00:36Z

...ns/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts

-      this.logger.debug('wrappedManifest was null, aborting dispatch');
-      return null;
-    }
+  public async syncArtifacts(snapshot: ManifestSnapshot, diffType?: 'add' | 'delete') {


nit: would maybe make diffType a type and have add and delete as their own types as well.

not done yet

…ana into user-allowlist-artifacts-pt3

…t-artifacts-pt3

…ana into user-allowlist-artifacts-pt3

XavierM · 2020-07-08T21:15:25Z

...ns/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.ts

+      return diffs;
+    }, []);
+
+    const adds = filteredDiffs.filter((diff) => {


nit -> to be more readable it will be nice have branch logic to show that you are only doing add || delete on this function.

XavierM · 2020-07-08T21:17:13Z

x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts

-      // TODO: confirm creation of package config
-      // then commit.
-      await manifestManager.commit(wrappedManifest);
+      if (snapshot.diffs.length > 0) {


nit -> I think it will be nice to have try/catch here to avoid bubble up error and have a better way to handle it

XavierM · 2020-07-08T21:17:58Z

x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts

@@ -57,9 +70,18 @@ export const getPackageConfigCreateCallback = (
    try {
      return updatedPackageConfig;
    } finally {


nit -> We might think about sending a promise instead of counting on finally. I think that will be more readable too

…ana into user-allowlist-artifacts-pt3

alexk307 · 2020-07-09T00:29:22Z

@elasticmachine merge upstream

peluja1012 · 2020-07-09T01:28:43Z

@elasticmachine merge upstream

peluja1012 · 2020-07-09T02:27:30Z

@elasticmachine merge upstream

* master: (39 commits) [APM] Add warning to notify user about legacy ML jobs (elastic#71030) updates consumer to siem (elastic#71117) Index pattern creation flow - fix spelling (elastic#71192) [Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression (elastic#70759) [SECURITY] Rearrange rule name's column in Alert Table (elastic#71020) [SECURITY] Alerts back to Detections (elastic#71142) [Security Solution][Exceptions Builder] - Fixes operator selection bug (elastic#71178) [SIEM][Detection Engine] Speeds up value list imports by enabling streaming of files. [APM] Update ML job ID in data telemetry tasks (elastic#71044) [Resolver] Remove `currentPanelView` selector (elastic#71154) add meta.managed to index templates (elastic#71135) Clarify trial subscription levels (elastic#70900) [Security Solution] fix panel links (elastic#71148) skip flaky suite (elastic#69632) skip suite failing ES Promotion (elastic#71018) [ML] DF Analytics: add results field to wizard and show regression stats (elastic#70893) [SIEM] update wordings (elastic#71119) [SECURITY SOLUTION] Rename to hosts and administration (elastic#70913) [ML] Improvements for urlState hook. (elastic#70576) Removing uptime guide (elastic#71124) ...

…ssion (#70759) (#71246) * Stateless exception list translation with improved runtime checks * use flatMap and reduce to simplify logic * Update to new manifest format * Fix test fixture SO data type * Fix another test fixture data type * Fix sha256 reference in artifact_client * Refactor to remove usages of 'then' and tidy up a bit * Zlib compression * prefer byteLength to length * Make ingestManager optional for security-solution startup * Fix download functionality * Use eql for deep equality check * Fix base64 download bug * Add test for artifact download * Add more tests to ensure cached versions of artifacts are correct * Convert to new format * Deflate * missed some refs * partial fix to wrapper format * update fixtures and integration test * Fixing unit tests * small bug fixes * artifact and manifest versioning changes * Remove access tag from download endpoint * Adding decompression to integration test * Removing tag from route * add try/catch in ingest callback handler * Fixing * Removing last expect from unit test for tag * type fixes * Add compression type to manifest * Reverting ingestManager back to being required for now Co-authored-by: Alex Kahan <alexander.kahan@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Alex Kahan <alexander.kahan@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

rylnd · 2020-07-09T16:11:04Z

x-pack/plugins/security_solution/server/plugin.ts

 export interface StartPlugins {
-  ingestManager: IngestManagerStartContract;
-  taskManager: TaskManagerStartContract;
+  ingestManager?: IngestManagerStartContract;


Since both ingestManager and taskManager are required plugins, these shouldn't be typed as optional here and the downstream null checks are unnecessary. Is this a temporary situation, or can we clean this up?

kibanamachine · 2020-07-09T17:50:05Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 899a673

Build metrics

✅ unchanged

History

💚 Build #60052 succeeded 899a673
💔 Build #60047 failed a4414f9
💔 Build #60034 failed 50b49a5
💔 Build #60007 failed 2bf4acb
💔 Build #59984 failed 0809b77

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

madirey added 9 commits July 2, 2020 14:12

Stateless exception list translation with improved runtime checks

133ea27

use flatMap and reduce to simplify logic

9e43633

Update to new manifest format

e9f74b4

Merge branch 'master' of github.com:elastic/kibana into user-allowlis…

2a266b3

…t-artifacts-pt2

Fix test fixture SO data type

ceca5fa

Fix another test fixture data type

49d22d2

Fix sha256 reference in artifact_client

9428ca5

Refactor to remove usages of 'then' and tidy up a bit

c376638

sync master

617d5f1

madirey requested review from a team as code owners July 5, 2020 17:59

madirey changed the title ~~[Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression [TODO]~~ [Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression Jul 5, 2020

madirey added Feature:Endpoint Elastic Endpoint feature Team:Endpoint Response Endpoint Response Team v7.9.0 v8.0.0 labels Jul 5, 2020

madirey added the release_note:skip Skip the PR/issue when compiling release notes label Jul 5, 2020

alexk307 reviewed Jul 6, 2020

View reviewed changes

alexk307 previously approved these changes Jul 6, 2020

View reviewed changes

Alex Kahan and others added 8 commits July 6, 2020 15:29

Zlib compression

13d9ec8

prefer byteLength to length

4718110

Make ingestManager optional for security-solution startup

feb1202

Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…

4ce792b

…ana into user-allowlist-artifacts-pt3

Fix download functionality

aecf718

Use eql for deep equality check

56886ad

Fix base64 download bug

03e75c9

Add test for artifact download

8620510

madirey and others added 5 commits July 8, 2020 14:17

Merge branch 'master' of github.com:elastic/kibana into user-allowlis…

206f222

…t-artifacts-pt3

Remove access tag from download endpoint

ae856b5

Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…

ced63a9

…ana into user-allowlist-artifacts-pt3

Adding decompression to integration test

69f59fd

Removing tag from route

4795132

XavierM reviewed Jul 8, 2020

View reviewed changes

XavierM approved these changes Jul 8, 2020

View reviewed changes

madirey and others added 6 commits July 8, 2020 17:24

add try/catch in ingest callback handler

c18948b

Fixing

8bef643

Merging

a2b3a1b

Removing last expect from unit test for tag

a24acc8

type fixes

04dbbcf

Merge branch 'user-allowlist-artifacts-pt3' of github.com:madirey/kib…

0809b77

…ana into user-allowlist-artifacts-pt3

alexk307 approved these changes Jul 8, 2020

View reviewed changes

Add compression type to manifest

2bf4acb

Merge branch 'master' into user-allowlist-artifacts-pt3

50b49a5

Merge branch 'master' into user-allowlist-artifacts-pt3

046f271

elasticmachine and others added 2 commits July 8, 2020 20:27

Merge branch 'master' into user-allowlist-artifacts-pt3

a4414f9

Reverting ingestManager back to being required for now

899a673

peluja1012 merged commit c3622e3 into elastic:master Jul 9, 2020

madirey mentioned this pull request Jul 9, 2020

[7.x] [Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression (#70759) #71246

Merged

rylnd reviewed Jul 9, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression#70759

[Security Solution][Endpoint] User Manifest Cleanup + Artifact Compression#70759
peluja1012 merged 53 commits intoelastic:masterfrom
madirey:user-allowlist-artifacts-pt3

madirey commented Jul 5, 2020 •

edited

Loading

Uh oh!

elasticmachine commented Jul 5, 2020

Uh oh!

elasticmachine commented Jul 5, 2020

Uh oh!

alexk307 Jul 6, 2020

Uh oh!

XavierM Jul 8, 2020

Uh oh!

XavierM Jul 8, 2020

Uh oh!

XavierM Jul 8, 2020

Uh oh!

alexk307 commented Jul 9, 2020

Uh oh!

peluja1012 commented Jul 9, 2020

Uh oh!

peluja1012 commented Jul 9, 2020

Uh oh!

rylnd Jul 9, 2020

Uh oh!

kibanamachine commented Jul 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

madirey commented Jul 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Jul 5, 2020

Uh oh!

elasticmachine commented Jul 5, 2020

Uh oh!

alexk307 Jul 6, 2020

Choose a reason for hiding this comment

Uh oh!

XavierM Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

XavierM Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

XavierM Jul 8, 2020

Choose a reason for hiding this comment

Uh oh!

alexk307 commented Jul 9, 2020

Uh oh!

peluja1012 commented Jul 9, 2020

Uh oh!

peluja1012 commented Jul 9, 2020

Uh oh!

rylnd Jul 9, 2020

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jul 9, 2020

💚 Build Succeeded

Build metrics

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

madirey commented Jul 5, 2020 •

edited

Loading