finish function tests

x-pack/plugins/alerts/common/alert.ts

@@ -15,7 +15,7 @@ export interface IntervalSchedule extends SavedObjectAttributes {
   interval: string;
 }
 
-export type AlertExecutionStatuses = 'ok' | 'active' | 'error' | 'noData' | 'unknown';
+export type AlertExecutionStatuses = 'ok' | 'active' | 'error' | 'unknown';
 export type AlertExecutionStatusErrorReasons = 'read' | 'decrypt' | 'execute' | 'unknown';
 
 export interface AlertExecutionStatus {

x-pack/plugins/alerts/server/alerts_client.ts

@@ -438,14 +438,6 @@ export class AlertsClient {
   }
 
   public async update({ id, data }: UpdateOptions): Promise<PartialAlert> {
-    return await retryForConflicts(
-      this.logger,
-      `update(${id})`,
-      async () => await this.updateBasic({ id, data })
-    );
-  }
-
-  private async updateBasic({ id, data }: UpdateOptions): Promise<PartialAlert> {
     let alertSavedObject: SavedObject<RawAlert>;
 
     try {
@@ -553,14 +545,6 @@ export class AlertsClient {
   }
 
   public async updateApiKey({ id }: { id: string }): Promise<void> {
-    return await retryForConflicts(
-      this.logger,
-      `updateApiKey(${id})`,
-      async () => await this.updateApiKeyBasic({ id })
-    );
-  }
-
-  private async updateApiKeyBasic({ id }: { id: string }): Promise<void> {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
 
@@ -621,14 +605,6 @@ export class AlertsClient {
   }
 
   public async enable({ id }: { id: string }): Promise<void> {
-    return await retryForConflicts(
-      this.logger,
-      `enable(${id})`,
-      async () => await this.enableBasic({ id })
-    );
-  }
-
-  private async enableBasic({ id }: { id: string }): Promise<void> {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
 
@@ -680,14 +656,6 @@ export class AlertsClient {
   }
 
   public async disable({ id }: { id: string }): Promise<void> {
-    return await retryForConflicts(
-      this.logger,
-      `disable(${id})`,
-      async () => await this.disableBasic({ id })
-    );
-  }
-
-  private async disableBasic({ id }: { id: string }): Promise<void> {
     let apiKeyToInvalidate: string | null = null;
     let attributes: RawAlert;
 
@@ -975,18 +943,34 @@ export class AlertsClient {
   }
 }
 
+type SavedObjectClient = SavedObjectsClientContract | ISavedObjectsRepository;
+
 type PartiallyUpdateableAlertAttributes = Partial<
   Pick<RawAlert, AlertAttributesExcludedFromAADType>
 >;
 
+interface PartiallyUpdateAlertSavedObjectOptions {
+  ignore404?: boolean;
+  namespace?: string; // only should be used  with ISavedObjectsRepository
+}
+
 // Specialized partial update for fields which do not contribute to AAD.
-async function partiallyUpdateAlertSavedObject(
-  savedObjectsClient: SavedObjectsClientContract,
+export async function partiallyUpdateAlertSavedObject(
+  savedObjectsClient: SavedObjectClient,
   id: string,
-  attributes: PartiallyUpdateableAlertAttributes
-): Promise<void | ReturnType<SavedObjectsClientContract['update']>> {
+  attributes: PartiallyUpdateableAlertAttributes,
+  options?: PartiallyUpdateAlertSavedObjectOptions
+): Promise<void> {
   const attributeUpdates = pick(attributes, AlertAttributesExcludedFromAAD);
-  return await savedObjectsClient.update<RawAlert>('alert', id, attributeUpdates);
+  const updateOptions = options?.namespace ? { namespace: options.namespace } : {};
+  try {
+    await savedObjectsClient.update<RawAlert>('alert', id, attributeUpdates, updateOptions);
+  } catch (err) {
+    if (options?.ignore404 && SavedObjectsErrorHelpers.isNotFoundError(err)) {
+      return;
+    }
+    throw err;
+  }
 }
 
 function parseDate(dateString: string | undefined, propertyName: string, defaultValue: Date): Date {

x-pack/plugins/alerts/server/task_runner/task_runner.ts

@@ -69,19 +69,15 @@ export class TaskRunner {
     const namespace = this.context.spaceIdToNamespace(spaceId);
     // Only fetch encrypted attributes here, we'll create a saved objects client
     // scoped with the API key to fetch the remaining data.
-    try {
-      const {
-        attributes: { apiKey },
-      } = await this.context.encryptedSavedObjectsClient.getDecryptedAsInternalUser<RawAlert>(
-        'alert',
-        alertId,
-        { namespace }
-      );
+    const {
+      attributes: { apiKey },
+    } = await this.context.encryptedSavedObjectsClient.getDecryptedAsInternalUser<RawAlert>(
+      'alert',
+      alertId,
+      { namespace }
+    );
 
-      return apiKey;
-    } catch (err) {
-      throw new ErrorWithReason('decrypt', err);
-    }
+    return apiKey;
   }
 
   private getFakeKibanaRequest(spaceId: string, apiKey: string | null) {
@@ -215,7 +211,7 @@ export class TaskRunner {
       event.event = event.event || {};
       event.event.outcome = 'failure';
       eventLogger.logEvent(event);
-      throw err;
+      throw new ErrorWithReason('execute', err);
     }
 
     eventLogger.stopTiming(event);
@@ -266,45 +262,36 @@ export class TaskRunner {
       params: { alertId, spaceId },
     } = this.taskInstance;
 
-    try {
-      // Validate
-      const validatedParams = validateAlertTypeParams(this.alertType, alert.params);
-      const executionHandler = this.getExecutionHandler(
-        alertId,
-        alert.name,
-        alert.tags,
-        spaceId,
-        apiKey,
-        alert.actions,
-        alert.params
-      );
-      return this.executeAlertInstances(
-        services,
-        alert,
-        validatedParams,
-        executionHandler,
-        spaceId
-      );
-    } catch (err) {
-      throw new ErrorWithReason('execute', err);
-    }
+    // Validate
+    const validatedParams = validateAlertTypeParams(this.alertType, alert.params);
+    const executionHandler = this.getExecutionHandler(
+      alertId,
+      alert.name,
+      alert.tags,
+      spaceId,
+      apiKey,
+      alert.actions,
+      alert.params
+    );
+    return this.executeAlertInstances(services, alert, validatedParams, executionHandler, spaceId);
   }
 
   async loadAlertAttributesAndRun(): Promise<Resultable<AlertTaskRunResult, Error>> {
     const {
       params: { alertId, spaceId },
     } = this.taskInstance;
-
     let apiKey: string | null;
-    let services: Services;
-    let alertsClient: Pick<AlertsClient, MethodKeysOf<AlertsClient>>;
-    let alert: SanitizedAlert;
-
     try {
       apiKey = await this.getApiKeyForAlertPermissions(alertId, spaceId);
-      [services, alertsClient] = this.getServicesWithSpaceLevelPermissions(spaceId, apiKey);
+    } catch (err) {
+      throw new ErrorWithReason('decrypt', err);
+    }
+    const [services, alertsClient] = this.getServicesWithSpaceLevelPermissions(spaceId, apiKey);
 
-      // Ensure API key is still valid and user has access
+    let alert: SanitizedAlert;
+
+    // Ensure API key is still valid and user has access
+    try {
       alert = await alertsClient.get({ id: alertId });
     } catch (err) {
       throw new ErrorWithReason('read', err);

x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/execution_status.ts

@@ -0,0 +1,87 @@
+/*
+ * 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 expect from '@kbn/expect';
+import { Spaces } from '../../scenarios';
+import { getUrlPrefix, getTestAlertData, ObjectRemover } from '../../../common/lib';
+import { FtrProviderContext } from '../../../common/ftr_provider_context';
+
+// eslint-disable-next-line import/no-default-export
+export default function executionStatusAlertTests({ getService }: FtrProviderContext) {
+  const supertest = getService('supertest');
+  const spaceId = Spaces[0].id;
+
+  // the only tests here are those that can't be run in spaces_only
+  describe('executionStatus', () => {
+    const objectRemover = new ObjectRemover(supertest);
+
+    after(async () => await objectRemover.removeAll());
+
+    it('should eventually have error reason "decrypt" when appropriate', async () => {
+      const response = await supertest
+        .post(`${getUrlPrefix(spaceId)}/api/alerts/alert`)
+        .set('kbn-xsrf', 'foo')
+        .send(
+          getTestAlertData({
+            alertTypeId: 'test.noop',
+            schedule: { interval: '1s' },
+          })
+        );
+      expect(response.status).to.eql(200);
+      const alertId = response.body.id;
+      objectRemover.add(spaceId, alertId, 'alert', 'alerts');
+
+      let executionStatus = await waitForStatus(alertId, new Set(['ok']), 10000);
+
+      // break AAD
+      await supertest
+        .put(`${getUrlPrefix(spaceId)}/api/alerts_fixture/saved_object/alert/${alertId}`)
+        .set('kbn-xsrf', 'foo')
+        .send({
+          attributes: {
+            name: 'bar',
+          },
+        })
+        .expect(200);
+
+      executionStatus = await waitForStatus(alertId, new Set(['error']));
+      expect(executionStatus.error).to.be.ok();
+      expect(executionStatus.error.reason).to.be('decrypt');
+      expect(executionStatus.error.message).to.be('Unable to decrypt attribute "apiKey"');
+    });
+  });
+
+  const WaitForStatusIncrement = 500;
+
+  async function waitForStatus(
+    id: string,
+    statuses: Set<string>,
+    waitMillis: number = 10000
+  ): Promise<Record<string, any>> {
+    if (waitMillis < 0) {
+      expect().fail(`waiting for alert ${id} statuses ${Array.from(statuses)} timed out`);
+    }
+
+    const response = await supertest.get(`${getUrlPrefix(spaceId)}/api/alerts/alert/${id}`);
+    expect(response.status).to.eql(200);
+    const { status } = response.body.executionStatus;
+    if (statuses.has(status)) return response.body.executionStatus;
+
+    // eslint-disable-next-line no-console
+    console.log(
+      `waitForStatus(${Array.from(statuses)}): got ${JSON.stringify(
+        response.body.executionStatus
+      )}, retrying`
+    );
+
+    await delay(WaitForStatusIncrement);
+    return await waitForStatus(id, statuses, waitMillis - WaitForStatusIncrement);
+  }
+}
+
+async function delay(millis: number): Promise<void> {
+  await new Promise((resolve) => setTimeout(resolve, millis));
+}

x-pack/test/alerting_api_integration/security_and_spaces/tests/alerting/index.ts

@@ -14,6 +14,7 @@ export default function alertingTests({ loadTestFile }: FtrProviderContext) {
     loadTestFile(require.resolve('./delete'));
     loadTestFile(require.resolve('./disable'));
     loadTestFile(require.resolve('./enable'));
+    loadTestFile(require.resolve('./execution_status'));
     loadTestFile(require.resolve('./get'));
     loadTestFile(require.resolve('./get_alert_state'));
     loadTestFile(require.resolve('./get_alert_instance_summary'));