sssd: add ldb-modules-path by liberodark · Pull Request #8116 · SSSD/sssd

liberodark · 2025-09-27T13:05:23Z

Hi,

This PR adds a configure option to specify LDB modules path at build time:
./configure --with-ldb-modules-path="/custom/path1:/custom/path2"

This fixes the "Module [memberof] not found" issue that occurs when LDB modules are installed in non-standard locations. Instead of requiring each distribution to maintain their own patch, this provides a standard way to handle different installation paths.

The implementation:

Adds the configure option via WITH_LDB_MODULES_PATH macro
Sets LDB_MODULES_PATH once during initialization
Respects any pre-existing environment variable (uses setenv with overwrite=0)

:packaging: Add --with-ldb-modules-path=PATH configure option to specify LDB modules path at build time.

Best regards

gemini-code-assist

Code Review

This pull request introduces a mechanism to override the LDB modules path at build time via a configure option. The implementation is mostly correct, but the core logic in sss_ldb_init_modules_path has a potential thread-safety issue and uses fprintf to stderr for error reporting, which is not suitable for daemonized processes. I've provided a suggestion to address these points by using an atomic flag for initialization and the SSSD logging infrastructure for error messages.

src/util/sss_ldb.c

alexey-tikhonov · 2025-09-30T10:40:00Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new configure option, --with-ldb-modules-path, to allow overriding the LDB_MODULES_PATH environment variable at build time. The implementation is well-structured, adding the necessary autotools macros and a new C source file to handle setting the environment variable. The logic to set the variable is correctly placed in confdb_init and is made thread-safe using an atomic_flag. My review found one high-severity issue: the implementation does not override an existing LDB_MODULES_PATH environment variable, which contradicts the documented behavior of the new option. A code suggestion has been provided to fix this.

src/util/sss_ldb.c

alexey-tikhonov · 2025-09-30T10:53:24Z

Hi,

./configure --with-ldb-modules-path="/custom/path1:/custom/path2"
Have fix issue about that and i think this would be good adding for not making specific patch.

what is your platform / use case?

liberodark · 2025-09-30T13:14:29Z

Hi,

./configure --with-ldb-modules-path="/custom/path1:/custom/path2"
Have fix issue about that and i think this would be good adding for not making specific patch.

what is your platform / use case?

Hi,

This is a fix that can be applied to many systems currently.
However, the problem is present on Nixos.
To avoid having to patch and have a cleaner solution, I took the time to find a clean solution for integration into SSSD directly, thus having:
--with-ldb-modules-path="${placeholder "out"}/modules/ldb:${ldb}/modules/ldb"

Best Regards

liberodark · 2025-09-30T13:30:00Z

/gemini review

gemini-code-assist

Code Review

This pull request adds a new configure option --with-ldb-modules-path to override the LDB_MODULES_PATH environment variable. This is a useful feature for custom builds. The implementation is generally sound, integrating well with the autotools build system and initializing the environment variable at an appropriate point. However, I've identified a critical thread-safety issue in the new initialization logic that could lead to a race condition and undefined behavior. My review includes a detailed comment and a code suggestion to resolve this issue.

src/util/sss_ldb.c

gemini-code-assist

Code Review

This pull request introduces a new configure option --with-ldb-modules-path to allow overriding the LDB_MODULES_PATH at build time. The changes correctly add the configure logic and call the initialization function at the right place. However, the implementation of the initialization function has a critical flaw that prevents it from working as intended, and it is also not thread-safe. My review includes a suggestion to fix both issues.

src/util/sss_ldb.c

src/util/sss_ldb.h

alexey-tikhonov · 2025-10-01T09:03:05Z

Please add a :packaging: release note to the commit message.
See

sssd/.git-commit-template

Line 15 in 481700d

# :packaging: Packaging change description.

src/confdb/confdb.c

alexey-tikhonov · 2025-10-01T10:34:09Z

:packaging: Add --with-ldb-modules-path=PATH configure option to specify LDB modules path at build time.

Thank you. But this should be included directly into commit message (not merely in the github PR description) - project has an automation to generate release notes automatically from git history.

liberodark · 2025-10-01T10:43:21Z

