Deprecate timeout.tcp_read AD/LDAP realm setting #47305
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
albertzaharovits
added
>deprecation
:Security/Authentication
Collaborator
|
Pinging @elastic/es-security |
Co-Authored-By: Ioannis Kakavas <[email protected]>
Co-Authored-By: Ioannis Kakavas <[email protected]>
Contributor
Author
|
@elasticmachine update branch |
Contributor
Author
|
Note, I plan to follow-up with the deprecation check and the breaking change PRs when this is merged and backported. |
tvernum
reviewed
Oct 7, 2019
|
|
||
| [float] | ||
| [[ldap-ad-realms-tcp-read-timeout-removed]] | ||
| ==== The `timeout.tcp_read` AD and LDAP realm settings have been removed |
Contributor
There was a problem hiding this comment.
I'm confused by the removed here. This PR doesn't remove them - I think this whole docs piece should be held over to the removal PR.
Contributor
Author
There was a problem hiding this comment.
Agreed. I'll hold this off for the actual removal PR.
| connection. This is equivalent to and is deprecated in favor of | ||
| `timeout.response` and they cannot be used simultaneously. An `s` at the end | ||
| indicates seconds, or `ms` indicates milliseconds. Defaults to the value of | ||
| `timeout.ldap_search`. |
Contributor
There was a problem hiding this comment.
I'd propose that we remove the "Defaults to ..." from here, since it is the deprecated setting, it doesn't really have a default.
Contributor
Author
There was a problem hiding this comment.
Good point! If you one doesn't specify the deprecated settings, it's the new settings that takes a default value, the old one is just deprecated.
Contributor
Author
|
@tvernum I have acted on your objections, please take another look. |
tvernum
approved these changes
Dec 5, 2019
Contributor
Author
|
@elasticmachine update branch |
…lastic#51492) Changes the find_file_structure response to include a CSV ingest processor in the ingest pipeline it suggests. Previously the Kibana file upload functionality parsed CSV in the browser, but by parsing CSV in the ingest pipeline it makes the Kibana file upload functionality more easily interchangable with Filebeat such that the configurations it creates can more easily be used to import data with the same structure repeatedly in production.
We no longer issue new sync_ids in 8.0, but we still need to make sure that the replica allocator prefers copies with matching sync_id. This commit adds tests for that. Relates elastic#50776
Previous the formatter was breaking simple if/else statements (i.e. without braces) onto separate lines, which could be fragile because the formatter cannot also introduce braces. Instead, keep such expressions on the same line.
Contributor
Author
|
Aaaaargh, apologies for the bogus commit message! |
albertzaharovits
added a commit
that referenced
this pull request
Jan 29, 2020
The timeout.tcp_read AD/LDAP realm setting, despite the low-level allusion, controls the time interval the realms wait for a response for a query (search or bind). If the connection to the server is synchronous (un-pooled) the response timeout is analogous to the tcp read timeout. But the tcp read timeout is irrelevant in the common case of a pooled connection (when a Bind DN is specified). The timeout.tcp_read qualifier is hereby deprecated in favor of timeout.response. In addition, the default value for both timeout.tcp_read and timeout.response is that of timeout.ldap_search, instead of the 5s (but the default for timeout.ldap_search is still 5s). The timeout.ldap_search defines the server-controlled timeout of a search request. There is no practical use case to have a smaller tcp_read timeout compared to ldap_search (in this case the request would time-out on the client but continue to be processed on the server). The proposed change aims to simplify configuration so that the more common configuration change, adjusting timeout.ldap_search up, has the expected result (no timeout during searches) without any additional modifications. Closes #46028
38 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
10 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
timeout.tcp_readAD/LDAP realm setting, despite the low-level allusion, controls the time interval the realms wait for a response for a query (search or bind). If the connection to the server is synchronous (un-pooled) the response timeout is analogous to the tcp read timeout. But the tcp read timeout is irrelevant in the common case of a pooled connection (when a Bind DN is specified).The
timeout.tcp_readqualifier is hereby deprecated in favor oftimeout.response.In addition, the default value for both
timeout.tcp_readandtimeout.responseis that oftimeout.ldap_search, instead of the5s(but the default fortimeout.ldap_searchis still5s). Thetimeout.ldap_searchdefines the server-controlled timeout of a search request. There is no practical use case to have a smallertcp_readtimeout compared toldap_search(in this case the request would time-out on the client but continue to be processed on the server). The proposed change aims to simplify configuration so that the more common configuration change, adjustingtimeout.ldap_searchup, has the expected result (no timeout during searches) without any additional modifications.Closes #46028