Skip to content

Commit

Permalink
[Fleet] Make integration names globally unique (#115212)
Browse files Browse the repository at this point in the history 
* [Fleet] Make integration names globally unique

* Fix Jest tests

* Append (copy) to names of integration packages belonging to duplicated policy

* Update current policy maintaining its name

* Fix failing tests
  • Loading branch information
@criamico
criamico committed Nov 3, 2021
1 parent 41216f2 commit 7432b9b
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 58 deletions.
52 changes: 34 additions & 18 deletions ...ons/fleet/sections/agent_policy/create_package_policy_page/step_define_package_policy.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,12 @@ import {

import styled from 'styled-components';

import type {
AgentPolicy,
PackageInfo,
PackagePolicy,
NewPackagePolicy,
RegistryVarsEntry,
} from '../../../types';
import type { AgentPolicy, PackageInfo, NewPackagePolicy, RegistryVarsEntry } from '../../../types';
import { packageToPackagePolicy, pkgKeyFromPackageInfo } from '../../../services';
import { Loading } from '../../../components';
import { useStartServices } from '../../../hooks';
import { useStartServices, useGetPackagePolicies } from '../../../hooks';
import { PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../../../../../constants';
import { SO_SEARCH_LIMIT } from '../../../../../../common';

import { isAdvancedVar } from './services';
import type { PackagePolicyValidationResults } from './services';
Expand Down Expand Up @@ -65,6 +61,14 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{
submitAttempted,
}) => {
const { docLinks } = useStartServices();

// Fetch all packagePolicies having the package name
const { data: packagePolicyData, isLoading: isLoadingPackagePolicies } = useGetPackagePolicies({
perPage: SO_SEARCH_LIMIT,
page: 1,
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.package.name:${packageInfo.name}`,
});

// Form show/hide states
const [isShowingAdvanced, setIsShowingAdvanced] = useState<boolean>(false);

Expand All @@ -84,33 +88,37 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{

// Update package policy's package and agent policy info
useEffect(() => {
if (isLoadingPackagePolicies) {
return;
}
const pkg = packagePolicy.package;
const currentPkgKey = pkg ? pkgKeyFromPackageInfo(pkg) : '';
const pkgKey = pkgKeyFromPackageInfo(packageInfo);

// If package has changed, create shell package policy with input&stream values based on package info
if (currentPkgKey !== pkgKey) {
// Existing package policies on the agent policy using the package name, retrieve highest number appended to package policy name
// Retrieve highest number appended to package policy name and increment it by one
const pkgPoliciesNamePattern = new RegExp(`${packageInfo.name}-(\\d+)`);
const pkgPoliciesWithMatchingNames = agentPolicy
? (agentPolicy.package_policies as PackagePolicy[])
const pkgPoliciesWithMatchingNames = packagePolicyData?.items
? packagePolicyData.items
.filter((ds) => Boolean(ds.name.match(pkgPoliciesNamePattern)))
.map((ds) => parseInt(ds.name.match(pkgPoliciesNamePattern)![1], 10))
.sort((a, b) => a - b)
: [];

const incrementedName = `${packageInfo.name}-${
pkgPoliciesWithMatchingNames.length
? pkgPoliciesWithMatchingNames[pkgPoliciesWithMatchingNames.length - 1] + 1
: 1
}`;

updatePackagePolicy(
packageToPackagePolicy(
packageInfo,
agentPolicy?.id || '',
packagePolicy.output_id,
packagePolicy.namespace,
packagePolicy.name ||
`${packageInfo.name}-${
pkgPoliciesWithMatchingNames.length
? pkgPoliciesWithMatchingNames[pkgPoliciesWithMatchingNames.length - 1] + 1
: 1
}`,
packagePolicy.name || incrementedName,
packagePolicy.description,
integrationToEnable
)
Expand All @@ -124,7 +132,15 @@ export const StepDefinePackagePolicy: React.FunctionComponent<{
namespace: agentPolicy.namespace,
});
}
}, [packagePolicy, agentPolicy, packageInfo, updatePackagePolicy, integrationToEnable]);
}, [
packagePolicy,
agentPolicy,
packageInfo,
updatePackagePolicy,
integrationToEnable,
packagePolicyData,
isLoadingPackagePolicies,
]);

return validationResults ? (
<FormGroupResponsiveFields
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/fleet/server/services/agent_policy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,12 +410,16 @@ class AgentPolicyService {
options
);

// Copy all package policies
// Copy all package policies and append (copy) to their names
if (baseAgentPolicy.package_policies.length) {
const newPackagePolicies = (baseAgentPolicy.package_policies as PackagePolicy[]).map(
(packagePolicy: PackagePolicy) => {
const { id: packagePolicyId, version, ...newPackagePolicy } = packagePolicy;
return newPackagePolicy;
const updatedPackagePolicy = {
...newPackagePolicy,
name: `${packagePolicy.name} (copy)`,
};
return updatedPackagePolicy;
}
);
await packagePolicyService.bulkCreate(
Expand Down
51 changes: 18 additions & 33 deletions x-pack/plugins/fleet/server/services/package_policy.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import type {
} from '../../common';
import { PACKAGE_POLICY_SAVED_OBJECT_TYPE } from '../constants';
import {
HostedAgentPolicyRestrictionRelatedError,
IngestManagerError,
ingestErrorToResponseOptions,
PackagePolicyIneligibleForUpgradeError,
Expand Down Expand Up @@ -99,24 +98,14 @@ class PackagePolicyService {
skipEnsureInstalled?: boolean;
}
): Promise<PackagePolicy> {
// Check that its agent policy does not have a package policy with the same name
const parentAgentPolicy = await agentPolicyService.get(soClient, packagePolicy.policy_id);
if (!parentAgentPolicy) {
throw new Error('Agent policy not found');
}
if (parentAgentPolicy.is_managed && !options?.force) {
throw new HostedAgentPolicyRestrictionRelatedError(
`Cannot add integrations to hosted agent policy ${parentAgentPolicy.id}`
);
}
if (
(parentAgentPolicy.package_policies as PackagePolicy[]).find(
(siblingPackagePolicy) => siblingPackagePolicy.name === packagePolicy.name
)
) {
throw new IngestManagerError(
'There is already a package with the same name on this agent policy'
);
const existingPoliciesWithName = await this.list(soClient, {
perPage: 1,
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name: "${packagePolicy.name}"`,
});

// Check that the name does not exist already
if (existingPoliciesWithName.items.length > 0) {
throw new IngestManagerError('There is already a package with the same name');
}
let elasticsearch: PackagePolicy['elasticsearch'];
// Add ids to stream
Expand Down Expand Up @@ -320,12 +309,12 @@ class PackagePolicyService {
});

return {
items: packagePolicies.saved_objects.map((packagePolicySO) => ({
items: packagePolicies?.saved_objects.map((packagePolicySO) => ({
id: packagePolicySO.id,
version: packagePolicySO.version,
...packagePolicySO.attributes,
})),
total: packagePolicies.total,
total: packagePolicies?.total,
page,
perPage,
};
Expand Down Expand Up @@ -369,19 +358,15 @@ class PackagePolicyService {
if (!oldPackagePolicy) {
throw new Error('Package policy not found');
}
// Check that the name does not exist already but exclude the current package policy
const existingPoliciesWithName = await this.list(soClient, {
perPage: 1,
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name: "${packagePolicy.name}"`,
});
const filtered = (existingPoliciesWithName?.items || []).filter((p) => p.id !== id);

// Check that its agent policy does not have a package policy with the same name
const parentAgentPolicy = await agentPolicyService.get(soClient, packagePolicy.policy_id);
if (!parentAgentPolicy) {
throw new Error('Agent policy not found');
}
if (
(parentAgentPolicy.package_policies as PackagePolicy[]).find(
(siblingPackagePolicy) =>
siblingPackagePolicy.id !== id && siblingPackagePolicy.name === packagePolicy.name
)
) {
throw new Error('There is already a package with the same name on this agent policy');
if (filtered.length > 0) {
throw new IngestManagerError('There is already a package with the same name');
}

let inputs = restOfPackagePolicy.inputs.map((input) =>
Expand Down
54 changes: 51 additions & 3 deletions x-pack/test/fleet_api_integration/apis/package_policy/create.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export default function (providerContext: FtrProviderContext) {
.post(`/api/fleet/package_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'filetest-1',
name: 'filetest',
description: '',
namespace: 'default',
policy_id: hostedPolicy.id,
Expand All @@ -85,7 +85,7 @@ export default function (providerContext: FtrProviderContext) {

expect(responseWithoutForce.statusCode).to.be(400);
expect(responseWithoutForce.message).to.contain(
'Cannot add integrations to hosted agent policy'
'Cannot update integrations of hosted agent policy'
);

// try same request with `force: true`
Expand Down Expand Up @@ -122,7 +122,7 @@ export default function (providerContext: FtrProviderContext) {
.post(`/api/fleet/package_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'filetest-1',
name: 'filetest-2',
description: '',
namespace: 'default',
policy_id: agentPolicyId,
Expand Down Expand Up @@ -276,5 +276,53 @@ export default function (providerContext: FtrProviderContext) {
})
.expect(400);
});

it('should return a 400 if there is a package policy with the same name on a different policy', async function () {
const { body: agentPolicyResponse } = await supertest
.post(`/api/fleet/agent_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Test policy 2',
namespace: 'default',
});
const otherAgentPolicyId = agentPolicyResponse.item.id;

await supertest
.post(`/api/fleet/package_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'same-name-test-2',
description: '',
namespace: 'default',
policy_id: otherAgentPolicyId,
enabled: true,
output_id: '',
inputs: [],
package: {
name: 'filetest',
title: 'For File Tests',
version: '0.1.0',
},
})
.expect(200);
await supertest
.post(`/api/fleet/package_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'same-name-test-2',
description: '',
namespace: 'default',
policy_id: agentPolicyId,
enabled: true,
output_id: '',
inputs: [],
package: {
name: 'filetest',
title: 'For File Tests',
version: '0.1.0',
},
})
.expect(400);
});
});
}
34 changes: 32 additions & 2 deletions x-pack/test/fleet_api_integration/apis/package_policy/update.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ export default function (providerContext: FtrProviderContext) {
});
});

it('should return a 500 if there is another package policy with the same name', async function () {
it('should return a 400 if there is another package policy with the same name', async function () {
await supertest
.put(`/api/fleet/package_policies/${packagePolicyId2}`)
.set('kbn-xsrf', 'xxxx')
Expand All @@ -176,7 +176,37 @@ export default function (providerContext: FtrProviderContext) {
version: '0.1.0',
},
})
.expect(500);
.expect(400);
});

it('should return a 400 if there is another package policy with the same name on a different policy', async function () {
const { body: agentPolicyResponse } = await supertest
.post(`/api/fleet/agent_policies`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'Test policy 2',
namespace: 'default',
});
const otherAgentPolicyId = agentPolicyResponse.item.id;

await supertest
.put(`/api/fleet/package_policies/${packagePolicyId2}`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'filetest-1',
description: '',
namespace: 'updated_namespace',
policy_id: otherAgentPolicyId,
enabled: true,
output_id: '',
inputs: [],
package: {
name: 'filetest',
title: 'For File Tests',
version: '0.1.0',
},
})
.expect(400);
});

it('should work with frozen input vars', async function () {
Expand Down

0 comments on commit 7432b9b

Please sign in to comment.