Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] RBAC - Make upgrade agent APIs space aware #190069

Merged
Merged
Show file tree
Hide file tree
Changes from 5 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
10 changes: 1 addition & 9 deletions x-pack/plugins/fleet/common/openapi/bundled.json
Original file line number Diff line number Diff line change
Expand Up @@ -2471,16 +2471,8 @@
}
}
},
"/agents/{agentId}/actions/{actionId}/cancel": {
"/agents/actions/{actionId}/cancel": {
"parameters": [
{
"schema": {
"type": "string"
},
"name": "agentId",
"in": "path",
"required": true
},
{
"schema": {
"type": "string"
Expand Down
7 changes: 1 addition & 6 deletions x-pack/plugins/fleet/common/openapi/bundled.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1564,13 +1564,8 @@ paths:
properties:
action:
$ref: '#/components/schemas/agent_action'
/agents/{agentId}/actions/{actionId}/cancel:
/agents/actions/{actionId}/cancel:
parameters:
- schema:
type: string
name: agentId
in: path
required: true
- schema:
type: string
name: actionId
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/common/openapi/entrypoint.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ paths:
$ref: 'paths/agents@{agent_id}.yaml'
'/agents/{agentId}/actions':
$ref: 'paths/agents@{agent_id}@actions.yaml'
'/agents/{agentId}/actions/{actionId}/cancel':
$ref: 'paths/agents@{agent_id}@actions@{action_id}@cancel.yaml'
'/agents/actions/{actionId}/cancel':
$ref: 'paths/agents@actions@{action_id}@cancel.yaml'
'/agents/files/{fileId}/{fileName}':
$ref: 'paths/agents@files@{file_id}@{file_name}.yaml'
'/agents/files/{fileId}':
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
parameters:
- schema:
type: string
name: agentId
in: path
required: true
- schema:
type: string
name: actionId
Expand Down
10 changes: 8 additions & 2 deletions x-pack/plugins/fleet/server/routes/agent/actions_handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,15 @@ export const postCancelActionHandlerBuilder = function (
): RequestHandler<TypeOf<typeof PostCancelActionRequestSchema.params>, undefined, undefined> {
return async (context, request, response) => {
try {
const esClient = (await context.core).elasticsearch.client.asInternalUser;
const core = await context.core;
const esClient = core.elasticsearch.client.asInternalUser;
const soClient = core.savedObjects.client;

const action = await actionsService.cancelAgentAction(esClient, request.params.actionId);
const action = await actionsService.cancelAgentAction(
esClient,
soClient,
request.params.actionId
);

const body: PostNewAgentActionResponse = {
item: action,
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/routes/agent/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ export const putAgentsReassignHandlerDeprecated: RequestHandler<
}
};

export const postAgentsReassignHandler: RequestHandler<
export const postAgentReassignHandler: RequestHandler<
TypeOf<typeof PostAgentReassignRequestSchema.params>,
undefined,
TypeOf<typeof PostAgentReassignRequestSchema.body>
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugins/fleet/server/routes/agent/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import {
getAgentUploadsHandler,
getAgentUploadFileHandler,
deleteAgentUploadFileHandler,
postAgentsReassignHandler,
postAgentReassignHandler,
postRetrieveAgentsByActionsHandler,
} from './handlers';
import {
Expand Down Expand Up @@ -271,7 +271,7 @@ export const registerAPIRoutes = (router: FleetAuthzRouter, config: FleetConfigT
version: API_VERSIONS.public.v1,
validate: { request: PostAgentReassignRequestSchema },
},
postAgentsReassignHandler
postAgentReassignHandler
);

router.versioned
Expand Down
17 changes: 10 additions & 7 deletions x-pack/plugins/fleet/server/services/agents/actions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import { elasticsearchServiceMock } from '@kbn/core/server/mocks';
import { elasticsearchServiceMock, savedObjectsClientMock } from '@kbn/core/server/mocks';

import type { NewAgentAction, AgentActionType } from '../../../common/types';

Expand Down Expand Up @@ -307,16 +307,17 @@ describe('Agent actions', () => {
});

describe('cancelAgentAction', () => {
it('throw if the target action is not found', async () => {
it('should throw if the target action is not found', async () => {
const esClient = elasticsearchServiceMock.createInternalClient();
esClient.search.mockResolvedValue({
hits: {
hits: [],
},
} as any);
await expect(() => cancelAgentAction(esClient, 'i-do-not-exists')).rejects.toThrowError(
/Action not found/
);
const soClient = savedObjectsClientMock.create();
await expect(() =>
cancelAgentAction(esClient, soClient, 'i-do-not-exists')
).rejects.toThrowError(/Action not found/);
});

it('should create one CANCEL action for each UPGRADE action found', async () => {
Expand All @@ -343,7 +344,8 @@ describe('Agent actions', () => {
],
},
} as any);
await cancelAgentAction(esClient, 'action1');
const soClient = savedObjectsClientMock.create();
await cancelAgentAction(esClient, soClient, 'action1');

expect(esClient.create).toBeCalledTimes(2);
expect(esClient.create).toBeCalledWith(
Expand Down Expand Up @@ -382,7 +384,8 @@ describe('Agent actions', () => {
],
},
} as any);
await cancelAgentAction(esClient, 'action1');
const soClient = savedObjectsClientMock.create();
await cancelAgentAction(esClient, soClient, 'action1');

expect(mockedBulkUpdateAgents).toBeCalled();
expect(mockedBulkUpdateAgents).toBeCalledWith(
Expand Down
43 changes: 30 additions & 13 deletions x-pack/plugins/fleet/server/services/agents/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ import { auditLoggingService } from '../audit_logging';

import { getAgentIdsForAgentPolicies } from '../agent_policies/agent_policies_to_agent_ids';

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

import { bulkUpdateAgents } from './crud';

const ONE_MONTH_IN_MS = 2592000000;
Expand Down Expand Up @@ -305,21 +308,28 @@ export async function getUnenrollAgentActions(
return result;
}

export async function cancelAgentAction(esClient: ElasticsearchClient, actionId: string) {
export async function cancelAgentAction(
esClient: ElasticsearchClient,
soClient: SavedObjectsClientContract,
actionId: string
) {
const currentNameSpace = getCurrentNamespace(soClient);

const getUpgradeActions = async () => {
const res = await esClient.search<FleetServerAgentAction>({
index: AGENT_ACTIONS_INDEX,
query: {
bool: {
filter: [
{
term: {
action_id: actionId,
},
const query = {
bool: {
filter: [
{
term: {
action_id: actionId,
},
],
},
},
],
},
};
const res = await esClient.search<FleetServerAgentAction>({
index: AGENT_ACTIONS_INDEX,
query: addNamespaceFilteringToQuery(query, currentNameSpace),
size: SO_SEARCH_LIMIT,
});

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

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

await createAgentAction(esClient, {
id: cancelActionId,
type: 'CANCEL',
...namespaces,
agents: action.agents!,
data: {
target_id: action.action_id,
Expand Down Expand Up @@ -505,7 +518,11 @@ export interface ActionsService {
agentId: string
) => Promise<Agent>;

cancelAgentAction: (esClient: ElasticsearchClient, actionId: string) => Promise<AgentAction>;
cancelAgentAction: (
esClient: ElasticsearchClient,
soClient: SavedObjectsClientContract,
actionId: string
) => Promise<AgentAction>;

createAgentAction: (
esClient: ElasticsearchClient,
Expand Down
10 changes: 4 additions & 6 deletions x-pack/plugins/fleet/server/services/agents/crud.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ import {
FleetUnauthorizedError,
} from '../../errors';
import { auditLoggingService } from '../audit_logging';
import { isAgentInNamespace } from '../spaces/agent_namespaces';
import { getCurrentNamespace } from '../spaces/get_current_namespace';

import { addNamespaceFilteringToQuery } from '../spaces/query_namespaces_filtering';

import { searchHitToAgent, agentSOAttributesToFleetServerAgentDoc } from './helpers';
import { buildAgentStatusRuntimeField } from './build_status_runtime_field';
import { getLatestAvailableAgentVersion } from './versions';
Expand Down Expand Up @@ -406,10 +407,6 @@ export async function getAgentById(
throw new AgentNotFoundError(`Agent ${agentId} not found`);
}

if (!isAgentInNamespace(agentHit, getCurrentNamespace(soClient))) {
throw new AgentNotFoundError(`${agentHit.id} not found in namespace`);
}

return agentHit;
}

Expand All @@ -431,6 +428,7 @@ async function _filterAgents(
}> {
const { page = 1, perPage = 20, sortField = 'enrolled_at', sortOrder = 'desc' } = options;
const runtimeFields = await buildAgentStatusRuntimeField(soClient);
const currentNameSpace = getCurrentNamespace(soClient);

let res;
try {
Expand All @@ -442,7 +440,7 @@ async function _filterAgents(
runtime_mappings: runtimeFields,
fields: Object.keys(runtimeFields),
sort: [{ [sortField]: { order: sortOrder } }],
query: { bool: { filter: query } },
query: addNamespaceFilteringToQuery({ bool: { filter: [query] } }, currentNameSpace),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nchaulet What do you think of this change? I realised that a lot of bulk endpoints were calling getAgentsById and then checking isAgentInNamespace on each agent. With this, getAgentsById directly adds a namespace filter in the ES query. The downside is we lose the more specific "agent not in namespace" error message.

Copy link
Member

Choose a reason for hiding this comment

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

it seems okay to me 👍

index: AGENTS_INDEX,
ignore_unavailable: true,
});
Expand Down
Loading