Skip to content

Commit

Permalink
[Fleet] RBAC - Make agents write APIs space aware - 4/4 (#191277)
Browse files Browse the repository at this point in the history
## Summary

Closes #185040

Followup to:
#188507
#189519
#190069

This PR contains the last required changed for making Fleet agents write
APIs space aware:
* Implement space awareness in the following endpoints:
   * `POST /agents/{agentId}/unenroll`
   * `POST /agents/{agentId}/request_diagnostics`
   * `POST /agents/bulk_unenroll`
   * `POST /agents/bulk_request_diagnostics`
* Fix a bug in `POST /agents/bulk_update_agent_tags` where the space id
was not passed.
* Simply the setting of the `namespaces` property when creating agent
actions when the space id is known.
* Rename `currentNamespace` to `currentSpaceId` where appropriate (see
comment below).
* Add API integration tests and consolidate existing ones. ~⚠️ At the
time of writing, I would like there to be more tests covering bulk query
processing in batches, which are currently lacking. I have experienced
difficulties getting those tests to pass consistently.~ Filed [followup
issue](#191643) for those.

### A note on terminology

As pointed out in
#191083 (comment),
it seems that the terms "namespace" and "space id" are occasionally used
interchangeably in some parts of the codebase to refer to a Kibana
space. For instance, documents in Fleet indices (agents, agent policies,
agent actions...) [possess a `namespaces`
property](elastic/elasticsearch#108363) to track
the spaces they belong to. The current space id is also returned using
the Saved Object client's `getCurrentNamespace` function.

However, "namespace" is also a datastream property. In the Agent policy
settings UI, the "Spaces" property (which will be linked to the saved
object's `namespaces` property) is above the "Default namespace"
property, which relates to the integration's data streams:
<img width="1916" alt="Screenshot 2024-08-26 at 14 51 10"
src="https://github.com/user-attachments/assets/fe2a0948-3387-4a93-96dc-90fc5cf1a683">

This should not be a source of major issues, but is best clarified for
future reference. In this PR, I've replaced some occurrences of
`namespace` with `spaceId` where appropriate to try to maximise the use
of the latter.

### Testing

* This PR should be put through the Flaky Test Runner prior to merging
(I will kick the job).
* Manual testing should also be performed for the new endpoints
mentioned above.

### 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
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed

---------

Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
jillguyonnet and elasticmachine authored Aug 30, 2024
1 parent 9ea0f06 commit 3ba243d
Show file tree
Hide file tree
Showing 19 changed files with 625 additions and 248 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export const requestDiagnosticsHandler: RequestHandler<

const result = await AgentService.requestDiagnostics(
esClient,
soClient,
request.params.agentId,
request.body?.additional_metrics
);
Expand Down
8 changes: 3 additions & 5 deletions x-pack/plugins/fleet/server/services/agents/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export async function cancelAgentAction(
soClient: SavedObjectsClientContract,
actionId: string
) {
const currentNameSpace = getCurrentNamespace(soClient);
const currentSpaceId = getCurrentNamespace(soClient);

const getUpgradeActions = async () => {
const query = {
Expand All @@ -329,7 +329,7 @@ export async function cancelAgentAction(
};
const res = await esClient.search<FleetServerAgentAction>({
index: AGENT_ACTIONS_INDEX,
query: await addNamespaceFilteringToQuery(query, currentNameSpace),
query: await addNamespaceFilteringToQuery(query, currentSpaceId),
size: SO_SEARCH_LIMIT,
});

Expand Down Expand Up @@ -358,12 +358,10 @@ export async function cancelAgentAction(
const cancelledActions: Array<{ agents: string[] }> = [];

const createAction = async (action: FleetServerAgentAction) => {
const namespaces = currentNameSpace ? { namespaces: [currentNameSpace] } : {};

await createAgentAction(esClient, {
id: cancelActionId,
type: 'CANCEL',
...namespaces,
namespaces: [currentSpaceId],
agents: action.agents!,
data: {
target_id: action.action_id,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/services/agents/crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,7 +433,7 @@ async function _filterAgents(
}> {
const { page = 1, perPage = 20, sortField = 'enrolled_at', sortOrder = 'desc' } = options;
const runtimeFields = await buildAgentStatusRuntimeField(soClient);
const currentNameSpace = getCurrentNamespace(soClient);
const currentSpaceId = getCurrentNamespace(soClient);

let res;
try {
Expand All @@ -445,7 +445,7 @@ async function _filterAgents(
runtime_mappings: runtimeFields,
fields: Object.keys(runtimeFields),
sort: [{ [sortField]: { order: sortOrder } }],
query: await addNamespaceFilteringToQuery({ bool: { filter: [query] } }, currentNameSpace),
query: await addNamespaceFilteringToQuery({ bool: { filter: [query] } }, currentSpaceId),
index: AGENTS_INDEX,
ignore_unavailable: true,
});
Expand Down
13 changes: 6 additions & 7 deletions x-pack/plugins/fleet/server/services/agents/reassign.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ export async function reassignAgent(
policy_revision: null,
});

const currentNameSpace = getCurrentNamespace(soClient);
const namespaces = currentNameSpace ? { namespaces: [currentNameSpace] } : {};
const currentSpaceId = getCurrentNamespace(soClient);

await createAgentAction(esClient, {
agents: [agentId],
Expand All @@ -88,7 +87,7 @@ export async function reassignAgent(
data: {
policy_id: newAgentPolicyId,
},
...namespaces,
namespaces: [currentSpaceId],
});
}

Expand All @@ -103,7 +102,7 @@ export async function reassignAgents(
): Promise<{ actionId: string }> {
await verifyNewAgentPolicy(soClient, newAgentPolicyId);

const currentNameSpace = getCurrentNamespace(soClient);
const currentSpaceId = getCurrentNamespace(soClient);
const outgoingErrors: Record<Agent['id'], Error> = {};
let givenAgents: Agent[] = [];
if ('agents' in options) {
Expand All @@ -121,7 +120,7 @@ export async function reassignAgents(
}
} else if ('kuery' in options) {
const batchSize = options.batchSize ?? SO_SEARCH_LIMIT;
const namespaceFilter = await agentsKueryNamespaceFilter(currentNameSpace);
const namespaceFilter = await agentsKueryNamespaceFilter(currentSpaceId);
const kuery = namespaceFilter ? `${namespaceFilter} AND ${options.kuery}` : options.kuery;
const res = await getAgentsByKuery(esClient, soClient, {
kuery,
Expand All @@ -138,7 +137,7 @@ export async function reassignAgents(
soClient,
{
...options,
spaceId: currentNameSpace,
spaceId: currentSpaceId,
batchSize,
total: res.total,
newAgentPolicyId,
Expand All @@ -150,7 +149,7 @@ export async function reassignAgents(

return await reassignBatch(
esClient,
{ newAgentPolicyId, spaceId: currentNameSpace },
{ newAgentPolicyId, spaceId: currentSpaceId },
givenAgents,
outgoingErrors
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export async function reassignBatch(
const actionId = options.actionId ?? uuidv4();
const total = options.total ?? givenAgents.length;
const now = new Date().toISOString();
const namespaces = spaceId ? { namespaces: [spaceId] } : {};
const namespaces = spaceId ? [spaceId] : [];

await createAgentAction(esClient, {
id: actionId,
Expand All @@ -100,7 +100,7 @@ export async function reassignBatch(
data: {
policy_id: options.newAgentPolicyId,
},
...namespaces,
namespaces,
});

await createErrorActionResults(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ describe('requestDiagnostics', () => {

describe('requestDiagnostics (singular)', () => {
it('can request diagnostics for single agent', async () => {
const { esClient, agentInRegularDoc } = createClientMock();
await requestDiagnostics(esClient, agentInRegularDoc._id);
const { soClient, esClient, agentInRegularDoc } = createClientMock();
await requestDiagnostics(esClient, soClient, agentInRegularDoc._id);

expect(esClient.create).toHaveBeenCalledWith(
expect.objectContaining({
Expand Down
20 changes: 17 additions & 3 deletions x-pack/plugins/fleet/server/services/agents/request_diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ import type { RequestDiagnosticsAdditionalMetrics } from '../../../common/types'

import { SO_SEARCH_LIMIT, REQUEST_DIAGNOSTICS_TIMEOUT_MS } from '../../constants';

import { getCurrentNamespace } from '../spaces/get_current_namespace';

import { agentsKueryNamespaceFilter } from '../spaces/agent_namespaces';

import type { GetAgentsOptions } from '.';
import { getAgents, getAgentsByKuery } from './crud';
import { createAgentAction } from './actions';
Expand All @@ -22,9 +26,12 @@ import {

export async function requestDiagnostics(
esClient: ElasticsearchClient,
soClient: SavedObjectsClientContract,
agentId: string,
additionalMetrics?: RequestDiagnosticsAdditionalMetrics[]
): Promise<{ actionId: string }> {
const currentSpaceId = getCurrentNamespace(soClient);

const response = await createAgentAction(esClient, {
agents: [agentId],
created_at: new Date().toISOString(),
Expand All @@ -33,6 +40,7 @@ export async function requestDiagnostics(
data: {
additional_metrics: additionalMetrics,
},
namespaces: [currentSpaceId],
});
return { actionId: response.id };
}
Expand All @@ -45,24 +53,29 @@ export async function bulkRequestDiagnostics(
additionalMetrics?: RequestDiagnosticsAdditionalMetrics[];
}
): Promise<{ actionId: string }> {
const currentSpaceId = getCurrentNamespace(soClient);

if ('agentIds' in options) {
const givenAgents = await getAgents(esClient, soClient, options);
return await requestDiagnosticsBatch(esClient, givenAgents, {
additionalMetrics: options.additionalMetrics,
spaceId: currentSpaceId,
});
}

const batchSize = options.batchSize ?? SO_SEARCH_LIMIT;
const namespaceFilter = await agentsKueryNamespaceFilter(currentSpaceId);
const kuery = namespaceFilter ? `${namespaceFilter} AND ${options.kuery}` : options.kuery;
const res = await getAgentsByKuery(esClient, soClient, {
kuery: options.kuery,
kuery,
showInactive: false,
page: 1,
perPage: batchSize,
});
if (res.total <= batchSize) {
const givenAgents = await getAgents(esClient, soClient, options);
return await requestDiagnosticsBatch(esClient, givenAgents, {
return await requestDiagnosticsBatch(esClient, res.agents, {
additionalMetrics: options.additionalMetrics,
spaceId: currentSpaceId,
});
} else {
return await new RequestDiagnosticsActionRunner(
Expand All @@ -72,6 +85,7 @@ export async function bulkRequestDiagnostics(
...options,
batchSize,
total: res.total,
spaceId: currentSpaceId,
},
{ pitId: await openPointInTime(esClient) }
).runActionAsyncWithRetry();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export async function requestDiagnosticsBatch(
actionId?: string;
total?: number;
additionalMetrics?: RequestDiagnosticsAdditionalMetrics[];
spaceId?: string;
}
): Promise<{ actionId: string }> {
const errors: Record<Agent['id'], Error> = {};
Expand All @@ -56,6 +57,8 @@ export async function requestDiagnosticsBatch(
});

const agentIds = givenAgents.map((agent) => agent.id);
const spaceId = options.spaceId;
const namespaces = spaceId ? [spaceId] : [];

await createAgentAction(esClient, {
id: actionId,
Expand All @@ -67,6 +70,7 @@ export async function requestDiagnosticsBatch(
data: {
additional_metrics: options.additionalMetrics,
},
namespaces,
});

await createErrorActionResults(
Expand Down
25 changes: 20 additions & 5 deletions x-pack/plugins/fleet/server/services/agents/unenroll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { v4 as uuidv4 } from 'uuid';
import type { Agent } from '../../types';
import { HostedAgentPolicyRestrictionRelatedError } from '../../errors';
import { SO_SEARCH_LIMIT } from '../../constants';
import { getCurrentNamespace } from '../spaces/get_current_namespace';
import { agentsKueryNamespaceFilter } from '../spaces/agent_namespaces';

import { createAgentAction } from './actions';
import type { GetAgentsOptions } from './crud';
Expand Down Expand Up @@ -49,17 +51,20 @@ export async function unenrollAgent(
revoke?: boolean;
}
) {
await getAgentById(esClient, soClient, agentId); // throw 404 if agent not in namespace
if (!options?.force) {
await unenrollAgentIsAllowed(soClient, esClient, agentId);
}
if (options?.revoke) {
return forceUnenrollAgent(esClient, soClient, agentId);
}
const now = new Date().toISOString();
const currentSpaceId = getCurrentNamespace(soClient);
await createAgentAction(esClient, {
agents: [agentId],
created_at: now,
type: 'UNENROLL',
namespaces: [currentSpaceId],
});
await updateAgent(esClient, agentId, {
unenrollment_started_at: now,
Expand All @@ -76,27 +81,37 @@ export async function unenrollAgents(
showInactive?: boolean;
}
): Promise<{ actionId: string }> {
const spaceId = getCurrentNamespace(soClient);

if ('agentIds' in options) {
const givenAgents = await getAgents(esClient, soClient, options);
return await unenrollBatch(soClient, esClient, givenAgents, options);
return await unenrollBatch(soClient, esClient, givenAgents, {
...options,
spaceId,
});
}

const batchSize = options.batchSize ?? SO_SEARCH_LIMIT;
const namespaceFilter = await agentsKueryNamespaceFilter(spaceId);
const kuery = namespaceFilter ? `${namespaceFilter} AND ${options.kuery}` : options.kuery;
const res = await getAgentsByKuery(esClient, soClient, {
kuery: options.kuery,
kuery,
showInactive: options.showInactive ?? false,
page: 1,
perPage: batchSize,
});
if (res.total <= batchSize) {
const givenAgents = await getAgents(esClient, soClient, options);
return await unenrollBatch(soClient, esClient, givenAgents, options);
return await unenrollBatch(soClient, esClient, res.agents, {
...options,
spaceId,
});
} else {
return await new UnenrollActionRunner(
esClient,
soClient,
{
...options,
spaceId,
batchSize,
total: res.total,
},
Expand All @@ -120,5 +135,5 @@ export async function forceUnenrollAgent(
active: false,
unenrolled_at: new Date().toISOString(),
});
await updateActionsForForceUnenroll(esClient, [agent.id], uuidv4(), 1);
await updateActionsForForceUnenroll(esClient, soClient, [agent.id], uuidv4(), 1);
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { invalidateAPIKeys } from '../api_keys';

import { appContextService } from '../app_context';

import { getCurrentNamespace } from '../spaces/get_current_namespace';

import { ActionRunner } from './action_runner';

import { bulkUpdateAgents } from './crud';
Expand Down Expand Up @@ -61,6 +63,7 @@ export async function unenrollBatch(
revoke?: boolean;
actionId?: string;
total?: number;
spaceId?: string;
}
): Promise<{ actionId: string }> {
const hostedPolicies = await getHostedPolicies(soClient, givenAgents);
Expand Down Expand Up @@ -100,11 +103,14 @@ export async function unenrollBatch(

const agentIds = agentsToUpdate.map((agent) => agent.id);

const spaceId = options.spaceId;
const namespaces = spaceId ? [spaceId] : [];

if (options.revoke) {
// Get all API keys that need to be invalidated
await invalidateAPIKeysForAgents(agentsToUpdate);

await updateActionsForForceUnenroll(esClient, agentIds, actionId, total);
await updateActionsForForceUnenroll(esClient, soClient, agentIds, actionId, total);
} else {
// Create unenroll action for each agent
await createAgentAction(esClient, {
Expand All @@ -113,6 +119,7 @@ export async function unenrollBatch(
created_at: now,
type: 'UNENROLL',
total,
namespaces,
});
}

Expand All @@ -130,17 +137,20 @@ export async function unenrollBatch(

export async function updateActionsForForceUnenroll(
esClient: ElasticsearchClient,
soClient: SavedObjectsClientContract,
agentIds: string[],
actionId: string,
total: number
) {
// creating an action doc so that force unenroll shows up in activity
const currentSpaceId = getCurrentNamespace(soClient);
await createAgentAction(esClient, {
id: actionId,
agents: agentIds,
created_at: new Date().toISOString(),
type: 'FORCE_UNENROLL',
total,
namespaces: [currentSpaceId],
});
await bulkCreateAgentActionResults(
esClient,
Expand Down
Loading

0 comments on commit 3ba243d

Please sign in to comment.