mirror of
https://github.com/elastic/kibana.git
synced 2025-06-27 18:51:07 -04:00
[Fleet] Ensure package policy names are unique when moving across spaces (#224804)
Fixes https://github.com/elastic/kibana/issues/222575 ## Summary Ensure package policy names are unique when moving across spaces. The check applies to any integration (not only Defend) but it's only applied when moving a policy from a space to another, not when creating a new policy) ### Testing - Ensure to have space awareness enabled - In `default` space, create an agent policy and add a package policy to it with name `defend1` - In a second space `space1`, create an agent policy and add a package policy to it with same name `defend1` - Try to update the settings of this agent policy changing the space to 'default' - you should get an error `an integration policy with name "defend" already exists. Please rename it or choose a different name." ### Checklist Check the PR satisfies following conditions. - [ ] [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: Elastic Machine <elasticmachine@users.noreply.github.com>
This commit is contained in:
parent
5adeebab61
commit
45abda5684
6 changed files with 240 additions and 7 deletions
|
@ -552,7 +552,7 @@ export const updateAgentPolicyHandler: FleetRequestHandler<
|
|||
currentSpaceId: spaceId,
|
||||
newSpaceIds: spaceIds,
|
||||
authorizedSpaces,
|
||||
options: { force },
|
||||
options: { force, validateUniqueName: true },
|
||||
});
|
||||
|
||||
spaceId = spaceIds[0];
|
||||
|
|
|
@ -396,14 +396,13 @@ class AgentPolicyService {
|
|||
}
|
||||
newAgentPolicy = updatedNewAgentPolicy;
|
||||
}
|
||||
logger.debug(`Running of external callbacks for [${externalCallbackType}] done`);
|
||||
return newAgentPolicy;
|
||||
} catch (error) {
|
||||
logger.error(`Error running external callbacks for [${externalCallbackType}]`);
|
||||
logger.error(error);
|
||||
throw error;
|
||||
}
|
||||
|
||||
logger.debug(`Running of external callbacks for [${externalCallbackType}] done`);
|
||||
}
|
||||
|
||||
public async create(
|
||||
|
@ -900,7 +899,6 @@ class AgentPolicyService {
|
|||
force: options?.force,
|
||||
});
|
||||
}
|
||||
|
||||
return this._update(soClient, esClient, id, agentPolicy, options?.user, {
|
||||
bumpRevision: options?.bumpRevision ?? true,
|
||||
removeProtection: false,
|
||||
|
|
|
@ -13,12 +13,18 @@ import { packagePolicyService } from '../package_policy';
|
|||
|
||||
import { updateAgentPolicySpaces } from './agent_policy';
|
||||
import { isSpaceAwarenessEnabled } from './helpers';
|
||||
import { validatePackagePoliciesUniqueNameAcrossSpaces } from './policy_namespaces';
|
||||
|
||||
jest.mock('./policy_namespaces');
|
||||
jest.mock('./helpers');
|
||||
jest.mock('../agent_policy');
|
||||
jest.mock('../package_policy');
|
||||
jest.mock('../agents');
|
||||
|
||||
const mockValidatePackagePoliciesUniqueNameAcrossSpaces =
|
||||
validatePackagePoliciesUniqueNameAcrossSpaces as jest.Mocked<
|
||||
typeof validatePackagePoliciesUniqueNameAcrossSpaces
|
||||
>;
|
||||
describe('updateAgentPolicySpaces', () => {
|
||||
beforeEach(() => {
|
||||
jest.mocked(isSpaceAwarenessEnabled).mockResolvedValue(true);
|
||||
|
@ -63,7 +69,11 @@ describe('updateAgentPolicySpaces', () => {
|
|||
} as any);
|
||||
});
|
||||
|
||||
it('does nothings if agent policy already in correct space', async () => {
|
||||
afterEach(() => {
|
||||
jest.resetAllMocks();
|
||||
});
|
||||
|
||||
it('does nothings if agent policy is already in correct space', async () => {
|
||||
await updateAgentPolicySpaces({
|
||||
agentPolicyId: 'policy1',
|
||||
currentSpaceId: 'default',
|
||||
|
@ -184,4 +194,30 @@ describe('updateAgentPolicySpaces', () => {
|
|||
})
|
||||
).rejects.toThrowError(/Not enough permissions to remove policies from space default/);
|
||||
});
|
||||
|
||||
it('throw when validateUniqueName is true and policy name already exists on another space', async () => {
|
||||
jest
|
||||
.mocked(mockValidatePackagePoliciesUniqueNameAcrossSpaces)
|
||||
.mockRejectedValueOnce(new Error('Name already exists'));
|
||||
await expect(
|
||||
updateAgentPolicySpaces({
|
||||
agentPolicyId: 'policy1',
|
||||
currentSpaceId: 'default',
|
||||
newSpaceIds: ['test'],
|
||||
authorizedSpaces: ['test'],
|
||||
options: { validateUniqueName: true },
|
||||
})
|
||||
).rejects.toThrowError(/Name already exists/);
|
||||
});
|
||||
|
||||
it('do not call validatePackagePoliciesUniqueNameAcrossSpaces when validateUniqueName is false', async () => {
|
||||
await updateAgentPolicySpaces({
|
||||
agentPolicyId: 'policy1',
|
||||
currentSpaceId: 'default',
|
||||
newSpaceIds: ['default'],
|
||||
authorizedSpaces: ['default'],
|
||||
options: { validateUniqueName: false },
|
||||
});
|
||||
expect(validatePackagePoliciesUniqueNameAcrossSpaces).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
|
@ -24,6 +24,8 @@ import { FleetError, HostedAgentPolicyRestrictionRelatedError } from '../../erro
|
|||
import type { UninstallTokenSOAttributes } from '../security/uninstall_token_service';
|
||||
import { closePointInTime, getAgentsByKuery, openPointInTime } from '../agents';
|
||||
|
||||
import { validatePackagePoliciesUniqueNameAcrossSpaces } from './policy_namespaces';
|
||||
|
||||
import { isSpaceAwarenessEnabled } from './helpers';
|
||||
|
||||
const UPDATE_AGENT_BATCH_SIZE = 1000;
|
||||
|
@ -39,7 +41,7 @@ export async function updateAgentPolicySpaces({
|
|||
currentSpaceId: string;
|
||||
newSpaceIds: string[];
|
||||
authorizedSpaces: string[];
|
||||
options?: { force?: boolean };
|
||||
options?: { force?: boolean; validateUniqueName?: boolean };
|
||||
}) {
|
||||
const useSpaceAwareness = await isSpaceAwarenessEnabled();
|
||||
if (!useSpaceAwareness || !newSpaceIds || newSpaceIds.length === 0) {
|
||||
|
@ -71,6 +73,9 @@ export async function updateAgentPolicySpaces({
|
|||
if (deepEqual(existingPolicy?.space_ids?.sort() ?? [DEFAULT_SPACE_ID], newSpaceIds.sort())) {
|
||||
return;
|
||||
}
|
||||
if (options?.validateUniqueName) {
|
||||
await validatePackagePoliciesUniqueNameAcrossSpaces(existingPackagePolicies, newSpaceIds);
|
||||
}
|
||||
|
||||
if (existingPackagePolicies.some((packagePolicy) => packagePolicy.policy_ids.length > 1)) {
|
||||
throw new FleetError(
|
||||
|
|
|
@ -9,8 +9,16 @@ import { savedObjectsClientMock } from '@kbn/core-saved-objects-api-server-mocks
|
|||
import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server';
|
||||
|
||||
import { appContextService } from '../app_context';
|
||||
import { packagePolicyService } from '../package_policy';
|
||||
|
||||
import { validatePolicyNamespaceForSpace } from './policy_namespaces';
|
||||
import type { PackagePolicyClient } from '../package_policy_service';
|
||||
|
||||
import { PackagePolicyNameExistsError } from '../../errors';
|
||||
|
||||
import {
|
||||
validatePolicyNamespaceForSpace,
|
||||
validatePackagePoliciesUniqueNameAcrossSpaces,
|
||||
} from './policy_namespaces';
|
||||
|
||||
jest.mock('../app_context');
|
||||
|
||||
|
@ -116,3 +124,148 @@ describe('validatePolicyNamespaceForSpace', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
const packagePolicy1 = {
|
||||
agents: 100,
|
||||
created_at: '2022-12-19T20:43:45.879Z',
|
||||
created_by: 'elastic',
|
||||
description: '',
|
||||
enabled: true,
|
||||
id: '1',
|
||||
inputs: [],
|
||||
name: 'Package Policy 1',
|
||||
namespace: 'default',
|
||||
package: {
|
||||
name: 'test-package',
|
||||
title: 'Test Package',
|
||||
version: '1.0.0',
|
||||
},
|
||||
policy_ids: ['agent-policy-id-a'],
|
||||
revision: 1,
|
||||
updated_at: '2022-12-19T20:43:45.879Z',
|
||||
updated_by: 'elastic',
|
||||
version: '1.0.0',
|
||||
spaceIds: ['space1'],
|
||||
};
|
||||
|
||||
const packagePolicyServiceMock = packagePolicyService as jest.Mocked<PackagePolicyClient>;
|
||||
|
||||
jest.mock(
|
||||
'../package_policy',
|
||||
(): {
|
||||
packagePolicyService: jest.Mocked<PackagePolicyClient>;
|
||||
} => {
|
||||
return {
|
||||
packagePolicyService: {
|
||||
buildPackagePolicyFromPackage: jest.fn(),
|
||||
bulkCreate: jest.fn(),
|
||||
create: jest.fn(),
|
||||
delete: jest.fn(),
|
||||
get: jest.fn(),
|
||||
getByIDs: jest.fn(),
|
||||
list: jest.fn(),
|
||||
listIds: jest.fn(),
|
||||
update: jest.fn(),
|
||||
|
||||
runExternalCallbacks: jest.fn(),
|
||||
upgrade: jest.fn(),
|
||||
bulkUpgrade: jest.fn(),
|
||||
getUpgradeDryRunDiff: jest.fn(),
|
||||
enrichPolicyWithDefaultsFromPackage: jest.fn(),
|
||||
} as any,
|
||||
};
|
||||
}
|
||||
);
|
||||
|
||||
describe('validatePackagePoliciesUniqueNameAcrossSpaces', () => {
|
||||
const soClient = savedObjectsClientMock.create();
|
||||
jest
|
||||
.mocked(appContextService.getInternalUserSOClientWithoutSpaceExtension)
|
||||
.mockReturnValue(soClient);
|
||||
|
||||
it('should not validate if package policies are empty', async () => {
|
||||
await expect(validatePackagePoliciesUniqueNameAcrossSpaces([], ['space1']));
|
||||
});
|
||||
|
||||
it('should throw if there are other policies with the same package name', async () => {
|
||||
const packagePolicyOnOtherSpace = {
|
||||
...packagePolicy1,
|
||||
spaceIds: ['default'],
|
||||
id: '3',
|
||||
};
|
||||
packagePolicyServiceMock.list.mockResolvedValue({
|
||||
total: 1,
|
||||
perPage: 10,
|
||||
page: 1,
|
||||
items: [packagePolicyOnOtherSpace],
|
||||
});
|
||||
await expect(
|
||||
validatePackagePoliciesUniqueNameAcrossSpaces([packagePolicy1], ['default'])
|
||||
).rejects.toThrowError(
|
||||
new PackagePolicyNameExistsError(
|
||||
'An integration policy with the name Package Policy 1 already exists in space "default". Please rename it or choose a different name.'
|
||||
)
|
||||
);
|
||||
});
|
||||
|
||||
it('should throw if there are other policies with the same package name in different spaces', async () => {
|
||||
const packagePolicyOnSpace1 = {
|
||||
...packagePolicy1,
|
||||
spaceIds: ['space1'],
|
||||
id: '3',
|
||||
};
|
||||
const packagePolicyOnSpace2 = {
|
||||
...packagePolicy1,
|
||||
spaceIds: ['space2'],
|
||||
id: '4',
|
||||
};
|
||||
packagePolicyServiceMock.list.mockResolvedValue({
|
||||
total: 1,
|
||||
perPage: 10,
|
||||
page: 1,
|
||||
items: [packagePolicyOnSpace1, packagePolicyOnSpace2],
|
||||
});
|
||||
await expect(
|
||||
validatePackagePoliciesUniqueNameAcrossSpaces([packagePolicy1], ['default', 'space1'])
|
||||
).rejects.toThrowError(
|
||||
new PackagePolicyNameExistsError(
|
||||
'An integration policy with the name Package Policy 1 already exists in space "space1". Please rename it or choose a different name.'
|
||||
)
|
||||
);
|
||||
});
|
||||
|
||||
it('should not throw if there are other policies with the same package name but in a space different than the target one', async () => {
|
||||
const packagePolicyOnOtherSpace = {
|
||||
...packagePolicy1,
|
||||
spaceIds: ['test'],
|
||||
id: '3',
|
||||
};
|
||||
packagePolicyServiceMock.list.mockResolvedValue({
|
||||
total: 1,
|
||||
perPage: 10,
|
||||
page: 1,
|
||||
items: [packagePolicyOnOtherSpace],
|
||||
});
|
||||
await expect(validatePackagePoliciesUniqueNameAcrossSpaces([packagePolicy1], ['default']));
|
||||
});
|
||||
|
||||
it('should exclude the policy itself', async () => {
|
||||
packagePolicyServiceMock.list.mockResolvedValue({
|
||||
total: 1,
|
||||
perPage: 10,
|
||||
page: 1,
|
||||
items: [packagePolicy1],
|
||||
});
|
||||
await expect(validatePackagePoliciesUniqueNameAcrossSpaces([packagePolicy1], ['default']));
|
||||
});
|
||||
|
||||
it('should not throw if there are no other policies with the same package name', async () => {
|
||||
packagePolicyServiceMock.list.mockResolvedValue({
|
||||
total: 1,
|
||||
perPage: 10,
|
||||
page: 1,
|
||||
items: [],
|
||||
});
|
||||
await expect(validatePackagePoliciesUniqueNameAcrossSpaces([packagePolicy1], ['default']));
|
||||
});
|
||||
});
|
||||
|
|
|
@ -5,9 +5,20 @@
|
|||
* 2.0.
|
||||
*/
|
||||
|
||||
import pMap from 'p-map';
|
||||
|
||||
import { appContextService } from '../app_context';
|
||||
import { PolicyNamespaceValidationError } from '../../../common/errors';
|
||||
|
||||
import type { PackagePolicy } from '../../types';
|
||||
import { packagePolicyService } from '../package_policy';
|
||||
import {
|
||||
MAX_CONCURRENT_AGENT_POLICIES_OPERATIONS_20,
|
||||
PACKAGE_POLICY_SAVED_OBJECT_TYPE,
|
||||
} from '../../constants';
|
||||
|
||||
import { PackagePolicyNameExistsError } from '../../errors';
|
||||
|
||||
import { getSpaceSettings } from './space_settings';
|
||||
|
||||
export async function validatePolicyNamespaceForSpace({
|
||||
|
@ -82,3 +93,33 @@ export async function validateAdditionalDatastreamsPermissionsForSpace({
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
export async function validatePackagePoliciesUniqueNameAcrossSpaces(
|
||||
packagePolicies: PackagePolicy[],
|
||||
newSpaceIds: string[] = []
|
||||
) {
|
||||
if (packagePolicies === undefined || packagePolicies.length === 0) return;
|
||||
const allSpacesSoClient = appContextService.getInternalUserSOClientWithoutSpaceExtension();
|
||||
|
||||
await pMap(
|
||||
packagePolicies,
|
||||
async (pkgPolicy) => {
|
||||
const { items } = await packagePolicyService.list(allSpacesSoClient, {
|
||||
kuery: `${PACKAGE_POLICY_SAVED_OBJECT_TYPE}.name:"${pkgPolicy.name}"`,
|
||||
spaceId: '*',
|
||||
});
|
||||
|
||||
const filteredItems = items.filter((item) => item.id !== pkgPolicy.id);
|
||||
const matchingSpaceId = newSpaceIds.find((spaceId) =>
|
||||
filteredItems.flatMap((item) => item.spaceIds ?? []).includes(spaceId)
|
||||
);
|
||||
if (matchingSpaceId)
|
||||
throw new PackagePolicyNameExistsError(
|
||||
`An integration policy with the name ${pkgPolicy.name} already exists in space "${matchingSpaceId}". Please rename it or choose a different name.`
|
||||
);
|
||||
},
|
||||
{
|
||||
concurrency: MAX_CONCURRENT_AGENT_POLICIES_OPERATIONS_20,
|
||||
}
|
||||
);
|
||||
}
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue