Dont send 500 for every call to /debug/errors endpoint (#19827)

relates to #15197

This updates the handler for `/debug/errors` so that we no longer always
send a 500 response.

- [x] Changes file added for user-visible changes in `changes/`,
`orbit/changes/` or `ee/fleetd-chrome/changes`.
See [Changes
files](https://fleetdm.com/docs/contributing/committing-changes#changes-files)
for more information.
- [ ] Added/updated tests
This commit is contained in:
Gabriel Hernandez 2024-06-21 13:12:06 +01:00 committed by GitHub
parent 477e46a96d
commit 8d064065d7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 20 additions and 28 deletions

View file

@ -0,0 +1 @@
- dont always send a 500 from the /debug/errors endpoint

View file

@ -84,8 +84,8 @@ type Service interface {
// to fleetd (formerly orbit).
GetOrbitConfig(ctx context.Context) (OrbitConfig, error)
// ReceiveFleetdError handles an erorr report from a `fleetd` component
ReceiveFleetdError(ctx context.Context, errData FleetdError) error
// LogFleetdError logs an error report from a `fleetd` component
LogFleetdError(ctx context.Context, errData FleetdError) error
// SetOrUpdateDeviceAuthToken creates or updates a device auth token for the given host.
SetOrUpdateDeviceAuthToken(ctx context.Context, authToken string) error

View file

@ -225,15 +225,7 @@ func (dc *DeviceClient) ReportError(token string, fleetdErr fleet.FleetdError) e
return retry.Do(
func() error {
err := dc.request(verb, path, token, "", req, nil)
scerr, ok := err.(*statusCodeErr)
// as backwards as this seems, this endpoint returns a
// `500` status code when we post an error (it might
// return `4xx` errors if the request is malformed or
// unauthenticated)
//
// see https://github.com/fleetdm/fleet/issues/13238#issuecomment-1671769460 for more details
if !ok || scerr.code != http.StatusInternalServerError {
if err != nil {
return err
}

View file

@ -434,29 +434,28 @@ type fleetdErrorResponse struct{}
func (r fleetdErrorResponse) error() error { return nil }
// for now, this endpoint must always return a 500 status code, this
// way errors are picked up and reported by any monitoring tool that
// looks for 5xx errors.
//
// since the handler is returning an error, this is redundant, but I'm adding
// it as a safeguard.
//
// See: https://github.com/fleetdm/fleet/issues/13238#issuecomment-1671769460
func (r fleetdErrorResponse) Status() int { return http.StatusInternalServerError }
func fleetdError(ctx context.Context, request interface{}, svc fleet.Service) (errorer, error) {
req := request.(*fleetdErrorRequest)
err := svc.ReceiveFleetdError(ctx, req.FleetdError)
// return the error as the second parameter to get better logs in the server.
return fleetdErrorResponse{}, err
err := svc.LogFleetdError(ctx, req.FleetdError)
if err != nil {
return nil, err
}
return fleetdErrorResponse{}, nil
}
func (svc *Service) ReceiveFleetdError(ctx context.Context, fleetdError fleet.FleetdError) error {
func (svc *Service) LogFleetdError(ctx context.Context, fleetdError fleet.FleetdError) error {
if !svc.authz.IsAuthenticatedWith(ctx, authz.AuthnDeviceToken) {
return ctxerr.Wrap(ctx, fleet.NewPermissionError("forbidden: only device-authenticated hosts can access this endpoint"))
}
return ctxerr.WrapWithData(ctx, fleetdError, "receive fleetd error", fleetdError.ToMap())
svc.logger.Log(
"msg",
"fleetd error",
"error",
ctxerr.WrapWithData(ctx, fleetdError, "receive fleetd error", fleetdError.ToMap()),
)
return nil
}
////////////////////////////////////////////////////////////////////////////////

View file

@ -304,7 +304,7 @@ func (s *integrationTestSuite) TestErrorReporting() {
res = s.DoRawNoAuth("POST", "/api/latest/fleet/device/"+token+"/debug/errors", jsonData, http.StatusBadRequest)
res.Body.Close()
res = s.DoRawNoAuth("POST", "/api/latest/fleet/device/"+token+"/debug/errors", []byte("{}"), http.StatusInternalServerError)
res = s.DoRawNoAuth("POST", "/api/latest/fleet/device/"+token+"/debug/errors", []byte("{}"), http.StatusOK)
res.Body.Close()
testTime, err := time.Parse(time.RFC3339, "1969-06-19T21:44:05Z")
@ -318,6 +318,6 @@ func (s *integrationTestSuite) TestErrorReporting() {
}
errBytes, err := json.Marshal(ferr)
require.NoError(t, err)
res = s.DoRawNoAuth("POST", "/api/latest/fleet/device/"+token+"/debug/errors", errBytes, http.StatusInternalServerError)
res = s.DoRawNoAuth("POST", "/api/latest/fleet/device/"+token+"/debug/errors", errBytes, http.StatusOK)
res.Body.Close()
}