Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Security Solution] Editing rules independently of source data #180407

Open
2 of 5 tasks
Tracked by #201502
banderror opened this issue Apr 9, 2024 Discussed in #178867 · 13 comments
Open
2 of 5 tasks
Tracked by #201502

[Security Solution] Editing rules independently of source data #180407

banderror opened this issue Apr 9, 2024 Discussed in #178867 · 13 comments
Labels
8.18 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Feature:Rule Creation Security Solution Detection Rule Creation workflow Feature:Rule Edit Security Solution Detection Rule Editing workflow Team:Detection Engine Security Solution Detection Engine Area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.

Comments

@banderror
Copy link
Contributor

banderror commented Apr 9, 2024 •
edited
Loading

Epics: https://github.com/elastic/security-team/issues/1974 (internal), #174168
Source of discussion: #178611

Summary

As part of the ongoing Prebuilt Rule Customization epic, the requirement to change the behaviour of rule validation on editing has come up.

Instead of blocking the editing of a rule when the rule's data source has not enough data for the query to work, the expected UX would only warn the user but continue to proceed with saving the rule.

However, such a change will have consequences on a number of features that depends on a rule's data source. We need to list them here, detail the consequences of such changes and find alternative behaviours where needed.

Please add any feature that might be impacted by this change, describing:

  • Which feature is impacted?
  • How is it impacted?
  • Is the impact acceptable from a UX point of view?
  • If not, what alternative behaviour could be desired?

Tasks
Preview Give feedback
  3. [Security Solution] Add validation error description on prebuilt rule editing #191832
    8.18 candidate Feature:Rule Edit Team: SecuritySolution Team:Detection Engine Team:Detection Rule Management Team:Detections and Resp needs design needs product v8.18.0
    ARWNightingale approksiu
  5. Editing rules independently of source data security-docs#5758
    Docset: ESS Docset: Serverless Team: Detections/Response enhancement v8.16.0
    joepeeples nastasha-solomon
@banderror banderror added Feature:Detection Rules Security Solution rules and Detection Engine Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Team:Detection Engine Security Solution Detection Engine Area labels Apr 9, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-rule-management (Team:Detection Rule Management)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detection-engine (Team:Detection Engine)

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-detections-response (Team:Detections and Resp)

@banderror banderror added Feature:Rule Creation Security Solution Detection Rule Creation workflow Feature:Rule Edit Security Solution Detection Rule Editing workflow triage_needed and removed Feature:Detection Rules Security Solution rules and Detection Engine labels Apr 9, 2024
@banderror banderror self-assigned this Apr 9, 2024
@banderror
Copy link
Contributor Author

banderror commented Apr 9, 2024 •
edited
Loading

@approksiu Could you please have a PM look at this discussion ticket + at the description and the whole thread of comments in #178611 -- when you have time? This is not urgent at all, this is just FYI that we have this problem to discuss and might want to make a decision in the next few months. I will assign you to both tickets and add needs product.

We'd like to understand potential risks of allowing users to save rules (when creating new rules or editing existing ones) when their queries or filters are syntactically valid, but the data they rely upon is missing (e.g. concrete indices with source events are missing) or invalid (e.g. certain fields used in queries are missing or have incorrect mappings).

Having a strict validation that validates source data (what we have now) or having a looser validation but allowing users to customize prebuilt rules (what we are suggesting to have with @jpdjere) is a tradeoff, and we'd like to better understand what we're potentially going to "trade" for a better prebuilt rule customization UX.

@jpdjere @yctercero @rylnd @marshallmain Let's move the general discussion from #178611 to here, and continue discussing any EQL validation specifics in #178611. Thank you for all your comments!

@banderror banderror mentioned this issue Apr 9, 2024
Closed
@banderror
Copy link
Contributor Author

@rylnd thank you, regarding your comments (one, two):

Required fields also doesn't (currently) allow this behavior, and I imagine other validations might also cause issue.

Actually, required fields are not currently editable in the app. @nikitaindik is working on adding support for editing them in #173594, this is work in progress.

