[Response Ops][Rules] Add New Rule Form to Stack Management #194655

JiaweiWu · 2024-10-02T05:33:34Z

Summary

Enables and adds the new rule form to stack management. We are only going to turn this on for stack management for now until we are confident that this is fairly bug free.

To test:

Switch USE_NEW_RULE_FORM_FEATURE_FLAG to true
Navigate to stack management -> rules list
Click "Create rule"
Assert the user is navigated to the new form
Create rule
Assert the user is navigated to the rule details page
Click "Edit"
Edit rule
Assert the user is navigated to the rule details page
Try editing a rule in the rules list and assert everything works as expected

We should also make sure this rule form is not enabled in other solutions.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

elasticmachine · 2024-10-02T05:33:37Z

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

cnasikas · 2024-10-07T09:48:05Z

@elasticmachine merge upstream

kibana-ci · 2024-10-07T11:45:53Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 3796e79

Failed CI Steps

x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group6.ts

Test Failures

[job] [logs] x-pack/test_serverless/functional/test_suites/observability/common_configs/config.group6.ts / discover/esql discover esql view ES|QL in Discover should render esql view correctly

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`alerting`	237	238	+1
`cases`	812	813	+1
`observability`	1061	1062	+1
`securitySolution`	5925	5926	+1
`triggersActionsUi`	798	844	+46
total			+50

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
`@kbn/rule-data-utils`	126	132	+6
`triggersActionsUi`	566	567	+1
total			+7

Async chunks

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

id	before	after	diff
`triggersActionsUi`	1.6MB	1.7MB	+62.4KB

Page load bundle

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

