Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Response Ops][Rules] Version Mute All Rule API #195572

Merged
merged 10 commits into from
Oct 14, 2024
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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.
*/

export { muteAllRuleRequestParamsSchema } from './schemas/latest';
export type { MuteAllRuleRequestParams } from './types/latest';

export { muteAllRuleRequestParamsSchema as muteAllRuleRequestParamsSchemaV1 } from './schemas/v1';
export type { MuteAllRuleRequestParams as MuteAllRuleRequestParamsV1 } from './types/v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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.
*/

export * from './v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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 { schema } from '@kbn/config-schema';

export const muteAllRuleRequestParamsSchema = schema.object({
id: schema.string({
meta: {
description: 'The identifier for the rule.',
},
}),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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.
*/

export * from './v1';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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 type { TypeOf } from '@kbn/config-schema';
import { muteAllRuleRequestParamsSchemaV1 } from '..';

export type MuteAllRuleRequestParams = TypeOf<typeof muteAllRuleRequestParamsSchemaV1>;
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
/*
* 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.
*/

export type { MuteAllRuleParams } from './types';
export { muteAll } from './mute_all';
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/*
* 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 { RulesClientContext } from '../../../../rules_client';
import { muteAll } from './mute_all';
import { savedObjectsRepositoryMock } from '@kbn/core-saved-objects-api-server-mocks';

jest.mock('../../../../lib/retry_if_conflicts', () => ({
retryIfConflicts: (_: unknown, id: unknown, asyncFn: () => Promise<unknown>) => {
return asyncFn();
},
}));

jest.mock('../../../../rules_client/lib', () => ({
updateMetaAttributes: () => {},
}));

jest.mock('../../../../saved_objects', () => ({
partiallyUpdateRule: async () => {},
}));

const loggerErrorMock = jest.fn();
const getBulkMock = jest.fn();

const savedObjectsMock = savedObjectsRepositoryMock.create();
savedObjectsMock.get = jest.fn().mockReturnValue({
attributes: {
actions: [],
},
version: '9.0.0',
});

const context = {
logger: { error: loggerErrorMock },
getActionsClient: () => {
return {
getBulk: getBulkMock,
};
},
unsecuredSavedObjectsClient: savedObjectsMock,
authorization: { ensureAuthorized: async () => {} },
ruleTypeRegistry: {
ensureRuleTypeEnabled: () => {},
},
getUserName: async () => {},
} as unknown as RulesClientContext;

describe('validateMuteAllParams', () => {
afterEach(() => {
jest.clearAllMocks();
});

it('should not throw an error for valid params', () => {
const validParams = {
id: 'ble',
};

expect(() => muteAll(context, validParams)).not.toThrow();
expect(savedObjectsMock.get).toHaveBeenCalled();
});

it('should throw Boom.badRequest for invalid params', async () => {
const invalidParams = {
id: 22 as unknown as string, // type workaround to send wrong data validation
};

await expect(muteAll(context, invalidParams)).rejects.toThrowErrorMatchingInlineSnapshot(
`"Error validating mute all parameters - [id]: expected value of type [string] but got [number]"`
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,38 @@
* 2.0.
*/

import { RawRule } from '../../types';
import { WriteOperations, AlertingAuthorizationEntity } from '../../authorization';
import { retryIfConflicts } from '../../lib/retry_if_conflicts';
import { partiallyUpdateRule, RULE_SAVED_OBJECT_TYPE } from '../../saved_objects';
import { ruleAuditEvent, RuleAuditAction } from '../common/audit_events';
import { RulesClientContext } from '../types';
import { updateMetaAttributes } from '../lib';
import { clearUnscheduledSnoozeAttributes } from '../common';
import { RuleAttributes } from '../../data/rule/types';
import Boom from '@hapi/boom';
import { RawRule } from '../../../../types';
import { WriteOperations, AlertingAuthorizationEntity } from '../../../../authorization';
import { retryIfConflicts } from '../../../../lib/retry_if_conflicts';
import { partiallyUpdateRule, RULE_SAVED_OBJECT_TYPE } from '../../../../saved_objects';
import { ruleAuditEvent, RuleAuditAction } from '../../../../rules_client/common/audit_events';
import { RulesClientContext } from '../../../../rules_client/types';
import { updateMetaAttributes } from '../../../../rules_client/lib';
import { clearUnscheduledSnoozeAttributes } from '../../../../rules_client/common';
import { RuleAttributes } from '../../../../data/rule/types';
import { MuteAllRuleParams } from './types';
import { muteAllRuleParamsSchema } from './schemas';

export async function muteAll(context: RulesClientContext, { id }: { id: string }): Promise<void> {
export async function muteAll(
context: RulesClientContext,
{ id }: MuteAllRuleParams
): Promise<void> {
return await retryIfConflicts(
context.logger,
`rulesClient.muteAll('${id}')`,
async () => await muteAllWithOCC(context, { id })
);
}

async function muteAllWithOCC(context: RulesClientContext, { id }: { id: string }) {
async function muteAllWithOCC(context: RulesClientContext, params: MuteAllRuleParams) {
try {
muteAllRuleParamsSchema.validate(params);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a unit test for this new change?

} catch (error) {
throw Boom.badRequest(`Error validating mute all parameters - ${error.message}`);
}

const { id } = params;
const { attributes, version } = await context.unsecuredSavedObjectsClient.get<RawRule>(
RULE_SAVED_OBJECT_TYPE,
id
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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.
*/

export * from './mute_all_rule_schemas';
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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 { schema } from '@kbn/config-schema';

export const muteAllRuleParamsSchema = schema.object({
id: schema.string(),
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/*
* 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.
*/

export * from './mute_all_rule_types';
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* 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 type { TypeOf } from '@kbn/config-schema';
import { muteAllRuleParamsSchema } from '../schemas';

export type MuteAllRuleParams = TypeOf<typeof muteAllRuleParamsSchema>;
2 changes: 1 addition & 1 deletion x-pack/plugins/alerting/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { getRuleStateRoute } from './get_rule_state';
import { healthRoute } from './health';
import { resolveRuleRoute } from './rule/apis/resolve';
import { ruleTypesRoute } from './rule_types';
import { muteAllRuleRoute } from './mute_all_rule';
import { muteAllRuleRoute } from './rule/apis/mute_all/mute_all_rule';
import { muteAlertRoute } from './rule/apis/mute_alert/mute_alert';
import { unmuteAllRuleRoute } from './unmute_all_rule';
import { unmuteAlertRoute } from './rule/apis/unmute_alert/unmute_alert_route';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@
import { usageCountersServiceMock } from '@kbn/usage-collection-plugin/server/usage_counters/usage_counters_service.mock';
import { muteAllRuleRoute } from './mute_all_rule';
import { httpServiceMock } from '@kbn/core/server/mocks';
import { licenseStateMock } from '../lib/license_state.mock';
import { mockHandlerArguments } from './_mock_handler_arguments';
import { rulesClientMock } from '../rules_client.mock';
import { RuleTypeDisabledError } from '../lib/errors/rule_type_disabled';
import { trackDeprecatedRouteUsage } from '../lib/track_deprecated_route_usage';
import { licenseStateMock } from '../../../../lib/license_state.mock';
import { mockHandlerArguments } from '../../../_mock_handler_arguments';
import { rulesClientMock } from '../../../../rules_client.mock';
import { RuleTypeDisabledError } from '../../../../lib/errors/rule_type_disabled';
import { trackDeprecatedRouteUsage } from '../../../../lib/track_deprecated_route_usage';

const rulesClient = rulesClientMock.create();
jest.mock('../lib/license_api_access', () => ({
jest.mock('../../../../lib/license_api_access', () => ({
verifyApiAccess: jest.fn(),
}));

jest.mock('../lib/track_deprecated_route_usage', () => ({
jest.mock('../../../../lib/track_deprecated_route_usage', () => ({
trackDeprecatedRouteUsage: jest.fn(),
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,15 @@
*/

import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { UsageCounter } from '@kbn/usage-collection-plugin/server';
import { ILicenseState, RuleTypeDisabledError } from '../lib';
import { verifyAccessAndContext } from './lib';
import { AlertingRequestHandlerContext, BASE_ALERTING_API_PATH } from '../types';
import { trackDeprecatedRouteUsage } from '../lib/track_deprecated_route_usage';

const paramSchema = schema.object({
id: schema.string({
meta: {
description: 'The identifier for the rule.',
},
}),
});
import { ILicenseState, RuleTypeDisabledError } from '../../../../lib';
import { verifyAccessAndContext } from '../../../lib';
import { AlertingRequestHandlerContext, BASE_ALERTING_API_PATH } from '../../../../types';
import { trackDeprecatedRouteUsage } from '../../../../lib/track_deprecated_route_usage';
import {
muteAllRuleRequestParamsSchemaV1,
MuteAllRuleRequestParamsV1,
} from '../../../../../common/routes/rule/apis/mute_all';

export const muteAllRuleRoute = (
router: IRouter<AlertingRequestHandlerContext>,
Expand All @@ -36,7 +31,7 @@ export const muteAllRuleRoute = (
},
validate: {
request: {
params: paramSchema,
params: muteAllRuleRequestParamsSchemaV1,
},
response: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, when we did this versioning we ended up also updating the documentation. I don't know if we will do another passing but I noticed 403 is missing from the response. Maybe you could update that here? 🙏

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe 400 and 404 also. I'm thinking of this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm not mistaken, there is an extra issue to do it afterwards (which should be also be ready for next ff) #195182. Are we talking about the same?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I didn't realize there was a separate issue. Nevermind then!

204: {
Expand All @@ -48,10 +43,10 @@ export const muteAllRuleRoute = (
router.handleLegacyErrors(
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesClient = (await context.alerting).getRulesClient();
const { id } = req.params;
const params: MuteAllRuleRequestParamsV1 = req.params;
trackDeprecatedRouteUsage('muteAll', usageCounter);
try {
await rulesClient.muteAll({ id });
await rulesClient.muteAll(params);
return res.noContent();
} catch (e) {
if (e instanceof RuleTypeDisabledError) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import { enableRule } from '../application/rule/methods/enable_rule/enable_rule'
import { updateRuleApiKey } from '../application/rule/methods/update_api_key/update_rule_api_key';
import { disableRule } from '../application/rule/methods/disable/disable_rule';
import { muteInstance } from '../application/rule/methods/mute_alert/mute_instance';
import { muteAll } from './methods/mute_all';
import { muteAll } from '../application/rule/methods/mute_all';
import { unmuteAll } from './methods/unmute_all';
import { unmuteInstance } from '../application/rule/methods/unmute_alert/unmute_instance';
import { runSoon } from './methods/run_soon';
Expand Down