Skip to content

Commit

Permalink
[8.13] [Fleet] Fix issue of agent sometimes not getting inputs using …
Browse files Browse the repository at this point in the history
…a new agent policy with system integration (elastic#177594) (elastic#177725)

# Backport

This will backport the following commits from `main` to `8.13`:
- [[Fleet] Fix issue of agent sometimes not getting inputs using a new
agent policy with system integration
(elastic#177594)](elastic#177594)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Julia
Bardi","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-02-23T14:46:48Z","message":"[Fleet]
Fix issue of agent sometimes not getting inputs using a new agent policy
with system integration (elastic#177594)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/177372\r\n\r\nWhen creating an
agent policy with a package policy immediately (e.g.\r\nsystem
integration), the `deployPolicy` logic was called once, creating\r\na
doc in `.fleet-policies` with `revision:1` without `inputs`, and
then\r\nupdating the doc with `inputs`, still on `revision:1`.\r\nThis
is causing an intermittent issue on the agents, if Fleet-server\r\npicks
up the first document, and delivers to agent without `inputs`.\r\nAs a
fix, added an option to skip `deploPolicy` when called from
the\r\n`createAgentPolicyWithPackages` function, as the policy will be
deployed\r\nafter creating the package policies.\r\n\r\nTo verify:\r\n-
create an agent policy with system monitoring (default option)\r\n-
check that the created documents in `.fleet-policies` are
correct:\r\nthere should be one doc with `revision_idx:1` and
`coordinator_idx:0`\r\n(created by Fleet API), and one doc with
`revision_idx:1` and\r\n`coordinator_idx:1` (created by
fleet-server)\r\n- verify that both documents have `data.inputs` field
populated\r\n\r\nUsed this query to verify:\r\n```\r\nPOST
.fleet-policies/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n
\"must\": [\r\n {\r\n \"term\": {\"coordinator_idx\": 0}\r\n }\r\n
],\r\n \"filter\": {\r\n \"term\": {\r\n \"policy_id\": \"<agent policy
id>\"\r\n }\r\n }\r\n }\r\n }, \r\n \"_source\": [\r\n
\"revision_idx\",\"coordinator_idx\", \"policy_id\", \"@timestamp\",
\"data.inputs\"\r\n ],\r\n \"sort\": [\r\n {\r\n \"revision_idx\": {\r\n
\"order\": \"desc\"\r\n }\r\n }\r\n ]\r\n}\r\n```\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"5f17b39a1d4aa326f8b75bc0d2375f620433e9be","branchLabelMapping":{"^v8.14.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Fleet","backport:prev-minor","v8.14.0"],"title":"[Fleet]
Fix issue of agent sometimes not getting inputs using a new agent policy
with system
integration","number":177594,"url":"https://github.com/elastic/kibana/pull/177594","mergeCommit":{"message":"[Fleet]
Fix issue of agent sometimes not getting inputs using a new agent policy
with system integration (elastic#177594)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/177372\r\n\r\nWhen creating an
agent policy with a package policy immediately (e.g.\r\nsystem
integration), the `deployPolicy` logic was called once, creating\r\na
doc in `.fleet-policies` with `revision:1` without `inputs`, and
then\r\nupdating the doc with `inputs`, still on `revision:1`.\r\nThis
is causing an intermittent issue on the agents, if Fleet-server\r\npicks
up the first document, and delivers to agent without `inputs`.\r\nAs a
fix, added an option to skip `deploPolicy` when called from
the\r\n`createAgentPolicyWithPackages` function, as the policy will be
deployed\r\nafter creating the package policies.\r\n\r\nTo verify:\r\n-
create an agent policy with system monitoring (default option)\r\n-
check that the created documents in `.fleet-policies` are
correct:\r\nthere should be one doc with `revision_idx:1` and
`coordinator_idx:0`\r\n(created by Fleet API), and one doc with
`revision_idx:1` and\r\n`coordinator_idx:1` (created by
fleet-server)\r\n- verify that both documents have `data.inputs` field
populated\r\n\r\nUsed this query to verify:\r\n```\r\nPOST
.fleet-policies/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n
\"must\": [\r\n {\r\n \"term\": {\"coordinator_idx\": 0}\r\n }\r\n
],\r\n \"filter\": {\r\n \"term\": {\r\n \"policy_id\": \"<agent policy
id>\"\r\n }\r\n }\r\n }\r\n }, \r\n \"_source\": [\r\n
\"revision_idx\",\"coordinator_idx\", \"policy_id\", \"@timestamp\",
\"data.inputs\"\r\n ],\r\n \"sort\": [\r\n {\r\n \"revision_idx\": {\r\n
\"order\": \"desc\"\r\n }\r\n }\r\n ]\r\n}\r\n```\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"5f17b39a1d4aa326f8b75bc0d2375f620433e9be"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.14.0","branchLabelMappingKey":"^v8.14.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/177594","number":177594,"mergeCommit":{"message":"[Fleet]
Fix issue of agent sometimes not getting inputs using a new agent policy
with system integration (elastic#177594)\n\n## Summary\r\n\r\nCloses
https://github.com/elastic/kibana/issues/177372\r\n\r\nWhen creating an
agent policy with a package policy immediately (e.g.\r\nsystem
integration), the `deployPolicy` logic was called once, creating\r\na
doc in `.fleet-policies` with `revision:1` without `inputs`, and
then\r\nupdating the doc with `inputs`, still on `revision:1`.\r\nThis
is causing an intermittent issue on the agents, if Fleet-server\r\npicks
up the first document, and delivers to agent without `inputs`.\r\nAs a
fix, added an option to skip `deploPolicy` when called from
the\r\n`createAgentPolicyWithPackages` function, as the policy will be
deployed\r\nafter creating the package policies.\r\n\r\nTo verify:\r\n-
create an agent policy with system monitoring (default option)\r\n-
check that the created documents in `.fleet-policies` are
correct:\r\nthere should be one doc with `revision_idx:1` and
`coordinator_idx:0`\r\n(created by Fleet API), and one doc with
`revision_idx:1` and\r\n`coordinator_idx:1` (created by
fleet-server)\r\n- verify that both documents have `data.inputs` field
populated\r\n\r\nUsed this query to verify:\r\n```\r\nPOST
.fleet-policies/_search\r\n{\r\n \"query\": {\r\n \"bool\": {\r\n
\"must\": [\r\n {\r\n \"term\": {\"coordinator_idx\": 0}\r\n }\r\n
],\r\n \"filter\": {\r\n \"term\": {\r\n \"policy_id\": \"<agent policy
id>\"\r\n }\r\n }\r\n }\r\n }, \r\n \"_source\": [\r\n
\"revision_idx\",\"coordinator_idx\", \"policy_id\", \"@timestamp\",
\"data.inputs\"\r\n ],\r\n \"sort\": [\r\n {\r\n \"revision_idx\": {\r\n
\"order\": \"desc\"\r\n }\r\n }\r\n ]\r\n}\r\n```\r\n\r\n\r\n###
Checklist\r\n\r\n- [x] [Unit or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common
scenarios","sha":"5f17b39a1d4aa326f8b75bc0d2375f620433e9be"}}]}]
BACKPORT-->

Co-authored-by: Julia Bardi <[email protected]>
  • Loading branch information
kibanamachine and juliaElastic authored Feb 23, 2024
1 parent 7ad328a commit 67d0d2c
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 9 deletions.
3 changes: 1 addition & 2 deletions x-pack/plugins/fleet/server/routes/agent_policy/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,7 @@ export const createAgentPolicyHandler: FleetRequestHandler<
const user = (await appContextService.getSecurity()?.authc.getCurrentUser(request)) || undefined;
const withSysMonitoring = request.query.sys_monitoring ?? false;
const monitoringEnabled = request.body.monitoring_enabled;
const force = request.body.force;
const { has_fleet_server: hasFleetServer, ...newPolicy } = request.body;
const { has_fleet_server: hasFleetServer, force, ...newPolicy } = request.body;
const spaceId = fleetContext.spaceId;
const authorizationHeader = HTTPAuthorizationHeader.parseFromRequest(request, user?.username);

Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/services/agent_policy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ describe('agent policy', () => {
expect(esClient.bulk).toBeCalledWith(
expect.objectContaining({
index: AGENT_POLICY_INDEX,
body: [
operations: [
expect.objectContaining({
index: {
_id: expect.anything(),
Expand Down
12 changes: 8 additions & 4 deletions x-pack/plugins/fleet/server/services/agent_policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ class AgentPolicyService {
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient,
action: 'created' | 'updated' | 'deleted',
agentPolicyId: string
agentPolicyId: string,
options?: { skipDeploy?: boolean }
) => {
return agentPolicyUpdateEventHandler(soClient, esClient, action, agentPolicyId);
return agentPolicyUpdateEventHandler(soClient, esClient, action, agentPolicyId, options);
};

private async _update(
Expand Down Expand Up @@ -286,6 +287,7 @@ class AgentPolicyService {
id?: string;
user?: AuthenticatedUser;
authorizationHeader?: HTTPAuthorizationHeader | null;
skipDeploy?: boolean;
} = {}
): Promise<AgentPolicy> {
// Ensure an ID is provided, so we can include it in the audit logs below
Expand Down Expand Up @@ -330,7 +332,9 @@ class AgentPolicyService {
);

await appContextService.getUninstallTokenService()?.generateTokenForPolicyId(newSo.id);
await this.triggerAgentPolicyUpdatedEvent(soClient, esClient, 'created', newSo.id);
await this.triggerAgentPolicyUpdatedEvent(soClient, esClient, 'created', newSo.id, {
skipDeploy: options.skipDeploy,
});
logger.debug(`Created new agent policy with id ${newSo.id}`);
return { id: newSo.id, ...newSo.attributes };
}
Expand Down Expand Up @@ -1034,7 +1038,7 @@ class AgentPolicyService {

const bulkResponse = await esClient.bulk({
index: AGENT_POLICY_INDEX,
body: fleetServerPoliciesBulkBody,
operations: fleetServerPoliciesBulkBody,
refresh: 'wait_for',
});

Expand Down
29 changes: 29 additions & 0 deletions x-pack/plugins/fleet/server/services/agent_policy_create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,35 @@ describe('createAgentPolicyWithPackages', () => {
);
});

it('should call deploy policy once when create policy with system package', async () => {
mockedAgentPolicyService.deployPolicy.mockClear();
mockedAgentPolicyService.create.mockImplementation((soClient, esClient, newPolicy, options) => {
if (!options?.skipDeploy) {
mockedAgentPolicyService.deployPolicy(soClientMock, 'new_id');
}
return Promise.resolve({
...newPolicy,
id: options?.id || 'new_id',
} as AgentPolicy);
});
const response = await createAgentPolicyWithPackages({
esClient: esClientMock,
soClient: soClientMock,
newPolicy: { name: 'Agent policy 1', namespace: 'default' },
withSysMonitoring: true,
spaceId: 'default',
});

expect(response.id).toEqual('new_id');
expect(mockedBulkInstallPackages).toHaveBeenCalledWith({
savedObjectsClient: soClientMock,
esClient: esClientMock,
packagesToInstall: ['system'],
spaceId: 'default',
});
expect(mockedAgentPolicyService.deployPolicy).toHaveBeenCalledTimes(1);
});

it('should create policy with system and elastic_agent package', async () => {
const response = await createAgentPolicyWithPackages({
esClient: esClientMock,
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/fleet/server/services/agent_policy_create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ async function createPackagePolicy(
spaceId: string;
user: AuthenticatedUser | undefined;
authorizationHeader?: HTTPAuthorizationHeader | null;
force?: boolean;
}
) {
const newPackagePolicy = await packagePolicyService
Expand All @@ -78,6 +79,7 @@ async function createPackagePolicy(
user: options.user,
bumpRevision: false,
authorizationHeader: options.authorizationHeader,
force: options.force,
});
}

Expand Down Expand Up @@ -140,6 +142,7 @@ export async function createAgentPolicyWithPackages({
user,
id: agentPolicyId,
authorizationHeader,
skipDeploy: true, // skip deploying the policy until package policies are added
});

// Create the fleet server package policy and add it to agent policy.
Expand All @@ -148,6 +151,7 @@ export async function createAgentPolicyWithPackages({
spaceId,
user,
authorizationHeader,
force,
});
}

Expand All @@ -157,6 +161,7 @@ export async function createAgentPolicyWithPackages({
spaceId,
user,
authorizationHeader,
force,
});
}

Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/fleet/server/services/agent_policy_update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export async function agentPolicyUpdateEventHandler(
soClient: SavedObjectsClientContract,
esClient: ElasticsearchClient,
action: string,
agentPolicyId: string
agentPolicyId: string,
options?: { skipDeploy?: boolean }
) {
// `soClient` from ingest `appContextService` is used to create policy change actions
// to ensure encrypted SOs are handled correctly
Expand All @@ -44,7 +45,9 @@ export async function agentPolicyUpdateEventHandler(
agentPolicyId,
forceRecreate: true,
});
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId);
if (!options?.skipDeploy) {
await agentPolicyService.deployPolicy(internalSoClient, agentPolicyId);
}
}

if (action === 'updated') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,32 @@ export default function (providerContext: FtrProviderContext) {
});
});

it('should create .fleet-policies document with inputs', async () => {
const res = await supertest
.post(`/api/fleet/agent_policies?sys_monitoring=true`)
.set('kbn-xsrf', 'xxxx')
.send({
name: 'test-policy-with-system',
namespace: 'default',
force: true, // using force to bypass package verification error
})
.expect(200);

const policyDocRes = await es.search({
index: '.fleet-policies',
query: {
term: {
policy_id: res.body.item.id,
},
},
});

expect(policyDocRes?.hits?.hits.length).to.eql(1);
const source = policyDocRes?.hits?.hits[0]?._source as any;
expect(source?.revision_idx).to.eql(1);
expect(source?.data?.inputs.length).to.eql(3);
});

it('should return a 400 with an empty namespace', async () => {
await supertest
.post(`/api/fleet/agent_policies`)
Expand Down

0 comments on commit 67d0d2c

Please sign in to comment.