Skip to content

Commit

Permalink
[8.10] [RAM] Type safety for RegistryRuleType (#164516) (#164590)
Browse files Browse the repository at this point in the history 
# Backport

This will backport the following commits from `main` to `8.10`:
- [[RAM] Type safety for RegistryRuleType
(#164516)](#164516)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Xavier
Mouligneau","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-08-23T13:32:34Z","message":"[RAM]
Type safety for RegistryRuleType (#164516)\n\n##
Summary\r\n\r\nRegisterRuleType was not representing its definition
correctly and the\r\ntype was not safe. So updated the code/type to
avoid missing attribute\r\nand functionality. Now summary alert is
back\r\n
\r\n\r\n![image](https://github.com/elastic/kibana/assets/189600/95cb786a-7ab9-4d40-a65c-ecbe59a0fbd3)\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"0d85af23a4992a4d37c0f69120cd46e840217e87","branchLabelMapping":{"^v8.11.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","impact:critical","Team:ResponseOps","v8.10.0","v8.11.0"],"number":164516,"url":"https://github.com/elastic/kibana/pull/164516","mergeCommit":{"message":"[RAM]
Type safety for RegistryRuleType (#164516)\n\n##
Summary\r\n\r\nRegisterRuleType was not representing its definition
correctly and the\r\ntype was not safe. So updated the code/type to
avoid missing attribute\r\nand functionality. Now summary alert is
back\r\n
\r\n\r\n![image](https://github.com/elastic/kibana/assets/189600/95cb786a-7ab9-4d40-a65c-ecbe59a0fbd3)\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"0d85af23a4992a4d37c0f69120cd46e840217e87"}},"sourceBranch":"main","suggestedTargetBranches":["8.10"],"targetPullRequestStates":[{"branch":"8.10","label":"v8.10.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.11.0","labelRegex":"^v8.11.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/164516","number":164516,"mergeCommit":{"message":"[RAM]
Type safety for RegistryRuleType (#164516)\n\n##
Summary\r\n\r\nRegisterRuleType was not representing its definition
correctly and the\r\ntype was not safe. So updated the code/type to
avoid missing attribute\r\nand functionality. Now summary alert is
back\r\n
\r\n\r\n![image](https://github.com/elastic/kibana/assets/189600/95cb786a-7ab9-4d40-a65c-ecbe59a0fbd3)\r\n\r\n\r\n###
Checklist\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"0d85af23a4992a4d37c0f69120cd46e840217e87"}}]}]
BACKPORT-->

Co-authored-by: Xavier Mouligneau <[email protected]>
  • Loading branch information
@kibanamachine @XavierM
kibanamachine and XavierM authored Aug 23, 2023
1 parent e1a7190 commit ea9ec1f
Show file tree
Hide file tree
Showing 12 changed files with 438 additions and 325 deletions.
630 changes: 333 additions & 297 deletions x-pack/plugins/alerting/server/authorization/alerting_authorization.test.ts
View file

Large diffs are not rendered by default.

8 changes: 2 additions & 6 deletions x-pack/plugins/alerting/server/authorization/alerting_authorization.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ interface HasPrivileges {
type AuthorizedConsumers = Record<string, HasPrivileges>;
export interface RegistryAlertTypeWithAuth extends RegistryRuleType {
authorizedConsumers: AuthorizedConsumers;
hasAlertsMappings?: boolean;
hasFieldsForAAD?: boolean;
}

type IsAuthorizedAtProducerLevel = boolean;
Expand Down Expand Up @@ -371,7 +369,7 @@ export class AlertingAuthorization {
const ruleTypesWithAuthorization = Array.from(
this.augmentWithAuthorizedConsumers(ruleTypes, {})
);
const ruleTypesAuthorized: Map<string, RegistryRuleType> = new Map();
const ruleTypesAuthorized: Map<string, RegistryAlertTypeWithAuth> = new Map();
// map from privilege to ruleType which we can refer back to when analyzing the result
// of checkPrivileges
const privilegeToRuleType = new Map<
Expand All @@ -386,9 +384,7 @@ export class AlertingAuthorization {
const ruleTypeAuth = ruleTypesWithAuthorization.find((rtwa) => rtwa.id === ruleTypeId);
if (ruleTypeAuth) {
if (!ruleTypesAuthorized.has(ruleTypeId)) {
const { authorizedConsumers, hasAlertsMappings, hasFieldsForAAD, ...ruleType } =
ruleTypeAuth;
ruleTypesAuthorized.set(ruleTypeId, ruleType);
ruleTypesAuthorized.set(ruleTypeId, ruleTypeAuth);
}
for (const operation of operations) {
privilegeToRuleType.set(
Expand Down
32 changes: 32 additions & 0 deletions x-pack/plugins/alerting/server/authorization/alerting_authorization_kuery.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -67,6 +69,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myOtherApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -105,6 +109,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -122,6 +128,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -139,6 +147,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -177,6 +187,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -194,6 +206,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -233,6 +247,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -250,6 +266,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -283,6 +301,8 @@ describe('asKqlFiltersByRuleTypeAndConsumer', () => {
isExportable: true,
authorizedConsumers: {},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -317,6 +337,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
myApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -379,6 +401,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
myOtherApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -449,6 +473,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -466,6 +492,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
{
actionGroups: [],
Expand All @@ -483,6 +511,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
myAppWithSubFeature: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down Expand Up @@ -646,6 +676,8 @@ describe('asEsDslFiltersByRuleTypeAndConsumer', () => {
isExportable: true,
authorizedConsumers: {},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]),
{
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/health.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ const ruleTypes = [
enabledInLicense: true,
minimumScheduleInterval: '1m',
defaultScheduleInterval: '10m',
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/alerting/server/routes/legacy/health.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const ruleTypes = [
producer: 'test',
enabledInLicense: true,
defaultScheduleInterval: '10m',
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/alerting/server/routes/legacy/list_alert_types.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ describe('listAlertTypesRoute', () => {
},
producer: 'test',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];
rulesClient.listRuleTypes.mockResolvedValueOnce(new Set(listTypes));
Expand All @@ -85,6 +87,8 @@ describe('listAlertTypesRoute', () => {
"authorizedConsumers": Object {},
"defaultActionGroupId": "default",
"enabledInLicense": true,
"hasAlertsMappings": false,
"hasFieldsForAAD": false,
"id": "1",
"isExportable": true,
"minimumLicenseRequired": "basic",
Expand Down Expand Up @@ -137,6 +141,8 @@ describe('listAlertTypesRoute', () => {
},
producer: 'alerts',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down Expand Up @@ -190,6 +196,8 @@ describe('listAlertTypesRoute', () => {
},
producer: 'alerts',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/alerting/server/routes/rule_types.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ describe('ruleTypesRoute', () => {
defaultScheduleInterval: '10m',
doesSetRecoveryContext: false,
hasAlertsMappings: true,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];
const expectedResult: Array<AsApiContract<RegistryAlertTypeWithAuth>> = [
Expand Down Expand Up @@ -169,6 +170,8 @@ describe('ruleTypesRoute', () => {
},
producer: 'alerts',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down Expand Up @@ -222,6 +225,8 @@ describe('ruleTypesRoute', () => {
},
producer: 'alerts',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
} as RegistryAlertTypeWithAuth,
];

Expand Down
50 changes: 28 additions & 22 deletions x-pack/plugins/alerting/server/rule_type_registry.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export interface RegistryRuleType
> {
id: string;
enabledInLicense: boolean;
hasFieldsForAAD: boolean;
hasAlertsMappings: boolean;
}

/**
Expand Down Expand Up @@ -362,26 +364,28 @@ export class RuleTypeRegistry {
}

public list(): Set<RegistryRuleType> {
return new Set(
Array.from(this.ruleTypes).map(
([
id,
{
name,
actionGroups,
recoveryActionGroup,
defaultActionGroupId,
actionVariables,
producer,
minimumLicenseRequired,
isExportable,
ruleTaskTimeout,
defaultScheduleInterval,
doesSetRecoveryContext,
alerts,
fieldsForAAD,
},
]: [string, UntypedNormalizedRuleType]) => ({
const mapRuleTypes: Array<[string, UntypedNormalizedRuleType]> = Array.from(this.ruleTypes);
const tempRegistryRuleType = mapRuleTypes.map<RegistryRuleType>(
([
id,
{
name,
actionGroups,
recoveryActionGroup,
defaultActionGroupId,
actionVariables,
producer,
minimumLicenseRequired,
isExportable,
ruleTaskTimeout,
defaultScheduleInterval,
doesSetRecoveryContext,
alerts,
fieldsForAAD,
},
]) => {
// KEEP the type here to be safe if not the map is ignoring it for some reason
const ruleType: RegistryRuleType = {
id,
name,
actionGroups,
Expand All @@ -402,9 +406,11 @@ export class RuleTypeRegistry {
hasFieldsForAAD: Boolean(fieldsForAAD),
hasAlertsMappings: !!alerts,
...(alerts ? { alerts } : {}),
})
)
};
return ruleType;
}
);
return new Set(tempRegistryRuleType);
}

public getAllTypes(): string[] {
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/tests/aggregate.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ describe('aggregate()', () => {
name: 'myType',
producer: 'myApp',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]);
beforeEach(() => {
Expand Down Expand Up @@ -156,6 +158,8 @@ describe('aggregate()', () => {
myApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
])
);
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/tests/find.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ describe('find()', () => {
name: 'myType',
producer: 'myApp',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]);
beforeEach(() => {
Expand Down Expand Up @@ -142,6 +144,8 @@ describe('find()', () => {
myApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
])
);
Expand Down Expand Up @@ -453,6 +457,8 @@ describe('find()', () => {
name: 'myType',
producer: 'myApp',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
])
);
Expand Down Expand Up @@ -659,6 +665,8 @@ describe('find()', () => {
name: 'myType',
producer: 'myApp',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
])
);
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugins/alerting/server/rules_client/tests/get_tags.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ const listedTypes = new Set<RegistryRuleType>([
name: 'myType',
producer: 'myApp',
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
]);

Expand Down Expand Up @@ -112,6 +114,8 @@ describe('getTags()', () => {
myApp: { read: true, all: true },
},
enabledInLicense: true,
hasAlertsMappings: false,
hasFieldsForAAD: false,
},
])
);
Expand Down
Loading

0 comments on commit ea9ec1f

Please sign in to comment.