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] New Rules Installation and Upgrade UI Workflows #158450

Merged
merged 27 commits into from
Jun 14, 2023
Merged

[Security Solution] New Rules Installation and Upgrade UI Workflows #158450

merged 27 commits into from
Jun 14, 2023

Conversation

jpdjere
Copy link
Contributor

@jpdjere jpdjere commented May 25, 2023 •
edited
Loading

Addresses: #154614 #154615

Figma designs: https://www.figma.com/file/gLHm8LpTtSkAUQHrkG3RHU/%5B8.7%5D-%5BRules%5D-Rule-Immutability%2FCustomization?type=design&node-id=2935-577576&t=ziqgnlEJBpowqa7F-0

Summary

  • Removes prebuiltRulesNewUpgradeAndInstallationWorkflowsEnabled feature flag. All new prebuilt endpoints now available by default.
  • Creates the UI for the new rules installation and rules upgrade workflows.
    • Creates new Add Rules page, which lists rules available for installation.
    • Creates new Rule Updates page, which lists rules which have available updates.
    • Creates new, separate contexts for the Add Rules and the Rule Updates page, and the hooks to use them (useAddPrebuiltRulesTableContext and useUpgradePrebuiltRulesTableContext respectively)
    • Creates prebuilt rule hooks, which consume new endpoints:
      • useFetchPrebuiltRulesStatusQuery and usePrebuiltRulesStatus consume the /internal/detection_engine/prebuilt_rules/status endpoint and provide information about number of rules available for installation, number of installed rules, and number of rules with available updates.
      • useFetchPrebuiltRulesInstallReviewQuery and usePrebuiltRulesInstallReview consume the /internal/detection_engine/prebuilt_rules/installation/_review endpoint and return the rules available for installation which are listed in the Add Rules page.
      • useFetchPrebuiltRulesUpgradeReviewQuery and usePrebuiltRulesUpgradeReview consume the /internal/detection_engine/prebuilt_rules/upgrade/_review endpoint and return the rules which have available updates, and are listed in the Rule Updates page.
      • usePerformInstallAllRules, usePerformInstallSpecificRules, and its respective mutation hooks usePerformAllRulesInstallMutation and usePerformSpecificRulesInstallMutation consume the /internal/detection_engine/prebuilt_rules/upgrade/_perform endpoint in order to install rules.
      • usePerformUpgradeAllRules, usePerformUpgradeSpecificRules and its respective mutation hooks usePerformAllRulesUpgradeMutation and usePerformSpecificRulesUpgradeMutation consume the /internal/detection_engine/prebuilt_rules/upgrade/_perform endpoint in order to upgrade rules.

Deprecated code

Hooks:

  • useCreatePrebuiltRulesMutation
  • useInstallPrePackagedRules
  • useCreatePrePackagedRules
  • usePrePackagedRulesInstallationStatus
  • usePrePackagedTimelinesInstallationStatus

Major points to resolve

  • Timeline templates installation: Since this PR stops using the /api/detection_engine/rules/prepackaged endpoint in favour of the new ones, we are not currently installing timeline templates. Serverside, we will need a new endpoint to install them separately from rules? In the UI, how would this still work: would they get installed in the background now? Or maybe have a new button for it somewhere?
  • ML Jobs warning: when updating rules, we currently have a wrapper to add confirmation modal for users who may be running older ML Jobs that would be overridden by updating their rules. This PR removes that code, but we'll need to reintroduce it for the cases of: upgrading single rules, upgrading a selection of rules, upgrading all rules.

Deviations from design

This PR includes a reduced scope to the final workflow shown in the Figma designs.

Most notably, in Milestone 2, to be released in 8.9, we did not build the flyout that, in the Add Rules page, shows the rule details when the user clicks on it, so the user can review it before installing. The same is true in the Rule Updates table, which does not allow, for now, reviewing the rules. In both cases, the user can only click in "Install Rule" and "Upgrade Rule".

