Add support for openSUSE UsrEtc by scabrero · Pull Request #8504 · SSSD/sssd

scabrero · 2026-03-06T16:52:13Z

To support transactional-updates in openSUSE, this PR adds support for UsrEtc.

Vendor provided configuration is installed in /usr/etc/sssd/sssd.conf.
Users can override the vendor creating /etc/sssd/sssd.conf or dropping config snippets to /etc/sssd/conf.d/

Doc: https://en.opensuse.org/openSUSE:Packaging_UsrEtc

gemini-code-assist

Code Review

This pull request adds support for openSUSE's UsrEtc layout, allowing SSSD to fall back to a vendor-provided configuration file. The changes are generally well-implemented across the build system and source code. However, I've identified a memory leak in src/monitor/monitor.c due to incorrect handling of talloc allocations when determining the configuration file path. Additionally, there's a minor bug in configure.ac that causes a user-facing notice to display an empty value. I have provided suggestions to address both of these issues.

gemini-code-assist · 2026-03-06T16:56:12Z

+              [], [enable_vendordir=no])
+if test "$enable_vendordir" != no; then
+  AC_DEFINE(USE_VENDORDIR, 1, [Define if distribution provided configuration files should be used.])
+  AC_MSG_NOTICE([Used vendor dir: $VENDORDIR])


The shell variable for the vendor directory is vendordir, not VENDORDIR. VENDORDIR is the C preprocessor macro. This will result in an empty value being printed in the notice.

AC_MSG_NOTICE([Used vendor dir: $vendordir])

ikerexxe

I only added comments to the first instance of each problem.

I'd highly recommend you to create a centralized place to manage this logic. A new file located in src/util/util_config.c would probably be the best location to place this logic. This way we reduce the maintenance burden and the possibility of applying fixes in one place but forgetting about the other

Signed-off-by: Samuel Cabrero <scabrero@suse.com>

pbrezina

Thank you, Samuel, it looks good. Can you also add a :packaging: release note to the last commit? See https://github.com/SSSD/sssd/blob/master/.git-commit-template

pbrezina · 2026-03-26T21:16:54Z

-            ret = ENOMEM;
-            goto done;
-        }
+        config_snippet_path = CONFDB_DEFAULT_CONFIG_DIR;


I'm not sure about this. The tools can be given a different config file path (-c /path/to/my/sssd.conf) and the idea here was to have snippet folder /path/to/my/sssd.conf.d. This change, while elegant, breaks this.

While I like it, I'm not sure if we can break it out of a sudden. And if do this, the snippet path should be empty if a non-default config file is provided (otherwise the snippets from default directory would be loaded, which may be unexpected).

@SSSD/developers opinions?

Ok, I see the use case now. Someone might want to check a config file + snippets out of the standard directories. I will wait for the feedback before reverting this change.

@thalman introduced support for '-c' in 61f4aaa

Upcoming sssd-2.13 will be used in C10S/RHEL10, I don't think we can remove a feature there (even if hardly used)...

@thalman, @sumit-bose, thoughts?

Ok, @scabrero let's revert.

Done. The snippets directory is built from the given config file path only if it is not specified with -s.

ikerexxe

Just a minor comment inline. Make sure to add some debugging every time config_file == NULL

ikerexxe

LGTM! Thank you for the patches

alexey-tikhonov · 2026-04-13T10:44:00Z

@ikerexxe, are you fine with the latest version?

ikerexxe

Yes, this is fine

ikerexxe

I know I already gave this the green light, but after a second look, I noticed we’re missing some critical documentation updates for the new configuration logic.

In addition to the inline comments, the introduction of the new --with-vendordir configure option and the updated configuration file resolution behavior currently lack documentation in the man pages, as well as the general build docs. It is essential that we clearly describe the new fallback hierarchy in these files so that administrators and packagers have a reliable reference for how the system prioritizes vendor-provided settings versus local configurations.

Vendor provided configuration is installed in /usr/etc/sssd/sssd.conf. Users can override it creating /etc/sssd/sssd.conf, or override defaults dropping config snippets to /etc/sssd/conf.d/ Doc: https://en.opensuse.org/openSUSE:Packaging_UsrEtc Doc: https://github.com/uapi-group/specifications/blob/main/specs/configuration_files_specification.md :packaging: New configure option '--with-vendordir' to enable reading the vendor provided configuration file. Signed-off-by: Samuel Cabrero <scabrero@suse.com>

Adds a section in sssd.conf manpage to explain how the vendor provided configuration can be masked or overriden. Signed-off-by: Samuel Cabrero <scabrero@suse.com>

scabrero · 2026-04-14T15:50:37Z

I know I already gave this the green light, but after a second look, I noticed we’re missing some critical documentation updates for the new configuration logic.

