Allow for LDAP server pool configuration instead of a single LDAP server

[nsballmann]

In the current configuration there is a single url: value with which a single LDAP server can be configured. For small setups this may be sufficient.

In bigger setups (in our case an AD) there are redundant LDAP servers (in our case multiple AD Controllers (ADCs) around the world with different latencies) available, to not have downtime in dependent applications (like devpi-server with devpi-ldap) while one of those ADCs is having its maintenance window (with downtime).

To my understanding at the moment I need to change the value for url: in ldap.yaml to switch to a different ADC/LDAP server in case my primarily configured server goes down or has maintenance.

In other applications there is the possibility to configure a Pool of LDAP servers with multiple URLs to avoid this problem. They keep a known-good list and query the first server in that list. If that server goes down it's taken off the list and the next one in the pool will be queried. It will be reintegrated into the list by different means, some add it to the list after some time, others check regularly the availability of the pools server and order them by response time.

To my understanding ldap3 is used for the LDAP connections and this package has the needed feature already built-in.

So please allow for configuration of multiple LDAP servers as a pool.

For backwards compatibility a single server configuration can be created as pool with a single server in it.

Sadly I don't have a configuration available which covers all the edge cases, but i think something like this can be made work:

---
devpi-ldap:
  pool:
    - url: ldaps://server1.example.com:636
      tls:
        validate: 2 # ssl.CERT_REQUIRED
        ca_certs_file: /etc/ssl/certs/ca-certificates.crt
    - url: ldap://server2.example.com:389
    - url: ldaps://server3.example.com:3269
      tls:
        validate: 2 # ssl.CERT_REQUIRED
        ca_certs_file: /etc/ssl/certs/ca-certificates.crt
    - url: ldap://server4.example.com:3268
  user_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=user)(sAMAccountName={username}))
    attribute_name: distinguishedName
  group_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=group)(member={userdn}))
    attribute_name: CN

[fschulze]

As this is an enterprise requirement I will either gladly accept a PR which implements this or you can contact mail@pyfidelity.com for a quote to implement it.

[nsballmann]

For backwards compatibility of the configuration this

---
devpi-ldap:
  url: ldaps://example.com:3269
  tls:
    validate: 2 # ssl.CERT_REQUIRED
    ca_certs_file: /etc/ssl/certs/ca-certificates.crt
  user_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=user)(sAMAccountName={username}))
    attribute_name: distinguishedName
  group_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=group)(member={userdn}))
    attribute_name: CN

can be mapped to this:

---
devpi-ldap:
  pool:
    - url: ldaps://example.com:3269
      tls:
        validate: 2 # ssl.CERT_REQUIRED
        ca_certs_file: /etc/ssl/certs/ca-certificates.crt
  user_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=user)(sAMAccountName={username}))
    attribute_name: distinguishedName
  group_search:
    base: CN=Partition1,DC=Example,DC=COM
    filter: (&(objectClass=group)(member={userdn}))
    attribute_name: CN

[nsballmann]

As this is an enterprise requirement I will either gladly accept a PR which implements this or you can contact mail@pyfidelity.com for a quote to implement it.

While i don't think that this is a pure enterprise requirement, I understand your approach on the subject and support it.

I will try to implement this and create a PR.

Just to clarify on the possibilities: Are you okay with restructuring the configuration like this? Do you have any other edge cases that may come along the way? Does the PR needs to have (mocked) tests included and if so, how detailed do they need to be? How much documentation need to be included?

[fschulze]

Thanks for trying to tackle this!

Are you okay with restructuring the configuration like this?

Yes, but the old one should still work. I'm fine if it is mapped to a pool internally. I'm not convinced we should deprecate the old way.

Do you have any other edge cases that may come along the way?

Can't think of any right now.

Does the PR needs to have (mocked) tests included and if so, how detailed do they need to be?

If you do the mapping, then it should be tested. It should also test whether the settings arrive correctly at the ldap library. Take a look at existing tests and feel free to ask more specific questions in a (draft) PR.

How much documentation need to be included?

It should mention the new feature and describe it enough to allow its use. It is fine to refer to the library docs for details. Again, roughly follow what exists.

[nsballmann]

The proposed code can be found in #51.

I slightly changed pool: to server_pool: for the name closer to its ldap3 counterpart. An old config is still working, but a new config takes precedence over the old one in case both are present. No mapping is needed, as I could make it work with "just a few touches".

Documentation is updated. Please let me know if anything else is needed.

I tested this manually (with the devpi-ldap script) against a bigger AD and it works as far as I have tested. For testing I made sure, our previous (single server) config worked. Then modified it to the new (server pool) one and made sure that worked as well. Then I broke the config at things I assumed will be the most common errors and made sure it can cope with a situation like this.

[nsballmann]

Do you already have plans on releasing this?

[fschulze]

It is released with devpi-ldap 2.1.0.