Icinga / icinga2

The core of our monitoring platform with a powerful configuration language and REST API.
https://icinga.com/docs/icinga2/latest
GNU General Public License v2.0
1.99k stars 570 forks source link

All API endpoints should return proper HTTP status codes #9535

Open julianbrost opened 1 year ago

julianbrost commented 1 year ago

Some API endpoints already have some logic to return helpful HTTP status codes, for example /v1/actions/...: https://github.com/Icinga/icinga2/blob/9be02e3f0486bd4c3aa9ecd55dda94febf5a1f79/lib/remote/actionshandler.cpp#L102-L130

However, many others don't and always return 200 OK, like for example POST to /v1/objects/...: https://github.com/Icinga/icinga2/blob/9be02e3f0486bd4c3aa9ecd55dda94febf5a1f79/lib/remote/modifyobjecthandler.cpp#L116

This leads to responses like the following when for example trying to set a non-existing attribute on some object:

$ curl -iskSu root:icinga -H 'Accept: application/json' -X POST 'https://localhost:5665/v1/objects/hosts/master-1' -d '{"attrs":{"does_not_exist": 42}, "pretty": true}'
HTTP/1.1 200 OK
Server: Icinga/v2.13.0-448-g9be02e3f0
Content-Type: application/json
Content-Length: 228

{
    "results": [
        {
            "code": 500,
            "name": "master-1",
            "status": "Attribute 'does_not_exist' could not be set: Error: Invalid field ID.\n",
            "type": "Host"
        }
    ]
}

git grep -F 'response.result(http::status::ok);' shows many more instances of this and especially for error handling in API clients, it would be great that if there was an error, something else than 200 OK is returned.

nr954 commented 7 months ago

I think Icinga supports multiple object modifications in a single request. Icinga then returns HTTP 200 and a set of results, each with its own status code.

This is not unreasonable, since its not obvious how to aggregate the results into one HTTP status code.

However I suggest 2 improvements:

  1. Return HTTP error-code, if any of the object-modifications fail. In this case the result set can still be inspected, to find out what exactly caused the error.
  2. If the request does not modify multiple objects but just one, the HTTP status code should match that of the single entry in the result set.