Skip to content

Commit

Permalink
[Cases] Case action enhancements and fixes (#180758)
Browse files Browse the repository at this point in the history
## Summary

In this PR:

- Address @adcoelho comments regarding documentation.
- Fix @js-jankisalvi bug about unsupported consumers
(#168369 (review)).
- Address @shanisagiv1 feedback regarding the title and the description.
Specifically:
- The title changed to "<rule_name> - Grouping by <grouping_by_value>
(Auto-created)".
- The description changed to "This case was created by the Case action
in <rule_name_link>. The assigned alerts are grouped by
<grouping_by_key>:<grouping_by_value>".
- Add the grouping key as a tag.

<img width="2289" alt="Screenshot 2024-04-13 at 4 41 36 PM"
src="https://github.com/elastic/kibana/assets/7871006/63e17947-5f39-4437-820b-7c69f42bfbe3">

The issue about the "Unknown" user will be fixed in another PR.

About @adcoelho bug:


https://github.com/elastic/kibana/assets/7871006/c46aa7c4-9d1a-475b-9d07-6bdff3ef00c8

I think it is fine to leave it as it is because a) the value will not be
saved even if they are added b) an error is being shown c) the only way
to do it properly is to validate while the user is typing which is going
to lead to bad UX. If you feel otherwise let me know.

### 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

### For maintainers

- [x] 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)
  • Loading branch information
cnasikas authored Apr 16, 2024
1 parent 8f6c04d commit e9266f3
Show file tree
Hide file tree
Showing 18 changed files with 457 additions and 293 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -684,7 +684,7 @@ async function getUpdatedAttributesFromOperations<Params extends RuleParams>({
actionsAuthorization: context.actionsAuthorization,
connectorAdapterRegistry: context.connectorAdapterRegistry,
systemActions: genSystemActions,
rule: { consumer: updatedRule.consumer },
rule: { consumer: updatedRule.consumer, producer: ruleType.producer },
});

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export async function createRule<Params extends RuleParams = never>(
actionsAuthorization: context.actionsAuthorization,
connectorAdapterRegistry: context.connectorAdapterRegistry,
systemActions: data.systemActions,
rule: { consumer: data.consumer },
rule: { consumer: data.consumer, producer: ruleType.producer },
})
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ async function updateWithOCC<Params extends RuleParams = never>(
actionsAuthorization: context.actionsAuthorization,
connectorAdapterRegistry: context.connectorAdapterRegistry,
systemActions: data.systemActions,
rule: { consumer: originalRuleSavedObject.attributes.consumer },
rule: { consumer: originalRuleSavedObject.attributes.consumer, producer: ruleType.producer },
});

// Throw error if schedule interval is less than the minimum and we are enforcing it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe('getSystemActionKibanaPrivileges', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
systemActions: [],
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual([]);
Expand All @@ -54,7 +54,7 @@ describe('getSystemActionKibanaPrivileges', () => {
it('should return an empty array if systemActions are not defined', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual([]);
Expand All @@ -64,7 +64,7 @@ describe('getSystemActionKibanaPrivileges', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
systemActions,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual(['my-priv:stackAlerts', 'my-priv-2:stackAlerts']);
Expand All @@ -74,7 +74,7 @@ describe('getSystemActionKibanaPrivileges', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
systemActions: [...systemActions, { id: 'my-id-2', actionTypeId: '.not-valid', params: {} }],
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual(['my-priv:stackAlerts', 'my-priv-2:stackAlerts']);
Expand All @@ -84,7 +84,7 @@ describe('getSystemActionKibanaPrivileges', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
systemActions: [...systemActions, { id: 'my-id-2', actionTypeId: '.no-priv', params: {} }],
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual(['my-priv:stackAlerts', 'my-priv-2:stackAlerts']);
Expand All @@ -94,7 +94,7 @@ describe('getSystemActionKibanaPrivileges', () => {
const privileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry: registry,
systemActions: [systemActions[0], systemActions[0]],
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(privileges).toEqual(['my-priv:stackAlerts']);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { ConnectorAdapterRegistry } from './connector_adapter_registry';

interface Args {
connectorAdapterRegistry: ConnectorAdapterRegistry;
rule: { consumer: string };
rule: { consumer: string; producer: string };
systemActions?: RuleSystemAction[];
}

Expand All @@ -22,7 +22,10 @@ export const getSystemActionKibanaPrivileges = ({
const kibanaPrivileges = systemActions
.filter((action) => connectorAdapterRegistry.has(action.actionTypeId))
.map((action) => connectorAdapterRegistry.get(action.actionTypeId))
.map((adapter) => adapter.getKibanaPrivileges?.({ consumer: rule.consumer }) ?? [])
.map(
(adapter) =>
adapter.getKibanaPrivileges?.({ consumer: rule.consumer, producer: rule.producer }) ?? []
)
.flat();

return Array.from(new Set(kibanaPrivileges));
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/alerting/server/connector_adapters/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ import { ObjectType } from '@kbn/config-schema';
import type { RuleTypeParams, SanitizedRule } from '../../common';
import { CombinedSummarizedAlerts } from '../types';

type Rule = Pick<SanitizedRule<RuleTypeParams>, 'id' | 'name' | 'tags' | 'consumer'>;
type Rule = Pick<SanitizedRule<RuleTypeParams>, 'id' | 'name' | 'tags' | 'consumer'> & {
producer: string;
};

export interface ConnectorAdapterParams {
[x: string]: unknown;
Expand Down Expand Up @@ -38,5 +40,5 @@ export interface ConnectorAdapter<
*/
ruleActionParamsSchema: ObjectType;
buildActionParams: (args: BuildActionParamsArgs<RuleActionParams>) => ConnectorParams;
getKibanaPrivileges?: ({ consumer }: { consumer: string }) => string[];
getKibanaPrivileges?: (args: { consumer: string; producer: string }) => string[];
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions: [],
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(res).toBe(undefined);
Expand Down Expand Up @@ -78,7 +78,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Action not-exist is not a system action"`);
});
Expand All @@ -103,7 +103,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Action not-exist is not a system action"`);
});
Expand All @@ -128,7 +128,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
})
).rejects.toThrowErrorMatchingInlineSnapshot(
`"Invalid system action params. System action type: .test - [foo]: expected value of type [string] but got [undefined]"`
Expand Down Expand Up @@ -161,7 +161,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
})
).rejects.toThrowErrorMatchingInlineSnapshot(`"Cannot use the same system action twice"`);
});
Expand Down Expand Up @@ -213,7 +213,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(res).toBe(undefined);
Expand Down Expand Up @@ -281,7 +281,7 @@ describe('validateAndAuthorizeSystemActions', () => {
systemActions,
actionsClient,
actionsAuthorization,
rule: { consumer: 'stackAlerts' },
rule: { consumer: 'stackAlerts', producer: 'alerts' },
});

