[Endpoint] add resolver middleware by kqualters-elastic · Pull Request #58288 · elastic/kibana

kqualters-elastic · 2020-02-21T22:47:30Z

Summary

First pass at creating a middleware for resolver, that loads data from the recently added resolver apis. Context is passed to a new middleware for the separate resolver store that uses kibana http services to load events from the 3 recently added resolver apis.

Related issues:
https://github.com/elastic/endpoint-app-team/issues/25
https://github.com/elastic/endpoint-app-team/issues/202

Checklist

non goals

we aren't supporting v1 events in this PR TODO add a github issue
we aren't adding a functional test to load the flyout. This has a react test. The functional test will come in a follow up PR after [Endpoint] Alert Details Overview #58412
error handling the http request. We don't have this behavior defined yet. What happens if the selected event can't be used to make a resolver request (missing fields?) what happens if the request fails? What happens if we cant draw the graph?
fixing the common types definitions of the indexed documents (Alert, etc). This is out of scope for this pr.

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
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, including a check against IE11

For maintainers

This was checked for breaking API changes and was labeled appropriately

x-pack/plugins/endpoint/public/embeddables/resolver/store/middleware.ts

elasticmachine · 2020-02-21T22:53:31Z

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

bkimmel · 2020-02-25T19:47:21Z

x-pack/plugins/endpoint/common/types.ts


 export interface EndpointEvent {
-  '@timestamp': Date;
+  '@timestamp': string;


❔ Give the vagaries of dealing with time that you are especially familiar with, it might benefit the reader to have some comment describing what format is expected here.

TBH I'm not sure "string" is right. Does anyone know how dates get serialized into JSON by default through hapi? The underlying value is a date equivalent in Elasticsearch, which according to the docs is stored as a 64-bit integer, so I would imagine that what we're more likely to see is that this is a number unless the elasticsearch node client/hapi combo is doing some number-->Date-->string conversion under the hood.

Then again, on second thought, this could be a string | number if we're working with the source document and just shoving it back to the client...

bkimmel · 2020-02-25T19:48:59Z

x-pack/plugins/endpoint/common/types.ts

    type: string;
  };
+  endgame: {
+    pid: number;


❔ Since we're bringing these things in, it might brook a short explanation in comments (Like why is a pid not the same as a unique_pid? - things we know, but maybe are not obvious to someone who hasn't worked with us before that might be expected to read and understand this.)

bkimmel · 2020-02-25T19:58:37Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx

          <Link
            data-testid="alertTypeCellLink"
-            to={urlFromQueryParams({ ...queryParams, selected_alert: 'TODO' })}
+            to={urlFromQueryParams({ ...queryParams, selected_alert: row.event.id })}


❔ row could resolve to alertListData[NaN] (Div by 0) and throw if the selector ever returned 0 for pageSize. Unlikely? Or maybe use the new chaining syntax?

bkimmel · 2020-02-25T20:00:12Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/resolver.tsx

+  height: 100%;
+  width: 100%;
+  display: flex;
+  flex-grow: 1;


❔ just flex: 1 in case it also needed a shrink?

bkimmel · 2020-02-25T20:01:37Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/resolver.tsx

+      coreStart,
+    }: {
+      className?: string;
+      selectedEvent: object;


❔ 2 question: is selectedEvent being used (or will it)? can it be a ResolverEvent type?

bkimmel · 2020-02-25T21:44:27Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/indexed_process_tree.ts

 import { uniquePidForProcess, uniqueParentPidForProcess } from './process_event';
-import { IndexedProcessTree, ProcessEvent } from '../types';
+import { IndexedProcessTree } from '../types';
+import { EndpointEvent } from '../../../../common/types';


❔ per @andrewstucki we will end up with two types

Yep, looks like this interface is currently describing legacy endgame events, while our focus should be on the ECS-specified elastic-native events. Ideally we can do both to support both the legacy platform and new endpoint product through resolver, and just union their types together--but yeah, separate PR for that sometime in the future is most welcome.

Yeah maybe eventually importing ResolverEvent and doing some type guard to narrow the type to either a LegacyEndpointEvent or EndpointEvent.

bkimmel · 2020-02-25T22:27:05Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/actions.ts

 }