In terms of this ticket: I think we need a little product/UX guidance on how we convey this situation to the user. It does look like the form hook lib has a concept of validation warnings, so we could potentially leverage that

@nikitaindik @dplumlee Could you please check that link?

As part of #173594 Nikita is working on implementing validation warnings for required fields instead of validation errors, and allowing the user to save a rule with required fields that don't necessarily exist. Also, @dplumlee is working on #173593 where we will also need some validation that generates warnings instead of errors.

@marshallmain
Copy link
Contributor

Having a strict validation that validates source data (what we have now) or having a looser validation but allowing users to customize prebuilt rules (what we are suggesting to have with @jpdjere) is a tradeoff, and we'd like to better understand what we're potentially going to "trade" for a better prebuilt rule customization UX.

With something like a confirmation modal described at the end of #178611 (comment), we could maybe get the best of both strict and loose validation. Users could save the rule with errors, but we would add an extra notice for users when they attempt to save the rule with errors or warnings that haven't been resolved.

@jpdjere jpdjere mentioned this issue Apr 10, 2024
Open
@banderror banderror removed their assignment Apr 13, 2024
@e40pud
Copy link
Contributor

e40pud commented Aug 2, 2024

Per our discussion

  1. EQL and ES|QL query syntax errors will be blocking and won't allow user to save a rule with such query errors.
  2. ES|QL rule query requires metadata operator with the _id field and will be blocking condition for user to be able to save a rule.
  3. Missing data source and/or field won't be a blocking error and will allow user to save a rule

cc @approksiu

@approksiu
Copy link

approksiu commented Aug 7, 2024 •
edited
Loading

Following up after additional research (details here)

  1. @e40pud please check that the other rule types (except ML) allow saving the rule with the edited data source.
  2. I want to allow users to change other data-source-dependent fields outside of the rule definition and save the rule independently of source data, I think this can be done later. Issue.