expect(actionsAuthorization.ensureAuthorized).toBeCalledWith({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ interface Params {
actionsAuthorization: ActionsAuthorization;
connectorAdapterRegistry: ConnectorAdapterRegistry;
systemActions: Array<RuleSystemAction | NormalizedSystemAction>;
rule: { consumer: string };
rule: { consumer: string; producer: string };
}

export const validateAndAuthorizeSystemActions = async ({
Expand Down Expand Up @@ -73,7 +73,7 @@ export const validateAndAuthorizeSystemActions = async ({
const additionalPrivileges = getSystemActionKibanaPrivileges({
connectorAdapterRegistry,
systemActions: systemActionsWithActionTypeId,
rule: { consumer: rule.consumer },
rule: { consumer: rule.consumer, producer: rule.producer },
});

await actionsAuthorization.ensureAuthorized({ operation: 'execute', additionalPrivileges });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2474,6 +2474,7 @@ describe('Execution Handler', () => {
name: rule.name,
tags: rule.tags,
consumer: 'test-consumer',
producer: 'alerts',
},
ruleUrl:
'https://example.com/s/test1/app/management/insightsAndAlerting/triggersActions/rule/1',
Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/alerting/server/task_runner/execution_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ interface RunSystemActionArgs<Params extends RuleTypeParams> {
connectorAdapter: ConnectorAdapter;
summarizedAlerts: CombinedSummarizedAlerts;
rule: SanitizedRule<Params>;
ruleProducer: string;
spaceId: string;
bulkActions: EnqueueExecutionOptions[];
}
Expand Down Expand Up @@ -318,6 +319,7 @@ export class ExecutionHandler<
connectorAdapter,
summarizedAlerts,
rule: this.rule,
ruleProducer: this.ruleType.producer,
spaceId,
bulkActions,
});
Expand Down Expand Up @@ -453,13 +455,20 @@ export class ExecutionHandler<
connectorAdapter,
summarizedAlerts,
rule,
ruleProducer,
bulkActions,
}: RunSystemActionArgs<Params>): Promise<LogAction> {
const ruleUrl = this.buildRuleUrl(spaceId);

const connectorAdapterActionParams = connectorAdapter.buildActionParams({
alerts: summarizedAlerts,
rule: { id: rule.id, tags: rule.tags, name: rule.name, consumer: rule.consumer },
rule: {
id: rule.id,
tags: rule.tags,
name: rule.name,
consumer: rule.consumer,
producer: ruleProducer,
},
ruleUrl: ruleUrl?.absoluteUrl,
spaceId,
params: action.params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,6 @@ export const DAYS = (timeValue: string) =>
values: { timeValue },
});

export const YEARS = (timeValue: string) =>
i18n.translate('xpack.cases.systemActions.casesConnector.yearsLabel', {
defaultMessage: '{timeValue, plural, one {year} other {years}}',
values: { timeValue },
});

export const MONTHS = (timeValue: string) =>
i18n.translate('xpack.cases.systemActions.casesConnector.monthsLabel', {
defaultMessage: '{timeValue, plural, one {month} other {months}}',
values: { timeValue },
});

export const WEEKS = (timeValue: string) =>
i18n.translate('xpack.cases.systemActions.casesConnector.weeksLabel', {
defaultMessage: '{timeValue, plural, one {week} other {weeks}}',
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/cases/server/connectors/cases/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ top --> bottom

## Grouping

The case action accepts an array of alerts provided by the connector adapter. Duplicate alerts will not be attached to the same case. The case action groups the alerts by the grouping field configured by the user. For example, if the grouping field is `host.name` the case action will group the alerts by the values of the `host.name` field. Users may define more than one grouping field. In this case, the grouping will be done by multiple fields. The grouping is performed in memory by the case action as the number of alerts is expected to be low on average and they are already loaded in memory by the alerting framework.
The case action accepts an array of alerts provided by the connector adapter. Duplicate alerts will not be attached to the same case. The case action groups the alerts by the grouping field configured by the user. For example, if the grouping field is `host.name` the case action will group the alerts by the values of the `host.name` field. In this case, the grouping will be done by multiple fields. The grouping is performed in memory by the case action as the number of alerts is expected to be low on average and they are already loaded in memory by the alerting framework.

```mermaid
flowchart LR
Expand Down
Loading

0 comments on commit e9266f3

Please sign in to comment.