bcuff / elasticsearch-net-aws

Add-on to Elasticsearch.Net & NEST for using AWS's elasticsearch service.
Apache License 2.0
72 stars 27 forks source link

Allow user to toggle Connection header #25

Closed troylar closed 6 years ago

troylar commented 6 years ago

Our requests were failing because the ElasticSearch library creates a Connection:Keep-Alive header by default. Amazon expects the request to be signed with the header Connection:close. Overriding this header allowed us to successfully query the AWS ES cluster.

This pull request simply allows you to switch between "Keep-Alive" and "close."

bcuff commented 6 years ago

Any chance you're going through a proxy that might be messing with the HTTP? We are successfully connecting directly to an AWS Elasticsearch service cluster using https:// and have never encountered this. Also using Keep Alive is important for performance reasons.

troylar commented 6 years ago

Interesting. Two of us are getting the same response in two different states on two completely different networks on two completely different clusters.

On Aug 10, 2017 5:21 PM, "Brandon" notifications@github.com wrote:

Any chance you're going through a proxy that might be messing with the HTTP? We are successfully connecting directly to an AWS Elasticsearch service cluster using https:// and have never encountered this. Also using Keep Alive is important for performance reasons.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/bcuff/elasticsearch-net-aws/pull/25#issuecomment-321676834, or mute the thread https://github.com/notifications/unsubscribe-auth/AB151FW0FSKgC6UbsPv7ubG5mcjyRRiBks5sW3RYgaJpZM4Oz_Hg .

bcuff commented 6 years ago

You can try omitting the connection header from the signature. I don't think it needs to be part of the signature. See https://github.com/bcuff/elasticsearch-net-aws/commit/f3f3a5252df675e60c7a0712ec4ad99c7eea33db

If that fixes the issue let me know and we can include that change. I would hate to turn off keep-alive by default.

troylar commented 6 years ago

100% agree with you and we can enable it by default. I don't like this solution long term because it doesn't really address the "why" but it does work for now.

troylar commented 6 years ago

So removing the Connection header worked as well.

bcuff commented 6 years ago

Cool. I'll merge that commit and publish tomorrow morning. Going to close this PR. thanks for reporting the issue.

troylar commented 6 years ago

@bcuff FYI -- this where the keep-alive is being set in the Elasticsearch-net code . . .It only happens for the DOTNETCORE directive. (Line 225)

https://github.com/elastic/elasticsearch-net/blob/70571f15a8fcd5b6397b9aa9c3a9c81d37014b51/src/Elasticsearch.Net/Connection/HttpConnection-CoreFx.cs

troylar commented 6 years ago

And where the addition was made two months ago. Just wanted to give a full context. Thanks for making this change.

https://github.com/elastic/elasticsearch-net/commit/0e66e5b3442c02b33ffc30f35e4a9b809a0f07c2

bcuff commented 6 years ago

Ah, that makes sense. New release is up https://www.nuget.org/packages/Elasticsearch.Net.Aws/2.3.5 @troylar