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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Change OpenLDAP image to bitnami/openldap #3494

Conversation

polarathene
Copy link
Member

@polarathene polarathene commented Aug 21, 2023

TL;DR:

  • New image is actively maintained vs existing one that is over 5 years old.
  • Slight improvement to LDAP tree config via .ldif files.
  • No more Dockerfile required to build, we can just rely on docker run.

Changing to bitnami/openldap (supported registries: Azure and Dockerhub).


Full Details

Startup time for this new image is around 5 seconds? (The LDAP test uses a standard 20 second timeout check to wait until the server is ready before continuing with starting the DMS image).

The image osixia/openldap has not seen any activity since Feb 2021, while our Dockerfile was fixed to v1.1.6` (Feb 2018). It was originally contributed to DMS in in Oct 2016 with no real changes since then.

This PR migrates to bitnami/openldap which required modifying the 01_mail-tree.ldif to also include adding the root object to start successfully.

The user account .ldif files have minimal changes:

  • Lines moved around for better organization
  • Additional comments for context
  • Removal of inherited objectClass attributes (person, top) from the orgnizationalPerson class. Attribute sn changed to long form surname and values corrected with givenName. changetype: add was also not necessary.

Additionally, the image does not support the .schema format, they must be converted to .ldif. We need this for supporting the postfix-book.schema (supposedly the source of the file we carry, docs have mentioned it since May 2021) which allows the LDAP user account objects (defined in .ldif files) to use extra attributes provided by the schema.

  • I've converted the postfix-book.schema (as explained here) into postfix-book.ldif.
  • This schema isn't always an option for LDAP providers (specifically ActiveDirectory apparently?), and AFAIK isn't actually required to support LDAP with DMS, it just adds some convenience which our defaults assume it but can be configured differently as our LDAP docs describe.

Benefit: We no longer need to build an image with the LDIF files to use it for tests, just an initial pull. The bitnami/openldap image is actively maintained and one of the most popular OpenLDAP images on DockerHub.

Concerns:

  • The issue tracker isn't too great as it's mixed in with all other bitnami images, and VMware has since acquired bitnami which branched off a commercial offering.
  • Docs amount to the README which is roughly adequate but wasn't entirely straight-forward. I ran into problems initially trying to migrate to this image, and needed to learn about the ENV BITNAMI_DEBUG=true plus looking through the image source scripts to get a bet understanding.
  • Actual issues raised for the openldap image also don't tend to pan out too well / helpful which isn't too reassuring..
  • That said I'm not sure if there is any better maintained/documented/reputable options out there, at least for LDAP with OpenLDAP. It does get easier when you actually understand LDAP/OpenLDAP better in jargon, setup, and commands which took me a couple days.

Alternative: https://github.com/clayrisser/docker-openldap

  • I did initially have better success with migrating to an image variant based off of bitnami/openldap (notes on trying to migrate, details this alternative and bitnami/openldap).
  • Pro: It adds some extra improvements including bundling of postfix-book.schema so that we don't have to.
  • Con: It seems to differ with its Access Control List (ACL) policy. Requiring a separate .ldif migration file to allow LDAP queries to authenticate for the userPassword attribute.
  • Con: This variant is still fairly young, and I can't comment on what maintainer support is like. Other than submitting a detailed issue 5 days ago with no response. Presently that's the first issue opened on that repo, so it's difficult to gauge how long that would be actively maintained / supported vs upstream bitnami/openldap. For DMS LDAP tests at least, since I finally got bitnami/openldap to work, the value of this alternative image is low.
  • I have noticed that clayrisser is or was a user of DMS with LDAP 馃槑 (I came across a comment from them in 2020 on our issue tracker while looking through LDAP issues).

When troubleshooting:

  • More log output can be added with the ENV BITNAMI_DEBUG=true.
  • testsaslauthd will use /etc/saslauthd.conf, but is not affected by the omission of -r (thus realm / domain-part is not silently ignored by filter_query), careful not to be misled.
  • If testsaslauthd fails, it could be due to the /etc/saslauthd.conf, and the same applies for auth through Postfix when proxied through SASLAuthd daemon. Make sure your LDAP container is configured correctly by verifying a user can authenticate with their credentials (run in the openldap container, not DMS): ldapwhoami -v -x -H ldap://ldap.example.test -D 'userID=some.user,ou=users,dc=example,dc=test' -w secret

If switching to the alternative, these changes are needed:

  • Image is registry.gitlab.com/bitspur/rock8s/docker-openldap

  • The postfix-book.schema is already added in the image, but you need a migration for fixing ACL for auth. That belongs at /migrations dir, so volume mount: 'local/path/:/migrations/:ro' with a file (eg: auth.ldif) and the following content:

    dn: olcDatabase={2}mdb,cn=config
    changetype: modify
    add: olcAccess
    olcAccess: to attrs=userPassword
      by self write
      by dn="cn=admin,dc=example,dc=test" write
      by anonymous auth
      by * none
    

Type of change

  • Improvement (non-breaking change that does improve existing functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • New and existing unit tests pass locally with my changes

`osixia/openldap` has not seen any activity since Feb 2021, while our `Dockerfile` was fixed to v1.1.6` (Feb 2018).

This commit migrates to `bitnami/openldap` which required modifying the `01_mail-tree.ldif` to also include adding the root object to start successfully. Additionally the image does not support the `.schema` format, they must be converted to `.ldif` which has been done for `postfix-book.schema`.

The user account `.ldif` files have minimal changes.
- Lines moved around for better organization
- Additional comments for context
- Removal of inherited `objectClass` attributes (`person`, `top`) from the `orgnizationalPerson` class. Attribute `sn` changed to long form `surname` and values corrected with `givenName`. `changetype: add` was also not necessary.

We no longer need to build an image with the LDIF files to use it. This image is actively maintained and one of the most popular OpenLDAP images on DockerHub.

Startup time is around 5 seconds? The LDAP test uses a standard 20 second timeout check to wait until the server is ready before continuing with starting DMS.
@polarathene polarathene added area/tests service/ldap kind/improvement Improve an existing feature, configuration file or the documentation labels Aug 21, 2023
@polarathene polarathene added this to the v13.0.0 milestone Aug 21, 2023
@polarathene polarathene self-assigned this Aug 21, 2023
Copy link
Member

@georglauterbach georglauterbach left a comment

Choose a reason for hiding this comment

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

Awesome work! 馃殌

@polarathene polarathene merged commit 39ae101 into docker-mailserver:master Aug 22, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/tests kind/improvement Improve an existing feature, configuration file or the documentation service/ldap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants