[RAM] Add Snooze UI and Unsnooze API #128214
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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,80 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { unsnoozeRuleRoute } from './unsnooze_rule'; | ||
| import { httpServiceMock } from 'src/core/server/mocks'; | ||
| import { licenseStateMock } from '../lib/license_state.mock'; | ||
| import { mockHandlerArguments } from './_mock_handler_arguments'; | ||
| import { rulesClientMock } from '../rules_client.mock'; | ||
| import { AlertTypeDisabledError } from '../lib/errors/alert_type_disabled'; | ||
|
|
||
| const rulesClient = rulesClientMock.create(); | ||
| jest.mock('../lib/license_api_access.ts', () => ({ | ||
| verifyApiAccess: jest.fn(), | ||
| })); | ||
|
|
||
| beforeEach(() => { | ||
| jest.resetAllMocks(); | ||
| }); | ||
|
|
||
| describe('unsnoozeAlertRoute', () => { | ||
| it('unsnoozes an alert', async () => { | ||
| const licenseState = licenseStateMock.create(); | ||
| const router = httpServiceMock.createRouter(); | ||
|
|
||
| unsnoozeRuleRoute(router, licenseState); | ||
|
|
||
| const [config, handler] = router.post.mock.calls[0]; | ||
|
|
||
| expect(config.path).toMatchInlineSnapshot(`"/internal/alerting/rule/{id}/_unsnooze"`); | ||
|
|
||
| rulesClient.unsnooze.mockResolvedValueOnce(); | ||
|
|
||
| const [context, req, res] = mockHandlerArguments( | ||
| { rulesClient }, | ||
| { | ||
| params: { | ||
| id: '1', | ||
| }, | ||
| }, | ||
| ['noContent'] | ||
| ); | ||
|
|
||
| expect(await handler(context, req, res)).toEqual(undefined); | ||
|
|
||
| expect(rulesClient.unsnooze).toHaveBeenCalledTimes(1); | ||
| expect(rulesClient.unsnooze.mock.calls[0]).toMatchInlineSnapshot(` | ||
| Array [ | ||
| Object { | ||
| "id": "1", | ||
| }, | ||
| ] | ||
| `); | ||
|
|
||
| expect(res.noContent).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('ensures the rule type gets validated for the license', async () => { | ||
| const licenseState = licenseStateMock.create(); | ||
| const router = httpServiceMock.createRouter(); | ||
|
|
||
| unsnoozeRuleRoute(router, licenseState); | ||
|
|
||
| const [, handler] = router.post.mock.calls[0]; | ||
|
|
||
| rulesClient.unsnooze.mockRejectedValue(new AlertTypeDisabledError('Fail', 'license_invalid')); | ||
|
|
||
| const [context, req, res] = mockHandlerArguments({ rulesClient }, { params: {}, body: {} }, [ | ||
| 'ok', | ||
| 'forbidden', | ||
| ]); | ||
|
|
||
| await handler(context, req, res); | ||
|
|
||
| expect(res.forbidden).toHaveBeenCalledWith({ body: { message: 'Fail' } }); | ||
| }); | ||
| }); |
This file contains hidden or 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,45 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| import { IRouter } from 'kibana/server'; | ||
| import { schema } from '@kbn/config-schema'; | ||
| import { ILicenseState, RuleMutedError } from '../lib'; | ||
| import { verifyAccessAndContext } from './lib'; | ||
| import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types'; | ||
|
|
||
| const paramSchema = schema.object({ | ||
| id: schema.string(), | ||
| }); | ||
|
|
||
| export const unsnoozeRuleRoute = ( | ||
| router: IRouter<AlertingRequestHandlerContext>, | ||
| licenseState: ILicenseState | ||
| ) => { | ||
| router.post( | ||
| { | ||
| path: `${INTERNAL_BASE_ALERTING_API_PATH}/rule/{id}/_unsnooze`, | ||
| validate: { | ||
| params: paramSchema, | ||
| }, | ||
| }, | ||
| router.handleLegacyErrors( | ||
| verifyAccessAndContext(licenseState, async function (context, req, res) { | ||
| const rulesClient = context.alerting.getRulesClient(); | ||
| const params = req.params; | ||
| try { | ||
| await rulesClient.unsnooze({ ...params }); | ||
|
Contributor
There was a problem hiding this comment. Just for my own information, we feel comfortable spreading the parameters here because the schema should ensure there isn't anything extra in the object?
Contributor
Author
There was a problem hiding this comment. I think so. This is how all the other routes do it. |
||
| return res.noContent(); | ||
| } catch (e) { | ||
| if (e instanceof RuleMutedError) { | ||
| return e.sendResponse(res); | ||
| } | ||
| throw e; | ||
| } | ||
| }) | ||
| ) | ||
| ); | ||
| }; | ||
This file contains hidden or 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 hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -1659,6 +1659,68 @@ export class RulesClient { | |
| ); | ||
| } | ||
|
|
||
| public async unsnooze({ id }: { id: string }): Promise<void> { | ||
| return await retryIfConflicts( | ||
| this.logger, | ||
| `rulesClient.unsnooze('${id}')`, | ||
| async () => await this.unsnoozeWithOCC({ id }) | ||
| ); | ||
| } | ||
|
|
||
| private async unsnoozeWithOCC({ id }: { id: string }) { | ||
| const { attributes, version } = await this.unsecuredSavedObjectsClient.get<RawRule>( | ||
| 'alert', | ||
| id | ||
| ); | ||
|
|
||
| try { | ||
| await this.authorization.ensureAuthorized({ | ||
| ruleTypeId: attributes.alertTypeId, | ||
| consumer: attributes.consumer, | ||
| operation: WriteOperations.Unsnooze, | ||
| entity: AlertingAuthorizationEntity.Rule, | ||
| }); | ||
|
Comment on lines
+1670
to
+1682
Contributor
There was a problem hiding this comment. I haven't tested this, but it appears that the authorization check will never occur if: 1. the user tries to unsnooze an alert, and 2. the alert is not found. In that case it will throw a 404 error before the authZ check occurs (and thus, it will never get audited). At least that's what it appears to be doing. It looks like the rest of the rules client functions the same way, too. Can you confirm? CC @XavierM |
||
|
|
||
| if (attributes.actions.length) { | ||
| await this.actionsAuthorization.ensureAuthorized('execute'); | ||
| } | ||
| } catch (error) { | ||
| this.auditLogger?.log( | ||
| ruleAuditEvent({ | ||
| action: RuleAuditAction.UNSNOOZE, | ||
| savedObject: { type: 'alert', id }, | ||
| error, | ||
| }) | ||
| ); | ||
| throw error; | ||
| } | ||
|
|
||
| this.auditLogger?.log( | ||
| ruleAuditEvent({ | ||
| action: RuleAuditAction.UNSNOOZE, | ||
| outcome: 'unknown', | ||
| savedObject: { type: 'alert', id }, | ||
| }) | ||
| ); | ||
|
|
||
| this.ruleTypeRegistry.ensureRuleTypeEnabled(attributes.alertTypeId); | ||
|
|
||
| const updateAttributes = this.updateMeta({ | ||
| snoozeEndTime: null, | ||
|
Zacqary marked this conversation as resolved.
|
||
| muteAll: false, | ||
| updatedBy: await this.getUserName(), | ||
| updatedAt: new Date().toISOString(), | ||
| }); | ||
| const updateOptions = { version }; | ||
|
Contributor
There was a problem hiding this comment. We don't seem to pass this in when update the rule from the task runner - why is this different here?
Contributor
Author
There was a problem hiding this comment. No idea I'm just copypasting from what the |
||
|
|
||
| await partiallyUpdateAlert( | ||
| this.unsecuredSavedObjectsClient, | ||
| id, | ||
| updateAttributes, | ||
| updateOptions | ||
| ); | ||
| } | ||
|
|
||
| public async muteAll({ id }: { id: string }): Promise<void> { | ||
| return await retryIfConflicts( | ||
| this.logger, | ||
|
|
||
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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 hidden or 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,28 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
| import dateMath from '@elastic/datemath'; | ||
| import { i18n } from '@kbn/i18n'; | ||
| export const INTERVAL_STRING_RE = new RegExp(`^([\\d\\.]+)\\s*(${dateMath.units.join('|')})$`); | ||
|
|
||
| export const parseInterval = (intervalString: string) => { | ||
| if (intervalString) { | ||
| const matches = intervalString.match(INTERVAL_STRING_RE); | ||
| if (matches) { | ||
| const value = Number(matches[1]); | ||
| const unit = matches[2]; | ||
| return { value, unit }; | ||
| } | ||
| } | ||
| throw new Error( | ||
| i18n.translate('xpack.triggersActionsUI.parseInterval.errorMessage', { | ||
| defaultMessage: '{value} is not an interval string', | ||
| values: { | ||
| value: intervalString, | ||
| }, | ||
| }) | ||
| ); | ||
| }; |
This file contains hidden or 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
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it turns out Jest fake timers doesn't actually work for mocking the output of
Dateormomentseveral imports deep. We just didn't realize this because thesnooze_ruletest isn't actually validating that the snooze date is in the future.I was able to use
jest.spyOnto mockDate.now()for the dropdown UI tests, but I haven't been able to find a reliable solution to mocknew Date()ormoment()with no arguments. We can work around this by usingnew Date(Date.now()ormoment(Date.now())in any code where we want to test against the current date.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd love to help figure out a way, but I don't know if I understand the problem. Can you elaborate more on what you're trying to do and what's not working?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's code in the
snoozeAPI that uses Moment to determine if thesnoozeEndTimeis in the future:And also in the new
RuleStatusDropdownto determine how far in the futuresnoozeEndTimeis:We thought that using
jest.useFakeTimerswould enable us to change the return result ofnew Date()andDate.now(). For example, in this test we calledjest.setSystemTime(new Date(2020, 3, 1))to set the value ofnowto March 2020 (which, now that I think about it, oh god, why would anyone want to make anything permanently March 2020, oh no oh no oh no).Turns out, this did not actually work as expected. It seems like Jest's fake timers system only applies to
Date()called within the Jest tests themselves, and not to any modules that the tests import.We didn't realize that this was broken because
snooze_rule.test.tsisn't actually testing the part of thesnoozeAPI that tries to check if the snooze end time is in the future. But we thought that maybe it was.It appears that
moment.fromNow()usesDate.now()to determine the value ofnow, as opposed tomoment([no arguments])which usesnew Date().jest.spyOnallows us to mock the value ofglobal.Date.nowthroughout all imports, but mockingglobal.Daterequires you to mock the entire Date object and there's no easy way to just override the constructor.So therefore, we can work around that limitation by doing something like:
in any code that we need to test.