[Actions] Introduce the `isSystemAction` property to actions by cnasikas · Pull Request #160753 · elastic/kibana

cnasikas · 2023-06-28T14:01:29Z

Summary

This PR introduces the isSystemAction: boolean to the ActionResult type. It also, adds the same to the ActionType type. I added validation to verify that system actions cannot be registered at the moment. More PRs are going to follow to incrementally build the feature.

Related: #160367

Checklist

Delete any items that are not applicable to this PR.

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

elasticmachine · 2023-06-28T17:55:30Z

Pinging @elastic/response-ops (Team:ResponseOps)

cnasikas · 2023-06-29T07:03:35Z

@elasticmachine merge upstream

…ana into system_actions_registration

elasticmachine · 2023-06-29T08:45:12Z

Pinging @elastic/uptime (Team:uptime)

shahzad31 · 2023-06-29T08:49:17Z

x-pack/plugins/synthetics/public/apps/synthetics/state/settings/api.ts

      is_preconfigured: isPreconfigured,
      is_deprecated: isDeprecated,
      is_missing_secrets: isMissingSecrets,
+      is_system_action: isSystemAction,


is it possible to move this util to actions plugin? or reuse?

I think it is better if the action plugin exposes a function that does the API call and the transformation as we do in cases. I think the recommendation is to not call the APIs of other plugins directly but use a thin client. This way the team can do changes in the API without affecting other teams. This will also be helpful when versioning our endpoints. @pmuellr Is there any issue for that?

I'm not aware of explicit issues covering this - sounds like a nice addition to the client-side actions plugin. If I understand correctly. Open an issue?

Ok, I will open an issue. @shahzad31 What do you think of this approach (addressing it on another PR)?

Done #160902

kibana-ci · 2023-06-29T09:56:51Z

💛 Build succeeded, but was flaky

Failed CI Steps

Explore - Security Solution Tests #1

Test Failures

[job] [logs] Explore - Security Solution Tests #1 / Overflow items Network stats and tables Shows Hover actions for more items in the popover

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
`actions`	263	265	+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`stackConnectors`	440.7KB	440.7KB	+18.0B
`synthetics`	907.1KB	907.2KB	+36.0B
`triggersActionsUi`	1.4MB	1.4MB	+18.0B
`uptime`	518.8KB	518.8KB	+36.0B
total			+108.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
`triggersActionsUi`	49	50	+1

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`cases`	143.2KB	143.3KB	+25.0B
`triggersActionsUi`	86.7KB	86.8KB	+108.0B
total			+133.0B

Unknown metric groups

API count

id	before	after	diff
`actions`	268	270	+2

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	14	16	+2
`securitySolution`	413	417	+4
total			+6

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	15	17	+2
`securitySolution`	492	496	+4
total			+6

History

💔 Build #138998 failed a8e7143
💔 Build #138982 failed b070bd9
💔 Build #138973 failed 45e260503c20396d9f01be2e235ca27280fe1047
💔 Build #138943 failed 85ca6a4
💔 Build #138905 failed 774b389

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

cc @cnasikas

shahzad31

LGTM !!

Probably a follow up to discuss using client to fetch list of connector !!

shahzad31

LGTM !!

Probably a follow up to discuss using client to fetch list of connector !!

machadoum

security-threat-hunting-explore changes LGTM!

ymao1

LGTM! Verified upgrading existing connectors still loads, exporting/importing works and listing all connectors via the API shows the new property.

banderror · 2023-06-29T13:14:34Z

Hey @cnasikas, could you please provide links to product tickets or any other context for what this change is related to and what does isSystemAction property mean?

cnasikas · 2023-06-29T13:19:06Z

Hey @cnasikas, could you please provide links to product tickets or any other context for what this change is related to and what does isSystemAction property mean?

Hey @banderror! Sorry my mistake, I should have put them in the description. This is one of many PRs to support System (Response) actions in the actions' framework like OsQuery, Isolating a host, etc. Here is the issue #160367. At the bottom, you can find all related tickets.

pmuellr · 2023-06-29T14:33:31Z

x-pack/plugins/triggers_actions_ui/public/types.ts