There are other differences in the UI, for technical reasons:

  • Both for the Add Rules page and the Rule Updates table we decided to use EUI's InMemoryTable. Since the endpoint that return the data to populate both of these tables do not allow for sorting, filtering and paging, we decided to use the InMemoryTable for both cases, as all of those functions are handled out-of-the-box by the EUI component. The relatively low number of items that populate these tables means that we won't face significant performance issues. However, this meant a number of deviations from the designs:
    • Since filtering, sorting and pagination are handled by the table, the contexts for these table do not includes any internal state relating to these functions. This makes it hard to recreate the RuleUtilityBar for each of these components or make the existing one reusable. We therefore decided to leave the Utility Bar for the new two tables out of scope, and deviate from the design by moving the button that the user can click on o install or upgrade the selected rules to beside the "Install all" or "Upgrade all" buttons. This button is shown only when at least one rule of the table is selected.
    • The tags filter box that comes out-of-the-box with the InMemoryTable can only be positioned to the right of the search bar, instead of the left like we have in our main Installed Rules table. Also, clicking on the tabs adds the text to the search bar, and the box does not allow for negative selection of tabs (exclusion).
    • The search bar filters on keystroke rather than on Enter. This behaviour can be changed, but it feels more useful than the other behaviour for these new two tables.
    • The search bar filters by searching the user's input in any of the string properties of first order within the rule object. This means that the search bar can be used to look up rules according to their name, description, rule_id, etc (but not for example for MITRE techniques, which are an object.) This behaviour, however, is also customisable.
  • Neither the Add Rules table nor the Rule Updates tables display the Last updated column which is shown in the design. Since the original intent of the designers is to show when the rule asset (security-rule) was created or updated, this is information we don't currently have within the SO. After discussion with @ksevasilyeva and @ARWNightingale, we decided, for now, to remove the column. In the meantime, @terrancedejesus created an issue to include createdAt and updatedAt fields within the rule assets, that we can use to display in the table in later iterations.

Other remaining work:

  • Introduce confirmation modals when the user clicks on the "Install all" or "Upgrade all" modal.
  • Unit testing for new hooks and components.
  • Other component redesign: Rule Filter, Tag Filter

How to test rule upgrade

  1. Have at least one rule installed
  2. Find its rule_id from the Network tab.
  3. Make a request to PATCH /api/detection_engine/rules with the rule_id in the payload, and also set the version to a number lower than the current version.
  4. Reload the page.
  5. The /upgrade/_review endpoint will now return that rule as available for upgrade.

Videos

Rule Installation Workflow

New.Rule.Installation.Workflow.mp4

Rule Upgrade Workflow

New.Role.Upgrade.Workflow.mp4

TODO

