Skip to content

Commit

Permalink
Change DELETE to POST for _bulk_delete to avoid incompatibility issues (
Browse files Browse the repository at this point in the history
#87914)

## Summary

Changes `DELETE` to `POST` for _bulk_delete on the client only for a variety of reasons.

According to the RFC, not all servers and proxies need to honor DELETE having a body. From: https://tools.ietf.org/html/rfc7231

```
A payload within a DELETE request message has no defined semantics;
sending a payload body on a DELETE request might cause some existing
implementations to reject the request.
```

Within at least one proxy, h2o2, we have found that it does indeed change request headers which will cause NodeJS to not attach the body of a `DELETE`:
hapijs/h2o2#124

Also from other communities such as OpenAPI where they debated this, they allow it but discourage it for reasons outlined there that I will not repeat here:
OAI/OpenAPI-Specification#1937

Elastic Search API's and other Kibana API's use `POST` rather than `DELETE` for their bodies that are attached to `DELETE`:
https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html

We still support bodies in `DELETE` and `POST` but are just changing the web client to utilize `POST` moving forward.


### Checklist

Reviewed and we already have unit tests and end to end tests for these use cases so we are good with just updating them. 

- [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
  • Loading branch information
FrankHassanabad authored Jan 15, 2021
1 parent 68288eb commit 2e28683
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ describe('Detections Rules API', () => {
await deleteRules({ ids: ['mySuperRuleId', 'mySuperRuleId_II'] });
expect(fetchMock).toHaveBeenCalledWith('/api/detection_engine/rules/_bulk_delete', {
body: '[{"id":"mySuperRuleId"},{"id":"mySuperRuleId_II"}]',
method: 'DELETE',
method: 'POST',
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ export const enableRules = async ({ ids, enabled }: EnableRulesProps): Promise<B
*/
export const deleteRules = async ({ ids }: DeleteRulesProps): Promise<BulkRuleResponse> =>
KibanaServices.get().http.fetch<Rule[]>(`${DETECTION_ENGINE_RULES_URL}/_bulk_delete`, {
method: 'DELETE',
method: 'POST',
body: JSON.stringify(ids.map((id) => ({ id }))),
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ curl -s -k \
-H 'Content-Type: application/json' \
-H 'kbn-xsrf: 123' \
-u ${ELASTICSEARCH_USERNAME}:${ELASTICSEARCH_PASSWORD} \
-X DELETE ${KIBANA_URL}${SPACE_URL}/api/detection_engine/rules/_bulk_delete \
-X POST ${KIBANA_URL}${SPACE_URL}/api/detection_engine/rules/_bulk_delete \
-d @${RULES} \
| jq .;

0 comments on commit 2e28683

Please sign in to comment.