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

Improve Teleport's ability to reconnect to LDAP #36281

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Jan 4, 2024

There are two separate changes here, broken into two separate commits for easier review.

changelog: ensure that Teleport can re-establish broken LDAP connections.

lib/srv/desktop/windows_server.go Show resolved Hide resolved
lib/srv/desktop/windows_server.go Outdated Show resolved Hide resolved
lib/srv/desktop/windows_server.go Outdated Show resolved Hide resolved
@zmb3
Copy link
Collaborator Author

zmb3 commented Feb 16, 2024

@ibeckermayer this one's ready for another look

Copy link
Member

@ravicious ravicious left a comment

Choose a reason for hiding this comment

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

bot

zmb3 added 2 commits October 28, 2024 08:38
If Teleport loses it's connection to the LDAP server, it will
attempt to initiate a new condition when:

1. The user tries to connect to a desktop and Teleport fails
   to obtain the user's SID.
2. The periodic desktop discovery routine attempts to search
   LDAP for desktops.

In some circumstances, #2 never gets the chance to apply, since
discovery is skipped when LDAP is not ready. Additionally, if
LDAP is not ready, then you can't connect to a desktop, so #1
can't happen either, which means Teleport won't connect again
until it is restarted.
If LDAP-based discovery is not enabled then we may go long periods
of time without trying to use the LDAP connection, which prevents
us from detecting disconnects (and restoring the connection) in a
timely manner.

When discovery is disabled, perform a read every 5 minutes and
reconnect if we detect a connection problem.
@zmb3 zmb3 force-pushed the zmb3/desktop-ldap-reconnect branch from b7147a1 to 488df9a Compare October 28, 2024 14:39
@zmb3 zmb3 force-pushed the zmb3/desktop-ldap-reconnect branch from 488df9a to 785de03 Compare October 28, 2024 14:46
@zmb3 zmb3 added this pull request to the merge queue Oct 28, 2024
Merged via the queue into master with commit 239723a Oct 28, 2024
39 checks passed
@zmb3 zmb3 deleted the zmb3/desktop-ldap-reconnect branch October 28, 2024 15:27
@public-teleport-github-review-bot

@zmb3 See the table below for backport results.

Branch Result
branch/v14 Failed
branch/v15 Failed
branch/v16 Create PR
branch/v17 Create PR

zmb3 added a commit that referenced this pull request Oct 28, 2024
In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
zmb3 added a commit that referenced this pull request Oct 28, 2024
In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
zmb3 added a commit that referenced this pull request Oct 28, 2024
In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
github-merge-queue bot pushed a commit that referenced this pull request Oct 28, 2024
In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
github-merge-queue bot pushed a commit that referenced this pull request Oct 29, 2024
* Always attempt desktop discovery, even if LDAP is not ready

If Teleport loses it's connection to the LDAP server, it will
attempt to initiate a new condition when:

1. The user tries to connect to a desktop and Teleport fails
   to obtain the user's SID.
2. The periodic desktop discovery routine attempts to search
   LDAP for desktops.

In some circumstances, #2 never gets the chance to apply, since
discovery is skipped when LDAP is not ready. Additionally, if
LDAP is not ready, then you can't connect to a desktop, so #1
can't happen either, which means Teleport won't connect again
until it is restarted.

* Periodically use the LDAP connection when discovery is not enabled

If LDAP-based discovery is not enabled then we may go long periods
of time without trying to use the LDAP connection, which prevents
us from detecting disconnects (and restoring the connection) in a
timely manner.

When discovery is disabled, perform a read every 5 minutes and
reconnect if we detect a connection problem.

* Address review comments

* Fix some LDAP connection bugs

In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.
github-merge-queue bot pushed a commit that referenced this pull request Nov 18, 2024
* Always attempt desktop discovery, even if LDAP is not ready

If Teleport loses it's connection to the LDAP server, it will
attempt to initiate a new condition when:

1. The user tries to connect to a desktop and Teleport fails
   to obtain the user's SID.
2. The periodic desktop discovery routine attempts to search
   LDAP for desktops.

In some circumstances, #2 never gets the chance to apply, since
discovery is skipped when LDAP is not ready. Additionally, if
LDAP is not ready, then you can't connect to a desktop, so #1
can't happen either, which means Teleport won't connect again
until it is restarted.

* Periodically use the LDAP connection when discovery is not enabled

If LDAP-based discovery is not enabled then we may go long periods
of time without trying to use the LDAP connection, which prevents
us from detecting disconnects (and restoring the connection) in a
timely manner.

When discovery is disabled, perform a read every 5 minutes and
reconnect if we detect a connection problem.

* Address review comments

* Fix some LDAP connection bugs

In #36281 we made some improvements to the LDAP reconnect behavior.
These changes considered the case where we had a connection to the
LDAP server but then got disconnected. They did not consider the case
where we never succesfully established a connection at all.

* Fix typo

---------

Co-authored-by: Gus Luxton <[email protected]>
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.

4 participants