Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing watcher to disable cookies in shared http client #97591

Merged

Conversation

masseyke
Copy link
Member

Watcher uses a single apache http client to make all outgoing connections. This client is shared by any user of the watcher API, as well as scheduled watches. Currently this client supports cookies. A cookie set for one user will be shared by all users. Watcher is meant to be used with stateless http requests, so cookies don't offer any advantage. But it could cause problems if they are shared across users.

@elasticsearchmachine
Copy link
Collaborator

Hi @masseyke, I've created a changelog YAML for you.

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, but I do also have the be that person who asks "Do we think this is a breaking change?"

@@ -135,6 +135,12 @@ private CloseableHttpClient createHttpClient() {
clientBuilder.evictExpiredConnections();
clientBuilder.setMaxConnPerRoute(MAX_CONNECTIONS);
clientBuilder.setMaxConnTotal(MAX_CONNECTIONS);
/*
* This client will potentially be used by multiple users. We do not want it to keep any state like cookies, because that will
* result in that state unexpectedlky being shared across all users.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in unexpectedlky

@masseyke masseyke merged commit 0027903 into elastic:main Aug 15, 2023
12 checks passed
@masseyke masseyke deleted the fix/watcher-puts-state-in-shared-connection branch August 15, 2023 13:32
JVerwolf pushed a commit to JVerwolf/elasticsearch that referenced this pull request Aug 15, 2023
csoulios pushed a commit to csoulios/elasticsearch that referenced this pull request Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants