[Ingest Manager] Unify install* under installPackage (#82916)

## Summary * Add `installPackage` with `installSource` param, to provide a single interface the `install*` functions. ```diff - const res = await installPackageFromRegistry({ + const res = await installPackage({ + installSource: 'registry', ``` and ```diff - const res = await installPackageByUpload({ + const res = await installPackage({ + installSource: 'upload', ``` * Push some repeated work (`install`, `removable`) from `install*` into `_installPackage`. Which also simplifies its interface. ### installPackage For now `installPackage` checks the `installSource` and calls the same `install*` functions to prevent any change in behavior but there's still a lot of overlap between `installPackageFromRegistry` & `installPackageByUpload`. I think we can bring them together into `installPackage` using the same branching on `installSource`. ### local checks with curl <details><summary>curl request/responses for happy path:</summary> ``` ## zip: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"response":[{"id":"apache-Logs-Apache-Dashboard-ecs","type":"dashboard"},{"id":"apache-Metrics-Apache-HTTPD-server-status-ecs","type":"dashboard"},{"id":"Apache-HTTPD-CPU-ecs","type":"visualization"},{"id":"Apache-HTTPD-Hostname-list-ecs","type":"visualization"},{"id":"Apache-HTTPD-Load1-slash-5-slash-15-ecs","type":"visualization"},{"id":"Apache-HTTPD-Scoreboard-ecs","type":"visualization"},{"id":"Apache-HTTPD-Total-accesses-and-kbytes-ecs","type":"visualization"},{"id":"Apache-HTTPD-Uptime-ecs","type":"visualization"},{"id":"Apache-HTTPD-Workers-ecs","type":"visualization"},{"id":"Apache-access-unique-IPs-map-ecs","type":"visualization"},{"id":"Apache-browsers-ecs","type":"visualization"},{"id":"Apache-error-logs-over-time-ecs","type":"visualization"},{"id":"Apache-operating-systems-ecs","type":"visualization"},{"id":"Apache-response-codes-of-top-URLs-ecs","type":"visualization"},{"id":"Apache-response-codes-over-time-ecs","type":"visualization"},{"id":"Apache-HTTPD-ecs","type":"search"},{"id":"Apache-access-logs-ecs","type":"search"},{"id":"Apache-errors-log-ecs","type":"search"}]} ## Uploaded packages can be deleted as expected: curl -X DELETE -u elastic:changeme http://localhost:5601/api/fleet/epm/packages/apache-0.1.4 -H 'kbn-xsrf: xxx' {"response":[{"id":"apache-Logs-Apache-Dashboard-ecs","type":"dashboard"},{"id":"apache-Metrics-Apache-HTTPD-server-status-ecs","type":"dashboard"},{"id":"Apache-HTTPD-CPU-ecs","type":"visualization"},{"id":"Apache-HTTPD-Hostname-list-ecs","type":"visualization"},{"id":"Apache-HTTPD-Load1-slash-5-slash-15-ecs","type":"visualization"},{"id":"Apache-HTTPD-Scoreboard-ecs","type":"visualization"},{"id":"Apache-HTTPD-Total-accesses-and-kbytes-ecs","type":"visualization"},{"id":"Apache-HTTPD-Uptime-ecs","type":"visualization"},{"id":"Apache-HTTPD-Workers-ecs","type":"visualization"},{"id":"Apache-access-unique-IPs-map-ecs","type":"visualization"},{"id":"Apache-browsers-ecs","type":"visualization"},{"id":"Apache-error-logs-over-time-ecs","type":"visualization"},{"id":"Apache-operating-systems-ecs","type":"visualization"},{"id":"Apache-response-codes-of-top-URLs-ecs","type":"visualization"},{"id":"Apache-response-codes-over-time-ecs","type":"visualization"},{"id":"Apache-HTTPD-ecs","type":"search"},{"id":"Apache-access-logs-ecs","type":"search"},{"id":"Apache-errors-log-ecs","type":"search"}]} ## Now upload curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_0.1.4.tar.gz -H 'kbn-xsrf: xyz' -H 'Content-Type: application/gzip' {"response":[{"id":"apache-Metrics-Apache-HTTPD-server-status-ecs","type":"dashboard"},{"id":"apache-Logs-Apache-Dashboard-ecs","type":"dashboard"},{"id":"Apache-access-unique-IPs-map-ecs","type":"visualization"},{"id":"Apache-HTTPD-CPU-ecs","type":"visualization"},{"id":"Apache-HTTPD-Load1-slash-5-slash-15-ecs","type":"visualization"},{"id":"Apache-response-codes-over-time-ecs","type":"visualization"},{"id":"Apache-HTTPD-Workers-ecs","type":"visualization"},{"id":"Apache-HTTPD-Hostname-list-ecs","type":"visualization"},{"id":"Apache-error-logs-over-time-ecs","type":"visualization"},{"id":"Apache-HTTPD-Scoreboard-ecs","type":"visualization"},{"id":"Apache-HTTPD-Uptime-ecs","type":"visualization"},{"id":"Apache-operating-systems-ecs","type":"visualization"},{"id":"Apache-HTTPD-Total-accesses-and-kbytes-ecs","type":"visualization"},{"id":"Apache-browsers-ecs","type":"visualization"},{"id":"Apache-response-codes-of-top-URLs-ecs","type":"visualization"},{"id":"Apache-access-logs-ecs","type":"search"},{"id":"Apache-errors-log-ecs","type":"search"},{"id":"Apache-HTTPD-ecs","type":"search"},{"id":"logs-apache.error-0.1.4","type":"ingest_pipeline"},{"id":"logs-apache.access-0.1.4","type":"ingest_pipeline"},{"id":"logs-apache.error","type":"index_template"},{"id":"metrics-apache.status","type":"index_template"},{"id":"logs-apache.access","type":"index_template"}]} ``` </details> <details><summary>curl request/responses for archive errors:</summary> ``` ## Wrong content type: ### tar.gz with application/zip: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_0.1.4.tar.gz -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Error during extraction of package: Error: end of central directory record signature not found. Assumed content type was application/zip, check if this matches the archive type."} ### zip with application/gzip: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/gzip' {"statusCode":400,"error":"Bad Request","message":"Archive seems empty. Assumed content type was application/gzip, check if this matches the archive type."} ## Invalid packages ### Two top-level directories: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_invalid_two_toplevels_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Package contains more than one top-level directory."} ### No manifest: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_invalid_no_manifest_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Package must contain a top-level manifest.yml file."} ### Invalid YAML in manifest: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_invalid_manifest_invalid_yaml_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Could not parse top-level package manifest: YAMLException: bad indentation of a mapping entry at line 2, column 7:\n name: apache\n ^."} ### Mandatory field missing in manifest: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_invalid_manifest_missing_field_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Invalid top-level package manifest: one or more fields missing of name, version, description, type, categories, format_version"} ### Top-level directory doesn't match name and version from manifest: curl -X POST -u elastic:changeme http://localhost:5601/api/fleet/epm/packages --data-binary @$KIBANA_HOME/x-pack/test/ingest_manager_api_integration/apis/fixtures/direct_upload_packages/apache_invalid_toplevel_mismatch_0.1.4.zip -H 'kbn-xsrf: xyz' -H 'Content-Type: application/zip' {"statusCode":400,"error":"Bad Request","message":"Name thisIsATypo and version 0.1.4 do not match top-level directory apache-0.1.4"} ``` </details> #### TS type check examples on `installPackage` <details><summary>screenshots</summary> <img width="379" alt="Screen Shot 2020-11-08 at 4 00 14 PM" src="https://user-images.githubusercontent.com/57655/98484251-1d1e9f80-21dc-11eb-93f8-601036b45355.png"> <img width="890" alt="Screen Shot 2020-11-08 at 4 00 21 PM" src="https://user-images.githubusercontent.com/57655/98484252-1db73600-21dc-11eb-88d1-5faa498f94fc.png"> <img width="396" alt="Screen Shot 2020-11-08 at 4 01 06 PM" src="https://user-images.githubusercontent.com/57655/98484253-1db73600-21dc-11eb-8e2a-10a5762f4a95.png"> <img width="441" alt="Screen Shot 2020-11-08 at 4 01 25 PM" src="https://user-images.githubusercontent.com/57655/98484254-1db73600-21dc-11eb-9d9a-c1620dcad11e.png"> <img width="879" alt="Screen Shot 2020-11-08 at 4 02 54 PM" src="https://user-images.githubusercontent.com/57655/98484255-1db73600-21dc-11eb-8f36-7da3e9256feb.png"> </details> ### Checklist - [ ] [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
elastic · Nov 10, 2020 · 4201f2e · 4201f2e
1 parent ac04243
commit 4201f2e
Show file tree

Hide file tree

Showing 5 changed files with 68 additions and 51 deletions.
diff --git a/x-pack/plugins/fleet/server/routes/epm/handlers.ts b/x-pack/plugins/fleet/server/routes/epm/handlers.ts
@@ -35,8 +35,7 @@ import {
   getPackageInfo,
   handleInstallPackageFailure,
   isBulkInstallError,
-  installPackageFromRegistry,
-  installPackageByUpload,
+  installPackage,
   removeInstallation,
   getLimitedPackages,
   getInstallationObject,
@@ -149,7 +148,8 @@ export const installPackageFromRegistryHandler: RequestHandler<
   const { pkgName, pkgVersion } = splitPkgKey(pkgkey);
   const installedPkg = await getInstallationObject({ savedObjectsClient, pkgName });
   try {
-    const res = await installPackageFromRegistry({
+    const res = await installPackage({
+      installSource: 'registry',
       savedObjectsClient,
       pkgkey,
       callCluster,
@@ -224,7 +224,8 @@ export const installPackageByUploadHandler: RequestHandler<
   const contentType = request.headers['content-type'] as string; // from types it could also be string[] or undefined but this is checked later
   const archiveBuffer = Buffer.from(request.body);
   try {
-    const res = await installPackageByUpload({
+    const res = await installPackage({
+      installSource: 'upload',
       savedObjectsClient,
       callCluster,
       archiveBuffer,

diff --git a/x-pack/plugins/fleet/server/services/epm/packages/_install_package.test.ts b/x-pack/plugins/fleet/server/services/epm/packages/_install_package.test.ts
@@ -61,11 +61,7 @@ describe('_installPackage', () => {
     const installationPromise = _installPackage({
       savedObjectsClient: soClient,
       callCluster,
-      pkgName: 'abc',
-      pkgVersion: '1.2.3',
       paths: [],
-      removable: false,
-      internal: false,
       packageInfo: {
         name: 'xyz',
         version: '4.5.6',

diff --git a/x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts b/x-pack/plugins/fleet/server/services/epm/packages/_install_package.ts
@@ -21,6 +21,7 @@ import { installPipelines, deletePreviousPipelines } from '../elasticsearch/inge
 import { installILMPolicy } from '../elasticsearch/ilm/install';
 import { installKibanaAssets, getKibanaAssets } from '../kibana/assets/install';
 import { updateCurrentWriteIndices } from '../elasticsearch/template/template';
+import { isRequiredPackage } from './index';
 import { deleteKibanaSavedObjectsAssets } from './remove';
 import { installTransform } from '../elasticsearch/transform/install';
 import { createInstallation, saveKibanaAssetsRefs, updateVersion } from './install';
@@ -32,28 +33,22 @@ import { createInstallation, saveKibanaAssetsRefs, updateVersion } from './insta
 export async function _installPackage({
   savedObjectsClient,
   callCluster,
-  pkgName,
-  pkgVersion,
   installedPkg,
   paths,
-  removable,
-  internal,
   packageInfo,
   installType,
   installSource,
 }: {
   savedObjectsClient: SavedObjectsClientContract;
   callCluster: CallESAsCurrentUser;
-  pkgName: string;
-  pkgVersion: string;
   installedPkg?: SavedObject<Installation>;
   paths: string[];
-  removable: boolean;
-  internal: boolean;
   packageInfo: InstallablePackage;
   installType: InstallType;
   installSource: InstallSource;
 }): Promise<AssetReference[]> {
+  const { internal = false, name: pkgName, version: pkgVersion } = packageInfo;
+  const removable = !isRequiredPackage(pkgName);
   const toSaveESIndexPatterns = generateESIndexPatterns(packageInfo.data_streams);
   // add the package installation to the saved object.
   // if some installation already exists, just update install info

diff --git a/x-pack/plugins/fleet/server/services/epm/packages/index.ts b/x-pack/plugins/fleet/server/services/epm/packages/index.ts
@@ -29,8 +29,7 @@ export {
   BulkInstallResponse,
   IBulkInstallPackageError,
   handleInstallPackageFailure,
-  installPackageFromRegistry,
-  installPackageByUpload,
+  installPackage,
   ensureInstalledPackage,
 } from './install';
 export { removeInstallation } from './remove';

diff --git a/x-pack/plugins/fleet/server/services/epm/packages/install.ts b/x-pack/plugins/fleet/server/services/epm/packages/install.ts
@@ -24,7 +24,6 @@ import * as Registry from '../registry';
 import {
   getInstallation,
   getInstallationObject,
-  isRequiredPackage,
   bulkInstallPackages,
   isBulkInstallError,
 } from './index';
@@ -52,7 +51,7 @@ export async function installLatestPackage(options: {
       name: latestPackage.name,
       version: latestPackage.version,
     });
-    return installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster });
+    return installPackage({ installSource: 'registry', savedObjectsClient, pkgkey, callCluster });
   } catch (err) {
     throw err;
   }
@@ -148,7 +147,8 @@ export async function handleInstallPackageFailure({
       }
       const prevVersion = `${pkgName}-${installedPkg.attributes.version}`;
       logger.error(`rolling back to ${prevVersion} after error installing ${pkgkey}`);
-      await installPackageFromRegistry({
+      await installPackage({
+        installSource: 'registry',
         savedObjectsClient,
         pkgkey: prevVersion,
         callCluster,
@@ -186,7 +186,12 @@ export async function upgradePackage({
     });
 
     try {
-      const assets = await installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster });
+      const assets = await installPackage({
+        installSource: 'registry',
+        savedObjectsClient,
+        pkgkey,
+        callCluster,
+      });
       return {
         name: pkgToUpgrade,
         newVersion: latestPkg.version,
@@ -218,19 +223,19 @@ export async function upgradePackage({
   }
 }
 
-interface InstallPackageParams {
+interface InstallRegistryPackageParams {
   savedObjectsClient: SavedObjectsClientContract;
   pkgkey: string;
   callCluster: CallESAsCurrentUser;
   force?: boolean;
 }
 
-export async function installPackageFromRegistry({
+async function installPackageFromRegistry({
   savedObjectsClient,
   pkgkey,
   callCluster,
   force = false,
-}: InstallPackageParams): Promise<AssetReference[]> {
+}: InstallRegistryPackageParams): Promise<AssetReference[]> {
   // TODO: change epm API to /packageName/version so we don't need to do this
   const { pkgName, pkgVersion } = Registry.splitPkgKey(pkgkey);
   // TODO: calls to getInstallationObject, Registry.fetchInfo, and Registry.fetchFindLatestPackge
@@ -250,37 +255,36 @@ export async function installPackageFromRegistry({
 
   const { paths, registryPackageInfo } = await Registry.loadRegistryPackage(pkgName, pkgVersion);
 
-  const removable = !isRequiredPackage(pkgName);
-  const { internal = false } = registryPackageInfo;
-  const installSource = 'registry';
-
   return _installPackage({
     savedObjectsClient,
     callCluster,
-    pkgName,
-    pkgVersion,
     installedPkg,
     paths,
-    removable,
-    internal,
     packageInfo: registryPackageInfo,
     installType,
-    installSource,
+    installSource: 'registry',
   });
 }
 
-export async function installPackageByUpload({
-  savedObjectsClient,
-  callCluster,
-  archiveBuffer,
-  contentType,
-}: {
+interface InstallUploadedArchiveParams {
   savedObjectsClient: SavedObjectsClientContract;
   callCluster: CallESAsCurrentUser;
   archiveBuffer: Buffer;
   contentType: string;
-}): Promise<AssetReference[]> {
+}
+
+export type InstallPackageParams =
+  | ({ installSource: Extract<InstallSource, 'registry'> } & InstallRegistryPackageParams)
+  | ({ installSource: Extract<InstallSource, 'upload'> } & InstallUploadedArchiveParams);
+
+async function installPackageByUpload({
+  savedObjectsClient,
+  callCluster,
+  archiveBuffer,
+  contentType,
+}: InstallUploadedArchiveParams): Promise<AssetReference[]> {
   const { paths, archivePackageInfo } = await loadArchivePackage({ archiveBuffer, contentType });
+
   const installedPkg = await getInstallationObject({
     savedObjectsClient,
     pkgName: archivePackageInfo.name,
@@ -292,25 +296,45 @@ export async function installPackageByUpload({
     );
   }
 
-  const removable = !isRequiredPackage(archivePackageInfo.name);
-  const { internal = false } = archivePackageInfo;
-  const installSource = 'upload';
-
   return _installPackage({
     savedObjectsClient,
     callCluster,
-    pkgName: archivePackageInfo.name,
-    pkgVersion: archivePackageInfo.version,
     installedPkg,
     paths,
-    removable,
-    internal,
     packageInfo: archivePackageInfo,
     installType,
-    installSource,
+    installSource: 'upload',
   });
 }
 
+export async function installPackage(args: InstallPackageParams) {
+  if (!('installSource' in args)) {
+    throw new Error('installSource is required');
+  }
+
+  if (args.installSource === 'registry') {
+    const { savedObjectsClient, pkgkey, callCluster, force } = args;
+
+    return installPackageFromRegistry({
+      savedObjectsClient,
+      pkgkey,
+      callCluster,
+      force,
+    });
+  } else if (args.installSource === 'upload') {
+    const { savedObjectsClient, callCluster, archiveBuffer, contentType } = args;
+
+    return installPackageByUpload({
+      savedObjectsClient,
+      callCluster,
+      archiveBuffer,
+      contentType,
+    });
+  }
+  // @ts-expect-error s/b impossibe b/c `never` by this point, but just in case
+  throw new Error(`Unknown installSource: ${args.installSource}`);
+}
+
 export const updateVersion = async (
   savedObjectsClient: SavedObjectsClientContract,
   pkgName: string,
@@ -421,7 +445,9 @@ export async function ensurePackagesCompletedInstall(
     const pkgkey = `${pkg.attributes.name}-${pkg.attributes.install_version}`;
     // reinstall package
     if (elapsedTime > MAX_TIME_COMPLETE_INSTALL) {
-      acc.push(installPackageFromRegistry({ savedObjectsClient, pkgkey, callCluster }));
+      acc.push(
+        installPackage({ installSource: 'registry', savedObjectsClient, pkgkey, callCluster })
+      );
     }
     return acc;
   }, []);