Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@ import {
savedObjectsClientMock,
} from '@kbn/core/server/mocks';
import { cloudMock } from '@kbn/cloud-plugin/server/mocks';
import { SavedObjectsErrorHelpers } from '@kbn/core/server';

import { createAppContextStartContractMock, createPackagePolicyServiceMock } from '../../mocks';
import { getPackageInfo } from '../epm/packages';
import { appContextService, cloudConnectorService } from '..';
import { agentPolicyService } from '../agent_policy';
import { agentlessAgentService } from '../agents/agentless_agent';

import { AgentlessPoliciesServiceImpl } from './agentless_policies';

Expand Down Expand Up @@ -310,6 +312,78 @@ describe('AgentlessPoliciesService', () => {

expect(jest.mocked(agentPolicyService.delete)).toHaveBeenCalledTimes(0);
});

it('should clean up orphaned resources when agent policy is not found (404)', async () => {
const deleteAgentlessAgentSpy = jest
.spyOn(agentlessAgentService, 'deleteAgentlessAgent')
.mockResolvedValueOnce(undefined as any);

jest
.mocked(agentPolicyService.get)
.mockRejectedValueOnce(SavedObjectsErrorHelpers.createGenericNotFoundError('test'));

packagePolicyService.findAllForAgentPolicy.mockResolvedValueOnce([
{ id: 'orphaned-pp-1' },
{ id: 'orphaned-pp-2' },
] as any);

packagePolicyService.delete.mockResolvedValueOnce([
{ id: 'orphaned-pp-1', success: true },
{ id: 'orphaned-pp-2', success: true },
] as any);

const soClient = savedObjectsClientMock.create();
const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
const logger = loggingSystemMock.createLogger();

const agentlessPoliciesService = new AgentlessPoliciesServiceImpl(
packagePolicyService,
soClient,
esClient,
logger
);

await agentlessPoliciesService.deleteAgentlessPolicy('orphaned-policy-id');

expect(jest.mocked(agentPolicyService.delete)).not.toHaveBeenCalled();
expect(packagePolicyService.findAllForAgentPolicy).toHaveBeenCalledWith(
soClient,
'orphaned-policy-id'
);
expect(packagePolicyService.delete).toHaveBeenCalledWith(
soClient,
esClient,
['orphaned-pp-1', 'orphaned-pp-2'],
expect.objectContaining({ force: true })
);
expect(deleteAgentlessAgentSpy).toHaveBeenCalledWith('orphaned-policy-id');

deleteAgentlessAgentSpy.mockRestore();
});

it('should rethrow non-404 errors from agentPolicyService.get', async () => {
jest.mocked(agentPolicyService.get).mockRejectedValueOnce({
output: { statusCode: 500 },
message: 'Internal server error',
});

const soClient = savedObjectsClientMock.create();
const esClient = elasticsearchServiceMock.createClusterClient().asInternalUser;
const logger = loggingSystemMock.createLogger();

const agentlessPoliciesService = new AgentlessPoliciesServiceImpl(
packagePolicyService,
soClient,
esClient,
logger
);

await expect(() =>
agentlessPoliciesService.deleteAgentlessPolicy('some-policy-id')
).rejects.toEqual(expect.objectContaining({ output: { statusCode: 500 } }));

expect(jest.mocked(agentPolicyService.delete)).not.toHaveBeenCalled();
});
});

describe('createAgentlessPolicy with cloud connectors', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
type Logger,
type RequestHandlerContext,
type SavedObjectsClientContract,
SavedObjectsErrorHelpers,
} from '@kbn/core/server';
import type { TypeOf } from '@kbn/config-schema';
import { v4 as uuidv4 } from 'uuid';
Expand All @@ -28,6 +29,7 @@ import type { PackagePolicyClient } from '../package_policy_service';
import { agentPolicyService } from '../agent_policy';
import { getPackageInfo } from '../epm/packages';
import { appContextService, cloudConnectorService } from '..';
import { FleetNotFoundError } from '../../errors';

import type { PackageInfo } from '../../types';
import {
Expand Down Expand Up @@ -274,7 +276,18 @@ export class AgentlessPoliciesServiceImpl implements AgentlessPoliciesService {
? appContextService.getSecurityCore().authc.getCurrentUser(request) || undefined
: undefined;

const agentPolicy = await agentPolicyService.get(this.soClient, policyId);
let agentPolicy;
try {
agentPolicy = await agentPolicyService.get(this.soClient, policyId);
} catch (e) {
if (e instanceof FleetNotFoundError || SavedObjectsErrorHelpers.isNotFoundError(e)) {
this.logger.warn(`Agent policy ${policyId} not found, cleaning up orphaned resources`);
await this.deleteOrphanedAgentlessResources(policyId, user);
return;
}
throw e;
}

if (!agentPolicy?.supports_agentless) {
throw new Error(`Policy ${policyId} is not an agentless policy`);
}
Expand All @@ -285,4 +298,28 @@ export class AgentlessPoliciesServiceImpl implements AgentlessPoliciesService {
user,
});
}

private async deleteOrphanedAgentlessResources(policyId: string, user?: any) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] Think we can avoid the any type here

Suggested change
private async deleteOrphanedAgentlessResources(policyId: string, user?: any) {
private async deleteOrphanedAgentlessResources(policyId: string, user?: AuthenticatedUser, {

WIth that may not need to ?? undefined below, since the AuthenticatedUser type is expected.

const packagePolicies = await this.packagePolicyService.findAllForAgentPolicy(
this.soClient,
policyId
);

if (packagePolicies.length > 0) {
await this.packagePolicyService.delete(
this.soClient,
this.esClient,
packagePolicies.map((pp) => pp.id),
{ force: true, user: user ?? undefined, skipUnassignFromAgentPolicies: true }
);
}

try {
await agentlessAgentService.deleteAgentlessAgent(policyId);
} catch (e) {
this.logger.warn(
`Failed to delete agentless deployment for orphaned policy ${policyId}: ${e.message}`
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,29 @@ export default function (providerContext: FtrProviderContext) {
expect(apiCalls[0].url).to.be(`/agentless-api/api/v1/ess/deployments/${policyId}`);
expect(apiCalls[0].method).to.be('DELETE');
});

it('should allow to delete an orphaned agentless policy when agent policy is missing', async () => {
// Orphan the policy by directly deleting the agent policy SO
await es.delete({
index: '.kibana_ingest',
id: `fleet-agent-policies:${policyId}`,
refresh: 'wait_for',
});

// Verify agent policy is gone
await expectToRejectWithNotFound(() => apiClient.getAgentPolicy(policyId));

// Delete via agentless API should succeed despite missing agent policy
await apiClient.deleteAgentlessPolicy(policyId);

// Verify package policy is cleaned up
await expectToRejectWithNotFound(() => apiClient.getPackagePolicy(policyId));

// Verify agentless API DELETE was called to clean up the deployment
expect(apiCalls.length).to.be(1);
expect(apiCalls[0].url).to.be(`/agentless-api/api/v1/ess/deployments/${policyId}`);
expect(apiCalls[0].method).to.be('DELETE');
});
});

describe('Update Agentless Policy', () => {
Expand Down
Loading