@e40pud e40pud mentioned this issue Aug 8, 2024
Merged
2 tasks
e40pud added a commit that referenced this issue Aug 15, 2024
@e40pud @elasticmachine
[Detections] EQL query validation: Separate syntax errors into a own …
2c5ae36 
…error type (#10181) (#190149)

## Summary

Addresses elastic/security-team#10181

This PR is a refactoring of EQL query validator:
* to separate different validation errors passed from ES. Before we
marked `parsing_exception`, `verification_exception` and
`mapping_exception` as the same error of type `ERR_INVALID_EQL`. After
these changes we will split these errors into separate ones: syntax
(`parsing_exception`), invalid EQL (`verification_exception` and
`mapping_exception`; can be split in future if needed)
* to handle missing data source as a new EQL error of type
`MISSING_DATA_SOURCE`. Before
`data.search.search<EqlSearchStrategyRequest,
EqlSearchStrategyResponse>()` call would throw an exception in case data
source does not exist and we would handle it as a failed request and
show an error toast (see relevant ticket
#178611). After these changes we
would not show a toast and handle missing data source error as other EQL
validation errors - showing an error message in the EQL query bar.

This will allow us to distinguish between different types of EQL
validation errors and will help to decide on whether certain errors are
blocking during the rule creation/editing flow
(#180407).

### Checklist

Delete any items that are not applicable to this PR.

- [x] [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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [Integration: Rule
execution](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6754)
(100 ESS & 100 Serverless)
- [Cypress: Detection
Engine](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6766)
(100 ESS & 100 Serverless)

---------

Co-authored-by: Elastic Machine <[email protected]>
e40pud added a commit to e40pud/kibana that referenced this issue Aug 27, 2024
@e40pud
[Security Solution] Editing rules independently of source data (elast…
010088d 
…ic#180407)
@e40pud e40pud mentioned this issue Aug 27, 2024
Merged
4 tasks
@e40pud
Copy link
Contributor

e40pud commented Aug 27, 2024 •
edited
Loading

@approksiu my changes are ready for review and I wanted to double check with you the wording inside confirmation modal that we are going to show to users:

Image

Also,

  1. Do we want different messages during rule creation and rule updating workflows? Right now same modal used.
  2. Do we want to cover this use case in documentation?

@approksiu
Copy link

approksiu commented Aug 28, 2024 •
edited
Loading

@e40pud I suggest to make the error message more clear, something like "No index matching index pattern ["non-existent-index"] defined in the rule was found."

@nastasha-solomon could you please check the modal wording?

Re questions:

  1. The same modal is fine for rule creation and editing workflows.
  2. It is good to cover this case in the docs to specify that the rule will fail if run on the system that is missing specified index patterns, and user can proceed if they intend to use the rule on the other system where data is present, or will setup indexes later.

This rule is attempting to query data from Elasticsearch indices listed in the "Index patterns" section of the rule definition, however no index matching: ["non-existent-index"] was found. This warning will continue to appear until a matching index is created or this rule is disabled.'

@approksiu
Copy link

After further discussion, we agreed to simplify the modal to mention validation errors, but list them out, as there can be multiple errors.

@e40pud
Copy link
Contributor

e40pud commented Aug 28, 2024

Here is the simplified version of the confirmation modal

Image

This was referenced Aug 30, 2024
Open
Open
e40pud added a commit that referenced this issue Aug 30, 2024
@e40pud @elasticmachine
@kibanamachine @vitaliidm
[Security Solution] Editing rules independently of source data (#180407…
32fb434 
…) (#191487)

## Summary

Addresses #180407
Addresses #178611

With these changes we allow user to create and update a rule even if
there are certain query bar validation error exist. Right now, we will
make any non-syntax validation errors in `EQL` and `ES|QL` rules types
to be non-blocking during the rule creation and rule updating workflows.

### Screenshot of the EQL rule creation workflow with existing
non-blocking validation errors:


https://github.com/user-attachments/assets/06b7f76c-e600-4a99-8ead-1445d429e9d3

### Screenshot of the EQL rule updating workflow with existing
non-blocking validation errors:


https://github.com/user-attachments/assets/9b35e113-b127-487b-bc23-afecf704db9d

## UPDATE

After discussing confirmation modal with @approksiu, we decided to
simplify it and show only title with generic description to avoid too be
too literal in the modal. User can see the full error description during
rule creation/editing workflows in the query bar where we show each
validation error as part of the query bar form item.

<img width="702" alt="Screenshot 2024-08-28 at 12 50 14"
src="https://github.com/user-attachments/assets/edfb791e-4e45-4fa5-8a46-c7e2772abdf9">

### Some test cases for local testing

<details>
  <summary><b>Create EQL rule with missing data source</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select EQL rule type
4. Set non-existing index in index patterns field
5. Add some valid EQL query (for example `any where true`)
6. Continue with other steps
7. Click create rule button

**Expected**: You will see the confirmation modal that warns user about
potentially failing rule executions. Clicking `Confirm` button will
create a rule.

</details>

<details>
  <summary><b>Create EQL rule with missing data field</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select EQL rule type
4. Set existing indices in index patterns field
5. Add some valid EQL query referring non-existing data field (for
example `any where agent.non_existing_field`)
6. Continue with other steps
7. Click create rule button

**Expected**: You will see the confirmation modal that warns user about
potentially failing rule executions. Clicking `Confirm` button will
create a rule.

</details>

<details>
<summary><b>Create EQL rule with syntax error in the query</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select EQL rule type
4. Set existing indices in index patterns field
5. Add some syntactically invalid EQL query (for example `hello world`)

**Expected**: The continue button does not allow user to proceed to the
About step due to existing syntax error.

</details>

<details>
  <summary><b>Create ES|QL rule with missing data source</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select ES|QL rule type
4. Add some valid ES|QL query with non-existing data source (for example
`from non-existing-index-* metadata _id, _version, _index | SORT
@timestamp`)
6. Continue with other steps
7. Click create rule button

**Expected**: You will see the confirmation modal that warns user about
potentially failing rule executions. Clicking `Confirm` button will
create a rule.

</details>

<details>
  <summary><b>Create ES|QL rule with missing data field</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select ES|QL rule type
4. Add some valid ES|QL query with non-existing data field (for example
`from logs-* metadata _id, _version, _index | SORT
agent.non_existing_field`)
6. Continue with other steps
7. Click create rule button

**Expected**: You will see the confirmation modal that warns user about
potentially failing rule executions. Clicking `Confirm` button will
create a rule.

</details>

<details>
<summary><b>Create ES|QL rule with syntax error in the
query</b></summary>

#### Steps:
1. Open rules management page
2. Click create new rule button
3. Select ES|QL rule type
4. Add some syntactically invalid ES|QL query (for example `hello
world`)

**Expected**: The continue button does not allow user to proceed to the
About step due to existing syntax error.

</details>

Same behaviour applies to the rule updating workflow. For example, you
can try to install one of the EQL or ES|QL rules that point to
non-existing data source or uses non-existing data field. User can still
update (add rule actions) to such installed pre-built rules.

### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
  * elastic/security-docs#5758
- [x] [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
- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
* [Detection Engine -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6831)
(100 ESS & 100 Serverless)
* [Rule Management -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6830)
(100 ESS & 100 Serverless)

---------

Co-authored-by: Elastic Machine <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Vitalii Dmyterko <[email protected]>
@e40pud e40pud mentioned this issue Sep 12, 2024
Merged
1 task
e40pud added a commit that referenced this issue Sep 13, 2024
@e40pud
[Security Solution] Add validation error description on prebuilt rule…
c937e95 
… editing (#191832) (#192683)

## Summary

Partially addressed #191832

With these changes:
- We revert to the
#180407 (comment).
Specifically, we return back the validation errors to the modal window.
An example of this modal is in the ticket description.
- Additionally, on the Rule Editing page and **only for prebuilt rules**
we: 1) hide the callout that says "You have an invalid input in this
tab: ...", and 2) we don't show the modal if there are any data
validation errors. We shouldn't show this modal and this callout until
we release the prebuilt rule customization feature. 3) We will only
validate the Actions tab.
- Fix MKI flaky cypress tests introduced in
#191487
([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main),
[2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main),
[3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)).
All three tests are failing due to missing
`[data-test-subj="eqlRuleType"]` element. After checking and comparing
my tests to other similar tests in the file, the only difference that
I've found was extra `login();` call. Thus removing those.

Here is the screen recording showing the new behaviour for prebuilt
rules. The has missing data source query validation error, though we do
not show it and allow user just to save the rule. Only Actions tab is
validated on rule save action.


https://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6


### Checklist

Delete any items that are not applicable to this PR.

- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
* [Detection Engine -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925)
(100 ESS & 100 Serverless)
* [Rule Management -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926)
(100 ESS & 100 Serverless)
* [Prebuilt Rules -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927)
(100 ESS & 100 Serverless)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this issue Sep 13, 2024
@e40pud
[Security Solution] Add validation error description on prebuilt rule…
217c9b1 
… editing (elastic#191832) (elastic#192683)

## Summary

Partially addressed elastic#191832

With these changes:
- We revert to the
elastic#180407 (comment).
Specifically, we return back the validation errors to the modal window.
An example of this modal is in the ticket description.
- Additionally, on the Rule Editing page and **only for prebuilt rules**
we: 1) hide the callout that says "You have an invalid input in this
tab: ...", and 2) we don't show the modal if there are any data
validation errors. We shouldn't show this modal and this callout until
we release the prebuilt rule customization feature. 3) We will only
validate the Actions tab.
- Fix MKI flaky cypress tests introduced in
elastic#191487
([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main),
[2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main),
[3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)).
All three tests are failing due to missing
`[data-test-subj="eqlRuleType"]` element. After checking and comparing
my tests to other similar tests in the file, the only difference that
I've found was extra `login();` call. Thus removing those.

Here is the screen recording showing the new behaviour for prebuilt
rules. The has missing data source query validation error, though we do
not show it and allow user just to save the rule. Only Actions tab is
validated on rule save action.

https://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6

### Checklist

Delete any items that are not applicable to this PR.

- [ ] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
* [Detection Engine -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925)
(100 ESS & 100 Serverless)
* [Rule Management -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926)
(100 ESS & 100 Serverless)
* [Prebuilt Rules -
Cypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927)
(100 ESS & 100 Serverless)

(cherry picked from commit c937e95)
kibanamachine added a commit that referenced this issue Sep 13, 2024
@kibanamachine @e40pud
[8.x] [Security Solution] Add validation error description on prebuil…
0457580 
…t rule editing (#191832) (#192683) (#192819)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security Solution] Add validation error description on prebuilt rule
editing (#191832)
(#192683)](#192683)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-13T08:37:39Z","message":"[Security
Solution] Add validation error description on prebuilt rule editing
(#191832) (#192683)\n\n## Summary\r\n\r\nPartially addressed
https://github.com/elastic/kibana/issues/191832\r\n\r\nWith these
changes:\r\n- We revert to
the\r\nhttps://github.com//issues/180407#issuecomment-2312891214.\r\nSpecifically,
we return back the validation errors to the modal window.\r\nAn example
of this modal is in the ticket description.\r\n- Additionally, on the
Rule Editing page and **only for prebuilt rules**\r\nwe: 1) hide the
callout that says \"You have an invalid input in this\r\ntab: ...\", and
2) we don't show the modal if there are any data\r\nvalidation errors.
We shouldn't show this modal and this callout until\r\nwe release the
prebuilt rule customization feature. 3) We will only\r\nvalidate the
Actions tab.\r\n- Fix MKI flaky cypress tests introduced
in\r\nhttps://github.com//pull/191487\r\n([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main),\r\n[2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main),\r\n[3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)).\r\nAll
three tests are failing due to
missing\r\n`[data-test-subj=\"eqlRuleType\"]` element. After checking
and comparing\r\nmy tests to other similar tests in the file, the only
difference that\r\nI've found was extra `login();` call. Thus removing
those.\r\n\r\nHere is the screen recording showing the new behaviour for
prebuilt\r\nrules. The has missing data source query validation error,
though we do\r\nnot show it and allow user just to save the rule. Only
Actions tab is\r\nvalidated on rule save
action.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n* [Detection Engine
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925)\r\n(100
ESS & 100 Serverless)\r\n* [Rule Management
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926)\r\n(100
ESS & 100 Serverless)\r\n* [Prebuilt Rules
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927)\r\n(100
ESS & 100
Serverless)","sha":"c937e95e3137821b510fa480ee28f0cf3afb85ad","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:
SecuritySolution","ci:cloud-deploy","Team:Detection
Engine","ci:project-deploy-security","v8.16.0"],"title":"[Security
Solution] Add validation error description on prebuilt rule editing
(#191832)","number":192683,"url":"https://github.com/elastic/kibana/pull/192683","mergeCommit":{"message":"[Security
Solution] Add validation error description on prebuilt rule editing
(#191832) (#192683)\n\n## Summary\r\n\r\nPartially addressed
https://github.com/elastic/kibana/issues/191832\r\n\r\nWith these
changes:\r\n- We revert to
the\r\nhttps://github.com//issues/180407#issuecomment-2312891214.\r\nSpecifically,
we return back the validation errors to the modal window.\r\nAn example
of this modal is in the ticket description.\r\n- Additionally, on the
Rule Editing page and **only for prebuilt rules**\r\nwe: 1) hide the
callout that says \"You have an invalid input in this\r\ntab: ...\", and
2) we don't show the modal if there are any data\r\nvalidation errors.
We shouldn't show this modal and this callout until\r\nwe release the
prebuilt rule customization feature. 3) We will only\r\nvalidate the
Actions tab.\r\n- Fix MKI flaky cypress tests introduced
in\r\nhttps://github.com//pull/191487\r\n([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main),\r\n[2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main),\r\n[3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)).\r\nAll
three tests are failing due to
missing\r\n`[data-test-subj=\"eqlRuleType\"]` element. After checking
and comparing\r\nmy tests to other similar tests in the file, the only
difference that\r\nI've found was extra `login();` call. Thus removing
those.\r\n\r\nHere is the screen recording showing the new behaviour for
prebuilt\r\nrules. The has missing data source query validation error,
though we do\r\nnot show it and allow user just to save the rule. Only
Actions tab is\r\nvalidated on rule save
action.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n* [Detection Engine
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925)\r\n(100
ESS & 100 Serverless)\r\n* [Rule Management
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926)\r\n(100
ESS & 100 Serverless)\r\n* [Prebuilt Rules
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927)\r\n(100
ESS & 100
Serverless)","sha":"c937e95e3137821b510fa480ee28f0cf3afb85ad"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/192683","number":192683,"mergeCommit":{"message":"[Security
Solution] Add validation error description on prebuilt rule editing
(#191832) (#192683)\n\n## Summary\r\n\r\nPartially addressed
https://github.com/elastic/kibana/issues/191832\r\n\r\nWith these
changes:\r\n- We revert to
the\r\nhttps://github.com//issues/180407#issuecomment-2312891214.\r\nSpecifically,
we return back the validation errors to the modal window.\r\nAn example
of this modal is in the ticket description.\r\n- Additionally, on the
Rule Editing page and **only for prebuilt rules**\r\nwe: 1) hide the
callout that says \"You have an invalid input in this\r\ntab: ...\", and
2) we don't show the modal if there are any data\r\nvalidation errors.
We shouldn't show this modal and this callout until\r\nwe release the
prebuilt rule customization feature. 3) We will only\r\nvalidate the
Actions tab.\r\n- Fix MKI flaky cypress tests introduced
in\r\nhttps://github.com//pull/191487\r\n([1](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/b1f442af-db44-8029-a9fb-7e3d988303b3?branch=main),\r\n[2](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/995655b6-ae70-86fd-b483-c65846cd8d66?branch=main),\r\n[3](https://buildkite.com/organizations/elastic/analytics/suites/serverless-mki-cypress-detection-engine/tests/02318f5c-6ca1-8779-a5a4-60f52a55b344?branch=main)).\r\nAll
three tests are failing due to
missing\r\n`[data-test-subj=\"eqlRuleType\"]` element. After checking
and comparing\r\nmy tests to other similar tests in the file, the only
difference that\r\nI've found was extra `login();` call. Thus removing
those.\r\n\r\nHere is the screen recording showing the new behaviour for
prebuilt\r\nrules. The has missing data source query validation error,
though we do\r\nnot show it and allow user just to save the rule. Only
Actions tab is\r\nvalidated on rule save
action.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/ce968f51-1a53-41b2-ad06-1b31dec085a6\r\n\r\n\r\n###
Checklist\r\n\r\nDelete any items that are not applicable to this
PR.\r\n\r\n- [ ] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n* [Detection Engine
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6925)\r\n(100
ESS & 100 Serverless)\r\n* [Rule Management
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6926)\r\n(100
ESS & 100 Serverless)\r\n* [Prebuilt Rules
-\r\nCypress](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/6927)\r\n(100
ESS & 100
Serverless)","sha":"c937e95e3137821b510fa480ee28f0cf3afb85ad"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Ievgen Sorokopud <[email protected]>
@yctercero yctercero mentioned this issue Sep 28, 2024
Closed
@maximpn maximpn mentioned this issue Nov 21, 2024
Open
@banderror banderror mentioned this issue Nov 23, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
8.18 candidate Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules area Feature:Rule Creation Security Solution Detection Rule Creation workflow Feature:Rule Edit Security Solution Detection Rule Editing workflow Team:Detection Engine Security Solution Detection Engine Area Team:Detection Rule Management Security Detection Rule Management Team Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@e40pud @banderror @elasticmachine @approksiu @marshallmain