In addition to the inline comments, the introduction of the new --with-vendordir configure option and the updated configuration file resolution behavior currently lack documentation in the man pages, as well as the general build docs. It is essential that we clearly describe the new fallback hierarchy in these files so that administrators and packagers have a reliable reference for how the system prioritizes vendor-provided settings versus local configurations.

Good point about the man page, I have added a section to explain the hierarchy if vendor dir is enabled. Regarding build docs, I will prepare an update for the sssd.io repository.

ikerexxe

In addition to the inline comment, there's a merge conflict in conf_macros.m4. Please fix it

ikerexxe · 2026-04-15T07:34:36Z

+          the system specific configurtion file <filename>&sssd_conf_dir;/sssd.conf</filename>
+          or partly overriden by creating config snippets in <filename>&sssd_conf_dir;/conf.d</filename>
+          directory.
+        </para>


I'm missing an explanation on the configuration file hierarchy. One question that comes to my mind that I'd like to get clarified in this documentation. What happens if there isn't any SSSD configuration file in the vendor directory, but it exists in the regular one (/etc/sssd/sssd.conf), does SSSD use this configuration?

alexey-tikhonov · 2026-04-16T11:07:06Z

make-distcheck fails:


ERROR: files left in build directory after distclean:
./src/man/src/man/sssd_conf_dir.include
./src/man/src/man/sssd_vendor_dir.include

alexey-tikhonov · 2026-04-23T10:24:45Z

@scabrero, ping :)

gemini-code-assist Bot reviewed Mar 6, 2026

View reviewed changes

alexey-tikhonov added the no-backport This should go to target branch only. label Mar 6, 2026

alexey-tikhonov requested a review from ikerexxe March 12, 2026 13:40

alexey-tikhonov assigned alexey-tikhonov and ikerexxe and unassigned alexey-tikhonov Mar 12, 2026

ikerexxe requested changes Mar 13, 2026

View reviewed changes

Comment thread src/monitor/monitor.c Outdated

Comment thread src/monitor/monitor.c Outdated

Comment thread src/monitor/monitor.c Outdated

Comment thread configure.ac Outdated

pbrezina self-assigned this Mar 19, 2026

pbrezina self-requested a review March 19, 2026 14:46

sssctl: Add missing new line

0f7227c

Signed-off-by: Samuel Cabrero <scabrero@suse.com>

scabrero force-pushed the scabrero-UsrEtc branch from 7e24e21 to 36cc0f7 Compare March 26, 2026 11:42

pbrezina requested changes Mar 26, 2026

View reviewed changes

ikerexxe reviewed Mar 30, 2026

View reviewed changes

Comment thread src/monitor/monitor.c

scabrero force-pushed the scabrero-UsrEtc branch from 36cc0f7 to 17b0927 Compare March 30, 2026 11:24

ikerexxe approved these changes Mar 30, 2026

View reviewed changes

scabrero force-pushed the scabrero-UsrEtc branch from 17b0927 to 6be9652 Compare March 30, 2026 13:30

scabrero force-pushed the scabrero-UsrEtc branch from 6be9652 to 95e10c8 Compare April 8, 2026 08:24

pbrezina approved these changes Apr 13, 2026

View reviewed changes

alexey-tikhonov requested a review from ikerexxe April 13, 2026 10:43

ikerexxe approved these changes Apr 13, 2026

View reviewed changes

ikerexxe reviewed Apr 14, 2026

View reviewed changes

Comment thread src/conf_macros.m4

Comment thread src/util/sss_config.c Outdated

scabrero added 2 commits April 14, 2026 17:18

doc: Document the config file hierarchy when vendor dir is enabled

8af8421

Adds a section in sssd.conf manpage to explain how the vendor provided configuration can be masked or overriden. Signed-off-by: Samuel Cabrero <scabrero@suse.com>

scabrero force-pushed the scabrero-UsrEtc branch from 95e10c8 to 8af8421 Compare April 14, 2026 15:45

ikerexxe reviewed Apr 15, 2026

View reviewed changes

alexey-tikhonov added the Changes requested label Apr 16, 2026

Conversation

scabrero commented Mar 6, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pbrezina left a comment

Choose a reason for hiding this comment

Uh oh!

pbrezina Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

pbrezina Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

scabrero Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

pbrezina Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

scabrero Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov commented Apr 13, 2026

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

scabrero commented Apr 14, 2026

Uh oh!

ikerexxe left a comment

Choose a reason for hiding this comment

Uh oh!

ikerexxe Apr 15, 2026

Choose a reason for hiding this comment

Uh oh!

alexey-tikhonov commented Apr 16, 2026

Uh oh!

alexey-tikhonov commented Apr 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants