[Security Solution][Endpoint] Policy creation callback fixes + Improv…

…ed error handling in user manifest loop (#71269)

* Clean up matcher types

* Rework promise and error-handling in ManifestManager

* Write tests for ingest callback and ensure policy is returned when errors occur

* More tests for ingest callback

* Update tests

* Fix tests

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

x-pack/plugins/ingest_manager/common/mocks.ts

@@ -0,0 +1,43 @@
+/*
+ * 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 { NewPackageConfig, PackageConfig } from './types/models/package_config';
+
+export const createNewPackageConfigMock = () => {
+  return {
+    name: 'endpoint-1',
+    description: '',
+    namespace: 'default',
+    enabled: true,
+    config_id: '93c46720-c217-11ea-9906-b5b8a21b268e',
+    output_id: '',
+    package: {
+      name: 'endpoint',
+      title: 'Elastic Endpoint',
+      version: '0.9.0',
+    },
+    inputs: [],
+  } as NewPackageConfig;
+};
+
+export const createPackageConfigMock = () => {
+  const newPackageConfig = createNewPackageConfigMock();
+  return {
+    ...newPackageConfig,
+    id: 'c6d16e42-c32d-4dce-8a88-113cfe276ad1',
+    version: 'abcd',
+    revision: 1,
+    updated_at: '2020-06-25T16:03:38.159292',
+    updated_by: 'kibana',
+    created_at: '2020-06-25T16:03:38.159292',
+    created_by: 'kibana',
+    inputs: [
+      {
+        config: {},
+      },
+    ],
+  } as PackageConfig;
+};

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

@@ -0,0 +1,91 @@
+/*
+ * 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.
+ */
+
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import { loggerMock } from 'src/core/server/logging/logger.mock';
+import { createNewPackageConfigMock } from '../../../ingest_manager/common/mocks';
+import { factory as policyConfigFactory } from '../../common/endpoint/models/policy_config';
+import { getManifestManagerMock } from './services/artifacts/manifest_manager/manifest_manager.mock';
+import { getPackageConfigCreateCallback } from './ingest_integration';
+
+describe('ingest_integration tests ', () => {
+  describe('ingest_integration sanity checks', () => {
+    test('policy is updated with manifest', async () => {
+      const logger = loggerMock.create();
+      const manifestManager = getManifestManagerMock();
+      const callback = getPackageConfigCreateCallback(logger, manifestManager);
+      const policyConfig = createNewPackageConfigMock();
+      const newPolicyConfig = await callback(policyConfig);
+      expect(newPolicyConfig.inputs[0]!.type).toEqual('endpoint');
+      expect(newPolicyConfig.inputs[0]!.config!.policy.value).toEqual(policyConfigFactory());
+      expect(newPolicyConfig.inputs[0]!.config!.artifact_manifest.value).toEqual({
+        artifacts: {
+          'endpoint-exceptionlist-linux-v1': {
+            compression_algorithm: 'zlib',
+            decoded_sha256: '1a8295e6ccb93022c6f5ceb8997b29f2912389b3b38f52a8f5a2ff7b0154b1bc',
+            decoded_size: 287,
+            encoded_sha256: 'c3dec543df1177561ab2aa74a37997ea3c1d748d532a597884f5a5c16670d56c',
+            encoded_size: 133,
+            encryption_algorithm: 'none',
+            relative_url:
+              '/api/endpoint/artifacts/download/endpoint-exceptionlist-linux-v1/1a8295e6ccb93022c6f5ceb8997b29f2912389b3b38f52a8f5a2ff7b0154b1bc',
+          },
+        },
+        manifest_version: 'WzAsMF0=',
+        schema_version: 'v1',
+      });
+    });
+
+    test('policy is returned even if error is encountered during artifact sync', async () => {
+      const logger = loggerMock.create();
+      const manifestManager = getManifestManagerMock();
+      manifestManager.syncArtifacts = jest.fn().mockRejectedValue([new Error('error updating')]);
+      const lastDispatched = await manifestManager.getLastDispatchedManifest();
+      const callback = getPackageConfigCreateCallback(logger, manifestManager);
+      const policyConfig = createNewPackageConfigMock();
+      const newPolicyConfig = await callback(policyConfig);
+      expect(newPolicyConfig.inputs[0]!.type).toEqual('endpoint');
+      expect(newPolicyConfig.inputs[0]!.config!.policy.value).toEqual(policyConfigFactory());
+      expect(newPolicyConfig.inputs[0]!.config!.artifact_manifest.value).toEqual(
+        lastDispatched.toEndpointFormat()
+      );
+    });
+
+    test('initial policy creation succeeds if snapshot retrieval fails', async () => {
+      const logger = loggerMock.create();
+      const manifestManager = getManifestManagerMock();
+      const lastDispatched = await manifestManager.getLastDispatchedManifest();
+      manifestManager.getSnapshot = jest.fn().mockResolvedValue(null);
+      const callback = getPackageConfigCreateCallback(logger, manifestManager);
+      const policyConfig = createNewPackageConfigMock();
+      const newPolicyConfig = await callback(policyConfig);
+      expect(newPolicyConfig.inputs[0]!.type).toEqual('endpoint');
+      expect(newPolicyConfig.inputs[0]!.config!.policy.value).toEqual(policyConfigFactory());
+      expect(newPolicyConfig.inputs[0]!.config!.artifact_manifest.value).toEqual(
+        lastDispatched.toEndpointFormat()
+      );
+    });
+
+    test('subsequent policy creations succeed', async () => {
+      const logger = loggerMock.create();
+      const manifestManager = getManifestManagerMock();
+      const snapshot = await manifestManager.getSnapshot();
+      manifestManager.getLastDispatchedManifest = jest.fn().mockResolvedValue(snapshot!.manifest);
+      manifestManager.getSnapshot = jest.fn().mockResolvedValue({
+        manifest: snapshot!.manifest,
+        diffs: [],
+      });
+      const callback = getPackageConfigCreateCallback(logger, manifestManager);
+      const policyConfig = createNewPackageConfigMock();
+      const newPolicyConfig = await callback(policyConfig);
+      expect(newPolicyConfig.inputs[0]!.type).toEqual('endpoint');
+      expect(newPolicyConfig.inputs[0]!.config!.policy.value).toEqual(policyConfigFactory());
+      expect(newPolicyConfig.inputs[0]!.config!.artifact_manifest.value).toEqual(
+        snapshot!.manifest.toEndpointFormat()
+      );
+    });
+  });
+});

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

@@ -8,7 +8,9 @@ import { Logger } from '../../../../../src/core/server';
 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 { ManifestManager, ManifestSnapshot } from './services/artifacts';
+import { reportErrors, ManifestConstants } from './lib/artifacts/common';
+import { ManifestSchemaVersion } from '../../common/endpoint/schema/common';
 
 /**
  * Callback to handle creation of PackageConfigs in Ingest Manager
@@ -29,58 +31,83 @@ export const getPackageConfigCreateCallback = (
     // follow the types/schema expected
     let updatedPackageConfig = newPackageConfig as NewPolicyData;
 
-    // get snapshot based on exception-list-agnostic SOs
-    // with diffs from last dispatched manifest, if it exists
-    const snapshot = await manifestManager.getSnapshot({ initialize: true });
+    // get current manifest from SO (last dispatched)
+    const manifest = (
+      await manifestManager.getLastDispatchedManifest(ManifestConstants.SCHEMA_VERSION)
+    )?.toEndpointFormat() ?? {
+      manifest_version: 'default',
+      schema_version: ManifestConstants.SCHEMA_VERSION as ManifestSchemaVersion,
+      artifacts: {},
+    };
 
-    if (snapshot === null) {
-      logger.warn('No manifest snapshot available.');
-      return updatedPackageConfig;
+    // Until we get the Default Policy Configuration in the Endpoint package,
+    // we will add it here manually at creation time.
+    if (newPackageConfig.inputs.length === 0) {
+      updatedPackageConfig = {
+        ...newPackageConfig,
+        inputs: [
+          {
+            type: 'endpoint',
+            enabled: true,
+            streams: [],
+            config: {
+              artifact_manifest: {
+                value: manifest,
+              },
+              policy: {
+                value: policyConfigFactory(),
+              },
+            },
+          },
+        ],
+      };
     }
 
-    if (snapshot.diffs.length > 0) {
-      // create new artifacts
-      await manifestManager.syncArtifacts(snapshot, 'add');
+    let snapshot: ManifestSnapshot | null = null;
+    let success = true;
+    try {
+      // Try to get most up-to-date manifest data.
 
-      // 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(),
-                },
-              },
-            },
-          ],
+      // get snapshot based on exception-list-agnostic SOs
+      // with diffs from last dispatched manifest, if it exists
+      snapshot = await manifestManager.getSnapshot({ initialize: true });
+
+      if (snapshot && snapshot.diffs.length) {
+        // create new artifacts
+        const errors = await manifestManager.syncArtifacts(snapshot, 'add');
+        if (errors.length) {
+          reportErrors(logger, errors);
+          throw new Error('Error writing new artifacts.');
+        }
+      }
+
+      if (snapshot) {
+        updatedPackageConfig.inputs[0].config.artifact_manifest = {
+          value: snapshot.manifest.toEndpointFormat(),
         };
       }
-    }
 
-    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/mocks.ts

@@ -6,6 +6,8 @@
 
 import { ILegacyScopedClusterClient, SavedObjectsClientContract } from 'kibana/server';
 import { loggingSystemMock, savedObjectsServiceMock } from 'src/core/server/mocks';
+// eslint-disable-next-line @kbn/eslint/no-restricted-paths
+import { loggerMock } from 'src/core/server/logging/logger.mock';
 import { xpackMocks } from '../../../../mocks';
 import {
   AgentService,
@@ -63,8 +65,8 @@ export const createMockEndpointAppContextServiceStartContract = (): jest.Mocked<
 > => {
   return {
     agentService: createMockAgentService(),
+    logger: loggerMock.create(),
     savedObjectsStart: savedObjectsServiceMock.createStartContract(),
-    // @ts-ignore
     manifestManager: getManifestManagerMock(),
     registerIngestCallback: jest.fn<
       ReturnType<IngestManagerStartContract['registerExternalCallback']>,