Skip to content

Commit 2b43dfd

Browse files
mikecotemikecote
committed
Don't create API key for disabled alerts when calling create API
1 parent 8aa2b61 commit 2b43dfd

2 files changed

Lines changed: 129 additions & 59 deletions

File tree

‎x-pack/legacy/plugins/alerting/server/alerts_client.test.ts‎

Lines changed: 125 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,20 @@ function getMockData(overwrites: Record<string, any> = {}) {
7979
}
8080

8181
describe('create()', () => {
82-
test('creates an alert', async () => {
83-
const alertsClient = new AlertsClient(alertsClientParams);
84-
const data = getMockData();
85-
alertTypeRegistry.get.mockReturnValueOnce({
82+
let alertsClient: AlertsClient;
83+
84+
beforeEach(() => {
85+
alertsClient = new AlertsClient(alertsClientParams);
86+
alertTypeRegistry.get.mockReturnValue({
8687
id: '123',
8788
name: 'Test',
8889
actionGroups: ['default'],
8990
async executor() {},
9091
});
92+
});
93+
94+
test('creates an alert', async () => {
95+
const data = getMockData();
9196
savedObjectsClient.bulkGet.mockResolvedValueOnce({
9297
saved_objects: [
9398
{
@@ -263,7 +268,6 @@ describe('create()', () => {
263268
});
264269

265270
test('creates an alert with multiple actions', async () => {
266-
const alertsClient = new AlertsClient(alertsClientParams);
267271
const data = getMockData({
268272
actions: [
269273
{
@@ -289,12 +293,6 @@ describe('create()', () => {
289293
},
290294
],
291295
});
292-
alertTypeRegistry.get.mockReturnValueOnce({
293-
id: '123',
294-
name: 'Test',
295-
actionGroups: ['default'],
296-
async executor() {},
297-
});
298296
savedObjectsClient.bulkGet.mockResolvedValueOnce({
299297
saved_objects: [
300298
{
@@ -446,14 +444,7 @@ describe('create()', () => {
446444
});
447445

448446
test('creates a disabled alert', async () => {
449-
const alertsClient = new AlertsClient(alertsClientParams);
450447
const data = getMockData({ enabled: false });
451-
alertTypeRegistry.get.mockReturnValueOnce({
452-
id: '123',
453-
name: 'Test',
454-
actionGroups: ['default'],
455-
async executor() {},
456-
});
457448
savedObjectsClient.bulkGet.mockResolvedValueOnce({
458449
saved_objects: [
459450
{
@@ -527,9 +518,8 @@ describe('create()', () => {
527518
});
528519

529520
test('should validate params', async () => {
530-
const alertsClient = new AlertsClient(alertsClientParams);
531521
const data = getMockData();
532-
alertTypeRegistry.get.mockReturnValueOnce({
522+
alertTypeRegistry.get.mockReturnValue({
533523
id: '123',
534524
name: 'Test',
535525
actionGroups: [],
@@ -547,14 +537,7 @@ describe('create()', () => {
547537
});
548538

549539
test('throws error if loading actions fails', async () => {
550-
const alertsClient = new AlertsClient(alertsClientParams);
551540
const data = getMockData();
552-
alertTypeRegistry.get.mockReturnValueOnce({
553-
id: '123',
554-
name: 'Test',
555-
actionGroups: ['default'],
556-
async executor() {},
557-
});
558541
savedObjectsClient.bulkGet.mockRejectedValueOnce(new Error('Test Error'));
559542
await expect(alertsClient.create({ data })).rejects.toThrowErrorMatchingInlineSnapshot(
560543
`"Test Error"`
@@ -564,14 +547,7 @@ describe('create()', () => {
564547
});
565548

566549
test('throws error if create saved object fails', async () => {
567-
const alertsClient = new AlertsClient(alertsClientParams);
568550
const data = getMockData();
569-
alertTypeRegistry.get.mockReturnValueOnce({
570-
id: '123',
571-
name: 'Test',
572-
actionGroups: ['default'],
573-
async executor() {},
574-
});
575551
savedObjectsClient.bulkGet.mockResolvedValueOnce({
576552
saved_objects: [
577553
{
@@ -592,14 +568,7 @@ describe('create()', () => {
592568
});
593569

594570
test('attempts to remove saved object if scheduling failed', async () => {
595-
const alertsClient = new AlertsClient(alertsClientParams);
596571
const data = getMockData();
597-
alertTypeRegistry.get.mockReturnValueOnce({
598-
id: '123',
599-
name: 'Test',
600-
actionGroups: ['default'],
601-
async executor() {},
602-
});
603572
savedObjectsClient.bulkGet.mockResolvedValueOnce({
604573
saved_objects: [
605574
{
@@ -655,14 +624,7 @@ describe('create()', () => {
655624
});
656625

657626
test('returns task manager error if cleanup fails, logs to console', async () => {
658-
const alertsClient = new AlertsClient(alertsClientParams);
659627
const data = getMockData();
660-
alertTypeRegistry.get.mockReturnValueOnce({
661-
id: '123',
662-
name: 'Test',
663-
actionGroups: ['default'],
664-
async executor() {},
665-
});
666628
savedObjectsClient.bulkGet.mockResolvedValueOnce({
667629
saved_objects: [
668630
{
@@ -714,7 +676,6 @@ describe('create()', () => {
714676
});
715677

716678
test('throws an error if alert type not registerd', async () => {
717-
const alertsClient = new AlertsClient(alertsClientParams);
718679
const data = getMockData();
719680
alertTypeRegistry.get.mockImplementation(() => {
720681
throw new Error('Invalid type');
@@ -725,14 +686,7 @@ describe('create()', () => {
725686
});
726687

727688
test('calls the API key function', async () => {
728-
const alertsClient = new AlertsClient(alertsClientParams);
729689
const data = getMockData();
730-
alertTypeRegistry.get.mockReturnValueOnce({
731-
id: '123',
732-
name: 'Test',
733-
actionGroups: ['default'],
734-
async executor() {},
735-
});
736690
alertsClientParams.createAPIKey.mockResolvedValueOnce({
737691
apiKeysEnabled: true,
738692
result: { id: '123', api_key: 'abc' },
@@ -845,6 +799,121 @@ describe('create()', () => {
845799
}
846800
);
847801
});
802+
803+
test(`doesn't create API key for disabled alerts`, async () => {
804+
const data = getMockData({ enabled: false });
805+
alertsClientParams.createAPIKey.mockResolvedValueOnce({
806+
apiKeysEnabled: true,
807+
result: { id: '123', api_key: 'abc' },
808+
});
809+
savedObjectsClient.bulkGet.mockResolvedValueOnce({
810+
saved_objects: [
811+
{
812+
id: '1',
813+
type: 'action',
814+
attributes: {
815+
actionTypeId: 'test',
816+
},
817+
references: [],
818+
},
819+
],
820+
});
821+
savedObjectsClient.create.mockResolvedValueOnce({
822+
id: '1',
823+
type: 'alert',
824+
attributes: {
825+
alertTypeId: '123',
826+
schedule: { interval: '10s' },
827+
params: {
828+
bar: true,
829+
},
830+
actions: [
831+
{
832+
group: 'default',
833+
actionRef: 'action_0',
834+
actionTypeId: 'test',
835+
params: {
836+
foo: true,
837+
},
838+
},
839+
],
840+
},
841+
references: [
842+
{
843+
name: 'action_0',
844+
type: 'action',
845+
id: '1',
846+
},
847+
],
848+
});
849+
taskManager.schedule.mockResolvedValueOnce({
850+
id: 'task-123',
851+
taskType: 'alerting:123',
852+
scheduledAt: new Date(),
853+
attempts: 1,
854+
status: TaskStatus.Idle,
855+
runAt: new Date(),
856+
startedAt: null,
857+
retryAt: null,
858+
state: {},
859+
params: {},
860+
ownerId: null,
861+
});
862+
savedObjectsClient.update.mockResolvedValueOnce({
863+
id: '1',
864+
type: 'alert',
865+
attributes: {
866+
scheduledTaskId: 'task-123',
867+
},
868+
references: [
869+
{
870+
id: '1',
871+
name: 'action_0',
872+
type: 'action',
873+
},
874+
],
875+
});
876+
await alertsClient.create({ data });
877+
878+
expect(alertsClientParams.createAPIKey).not.toHaveBeenCalled();
879+
expect(savedObjectsClient.create).toHaveBeenCalledWith(
880+
'alert',
881+
{
882+
actions: [
883+
{
884+
actionRef: 'action_0',
885+
group: 'default',
886+
actionTypeId: 'test',
887+
params: { foo: true },
888+
},
889+
],
890+
alertTypeId: '123',
891+
consumer: 'bar',
892+
name: 'abc',
893+
params: { bar: true },
894+
apiKey: null,
895+
apiKeyOwner: null,
896+
createdBy: 'elastic',
897+
createdAt: '2019-02-12T21:01:22.479Z',
898+
updatedBy: 'elastic',
899+
enabled: false,
900+
schedule: { interval: '10s' },
901+
throttle: null,
902+
muteAll: false,
903+
mutedInstanceIds: [],
904+
tags: ['foo'],
905+
},
906+
{
907+
references: [
908+
{
909+
id: '1',
910+
name: 'action_0',
911+
type: 'action',
912+
},
913+
],
914+
}
915+
);
916+
});
848917
});
849918

850919
describe('enable()', () => {

‎x-pack/legacy/plugins/alerting/server/alerts_client.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,14 @@ export class AlertsClient {
151151
const alertType = this.alertTypeRegistry.get(data.alertTypeId);
152152
const validatedAlertTypeParams = validateAlertTypeParams(alertType, data.params);
153153
const username = await this.getUserName();
154+
const createdAPIKey = data.enabled ? await this.createAPIKey() : null;
154155

155156
this.validateActions(alertType, data.actions);
156157

157158
const { references, actions } = await this.denormalizeActions(data.actions);
158159
const rawAlert: RawAlert = {
159160
...data,
160-
...this.apiKeyAsAlertAttributes(await this.createAPIKey(), username),
161+
...this.apiKeyAsAlertAttributes(createdAPIKey, username),
161162
actions,
162163
createdBy: username,
163164
updatedBy: username,
@@ -317,10 +318,10 @@ export class AlertsClient {
317318
}
318319

319320
private apiKeyAsAlertAttributes(
320-
apiKey: CreateAPIKeyResult,
321+
apiKey: CreateAPIKeyResult | null,
321322
username: string | null
322323
): Pick<RawAlert, 'apiKey' | 'apiKeyOwner'> {
323-
return apiKey.apiKeysEnabled
324+
return apiKey && apiKey.apiKeysEnabled
324325
? {
325326
apiKeyOwner: username,
326327
apiKey: Buffer.from(`${apiKey.result.id}:${apiKey.result.api_key}`).toString('base64'),

0 commit comments

Comments
 (0)