Skip to content

Commit

Permalink
[Fleet] Finer-grained error information from install/upgrade API (#95649
Browse files Browse the repository at this point in the history
)

* Intercept installation errors and add meta info.

* Adjust mock.

* Catch errors in all steps of install/upgrade.

* Adjust handler for direct package upload.

* Don't throw not-found errors on assets during rollback.

* Correctly catch errors from _installPackage()

* Propagate error from installResult in bulk install case.

* Add tests for rollback.

* Remove unused code.

* Skipping test that doesn't test what it says.

* Fix and reenable test.
  • Loading branch information
skh authored Apr 18, 2021
1 parent fed17c2 commit 05bd1c0
Show file tree
Hide file tree
Showing 15 changed files with 327 additions and 140 deletions.
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/common/types/models/epm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export enum InstallStatus {
uninstalling = 'uninstalling',
}

export type InstallType = 'reinstall' | 'reupdate' | 'rollback' | 'update' | 'install';
export type InstallType = 'reinstall' | 'reupdate' | 'rollback' | 'update' | 'install' | 'unknown';
export type InstallSource = 'registry' | 'upload';

export type EpmPackageInstallStatus = 'installed' | 'installing';
Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/fleet/common/types/rest_spec/epm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
RegistrySearchResult,
PackageInfo,
PackageUsageStats,
InstallType,
} from '../models/epm';

export interface GetCategoriesRequest {
Expand Down Expand Up @@ -83,8 +84,10 @@ export interface IBulkInstallPackageHTTPError {
}

export interface InstallResult {
assets: AssetReference[];
status: 'installed' | 'already_installed';
assets?: AssetReference[];
status?: 'installed' | 'already_installed';
error?: Error;
installType: InstallType;
}

export interface BulkInstallPackageInfo {
Expand Down
46 changes: 24 additions & 22 deletions x-pack/plugins/fleet/server/routes/epm/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,20 +226,21 @@ export const installPackageFromRegistryHandler: RequestHandler<
const savedObjectsClient = context.core.savedObjects.client;
const esClient = context.core.elasticsearch.client.asCurrentUser;
const { pkgkey } = request.params;
try {
const res = await installPackage({
installSource: 'registry',
savedObjectsClient,
pkgkey,
esClient,
force: request.body?.force,
});

const res = await installPackage({
installSource: 'registry',
savedObjectsClient,
pkgkey,
esClient,
force: request.body?.force,
});
if (!res.error) {
const body: InstallPackageResponse = {
response: res.assets,
response: res.assets || [],
};
return response.ok({ body });
} catch (e) {
return await defaultIngestErrorHandler({ error: e, response });
} else {
return await defaultIngestErrorHandler({ error: res.error, response });
}
};

Expand Down Expand Up @@ -292,20 +293,21 @@ export const installPackageByUploadHandler: RequestHandler<
const esClient = context.core.elasticsearch.client.asCurrentUser;
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 installPackage({
installSource: 'upload',
savedObjectsClient,
esClient,
archiveBuffer,
contentType,
});

const res = await installPackage({
installSource: 'upload',
savedObjectsClient,
esClient,
archiveBuffer,
contentType,
});
if (!res.error) {
const body: InstallPackageResponse = {
response: res.assets,
response: res.assets || [],
};
return response.ok({ body });
} catch (error) {
return defaultIngestErrorHandler({ error, response });
} else {
return defaultIngestErrorHandler({ error: res.error, response });
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,22 +32,27 @@ export async function bulkInstallPackages({
);

logger.debug(`kicking off bulk install of ${packagesToInstall.join(', ')} from registry`);
const installResults = await Promise.allSettled(
const bulkInstallResults = await Promise.allSettled(
latestPackagesResults.map(async (result, index) => {
const packageName = packagesToInstall[index];
if (result.status === 'fulfilled') {
const latestPackage = result.value;
return {
name: packageName,
version: latestPackage.version,
result: await installPackage({
savedObjectsClient,
esClient,
pkgkey: Registry.pkgToPkgKey(latestPackage),
installSource,
skipPostInstall: true,
}),
};
const installResult = await installPackage({
savedObjectsClient,
esClient,
pkgkey: Registry.pkgToPkgKey(latestPackage),
installSource,
skipPostInstall: true,
});
if (installResult.error) {
return { name: packageName, error: installResult.error };
} else {
return {
name: packageName,
version: latestPackage.version,
result: installResult,
};
}
}
return { name: packageName, error: result.reason };
})
Expand All @@ -56,18 +61,27 @@ export async function bulkInstallPackages({
// only install index patterns if we completed install for any package-version for the
// first time, aka fresh installs or upgrades
if (
installResults.find(
(result) => result.status === 'fulfilled' && result.value.result?.status === 'installed'
bulkInstallResults.find(
(result) =>
result.status === 'fulfilled' &&
!result.value.result?.error &&
result.value.result?.status === 'installed'
)
) {
await installIndexPatterns({ savedObjectsClient, esClient, installSource });
}

return installResults.map((result, index) => {
return bulkInstallResults.map((result, index) => {
const packageName = packagesToInstall[index];
return result.status === 'fulfilled'
? result.value
: { name: packageName, error: result.reason };
if (result.status === 'fulfilled') {
if (result.value && result.value.error) {
return { name: packageName, error: result.value.error };
} else {
return result.value;
}
} else {
return { name: packageName, error: result.reason };
}
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe('ensureInstalledDefaultPackages', () => {
return [
{
name: mockInstallation.attributes.name,
result: { assets: [], status: 'installed' },
result: { assets: [], status: 'installed', installType: 'install' },
version: '',
statusCode: 200,
},
Expand All @@ -95,13 +95,13 @@ describe('ensureInstalledDefaultPackages', () => {
return [
{
name: 'success one',
result: { assets: [], status: 'installed' },
result: { assets: [], status: 'installed', installType: 'install' },
version: '',
statusCode: 200,
},
{
name: 'success two',
result: { assets: [], status: 'installed' },
result: { assets: [], status: 'installed', installType: 'install' },
version: '',
statusCode: 200,
},
Expand All @@ -111,7 +111,7 @@ describe('ensureInstalledDefaultPackages', () => {
},
{
name: 'success three',
result: { assets: [], status: 'installed' },
result: { assets: [], status: 'installed', installType: 'install' },
version: '',
statusCode: 200,
},
Expand All @@ -134,7 +134,7 @@ describe('ensureInstalledDefaultPackages', () => {
return [
{
name: 'undefined package',
result: { assets: [], status: 'installed' },
result: { assets: [], status: 'installed', installType: 'install' },
version: '',
statusCode: 200,
},
Expand Down
Loading

0 comments on commit 05bd1c0

Please sign in to comment.