-
Notifications
You must be signed in to change notification settings - Fork 8.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[Alerting] Improves performance of the authorization filter in Alerts…
…Client.find by skipping KQL parsing (#77040) (#77439) The Alerts Authorisation now manually builds the authorization filter instead of parsing a KQL string in order to get around the performance issue we recently identified in KQL.
- Loading branch information
Showing
7 changed files
with
238 additions
and
91 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
144 changes: 144 additions & 0 deletions
144
x-pack/plugins/alerts/server/authorization/alerts_authorization_kuery.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,144 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License; | ||
| * you may not use this file except in compliance with the Elastic License. | ||
| */ | ||
| import { esKuery } from '../../../../../src/plugins/data/server'; | ||
| import { | ||
| asFiltersByAlertTypeAndConsumer, | ||
| ensureFieldIsSafeForQuery, | ||
| } from './alerts_authorization_kuery'; | ||
|
|
||
| describe('asFiltersByAlertTypeAndConsumer', () => { | ||
| test('constructs filter for single alert type with single authorized consumer', async () => { | ||
| expect( | ||
| asFiltersByAlertTypeAndConsumer( | ||
| new Set([ | ||
| { | ||
| actionGroups: [], | ||
| defaultActionGroupId: 'default', | ||
| id: 'myAppAlertType', | ||
| name: 'myAppAlertType', | ||
| producer: 'myApp', | ||
| authorizedConsumers: { | ||
| myApp: { read: true, all: true }, | ||
| }, | ||
| }, | ||
| ]) | ||
| ) | ||
| ).toEqual( | ||
| esKuery.fromKueryExpression( | ||
| `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(myApp)))` | ||
| ) | ||
| ); | ||
| }); | ||
|
|
||
| test('constructs filter for single alert type with multiple authorized consumer', async () => { | ||
| expect( | ||
| asFiltersByAlertTypeAndConsumer( | ||
| new Set([ | ||
| { | ||
| actionGroups: [], | ||
| defaultActionGroupId: 'default', | ||
| id: 'myAppAlertType', | ||
| name: 'myAppAlertType', | ||
| producer: 'myApp', | ||
| authorizedConsumers: { | ||
| alerts: { read: true, all: true }, | ||
| myApp: { read: true, all: true }, | ||
| myOtherApp: { read: true, all: true }, | ||
| }, | ||
| }, | ||
| ]) | ||
| ) | ||
| ).toEqual( | ||
| esKuery.fromKueryExpression( | ||
| `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp)))` | ||
| ) | ||
| ); | ||
| }); | ||
|
|
||
| test('constructs filter for multiple alert types across authorized consumer', async () => { | ||
| expect( | ||
| asFiltersByAlertTypeAndConsumer( | ||
| new Set([ | ||
| { | ||
| actionGroups: [], | ||
| defaultActionGroupId: 'default', | ||
| id: 'myAppAlertType', | ||
| name: 'myAppAlertType', | ||
| producer: 'myApp', | ||
| authorizedConsumers: { | ||
| alerts: { read: true, all: true }, | ||
| myApp: { read: true, all: true }, | ||
| myOtherApp: { read: true, all: true }, | ||
| myAppWithSubFeature: { read: true, all: true }, | ||
| }, | ||
| }, | ||
| { | ||
| actionGroups: [], | ||
| defaultActionGroupId: 'default', | ||
| id: 'myOtherAppAlertType', | ||
| name: 'myOtherAppAlertType', | ||
| producer: 'alerts', | ||
| authorizedConsumers: { | ||
| alerts: { read: true, all: true }, | ||
| myApp: { read: true, all: true }, | ||
| myOtherApp: { read: true, all: true }, | ||
| myAppWithSubFeature: { read: true, all: true }, | ||
| }, | ||
| }, | ||
| { | ||
| actionGroups: [], | ||
| defaultActionGroupId: 'default', | ||
| id: 'mySecondAppAlertType', | ||
| name: 'mySecondAppAlertType', | ||
| producer: 'myApp', | ||
| authorizedConsumers: { | ||
| alerts: { read: true, all: true }, | ||
| myApp: { read: true, all: true }, | ||
| myOtherApp: { read: true, all: true }, | ||
| myAppWithSubFeature: { read: true, all: true }, | ||
| }, | ||
| }, | ||
| ]) | ||
| ) | ||
| ).toEqual( | ||
| esKuery.fromKueryExpression( | ||
| `((alert.attributes.alertTypeId:myAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:myOtherAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)) or (alert.attributes.alertTypeId:mySecondAppAlertType and alert.attributes.consumer:(alerts or myApp or myOtherApp or myAppWithSubFeature)))` | ||
| ) | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe('ensureFieldIsSafeForQuery', () => { | ||
| test('throws if field contains character that isnt safe in a KQL query', () => { | ||
| expect(() => ensureFieldIsSafeForQuery('id', 'alert-*')).toThrowError( | ||
| `expected id not to include invalid character: *` | ||
| ); | ||
|
|
||
| expect(() => ensureFieldIsSafeForQuery('id', '<=""')).toThrowError( | ||
| `expected id not to include invalid character: <=` | ||
| ); | ||
|
|
||
| expect(() => ensureFieldIsSafeForQuery('id', '>=""')).toThrowError( | ||
| `expected id not to include invalid character: >=` | ||
| ); | ||
|
|
||
| expect(() => ensureFieldIsSafeForQuery('id', '1 or alertid:123')).toThrowError( | ||
| `expected id not to include whitespace and invalid character: :` | ||
| ); | ||
|
|
||
| expect(() => ensureFieldIsSafeForQuery('id', ') or alertid:123')).toThrowError( | ||
| `expected id not to include whitespace and invalid characters: ), :` | ||
| ); | ||
|
|
||
| expect(() => ensureFieldIsSafeForQuery('id', 'some space')).toThrowError( | ||
| `expected id not to include whitespace` | ||
| ); | ||
| }); | ||
|
|
||
| test('doesnt throws if field is safe as part of a KQL query', () => { | ||
| expect(() => ensureFieldIsSafeForQuery('id', '123-0456-678')).not.toThrow(); | ||
| }); | ||
| }); |
Oops, something went wrong.