From 53f3b60b81080c186de2061b8ea6ed87ad5ad713 Mon Sep 17 00:00:00 2001 From: Kyle Pollich Date: Tue, 15 Feb 2022 14:49:16 -0500 Subject: [PATCH] Refactor latest package fetching to fix broken logic/tests --- .../server/services/epm/package_service.ts | 4 +- .../server/services/epm/packages/get.test.ts | 6 +- .../fleet/server/services/epm/packages/get.ts | 4 +- .../server/services/epm/packages/install.ts | 6 +- .../server/services/epm/registry/index.ts | 72 ++++++++++++------- 5 files changed, 56 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/fleet/server/services/epm/package_service.ts b/x-pack/plugins/fleet/server/services/epm/package_service.ts index 0d9b8cb74b503..68327932afdbf 100644 --- a/x-pack/plugins/fleet/server/services/epm/package_service.ts +++ b/x-pack/plugins/fleet/server/services/epm/package_service.ts @@ -25,7 +25,7 @@ import { checkSuperuser } from '../../routes/security'; import { FleetUnauthorizedError } from '../../errors'; import { installTransform, isTransform } from './elasticsearch/transform/install'; -import { fetchFindLatestPackage, getRegistryPackage } from './registry'; +import { fetchFindLatestPackageOrThrow, getRegistryPackage } from './registry'; import { ensureInstalledPackage, getInstallation } from './packages'; export type InstalledAssetType = EsAssetReference; @@ -117,7 +117,7 @@ class PackageClientImpl implements PackageClient { public async fetchFindLatestPackage(packageName: string) { await this.#runPreflight(); - return fetchFindLatestPackage(packageName); + return fetchFindLatestPackageOrThrow(packageName); } public async getRegistryPackage(packageName: string, packageVersion: string) { diff --git a/x-pack/plugins/fleet/server/services/epm/packages/get.test.ts b/x-pack/plugins/fleet/server/services/epm/packages/get.test.ts index 8bed40a9a2522..5c4c9bb2a907b 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/get.test.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/get.test.ts @@ -283,8 +283,8 @@ describe('When using EPM `get` services', () => { }); describe('registry fetch errors', () => { - it('throws when a package that is not installed is not available in the registry', async () => { - MockRegistry.fetchFindLatestPackage.mockResolvedValue(undefined); + it('throws when a package that is not installed is not available in the registry and not bundled', async () => { + MockRegistry.fetchFindLatestPackageWithFallbackToBundled.mockResolvedValue(undefined); const soClient = savedObjectsClientMock.create(); soClient.get.mockRejectedValue(SavedObjectsErrorHelpers.createGenericNotFoundError()); @@ -298,7 +298,7 @@ describe('When using EPM `get` services', () => { }); it('sets the latestVersion to installed version when an installed package is not available in the registry', async () => { - MockRegistry.fetchFindLatestPackage.mockResolvedValue(undefined); + MockRegistry.fetchFindLatestPackageWithFallbackToBundled.mockResolvedValue(undefined); const soClient = savedObjectsClientMock.create(); soClient.get.mockResolvedValue({ id: 'my-package', diff --git a/x-pack/plugins/fleet/server/services/epm/packages/get.ts b/x-pack/plugins/fleet/server/services/epm/packages/get.ts index cf739c837da5a..c0f45083617aa 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/get.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/get.ts @@ -106,7 +106,7 @@ export async function getPackageInfoFromRegistry(options: { const { savedObjectsClient, pkgName, pkgVersion } = options; const [savedObject, latestPackage] = await Promise.all([ getInstallationObject({ savedObjectsClient, pkgName }), - Registry.fetchFindLatestPackage(pkgName), + Registry.fetchFindLatestPackageOrThrow(pkgName), ]); // If no package version is provided, use the installed version in the response @@ -146,7 +146,7 @@ export async function getPackageInfo(options: { const [savedObject, latestPackage] = await Promise.all([ getInstallationObject({ savedObjectsClient, pkgName }), - Registry.fetchFindLatestPackageWithFallbackToBundled(pkgName), + Registry.fetchFindLatestPackageWithFallbackToBundled(pkgName, { throwIfNotFound: false }), ]); if (!savedObject && !latestPackage) { 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 b441bd4f432a6..fd3664d2c9ea3 100644 --- a/x-pack/plugins/fleet/server/services/epm/packages/install.ts +++ b/x-pack/plugins/fleet/server/services/epm/packages/install.ts @@ -89,7 +89,7 @@ export async function ensureInstalledPackage(options: { // If pkgVersion isn't specified, find the latest package version const pkgKeyProps = pkgVersion ? { name: pkgName, version: pkgVersion } - : await Registry.fetchFindLatestPackage(pkgName); + : await Registry.fetchFindLatestPackageOrThrow(pkgName); const installedPackageResult = await isPackageVersionOrLaterInstalled({ savedObjectsClient, @@ -252,7 +252,9 @@ async function installPackageFromRegistry({ installType = getInstallType({ pkgVersion, installedPkg }); // get latest package version - const latestPackage = await Registry.fetchFindLatestPackage(pkgName, { ignoreConstraints }); + const latestPackage = await Registry.fetchFindLatestPackageOrThrow(pkgName, { + ignoreConstraints, + }); // let the user install if using the force flag or needing to reinstall or install a previous version due to failed update const installOutOfDateVersionOk = diff --git a/x-pack/plugins/fleet/server/services/epm/registry/index.ts b/x-pack/plugins/fleet/server/services/epm/registry/index.ts index 05843db24e115..e2c9ce6d0b2b5 100644 --- a/x-pack/plugins/fleet/server/services/epm/registry/index.ts +++ b/x-pack/plugins/fleet/server/services/epm/registry/index.ts @@ -21,7 +21,6 @@ import type { InstallSource, RegistryPackage, RegistrySearchResults, - RegistrySearchResult, GetCategoriesRequest, } from '../../../types'; import { @@ -67,20 +66,16 @@ export async function fetchList(params?: SearchParams): Promise; -export async function fetchFindLatestPackage( - packageName: string, - options: { ignoreConstraints?: boolean; throwIfNotFound: false } -): Promise; -export async function fetchFindLatestPackage( +interface FetchFindLatestPackageOptions { + ignoreConstraints?: boolean; +} + +async function _fetchFindLatestPackage( packageName: string, - options?: { ignoreConstraints?: boolean; throwIfNotFound?: boolean } -): Promise { - const { ignoreConstraints = false, throwIfNotFound = true } = options ?? {}; + options?: FetchFindLatestPackageOptions +) { + const { ignoreConstraints = false } = options ?? {}; + const registryUrl = getRegistryUrl(); const url = new URL(`${registryUrl}/search?package=${packageName}&experimental=true`); @@ -89,34 +84,57 @@ export async function fetchFindLatestPackage( } const res = await fetchUrl(url.toString(), 1); - const searchResults = JSON.parse(res); - if (searchResults.length) { - return searchResults[0]; - } else if (throwIfNotFound) { + const searchResults: RegistryPackage[] = JSON.parse(res); + + return searchResults; +} + +export async function fetchFindLatestPackageOrThrow( + packageName: string, + options?: FetchFindLatestPackageOptions +) { + const searchResults = await _fetchFindLatestPackage(packageName, options); + + if (!searchResults.length) { throw new PackageNotFoundError(`[${packageName}] package not found in registry`); } + + return searchResults[0]; } -export async function fetchFindLatestPackageWithFallbackToBundled( +export async function fetchFindLatestPackageOrUndefined( packageName: string, - options?: { ignoreConstraints?: boolean } + options?: FetchFindLatestPackageOptions ) { - const { ignoreConstraints = false } = options ?? {}; + const searchResults = await _fetchFindLatestPackage(packageName, options); + + if (!searchResults.length) { + return undefined; + } + return searchResults[0]; +} + +export async function fetchFindLatestPackageWithFallbackToBundled( + packageName: string, + options?: FetchFindLatestPackageOptions & { throwIfNotFound?: boolean } +) { try { - const latestPackage = await fetchFindLatestPackage(packageName, { - ignoreConstraints, - throwIfNotFound: true, - }); + const latestPackage = await fetchFindLatestPackageOrThrow(packageName, options); return latestPackage; } catch (error) { const bundledPackages = await getBundledPackages(); const bundledPackage = bundledPackages.find((b) => b.name === packageName); - // If we don't find a bundled package, re-throw the original error if (!bundledPackage) { - throw error; + // Callers must opt into re-throwing the original error from the registry request + if (options?.throwIfNotFound) { + throw error; + } else { + // By default, we'll just return undefined + return undefined; + } } return {