+
+export type SystemAction = Omit<ActionConnectorProps<never, never>, 'config' | 'secrets'> & {
+  isSystemAction: true;
+  isPreconfigured: false;


If you imagine we end up changing isPreconfigured to isInMemory or similiar, we'd end up probably wanting isPreconfigured to be true, whenever isSystemAction is true. That way, we won't have to change any of our current isPreconfigured checks, that are typically to check if it's persisted as an SO or not.

I don't know how this will play out in the UI yet. I suspect that we would like to have the separation. I will keep your suggestion in mind when the time comes. In the backend, we should have the separation as we treat the preconfigured and the system actions a bit differently.

pmuellr

LGTM

banderror

Detection Rule Management changes LGTM 👍

Thanks for providing the context info, I read the #160367 ticket and have a few questions.

Do you or @patrykkopycinski already have a plan for migrating Response Actions from Security Solution to the Framework? Or is this on any team's roadmap? I saw #155644 but it doesn't have any version labels etc.
Is there any clarity around exporting/importing system actions? In Security Solution, we have our own Detection API endpoints for exporting and importing rules. With rules, we also export and import connectors, if any of the rules have actions related to them. Do you have any recommendations for how should we handle system actions in these endpoints? I'd assume we should skip exporting them and ignore them or return an error on import.

Here's a function that is called when the detection rule export endpoint gets the connectors to be exported:

kibana/x-pack/plugins/security_solution/server/lib/detection_engine/rule_management/logic/export/get_export_rule_action_connectors.ts

Lines 46 to 57 in df408bd

    
           const filterOutPredefinedActionConnectorsIds = async ( 
        
             actionsClient: ActionsClient, 
        
             actionsIdsToExport: string[] 
        
           ): Promise<string[]> => { 
        
             const allActions = await actionsClient.getAll(); 
        
             const predefinedActionsIds = allActions 
        
               .filter(({ isPreconfigured }) => isPreconfigured) 
        
               .map(({ id }) => id); 
        
             if (predefinedActionsIds.length) 
        
               return actionsIdsToExport.filter((id) => !predefinedActionsIds.includes(id)); 
        
             return actionsIdsToExport; 
        
           };

banderror · 2023-06-30T10:02:00Z

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

  config?: Config;
  isPreconfigured: boolean;
  isDeprecated: boolean;
+  isSystemAction: boolean;


It would be great to start documenting at least public interfaces that are exported from the Framework -- for us on the solution side it's often hard to navigate the Framework's code, but we need to do it from time to time.

Here we could add a JSDoc comment with a description of the field and a link to #160367.

Good point! I will do it in the following PRs as I will probably introduce more types.

cnasikas · 2023-06-30T11:17:37Z

Thank you @banderror! We target 8.10 for the system actions. The migration should be handled by the Security solution team. @patrykkopycinski and the team are aware of our work. We are working with @kobelb to find a seamless backward compatibility migration path. We will let you know about it soon.

Regarding, import/export we will not support it for system actions as they should always be available in every deployment (they are created on Kibana startup on the fly). I was not aware of this functionality in Security Solution. I will address it myself in the following PRs when I do the same for the import/export from the stack management. Thanks for letting me know!

cnasikas added 2 commits June 28, 2023 10:51

Add isSystemAction to ActionResult type and default it to false

1352e52

Disallow registering system actions

09b82f6

cnasikas added Feature:Actions Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// Feature:Actions/Framework Issues related to the Actions Framework labels Jun 28, 2023

cnasikas self-assigned this Jun 28, 2023

cnasikas added 3 commits June 28, 2023 18:18

Add test

dc1a2c8

Fix alerting types

bb7e9a5

Fix integration tests

774b389

cnasikas changed the title ~~[Actions] System actions registration~~ [Actions] Introduce the isSystemAction to actions Jun 28, 2023

cnasikas added 4 commits June 28, 2023 19:45

Fix triggers_actions_ui types

83ee747

Fix cases types

d2b8e67

Fix cases tests

31f1f39

Fix integration tests

85ca6a4

cnasikas marked this pull request as ready for review June 28, 2023 17:55

cnasikas requested a review from a team as a code owner June 28, 2023 17:55

cnasikas requested review from pmuellr and ymao1 June 28, 2023 17:55

cnasikas added release_note:skip Skip the PR/issue when compiling release notes v8.1.0 v8.10.0 and removed v8.1.0 labels Jun 28, 2023

cnasikas changed the title ~~[Actions] Introduce the isSystemAction to actions~~ [Actions] Introduce the isSystemAction property to actions Jun 28, 2023

Fix cases tests

b070bd9

cnasikas force-pushed the system_actions_registration branch from 45e2605 to b070bd9 Compare June 28, 2023 21:28

kibanamachine and others added 3 commits June 29, 2023 03:03

Merge branch 'main' into system_actions_registration

a8e7143

Fix types from other plugins

b50a3e0

Merge branch 'system_actions_registration' of github.com:cnasikas/kib…

ee5df7b

…ana into system_actions_registration

cnasikas requested review from a team as code owners June 29, 2023 08:44

cnasikas requested a review from xcrzx June 29, 2023 08:44

botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Jun 29, 2023

shahzad31 reviewed Jun 29, 2023

View reviewed changes

shahzad31 approved these changes Jun 29, 2023

View reviewed changes

machadoum approved these changes Jun 29, 2023

View reviewed changes

banderror requested review from banderror and removed request for xcrzx June 29, 2023 13:11

ymao1 approved these changes Jun 29, 2023

View reviewed changes

cnasikas mentioned this pull request Jun 29, 2023

[Actions] System actions #160367

Closed

15 tasks

pmuellr reviewed Jun 29, 2023

View reviewed changes

pmuellr approved these changes Jun 29, 2023

View reviewed changes

banderror approved these changes Jun 30, 2023

View reviewed changes

cnasikas merged commit 3a6d710 into elastic:main Jun 30, 2023

cnasikas deleted the system_actions_registration branch June 30, 2023 11:19

kibanamachine added the backport:skip This PR does not require backporting label Jun 30, 2023

cnasikas mentioned this pull request Jul 3, 2023

[Actions] Set system actions on Kibana start #160983

Merged

2 tasks

	const filterOutPredefinedActionConnectorsIds = async (
	actionsClient: ActionsClient,
	actionsIdsToExport: string[]
	): Promise<string[]> => {
	const allActions = await actionsClient.getAll();
	const predefinedActionsIds = allActions
	.filter(({ isPreconfigured }) => isPreconfigured)
	.map(({ id }) => id);
	if (predefinedActionsIds.length)
	return actionsIdsToExport.filter((id) => !predefinedActionsIds.includes(id));
	return actionsIdsToExport;
	};

Conversation

cnasikas commented Jun 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

elasticmachine commented Jun 28, 2023

Uh oh!

cnasikas commented Jun 29, 2023

Uh oh!

elasticmachine commented Jun 29, 2023

Uh oh!

shahzad31 Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

pmuellr Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Jun 29, 2023 • edited by pmuellr Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kibana-ci commented Jun 29, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

Metrics [docs]

Public APIs missing comments

Async chunks

Public APIs missing exports

Page load bundle

API count

ESLint disabled line counts

Total ESLint disabled count

History

Uh oh!

shahzad31 left a comment

Choose a reason for hiding this comment

Uh oh!

shahzad31 left a comment

Choose a reason for hiding this comment

Uh oh!

machadoum left a comment

Choose a reason for hiding this comment

Uh oh!

ymao1 left a comment

Choose a reason for hiding this comment

Uh oh!

banderror commented Jun 29, 2023

Uh oh!

cnasikas commented Jun 29, 2023

Uh oh!

pmuellr Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas Jun 29, 2023

Choose a reason for hiding this comment

Uh oh!

pmuellr left a comment

Choose a reason for hiding this comment

Uh oh!

banderror left a comment

Choose a reason for hiding this comment

Uh oh!

banderror Jun 30, 2023 • 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!

cnasikas Jun 30, 2023

Choose a reason for hiding this comment

Uh oh!

cnasikas commented Jun 30, 2023

Uh oh!

cnasikas commented Jun 28, 2023 •

edited

Loading

cnasikas Jun 29, 2023 •

edited by pmuellr

Loading

banderror Jun 30, 2023 •

edited

Loading