Skip to content

Commit 461cd09

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) # Conflicts: # internal/pkg/api/handleCheckin.go
1 parent 588594f commit 461cd09

File tree

5 files changed

+144
-49
lines changed

5 files changed

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

953-
// Deserialize the agent's metadata copy
953+
// Deserialize the agent's metadata copy. If it fails, it's ignored as it will just
954+
// be replaced with the correct contents from the clients checkin.
954955
var agentLocalMeta interface{}
955956
if err := json.Unmarshal(agent.LocalMetadata, &agentLocalMeta); err != nil {
956-
return nil, fmt.Errorf("parseMeta local: %w", err)
957+
zlog.Warn().Err(err).Msg("local_metadata in document invalid; ignoring it")
957958
}
958959

959960
var outMeta []byte
@@ -990,14 +991,13 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
990991
return nil, &unhealthyReason, nil
991992
}
992993

993-
agentComponentsJSON, err := json.Marshal(agent.Components)
994-
if err != nil {
995-
return nil, &unhealthyReason, fmt.Errorf("agent.Components marshal: %w", err)
996-
}
997-
998994
// Quick comparison first; compare the JSON payloads.
999995
// If the data is not consistently normalized, this short-circuit will not work.
996+
<<<<<<< HEAD
1000997
if bytes.Equal(*req.Components, agentComponentsJSON) {
998+
=======
999+
if bytes.Equal(req.Components, agent.Components) {
1000+
>>>>>>> f9dbe41 (Handle malformatted JSON in `.fleet-agents` `components` field (#5858))
10011001
zlog.Trace().Msg("quick comparing agent components data is equal")
10021002
return nil, &unhealthyReason, nil
10031003
}
@@ -1015,14 +1015,26 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques
10151015
return nil, &unhealthyReason, nil
10161016
}
10171017

1018+
// Deserialize the agent's components. If it fails, it's ignored as it will just
1019+
// be replaced with the correct contents from the clients checkin.
1020+
var agentComponents []model.ComponentsItems
1021+
if err := json.Unmarshal(agent.Components, &agentComponents); err != nil {
1022+
zlog.Warn().Err(err).Msg("components in document invalid; ignoring it")
1023+
}
1024+
10181025
var outComponents []byte
10191026

10201027
// Compare the deserialized meta structures and return the bytes to update if different
1028+
<<<<<<< HEAD
10211029
if !reflect.DeepEqual(reqComponents, agent.Components) {
10221030

10231031
reqComponentsJSON, _ := json.Marshal(*req.Components)
1032+
=======
1033+
if !reflect.DeepEqual(reqComponents, agentComponents) {
1034+
reqComponentsJSON, _ := json.Marshal(req.Components)
1035+
>>>>>>> f9dbe41 (Handle malformatted JSON in `.fleet-agents` `components` field (#5858))
10241036
zlog.Trace().
1025-
Str("oldComponents", string(agentComponentsJSON)).
1037+
Str("oldComponents", string(agent.Components)).
10261038
Str("req.Components", string(reqComponentsJSON)).
10271039
Msg("local components data is not equal")
10281040

‎internal/pkg/api/handleCheckin_test.go‎

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -939,12 +939,12 @@ func TestParseComponents(t *testing.T) {
939939
agent: &model.Agent{
940940
LastCheckinStatus: FailedStatus,
941941
UnhealthyReason: []string{"input"},
942-
Components: []model.ComponentsItems{{
942+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
943943
Status: "DEGRADED",
944944
Units: []model.UnitsItems{{
945945
Status: "DEGRADED", Type: "input",
946946
}},
947-
}},
947+
}}),
948948
},
949949
req: &CheckinRequest{
950950
Components: &degradedInputReqComponents,
@@ -958,12 +958,40 @@ func TestParseComponents(t *testing.T) {
958958
agent: &model.Agent{
959959
LastCheckinStatus: "online",
960960
UnhealthyReason: nil,
961-
Components: []model.ComponentsItems{{
961+
Components: requireMarshalJSON(t, []model.ComponentsItems{{
962962
Status: "HEALTHY",
963963
Units: []model.UnitsItems{{
964964
Status: "HEALTHY", Type: "input",
965965
}},
966-
}},
966+
}}),
967+
},
968+
req: &CheckinRequest{
969+
Status: "DEGRADED",
970+
Components: degradedInputReqComponents,
971+
},
972+
outComponents: degradedInputReqComponents,
973+
unhealthyReason: &[]string{"input"},
974+
err: nil,
975+
}, {
976+
name: "bad stored components",
977+
agent: &model.Agent{
978+
LastCheckinStatus: "online",
979+
UnhealthyReason: nil,
980+
Components: requireMarshalJSON(t, "string stored in components incorrectly"),
981+
},
982+
req: &CheckinRequest{
983+
Status: "DEGRADED",
984+
Components: degradedInputReqComponents,
985+
},
986+
outComponents: degradedInputReqComponents,
987+
unhealthyReason: &[]string{"input"},
988+
err: nil,
989+
}, {
990+
name: "invalid JSON ignored",
991+
agent: &model.Agent{
992+
LastCheckinStatus: "online",
993+
UnhealthyReason: nil,
994+
Components: json.RawMessage("{s"),
967995
},
968996
req: &CheckinRequest{
969997
Status: "DEGRADED",
@@ -984,6 +1012,12 @@ func TestParseComponents(t *testing.T) {
9841012
}
9851013
}
9861014

1015+
func requireMarshalJSON(t *testing.T, obj interface{}) json.RawMessage {
1016+
data, err := json.Marshal(obj)
1017+
require.NoError(t, err)
1018+
return data
1019+
}
1020+
9871021
func TestValidateCheckinRequest(t *testing.T) {
9881022
verCon := mustBuildConstraints("8.0.0")
9891023

‎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)