sdap: defer libldap global options setup to first connection#8709
Open
alexey-tikhonov wants to merge 1 commit into
Open
sdap: defer libldap global options setup to first connection#8709alexey-tikhonov wants to merge 1 commit into
alexey-tikhonov wants to merge 1 commit into
Conversation
alexey-tikhonov
added
no-backport
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the initialization of global LDAP options by consolidating calls to setup_ldap_debug and setup_tls_config into a single function, sdap_setup_libldap_global_options, which is now called once during the connection process. This change affects the AD, IPA, and LDAP providers. Feedback suggests adding a NULL check for the basic_opts parameter in the new function to improve robustness and prevent potential null pointer dereferences.
During initialization LDAP/AD/IPA backends unconditionally call `setup_tls_config()` and `setup_ldap_debug()` that call `ldap_set_option()`. This triggers `ldap_int_initialize()` -> `getaddrinfo(local_hostname)`. If DNS is unresponsive, this blocks and the backend doesn't complete initialization in time, so that 'monitor' terminates the entire SSSD. Move these calls out of the module init path into a new `sdap_setup_libldap_global_options()` wrapper guarded by a static bool. Call it from `sdap_connect_send()` just before `sss_ldap_init_send()`, which is the single entry point for all LDAP connections. :fixes:Fixed an issue where SSSD fails to start when DNS is unresponsive. Assisted-By: Claude Code (Opus 4.6)
alexey-tikhonov
force-pushed
the
openldap-init
branch
from
166b457 to
7ac8608
Compare
alexey-tikhonov
added
coverity
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
During initialization LDAP/AD/IPA backends unconditionally call
setup_tls_config()andsetup_ldap_debug()that callldap_set_option(). This triggersldap_int_initialize()->getaddrinfo(local_hostname). If DNS is unresponsive, this blocks and the backend doesn't complete initialization in time, so that 'monitor' terminates the entire SSSD.Move these calls out of the module init path into a new
sdap_setup_libldap_global_options()wrapper guarded by a static bool. Call it fromsdap_connect_send()just beforesss_ldap_init_send(), which is the single entry point for all LDAP connections.:fixes:Fixed an issue where SSSD fails to start when DNS is unresponsive.
Assisted-By: Claude Code (Opus 4.6)