Seek alternative to Active Directory recursive filter search

Zendesk issue: https://gitlab.zendesk.com/agent/tickets/23086

The LDAP_MATCHING_RULE_IN_CHAIN filter we use to resolve AD subgroups can be really intense. We had a customer report that setting active_directory: false in configuration reduced sync time from 144 seconds to 2 seconds on a test machine. They reported the problem because in production the sync was taking 900 seconds and was pegging the AD server's CPU and triggering operations alerts.

Is there a more efficient way to get recursive membership?

cc/ @jacobvosmaer-gitlab

@dblessing does this customer need recursive search? If not then disabling it might be the best solution.

Theoretically we could reimplement recursive search by doing an LDAP lookup for each subgroup ourselves. (We may be able to distinguish LDAP group DN's from user DN's by looking if the DN ends with the group_base). If certain subgroups are repeated a lot they would only get queried once because GitLab caches group query results during a sync job. On the other hand this generates extra (non-recursive) LDAP queries.

@jacobvosmaer-gitlab Yes, I think they can disable it. That solves the problem for this customer, but we already saw another case where these recursive searches are taking more than 1 minute for fairly small nested groups. Nested groups seem to be quite common in AD. I think it's only a matter of time until we have to do something different.

Should we add an LDAP sync 'turbo' option? If turbo is off, run recursive query. If turbo is on, run multiple queries to resolve nested groups

Unless people commonly use byzantine group nesting in the real world, doing the recursive queries ourselves is probably reasonable. It is just more complexity on our end to implement that.

If we write that we do have to watch out for loops in the membership graph :)

Yep, that would be fun. Loop infinitely :P I'm inclined to put this off as long as possible. Not sure when we would be pressed to implement, though. And at that point it probably isn't an ideal situation.

I don't think it is that bad but it is another chunk of complexity we would start owning.

mentioned in merge request !422

mentioned in issue #666

Milestone changed to %8.12

Resolved in 8.12!

Status changed to closed