id	before	after	diff
`triggersActionsUi`	123.3KB	127.4KB	+4.1KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/rule-data-utils`	129	135	+6
`triggersActionsUi`	592	593	+1
total			+7

async chunk count

id	before	after	diff
`triggersActionsUi`	64	68	+4

ESLint disabled line counts

id	before	after	diff
`triggersActionsUi`	127	130	+3

Total ESLint disabled count

id	before	after	diff
`triggersActionsUi`	133	136	+3

History

💚 Build #238621 succeeded 9eb2840

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

cnasikas · 2024-10-08T06:53:22Z

packages/kbn-alerts-ui-shared/src/common/hooks/use_load_connectors.ts

@@ -27,6 +27,7 @@ export const useLoadConnectors = (props: UseLoadConnectorsProps) => {
  const { data, isLoading, isFetching, isInitialLoading } = useQuery({
    queryKey: ['useLoadConnectors', includeSystemActions],
    queryFn,
+    cacheTime: 0,


What is the purpose of changing the cache time (garbage collection) to zero? Will this change affect other parts of the app?

@JiaweiWu Any update on this?

should be fine, nothing else is using this

Yes, but why do we need to set it to 0? What does it do and what issue does it solve?

Ok, I spoke with @umbopepato, and help me understand. I made the hooks parameterized because they can be used in the future by other parts of the application and we may not want the caching to be set to zero.

jcger · 2024-10-09T08:02:57Z

Missing monitoring data

There are two options hidden that in the flyout show up the first time "save" is clicked. In the rules page the create button is disabled and these hidden options are not shown

Flyout recording showing how two options show when save is clicked

Screenshot.4.mov

JiaweiWu · 2024-10-10T02:02:24Z

@elasticmachine merge upstream

cnasikas · 2024-10-11T14:17:24Z

I can verify that all the bugs mentioned here #194655 (comment) are fixed. I found the following ones:

If I do not have permission to the "Rule Settings" the flapping section is being shown but the right part is empty. I would say to hide the whole section entirely. Wdyt?

If I do not have permission to "Actions" the modal appears to inform me about a rule with no actions. If I create and then edit I do not see it again. I think we should not show it as I cannot do anything as a user. Wdyt?

Some connectors are not available on the basic license. If there is a rule with connectors available on a greater license than basic and you are on basic you can see them but you cannot save the rule. In main the following is being shown:

and I can save the rule if I want. Should we do something similar on the new rule form?

On main if I press cancel or exit the flyout I get the following callout

Should we do something similar on the new rule form (cancel or return button)?

On basic license, if I hover on unsupported connectors a tooltip is being shown explaining the reasoning. This is not happening for rule types. Should we do the same? Probably unrelated to this PR.

On basic license, I can edit a rule that is not available on the basic. This leads to a bunch of errors in edit mode. The same is happening on main. Should we prevent accessing this kind of rule types (for another PR)?

yctercero

Codeowner changes packages/kbn-rule-data-utils/src/routes/stack_rule_paths.ts LGTM.

JiaweiWu · 2024-10-12T05:11:04Z

In my last commit (cb8f068 I addressed the following:

@jcger:

There are two options hidden that in the flyout show up the first time "save" is clicked. In the rules page the create button is disabled and these hidden options are not shown [Fixed]

@js-jankisalvi:

Log threshold rule type validation still didn't throw any error for edit rule and allowed to save. However I saw similar issue in old rule flyout as well. [This seems to be the existing behaviour, so I won't fix it here, but please create a ticket to fix later]

@adcoelho:

This was frustrating because there was no error in the form. [Fixed, added danger status to the step indicating there are errors]

@cnasikas:

If I do not have permission to the "Rule Settings" the flapping section is being shown but the right part is empty. I would say to hide the whole section entirely. Wdyt? [Fixed]
If I do not have permission to "Actions" the modal appears to inform me about a rule with no actions. If I create and then edit I do not see it again. I think we should not show it as I cannot do anything as a user. Wdyt? [Fixed]
Some connectors are not available on the basic license. If there is a rule with connectors available on a greater license than basic and you are on basic you can see them but you cannot save the rule. In main the following is being shown: [Fixed]

On main if I press cancel or exit the flyout I get the following callout [Fixed]
On basic license, if I hover on unsupported connectors a tooltip is being shown explaining the reasoning. This is not happening for rule types. Should we do the same? Probably unrelated to this PR. [Looked like an improvement for the rule type modal, I won't address it in this PR]
On basic license, I can edit a rule that is not available on the basic. This leads to a bunch of errors in edit mode. The same is happening on main. Should we prevent accessing this kind of rule types (for another PR)? [Happening in main, therefore won't address it in this PR]

js-jankisalvi · 2024-10-14T09:20:48Z

@js-jankisalvi:

Log threshold rule type validation still didn't throw any error for edit rule and allowed to save. However I saw similar issue in old rule flyout as well. [This seems to be the existing behaviour, so I won't fix it here, but please create a ticket to fix later]

Created issue

jcger

LGTM 🍠

maryam-saeidi

Changes LGTM!

I tested the issue that I mentioned after this commit, and I was able to see the page.

pheyos

x-pack/test_serverless/functional/config.base.ts changes LGTM

elasticmachine · 2024-10-15T09:15:41Z

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

[job] [logs] FTR Configs #103 / Serverless security API cloud_security_posture Intercept the usage API request sent by the metering background task manager Should intercept usage API request for CSPM

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`triggersActionsUi`	805	850	+45

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
`@kbn/alerts-ui-shared`	305	304	-1
`@kbn/rule-data-utils`	127	133	+6
`triggersActionsUi`	567	568	+1
total			+6

Async chunks

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

id	before	after	diff
`triggersActionsUi`	1.6MB	1.7MB	+81.9KB

Page load bundle

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

id	before	after	diff
`triggersActionsUi`	123.3KB	127.4KB	+4.1KB

Unknown metric groups

API count

id	before	after	diff
`@kbn/alerts-ui-shared`	321	320	-1
`@kbn/rule-data-utils`	130	136	+6
`triggersActionsUi`	593	594	+1
total			+6

async chunk count

id	before	after	diff
`triggersActionsUi`	64	66	+2

ESLint disabled line counts

id	before	after	diff
`@kbn/alerts-ui-shared`	7	8	+1
`triggersActionsUi`	127	130	+3
total			+4

Total ESLint disabled count

id	before	after	diff
`@kbn/alerts-ui-shared`	8	9	+1
`triggersActionsUi`	133	136	+3
total			+4

History

💔 Build #242270 failed 4fdf3a7
💚 Build #241914 succeeded 15a5427
💔 Build #241908 failed b570c70
💔 Build #241878 failed 70c8baa
💔 Build #241834 failed cb8f068

kibanamachine · 2024-10-15T10:18:16Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11344036469

kibanamachine · 2024-10-15T10:22:51Z

💔 All backports failed

Status	Branch	Result
❌	8.x	Backport failed because of merge conflicts You might need to backport the following PRs to 8.x: - [FTR] support custom native roles in serverless tests (#194677)

Manual backport

To create the backport manually run:

node scripts/backport --pr 194655

Questions ?

Please refer to the Backport tool documentation

cnasikas · 2024-10-15T12:07:49Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…194655) ## Summary Enables and adds the new rule form to stack management. We are only going to turn this on for stack management for now until we are confident that this is fairly bug free. ### To test: 1. Switch `USE_NEW_RULE_FORM_FEATURE_FLAG` to true 2. Navigate to stack management -> rules list 3. Click "Create rule" 4. Assert the user is navigated to the new form 5. Create rule 6. Assert the user is navigated to the rule details page 7. Click "Edit" 8. Edit rule 9. Assert the user is navigated to the rule details page 10. Try editing a rule in the rules list and assert everything works as expected We should also make sure this rule form is not enabled in other solutions. ### Checklist - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios --------- Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Christos Nasikas <[email protected]> (cherry picked from commit 5c2df63) # Conflicts: # x-pack/test_serverless/functional/config.base.ts

…94655) (#196290) # Backport This will backport the following commits from `main` to `8.x`: - [[Response Ops][Rules] Add New Rule Form to Stack Management (#194655)](#194655)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Jiawei Wu <[email protected]>

Add rule form to stack management

9eb2840

JiaweiWu added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) v9.0.0 backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) v8.16.0 labels Oct 2, 2024

JiaweiWu requested review from a team as code owners October 2, 2024 05:33