Skip to content

Commit

Permalink
[RAM][Flapping] Convert rule flapping settings API to snake case (#15…
Browse files Browse the repository at this point in the history
…0951)

## Summary
Resolves: #150623

Convert the rule flapping settings API properties from camel case to
snake case. The properties are rewritten once it hits the application
code to camel case again.

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

---------

Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
JiaweiWu and kibanamachine authored Feb 13, 2023
1 parent eb4b92d commit c6641c9
Show file tree
Hide file tree
Showing 12 changed files with 173 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,16 @@ describe('getFlappingSettingsRoute', () => {
await handler(context, req, res);

expect(rulesSettingsClient.flapping().get).toHaveBeenCalledTimes(1);
expect(res.ok).toHaveBeenCalled();
expect(res.ok).toHaveBeenCalledWith({
body: expect.objectContaining({
enabled: true,
look_back_window: 10,
status_change_threshold: 10,
created_by: 'test name',
updated_by: 'test name',
created_at: expect.any(String),
updated_at: expect.any(String),
}),
});
});
});
24 changes: 21 additions & 3 deletions x-pack/plugins/alerting/server/routes/get_flapping_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,26 @@
import { IRouter } from '@kbn/core/server';
import { ILicenseState } from '../lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { verifyAccessAndContext } from './lib';
import { API_PRIVILEGES } from '../../common';
import { verifyAccessAndContext, RewriteResponseCase } from './lib';
import { API_PRIVILEGES, RulesSettingsFlapping } from '../../common';

const rewriteBodyRes: RewriteResponseCase<RulesSettingsFlapping> = ({
lookBackWindow,
statusChangeThreshold,
createdBy,
updatedBy,
createdAt,
updatedAt,
...rest
}) => ({
...rest,
look_back_window: lookBackWindow,
status_change_threshold: statusChangeThreshold,
created_by: createdBy,
updated_by: updatedBy,
created_at: createdAt,
updated_at: updatedAt,
});

export const getFlappingSettingsRoute = (
router: IRouter<AlertingRequestHandlerContext>,
Expand All @@ -27,7 +45,7 @@ export const getFlappingSettingsRoute = (
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesSettingsClient = (await context.alerting).getRulesSettingsClient();
const flappingSettings = await rulesSettingsClient.flapping().get();
return res.ok({ body: flappingSettings });
return res.ok({ body: rewriteBodyRes(flappingSettings) });
})
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,16 @@ beforeEach(() => {
rulesSettingsClient = rulesSettingsClientMock.create();
});

const mockFlappingSettings = {
enabled: true,
lookBackWindow: 10,
statusChangeThreshold: 10,
createdBy: 'test name',
updatedBy: 'test name',
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
};

describe('updateFlappingSettingsRoute', () => {
test('updates flapping settings', async () => {
const licenseState = licenseStateMock.create();
Expand All @@ -40,20 +50,13 @@ describe('updateFlappingSettingsRoute', () => {
}
`);

(rulesSettingsClient.flapping().get as jest.Mock).mockResolvedValue({
enabled: true,
lookBackWindow: 10,
statusChangeThreshold: 10,
createdBy: 'test name',
updatedBy: 'test name',
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
});
(rulesSettingsClient.flapping().get as jest.Mock).mockResolvedValue(mockFlappingSettings);
(rulesSettingsClient.flapping().update as jest.Mock).mockResolvedValue(mockFlappingSettings);

const updateResult = {
enabled: false,
lookBackWindow: 6,
statusChangeThreshold: 5,
look_back_window: 6,
status_change_threshold: 5,
};

const [context, req, res] = mockHandlerArguments(
Expand All @@ -77,6 +80,16 @@ describe('updateFlappingSettingsRoute', () => {
},
]
`);
expect(res.ok).toHaveBeenCalled();
expect(res.ok).toHaveBeenCalledWith({
body: expect.objectContaining({
enabled: true,
look_back_window: 10,
status_change_threshold: 10,
created_by: 'test name',
updated_by: 'test name',
created_at: expect.any(String),
updated_at: expect.any(String),
}),
});
});
});
46 changes: 40 additions & 6 deletions x-pack/plugins/alerting/server/routes/update_flapping_settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,46 @@
import { IRouter } from '@kbn/core/server';
import { schema } from '@kbn/config-schema';
import { ILicenseState } from '../lib';
import { verifyAccessAndContext } from './lib';
import { verifyAccessAndContext, RewriteResponseCase, RewriteRequestCase } from './lib';
import { AlertingRequestHandlerContext, INTERNAL_BASE_ALERTING_API_PATH } from '../types';
import { API_PRIVILEGES } from '../../common';
import {
API_PRIVILEGES,
RulesSettingsFlapping,
RulesSettingsFlappingProperties,
} from '../../common';

