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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create olcDbDirectory before its database and the start of slapd #428

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gcoxmoz
Copy link
Contributor

@gcoxmoz gcoxmoz commented Jul 3, 2024

Rehash of #397

Pull Request (PR) description

There is an ordering in manifests/server/database.pp:

  Class['openldap::server::service']
  -> Openldap::Server::Database[$title]

This is subtly bad. The service (slapd) must be spun up before a database can be created, and that makes sense. However, it means the service happens before the whole defined resource Openldap::Server::Database ... and there is more going on in the defined resource of manifests/server/database.pp than just the openldap_database creation: there is also the creation of File[$manage_directory]. In most folks' cases, using a vendor-made package, this directory will be something like /var/lib/ldap, which happens to be installed by the RPM/dpkg package, so "you get it for free" / it already exists. Thus the file creation doesn't need to be done by puppet and ordering doesn't matter. However, if you set the directory to something else (that doesn't exist), you have a circular dependency problem. slapd (the service) needs the database's directory to exist before slapd starts up -> slapd is ordered before the database manifest -> the database manifest creates the database directory -> the database directory has to happen before the service.

Ultimately, the ordering is in error. The service has to happen before openldap_database BUT NOT all of the ridealong items in openldap::server::database. That breaks out of the dependency loop, and allows the directory creation to be marked as required before the Service is started.

Very likely, most folks are running one-DB-only in /var/lib/ldap (which matches most examples) and haven't tickled this issue. That said, OpenLDAP maintainers are advising folks to use subdirectories which puts this into the realm of needing to make a directory upon install, particularly when you want a second database.

@gcoxmoz
Copy link
Contributor Author

gcoxmoz commented Jul 3, 2024

Second commit, after failing acceptance tests initially.

This is a simple change that belies a lot.

The edit is Class['openldap::server::slapdconf'] is no longer forced to come after Class['openldap::server::service'] (and, test cleanup related to that).

Looking at openldap::server::slapdconf, there are calls to openldap::server::globalconf (which themselves correctly police "do this only after the service starts" and are thus safe and boring). And then slapdconf invokes create_resources(openldap::server::database) which is where the break happens. In the current world, Class['openldap::server::service'] has to finish before we start Class['openldap::server::slapdconf'], which is the trigger into making databases. Which means the service has already tried to come up (and failed) before openldap::server::slapdconf can come in and try to make things right, by creating a necessary empty directory.

slapdconf seems to have sufficient ordering on its own and its invokees, so, removing this forced class-ordering seems to be a good path forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant