Consider all errors non-fatal during dry runs (#117594) (#117704)

Since we've enabled auto upgrades for some packages in 7.16, we need to
revert a previous change that only considered policy validation errors
non-fatal. Any error encountered during the dry-run/upgrade process
should be considered non-fatal so that users can still access the Fleet
UI if setup fails.

Fixes #116985

Co-authored-by: Kyle Pollich <kyle.pollich@elastic.co>
This commit is contained in:
Kibana Machine 2021-11-05 15:06:10 -04:00 committed by GitHub
parent ef7ef75431
commit 32f77064df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 35 additions and 20 deletions

View file

@ -67,6 +67,12 @@ export type DeletePackagePoliciesResponse = Array<{
export interface UpgradePackagePolicyBaseResponse {
name?: string;
// Support generic errors
statusCode?: number;
body?: {
message: string;
};
}
export interface UpgradePackagePolicyDryRunResponseItem extends UpgradePackagePolicyBaseResponse {

View file

@ -192,6 +192,8 @@ export const deletePackagePolicyHandler: RequestHandler<
}
};
// TODO: Separate the upgrade and dry-run processes into separate endpoints, and address
// duplicate logic in error handling as part of https://github.com/elastic/kibana/issues/63123
export const upgradePackagePolicyHandler: RequestHandler<
unknown,
unknown,
@ -212,6 +214,16 @@ export const upgradePackagePolicyHandler: RequestHandler<
);
body.push(result);
}
const firstFatalError = body.find((item) => item.statusCode && item.statusCode !== 200);
if (firstFatalError) {
return response.customError({
statusCode: firstFatalError.statusCode!,
body: { message: firstFatalError.body!.message },
});
}
return response.ok({
body,
});
@ -222,6 +234,15 @@ export const upgradePackagePolicyHandler: RequestHandler<
request.body.packagePolicyIds,
{ user }
);
const firstFatalError = body.find((item) => item.statusCode && item.statusCode !== 200);
if (firstFatalError) {
return response.customError({
statusCode: firstFatalError.statusCode!,
body: { message: firstFatalError.body!.message },
});
}
return response.ok({
body,
});

View file

@ -72,7 +72,10 @@ export const upgradeManagedPackagePolicies = async (
);
if (dryRunResults.hasErrors) {
const errors = dryRunResults.diff?.[1].errors;
const errors = dryRunResults.diff
? dryRunResults.diff?.[1].errors
: dryRunResults.body?.message;
appContextService
.getLogger()
.error(

View file

@ -620,13 +620,6 @@ class PackagePolicyService {
success: true,
});
} catch (error) {
// We only want to specifically handle validation errors for the new package policy. If a more severe or
// general error is thrown elsewhere during the upgrade process, we want to surface that directly in
// order to preserve any status code mappings, etc that might be included w/ the particular error type
if (!(error instanceof PackagePolicyValidationError)) {
throw error;
}
result.push({
id,
success: false,
@ -704,10 +697,6 @@ class PackagePolicyService {
hasErrors,
};
} catch (error) {
if (!(error instanceof PackagePolicyValidationError)) {
throw error;
}
return {
hasErrors: true,
...ingestErrorToResponseOptions(error),

View file

@ -570,16 +570,14 @@ export default function (providerContext: FtrProviderContext) {
describe('upgrade', function () {
it('fails to upgrade package policy', async function () {
const { body }: { body: UpgradePackagePolicyResponse } = await supertest
await supertest
.post(`/api/fleet/package_policies/upgrade`)
.set('kbn-xsrf', 'xxxx')
.send({
packagePolicyIds: [packagePolicyId],
dryRun: false,
})
.expect(200);
expect(body[0].success).to.be(false);
.expect(400);
});
});
});
@ -672,16 +670,14 @@ export default function (providerContext: FtrProviderContext) {
describe('upgrade', function () {
it('fails to upgrade package policy', async function () {
const { body }: { body: UpgradePackagePolicyResponse } = await supertest
await supertest
.post(`/api/fleet/package_policies/upgrade`)
.set('kbn-xsrf', 'xxxx')
.send({
packagePolicyIds: [packagePolicyId],
dryRun: false,
})
.expect(200);
expect(body[0].success).to.be(false);
.expect(400);
});
});
});