migrate Actions to Kibana platform #55026

gmmorris · 2020-01-16T11:32:47Z

Summary

Migrates the Actions plugin from Legacy on to the Kibana Platform.
Closes #51651

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~This was checked for cross-browser compatibility, including a check against IE11~~
~~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~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

* upstream/master: (26 commits) Take page offset into account too (elastic#54567) [APM] Support error.{log,exception}.stacktrace.classname (elastic#54577) Np migration tsvb route validation (elastic#51850) [ML] MML calculator enhancements for multi-metric job wizard (elastic#54573) [SIEM] Fix Inspect query 'request timestamp' value changes when curso… (elastic#54223) Fix chromeless NP apps not using full page width (elastic#54550) Remove extraneous public import to prevent failing Kibana startup (elastic#54676) [Uptime] Temporarily skip flakey tests (elastic#54675) Skip failing uptime tests Create UI for alerting and actions plugin (elastic#48959) [dev/build/sass] build stylesheets for disabled plugins too (elastic#54654) [SIEM] Use bulk actions API when updating or deleting rules (elastic#54521) Support "Deprecated" label in advanced settings (elastic#54539) [Maps] add text halo color and width style properties (elastic#53827) Service Map Data API at Runtime (elastic#54027) [SIEM] Detection Engine Create Rule Design Review #1 (elastic#54442) Skip flaky test [Canvas] Enable Embeddable maps (elastic#53971) [SIEM][Detection Engine] Increases the number or rules you can view on a single page (elastic#54628) uiSettings - use validation field for image field maxSize (elastic#54522) ...

* upstream/master: (72 commits) [ML] Calculate model memory limit API integration tests (elastic#54557) Skip flakey index template component integration tests. (elastic#54878) Add label and icon to nested fields in the doc table (elastic#54199) Reverse dependency of home plugin and apm/ml/cloud (elastic#52883) [SIEM][Detection Engine] Order JSON keys, fix scripts, update pre-packaged rules update invalid snapshot add readme note about alerting / manage_api_key cluster privilege (elastic#54639) [SIEM] New Overview Page (elastic#54783) [Uptime] Feature/refactor context initialization (elastic#54494) Upgrade EUI to v18.2.0 (elastic#54786) [SIEM] [Detection engine] from signals to timeline (elastic#54769) [Index Management] Add Mappings Editor to Index Template Wizard (elastic#47562) [SIEM][Detection Engine] Removes deprecated filter from mapping [Maps] Add categorical styling (elastic#54408) Add mapbox-gl-rtl-text library (elastic#54842) [SIEM][Detection Engine] Adds actions to Rule Details (elastic#54828) Lexicographically sort location tags (elastic#54832) [Maps] expand extent filter to tile boundaries (elastic#54276) [Maps] Use v7.6 Elastic Maps Service API (elastic#54399) [DOCS] Adds monitoring setting (elastic#54819) ...

elasticmachine · 2020-01-16T11:32:49Z

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

This reverts commit d38db5b.

* upstream/master: (83 commits) [Reporting] Fix map tiles not loading by using Chrome's Remote Protocol (elastic#55137) [Data Plugin] combine autocomplete provider and suggestions provider (elastic#54451) resolves elastic#53038 - remove references to specific license levels (elastic#53858) highlighting rules should still know about url parts when in sql state (elastic#55200) [Metric] convert mocha tests to jest (elastic#54054) [skip-ci] Update vector styling docs for 7.6 UI changes and new features (elastic#55087) Fix enable API to schedule task after alert is updated (elastic#55095) chore(NA): add 7.6 branch to the list of backport branches (elastic#54998) Convert tests to jest in vis_type_timeseries/public & common folders (elastic#55023) [ML] Accessibility fix for structural markup on table rows (elastic#55075) [Mappings editor] include/exclude fields only support custom options (elastic#54949) [Vis] Move Timelion Vis to vis_type_timelion (elastic#52069) Deprecate `chrome.navlinks.update` and add documentation (elastic#54893) [ML] Single Metric Viewer: Fix time bounds with custom strings. (elastic#55045) [Vis: Default editor] EUIficate and Reactify the sidebar (elastic#49864) [Mappings editor] Fix cannot set boolean value for "null_value" param (elastic#55015) [SIEM] Adds support for apm-* to the network map (elastic#54876) [Reporting] Define shims of legacy dependencies (elastic#54082) Resolver is overflow: hidden to prevent obscured elements from showing up (elastic#55076) Upgraded EUI to 18.2.1 (elastic#55090) ...

* upstream/master: (24 commits) Show error page when accessing unavailable app (elastic#54656) [ML] Improving job wizards with datafeed aggregations (elastic#55180) remove flaly assetion. a license presence tested anyway (elastic#55289) fix commonly used ranges uptime (elastic#54930) [SIEM] Use proper icons on Detections view (elastic#55215) Fix: invalid translation referenced (elastic#54901) [State Management] Remove AppState from edit_index_pattern page (elastic#54104) Implements `getStartServices` on server-side (elastic#55156) Move vis_vega_type/data_model tests to jest (elastic#55186) [SIEM] [Detection Engine] Update status on rule details page (elastic#55201) Fix KQL value suggestions for nested fields (elastic#54820) Enforce camelCase format for a plugin id (elastic#53759) [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069) Remove nested root from index pattern (elastic#54978) [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198) [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252) [SIEM] Detections add alert & signal tab (elastic#55127) Management API - redirect on disabled app path (elastic#55136) [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags update local (elastic#55177) ...

* upstream/master: [ML] Single Metric Viewer: Fix job check. (elastic#55191)

* master: (108 commits) [ML] Single Metric Viewer: Fix job check. (elastic#55191) Show error page when accessing unavailable app (elastic#54656) [ML] Improving job wizards with datafeed aggregations (elastic#55180) remove flaly assetion. a license presence tested anyway (elastic#55289) fix commonly used ranges uptime (elastic#54930) [SIEM] Use proper icons on Detections view (elastic#55215) Fix: invalid translation referenced (elastic#54901) [State Management] Remove AppState from edit_index_pattern page (elastic#54104) Implements `getStartServices` on server-side (elastic#55156) Move vis_vega_type/data_model tests to jest (elastic#55186) [SIEM] [Detection Engine] Update status on rule details page (elastic#55201) Fix KQL value suggestions for nested fields (elastic#54820) Enforce camelCase format for a plugin id (elastic#53759) [SIEM] Detection engine cleanup for rule details/creation/edit page (elastic#55069) Remove nested root from index pattern (elastic#54978) [Reporting/Migration] ReportingSetup, LegacySetup (elastic#54198) [SIEM] [Detection Engine] Fixes duplicate rule action (elastic#55252) [SIEM] Detections add alert & signal tab (elastic#55127) Management API - redirect on disabled app path (elastic#55136) [SIEM][Detection Engine] Fixes critical regression on the backend with immutable and tags ...

* master: [ML] Fixing missing job_type in job messages search (elastic#55330) [ML] Correctly pass on severity value to anomaly explorer charts. (elastic#55207)

* master: [State Management] remove AppState from Dashboard app (elastic#54105) Expose fatalErrors API from the Start contract (elastic#55300) [BUG] Data fetching twice on discover timefilter change (elastic#55279) [Mappings editor] Add missing max_shingle_size parameter to search_as_you_type (elastic#55161) [Logs UI] Fix z-index of logs page toolbar (elastic#54469) removes CTA from Task Manager info message (elastic#55334)

joshdover

Migration guide updates LGTM

YulNaumenko

LGTM

* master: (38 commits) [ML] Fix counters and percentages for array fields on the Data visualizer page (elastic#55209) [SIEM][Detection Engine] Tags being turned into null rules part deux (elastic#55507) [DOCS] Add tip for using elasticsearch-certutil http command (elastic#55357) [SIEM][Detection Engine] Critical blocker, fixes schema accepting values it should not (elastic#55488) [SIEM] Detections create prepackage rules (elastic#55403) [Reporting] Convert CSV Export libs to Typescript (elastic#55117) [Maps] show field type icons in data driven styling field select (elastic#55166) Adds event log for actions and alerting (elastic#45081) [SIEM][Detection Engine] Fixes critical blocker where signals on signals are not operating [SIEM][Detection Engine] Critical blocker, adds need REST prefix for cloud remove incorrect config (elastic#55427) Retain pinned filters when loading and clearing saved queries (elastic#54307) Resolver zoom, pan, and center controls (elastic#55221) Skip failing endpoint saga tests [skip-ci] Update migration guide to add rendering service example (elastic#54744) [DOCS] Updates to heat map page (elastic#55097) [Endpoint] Fix saga to start only after store is created and stopped on app unmount (elastic#55245) [Logs UI] Use the correct icons and labels in the feature cont… (elastic#55292) [Uptime] Handle locations with names but no geo data (elastic#55234) ...

mikecote

I'll defer to @pmuellr and @YulNaumenko do to a full review but just added a few comments after scanning through.

I noticed in 4 places we require the task manager mock from legacy when I believe the NP task manager exposes the same mock we could use (x-pack/plugins/task_manager/server/task_manager.mock.ts).

x-pack/legacy/plugins/actions/index.ts

x-pack/plugins/actions/server/routes/update.test.ts

gmmorris

x-pack/plugins/task_manager/server/task_manager.mock.ts

That's weird, that was removed... somehow came back in merge conflict :/

pmuellr

LGTM, but noted a few things to change, and had some questions about some NP stuff. Thanks for all this grunt work!!!

pmuellr · 2020-01-22T12:45:54Z

x-pack/legacy/plugins/siem/server/lib/detection_engine/routes/__mocks__/_mock_server.ts

@@ -9,7 +9,8 @@ import { KibanaConfig } from 'src/legacy/server/kbn_server';
 import { ElasticsearchPlugin } from 'src/legacy/core_plugins/elasticsearch';
 import { savedObjectsClientMock } from '../../../../../../../../../src/core/server/mocks';
 import { alertsClientMock } from '../../../../../../alerting/server/alerts_client.mock';
-import { actionsClientMock } from '../../../../../../actions/server/actions_client.mock';
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths


That's unfortunate. Are you not supposed to go deeper into a NP plugin than server or something?

I'll need to check if we documented this, but we want to restrict all imports to either plugins/plugin/server/index.ts or plugins/plugin/server/mocks.ts

Docs say it should be from the root folder, so we should update that to make it more clear.

kibana/src/core/MIGRATION.md

Lines 1522 to 1525 in 856c85b

Although it isn't mandatory, we strongly recommended you export your plugin mocks as well, in order for dependent plugins to use them in tests. Your plugin mocks should be exported from the root `/server` and `/public` directories in your plugin:

```typescript

// my_plugin/server/mocks.ts or my_plugin/public/mocks.ts

Ah cool, I just didn't want to expose off of index testing purposes, but if mock.ts is good, then we'll do that.
Thanks

pmuellr · 2020-01-22T13:18:36Z

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

+  ): (request: KibanaRequest) => Services {
+    const { adminClient } = this;
+    return request => ({
+      callCluster: adminClient!.asScoped(request).callAsCurrentUser,


In event_log, I changed the usage of foo!.bar.baz() to

if (!foo.bar) throw new Error('nicer null reference message here'); foo.bar.baz()

If it's going to throw, might as well be a slightly nicer message, and only adds one line per undefineable.

My fear is that we're going to end up with dozens of these, and it's annoying because this would not be an issue if it wasn't for the use of a class.
Fields are always nullable if you don't set them at constructor level, but we can't do that because of the NP plugin API.

I'm thinking of forcing this via schema validation of all plugins, but that should probably be a separate PR.
Any thoughts?

huh, yeah, would be an interesting approach. We'd have some bag of all the potential nullables, pass them into a validator which returns the non-null versions? That might also get unwieldy, but seems like it's worth a try.

x-pack/plugins/actions/server/routes/_mock_handler_arguments.ts

x-pack/plugins/actions/server/routes/create.ts

pmuellr · 2020-01-22T14:10:18Z

x-pack/plugins/actions/server/routes/create.ts

+      req: KibanaRequest<any, any, TypeOf<typeof bodySchema>, any>,
+      res: KibanaResponseFactory
+    ): Promise<IKibanaResponse<any>> {
+      verifyApiAccess(licenseState);


I guess we used some "middleware" to do this before, that capability isn't available in NP? Or maybe the state we needed wasn't available where we used to need it? Unfortunate.

There doesn't seem to be this equivalent middleware in NP.
The state is the same as before, we did this verify manually at setup, now its done manually at the call site.

It's not ideal, but at least its more visible to maintainers now.

pmuellr · 2020-01-22T14:33:59Z

x-pack/plugins/actions/server/routes/find.ts

+  }),
+  search_fields: schema.maybe(schema.oneOf([schema.arrayOf(schema.string()), schema.string()])),
+  sort_field: schema.maybe(schema.string()),
+  has_reference: schema.maybe(


do we even need the maybe() here, on a nullable()? I guess we want maybe() long-term, the nullable() use is until it's fixed?

nullable lets you specify null in the type, which is another case to support.
Better to either have the key with a value, or not have the key at all, IMHO.

pmuellr · 2020-01-22T14:43:41Z

x-pack/plugins/actions/server/routes/get.test.ts

+      }
+    `);
+
+    expect(verifyApiAccess).toHaveBeenCalledWith(licenseState);


I don't think the test below this one - "ensures the license allows getting actions" is required, since the thing it's testing is this particular expect().

I prefer the explicit test and removing the redundant expect -test are just as much about documentation as about logical testing.

generally speaking, that's true. But it adds 30 lines to 5 test cases, so 150 more lines of tests that are technically redundant. I can go either way. Some day we'll spend some time and framework-ize some of this test boilerplate, will be less of an issue to me :-)

x-pack/plugins/actions/server/routes/update.test.ts

x-pack/plugins/actions/server/routes/list_action_types.test.ts

x-pack/plugins/actions/server/routes/update.test.ts

kibanamachine · 2020-01-22T16:58:44Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 2e16269

History

💚 Build #21817 succeeded 0dae5de
💚 Build #21583 succeeded 696ebcd
💚 Build #21536 succeeded 78e0253
💚 Build #21503 succeeded 1ba073b
💔 Build #21455 failed b21e216

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

Migrates the Actions plugin from Legacy on to the Kibana Platform.

gmmorris added 4 commits January 13, 2020 19:04

fixed snapshots broken by merge

f9e63f7

added allow and array.sparse to config schema package

d38db5b

gmmorris added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.7.0 Feature:Actions Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) labels Jan 16, 2020

gmmorris added 15 commits January 16, 2020 11:50

Revert "added allow and array.sparse to config schema package"

1c2df0a

This reverts commit d38db5b.

moved actions into Plugins but not wired up

aa72e9f

migrated actions into a NP plugin, still has issues though

4bfb25c

fixed unit tests around routes in actions

b244516

Merge remote-tracking branch 'upstream/master'

e50378d

* upstream/master: [ML] Single Metric Viewer: Fix job check. (elastic#55191)

fixed merge issue with service hnow

a69e1ee

Merge branch 'master' into actions/platform

774d3f0

* master: [ML] Fixing missing job_type in job messages search (elastic#55330) [ML] Correctly pass on severity value to anomaly explorer charts. (elastic#55207)

fixed i18n config

99fc277

moved security and spaces into optionalPlugins

b21e216

removed accidental change in alerting

1ba073b

cleaned up routes and plugin deps

6e1abea

gmmorris marked this pull request as ready for review January 21, 2020 12:18

gmmorris requested a review from a team as a code owner January 21, 2020 12:18

removed dep on security in actions

696ebcd

YulNaumenko self-requested a review January 21, 2020 15:28

joshdover approved these changes Jan 21, 2020

View reviewed changes

YulNaumenko approved these changes Jan 21, 2020

View reviewed changes

mikecote approved these changes Jan 22, 2020

View reviewed changes

x-pack/legacy/plugins/actions/index.ts Show resolved Hide resolved

x-pack/plugins/actions/server/routes/update.test.ts Outdated Show resolved Hide resolved

mikecote requested a review from pmuellr January 22, 2020 13:44

gmmorris commented Jan 22, 2020

View reviewed changes

gmmorris added 2 commits January 22, 2020 14:37

removed dead code

ad7c03b

brought back index

77f9d40

pmuellr approved these changes Jan 22, 2020

View reviewed changes

cleaned up actions

2e16269

gmmorris merged commit 838d7ba into elastic:master Jan 22, 2020

gmmorris added a commit to gmmorris/kibana that referenced this pull request Jan 22, 2020

migrate Actions to Kibana platform (elastic#55026)

1d891e0

Migrates the Actions plugin from Legacy on to the Kibana Platform.

gmmorris mentioned this pull request Jan 22, 2020

[7.x] migrate Actions to Kibana platform (#55026) #55586

Merged

gmmorris deleted the actions/platform branch January 22, 2020 17:13

mikecote mentioned this pull request Jan 22, 2020

Add xpack_main as a dependency in alerting test fixture plugins #55602

Merged

mikecote pushed a commit that referenced this pull request Jan 22, 2020

migrate Actions to Kibana platform (#55026) (#55586)

5fa45ee

Migrates the Actions plugin from Legacy on to the Kibana Platform.

gmmorris added the backported label Jan 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

migrate Actions to Kibana platform #55026

migrate Actions to Kibana platform #55026

gmmorris commented Jan 16, 2020 •

edited

Loading

elasticmachine commented Jan 16, 2020

joshdover left a comment

YulNaumenko left a comment

mikecote left a comment

gmmorris left a comment •

edited

Loading

pmuellr left a comment

pmuellr Jan 22, 2020

rudolf Jan 22, 2020

rudolf Jan 22, 2020

gmmorris Jan 22, 2020

pmuellr Jan 22, 2020

gmmorris Jan 22, 2020

pmuellr Jan 22, 2020

pmuellr Jan 22, 2020

gmmorris Jan 22, 2020

pmuellr Jan 22, 2020

gmmorris Jan 22, 2020

pmuellr Jan 22, 2020 •

edited

Loading

gmmorris Jan 22, 2020

pmuellr Jan 22, 2020

kibanamachine commented Jan 22, 2020

	Although it isn't mandatory, we strongly recommended you export your plugin mocks as well, in order for dependent plugins to use them in tests. Your plugin mocks should be exported from the root `/server` and `/public` directories in your plugin:

	```typescript
	// my_plugin/server/mocks.ts or my_plugin/public/mocks.ts

migrate Actions to Kibana platform #55026

migrate Actions to Kibana platform #55026

Conversation

gmmorris commented Jan 16, 2020 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Jan 16, 2020

joshdover left a comment

Choose a reason for hiding this comment

YulNaumenko left a comment

Choose a reason for hiding this comment

mikecote left a comment

Choose a reason for hiding this comment

gmmorris left a comment • edited Loading

Choose a reason for hiding this comment

pmuellr left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmuellr Jan 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kibanamachine commented Jan 22, 2020

💚 Build Succeeded

History

gmmorris commented Jan 16, 2020 •

edited

Loading

gmmorris left a comment •

edited

Loading

pmuellr Jan 22, 2020 •

edited

Loading