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 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
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 @@ -2520,16 +2520,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 @@ -1580,13 +1580,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 @@ -91,8 +91,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
14 changes: 11 additions & 3 deletions x-pack/plugins/fleet/server/services/agents/action_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import moment from 'moment';
import type { Agent } from '../../types';
import { appContextService } from '..';
import { SO_SEARCH_LIMIT } from '../../../common/constants';
import { agentsKueryNamespaceFilter } from '../spaces/agent_namespaces';

import { getAgentActions } from './actions';
import { closePointInTime, getAgentsByKuery } from './crud';
Expand All @@ -29,6 +30,7 @@ export interface ActionParams {
batchSize?: number;
total?: number;
actionId?: string;
spaceId?: string;
// additional parameters specific to an action e.g. reassign to new policy id
[key: string]: any;
}
Expand Down Expand Up @@ -195,15 +197,21 @@ export abstract class ActionRunner {

appContextService.getLogger().debug('kuery: ' + this.actionParams.kuery);

const getAgents = () =>
getAgentsByKuery(this.esClient, this.soClient, {
kuery: this.actionParams.kuery,
const getAgents = async () => {
const namespaceFilter = await agentsKueryNamespaceFilter(this.actionParams.spaceId);
const kuery = namespaceFilter
? `${namespaceFilter} AND ${this.actionParams.kuery}`
: this.actionParams.kuery;

return getAgentsByKuery(this.esClient, this.soClient, {
kuery,
showInactive: this.actionParams.showInactive ?? false,
page: 1,
perPage,
pitId,
searchAfter: this.retryParams.searchAfter,
});
};

const res = await getAgents();

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: await 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
6 changes: 4 additions & 2 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 { isSpaceAwarenessEnabled } from '../spaces/helpers';
import { isAgentInNamespace } from '../spaces/agent_namespaces';
import { addNamespaceFilteringToQuery } from '../spaces/query_namespaces_filtering';

import { searchHitToAgent, agentSOAttributesToFleetServerAgentDoc } from './helpers';
import { buildAgentStatusRuntimeField } from './build_status_runtime_field';
Expand Down Expand Up @@ -432,6 +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);

let res;
try {
Expand All @@ -443,7 +445,7 @@ async function _filterAgents(
runtime_mappings: runtimeFields,
fields: Object.keys(runtimeFields),
sort: [{ [sortField]: { order: sortOrder } }],
query: { bool: { filter: query } },
query: await addNamespaceFilteringToQuery({ bool: { filter: [query] } }, currentNameSpace),
index: AGENTS_INDEX,
ignore_unavailable: true,
});
Expand Down
19 changes: 12 additions & 7 deletions x-pack/plugins/fleet/server/services/agents/reassign.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,15 @@ import { reassignAgent, reassignAgents } from './reassign';
import { createClientMock } from './action.mock';

describe('reassignAgent', () => {
let mocks: ReturnType<typeof createClientMock>;

beforeEach(async () => {
const { soClient } = createClientMock();
mocks = createClientMock();

appContextService.start(
createAppContextStartContractMock({}, false, {
withoutSpaceExtensions: soClient,
internal: mocks.soClient,
withoutSpaceExtensions: mocks.soClient,
})
);
});
Expand All @@ -29,7 +33,7 @@ describe('reassignAgent', () => {
});
describe('reassignAgent (singular)', () => {
it('can reassign from regular agent policy to regular', async () => {
const { soClient, esClient, agentInRegularDoc, regularAgentPolicySO } = createClientMock();
const { soClient, esClient, agentInRegularDoc, regularAgentPolicySO } = mocks;
await reassignAgent(soClient, esClient, agentInRegularDoc._id, regularAgentPolicySO.id);

// calls ES update with correct values
Expand All @@ -43,7 +47,7 @@ describe('reassignAgent', () => {
});

it('cannot reassign from regular agent policy to hosted', async () => {
const { soClient, esClient, agentInRegularDoc, hostedAgentPolicySO } = createClientMock();
const { soClient, esClient, agentInRegularDoc, hostedAgentPolicySO } = mocks;
await expect(
reassignAgent(soClient, esClient, agentInRegularDoc._id, hostedAgentPolicySO.id)
).rejects.toThrowError(HostedAgentPolicyRestrictionRelatedError);
Expand All @@ -54,7 +58,7 @@ describe('reassignAgent', () => {

it('cannot reassign from hosted agent policy', async () => {
const { soClient, esClient, agentInHostedDoc, hostedAgentPolicySO, regularAgentPolicySO } =
createClientMock();
mocks;
await expect(
reassignAgent(soClient, esClient, agentInHostedDoc._id, regularAgentPolicySO.id)
).rejects.toThrowError(HostedAgentPolicyRestrictionRelatedError);
Expand All @@ -78,7 +82,7 @@ describe('reassignAgent', () => {
agentInHostedDoc,
agentInHostedDoc2,
regularAgentPolicySO2,
} = createClientMock();
} = mocks;

esClient.search.mockResponse({
hits: {
Expand Down Expand Up @@ -116,7 +120,8 @@ describe('reassignAgent', () => {
});

it('should report errors from ES agent update call', async () => {
const { soClient, esClient, agentInRegularDoc, regularAgentPolicySO2 } = createClientMock();
const { soClient, esClient, agentInRegularDoc, regularAgentPolicySO2 } = mocks;

esClient.bulk.mockResponse({
items: [
{
Expand Down
Loading
Loading