From 8bd7124ca1fad9832b7494290fc1b8ecf978fffe Mon Sep 17 00:00:00 2001 From: Cristina Amico Date: Mon, 24 Jun 2024 11:53:39 +0200 Subject: [PATCH] [Fleet] When deleting an agent policy, only delete integration policies if no other agent policies use it (#186601) Closes https://github.com/elastic/kibana/issues/182222 ## Summary - If `enableReusableIntegrationPolicies` is enabled, the `agentPolicyService.delete` method finds the integration policies that are shared with other agent policies and doesn't delete them. - Updated the UI so that the modal shows an info box to the user: ![Screenshot 2024-06-24 at 10 20 53](https://github.com/elastic/kibana/assets/16084106/6db8f225-bfc7-47cf-a49c-adc0e0b794ac) ### Checklist - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [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: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../agent_policy/components/actions_menu.tsx | 1 + .../agent_policy_delete_provider.tsx | 62 +++- .../server/services/agent_policy.test.ts | 302 +++++++++++++----- .../fleet/server/services/agent_policy.ts | 18 +- 4 files changed, 294 insertions(+), 89 deletions(-) diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/actions_menu.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/actions_menu.tsx index 99004b53ec349..d63e626bc5e12 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/actions_menu.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/components/actions_menu.tsx @@ -143,6 +143,7 @@ export const AgentPolicyActionMenu = memo<{ {(deleteAgentPolicyPrompt) => ( React.ReactElement; hasFleetServer: boolean; + packagePolicies?: PackagePolicy[]; } export type DeleteAgentPolicy = (agentPolicy: string, onSuccess?: OnSuccessCallback) => void; @@ -34,6 +38,7 @@ type OnSuccessCallback = (agentPolicyDeleted: string) => void; export const AgentPolicyDeleteProvider: React.FunctionComponent = ({ children, hasFleetServer, + packagePolicies, }) => { const { notifications } = useStartServices(); const { @@ -48,6 +53,7 @@ export const AgentPolicyDeleteProvider: React.FunctionComponent = ({ const { getPath } = useLink(); const history = useHistory(); const deleteAgentPolicyMutation = useDeleteAgentPolicyMutation(); + const { enableReusableIntegrationPolicies } = ExperimentalFeaturesService.get(); const deleteAgentPolicyPrompt: DeleteAgentPolicy = ( agentPolicyToDelete, @@ -106,20 +112,31 @@ export const AgentPolicyDeleteProvider: React.FunctionComponent = ({ history.push(getPath('policies_list')); }; - const fetchAgentsCount = async (agentPolicyToCheck: string) => { - if (!isFleetEnabled || isLoadingAgentsCount) { - return; + const fetchAgentsCount = useCallback( + async (agentPolicyToCheck: string) => { + if (!isFleetEnabled || isLoadingAgentsCount) { + return; + } + setIsLoadingAgentsCount(true); + // filtering out the unenrolled agents assigned to this policy + const agents = await sendGetAgents({ + showInactive: true, + kuery: `policy_id:"${agentPolicyToCheck}" and not status: unenrolled`, + perPage: SO_SEARCH_LIMIT, + }); + setAgentsCount(agents.data?.total ?? 0); + setIsLoadingAgentsCount(false); + }, + [isFleetEnabled, isLoadingAgentsCount] + ); + + const packagePoliciesWithMultiplePolicies = useMemo(() => { + // Find if there are package policies that have multiple agent policies + if (packagePolicies && enableReusableIntegrationPolicies) { + return packagePolicies.some((policy) => policy?.policy_ids.length > 1); } - setIsLoadingAgentsCount(true); - // filtering out the unenrolled agents assigned to this policy - const agents = await sendGetAgents({ - showInactive: true, - kuery: `policy_id:"${agentPolicyToCheck}" and not status: unenrolled`, - perPage: SO_SEARCH_LIMIT, - }); - setAgentsCount(agents.data?.total ?? 0); - setIsLoadingAgentsCount(false); - }; + return false; + }, [enableReusableIntegrationPolicies, packagePolicies]); const renderModal = () => { if (!isModalOpen) { @@ -158,6 +175,21 @@ export const AgentPolicyDeleteProvider: React.FunctionComponent = ({ buttonColor="danger" confirmButtonDisabled={isLoading || isLoadingAgentsCount || !!agentsCount} > + {packagePoliciesWithMultiplePolicies && ( + <> + + } + /> + + + )} {isLoadingAgentsCount ? ( { let soClient: ReturnType; let esClient: ReturnType['asInternalUser']; - beforeEach(() => { - soClient = getSavedObjectMock({ revision: 1, package_policies: ['package-1'] }); - mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ - { - id: 'package-1', - }, - ] as any); - esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + describe('with enableReusableIntegrationPolicies disabled', () => { + beforeEach(() => { + soClient = getSavedObjectMock({ revision: 1, package_policies: ['package-1'] }); + mockedPackagePolicyService.create.mockReset(); + esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + (getAgentsByKuery as jest.Mock).mockResolvedValue({ + agents: [], + total: 0, + page: 1, + perPage: 10, + }); - (getAgentsByKuery as jest.Mock).mockResolvedValue({ - agents: [], - total: 0, - page: 1, - perPage: 10, + mockedPackagePolicyService.delete.mockResolvedValue([ + { + id: 'package-1', + } as any, + ]); + jest + .spyOn(appContextService, 'getExperimentalFeatures') + .mockReturnValue({ enableReusableIntegrationPolicies: false } as any); }); - mockedPackagePolicyService.delete.mockResolvedValue([ - { - id: 'package-1', - } as any, - ]); - }); + it('should throw error for agent policy which has managed package policy', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + is_managed: true, + }, + ] as any); + try { + await agentPolicyService.delete(soClient, esClient, 'mocked'); + } catch (e) { + expect(e.message).toEqual( + new PackagePolicyRestrictionRelatedError( + `Cannot delete agent policy mocked that contains managed package policies` + ).message + ); + } + }); - it('should throw error for agent policy which has managed package policy', async () => { - mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ - { - id: 'package-1', - is_managed: true, - }, - ] as any); - try { + it('should allow delete with force for agent policy which has managed package policy', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + is_managed: true, + }, + ] as any); + const response = await agentPolicyService.delete(soClient, esClient, 'mocked', { + force: true, + }); + expect(response.id).toEqual('mocked'); + }); + + it('should call audit logger', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + }, + ] as any); await agentPolicyService.delete(soClient, esClient, 'mocked'); - } catch (e) { - expect(e.message).toEqual( - new PackagePolicyRestrictionRelatedError( - `Cannot delete agent policy mocked that contains managed package policies` - ).message + + expect(mockedAuditLoggingService.writeCustomSoAuditLog).toHaveBeenCalledWith({ + action: 'delete', + id: 'mocked', + savedObjectType: AGENT_POLICY_SAVED_OBJECT_TYPE, + }); + }); + + it('should throw error if active agents are assigned to the policy', async () => { + (getAgentsByKuery as jest.Mock).mockResolvedValue({ + agents: [], + total: 2, + page: 1, + perPage: 10, + }); + await expect(agentPolicyService.delete(soClient, esClient, 'mocked')).rejects.toThrowError( + 'Cannot delete an agent policy that is assigned to any active or inactive agents' ); - } - }); + }); - it('should allow delete with force for agent policy which has managed package policy', async () => { - mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ - { - id: 'package-1', - is_managed: true, - }, - ] as any); - const response = await agentPolicyService.delete(soClient, esClient, 'mocked', { - force: true, + it('should delete .fleet-policies entries on agent policy delete', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + }, + ] as any); + esClient.deleteByQuery.mockResolvedValueOnce({ + deleted: 2, + }); + + await agentPolicyService.delete(soClient, esClient, 'mocked'); + + expect(esClient.deleteByQuery).toHaveBeenCalledWith( + expect.objectContaining({ + index: AGENT_POLICY_INDEX, + query: { + term: { + policy_id: 'mocked', + }, + }, + }) + ); + }); + + it('should delete all integration polices', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + policy_id: ['policy_1'], + policy_ids: ['policy_1', 'int_policy_2'], + }, + { + id: 'package-2', + policy_id: ['policy_1'], + policy_ids: ['policy_1'], + }, + { + id: 'package-3', + }, + ] as any); + await agentPolicyService.delete(soClient, esClient, 'mocked'); + expect(mockedPackagePolicyService.delete).toBeCalledWith( + expect.anything(), + expect.anything(), + ['package-1', 'package-2', 'package-3'], + expect.anything() + ); }); - expect(response.id).toEqual('mocked'); }); - it('should call audit logger', async () => { - await agentPolicyService.delete(soClient, esClient, 'mocked'); + describe('with enableReusableIntegrationPolicies enabled', () => { + beforeEach(() => { + soClient = getSavedObjectMock({ revision: 1, package_policies: ['package-1'] }); + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + }, + ] as any); + esClient = elasticsearchServiceMock.createClusterClient().asInternalUser; + + (getAgentsByKuery as jest.Mock).mockResolvedValue({ + agents: [], + total: 0, + page: 1, + perPage: 10, + }); + mockedPackagePolicyService.create.mockReset(); + jest + .spyOn(appContextService, 'getExperimentalFeatures') + .mockReturnValue({ enableReusableIntegrationPolicies: true } as any); + }); - expect(mockedAuditLoggingService.writeCustomSoAuditLog).toHaveBeenCalledWith({ - action: 'delete', - id: 'mocked', - savedObjectType: AGENT_POLICY_SAVED_OBJECT_TYPE, + it('should throw error for agent policy which has managed package policy', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + is_managed: true, + }, + ] as any); + try { + await agentPolicyService.delete(soClient, esClient, 'mocked'); + } catch (e) { + expect(e.message).toEqual( + new PackagePolicyRestrictionRelatedError( + `Cannot delete agent policy mocked that contains managed package policies` + ).message + ); + } }); - }); - it('should throw error if active agents are assigned to the policy', async () => { - (getAgentsByKuery as jest.Mock).mockResolvedValue({ - agents: [], - total: 2, - page: 1, - perPage: 10, + it('should allow delete with force for agent policy which has managed package policy', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + is_managed: true, + }, + ] as any); + const response = await agentPolicyService.delete(soClient, esClient, 'mocked', { + force: true, + }); + expect(response.id).toEqual('mocked'); }); - await expect(agentPolicyService.delete(soClient, esClient, 'mocked')).rejects.toThrowError( - 'Cannot delete an agent policy that is assigned to any active or inactive agents' - ); - }); - it('should delete .fleet-policies entries on agent policy delete', async () => { - esClient.deleteByQuery.mockResolvedValueOnce({ - deleted: 2, + it('should call audit logger', async () => { + await agentPolicyService.delete(soClient, esClient, 'mocked'); + + expect(mockedAuditLoggingService.writeCustomSoAuditLog).toHaveBeenCalledWith({ + action: 'delete', + id: 'mocked', + savedObjectType: AGENT_POLICY_SAVED_OBJECT_TYPE, + }); }); - await agentPolicyService.delete(soClient, esClient, 'mocked'); + it('should throw error if active agents are assigned to the policy', async () => { + (getAgentsByKuery as jest.Mock).mockResolvedValue({ + agents: [], + total: 2, + page: 1, + perPage: 10, + }); + await expect(agentPolicyService.delete(soClient, esClient, 'mocked')).rejects.toThrowError( + 'Cannot delete an agent policy that is assigned to any active or inactive agents' + ); + }); - expect(esClient.deleteByQuery).toHaveBeenCalledWith( - expect.objectContaining({ - index: AGENT_POLICY_INDEX, - query: { - term: { - policy_id: 'mocked', + it('should delete .fleet-policies entries on agent policy delete', async () => { + esClient.deleteByQuery.mockResolvedValueOnce({ + deleted: 2, + }); + + await agentPolicyService.delete(soClient, esClient, 'mocked'); + + expect(esClient.deleteByQuery).toHaveBeenCalledWith( + expect.objectContaining({ + index: AGENT_POLICY_INDEX, + query: { + term: { + policy_id: 'mocked', + }, }, + }) + ); + }); + + it('should only delete package polices that are not shared with other agent policies', async () => { + mockedPackagePolicyService.findAllForAgentPolicy.mockReturnValue([ + { + id: 'package-1', + policy_id: ['policy_1'], + policy_ids: ['policy_1', 'int_policy_2'], }, - }) - ); + { + id: 'package-2', + policy_id: ['policy_1'], + policy_ids: ['policy_1'], + }, + { + id: 'package-3', + }, + ] as any); + await agentPolicyService.delete(soClient, esClient, 'mocked'); + expect(mockedPackagePolicyService.delete).toBeCalledWith( + expect.anything(), + expect.anything(), + ['package-2', 'package-3'], + expect.anything() + ); + }); }); }); diff --git a/x-pack/plugins/fleet/server/services/agent_policy.ts b/x-pack/plugins/fleet/server/services/agent_policy.ts index 029352e145ca6..041b644951e25 100644 --- a/x-pack/plugins/fleet/server/services/agent_policy.ts +++ b/x-pack/plugins/fleet/server/services/agent_policy.ts @@ -1008,16 +1008,22 @@ class AgentPolicyService { `Cannot delete agent policy ${id} that contains managed package policies` ); } + const packagePoliciesToDelete = this.packagePoliciesWithoutMultiplePolicies(packagePolicies); await packagePolicyService.delete( soClient, esClient, - packagePolicies.map((p) => p.id), + packagePoliciesToDelete.map((p) => p.id), { force: options?.force, skipUnassignFromAgentPolicies: true, } ); + logger.debug( + `Deleted package policies with ids ${packagePoliciesToDelete + .map((policy) => policy.id) + .join(', ')}` + ); } if (agentPolicy.is_preconfigured && !options?.force) { @@ -1550,6 +1556,16 @@ class AgentPolicyService { ); } } + + private packagePoliciesWithoutMultiplePolicies(packagePolicies: PackagePolicy[]) { + // Find package policies that don't have multiple agent policies and mark them for deletion + if (appContextService.getExperimentalFeatures().enableReusableIntegrationPolicies) { + return packagePolicies.filter( + (policy) => !policy?.policy_ids || policy?.policy_ids.length <= 1 + ); + } + return packagePolicies; + } } export const agentPolicyService = new AgentPolicyService();