Skip to content

Commit

Permalink
Refactor latest package fetching to fix broken logic/tests
Browse files Browse the repository at this point in the history
  • Loading branch information
kpollich committed Feb 15, 2022
1 parent 2310ec8 commit 53f3b60
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 36 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/package_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions x-pack/plugins/fleet/server/services/epm/packages/get.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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',
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/packages/get.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/fleet/server/services/epm/packages/install.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 =
Expand Down
72 changes: 45 additions & 27 deletions x-pack/plugins/fleet/server/services/epm/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import type {
InstallSource,
RegistryPackage,
RegistrySearchResults,
RegistrySearchResult,
GetCategoriesRequest,
} from '../../../types';
import {
Expand Down Expand Up @@ -67,20 +66,16 @@ export async function fetchList(params?: SearchParams): Promise<RegistrySearchRe
return fetchUrl(url.toString()).then(JSON.parse);
}

// When `throwIfNotFound` is true or undefined, return type will never be undefined.
export async function fetchFindLatestPackage(
packageName: string,
options?: { ignoreConstraints?: boolean; throwIfNotFound?: true }
): Promise<RegistrySearchResult>;
export async function fetchFindLatestPackage(
packageName: string,
options: { ignoreConstraints?: boolean; throwIfNotFound: false }
): Promise<RegistrySearchResult | undefined>;
export async function fetchFindLatestPackage(
interface FetchFindLatestPackageOptions {
ignoreConstraints?: boolean;
}

async function _fetchFindLatestPackage(
packageName: string,
options?: { ignoreConstraints?: boolean; throwIfNotFound?: boolean }
): Promise<RegistrySearchResult | undefined> {
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`);

Expand All @@ -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 {
Expand Down

0 comments on commit 53f3b60

Please sign in to comment.