From 67d1a4695f141d1b5f4f400a6185d4c4d6d9e537 Mon Sep 17 00:00:00 2001 From: Julia Bardi <90178898+juliaElastic@users.noreply.github.com> Date: Wed, 8 Feb 2023 10:12:40 +0100 Subject: [PATCH] [Fleet] refactor install registry and upload to extract common logic (#150444) ## Summary Related to https://github.com/elastic/kibana/issues/148599 Closes https://github.com/elastic/kibana/issues/82007 Factored out common logic in `installPackageFromRegistry` and `installPackageByUpload` to a new function. This improves the upload flow as well to handle install failure. We might want to introduce a `force` flag for the upload API to reinstall a package, now the logic does not do anything if the installed version package is uploaded again. EDIT: found [here](https://github.com/elastic/kibana/issues/82007) that upload should work with an implicit `force` flag, so will update that. More manual and automated tests needed. ### 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 --- .../services/epm/packages/install.test.ts | 44 ++++ .../server/services/epm/packages/install.ts | 226 ++++++++++-------- 2 files changed, 169 insertions(+), 101 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts b/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts index d8f4f4a70adaa..5466c61313d97 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.test.ts @@ -233,6 +233,50 @@ describe('install', () => { expect.objectContaining({ installSource: 'upload' }) ); }); + + it('should fetch latest version if version not provided', async () => { + jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(true); + const response = await installPackage({ + spaceId: DEFAULT_SPACE_ID, + installSource: 'registry', + pkgkey: 'test_package', + savedObjectsClient: savedObjectsClientMock.create(), + esClient: {} as ElasticsearchClient, + }); + + expect(response.status).toEqual('installed'); + + expect(sendTelemetryEvents).toHaveBeenCalledWith( + expect.anything(), + undefined, + expect.objectContaining({ + newVersion: '1.3.0', + }) + ); + }); + + it('should do nothing if same version is installed', async () => { + jest.spyOn(obj, 'getInstallationObject').mockImplementationOnce(() => + Promise.resolve({ + attributes: { + version: '1.2.0', + install_status: 'installed', + installed_es: [], + installed_kibana: [], + }, + } as any) + ); + jest.spyOn(licenseService, 'hasAtLeast').mockReturnValue(true); + const response = await installPackage({ + spaceId: DEFAULT_SPACE_ID, + installSource: 'registry', + pkgkey: 'apache-1.2.0', + savedObjectsClient: savedObjectsClientMock.create(), + esClient: {} as ElasticsearchClient, + }); + + expect(response.status).toEqual('already_installed'); + }); }); describe('upload', () => { 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 cf00ec4e3aed1..a3d087d828b01 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -30,6 +30,7 @@ import { FLEET_INSTALL_FORMAT_VERSION } from '../../../constants/fleet_es_assets import { generateESIndexPatterns } from '../elasticsearch/template/template'; import type { + ArchivePackage, BulkInstallPackageInfo, EpmPackageInstallStatus, EsAssetReference, @@ -293,15 +294,11 @@ async function installPackageFromRegistry({ const logger = appContextService.getLogger(); // TODO: change epm API to /packageName/version so we don't need to do this const { pkgName, pkgVersion: version } = Registry.splitPkgKey(pkgkey); - let pkgVersion = version; - - // Workaround apm issue with async spans: https://github.com/elastic/apm-agent-nodejs/issues/2611 - await Promise.resolve(); - const span = apm.startSpan(`Install package from registry ${pkgName}@${pkgVersion}`, 'package'); + let pkgVersion = version ?? ''; // if an error happens during getInstallType, report that we don't know let installType: InstallType = 'unknown'; - + const installSource = 'registry'; const telemetryEvent: PackageUpdateEvent = getTelemetryEvent(pkgName, pkgVersion); try { @@ -309,11 +306,8 @@ async function installPackageFromRegistry({ const installedPkg = await getInstallationObject({ savedObjectsClient, pkgName }); installType = getInstallType({ pkgVersion, installedPkg }); - span?.addLabels({ - packageName: pkgName, - packageVersion: pkgVersion, - installType, - }); + telemetryEvent.installType = installType; + telemetryEvent.currentVersion = installedPkg?.attributes.version || 'not_installed'; const queryLatest = () => Registry.fetchFindLatestPackageOrThrow(pkgName, { @@ -340,6 +334,100 @@ async function installPackageFromRegistry({ const installOutOfDateVersionOk = force || ['reinstall', 'reupdate', 'rollback'].includes(installType); + // if the requested version is out-of-date of the latest package version, check if we allow it + // if we don't allow it, return an error + if (semverLt(pkgVersion, latestPackage.version)) { + if (!installOutOfDateVersionOk) { + throw new PackageOutdatedError( + `${pkgkey} is out-of-date and cannot be installed or updated` + ); + } + logger.debug( + `${pkgkey} is out-of-date, installing anyway due to ${ + force ? 'force flag' : `install type ${installType}` + }` + ); + } + + return await installPackageCommon({ + pkgName, + pkgVersion, + installSource, + installedPkg, + installType, + savedObjectsClient, + esClient, + spaceId, + force, + packageInfo, + paths, + verificationResult, + }); + } catch (e) { + sendEvent({ + ...telemetryEvent, + errorMessage: e.message, + }); + return { + error: e, + installType, + installSource, + }; + } +} + +async function installPackageCommon(options: { + pkgName: string; + pkgVersion: string; + installSource: 'registry' | 'upload'; + installedPkg?: SavedObject<Installation>; + installType: InstallType; + savedObjectsClient: SavedObjectsClientContract; + esClient: ElasticsearchClient; + spaceId: string; + force?: boolean; + packageInfo: ArchivePackage; + paths: string[]; + verificationResult?: PackageVerificationResult; + telemetryEvent?: PackageUpdateEvent; +}): Promise<InstallResult> { + const { + pkgName, + pkgVersion, + installSource, + installedPkg, + installType, + savedObjectsClient, + force, + esClient, + spaceId, + packageInfo, + paths, + verificationResult, + } = options; + let { telemetryEvent } = options; + const logger = appContextService.getLogger(); + + // Workaround apm issue with async spans: https://github.com/elastic/apm-agent-nodejs/issues/2611 + await Promise.resolve(); + const span = apm.startSpan( + `Install package from ${installSource} ${pkgName}@${pkgVersion}`, + 'package' + ); + + if (!telemetryEvent) { + telemetryEvent = getTelemetryEvent(pkgName, pkgVersion); + telemetryEvent.installType = installType; + telemetryEvent.currentVersion = installedPkg?.attributes.version || 'not_installed'; + } + + try { + span?.addLabels({ + packageName: pkgName, + packageVersion: pkgVersion, + installType, + }); + // if the requested version is the same as installed version, check if we allow it based on // current installed package status and force flag, if we don't allow it, // just return the asset references from the existing installation @@ -348,7 +436,7 @@ async function installPackageFromRegistry({ installedPkg?.attributes.install_status === 'installed' ) { if (!force) { - logger.debug(`${pkgkey} is already installed, skipping installation`); + logger.debug(`${pkgName}-${pkgVersion} is already installed, skipping installation`); return { assets: [ ...installedPkg.attributes.installed_es, @@ -356,36 +444,18 @@ async function installPackageFromRegistry({ ], status: 'already_installed', installType, - installSource: 'registry', + installSource, }; } } - telemetryEvent.installType = installType; - telemetryEvent.currentVersion = installedPkg?.attributes.version || 'not_installed'; - - // if the requested version is out-of-date of the latest package version, check if we allow it - // if we don't allow it, return an error - if (semverLt(pkgVersion, latestPackage.version)) { - if (!installOutOfDateVersionOk) { - throw new PackageOutdatedError( - `${pkgkey} is out-of-date and cannot be installed or updated` - ); - } - logger.debug( - `${pkgkey} is out-of-date, installing anyway due to ${ - force ? 'force flag' : `install type ${installType}` - }` - ); - } - if (!licenseService.hasAtLeast(packageInfo.license || 'basic')) { const err = new Error(`Requires ${packageInfo.license} license`); sendEvent({ ...telemetryEvent, errorMessage: err.message, }); - return { error: err, installType, installSource: 'registry' }; + return { error: err, installType, installSource }; } const savedObjectsImporter = appContextService @@ -415,7 +485,7 @@ async function installPackageFromRegistry({ installType, spaceId, verificationResult, - installSource: 'registry', + installSource, }) .then(async (assets) => { await removeOldAssets({ @@ -424,10 +494,10 @@ async function installPackageFromRegistry({ currentVersion: packageInfo.version, }); sendEvent({ - ...telemetryEvent, + ...telemetryEvent!, status: 'success', }); - return { assets, status: 'installed', installType, installSource: 'registry' }; + return { assets, status: 'installed', installType, installSource }; }) .catch(async (err: Error) => { logger.warn(`Failure to install package [${pkgName}]: [${err.toString()}]`); @@ -441,10 +511,10 @@ async function installPackageFromRegistry({ esClient, }); sendEvent({ - ...telemetryEvent, + ...telemetryEvent!, errorMessage: err.message, }); - return { error: err, installType, installSource: 'registry' }; + return { error: err, installType, installSource }; }); } catch (e) { sendEvent({ @@ -454,7 +524,7 @@ async function installPackageFromRegistry({ return { error: e, installType, - installSource: 'registry', + installSource, }; } finally { span?.end(); @@ -469,16 +539,12 @@ async function installPackageByUpload({ spaceId, version, }: InstallUploadedArchiveParams): Promise<InstallResult> { - // Workaround apm issue with async spans: https://github.com/elastic/apm-agent-nodejs/issues/2611 - await Promise.resolve(); - const span = apm.startSpan(`Install package from upload`, 'package'); - - const logger = appContextService.getLogger(); // if an error happens during getInstallType, report that we don't know let installType: InstallType = 'unknown'; - const telemetryEvent: PackageUpdateEvent = getTelemetryEvent('', ''); + const installSource = 'upload'; try { const { packageInfo } = await generatePackageInfoFromArchiveBuffer(archiveBuffer, contentType); + const pkgName = packageInfo.name; // Allow for overriding the version in the manifest for cases where we install // stack-aligned bundled packages to support special cases around the @@ -487,23 +553,11 @@ async function installPackageByUpload({ const installedPkg = await getInstallationObject({ savedObjectsClient, - pkgName: packageInfo.name, + pkgName, }); installType = getInstallType({ pkgVersion, installedPkg }); - span?.addLabels({ - packageName: packageInfo.name, - packageVersion: pkgVersion, - installType, - }); - - telemetryEvent.packageName = packageInfo.name; - telemetryEvent.newVersion = pkgVersion; - telemetryEvent.installType = installType; - telemetryEvent.currentVersion = installedPkg?.attributes.version || 'not_installed'; - - const installSource = 'upload'; // as we do not verify uploaded packages, we must invalidate the verification cache deleteVerificationResult(packageInfo); const paths = await unpackBufferToCache({ @@ -519,55 +573,25 @@ async function installPackageByUpload({ packageInfo, }); - const savedObjectsImporter = appContextService - .getSavedObjects() - .createImporter(savedObjectsClient); - - const savedObjectTagAssignmentService = appContextService - .getSavedObjectsTagging() - .createInternalAssignmentService({ client: savedObjectsClient }); - - const savedObjectTagClient = appContextService - .getSavedObjectsTagging() - .createTagClient({ client: savedObjectsClient }); - - // @ts-expect-error status is string instead of InstallResult.status 'installed' | 'already_installed' - return await _installPackage({ + return await installPackageCommon({ + pkgName, + pkgVersion, + installSource, + installedPkg, + installType, savedObjectsClient, - savedObjectsImporter, - savedObjectTagAssignmentService, - savedObjectTagClient, esClient, - logger, - installedPkg, + spaceId, + force: true, // upload has implicit force + packageInfo, paths, - packageInfo: { ...packageInfo, version: pkgVersion }, + }); + } catch (e) { + return { + error: e, installType, installSource, - spaceId, - }) - .then((assets) => { - sendEvent({ - ...telemetryEvent, - status: 'success', - }); - return { assets, status: 'installed', installType }; - }) - .catch(async (err: Error) => { - sendEvent({ - ...telemetryEvent, - errorMessage: err.message, - }); - return { error: err, installType }; - }); - } catch (e) { - sendEvent({ - ...telemetryEvent, - errorMessage: e.message, - }); - return { error: e, installType, installSource: 'upload' }; - } finally { - span?.end(); + }; } }