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

Backport of sdk/ldaputil: add connection_timeout configurable into release/1.13.x #20148

Merged

Conversation

hc-github-team-secure-vault-core
Copy link
Contributor

Backport

This PR is auto-generated from #20144 to be assessed for backporting due to the inclusion of the label backport/1.13.x.

WARNING automatic cherry-pick of commits failed. Commits will require human attention.

merge conflict error: POST https://api.github.com/repos/hashicorp/vault/merges: 409 Merge conflict []

The below text is copied from the body of the original PR.


The go-ldap package sets a 60 second timeout in their package for dial attempts which aligns with Vault's 60 second client timeout. Since these timeouts are the same if the first value in our LDAP config url is unavailable, no other URLs in the list will be tried and all connections will fail.

This adds a new LDAP config connection_timeout which allows operators to override this timeout value. By setting this value lower you can leverage the backup URLs in the config.

This is not the same as the existing config request_timeout which tunes timeout values after a connection is made to LDAP.

A workaround exists where clients can set VAULT_CLIENT_TIMEOUT. By adding this config though we can set timeouts for all user connections and may be a better overall experience.


Overview of commits

@hashicorp-cla
Copy link

hashicorp-cla commented Apr 13, 2023

CLA assistant check
All committers have signed the CLA.

* sdk/ldaputil: add connection_timeout configurable

* changelog

* Update doc

* Fix test

* Change default to 30s
@jasonodonnell jasonodonnell force-pushed the backport/ldap-dial-timeout/hopefully-proper-mouse branch from 6e75042 to 146e6bd Compare April 13, 2023 18:08
@jasonodonnell jasonodonnell marked this pull request as ready for review April 13, 2023 19:49
@jasonodonnell jasonodonnell requested a review from a team April 13, 2023 19:49
@jasonodonnell jasonodonnell enabled auto-merge (squash) April 13, 2023 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants