Skip to content

Commit 069f4b1

Browse files
blakerousemergify[bot]
blakerouse
authored and
mergify[bot]
committed
Handle malformatted JSON in .fleet-agents components field (#5858)
* Handle malformatted JSON in .fleet-agents components field. * Add changelog. * Add logs and update test to use require. (cherry picked from commit f9dbe41)
1 parent 433db3c commit 069f4b1

File tree

5 files changed

+137
-51
lines changed

5 files changed

+137
-51
lines changed

‎changelog/fragments/1762447389-fix-issue-where-malformed-components-field-prevents-agent-authentication.yaml‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
# REQUIRED
2+
# Kind can be one of:
3+
# - breaking-change: a change to previously-documented behavior
4+
# - deprecation: functionality that is being removed in a later release
5+
# - bug-fix: fixes a problem in a previous version
6+
# - enhancement: extends functionality but does not break or fix existing behavior
7+
# - feature: new functionality
8+
# - known-issue: problems that we are aware of in a given version
9+
# - security: impacts on the security of a product or a user’s deployment.
10+
# - upgrade: important information for someone upgrading from a prior version
11+
# - other: does not fit into any of the other categories
12+
kind: bug-fix
13+
14+
# REQUIRED for all kinds
15+
# Change summary; a 80ish characters long description of the change.
16+
summary: fix issue where malformed components field prevents agent authentication
17+
18+
# REQUIRED for breaking-change, deprecation, known-issue
19+
# Long description; in case the summary is not enough to describe the change
20+
# this field accommodate a description without length limits.
21+
# description:
22+
23+
# REQUIRED for breaking-change, deprecation, known-issue
24+
# impact:
25+
26+
# REQUIRED for breaking-change, deprecation, known-issue
27+
# action:
28+
29+
# REQUIRED for all kinds
30+
# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
31+
component: fleet-server
32+
33+
# AUTOMATED
34+
# OPTIONAL to manually add other PR URLs
35+
# PR URL: A link the PR that added the changeset.
36+
# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
37+
# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
38+
# Please provide it if you are adding a fragment for a different PR.
39+
# pr: https://github.com/owner/repo/1234
40+
41+
# AUTOMATED
42+
# OPTIONAL to manually add other issue URLs
43+
# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
44+
# If not present is automatically filled by the tooling with the issue linked to the PR number.
45+
# issue: https://github.com/owner/repo/1234

‎internal/pkg/api/handleCheckin.go‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,10 +1037,11 @@ func parseMeta(zlog zerolog.Logger, agent *model.Agent, req *CheckinRequest) ([]
10371037
return nil, nil
10381038
}
10391039

1040-
// Deserialize the agent's metadata copy
1040+
// Deserialize the agent's metadata copy. If it fails, it's ignored as it will just
1041+
// be replaced with the correct contents from the clients checkin.
10411042
var agentLocalMeta interface{}
10421043
if err := json.Unmarshal(agent.LocalMetadata, &agentLocalMeta); err != nil {
1043-
return nil, fmt.Errorf("parseMeta local: %w", err)
1044+
zlog.Warn().Err(err).Msg("local_metadata in document invalid; ignoring it")
10441045
}
10451046

10461047
var outMeta []byte
@@ -1077,14 +1078,9 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
10771078
return nil, &unhealthyReason, nil
10781079
}
10791080

1080-
agentComponentsJSON, err := json.Marshal(agent.Components)
1081-
if err != nil {
1082-
return nil, &unhealthyReason, fmt.Errorf("agent.Components marshal: %w", err)
1083-
}
1084-
10851081
// Quick comparison first; compare the JSON payloads.
10861082
// If the data is not consistently normalized, this short-circuit will not work.
1087-
if bytes.Equal(req.Components, agentComponentsJSON) {
1083+
if bytes.Equal(req.Components, agent.Components) {
10881084
zlog.Trace().Msg("quick comparing agent components data is equal")
10891085
return nil, &unhealthyReason, nil
10901086
}
@@ -1102,13 +1098,20 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
11021098
return nil, &unhealthyReason, nil
11031099
}
11041100

1101+
// Deserialize the agent's components. If it fails, it's ignored as it will just
1102+
// be replaced with the correct contents from the clients checkin.
1103+
var agentComponents []model.ComponentsItems
1104+
if err := json.Unmarshal(agent.Components, &agentComponents); err != nil {
1105+
zlog.Warn().Err(err).Msg("components in document invalid; ignoring it")
1106+
}
1107+
11051108
var outComponents []byte
11061109

11071110
// Compare the deserialized meta structures and return the bytes to update if different
1108-
if !reflect.DeepEqual(reqComponents, agent.Components) {
1111+
if !reflect.DeepEqual(reqComponents, agentComponents) {
11091112
reqComponentsJSON, _ := json.Marshal(req.Components)
11101113
zlog.Trace().
1111-
Str("oldComponents", string(agentComponentsJSON)).
1114+
Str("oldComponents", string(agent.Components)).
11121115
Str("req.Components", string(reqComponentsJSON)).
11131116
Msg("local components data is not equal")
11141117

‎internal/pkg/api/handleCheckin_test.go‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1001,12 +1001,12 @@ func TestParseComponents(t *testing.T) {
10011001
agent: &model.Agent{
10021002
LastCheckinStatus: FailedStatus,
10031003
UnhealthyReason: []string{"input"},
1004-
Components: []model.ComponentsItems{{
1004+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
10051005
Status: "DEGRADED",
10061006
Units: []model.UnitsItems{{
10071007
Status: "DEGRADED", Type: "input",
10081008
}},
1009-
}},
1009+
}}),
10101010
},
10111011
req: &CheckinRequest{
10121012
Components: degradedInputReqComponents,
@@ -1020,12 +1020,40 @@ func TestParseComponents(t *testing.T) {
10201020
agent: &model.Agent{
10211021
LastCheckinStatus: "online",
10221022
UnhealthyReason: nil,
1023-
Components: []model.ComponentsItems{{
1023+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
10241024
Status: "HEALTHY",
10251025
Units: []model.UnitsItems{{
10261026
Status: "HEALTHY", Type: "input",
10271027
}},
1028-
}},
1028+
}}),
1029+
},
1030+
req: &CheckinRequest{
1031+
Status: "DEGRADED",
1032+
Components: degradedInputReqComponents,
1033+
},
1034+
outComponents: degradedInputReqComponents,
1035+
unhealthyReason: &[]string{"input"},
1036+
err: nil,
1037+
}, {
1038+
name: "bad stored components",
1039+
agent: &model.Agent{
1040+
LastCheckinStatus: "online",
1041+
UnhealthyReason: nil,
1042+
Components: requireMarshalJSON(t, "string stored in components incorrectly"),
1043+
},
1044+
req: &CheckinRequest{
1045+
Status: "DEGRADED",
1046+
Components: degradedInputReqComponents,
1047+
},
1048+
outComponents: degradedInputReqComponents,
1049+
unhealthyReason: &[]string{"input"},
1050+
err: nil,
1051+
}, {
1052+
name: "invalid JSON ignored",
1053+
agent: &model.Agent{
1054+
LastCheckinStatus: "online",
1055+
UnhealthyReason: nil,
1056+
Components: json.RawMessage("{s"),
10291057
},
10301058
req: &CheckinRequest{
10311059
Status: "DEGRADED",
@@ -1046,6 +1074,12 @@ func TestParseComponents(t *testing.T) {
10461074
}
10471075
}
10481076

1077+
func requireMarshalJSON(t *testing.T, obj interface{}) json.RawMessage {
1078+
data, err := json.Marshal(obj)
1079+
require.NoError(t, err)
1080+
return data
1081+
}
1082+
10491083
func TestValidateCheckinRequest(t *testing.T) {
10501084
verCon := mustBuildConstraints("8.0.0")
10511085

‎internal/pkg/model/schema.go‎

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎model/schema.json‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,43 @@
477477
}
478478
},
479479

480+
"components_items": {
481+
"title": "Component Items",
482+
"description": "Elastic Agent component detailed status information",
483+
"type": "object",
484+
"properties": {
485+
"id": {
486+
"type": "string"
487+
},
488+
"status": {
489+
"type": "string"
490+
},
491+
"message": {
492+
"type": "string"
493+
},
494+
"units": {
495+
"type": "array",
496+
"items": {
497+
"type": "object",
498+
"properties": {
499+
"id": {
500+
"type": "string"
501+
},
502+
"type": {
503+
"type": "string"
504+
},
505+
"status": {
506+
"type": "string"
507+
},
508+
"message": {
509+
"type": "string"
510+
}
511+
}
512+
}
513+
}
514+
}
515+
},
516+
480517
"agent": {
481518
"title": "Agent",
482519
"description": "An Elastic Agent that has enrolled into Fleet",
@@ -628,41 +665,7 @@
628665
},
629666
"components": {
630667
"description": "Elastic Agent components detailed status information",
631-
"type": "array",
632-
"items": {
633-
"type": "object",
634-
"properties": {
635-
"id": {
636-
"type": "string"
637-
},
638-
"status": {
639-
"type": "string"
640-
},
641-
"message": {
642-
"type": "string"
643-
},
644-
"units": {
645-
"type": "array",
646-
"items": {
647-
"type": "object",
648-
"properties": {
649-
"id": {
650-
"type": "string"
651-
},
652-
"type": {
653-
"type": "string"
654-
},
655-
"status": {
656-
"type": "string"
657-
},
658-
"message": {
659-
"type": "string"
660-
}
661-
}
662-
}
663-
}
664-
}
665-
}
668+
"format": "raw"
666669
},
667670
"default_api_key_id": {
668671
"description": "Deprecated. Use Outputs instead. ID of the API key the Elastic Agent uses to authenticate with elasticsearch",

0 commit comments

Comments
 (0)