const bodySchema = schema.object({
enabled: schema.boolean(),
lookBackWindow: schema.number(),
statusChangeThreshold: schema.number(),
look_back_window: schema.number(),
status_change_threshold: schema.number(),
});

const rewriteQueryReq: RewriteRequestCase<RulesSettingsFlappingProperties> = ({
look_back_window: lookBackWindow,
status_change_threshold: statusChangeThreshold,
...rest
}) => ({
...rest,
lookBackWindow,
statusChangeThreshold,
});

const rewriteBodyRes: RewriteResponseCase<RulesSettingsFlapping> = ({
lookBackWindow,
statusChangeThreshold,
createdBy,
updatedBy,
createdAt,
updatedAt,
...rest
}) => ({
...rest,
look_back_window: lookBackWindow,
status_change_threshold: statusChangeThreshold,
created_by: createdBy,
updated_by: updatedBy,
created_at: createdAt,
updated_at: updatedAt,
});

export const updateFlappingSettingsRoute = (
Expand All @@ -36,10 +68,12 @@ export const updateFlappingSettingsRoute = (
verifyAccessAndContext(licenseState, async function (context, req, res) {
const rulesSettingsClient = (await context.alerting).getRulesSettingsClient();

const updatedFlappingSettings = await rulesSettingsClient.flapping().update(req.body);
const updatedFlappingSettings = await rulesSettingsClient
.flapping()
.update(rewriteQueryReq(req.body));

return res.ok({
body: updatedFlappingSettings,
body: updatedFlappingSettings && rewriteBodyRes(updatedFlappingSettings),
});
})
)
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/triggers_actions_ui/.storybook/context/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,8 @@ const rulesSettingsGetResponse = (path: string) => {
if (path.endsWith('/settings/_flapping')) {
return {
enabled: true,
lookBackWindow: 20,
statusChangeThreshold: 4,
look_back_window: 20,
status_change_threshold: 4,
};
}
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,23 @@
*/

import { HttpSetup } from '@kbn/core/public';
import { AsApiContract, RewriteRequestCase } from '@kbn/actions-plugin/common';
import { RulesSettingsFlapping } from '@kbn/alerting-plugin/common';
import { INTERNAL_BASE_ALERTING_API_PATH } from '../../constants';

export const getFlappingSettings = ({ http }: { http: HttpSetup }) => {
return http.get<RulesSettingsFlapping>(
const rewriteBodyRes: RewriteRequestCase<RulesSettingsFlapping> = ({
look_back_window: lookBackWindow,
status_change_threshold: statusChangeThreshold,
...rest
}: any) => ({
...rest,
lookBackWindow,
statusChangeThreshold,
});

export const getFlappingSettings = async ({ http }: { http: HttpSetup }) => {
const res = await http.get<AsApiContract<RulesSettingsFlapping>>(
`${INTERNAL_BASE_ALERTING_API_PATH}/rules/settings/_flapping`
);
return rewriteBodyRes(res);
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,20 @@ import {
RulesSettingsFlapping,
RulesSettingsFlappingProperties,
} from '@kbn/alerting-plugin/common';
import { AsApiContract, RewriteRequestCase } from '@kbn/actions-plugin/common';
import { INTERNAL_BASE_ALERTING_API_PATH } from '../../constants';

export const updateFlappingSettings = ({
const rewriteBodyRes: RewriteRequestCase<RulesSettingsFlapping> = ({
look_back_window: lookBackWindow,
status_change_threshold: statusChangeThreshold,
...rest
}: any) => ({
...rest,
lookBackWindow,
statusChangeThreshold,
});

export const updateFlappingSettings = async ({
http,
flappingSettings,
}: {
Expand All @@ -21,14 +32,21 @@ export const updateFlappingSettings = ({
}) => {
let body: string;
try {
body = JSON.stringify(flappingSettings);
body = JSON.stringify({
enabled: flappingSettings.enabled,
look_back_window: flappingSettings.lookBackWindow,
status_change_threshold: flappingSettings.statusChangeThreshold,
});
} catch (e) {
throw new Error(`Unable to parse flapping settings update params: ${e}`);
}
return http.post<RulesSettingsFlapping>(

const res = await http.post<AsApiContract<RulesSettingsFlapping>>(
`${INTERNAL_BASE_ALERTING_API_PATH}/rules/settings/_flapping`,
{
body,
}
);

return rewriteBodyRes(res);
};
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ export const resetRulesSettings = (supertest: any, space: string) => {
.post(`${getUrlPrefix(space)}/internal/alerting/rules/settings/_flapping`)
.set('kbn-xsrf', 'foo')
.auth(Superuser.username, Superuser.password)
.send(DEFAULT_FLAPPING_SETTINGS)
.send({
enabled: DEFAULT_FLAPPING_SETTINGS.enabled,
look_back_window: DEFAULT_FLAPPING_SETTINGS.lookBackWindow,
status_change_threshold: DEFAULT_FLAPPING_SETTINGS.statusChangeThreshold,
})
.expect(200);
};
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,16 @@ export default function getFlappingSettingsTests({ getService }: FtrProviderCont
case 'space_1_all at space1':
expect(response.statusCode).to.eql(200);
expect(response.body.enabled).to.eql(DEFAULT_FLAPPING_SETTINGS.enabled);
expect(response.body.lookBackWindow).to.eql(DEFAULT_FLAPPING_SETTINGS.lookBackWindow);
expect(response.body.statusChangeThreshold).to.eql(
expect(response.body.look_back_window).to.eql(
DEFAULT_FLAPPING_SETTINGS.lookBackWindow
);
expect(response.body.status_change_threshold).to.eql(
DEFAULT_FLAPPING_SETTINGS.statusChangeThreshold
);
expect(response.body.createdBy).to.be.a('string');
expect(response.body.updatedBy).to.be.a('string');
expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0);
expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0);
expect(response.body.created_by).to.be.a('string');
expect(response.body.updated_by).to.be.a('string');
expect(Date.parse(response.body.created_at)).to.be.greaterThan(0);
expect(Date.parse(response.body.updated_at)).to.be.greaterThan(0);
break;
default:
throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
.auth(user.username, user.password)
.send({
enabled: false,
lookBackWindow: 20,
statusChangeThreshold: 20,
look_back_window: 20,
status_change_threshold: 20,
});

switch (scenario.id) {
Expand All @@ -51,12 +51,12 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
case 'space_1_all at space1':
expect(response.statusCode).to.eql(200);
expect(response.body.enabled).to.eql(false);
expect(response.body.lookBackWindow).to.eql(20);
expect(response.body.statusChangeThreshold).to.eql(20);
expect(response.body.createdBy).to.eql(user.username);
expect(response.body.updatedBy).to.eql(user.username);
expect(Date.parse(response.body.createdAt)).to.be.greaterThan(0);
expect(Date.parse(response.body.updatedAt)).to.be.greaterThan(0);
expect(response.body.look_back_window).to.eql(20);
expect(response.body.status_change_threshold).to.eql(20);
expect(response.body.created_by).to.eql(user.username);
expect(response.body.updated_by).to.eql(user.username);
expect(Date.parse(response.body.created_at)).to.be.greaterThan(0);
expect(Date.parse(response.body.updated_at)).to.be.greaterThan(0);
break;
default:
throw new Error(`Scenario untested: ${JSON.stringify(scenario)}`);
Expand All @@ -72,8 +72,8 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
.auth(Superuser.username, Superuser.password)
.send({
enabled: true,
lookBackWindow: 200,
statusChangeThreshold: 200,
look_back_window: 200,
status_change_threshold: 200,
})
.expect(400);

Expand All @@ -87,8 +87,8 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
.auth(Superuser.username, Superuser.password)
.send({
enabled: true,
lookBackWindow: 20,
statusChangeThreshold: 200,
look_back_window: 20,
status_change_threshold: 200,
})
.expect(400);

Expand All @@ -102,8 +102,8 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
.auth(Superuser.username, Superuser.password)
.send({
enabled: true,
lookBackWindow: 5,
statusChangeThreshold: 10,
look_back_window: 5,
status_change_threshold: 10,
})
.expect(400);

Expand All @@ -121,14 +121,14 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo
.auth(Superuser.username, Superuser.password)
.send({
enabled: false,
lookBackWindow: 20,
statusChangeThreshold: 20,
look_back_window: 20,
status_change_threshold: 20,
});

expect(postResponse.statusCode).to.eql(200);
expect(postResponse.body.enabled).to.eql(false);
expect(postResponse.body.lookBackWindow).to.eql(20);
expect(postResponse.body.statusChangeThreshold).to.eql(20);
expect(postResponse.body.look_back_window).to.eql(20);
expect(postResponse.body.status_change_threshold).to.eql(20);

// Get the rules settings in space2
const getResponse = await supertestWithoutAuth
Expand All @@ -137,8 +137,8 @@ export default function updateFlappingSettingsTest({ getService }: FtrProviderCo

expect(getResponse.statusCode).to.eql(200);
expect(getResponse.body.enabled).to.eql(true);
expect(getResponse.body.lookBackWindow).to.eql(20);
expect(getResponse.body.statusChangeThreshold).to.eql(4);
expect(getResponse.body.look_back_window).to.eql(20);
expect(getResponse.body.status_change_threshold).to.eql(4);
});
});
});
Expand Down
Loading

0 comments on commit c6641c9

Please sign in to comment.