Skip to content

Commit

Permalink
[Fleet] Avoid extra fetch package policy calls (elastic#190398)
Browse files Browse the repository at this point in the history
## Summary

i noticed few extra calls which can be prevented during package policy
updates/creation while investigating synthetics project monitor API call
!!

---------

Co-authored-by: Nicolas Chaulet <n.chaulet@gmail.com>
  • Loading branch information
shahzad31 and nchaulet authored Aug 14, 2024
1 parent 12e6a92 commit 8e736c4
Show file tree
Hide file tree
Showing 7 changed files with 66 additions and 20 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/fleet/common/types/rest_spec/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export interface ListWithKuery extends HttpFetchQuery {
sortField?: string;
sortOrder?: 'desc' | 'asc';
kuery?: string;
fields?: string[];
}

export interface ListResult<T> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import type {
FullAgentPolicyOutput,
FleetProxy,
FleetServerHost,
AgentPolicy,
} from '../../types';
import type {
FullAgentPolicyMonitoring,
Expand Down Expand Up @@ -67,11 +68,17 @@ async function fetchAgentPolicy(soClient: SavedObjectsClientContract, id: string
export async function getFullAgentPolicy(
soClient: SavedObjectsClientContract,
id: string,
options?: { standalone: boolean }
options?: { standalone?: boolean; agentPolicy?: AgentPolicy }
): Promise<FullAgentPolicy | null> {
const standalone = options?.standalone ?? false;

const agentPolicy = await fetchAgentPolicy(soClient, id);
let agentPolicy: AgentPolicy | null;
if (options?.agentPolicy?.package_policies) {
agentPolicy = options.agentPolicy;
} else {
agentPolicy = await fetchAgentPolicy(soClient, id);
}

if (!agentPolicy) {
return null;
}
Expand Down
49 changes: 37 additions & 12 deletions x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class AgentPolicyService {
esClient: ElasticsearchClient,
action: 'created' | 'updated' | 'deleted',
agentPolicyId: string,
options?: { skipDeploy?: boolean; spaceId?: string }
options?: { skipDeploy?: boolean; spaceId?: string; agentPolicy?: AgentPolicy | null }
) => {
return agentPolicyUpdateEventHandler(esClient, action, agentPolicyId, options);
};
Expand All @@ -132,10 +132,16 @@ class AgentPolicyService {
id: string,
agentPolicy: Partial<AgentPolicySOAttributes>,
user?: AuthenticatedUser,
options: { bumpRevision: boolean; removeProtection: boolean; skipValidation: boolean } = {
options: {
bumpRevision: boolean;
removeProtection: boolean;
skipValidation: boolean;
returnUpdatedPolicy?: boolean;
} = {
bumpRevision: true,
removeProtection: false,
skipValidation: false,
returnUpdatedPolicy: true,
}
): Promise<AgentPolicy> {
auditLoggingService.writeCustomSoAuditLog({
Expand Down Expand Up @@ -173,6 +179,7 @@ class AgentPolicyService {
getAllowedOutputTypeForPolicy(existingAgentPolicy)
);
}

await soClient.update<AgentPolicySOAttributes>(SAVED_OBJECT_TYPE, id, {
...agentPolicy,
...(options.bumpRevision ? { revision: existingAgentPolicy.revision + 1 } : {}),
Expand All @@ -183,17 +190,25 @@ class AgentPolicyService {
updated_by: user ? user.username : 'system',
});

const newAgentPolicy = await this.get(soClient, id, false);

newAgentPolicy!.package_policies = existingAgentPolicy.package_policies;

if (options.bumpRevision || options.removeProtection) {
await this.triggerAgentPolicyUpdatedEvent(esClient, 'updated', id, {
spaceId: soClient.getCurrentNamespace(),
agentPolicy: newAgentPolicy,
});
}
logger.debug(
`Agent policy ${id} update completed, revision: ${
options.bumpRevision ? existingAgentPolicy.revision + 1 : existingAgentPolicy.revision
}`
);
return (await this.get(soClient, id)) as AgentPolicy;
if (options.returnUpdatedPolicy !== false) {
return (await this.get(soClient, id)) as AgentPolicy;
}
return newAgentPolicy as AgentPolicy;
}

public async ensurePreconfiguredAgentPolicy(
Expand Down Expand Up @@ -772,14 +787,13 @@ class AgentPolicyService {
esClient: ElasticsearchClient,
id: string,
options?: { user?: AuthenticatedUser; removeProtection?: boolean }
): Promise<AgentPolicy> {
const res = await this._update(soClient, esClient, id, {}, options?.user, {
): Promise<void> {
await this._update(soClient, esClient, id, {}, options?.user, {
bumpRevision: true,
removeProtection: options?.removeProtection ?? false,
skipValidation: false,
returnUpdatedPolicy: false,
});

return res;
}

/**
Expand Down Expand Up @@ -1132,11 +1146,19 @@ class AgentPolicyService {
};
}

public async deployPolicy(soClient: SavedObjectsClientContract, agentPolicyId: string) {
await this.deployPolicies(soClient, [agentPolicyId]);
public async deployPolicy(
soClient: SavedObjectsClientContract,
agentPolicyId: string,
agentPolicy?: AgentPolicy | null
) {
await this.deployPolicies(soClient, [agentPolicyId], agentPolicy ? [agentPolicy] : undefined);
}

public async deployPolicies(soClient: SavedObjectsClientContract, agentPolicyIds: string[]) {
public async deployPolicies(
soClient: SavedObjectsClientContract,
agentPolicyIds: string[],
agentPolicies?: AgentPolicy[]
) {
// Use internal ES client so we have permissions to write to .fleet* indices
const esClient = appContextService.getInternalUserESClient();
const defaultOutputId = await outputService.getDefaultDataOutputId(soClient);
Expand All @@ -1158,7 +1180,10 @@ class AgentPolicyService {
// There are some potential performance concerns around using `getFullAgentPolicy` in this context, e.g.
// re-fetching outputs, settings, and upgrade download source URI data for each policy. This could potentially
// be a bottleneck in environments with several thousand agent policies being deployed here.
(agentPolicyId) => agentPolicyService.getFullAgentPolicy(soClient, agentPolicyId),
(agentPolicyId) =>
agentPolicyService.getFullAgentPolicy(soClient, agentPolicyId, {
agentPolicy: agentPolicies?.find((policy) => policy.id === agentPolicyId),
}),
{
concurrency: 50,
}
Expand Down Expand Up @@ -1359,7 +1384,7 @@ class AgentPolicyService {
public async getFullAgentPolicy(
soClient: SavedObjectsClientContract,
id: string,
options?: { standalone: boolean }
options?: { standalone?: boolean; agentPolicy?: AgentPolicy }
): Promise<FullAgentPolicy | null> {
return getFullAgentPolicy(soClient, id, options);
}
Expand Down
8 changes: 5 additions & 3 deletions x-pack/plugins/fleet/server/services/agent_policy_update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import type { KibanaRequest } from '@kbn/core/server';
import type { ElasticsearchClient } from '@kbn/core/server';

import type { AgentPolicy } from '../../common';

import { generateEnrollmentAPIKey, deleteEnrollmentApiKeyForAgentPolicyId } from './api_keys';
import { unenrollForAgentPolicyId } from './agents';
import { agentPolicyService } from './agent_policy';
Expand All @@ -32,7 +34,7 @@ export async function agentPolicyUpdateEventHandler(
esClient: ElasticsearchClient,
action: string,
agentPolicyId: string,
options?: { skipDeploy?: boolean; spaceId?: string }
options?: { skipDeploy?: boolean; spaceId?: string; agentPolicy?: AgentPolicy | null }
) {
// `soClient` from ingest `appContextService` is used to create policy change actions
// to ensure encrypted SOs are handled correctly
Expand All @@ -47,12 +49,12 @@ export async function agentPolicyUpdateEventHandler(
forceRecreate: true,
});
if (!options?.skipDeploy) {
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId);
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId, options?.agentPolicy);
}
}

if (action === 'updated') {
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId);
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId, options?.agentPolicy);
}

if (action === 'deleted') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ describe('checkFleetServerVersionsForSecretsStorage', () => {
afterEach(() => {
appContextService.stop();
jest.clearAllMocks();
jest.restoreAllMocks();
});

const esClientMock = elasticsearchServiceMock.createElasticsearchClient();
Expand Down
6 changes: 4 additions & 2 deletions x-pack/plugins/fleet/server/services/fleet_server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,18 +113,20 @@ export async function checkFleetServerVersionsForSecretsStorage(
let hasMore = true;
const policyIds = new Set<string>();
let page = 1;
const perPage = 200;
while (hasMore) {
const res = await packagePolicyService.list(soClient, {
page: page++,
perPage: 20,
perPage,
kuery: 'ingest-package-policies.package.name:fleet_server',
fields: ['policy_ids'],
});

for (const item of res.items) {
item.policy_ids.forEach((id) => policyIds.add(id));
}

if (res.items.length === 0) {
if (res.items.length < perPage) {
hasMore = false;
}
}
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/fleet/server/services/package_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -739,14 +739,22 @@ class PackagePolicyClientImpl implements PackagePolicyClient {
soClient: SavedObjectsClientContract,
options: ListWithKuery & { spaceId?: string }
): Promise<ListResult<PackagePolicy>> {
const { page = 1, perPage = 20, sortField = 'updated_at', sortOrder = 'desc', kuery } = options;
const {
page = 1,
perPage = 20,
sortField = 'updated_at',
sortOrder = 'desc',
kuery,
fields,
} = options;

const packagePolicies = await soClient.find<PackagePolicySOAttributes>({
type: SAVED_OBJECT_TYPE,
sortField,
sortOrder,
page,
perPage,
fields,
filter: kuery ? normalizeKuery(SAVED_OBJECT_TYPE, kuery) : undefined,
namespaces: options.spaceId ? [options.spaceId] : undefined,
});
Expand Down

0 comments on commit 8e736c4

Please sign in to comment.