Rework promise and error-handling in ManifestManager

x-pack/plugins/security_solution/server/endpoint/ingest_integration.ts

@@ -9,6 +9,7 @@ import { NewPackageConfig } from '../../../ingest_manager/common/types/models';
 import { factory as policyConfigFactory } from '../../common/endpoint/models/policy_config';
 import { NewPolicyData } from '../../common/endpoint/types';
 import { ManifestManager } from './services/artifacts';
+import { reportErrors } from './lib/artifacts/common';
 
 /**
  * Callback to handle creation of PackageConfigs in Ingest Manager
@@ -33,54 +34,62 @@ export const getPackageConfigCreateCallback = (
     // with diffs from last dispatched manifest, if it exists
     const snapshot = await manifestManager.getSnapshot({ initialize: true });
 
-    if (snapshot === null) {
-      logger.warn('No manifest snapshot available.');
-      return updatedPackageConfig;
-    }
-
-    if (snapshot.diffs.length > 0) {
-      // create new artifacts
-      await manifestManager.syncArtifacts(snapshot, 'add');
+    let success = true;
+    try {
+      if (snapshot && snapshot.diffs.length > 0) {
+        // create new artifacts
+        const errors = await manifestManager.syncArtifacts(snapshot, 'add');
+        if (errors.length) {
+          reportErrors(logger, errors);
+          throw new Error('Error writing new artifacts.');
+        }
 
-      // Until we get the Default Policy Configuration in the Endpoint package,
-      // we will add it here manually at creation time.
-      // @ts-ignore
-      if (newPackageConfig.inputs.length === 0) {
-        updatedPackageConfig = {
-          ...newPackageConfig,
-          inputs: [
-            {
-              type: 'endpoint',
-              enabled: true,
-              streams: [],
-              config: {
-                artifact_manifest: {
-                  value: snapshot.manifest.toEndpointFormat(),
-                },
-                policy: {
-                  value: policyConfigFactory(),
+        // Until we get the Default Policy Configuration in the Endpoint package,
+        // we will add it here manually at creation time.
+        // @ts-ignore
+        if (newPackageConfig.inputs.length === 0) {
+          updatedPackageConfig = {
+            ...newPackageConfig,
+            inputs: [
+              {
+                type: 'endpoint',
+                enabled: true,
+                streams: [],
+                config: {
+                  artifact_manifest: {
+                    value: snapshot.manifest.toEndpointFormat(),
+                  },
+                  policy: {
+                    value: policyConfigFactory(),
+                  },
                 },
               },
-            },
-          ],
-        };
+            ],
+          };
+        }
       }
-    }
 
-    try {
+      return updatedPackageConfig;
+    } catch (err) {
+      success = false;
+      logger.error(err);
       return updatedPackageConfig;
     } finally {
-      if (snapshot.diffs.length > 0) {
-        // TODO: let's revisit the way this callback happens... use promises?
-        // only commit when we know the package config was created
+      if (success && snapshot !== null) {
         try {
-          await manifestManager.commit(snapshot.manifest);
+          if (snapshot.diffs.length > 0) {
+            // TODO: let's revisit the way this callback happens... use promises?
+            // only commit when we know the package config was created
+            await manifestManager.commit(snapshot.manifest);
 
-          // clean up old artifacts
-          await manifestManager.syncArtifacts(snapshot, 'delete');
+            // clean up old artifacts
+            await manifestManager.syncArtifacts(snapshot, 'delete');
+          }
         } catch (err) {
           logger.error(err);
         }
+      } else if (snapshot === null) {
+        logger.error('No manifest snapshot available.');
       }
     }
   };

x-pack/plugins/security_solution/server/endpoint/lib/artifacts/common.ts

@@ -3,6 +3,7 @@
  * or more contributor license agreements. Licensed under the Elastic License;
  * you may not use this file except in compliance with the Elastic License.
  */
+import { Logger } from 'src/core/server';
 
 export const ArtifactConstants = {
   GLOBAL_ALLOWLIST_NAME: 'endpoint-exceptionlist',
@@ -16,3 +17,9 @@ export const ManifestConstants = {
   SCHEMA_VERSION: 'v1',
   INITIAL_VERSION: 'WzAsMF0=',
 };
+
+export const reportErrors = (logger: Logger, errors: Error[]) => {
+  errors.forEach((err) => {
+    logger.error(err);
+  });
+};

x-pack/plugins/security_solution/server/endpoint/lib/artifacts/task.ts

@@ -11,6 +11,7 @@ import {
   TaskManagerStartContract,
 } from '../../../../../task_manager/server';
 import { EndpointAppContext } from '../../types';
+import { reportErrors } from './common';
 
 export const ManifestTaskConstants = {
   TIMEOUT: '1m',
@@ -88,19 +89,36 @@ export class ManifestTask {
       return;
     }
 
+    let errors: Error[] = [];
     try {
       // get snapshot based on exception-list-agnostic SOs
       // with diffs from last dispatched manifest
       const snapshot = await manifestManager.getSnapshot();
       if (snapshot && snapshot.diffs.length > 0) {
         // create new artifacts
-        await manifestManager.syncArtifacts(snapshot, 'add');
+        errors = await manifestManager.syncArtifacts(snapshot, 'add');
+        if (errors.length) {
+          reportErrors(this.logger, errors);
+          throw new Error('Error writing new artifacts.');
+        }
         // write to ingest-manager package config
-        await manifestManager.dispatch(snapshot.manifest);
+        errors = await manifestManager.dispatch(snapshot.manifest);
+        if (errors.length) {
+          reportErrors(this.logger, errors);
+          throw new Error('Error dispatching manifest.');
+        }
         // commit latest manifest state to user-artifact-manifest SO
-        await manifestManager.commit(snapshot.manifest);
+        const error = await manifestManager.commit(snapshot.manifest);
+        if (error) {
+          reportErrors(this.logger, [error]);
+          throw new Error('Error committing manifest.');
+        }
         // clean up old artifacts
-        await manifestManager.syncArtifacts(snapshot, 'delete');
+        errors = await manifestManager.syncArtifacts(snapshot, 'delete');
+        if (errors.length) {
+          reportErrors(this.logger, errors);
+          throw new Error('Error cleaning up outdated artifacts.');
+        }
       }
     } catch (err) {
       this.logger.error(err);

x-pack/plugins/security_solution/server/endpoint/services/artifacts/manifest_manager/manifest_manager.test.ts

@@ -77,7 +77,7 @@ describe('manifest_manager', () => {
       const manifestManager = getManifestManagerMock({ packageConfigService });
       const snapshot = await manifestManager.getSnapshot();
       const dispatched = await manifestManager.dispatch(snapshot!.manifest);
-      expect(dispatched).toEqual(true);
+      expect(dispatched).toEqual([]);
       const entries = snapshot!.manifest.getEntries();
       const artifact = Object.values(entries)[0].getArtifact();
       expect(
@@ -115,7 +115,7 @@ describe('manifest_manager', () => {
       snapshot!.diffs.push(diff);
 
       const dispatched = await manifestManager.dispatch(snapshot!.manifest);
-      expect(dispatched).toEqual(true);
+      expect(dispatched).toEqual([]);
 
       await manifestManager.commit(snapshot!.manifest);