Skip to content

[7.x] Change DELETE to POST for _bulk_delete to avoid incompatibility issues (#87914)#88509

Merged
FrankHassanabad merged 1 commit intoelastic:7.xfrom
FrankHassanabad:backport/7.x/pr-87914
Jan 15, 2021
Merged

[7.x] Change DELETE to POST for _bulk_delete to avoid incompatibility issues (#87914)#88509
FrankHassanabad merged 1 commit intoelastic:7.xfrom
FrankHassanabad:backport/7.x/pr-87914

Conversation

@FrankHassanabad
Copy link
Copy Markdown
Contributor

Backports the following commits to 7.x:

elastic#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
@FrankHassanabad FrankHassanabad added the backport This PR is a backport of another PR label Jan 15, 2021
@kibanamachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 8.5MB 8.5MB -2.0B

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@FrankHassanabad FrankHassanabad merged commit fbeba3e into elastic:7.x Jan 15, 2021
@FrankHassanabad FrankHassanabad deleted the backport/7.x/pr-87914 branch January 15, 2021 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport This PR is a backport of another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants