Skip to content

Commit 4343c11

Browse files
[8.17](backport #5069) Include base error for json decode error responses (#5083)
* Include base error for json decode error responses (#5069) * Include base error for json decode error responses (cherry picked from commit 966c4f8) # Conflicts: # internal/pkg/api/handleEnroll_test.go * Fix backport * remove FIPS unit test --------- Co-authored-by: Michel Laterman <82832767+michel-laterman@users.noreply.github.com> Co-authored-by: michel-laterman <michel.laterman@elastic.co>
1 parent 2cbc202 commit 4343c11

File tree

6 files changed

+56
-14
lines changed

6 files changed

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

internal/pkg/api/error.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,11 @@ type BadRequestErr struct {
5252
}
5353

5454
func (e *BadRequestErr) Error() string {
55-
return fmt.Sprintf("Bad request: %s", e.msg)
55+
s := fmt.Sprintf("Bad request: %s", e.msg)
56+
if e.nextErr != nil {
57+
s += ": " + e.nextErr.Error()
58+
}
59+
return s
5660
}
5761

5862
func (e *BadRequestErr) Unwrap() error {

internal/pkg/api/handleAck_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ func TestValidateAckRequest(t *testing.T) {
776776
cfg: &config.Server{
777777
Limits: config.ServerLimits{},
778778
},
779-
expErr: &BadRequestErr{msg: "unable to decode ack request"},
779+
expErr: &BadRequestErr{msg: "unable to decode ack request", nextErr: errors.New("invalid character 'o' in literal null (expecting 'u')")},
780780
expAck: nil,
781781
},
782782
}

internal/pkg/api/handleAudit_test.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,22 @@ package api
66

77
import (
88
"context"
9+
"errors"
910
"io"
1011
"net/http"
1112
"net/http/httptest"
1213
"strings"
1314
"testing"
1415
"time"
1516

17+
"github.com/stretchr/testify/mock"
18+
"github.com/stretchr/testify/require"
19+
1620
"github.com/elastic/fleet-server/v7/internal/pkg/config"
1721
"github.com/elastic/fleet-server/v7/internal/pkg/dl"
1822
"github.com/elastic/fleet-server/v7/internal/pkg/model"
1923
ftesting "github.com/elastic/fleet-server/v7/internal/pkg/testing"
2024
testlog "github.com/elastic/fleet-server/v7/internal/pkg/testing/log"
21-
"github.com/stretchr/testify/mock"
22-
"github.com/stretchr/testify/require"
2325
)
2426

2527
func Test_Audit_validateUnenrollRequst(t *testing.T) {
@@ -47,7 +49,7 @@ func Test_Audit_validateUnenrollRequst(t *testing.T) {
4749
},
4850
cfg: &config.Server{},
4951
valid: nil,
50-
err: &BadRequestErr{msg: "unable to decode audit/unenroll request"},
52+
err: &BadRequestErr{msg: "unable to decode audit/unenroll request", nextErr: errors.New("invalid character '}' looking for beginning of value")},
5153
}, {
5254
name: "bad reason",
5355
req: &http.Request{
@@ -69,7 +71,7 @@ func Test_Audit_validateUnenrollRequst(t *testing.T) {
6971
},
7072
},
7173
valid: nil,
72-
err: &BadRequestErr{msg: "unable to decode audit/unenroll request"},
74+
err: &BadRequestErr{msg: "unable to decode audit/unenroll request", nextErr: errors.New("http: request body too large")},
7375
}}
7476

7577
for _, tc := range tests {

internal/pkg/api/handleCheckin_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"compress/flate"
1111
"context"
1212
"encoding/json"
13+
"errors"
1314
"io"
1415
"net/http"
1516
"net/http/httptest"
@@ -929,7 +930,7 @@ func TestValidateCheckinRequest(t *testing.T) {
929930
req: &http.Request{
930931
Body: io.NopCloser(strings.NewReader(`{"invalidJson":}`)),
931932
},
932-
expErr: &BadRequestErr{msg: "unable to decode checkin request"},
933+
expErr: &BadRequestErr{msg: "unable to decode checkin request", nextErr: errors.New("invalid character '}' looking for beginning of value")},
933934
cfg: &config.Server{
934935
Limits: config.ServerLimits{
935936
CheckinLimit: config.Limit{
@@ -959,7 +960,7 @@ func TestValidateCheckinRequest(t *testing.T) {
959960
req: &http.Request{
960961
Body: io.NopCloser(strings.NewReader(`{"validJson": "test", "status": "test", "poll_timeout": "not a timeout", "message": "test message"}`)),
961962
},
962-
expErr: &BadRequestErr{msg: "poll_timeout cannot be parsed as duration"},
963+
expErr: &BadRequestErr{msg: "poll_timeout cannot be parsed as duration", nextErr: errors.New("time: invalid duration \"not a timeout\"")},
963964
cfg: &config.Server{
964965
Limits: config.ServerLimits{
965966
CheckinLimit: config.Limit{

internal/pkg/api/handleEnroll_test.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@ import (
1313
"strings"
1414
"testing"
1515

16+
"github.com/rs/zerolog"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/mock"
19+
1620
"github.com/elastic/fleet-server/v7/internal/pkg/apikey"
1721
"github.com/elastic/fleet-server/v7/internal/pkg/bulk"
1822
"github.com/elastic/fleet-server/v7/internal/pkg/cache"
@@ -21,9 +25,6 @@ import (
2125
"github.com/elastic/fleet-server/v7/internal/pkg/model"
2226
"github.com/elastic/fleet-server/v7/internal/pkg/rollback"
2327
ftesting "github.com/elastic/fleet-server/v7/internal/pkg/testing"
24-
"github.com/rs/zerolog"
25-
"github.com/stretchr/testify/assert"
26-
"github.com/stretchr/testify/mock"
2728
)
2829

2930
func TestRemoveDuplicateStr(t *testing.T) {
@@ -250,7 +251,9 @@ func TestEnrollerT_retrieveStaticTokenEnrollmentToken(t *testing.T) {
250251
}
251252

252253
func TestValidateEnrollRequest(t *testing.T) {
253-
req, err := validateRequest(context.Background(), strings.NewReader("not a json"))
254-
assert.Equal(t, "Bad request: unable to decode enroll request", err.Error())
255-
assert.Nil(t, req)
254+
t.Run("invalid json", func(t *testing.T) {
255+
req, err := validateRequest(context.Background(), strings.NewReader("not a json"))
256+
assert.Equal(t, "Bad request: unable to decode enroll request: invalid character 'o' in literal null (expecting 'u')", err.Error())
257+
assert.Nil(t, req)
258+
})
256259
}

0 commit comments

Comments
 (0)