Skip to content

Commit

Permalink
[8.9] [Fleet] Fix exception in agents list when units field is missin…
Browse files Browse the repository at this point in the history
…g from components (#161360) (#161442)

# Backport

This will backport the following commits from `main` to `8.9`:
- [[Fleet] Fix exception in agents list when units field is missing from
components (#161360)](#161360)

<!--- Backport version: 8.9.7 -->

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

<!--BACKPORT [{"author":{"name":"Cristina
Amico","email":"[email protected]"},"sourceCommit":{"committedDate":"2023-07-07T10:13:37Z","message":"[Fleet]
Fix exception in agents list when units field is missing from components
(#161360)\n\n## Summary\r\nFix an issue found in 8.8.2 with `8.7.1`
inactive agents. The agents\r\nlist endpoint was returning an exception,
after some investigation it\r\nwas found that the issue was a missing
`units` property in the\r\ncomponents field of the agents.\r\n\r\nThis
was a very specific edge case and in fact I could not
reproduce\r\nlocally, but it still happened in a minority of cases after
upgrading to\r\n8.8.2.\r\n\r\nThe agents in this case have components
that look like this:\r\n```\r\n \"components\": [\r\n {\r\n \"id\":
\"http/metrics-monitoring\",\r\n \"type\": \"http/metrics\",\r\n
\"message\": \"Starting\",\r\n \"status\": \"STARTING\"\r\n },\r\n
...\r\n]\r\n```\r\nBut this property was typed as mandatory so we didn't
check for\r\nundefined. The actual bug was tracked
here:\r\n\r\nhttps://github.com/elastic/kibana/blob/2070836060046aaec0ff28ddfcce91356cf98930/x-pack/plugins/fleet/server/services/agents/helpers.ts#L50\r\n\r\nand
was causing this
error:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/607b5aed-155b-4509-896a-ada7507e4dc3\r\n\r\nI
also updated the type to have `units` as optional and fixed a
couple\r\nof other places where this could cause issues.\r\n\r\n###
Checklist\r\n- [ ] [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\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"36e7d600ce9cbc39b7a0e55165a65a4634b30b6c","branchLabelMapping":{"^v8.10.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:Fleet","backport:prev-minor","v8.9.0","v8.10.0","v8.8.3"],"number":161360,"url":"https://github.com/elastic/kibana/pull/161360","mergeCommit":{"message":"[Fleet]
Fix exception in agents list when units field is missing from components
(#161360)\n\n## Summary\r\nFix an issue found in 8.8.2 with `8.7.1`
inactive agents. The agents\r\nlist endpoint was returning an exception,
after some investigation it\r\nwas found that the issue was a missing
`units` property in the\r\ncomponents field of the agents.\r\n\r\nThis
was a very specific edge case and in fact I could not
reproduce\r\nlocally, but it still happened in a minority of cases after
upgrading to\r\n8.8.2.\r\n\r\nThe agents in this case have components
that look like this:\r\n```\r\n \"components\": [\r\n {\r\n \"id\":
\"http/metrics-monitoring\",\r\n \"type\": \"http/metrics\",\r\n
\"message\": \"Starting\",\r\n \"status\": \"STARTING\"\r\n },\r\n
...\r\n]\r\n```\r\nBut this property was typed as mandatory so we didn't
check for\r\nundefined. The actual bug was tracked
here:\r\n\r\nhttps://github.com/elastic/kibana/blob/2070836060046aaec0ff28ddfcce91356cf98930/x-pack/plugins/fleet/server/services/agents/helpers.ts#L50\r\n\r\nand
was causing this
error:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/607b5aed-155b-4509-896a-ada7507e4dc3\r\n\r\nI
also updated the type to have `units` as optional and fixed a
couple\r\nof other places where this could cause issues.\r\n\r\n###
Checklist\r\n- [ ] [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\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"36e7d600ce9cbc39b7a0e55165a65a4634b30b6c"}},"sourceBranch":"main","suggestedTargetBranches":["8.9","8.8"],"targetPullRequestStates":[{"branch":"8.9","label":"v8.9.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.10.0","labelRegex":"^v8.10.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/161360","number":161360,"mergeCommit":{"message":"[Fleet]
Fix exception in agents list when units field is missing from components
(#161360)\n\n## Summary\r\nFix an issue found in 8.8.2 with `8.7.1`
inactive agents. The agents\r\nlist endpoint was returning an exception,
after some investigation it\r\nwas found that the issue was a missing
`units` property in the\r\ncomponents field of the agents.\r\n\r\nThis
was a very specific edge case and in fact I could not
reproduce\r\nlocally, but it still happened in a minority of cases after
upgrading to\r\n8.8.2.\r\n\r\nThe agents in this case have components
that look like this:\r\n```\r\n \"components\": [\r\n {\r\n \"id\":
\"http/metrics-monitoring\",\r\n \"type\": \"http/metrics\",\r\n
\"message\": \"Starting\",\r\n \"status\": \"STARTING\"\r\n },\r\n
...\r\n]\r\n```\r\nBut this property was typed as mandatory so we didn't
check for\r\nundefined. The actual bug was tracked
here:\r\n\r\nhttps://github.com/elastic/kibana/blob/2070836060046aaec0ff28ddfcce91356cf98930/x-pack/plugins/fleet/server/services/agents/helpers.ts#L50\r\n\r\nand
was causing this
error:\r\n\r\n\r\nhttps://github.com/elastic/kibana/assets/16084106/607b5aed-155b-4509-896a-ada7507e4dc3\r\n\r\nI
also updated the type to have `units` as optional and fixed a
couple\r\nof other places where this could cause issues.\r\n\r\n###
Checklist\r\n- [ ] [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\r\n\r\n---------\r\n\r\nCo-authored-by: Kibana Machine
<[email protected]>","sha":"36e7d600ce9cbc39b7a0e55165a65a4634b30b6c"}},{"branch":"8.8","label":"v8.8.3","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Cristina Amico <[email protected]>
  • Loading branch information
kibanamachine and criamico authored Jul 7, 2023
1 parent a943ac0 commit 3c59c4d
Show file tree
Hide file tree
Showing 6 changed files with 154 additions and 4 deletions.
3 changes: 2 additions & 1 deletion x-pack/plugins/fleet/common/types/models/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ export interface FleetServerAgentComponent {
type: string;
status: FleetServerAgentComponentStatus;
message: string;
units: FleetServerAgentComponentUnit[];
// In some case units could be missing
units?: FleetServerAgentComponentUnit[];
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,4 +106,21 @@ describe('AgentDetailsIntegrationInputs', () => {
component.getByTestId('agentDetailsIntegrationsInputStatusHealthSuccess')
).toBeInTheDocument();
});

it('does not render when there is no units array', () => {
agent.components = [
{
id: 'endpoint-default',
type: 'endpoint',
status: 'HEALTHY',
message: 'Healthy',
},
];

const component = renderComponent();
userEvent.click(component.getByTestId('agentIntegrationsInputsTitle'));
expect(
component.queryByTestId('agentDetailsIntegrationsInputStatusHealthSuccess')
).not.toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,13 @@ describe('getInputUnitsByPackage', () => {
},
],
},
// test a component with no units
{
id: 'beat/metrics-monitoring',
type: 'beat/metrics',
status: 'HEALTHY',
message: "Healthy: communicating with pid '2234'",
},
];

const packageMock = createPackagePolicyMock();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ export const getInputUnitsByPackage = (
packagePolicy: PackagePolicy
): FleetServerAgentComponentUnit[] => {
const re = new RegExp(packagePolicy.id);

return agentComponents
.map((c) => c.units)
.map((c) => c?.units || [])
.flat()
.filter((u) => u.id.match(re));
.filter((u) => !!u && u.id.match(re));
};
124 changes: 124 additions & 0 deletions x-pack/plugins/fleet/server/services/agents/helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,4 +163,128 @@ describe('searchHitToAgent', () => {
},
});
});

it('should work when units is not present', () => {
const hit = {
_source: {
access_api_key_id: 'EH_RlIgBn_WkCEINY-qh',
active: true,
enrolled_at: '2023-06-07T07:45:30Z',
local_metadata: {
elastic: {
agent: {
'build.original':
'8.9.0-SNAPSHOT (build: 953fda060f317c2389ef6fd1cac8806a2bfe92ac at 2023-05-29 14:51:32 +0000 UTC)',
},
},
},
agent: {
id: '504b3006-52df-46a6-b7db-f3dc67aca7ac',
version: '8.9.0',
},
policy_id: '76c5b020-0486-11ee-97a3-c3856dd800f7',
type: 'PERMANENT',
outputs: {
'68233290-0486-11ee-97a3-c3856dd800f7': {
api_key: 'En_RlIgBn_WkCEINb-pQ:mfeV4ji6RNGyCOBs25gteg',
permissions_hash: '6ac9e595a2f8cba8893f9ea1fbfb6cba4b4d6f16d935c17a6368f11ee0b0a5d8',
type: 'elasticsearch',
api_key_id: 'En_RlIgBn_WkCEINb-pQ',
to_retire_api_key_ids: [],
},
},
policy_revision_idx: 2,
components: [
{
id: 'system/metrics-68233290-0486-11ee-97a3-c3856dd800f7',
type: 'system/metrics',
message: "Healthy: communicating with pid '36'",
status: 'HEALTHY',
},
],
last_checkin_message: 'Running',
last_checkin_status: 'online',
last_checkin: '2023-06-07T08:39:03Z',
unenrolled_at: '2023-06-07T07:45:30Z',
unenrollment_started_at: '2023-06-07T07:45:30Z',
upgraded_at: '2023-06-07T07:45:30Z',
upgrade_started_at: '2023-06-07T07:45:30Z',
default_api_key_id: 'EH_RlIgBn_WkCEINY-qh',
packages: ['system'],
tags: ['agent'],
user_provided_metadata: {
key: 'val',
},
default_api_key_history: [
{
retired_at: '',
},
],
},
sort: [1686123930000, 'beb13bf6a73e'],
fields: {
status: ['online'],
},
_id: '504b3006-52df-46a6-b7db-f3dc67aca7ac',
};
const agent = searchHitToAgent(hit as any);
expect(agent).toEqual({
id: '504b3006-52df-46a6-b7db-f3dc67aca7ac',
type: 'PERMANENT',
active: true,
enrolled_at: '2023-06-07T07:45:30Z',
access_api_key_id: 'EH_RlIgBn_WkCEINY-qh',
policy_id: '76c5b020-0486-11ee-97a3-c3856dd800f7',
last_checkin: '2023-06-07T08:39:03Z',
last_checkin_status: 'online',
last_checkin_message: 'Running',
policy_revision: 2,
sort: [1686123930000, 'beb13bf6a73e'],
outputs: {
'68233290-0486-11ee-97a3-c3856dd800f7': {
api_key_id: 'En_RlIgBn_WkCEINb-pQ',
type: 'elasticsearch',
to_retire_api_key_ids: [],
},
},
components: [
{
id: 'system/metrics-68233290-0486-11ee-97a3-c3856dd800f7',
type: 'system/metrics',
status: 'HEALTHY',
message: "Healthy: communicating with pid '36'",
units: undefined,
},
],
local_metadata: {
elastic: {
agent: {
'build.original':
'8.9.0-SNAPSHOT (build: 953fda060f317c2389ef6fd1cac8806a2bfe92ac at 2023-05-29 14:51:32 +0000 UTC)',
},
},
},
status: 'online',
unenrolled_at: '2023-06-07T07:45:30Z',
unenrollment_started_at: '2023-06-07T07:45:30Z',
upgraded_at: '2023-06-07T07:45:30Z',
upgrade_started_at: '2023-06-07T07:45:30Z',
default_api_key_id: 'EH_RlIgBn_WkCEINY-qh',
packages: ['system'],
tags: ['agent'],
user_provided_metadata: {
key: 'val',
},
default_api_key_history: [
{
id: undefined,
retired_at: '',
},
],
agent: {
id: '504b3006-52df-46a6-b7db-f3dc67aca7ac',
version: '8.9.0',
},
});
});
});
2 changes: 1 addition & 1 deletion x-pack/plugins/fleet/server/services/agents/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export function searchHitToAgent(
type: component.type,
status: component.status,
message: component.message,
units: component.units.map((unit) => ({
units: component.units?.map((unit) => ({
id: unit.id,
type: unit.type,
status: unit.status,
Expand Down

0 comments on commit 3c59c4d

Please sign in to comment.