Skip to content

Commit

Permalink
[Security Solution][Detections] Related Integrations & Required Field…
Browse files Browse the repository at this point in the history
…s Feedback & Fixes (#133050)

## Summary

Addressing the following feedback from #132847:

- [X] On the Rule Management page package name is used instead of package title when the package has only 1 integration:
- [X] move integrations_popover to `related_integrations` directory
- [X] update useInstalledIntegrations to always return array of installedIntegration
- [X] move useInstalledIntegrations to `related_integrations` directory
- [X] Slight update to copy in Rules Table popover
- [X] Ok to use Rule Details UI within Rules Table popover content
- [X] Sort integrations alphabetically
- [X] Update left padding on version mis-match tooltip
- [X] #133291
- [X] #133269
- [X]  Add Kibana Advanced Setting for disabling integrations badge on Rules Table
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/171750790-ffa2e3ef-dd7a-499c-9b08-89bafc06dd50.png" />
</p>

- [ ]  Adds tests
  - [x] `useInstalledIntegrations` hook 
  - [X] relatedIntegrations utils
  - [x] IntegrationDescription
- [ ]  Add loaders where necessary since there can now be API delay
  - May hold off on loaders as transition from no installed integrations -> installed integrations isn't too bad as-is

##### Updated integrations popover content on Rules Table to match Rule Details design
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/172263941-3e948b41-7ef7-4281-8354-57e77ddeb433.png" />
</p>


In discussions with @banderror reviewing the different integration states (uninstalled, installed, enabled, and agents enrolled), we are now capturing the distinction between `Installed` and `Enabled` so that we don't confuse users when a package is installed but the integration isn't enabled/configured. I also added tooltips for clarifying each state and what action the user should perform to ensure compatibility. In collab with @yiyangliu9286 @jethr0null (comments below) -- we've consolidated to a single `Installed: enabled` badge, and updated `Uninstalled` to `Not installed` as well.


##### Tooltips

<details><summary>Not installed</summary>
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/172264210-00064485-2df9-408e-953b-9294f16dedf2.png" />
</p>
</details>





<details><summary>Installed</summary>
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/172263672-67b641cd-5895-464a-8897-f26bd0a61073.png" />
</p>
</details>




<details><summary>Installed: enabled</summary>
<p align="center">
  <img width="500" src="https://user-images.githubusercontent.com/2946766/172263783-563ea48d-c96c-4519-87b4-7076582f5da2.png" />
</p>
</details>



### 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
  - Collaborating with docs teams on this dedicated docs issue: elastic/security-docs#2015
- [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] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
  • Loading branch information
spong authored Jun 9, 2022
1 parent e09ff34 commit 7bfcb52
Show file tree
Hide file tree
Showing 23 changed files with 1,003 additions and 433 deletions.
8 changes: 7 additions & 1 deletion docs/management/advanced-options.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@ events.
[[securitysolution-threatindices]]`securitySolution:defaultThreatIndex`::
A comma-delimited list of Threat Intelligence indices from which the {security-app} collects indicators.

[[securitysolution-enableCcsWarning]]`securitySolution:enableCcsWarning`:: Enables
privilege check warnings in rules for CCS indices.

[[securitysolution-enablenewsfeed]]`securitySolution:enableNewsFeed`:: Enables
the security news feed on the Security *Overview* page.

Expand All @@ -464,7 +467,10 @@ The URL from which the security news feed content is retrieved.
The default refresh interval for the Security time filter, in milliseconds.

[[security-solution-rules-table-refresh]]`securitySolution:rulesTableRefresh`::
The default period of time in the Security time filter.
Enables auto refresh on the rules and monitoring tables, in milliseconds.

[[securitySolution-showRelatedIntegrations]]`securitySolution:showRelatedIntegrations`::
Shows related integrations on the rules and monitoring tables.

[[securitysolution-timedefaults]]`securitySolution:timeDefaults`::
The default period of time in the Security time filter.
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/security_solution/common/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,10 @@ export const IP_REPUTATION_LINKS_SETTING_DEFAULT = `[
{ "name": "talosIntelligence.com", "url_template": "https://talosintelligence.com/reputation_center/lookup?search={{ip}}" }
]`;

/** This Kibana Advanced Setting shows related integrations on the Rules Table */
export const SHOW_RELATED_INTEGRATIONS_SETTING =
'securitySolution:showRelatedIntegrations' as const;

/**
* Id for the notifications alerting type
* @deprecated Once we are confident all rules relying on side-car actions SO's have been migrated to SO references we should remove this function
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

export * from './error_schema';
export * from './get_installed_integrations_response_schema';
export * from './get_rule_execution_events_response';
export * from './import_rules_schema';
export * from './prepackaged_rules_schema';
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
} from '@elastic/eui';
import { ALERT_RISK_SCORE } from '@kbn/rule-data-utils';

import { castEsToKbnFieldTypeName } from '@kbn/field-types';

import { isEmpty } from 'lodash/fp';
import React from 'react';
import styled from 'styled-components';
Expand Down Expand Up @@ -556,7 +558,7 @@ export const buildRequiredFieldsDescription = (
label: string,
requiredFields: RequiredFieldArray
): ListItems[] => {
if (requiredFields == null) {
if (isEmpty(requiredFields)) {
return [];
}

Expand All @@ -569,7 +571,11 @@ export const buildRequiredFieldsDescription = (
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize={'xs'}>
<EuiFlexItem grow={false}>
<FieldIcon data-test-subj="field-type-icon" type={rF.type} />
<FieldIcon
data-test-subj="field-type-icon"
type={castEsToKbnFieldTypeName(rF.type)}
label={rF.type}
/>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<FieldTypeText grow={false} size={'s'}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import styled from 'styled-components';
import { ThreatMapping, Threats, Type } from '@kbn/securitysolution-io-ts-alerting-types';
import { DataViewBase, Filter, FilterStateStore } from '@kbn/es-query';
import { FilterManager } from '@kbn/data-plugin/public';
import { buildRelatedIntegrationsDescription } from './required_integrations_description';
import { buildRelatedIntegrationsDescription } from '../related_integrations/integrations_description';
import type {
RelatedIntegrationArray,
RequiredFieldArray,
Expand Down
Loading

0 comments on commit 7bfcb52

Please sign in to comment.