Only connect to LDAP server if group links exist for that provider
Zendesk: https://gitlab.zendesk.com/agent/tickets/90394
LDAP group sync optimizes by opening a single connection to each LDAP server at the beginning of the sync, and re-uses that connection for many queries. As a result of this early connection we don't even get a chance to see if there are any group links that we'll need to query the server for. In that regard, we're sometimes opening unnecessary connections. In most cases that's benign and no one cares. We recently encountered an issue with a customer where they added a new provider for some testing but it wasn't fully working. On sync, the connection to the secondary LDAP failed and caused the entire sync to fail.
What we we do?
We can probably add some quick queries and logic prior to the connection opening to determine if the connection is necessary. There are two obvious places where this is necessary, but more testing is needed.
In EE::Gitlab::LDAP::Sync::Group.execute_all_providers
and EE::Gitlab::LDAP::Sync:Groups.execute
we need to add some checks for several different things:
LdapGroupLink.where(provider: provider).any?
Gitlab::LDAP::Config.new(provider).admin_group.present?
!Gitlab::LDAP::Config.new(provider).external_groups.empty?
If all of the above are true, we should open a connection and proceed.
- How expensive is the
LdapGroupLink
query? I suspect not bad, but we should test with many LdapGroupLinks.
cc/ @dstanley