From 4201f2e08946970c38b956039830865fdf2713b6 Mon Sep 17 00:00:00 2001 From: John Schulz Date: Mon, 9 Nov 2020 05:42:45 -0500 Subject: [PATCH] [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
curl request/responses for happy path: ``` ## 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"}]} ```
curl request/responses for archive errors: ``` ## 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"} ```
#### TS type check examples on `installPackage`
screenshots Screen Shot 2020-11-08 at 4 00 14 PM Screen Shot 2020-11-08 at 4 00 21 PM Screen Shot 2020-11-08 at 4 01 06 PM Screen Shot 2020-11-08 at 4 01 25 PM Screen Shot 2020-11-08 at 4 02 54 PM
### 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 --- .../fleet/server/routes/epm/handlers.ts | 9 +- .../epm/packages/_install_package.test.ts | 4 - .../services/epm/packages/_install_package.ts | 11 +-- .../server/services/epm/packages/index.ts | 3 +- .../server/services/epm/packages/install.ts | 92 ++++++++++++------- 5 files changed, 68 insertions(+), 51 deletions(-) diff --git a/x-pack/plugins/fleet/server/routes/epm/handlers.ts b/x-pack/plugins/fleet/server/routes/epm/handlers.ts index 198a54ca84125..1d221b8b1eead 100644 --- 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 index 5d3e8e9ce87d1..b7650d10b6b25 100644 --- 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 index f570984cc61aa..a83d9428b7c93 100644 --- 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; paths: string[]; - removable: boolean; - internal: boolean; packageInfo: InstallablePackage; installType: InstallType; installSource: InstallSource; }): Promise { + 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 index 410a9c0b22537..a1128011d81e6 100644 --- 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 index 0496a6e9aeef1..1791e62ca35e0 100644 --- 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 { +}: InstallRegistryPackageParams): Promise { // 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 { +} + +export type InstallPackageParams = + | ({ installSource: Extract } & InstallRegistryPackageParams) + | ({ installSource: Extract } & InstallUploadedArchiveParams); + +async function installPackageByUpload({ + savedObjectsClient, + callCluster, + archiveBuffer, + contentType, +}: InstallUploadedArchiveParams): Promise { 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; }, []);