:packaging: Add --with-ldb-modules-path=PATH configure option to specify LDB modules path at build time.

Thank you. But this should be included directly into commit message (not merely in the github PR description) - project has an automation to generate release notes automatically from git history.

Hi,

Sorry I misunderstood, I'll correct that.

Best Regards

sumit-bose · 2025-10-02T13:16:36Z

Hi,

./configure --with-ldb-modules-path="/custom/path1:/custom/path2"
Have fix issue about that and i think this would be good adding for not making specific patch.

what is your platform / use case?

Hi,

This is a fix that can be applied to many systems currently. However, the problem is present on Nixos. To avoid having to patch and have a cleaner solution, I took the time to find a clean solution for integration into SSSD directly, thus having: --with-ldb-modules-path="${placeholder "out"}/modules/ldb:${ldb}/modules/ldb"

Best Regards

Hi,

would something like ldbmoddir="$PKG_CONFIG --variable=modulesdir ldb" or similar in a configure m4 file work as well to automatically discover the modules paths of the platform?

bye,
Sumit

liberodark · 2025-10-02T19:45:47Z

Hi,

./configure --with-ldb-modules-path="/custom/path1:/custom/path2"
Have fix issue about that and i think this would be good adding for not making specific patch.

what is your platform / use case?

Hi,
This is a fix that can be applied to many systems currently. However, the problem is present on Nixos. To avoid having to patch and have a cleaner solution, I took the time to find a clean solution for integration into SSSD directly, thus having: --with-ldb-modules-path="${placeholder "out"}/modules/ldb:${ldb}/modules/ldb"
Best Regards

Hi,

would something like ldbmoddir="$PKG_CONFIG --variable=modulesdir ldb" or similar in a configure m4 file work as well to automatically discover the modules paths of the platform?

bye, Sumit

Hi,

In nixOS build-time path from pkg-config would differ from the runtime path due to store-based packaging.
But is good suggestion.
My approach is more global to avoid making specific cases.

Best Regards

sumit-bose · 2025-10-06T08:28:27Z

Hi,

In nixOS build-time path from pkg-config would differ from the runtime path due to store-based packaging. But is good suggestion. My approach is more global to avoid making specific cases.

Hi,

thanks for the explanation. It would be nice if nixOS developers would consider to fix this so that pkg-config and similar tools can show proper run-time paths, where needed. But let's continue with your approach for the time being.

bye,
Sumit

Best Regards

src/conf_macros.m4

src/confdb/confdb.c

liberodark · 2025-10-07T06:48:39Z

Hi,
In nixOS build-time path from pkg-config would differ from the runtime path due to store-based packaging. But is good suggestion. My approach is more global to avoid making specific cases.

Hi,

thanks for the explanation. It would be nice if nixOS developers would consider to fix this so that pkg-config and similar tools can show proper run-time paths, where needed. But let's continue with your approach for the time being.

bye, Sumit

Best Regards

Hi,

I'll take the time to look into it, but I don't think the two are incompatible.

Best Regards

liberodark · 2025-10-16T08:35:16Z

Hi, @sumit-bose

Sorry for the delay in updating.

Best Regards

sumit-bose

Hi,

thank you for the updates, I have no further comments, ACK.

bye,
Sumit

alexey-tikhonov · 2025-11-05T09:20:22Z

Note: Covscan is green.

Add configure option to specify LDB modules path at build time. This allows distributions to set the correct path when LDB modules are installed in non-standard locations. The path is set once during confdb initialization if configured, respecting any pre-existing LDB_MODULES_PATH environment variable. :packaging: Add --with-ldb-modules-path=PATH configure option to specify LDB modules path at build time. Reviewed-by: Alexey Tikhonov <atikhono@redhat.com> Reviewed-by: Sumit Bose <sbose@redhat.com>

sssd-bot · 2025-11-05T09:21:15Z

The pull request was accepted by @alexey-tikhonov with the following PR CI status:

There are unsuccessful or unfinished checks. Make sure that the failures are not related to this pull request before merging.

liberodark marked this pull request as draft September 27, 2025 13:06

gemini-code-assist bot reviewed Sep 27, 2025

View reviewed changes