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.

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

[Rules migration] ES|QL query editing and validation in translation tab in the flyout (#11381) #203601

Merged

Conversation

e40pud
Copy link
Contributor

@e40pud e40pud commented Dec 10, 2024

Summary

Internal link to the feature details

These changes add a possibility to edit, validate and save custom migration rules:

  • There are new edit, save and cancel buttons in the translation tab of the details flyout for the non-installed custom rules
  • There is a new ES|QL query editing component in the translation tab which allows edit and validate the ES|QL query
  • On saving the ES|QL query the custom migration rule will be updated and based on the ES|QL validation a new translation_result might be set: full if query is valid, partial if query has syntax errors, untraslated if query is an empty string.

Screen recording

Screen.Recording.2024-12-10.at.16.01.10.mov

Other changes

Next fixes and adjustments were also implemented as part of this PR:

  • Error status in migration rules table to indicate whether the rule translation has been failed
  • Callouts inside the translation tab in details flyout
  • Updated Fully translated status title into Translated

Known issue

There is an issue with the autocompletion menu of the ES|QL query editor component. It is being shifted. It could be because we are using this component within the flyout and we might need to ask help from the team which takes care of it.

@e40pud e40pud added release_note:skip Skip the PR/issue when compiling release notes Team:Threat Hunting Security Solution Threat Hunting Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Dec 10, 2024
@e40pud e40pud requested a review from semd December 10, 2024 15:04
@e40pud e40pud self-assigned this Dec 10, 2024
@e40pud e40pud requested a review from a team as a code owner December 10, 2024 15:04
@elasticmachine
Copy link
Contributor

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

@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

return [
{ update: { _index: index, _id: id } },
{
doc: {
...rest,
elastic_rule: elasticRule,
translation_result:
translationResult ?? convertEsqlQueryToTranslationResult(elasticRule?.query),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here we update translation result based on the query. If query is not passed for update, we will not update translation_result. Also, translation_result will be used if passed as in UpdateRuleMigrationData object.

Comment on lines 94 to 98
const { data: { ruleMigrations } = { ruleMigrations: [] }, isLoading: isDataLoading } =
useGetMigrationRules({
migrationId,
ids: [migrationRuleId],
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to retrieve the migration data again, isn't it loaded in the table when we open the flyout? it would just be passed via props, right?

Copy link
Contributor Author

@e40pud e40pud Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, just overcomplicated things 😄 Will adjust it. As part of these changes, I exposed ids filter parameter, would you prefer to hide it?

import {
RuleOverviewTab,
useOverviewTabSections,
} from '../../../../detection_engine/rule_management/components/rule_details/rule_overview_tab';
import type { RuleResponse } from '../../../../../common/api/detection_engine/model/rule_schema';

import * as logicI18n from '../../logic/translations';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does logic mean? I see they are API related translations

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use logic to keep the wrapping hooks for API calls. Those hooks are responsible for any configurations and error handling of API calls. Like requests caching setup and showing error toast etc. This structure is used within detection & engine, thus I inherited it.

What is the preferred structure in Threat Hunting? If you think we can simplify things, by moving hooks closer to the APIs (inside api folder), we can adjust folders setup as well. I don't have a strong NO against that 😄

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of moving them within the API directory, they are hooks to interact with the API in the end. They don't have much logic inside 😄
Not need to do it now though, I plan to do a code consolidation refactor later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes indeed! I also like the SiemRulesMigrationsService and hooks laying next to it, so we could move my hooks there as well to have same approach

@e40pud e40pud requested a review from semd December 12, 2024 09:51
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 6363 6366 +3

Async chunks

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

id before after diff
securitySolution 14.7MB 14.7MB +6.6KB

History

cc @e40pud

@e40pud e40pud merged commit 668f776 into elastic:main Dec 12, 2024
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Dec 12, 2024
…ab in the flyout (elastic#11381) (elastic#203601)

## Summary

[Internal link](elastic/security-team#10820)
to the feature details

These changes add a possibility to edit, validate and save custom
migration rules:
* There are new `edit`, `save` and `cancel` buttons in the translation
tab of the details flyout for the non-installed custom rules
* There is a new ES|QL query editing component in the translation tab
which allows edit and validate the ES|QL query
* On saving the ES|QL query the custom migration rule will be updated
and based on the ES|QL validation a new `translation_result` might be
set: `full` if query is valid, `partial` if query has syntax errors,
`untraslated` if query is an empty string.

## Screen recording

https://github.com/user-attachments/assets/59cfc56f-3de8-4f7a-a2f9-79cb3fdee1c7

### Other changes

Next fixes and adjustments were also implemented as part of this PR:
* `Error` status in migration rules table to indicate whether the rule
translation has been failed
* Callouts inside the translation tab in details flyout
* Updated `Fully translated` status title into `Translated`

### Known issue

There is an issue with the autocompletion menu of the ES|QL query editor
component. It is being shifted. It could be because we are using this
component within the flyout and we might need to ask help from the team
which takes care of it.

(cherry picked from commit 668f776)
@kibanamachine
Copy link
Contributor

💚 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

kibanamachine added a commit that referenced this pull request Dec 12, 2024
…tion tab in the flyout (#11381) (#203601) (#204078)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Rules migration] ES|QL query editing and validation in translation
tab in the flyout (#11381)
(#203601)](#203601)

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

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

<!--BACKPORT [{"author":{"name":"Ievgen
Sorokopud","email":"ievgen.sorokopud@elastic.co"},"sourceCommit":{"committedDate":"2024-12-12T16:22:38Z","message":"[Rules
migration] ES|QL query editing and validation in translation tab in the
flyout (#11381) (#203601)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add a possibility to edit, validate
and save custom\r\nmigration rules:\r\n* There are new `edit`, `save`
and `cancel` buttons in the translation\r\ntab of the details flyout for
the non-installed custom rules\r\n* There is a new ES|QL query editing
component in the translation tab\r\nwhich allows edit and validate the
ES|QL query\r\n* On saving the ES|QL query the custom migration rule
will be updated\r\nand based on the ES|QL validation a new
`translation_result` might be\r\nset: `full` if query is valid,
`partial` if query has syntax errors,\r\n`untraslated` if query is an
empty string.\r\n\r\n## Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/59cfc56f-3de8-4f7a-a2f9-79cb3fdee1c7\r\n\r\n###
Other changes\r\n\r\nNext fixes and adjustments were also implemented as
part of this PR:\r\n* `Error` status in migration rules table to
indicate whether the rule\r\ntranslation has been failed\r\n* Callouts
inside the translation tab in details flyout\r\n* Updated `Fully
translated` status title into `Translated`\r\n\r\n### Known
issue\r\n\r\nThere is an issue with the autocompletion menu of the ES|QL
query editor\r\ncomponent. It is being shifted. It could be because we
are using this\r\ncomponent within the flyout and we might need to ask
help from the team\r\nwhich takes care of
it.","sha":"668f776583a60d35283a119fa007f7af9d1c57da","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","Team:Threat
Hunting","Team: SecuritySolution","backport:prev-minor"],"title":"[Rules
migration] ES|QL query editing and validation in translation tab in the
flyout
(#11381)","number":203601,"url":"https://github.com/elastic/kibana/pull/203601","mergeCommit":{"message":"[Rules
migration] ES|QL query editing and validation in translation tab in the
flyout (#11381) (#203601)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add a possibility to edit, validate
and save custom\r\nmigration rules:\r\n* There are new `edit`, `save`
and `cancel` buttons in the translation\r\ntab of the details flyout for
the non-installed custom rules\r\n* There is a new ES|QL query editing
component in the translation tab\r\nwhich allows edit and validate the
ES|QL query\r\n* On saving the ES|QL query the custom migration rule
will be updated\r\nand based on the ES|QL validation a new
`translation_result` might be\r\nset: `full` if query is valid,
`partial` if query has syntax errors,\r\n`untraslated` if query is an
empty string.\r\n\r\n## Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/59cfc56f-3de8-4f7a-a2f9-79cb3fdee1c7\r\n\r\n###
Other changes\r\n\r\nNext fixes and adjustments were also implemented as
part of this PR:\r\n* `Error` status in migration rules table to
indicate whether the rule\r\ntranslation has been failed\r\n* Callouts
inside the translation tab in details flyout\r\n* Updated `Fully
translated` status title into `Translated`\r\n\r\n### Known
issue\r\n\r\nThere is an issue with the autocompletion menu of the ES|QL
query editor\r\ncomponent. It is being shifted. It could be because we
are using this\r\ncomponent within the flyout and we might need to ask
help from the team\r\nwhich takes care of
it.","sha":"668f776583a60d35283a119fa007f7af9d1c57da"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/203601","number":203601,"mergeCommit":{"message":"[Rules
migration] ES|QL query editing and validation in translation tab in the
flyout (#11381) (#203601)\n\n## Summary\r\n\r\n[Internal
link](https://github.com/elastic/security-team/issues/10820)\r\nto the
feature details\r\n\r\nThese changes add a possibility to edit, validate
and save custom\r\nmigration rules:\r\n* There are new `edit`, `save`
and `cancel` buttons in the translation\r\ntab of the details flyout for
the non-installed custom rules\r\n* There is a new ES|QL query editing
component in the translation tab\r\nwhich allows edit and validate the
ES|QL query\r\n* On saving the ES|QL query the custom migration rule
will be updated\r\nand based on the ES|QL validation a new
`translation_result` might be\r\nset: `full` if query is valid,
`partial` if query has syntax errors,\r\n`untraslated` if query is an
empty string.\r\n\r\n## Screen
recording\r\n\r\n\r\nhttps://github.com/user-attachments/assets/59cfc56f-3de8-4f7a-a2f9-79cb3fdee1c7\r\n\r\n###
Other changes\r\n\r\nNext fixes and adjustments were also implemented as
part of this PR:\r\n* `Error` status in migration rules table to
indicate whether the rule\r\ntranslation has been failed\r\n* Callouts
inside the translation tab in details flyout\r\n* Updated `Fully
translated` status title into `Translated`\r\n\r\n### Known
issue\r\n\r\nThere is an issue with the autocompletion menu of the ES|QL
query editor\r\ncomponent. It is being shifted. It could be because we
are using this\r\ncomponent within the flyout and we might need to ask
help from the team\r\nwhich takes care of
it.","sha":"668f776583a60d35283a119fa007f7af9d1c57da"}}]}] BACKPORT-->

Co-authored-by: Ievgen Sorokopud <ievgen.sorokopud@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting Security Solution Threat Hunting Team v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants