Skip to content

Commit deb2017

Browse files
blakerousemergify[bot]
authored andcommitted
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) # Conflicts: # internal/pkg/api/handleCheckin.go
1 parent c3b01dc commit deb2017

File tree

5 files changed

+144
-49
lines changed

5 files changed

+144
-49
lines changed
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: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -959,10 +959,11 @@ func parseMeta(zlog zerolog.Logger, agent *model.Agent, req *CheckinRequest) ([]
959959
return nil, nil
960960
}
961961

962-
// Deserialize the agent's metadata copy
962+
// Deserialize the agent's metadata copy. If it fails, it's ignored as it will just
963+
// be replaced with the correct contents from the clients checkin.
963964
var agentLocalMeta interface{}
964965
if err := json.Unmarshal(agent.LocalMetadata, &agentLocalMeta); err != nil {
965-
return nil, fmt.Errorf("parseMeta local: %w", err)
966+
zlog.Warn().Err(err).Msg("local_metadata in document invalid; ignoring it")
966967
}
967968

968969
var outMeta []byte
@@ -999,14 +1000,13 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
9991000
return nil, &unhealthyReason, nil
10001001
}
10011002

1002-
agentComponentsJSON, err := json.Marshal(agent.Components)
1003-
if err != nil {
1004-
return nil, &unhealthyReason, fmt.Errorf("agent.Components marshal: %w", err)
1005-
}
1006-
10071003
// Quick comparison first; compare the JSON payloads.
10081004
// If the data is not consistently normalized, this short-circuit will not work.
1005+
<<<<<<< HEAD
10091006
if bytes.Equal(*req.Components, agentComponentsJSON) {
1007+
=======
1008+
if bytes.Equal(req.Components, agent.Components) {
1009+
>>>>>>> f9dbe41 (Handle malformatted JSON in `.fleet-agents` `components` field (#5858))
10101010
zlog.Trace().Msg("quick comparing agent components data is equal")
10111011
return nil, &unhealthyReason, nil
10121012
}
@@ -1024,14 +1024,26 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
10241024
return nil, &unhealthyReason, nil
10251025
}
10261026

1027+
// Deserialize the agent's components. If it fails, it's ignored as it will just
1028+
// be replaced with the correct contents from the clients checkin.
1029+
var agentComponents []model.ComponentsItems
1030+
if err := json.Unmarshal(agent.Components, &agentComponents); err != nil {
1031+
zlog.Warn().Err(err).Msg("components in document invalid; ignoring it")
1032+
}
1033+
10271034
var outComponents []byte
10281035

10291036
// Compare the deserialized meta structures and return the bytes to update if different
1037+
<<<<<<< HEAD
10301038
if !reflect.DeepEqual(reqComponents, agent.Components) {
10311039

10321040
reqComponentsJSON, _ := json.Marshal(*req.Components)
1041+
=======
1042+
if !reflect.DeepEqual(reqComponents, agentComponents) {
1043+
reqComponentsJSON, _ := json.Marshal(req.Components)
1044+
>>>>>>> f9dbe41 (Handle malformatted JSON in `.fleet-agents` `components` field (#5858))
10331045
zlog.Trace().
1034-
Str("oldComponents", string(agentComponentsJSON)).
1046+
Str("oldComponents", string(agent.Components)).
10351047
Str("req.Components", string(reqComponentsJSON)).
10361048
Msg("local components data is not equal")
10371049

internal/pkg/api/handleCheckin_test.go

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -967,12 +967,12 @@ func TestParseComponents(t *testing.T) {
967967
agent: &model.Agent{
968968
LastCheckinStatus: FailedStatus,
969969
UnhealthyReason: []string{"input"},
970-
Components: []model.ComponentsItems{{
970+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
971971
Status: "DEGRADED",
972972
Units: []model.UnitsItems{{
973973
Status: "DEGRADED", Type: "input",
974974
}},
975-
}},
975+
}}),
976976
},
977977
req: &CheckinRequest{
978978
Components: &degradedInputReqComponents,
@@ -986,12 +986,40 @@ func TestParseComponents(t *testing.T) {
986986
agent: &model.Agent{
987987
LastCheckinStatus: "online",
988988
UnhealthyReason: nil,
989-
Components: []model.ComponentsItems{{
989+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
990990
Status: "HEALTHY",
991991
Units: []model.UnitsItems{{
992992
Status: "HEALTHY", Type: "input",
993993
}},
994-
}},
994+
}}),
995+
},
996+
req: &CheckinRequest{
997+
Status: "DEGRADED",
998+
Components: degradedInputReqComponents,
999+
},
1000+
outComponents: degradedInputReqComponents,
1001+
unhealthyReason: &[]string{"input"},
1002+
err: nil,
1003+
}, {
1004+
name: "bad stored components",
1005+
agent: &model.Agent{
1006+
LastCheckinStatus: "online",
1007+
UnhealthyReason: nil,
1008+
Components: requireMarshalJSON(t, "string stored in components incorrectly"),
1009+
},
1010+
req: &CheckinRequest{
1011+
Status: "DEGRADED",
1012+
Components: degradedInputReqComponents,
1013+
},
1014+
outComponents: degradedInputReqComponents,
1015+
unhealthyReason: &[]string{"input"},
1016+
err: nil,
1017+
}, {
1018+
name: "invalid JSON ignored",
1019+
agent: &model.Agent{
1020+
LastCheckinStatus: "online",
1021+
UnhealthyReason: nil,
1022+
Components: json.RawMessage("{s"),
9951023
},
9961024
req: &CheckinRequest{
9971025
Status: "DEGRADED",
@@ -1012,6 +1040,12 @@ func TestParseComponents(t *testing.T) {
10121040
}
10131041
}
10141042

1043+
func requireMarshalJSON(t *testing.T, obj interface{}) json.RawMessage {
1044+
data, err := json.Marshal(obj)
1045+
require.NoError(t, err)
1046+
return data
1047+
}
1048+
10151049
func TestValidateCheckinRequest(t *testing.T) {
10161050
verCon := mustBuildConstraints("8.0.0")
10171051

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
@@ -464,6 +464,43 @@
464464
}
465465
},
466466

467+
"components_items": {
468+
"title": "Component Items",
469+
"description": "Elastic Agent component detailed status information",
470+
"type": "object",
471+
"properties": {
472+
"id": {
473+
"type": "string"
474+
},
475+
"status": {
476+
"type": "string"
477+
},
478+
"message": {
479+
"type": "string"
480+
},
481+
"units": {
482+
"type": "array",
483+
"items": {
484+
"type": "object",
485+
"properties": {
486+
"id": {
487+
"type": "string"
488+
},
489+
"type": {
490+
"type": "string"
491+
},
492+
"status": {
493+
"type": "string"
494+
},
495+
"message": {
496+
"type": "string"
497+
}
498+
}
499+
}
500+
}
501+
}
502+
},
503+
467504
"agent": {
468505
"title": "Agent",
469506
"description": "An Elastic Agent that has enrolled into Fleet",
@@ -610,41 +647,7 @@
610647
},
611648
"components": {
612649
"description": "Elastic Agent components detailed status information",
613-
"type": "array",
614-
"items": {
615-
"type": "object",
616-
"properties": {
617-
"id": {
618-
"type": "string"
619-
},
620-
"status": {
621-
"type": "string"
622-
},
623-
"message": {
624-
"type": "string"
625-
},
626-
"units": {
627-
"type": "array",
628-
"items": {
629-
"type": "object",
630-
"properties": {
631-
"id": {
632-
"type": "string"
633-
},
634-
"type": {
635-
"type": "string"
636-
},
637-
"status": {
638-
"type": "string"
639-
},
640-
"message": {
641-
"type": "string"
642-
}
643-
}
644-
}
645-
}
646-
}
647-
}
650+
"format": "raw"
648651
},
649652
"default_api_key_id": {
650653
"description": "Deprecated. Use Outputs instead. ID of the API key the Elastic Agent uses to authenticate with elasticsearch",

0 commit comments

Comments
 (0)