@jpdjere jpdjere force-pushed the new-installation-upgrade-rules-ui branch 2 times, most recently from 2835282 to d6e2387 Compare May 26, 2023 08:31
@jpdjere jpdjere self-assigned this May 26, 2023
@jpdjere jpdjere added the ci:no-auto-commit Disable auto-committing changes on CI label May 26, 2023
@jpdjere jpdjere force-pushed the new-installation-upgrade-rules-ui branch 2 times, most recently from c4573e4 to 9c5a781 Compare May 26, 2023 12:44
jpdjere
jpdjere commented Jun 1, 2023
View reviewed changes
...ver/lib/detection_engine/prebuilt_rules/api/review_rule_upgrade/review_rule_upgrade_route.ts
@@ -92,6 +92,7 @@ const calculateRuleInfos = (results: CalculateRuleDiffResult[]): RuleUpgradeInfo
return {
id: installedCurrentVersion.id,
rule_id: installedCurrentVersion.rule_id,
revision: installedCurrentVersion.revision,
Copy link
Contributor Author

@jpdjere jpdjere Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@xcrzx The RuleUpgradeSpecifier expects to be sent the current revision of the rule, but the response payload of upgrade _review as it was did not return it. I added it here so it can be resent to the backend on rule upgrade _perform.
Does this make sense to you? Or did I misunderstand the OCC mechanism?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yup, that's correct. I overlooked the revision in my previous PR. Thank you for adding it, @jpdjere. 👍

@jpdjere jpdjere force-pushed the new-installation-upgrade-rules-ui branch from c0c59c2 to e63db16 Compare June 2, 2023 14:19
@jpdjere jpdjere changed the title [Security Solution] New Rules Installation and Upgrade Workflows [Security Solution] New Rules Installation and Upgrade UI Workflows Jun 2, 2023
@jpdjere jpdjere removed the ci:no-auto-commit Disable auto-committing changes on CI label Jun 2, 2023
@jpdjere jpdjere force-pushed the new-installation-upgrade-rules-ui branch from 0d92ba0 to fd43956 Compare June 2, 2023 17:42
@jpdjere jpdjere requested a review from xcrzx June 2, 2023 17:43
@jpdjere jpdjere marked this pull request as ready for review June 2, 2023 17:43
@jpdjere jpdjere requested review from a team as code owners June 2, 2023 17:43
@jpdjere jpdjere added Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Detection Rule Management Security Detection Rule Management Team 8.9 candidate v8.9.0 labels Jun 2, 2023
@elasticmachine
Copy link
Contributor

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

@jpdjere jpdjere added release_note:enhancement backport:skip This commit does not require backporting Feature:Rule Management Security Solution Detection Rule Management Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules Team:Detections and Resp Security Detection Response Team labels Jun 2, 2023
@elasticmachine
Copy link
Contributor

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

@xcrzx xcrzx force-pushed the new-installation-upgrade-rules-ui branch from 1a9b099 to 27a5ed3 Compare June 13, 2023 13:01
@xcrzx
Copy link
Contributor

xcrzx commented Jun 13, 2023

Files by Code Owner

elastic/security-defend-workflows

  • x-pack/plugins/security_solution/public/management/links.ts

elastic/security-detection-engine

  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/bulk_edit_rules_actions.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/export_rule.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/prebuilt_rules.cy.ts
  • x-pack/plugins/security_solution/cypress/e2e/detection_rules/rules_selection.cy.ts

elastic/security-threat-hunting

  • x-pack/plugins/security_solution/server/routes/index.ts

elastic/security-threat-hunting-explore

  • x-pack/plugins/security_solution/public/common/components/navigation/breadcrumbs/index.ts
  • x-pack/plugins/security_solution/public/common/components/navigation/tab_navigation/tab_navigation.tsx

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
securitySolution 4104 4121 +17

Async chunks

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

id before after diff
securitySolution 10.8MB 10.8MB +21.2KB

Page load bundle

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

id before after diff
securitySolution 51.2KB 51.2KB +22.0B
Unknown metric groups

ESLint disabled line counts

id before after diff
enterpriseSearch 16 18 +2
securitySolution 409 413 +4
total +6

Total ESLint disabled count

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

History

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

cc @jpdjere

e40pud
e40pud approved these changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@e40pud e40pud left a comment

Choose a reason for hiding this comment

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

Detection engine changes LGTM

@xcrzx xcrzx merged commit 17b5fd3 into elastic:main Jun 14, 2023
This was referenced Jun 14, 2023
Closed
Closed
@jpdjere jpdjere mentioned this pull request Jun 14, 2023
Merged
9 tasks
@xcrzx xcrzx mentioned this pull request Jun 15, 2023
Merged
jpdjere pushed a commit that referenced this pull request Jun 16, 2023
@xcrzx
[Security Solution] Rule installation/upgrade error handling fixes (#…
a8ef2c8 
…159823)

**Related to: #158450

## Summary

Various fixes to the rule installation and upgrade workflows:
- Properly reset rule selection and loading state when API calls fail
- Improve copy and message formatting
@jpdjere jpdjere mentioned this pull request Jun 16, 2023
Merged
5 tasks
jpdjere added a commit that referenced this pull request Jun 19, 2023
@jpdjere
[Security Solution] Replace EUI filtering with custom in-memory filte…
2f0c12a 
…ring in Add Rules and Rule Upgrade tables (#159700)

## Summary

**1.** Replaces the EUI out-of-the-box filtering by rule name and tags
[used in the initial
implementation](#158450) with
custom in-memory filtering.

This aligns the look-and-feel of the Rules Management table with the new
Add Elastic Rules and Rule Upgrades table


![image](https://github.com/elastic/kibana/assets/5354282/e4b01221-74c1-40e5-abf4-87344a080e5d)


![image](https://github.com/elastic/kibana/assets/5354282/9684cee2-a2bf-4850-82e0-1d3679c55c99)


**2.** Adds a CTA in the Add Elastic RUles table when all rules have
been installed, that navigates the user back to the Rules Management
table.


![image](https://github.com/elastic/kibana/assets/5354282/15825af2-005d-47c8-a2a6-97603ea32646)



### Checklist

Delete any items that are not applicable to this PR.

- [ ] 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)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [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
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)




### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@banderror banderror mentioned this pull request Jun 20, 2023
Closed
jpdjere added a commit that referenced this pull request Jun 20, 2023
@jpdjere
[Security Solution] Reintroduce ML Jobs warning popover on rule upgra…
21dbbd0 
…de (#159868)

## Summary

Reintroduces ML Jobs warning popover that was removed during [new
install/upgrade initial
implementation.](#158450)

ML Jobs warning popover that appears when user has legacy ML jobs
installed, and the user attempts to update their prebuilt rules.

Modified behaviour so that the popover now appears in any of the three
cases: upgrading **all rules**, upgrading **specific rules** and
upgrading **a single rule**.

### Testing

In the `state` returned by `UpgradePrebuiltRulesTableContextProvider`,
replace the value of `legacyJobsInstalled` for a mock value. See below:
```ts
    return {
      state: {
        rules,
        tags,
        isFetched,
        isLoading: isLoading && loadingJobs,
        isRefetching,
        selectedRules,
        loadingRules,
        lastUpdated: dataUpdatedAt,
        legacyJobsInstalled: [
          {
            id: 'rc-rare-process-windows-5',
            description:
              'Looks for rare and anomalous processes on a Windows host. Requires process execution events from Sysmon.',
            groups: ['host'],
            processed_record_count: 8577,
            memory_status: 'ok',
            jobState: 'closed',
            hasDatafeed: true,
            datafeedId: 'datafeed-rc-rare-process-windows-5',
            datafeedIndices: ['winlogbeat-*'],
            datafeedState: 'stopped',
            latestTimestampMs: 1561402325194,
            earliestTimestampMs: 1554327458406,
            isSingleMetricViewerJob: true,
            awaitingNodeAssignment: false,
            jobTags: {},
            bucketSpanSeconds: 900,
          },
          {
            id: 'siem-api-rare_process_linux_ecs',
            description: 'SIEM Auditbeat: Detect unusually rare processes on Linux (beta)',
            groups: ['siem'],
            processed_record_count: 582251,
            memory_status: 'hard_limit',
            jobState: 'closed',
            hasDatafeed: true,
            datafeedId: 'datafeed-siem-api-rare_process_linux_ecs',
            datafeedIndices: ['auditbeat-*'],
            datafeedState: 'stopped',
            latestTimestampMs: 1557434782207,
            earliestTimestampMs: 1557353420495,
            isSingleMetricViewerJob: true,
            awaitingNodeAssignment: false,
            jobTags: {},
            bucketSpanSeconds: 900,
          },
        ],
        isUpgradeModalVisible,
        ruleIdToUpgrade,
        modalConfirmationUpdateMethod,
      },
      actions,
    };
```

### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)




### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
@xcrzx xcrzx mentioned this pull request Jun 28, 2023
Closed
@yuliacech yuliacech mentioned this pull request Oct 31, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xcrzx xcrzx xcrzx approved these changes

@semd semd semd approved these changes

@joeypoon joeypoon joeypoon approved these changes

@patrykkopycinski patrykkopycinski patrykkopycinski approved these changes

@e40pud e40pud e40pud approved these changes

Assignees

@jpdjere jpdjere

Labels
8.9 candidate backport:skip This commit does not require backporting Feature:Prebuilt Detection Rules Security Solution Prebuilt Detection Rules Feature:Rule Management Security Solution Detection Rule Management release_note:enhancement 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. v8.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jpdjere @elasticmachine @xcrzx @kibana-ci @e40pud @patrykkopycinski @joeypoon @semd