-export type ResolverAction = CameraAction | DataAction | UserBroughtProcessIntoView;
+interface UserChangedSelectedEvent {


🔴 Follow the pattern in the file of describing actions with comments

bkimmel · 2020-02-25T22:33:19Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/middleware.ts

+      } = coreStart;
+      //const uniquePid = action.payload.selectedEvent?.value?.source.endgame.data.pid;
+      const uniquePid = '3096';
+      const { lifecycle } = await http.get(`/api/endpoint/resolver/${uniquePid}`, {


❔ Looks like you could cut a little time out by doing these requests in parallel rather than in a series of awaits since they don't seem to depend on the previous one. (Or maybe that will change?)

bkimmel · 2020-02-25T22:34:55Z

x-pack/plugins/endpoint/public/embeddables/resolver/view/index.tsx

-        <StyledPanel />
-        <StyledGraphControls />
+        {isLoading ? (
+          <div className="loading-container">


❔ May want to check with a11y re: putting a live role on this.

bkimmel

👍 Call for a thumb when you do the revisions. I had one 🔴 about putting better comments in actions, everything else is ❔

paul-tavares · 2020-02-26T14:20:32Z

x-pack/plugins/endpoint/public/applications/endpoint/index.tsx

 interface RouterProps {
  basename: string;
  store: Store;
+  coreStart: CoreStart;


@kqualters-elastic - maybe not for this PR, but we should consider using the kibana-react components for sharing coreStart in the app. I think the use of hooks rather than to force coreStart down to each route's View would be better DX. If we did not want to use the kibana-react components, we could just create our own react Context and set of hooks.

agreed, updated to do just that.

jonathan-buttner · 2020-02-26T14:53:24Z

x-pack/plugins/endpoint/common/types.ts

  agent: {
    type: string;
  };
+  endgame: {


This interface is for the new elastic endpoint events which will not have any references to endgame. I think these belong in LegacyEndpointEvent.

jonathan-buttner · 2020-02-26T14:56:46Z

x-pack/plugins/endpoint/public/embeddables/resolver/models/indexed_process_tree.ts

 import { uniquePidForProcess, uniqueParentPidForProcess } from './process_event';
-import { IndexedProcessTree, ProcessEvent } from '../types';
+import { IndexedProcessTree } from '../types';
+import { EndpointEvent } from '../../../../common/types';


Yeah maybe eventually importing ResolverEvent and doing some type guard to narrow the type to either a LegacyEndpointEvent or EndpointEvent.

Kevin addressed my comments

peluja1012

We need to coordinate merging with this PR #58412, as there is overlapping code . We could base one on the other and make sure they play well together, then merge in sync.

Spoke about coordinating merge

oatkiller

do it

oatkiller · 2020-02-28T16:57:27Z

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts

+  ({ selected_alert: selectedAlert }, alertList) => {
+    return (alertList.find(
+      alert => alert.event.id === selectedAlert
+    ) as unknown) as LegacyEndpointEvent;


TODO stop it

oatkiller · 2020-02-28T17:00:10Z

x-pack/plugins/endpoint/public/embeddables/resolver/factory.ts

  }

-  public async create(initialInput: EmbeddableInput, parent?: IContainer) {
+  public async create(initialInput: EmbeddableInput, parent: IContainer) {


oatkiller · 2020-02-28T17:02:47Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/actions.ts

-export type ResolverAction = CameraAction | DataAction | UserBroughtProcessIntoView;
+interface UserChangedSelectedEvent {
+  readonly type: 'userChangedSelectedEvent';
+  readonly payload: {


/** * Used when the alert list selects an alert and the flyout shows resolver. */

oatkiller · 2020-02-28T17:03:06Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/actions.ts

+interface UserChangedSelectedEvent {
+  readonly type: 'userChangedSelectedEvent';
+  readonly payload: {
+    selectedEvent?: LegacyEndpointEvent;


/** * Optional because they could have unselected the event. */

oatkiller · 2020-02-28T17:03:35Z

x-pack/plugins/endpoint/public/embeddables/resolver/store/actions.ts

+}
+
+interface AppRequestedResolverData {
+  readonly type: 'appRequestedResolverData';


/** * Triggered by middleware when the data for resolver needs to be loaded. Used to set state in redux to 'loading'. */

oatkiller · 2020-02-28T17:09:42Z

x-pack/plugins/endpoint/public/embeddables/resolver/view/index.tsx

    const { projectionMatrix, ref, onMouseDown } = useCamera();
+    const isLoading = useSelector(selectors.isLoading);

+    useEffect(() => {


try useLayoutEffect. This way, if the resolver is rendered with a selected event initially, it'll dispatch the event and rerender before being flushed to the DOM. Otherwise you'll have a flash of uninitialized resolver DOM.

oatkiller · 2020-02-28T17:13:26Z

x-pack/plugins/endpoint/public/embeddables/resolver/view/use_camera.test.tsx

-        // At this time, processes are provided via mock data. In the future, this test will have to provide those mocks.
-        const processes: ProcessEvent[] = [
+        const serverResponseAction: ResolverAction = {
+          type: 'serverReturnedResolverData',


why not use the existing mock factory?

oatkiller · 2020-02-28T17:13:47Z

x-pack/plugins/endpoint/public/plugin.ts


 export interface EndpointPluginStartDependencies {} // eslint-disable-line @typescript-eslint/no-empty-interface

+export interface EndpointPluginServices extends Partial<CoreStart> {


TODO doc comment

oatkiller · 2020-02-28T17:17:41Z

x-pack/plugins/endpoint/server/routes/resolver/queries/children.test.ts

 describe('children events query', () => {
  it('generates the correct legacy queries', () => {
-    const timestamp = new Date();
+    const timestamp = new Date().getTime();


consider reverting this file

going to keep it as is, the ChildrenQuery function uses the PaginationParams interface, which has timestamp as a number https://github.com/elastic/kibana/pull/58288/files/0626d65f70f9164285fce3659c7643625a2ee085#diff-530e696bd60363b2b701f88bbb9bc6feR14

oatkiller · 2020-02-28T17:17:48Z

x-pack/plugins/endpoint/server/routes/resolver/queries/related_events.test.ts

 describe('related events query', () => {
  it('generates the correct legacy queries', () => {
-    const timestamp = new Date();
+    const timestamp = new Date().getTime();


peluja1012 · 2020-02-28T20:50:35Z

x-pack/plugins/endpoint/public/applications/endpoint/view/alerts/index.tsx

          <Link
            data-testid="alertTypeCellLink"
-            to={urlFromQueryParams({ ...queryParams, selected_alert: 'TODO' })}
+            to={urlFromQueryParams({ ...queryParams, selected_alert: row.event.id })}


@kqualters-elastic We use row.id in the rest of the alert details flyout. We use row.id to fetch the details of the alert.

peluja1012 · 2020-02-28T20:51:51Z

x-pack/plugins/endpoint/public/applications/endpoint/store/alerts/selectors.ts

+  uiQueryParams,
+  alertListData,
+  ({ selected_alert: selectedAlert }, alertList) => {
+    const found = alertList.find(alert => alert.event.id === selectedAlert);


These will never equal each other based on how the rest of the details flyout is implemented. We use row.id for selectedAlert.

…a-fetching

kibanamachine · 2020-03-02T09:49:45Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 24fb4fe

History

💔 Build #29913 failed 2600144
💔 Build #29866 failed c6a513e
💚 Build #29844 succeeded bcda2d5
💔 Build #29805 failed d291a46
💛 Build #29576 was flaky 0626d65

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

* master: (26 commits) [Endpoint] Alert Details Overview (elastic#58412) Service map language icons (elastic#58633) [SIEM] [Case] Comments to case view (elastic#58315) Remove appBasePath from docs + add mock for AppMountParameters (elastic#58775) [kbn/optimizer] fix ui/* url rewrites in dist (elastic#58627) Dashboard a11y tests (elastic#58122) Downgrade "setting up plugin" log to debug (elastic#58776) [CI] Pipeline refactoring (elastic#56447) [Advanced Settings] Fix a11y of unsaved indicator (elastic#58511) put params into short url instead of behind it (elastic#58846) show timepicker in timelion and tsvb (elastic#58857) improve graph missing workspace error message (elastic#58876) [Maps] direct Discover "visualize" to open Maps application (elastic#58549) Disallow duplicate percentiles (elastic#57444) (elastic#58299) removing references to visTypes uiExports (elastic#58337) [SIEM] Default the Timeline events filter to show All events (elastic#58953) [Remote clusters] Add indexManagement as required plugin (elastic#58915) [DOCS] Rework of main get started page (elastic#58260) [Endpoint] [Tests] fixes elastic#57946 flaky endpoint policy list test (elastic#58348) [Endpoint] add resolver middleware (elastic#58288) ...

kibanamachine · 2020-03-03T14:28:44Z

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 58288 or prevent reminders by adding the backport:skip label.

* Add resolver middleware * Update types to match events, use sample events in useCamera tests * add predicate to convert alertdata to legacy endpoint event * Use mock event generator in tests * Guard against events not having agent or endpoint fields Co-authored-by: Robert Austin <robert.austin@elastic.co>

kibanamachine · 2020-03-04T14:38:44Z

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

Co-authored-by: kqualters-elastic <56408403+kqualters-elastic@users.noreply.github.com> Co-authored-by: Robert Austin <robert.austin@elastic.co>

WIP add resolver middleware

7a41134

kqualters-elastic added the WIP Work in progress label Feb 21, 2020

kqualters-elastic requested review from bkimmel, dplumlee, kevinlog, oatkiller, paul-tavares and peluja1012 February 21, 2020 22:47

kqualters-elastic commented Feb 21, 2020

View reviewed changes

x-pack/plugins/endpoint/public/embeddables/resolver/store/middleware.ts Outdated Show resolved Hide resolved

kqualters-elastic added the Feature:Resolver Security Solution Resolver feature label Feb 21, 2020

bkimmel reviewed Feb 25, 2020

View reviewed changes

bkimmel approved these changes Feb 25, 2020

View reviewed changes

Change how coreStart is shared, clean up types

e67ac7a

paul-tavares reviewed Feb 26, 2020

View reviewed changes

jonathan-buttner previously requested changes Feb 26, 2020

View reviewed changes

kqualters-elastic added 2 commits February 26, 2020 16:01

Use kibana context

57133a6

Update types to match events, use sample events in useCamera tests

d17e339

kqualters-elastic added 2 commits February 27, 2020 14:07

Add a resolver rendering test

2c0df81

Merge branch 'master' into task/resolver-data-fetching

8ae65ae

kqualters-elastic removed the WIP Work in progress label Feb 27, 2020

kqualters-elastic changed the title ~~WIP add resolver middleware~~ [Endpoint] add resolver middleware Feb 27, 2020

kqualters-elastic added 2 commits February 27, 2020 15:55

Use correct type for timestamp

c809dcd

Use existing resolver es_archive

0626d65

peluja1012 previously requested changes Feb 28, 2020

View reviewed changes

paul-tavares mentioned this pull request Feb 28, 2020

[Endpoint] Task/add nav bar #58604

Merged

7 tasks

oatkiller reviewed Feb 28, 2020

View reviewed changes

oatkiller and others added 2 commits February 28, 2020 12:33

add predicate to convert alertdata to legacy endpoint event

d291a46

Use mock event generator in tests, add comments

bcda2d5

peluja1012 reviewed Feb 28, 2020

View reviewed changes

kqualters-elastic added 2 commits February 28, 2020 16:30

Guard against events not having agent or endpoint fields

a609c11

Merge remote-tracking branch 'upstream/master' into task/resolver-dat…

c6a513e

…a-fetching

peluja1012 approved these changes Feb 28, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into task/resolver-dat…

2600144

…a-fetching

dplumlee approved these changes Mar 1, 2020

View reviewed changes

Merge remote-tracking branch 'upstream/master' into task/resolver-dat…

24fb4fe

…a-fetching

kqualters-elastic merged commit a3be4e2 into elastic:master Mar 2, 2020

kqualters-elastic deleted the task/resolver-data-fetching branch March 2, 2020 14:20

kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 3, 2020

dplumlee mentioned this pull request Mar 3, 2020

[7.x] [Endpoint] add resolver middleware (#58288) #59180

Merged

kqualters-elastic mentioned this pull request Mar 4, 2020

[7.x] [Endpoint] add resolver middleware (#58288) #59315

Closed

dplumlee added a commit that referenced this pull request Mar 4, 2020

[Endpoint] add resolver middleware (#58288) (#59180)

77f023f

Co-authored-by: kqualters-elastic <56408403+kqualters-elastic@users.noreply.github.com> Co-authored-by: Robert Austin <robert.austin@elastic.co>

kibanamachine removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 4, 2020


		export interface EndpointPluginStartDependencies {} // eslint-disable-line @typescript-eslint/no-empty-interface

		export interface EndpointPluginServices extends Partial<CoreStart> {

Conversation

kqualters-elastic commented Feb 21, 2020 • edited by oatkiller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

non goals

For maintainers

Uh oh!

Uh oh!

elasticmachine commented Feb 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bkimmel left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peluja1012 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oatkiller left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Mar 2, 2020

kqualters-elastic commented Feb 21, 2020 •

edited by oatkiller

Loading

peluja1012 left a comment •

edited

Loading