Skip to content

Commit

Permalink
[Fleet] /bulk_unenroll response matches other bulk action APIs (elast…
Browse files Browse the repository at this point in the history
…ic#95571)

## Summary
`/agents/bulk_unenroll` should return a response with a result for each agent given; including invalid or missing ids. It currently returns an empty object. elastic#90437


[TS type diff for response](https://github.com/jfsiii/kibana/blob/dd34e4c5ef9266a9c5162e33ca0efb4affd9b25b/x-pack/plugins/fleet/common/types/rest_spec/agent.ts#L124-L130)

```diff
- // eslint-disable-next-line @typescript-eslint/no-empty-interface
- export interface PostBulkAgentUnenrollResponse {}
+ export type PostBulkAgentUnenrollResponse = Record<
+   Agent['id'],
+   {
+     success: boolean;
+     error?: string;
+   }
+ >;
```

### 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
  • Loading branch information
John Schulz authored and kibanamachine committed Mar 26, 2021
1 parent 3c14e0b commit c0b1c20
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 38 deletions.
9 changes: 7 additions & 2 deletions x-pack/plugins/fleet/common/types/rest_spec/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,13 @@ export interface PostBulkAgentUnenrollRequest {
};
}

// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface PostBulkAgentUnenrollResponse {}
export type PostBulkAgentUnenrollResponse = Record<
Agent['id'],
{
success: boolean;
error?: string;
}
>;

export interface PostAgentUpgradeRequest {
params: {
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/fleet/server/routes/agent/unenroll_handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,17 @@ export const postBulkAgentsUnenrollHandler: RequestHandler<
: { kuery: request.body.agents };

try {
await AgentService.unenrollAgents(soClient, esClient, {
const results = await AgentService.unenrollAgents(soClient, esClient, {
...agentOptions,
force: request.body?.force,
});
const body: PostBulkAgentUnenrollResponse = {};
const body = results.items.reduce<PostBulkAgentUnenrollResponse>((acc, so) => {
acc[so.id] = {
success: !so.error,
error: so.error?.message,
};
return acc;
}, {});

return response.ok({ body });
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export const postBulkAgentsUpgradeHandler: RequestHandler<
const body = results.items.reduce<PostBulkAgentUpgradeResponse>((acc, so) => {
acc[so.id] = {
success: !so.error,
error: so.error ? so.error.message || so.error.toString() : undefined,
error: so.error?.message,
};
return acc;
}, {});
Expand Down
1 change: 0 additions & 1 deletion x-pack/plugins/fleet/server/services/agents/crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import type { estypes } from '@elastic/elasticsearch';
import type { SavedObjectsClientContract, ElasticsearchClient } from 'src/core/server';

import type { AgentSOAttributes, Agent, BulkActionResult, ListWithKuery } from '../../types';

import { appContextService, agentPolicyService } from '../../services';
import type { FleetServerAgent } from '../../../common';
import { isAgentUpgradeable, SO_SEARCH_LIMIT } from '../../../common';
Expand Down
69 changes: 41 additions & 28 deletions x-pack/plugins/fleet/server/services/agents/unenroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import type { ElasticsearchClient, SavedObjectsClientContract } from 'src/core/server';

import type { Agent, BulkActionResult } from '../../types';
import * as APIKeyService from '../api_keys';
import { AgentUnenrollmentError } from '../../errors';

Expand Down Expand Up @@ -57,26 +58,35 @@ export async function unenrollAgents(
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient,
options: GetAgentsOptions & { force?: boolean }
) {
): Promise<{ items: BulkActionResult[] }> {
// start with all agents specified
const agents = await getAgents(esClient, options);
const givenAgents = await getAgents(esClient, options);
const outgoingErrors: Record<Agent['id'], Error> = {};

// Filter to those not already unenrolled, or unenrolling
const agentsEnrolled = agents.filter((agent) => {
const agentsEnrolled = givenAgents.filter((agent) => {
if (options.force) {
return !agent.unenrolled_at;
}
return !agent.unenrollment_started_at && !agent.unenrolled_at;
});
// And which are allowed to unenroll
const settled = await Promise.allSettled(
const agentResults = await Promise.allSettled(
agentsEnrolled.map((agent) =>
unenrollAgentIsAllowed(soClient, esClient, agent.id).then((_) => agent)
)
);
const agentsToUpdate = agentsEnrolled.filter((_, index) => settled[index].status === 'fulfilled');
const now = new Date().toISOString();
const agentsToUpdate = agentResults.reduce<Agent[]>((agents, result, index) => {
if (result.status === 'fulfilled') {
agents.push(result.value);
} else {
const id = givenAgents[index].id;
outgoingErrors[id] = result.reason;
}
return agents;
}, []);

const now = new Date().toISOString();
if (options.force) {
// Get all API keys that need to be invalidated
const apiKeys = agentsToUpdate.reduce<string[]>((keys, agent) => {
Expand All @@ -94,17 +104,6 @@ export async function unenrollAgents(
if (apiKeys.length) {
await APIKeyService.invalidateAPIKeys(soClient, apiKeys);
}
// Update the necessary agents
return bulkUpdateAgents(
esClient,
agentsToUpdate.map((agent) => ({
agentId: agent.id,
data: {
active: false,
unenrolled_at: now,
},
}))
);
} else {
// Create unenroll action for each agent
await bulkCreateAgentActions(
Expand All @@ -116,18 +115,32 @@ export async function unenrollAgents(
type: 'UNENROLL',
}))
);

// Update the necessary agents
return bulkUpdateAgents(
esClient,
agentsToUpdate.map((agent) => ({
agentId: agent.id,
data: {
unenrollment_started_at: now,
},
}))
);
}

// Update the necessary agents
const updateData = options.force
? { unenrolled_at: now, active: false }
: { unenrollment_started_at: now };

await bulkUpdateAgents(
esClient,
agentsToUpdate.map(({ id }) => ({ agentId: id, data: updateData }))
);

const out = {
items: givenAgents.map((agent, index) => {
const hasError = agent.id in outgoingErrors;
const result: BulkActionResult = {
id: agent.id,
success: !hasError,
};
if (hasError) {
result.error = outgoingErrors[agent.id];
}
return result;
}),
};
return out;
}

export async function forceUnenrollAgent(
Expand Down
24 changes: 20 additions & 4 deletions x-pack/test/fleet_api_integration/apis/agents/unenroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export default function (providerContext: FtrProviderContext) {
.expect(200);

// try to unenroll
await supertest
const { body: unenrolledBody } = await supertest
.post(`/api/fleet/agents/bulk_unenroll`)
.set('kbn-xsrf', 'xxx')
.send({
Expand All @@ -128,6 +128,16 @@ export default function (providerContext: FtrProviderContext) {
// http request succeeds
.expect(200);

expect(unenrolledBody).to.eql({
agent2: {
success: false,
error: 'Cannot unenroll agent2 from a managed agent policy policy1',
},
agent3: {
success: false,
error: 'Cannot unenroll agent3 from a managed agent policy policy1',
},
});
// but agents are still enrolled
const [agent2data, agent3data] = await Promise.all([
supertest.get(`/api/fleet/agents/agent2`),
Expand All @@ -148,17 +158,23 @@ export default function (providerContext: FtrProviderContext) {
.set('kbn-xsrf', 'xxx')
.send({ name: 'Test policy', namespace: 'default', is_managed: false })
.expect(200);
await supertest
const { body: unenrolledBody } = await supertest
.post(`/api/fleet/agents/bulk_unenroll`)
.set('kbn-xsrf', 'xxx')
.send({
agents: ['agent2', 'agent3'],
})
.expect(200);
});

expect(unenrolledBody).to.eql({
agent2: { success: true },
agent3: { success: true },
});

const [agent2data, agent3data] = await Promise.all([
supertest.get(`/api/fleet/agents/agent2`),
supertest.get(`/api/fleet/agents/agent3`),
]);

expect(typeof agent2data.body.item.unenrollment_started_at).to.eql('string');
expect(agent2data.body.item.active).to.eql(true);
expect(typeof agent3data.body.item.unenrollment_started_at).to.be('string');
Expand Down

0 comments on commit c0b1c20

Please sign in to comment.