From e589f08b1ca57e3f2c8f7d1790f3b991cef02860 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 7 Feb 2023 16:17:45 +0100 Subject: [PATCH 1/3] refactor install registry and upload to extract common logic --- .../server/services/epm/packages/install.ts | 224 ++++++++++-------- 1 file changed, 124 insertions(+), 100 deletions(-) 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..b6a274431f7b2 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, @@ -295,13 +296,9 @@ async function installPackageFromRegistry({ 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'); - // 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; + installType: InstallType; + savedObjectsClient: SavedObjectsClientContract; + esClient: ElasticsearchClient; + spaceId: string; + force?: boolean; + packageInfo: ArchivePackage; + paths: string[]; + verificationResult?: PackageVerificationResult; + telemetryEvent?: PackageUpdateEvent; +}): Promise { + 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 { - // 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: false, + 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(); + }; } } From ba781b93ddc140339d4f409c4fa9d965a084ee07 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 7 Feb 2023 17:07:25 +0100 Subject: [PATCH 2/3] added force:true to upload --- x-pack/plugins/fleet/server/services/epm/packages/install.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 b6a274431f7b2..a1f1d000d0e05 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -582,7 +582,7 @@ async function installPackageByUpload({ savedObjectsClient, esClient, spaceId, - force: false, + force: true, // upload has implicit force packageInfo, paths, }); From 1c56354ae4f706d982cc5daf643decd6f6495215 Mon Sep 17 00:00:00 2001 From: Julia Bardi Date: Tue, 7 Feb 2023 17:30:36 +0100 Subject: [PATCH 3/3] added more unit tests --- .../services/epm/packages/install.test.ts | 44 +++++++++++++++++++ .../server/services/epm/packages/install.ts | 2 +- 2 files changed, 45 insertions(+), 1 deletion(-) 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 a1f1d000d0e05..a3d087d828b01 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -294,7 +294,7 @@ 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; + let pkgVersion = version ?? ''; // if an error happens during getInstallType, report that we don't know let installType: InstallType = 'unknown';