Skip to content

Commit

Permalink
[Fleet] rollback input package install on failure (#182665)
Browse files Browse the repository at this point in the history
## Summary

Closes #181032

2 improvements on input package policy creation failure handling:
- if the package was not installed initially, rolling back on failure
- only saving es references with the input package installation if the
templates are added successfully, to prevent issues with upgrade later
if the references would contain invalid template names
- this is needed if the input package was installed before attempting to
add a package policy, in this case we don't want to completely uninstall
the package on failure

To verify:
Custom Logs package uninstalled:
- add Custom Logs integration with dataset name with a * in it e.g.
`generic*`
- the package policy creation is expected to fail
- verify that the Custom Logs package is not installed 

Custom Logs package installed:
- Install Custom Logs package without package policy or add integration
with the default dataset name to succeed
- try adding another policy with dataset `generic*`
- the package policy creation is expected to fail
- verify that the Custom Logs package doesn't have any `installed_es`
references with the invalid `generic*` prefix
`GET .kibana_ingest/_search?q=epm-packages.name:log`


### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

---------

Co-authored-by: Jen Huang <its.jenetic@gmail.com>
  • Loading branch information
juliaElastic and jen-huang authored May 8, 2024
1 parent 718e6f0 commit 0833045
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,8 @@ export function useOnSubmit({
setFormState('LOADING');
if ((withSysMonitoring || newAgentPolicy.monitoring_enabled?.length) ?? 0 > 0) {
const packagesToPreinstall: Array<string | { name: string; version: string }> = [];
if (packageInfo) {
// skip preinstall of input package, to be able to rollback when package policy creation fails
if (packageInfo && packageInfo.type !== 'input') {
packagesToPreinstall.push({ name: packageInfo.name, version: packageInfo.version });
}
if (withSysMonitoring) {
Expand Down
35 changes: 34 additions & 1 deletion x-pack/plugins/fleet/server/routes/package_policy/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ import {
PackagePolicyNotFoundError,
PackagePolicyRequestError,
} from '../../errors';
import { getInstallations, getPackageInfo } from '../../services/epm/packages';
import {
getInstallation,
getInstallations,
getPackageInfo,
removeInstallation,
} from '../../services/epm/packages';
import { PACKAGES_SAVED_OBJECT_TYPE, SO_SEARCH_LIMIT } from '../../constants';
import {
simplifiedPackagePolicytoNewPackagePolicy,
Expand Down Expand Up @@ -229,6 +234,7 @@ export const createPackagePolicyHandler: FleetRequestHandler<
const user = appContextService.getSecurity()?.authc.getCurrentUser(request) || undefined;
const { force, id, package: pkg, ...newPolicy } = request.body;
const authorizationHeader = HTTPAuthorizationHeader.parseFromRequest(request, user?.username);
let wasPackageAlreadyInstalled = false;

if ('output_id' in newPolicy) {
// TODO Remove deprecated APIs https://github.com/elastic/kibana/issues/121485
Expand Down Expand Up @@ -258,6 +264,12 @@ export const createPackagePolicyHandler: FleetRequestHandler<
} as NewPackagePolicy);
}

const installation = await getInstallation({
savedObjectsClient: soClient,
pkgName: pkg!.name,
});
wasPackageAlreadyInstalled = installation?.install_status === 'installed';

// Create package policy
const packagePolicy = await fleetContext.packagePolicyService.asCurrentUser.create(
soClient,
Expand All @@ -282,6 +294,27 @@ export const createPackagePolicyHandler: FleetRequestHandler<
},
});
} catch (error) {
appContextService
.getLogger()
.error(`Error while creating package policy due to error: ${error.message}`);
if (!wasPackageAlreadyInstalled) {
const installation = await getInstallation({
savedObjectsClient: soClient,
pkgName: pkg!.name,
});
if (installation) {
appContextService
.getLogger()
.info(`rollback ${pkg!.name}-${pkg!.version} package installation after error`);
await removeInstallation({
savedObjectsClient: soClient,
pkgName: pkg!.name,
pkgVersion: pkg!.version,
esClient,
});
}
}

if (error.statusCode) {
return response.customError({
statusCode: error.statusCode,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,16 +68,7 @@ export async function installIndexTemplatesAndPipelines({
// cleanup in the case that a single asset fails to install.
let newEsReferences: EsAssetReference[] = [];

if (onlyForDataStreams) {
// if onlyForDataStreams is present that means we are in create package policy flow
// not install flow, meaning we do not have a lock on the installation SO
// so we need to use optimistic concurrency control
newEsReferences = await optimisticallyAddEsAssetReferences(
savedObjectsClient,
packageInstallContext.packageInfo.name,
[...preparedIngestPipelines.assetsToAdd, ...preparedIndexTemplates.assetsToAdd]
);
} else {
if (!onlyForDataStreams) {
newEsReferences = await updateEsAssetReferences(
savedObjectsClient,
packageInstallContext.packageInfo.name,
Expand All @@ -103,6 +94,18 @@ export async function installIndexTemplatesAndPipelines({
),
]);

// only add ES references if templates and pipelines were installed successfully, to prevent upgrade issues for referencing invalid template name
if (onlyForDataStreams) {
// if onlyForDataStreams is present that means we are in create package policy flow
// not install flow, meaning we do not have a lock on the installation SO
// so we need to use optimistic concurrency control
newEsReferences = await optimisticallyAddEsAssetReferences(
savedObjectsClient,
packageInstallContext.packageInfo.name,
[...preparedIngestPipelines.assetsToAdd, ...preparedIndexTemplates.assetsToAdd]
);
}

return {
esReferences: newEsReferences,
installedTemplates,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ export default function loadTests({ loadTestFile, getService }) {
loadTestFile(require.resolve('./delete'));
loadTestFile(require.resolve('./upgrade'));
loadTestFile(require.resolve('./input_package_create_upgrade'));
loadTestFile(require.resolve('./input_package_rollback'));
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import expect from '@kbn/expect';
import { sortBy } from 'lodash';
import { FtrProviderContext } from '../../../api_integration/ftr_provider_context';
import { skipIfNoDockerRegistry } from '../../helpers';
import { setupFleetAndAgents } from '../agents/services';
const PACKAGE_NAME = 'input_package_upgrade';
const START_VERSION = '1.0.0';

const expectIdArraysEqual = (arr1: any[], arr2: any[]) => {
expect(sortBy(arr1, 'id')).to.eql(sortBy(arr2, 'id'));
};
export default function (providerContext: FtrProviderContext) {
const { getService } = providerContext;
const supertest = getService('supertest');
const uninstallPackage = async (name: string, version: string) => {
await supertest.delete(`/api/fleet/epm/packages/${name}/${version}`).set('kbn-xsrf', 'xxxx');
};

const installPackage = async (name: string, version: string) => {
return await supertest
.post(`/api/fleet/epm/packages/${name}/${version}`)
.set('kbn-xsrf', 'xxxx')
.send({ force: true })
.expect(200);
};

const getInstallationSavedObject = async (name: string, version: string) => {
const res = await supertest.get(`/api/fleet/epm/packages/${name}/${version}`).expect(200);
return res.body.item.savedObject.attributes;
};

const getPackage = async (name: string, version: string) => {
const res = await supertest.get(`/api/fleet/epm/packages/${name}/${version}`).expect(200);
return res.body.item;
};

const createPackagePolicyWithDataset = async (
agentPolicyId: string,
dataset: string,
expectStatusCode = 200,
force = false
) => {
const policy = {
force,
policy_id: agentPolicyId,
package: {
name: PACKAGE_NAME,
version: START_VERSION,
},
name: 'test-policy-' + dataset,
description: '',
namespace: 'default',
inputs: {
'logs-logfile': {
enabled: true,
streams: {
'input_package_upgrade.logs': {
enabled: true,
vars: {
paths: ['/tmp/test/log'],
tags: ['tag1'],
ignore_older: '72h',
'data_stream.dataset': dataset,
},
},
},
},
},
};
const res = await supertest
.post(`/api/fleet/package_policies`)
.set('kbn-xsrf', 'xxxx')
.send(policy)
.expect(expectStatusCode);

return res.body.item;
};

const createAgentPolicy = async (name = 'Input Package Test 3') => {
const res = await supertest
.post(`/api/fleet/agent_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name,
namespace: 'default',
})
.expect(200);
return res.body.item;
};

const deleteAgentPolicy = async (agentPolicyId: string) => {
if (!agentPolicyId) return;
return supertest
.post(`/api/fleet/agent_policies/delete`)
.set('kbn-xsrf', 'xxxx')
.send({ agentPolicyId });
};

describe('input package policy rollback', async function () {
skipIfNoDockerRegistry(providerContext);

let agentPolicyId: string;
before(async () => {
const agentPolicy = await createAgentPolicy();
agentPolicyId = agentPolicy.id;
});

after(async () => {
await deleteAgentPolicy(agentPolicyId);
});
setupFleetAndAgents(providerContext);

it('should rollback package install on package policy create failure', async () => {
await createPackagePolicyWithDataset(agentPolicyId, 'test*', 400);

const pkg = await getPackage(PACKAGE_NAME, START_VERSION);
expect(pkg?.status).to.eql('not_installed');
});

it('should not add es references on package policy create failure when package is already installed', async () => {
await installPackage(PACKAGE_NAME, START_VERSION);
await createPackagePolicyWithDataset(agentPolicyId, 'test*', 400);

const installation = await getInstallationSavedObject(PACKAGE_NAME, START_VERSION);
expectIdArraysEqual(installation.installed_es, []);

await uninstallPackage(PACKAGE_NAME, START_VERSION);
});
});
}

0 comments on commit 0833045

Please sign in to comment.