SSSD / ding-libs

#3182 `ini_config_augment` reports errors on match failure
Closed: fixed Opened by rharwood.

Closed: fixed
Reopen Issue

When ini_config_augment() is called on a directory with a pattern supplied, error_list is populated with an entry for each file that does not match the pattern.

These not being errors and rather the point of passing a pattern at all, it seems incorrect to provide them here. Calling programs (e.g., gssproxy) try to display the contents of the library errors to the user, which results in spurious messages like:

Error when reading config directory: File /etc/gssproxy/gssproxy.conf did not match provided patterns. Skipping.

which is very confusing (including to systemd).

This looks like an original intent of the code (blame). However, it was never surfaced, so I think it should be safe to remove.

Edited by cipherboy

gssproxy workaround: https://pagure.io/gssproxy/pull-request/219

We use this message in SSSD. It is more like a warning than an error, but the current ding-libs interface does not make it possible to differentiate between errors and warnings (it only populates strings in the error_list).

However we do have the list of files that were merged during the augmentation, so the list of skipped files is not that important and removing the message is acceptable IMO.

Metadata Update from @mzidek:
- Custom field component adjusted to None
- Custom field selected adjusted to None
- Custom field type adjusted to None
- Custom field version adjusted to None

@mzidek: Would you prefer we add these messages to the ra_ok section? It seems like these are more of "notices" than "warnings" or "errors".

Per ini_configobj.h:

 * @param[out] error_list       List of strings that
 *                              contains all encountered
 *                              errors.
 *                              It can be NULL, in this case list of errors
 *                              is not populated.
 * @param[out] success_list     List of strings that
 *                              contains file names of snippets that were
 *                              successfully merged.
 *                              It can be NULL, in this case list of files
 *                              is not populated.

This seems to be out of scope for both error_list and success_list... And if users depend on behavior of success_list, could mess with that too. :/

-- Alex

One last thought: in case we still want access to these strings for debugging, we could log them with TRACE_INFO_STRING.

@cipherboy That is good idea.

@cipherboy do you want to update the PR?

Sure; question: do you want all three to use that interface, or just the two earlier ones?

[DEBUG]                        ini/ini_augment.c ( 463) File did not match provided patterns. Skipping: /home/cipherboy/GitHub/ding-libs/ini/ini.d/real32be.conf
(snip)
[DEBUG]                        ini/ini_augment.c ( 614) No sections found in file. Skipping: /home/cipherboy/GitHub/ding-libs/ini/ini.d/comment.conf

@cipherboy I will leave it to you. I would prefer just the two, but it is not a strong preference.

Merged in master:
be9ca3a2c26b061d1f22bd4a09009bba7a01f67b

Metadata Update from @mzidek:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)

Metadata Update from @lslebodn:
- Issue status updated to: Open (was: Closed)

Please revert commit because this introduced regression caught by sssd integration tests.

I all 5 for fixing this bug but please please please introduce new function which will be more flexible than current one.

BTW I hope it was not back-ported to enterprise distributions. 

=================================== FAILURES ===================================
___________________________ test_show_user_override ____________________________
Traceback (most recent call last):
  File "/home/build/sssd/src/tests/intg/ldap_local_override_test.py", line 590, in test_show_user_override
    assert out == "user1@LDAP:ov_user1:10010:20010:Overriden User 1:"\
AssertionError: assert '[DEBUG]     ...user1_shell:\n' == 'user1@LDAP:ov...ser1_shell:\n'
  - [DEBUG]                        ini/ini_augment.c ( 519) ini_aug_preprare Entry
  - [DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry
  - [DEBUG]                        ini/ini_augment.c ( 483) Input path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
  - [DEBUG]                        ini/ini_augment.c ( 503) Output path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
  - [DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit
  - [DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry
  - [DEBUG]                  ...
  ...Full output truncated (14 lines hidden), use '-vv' to show
---------------------------- Captured stdout setup -----------------------------
[DEBUG]                        ini/ini_augment.c ( 519) ini_aug_preprare Entry
[DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry
[DEBUG]                        ini/ini_augment.c ( 483) Input path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 503) Output path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit
[DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry
[DEBUG]                        ini/ini_augment.c ( 146) ini_aug_regex_prepare Entry
[DEBUG]                        ini/ini_augment.c ( 165) Pattern: ^[^\.].*\.conf$
[DEBUG]                        ini/ini_augment.c ( 210) ini_aug_regex_prepare Exit
[DEBUG]                        ini/ini_augment.c ( 416) Processing .
[DEBUG]                        ini/ini_augment.c ( 416) Processing ..
[DEBUG]                        ini/ini_augment.c ( 125) regex_cleanup Entry
[DEBUG]                        ini/ini_augment.c ( 128) regex_cleanup Exit
[DEBUG]                        ini/ini_augment.c ( 308) ini_aug_sort_list Entry
[DEBUG]                        ini/ini_augment.c ( 472) ini_aug_construct_list Exit
[DEBUG]                        ini/ini_augment.c ( 545) ini_aug_preprare Exit
[DEBUG]                        ini/ini_augment.c ( 666) ini_aug_apply Entry
[DEBUG]                        ini/ini_augment.c ( 679) ini_aug_apply Exit
[DEBUG]                        ini/ini_augment.c ( 953) ini_config_augment Exit
SSSD needs to be restarted for the changes to take effect.
[DEBUG]                        ini/ini_augment.c ( 519) ini_aug_preprare Entry
[DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry
[DEBUG]                        ini/ini_augment.c ( 483) Input path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 503) Output path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit
[DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry
[DEBUG]                        ini/ini_augment.c ( 146) ini_aug_regex_prepare Entry
[DEBUG]                        ini/ini_augment.c ( 165) Pattern: ^[^\.].*\.conf$
[DEBUG]                        ini/ini_augment.c ( 210) ini_aug_regex_prepare Exit
[DEBUG]                        ini/ini_augment.c ( 416) Processing .
[DEBUG]                        ini/ini_augment.c ( 416) Processing ..
[DEBUG]                        ini/ini_augment.c ( 125) regex_cleanup Entry
[DEBUG]                        ini/ini_augment.c ( 128) regex_cleanup Exit
[DEBUG]                        ini/ini_augment.c ( 308) ini_aug_sort_list Entry
[DEBUG]                        ini/ini_augment.c ( 472) ini_aug_construct_list Exit
[DEBUG]                        ini/ini_augment.c ( 545) ini_aug_preprare Exit
[DEBUG]                        ini/ini_augment.c ( 666) ini_aug_apply Entry
[DEBUG]                        ini/ini_augment.c ( 679) ini_aug_apply Exit
[DEBUG]                        ini/ini_augment.c ( 953) ini_config_augment Exit
___________________________ test_find_user_override ____________________________
Traceback (most recent call last):
  File "/home/build/sssd/src/tests/intg/ldap_local_override_test.py", line 624, in test_find_user_override
    assert set(out.splitlines()) == set(exp_usr_ovrd)
AssertionError: assert set(['[DEBUG]... Entry', ...]) == set(['user1@LD...ser2_shell:'])
  Extra items in the left set:
  '[DEBUG]                        ini/ini_augment.c ( 953) ini_config_augment Exit'
  '[DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit'
  '[DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry'
  '[DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry'
  '[DEBUG]                        ini/ini_augment.c ( 472) ini_aug_construct_list Exit'
  '[DEBUG]                        ini/ini_augment.c ( 210) ini_aug_regex_prepare Exit'...
  ...Full output truncated (41 lines hidden), use '-vv' to show
---------------------------- Captured stdout setup -----------------------------
[DEBUG]                        ini/ini_augment.c ( 519) ini_aug_preprare Entry
[DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry
[DEBUG]                        ini/ini_augment.c ( 483) Input path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 503) Output path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit
[DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry
[DEBUG]                        ini/ini_augment.c ( 146) ini_aug_regex_prepare Entry
[DEBUG]                        ini/ini_augment.c ( 165) Pattern: ^[^\.].*\.conf$
[DEBUG]                        ini/ini_augment.c ( 210) ini_aug_regex_prepare Exit
[DEBUG]                        ini/ini_augment.c ( 416) Processing .
[DEBUG]                        ini/ini_augment.c ( 416) Processing ..
[DEBUG]                        ini/ini_augment.c ( 125) regex_cleanup Entry
[DEBUG]                        ini/ini_augment.c ( 128) regex_cleanup Exit
[DEBUG]                        ini/ini_augment.c ( 308) ini_aug_sort_list Entry
[DEBUG]                        ini/ini_augment.c ( 472) ini_aug_construct_list Exit
[DEBUG]                        ini/ini_augment.c ( 545) ini_aug_preprare Exit
[DEBUG]                        ini/ini_augment.c ( 666) ini_aug_apply Entry
[DEBUG]                        ini/ini_augment.c ( 679) ini_aug_apply Exit
[DEBUG]                        ini/ini_augment.c ( 953) ini_config_augment Exit
SSSD needs to be restarted for the changes to take effect.
[DEBUG]                        ini/ini_augment.c ( 519) ini_aug_preprare Entry
[DEBUG]                        ini/ini_augment.c ( 482) ini_aug_expand_path Entry
[DEBUG]                        ini/ini_augment.c ( 483) Input path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 503) Output path /tmp/sssd-intg.boodn5bd/etc/sssd/conf.d
[DEBUG]                        ini/ini_augment.c ( 504) ini_aug_expand_path Exit
[DEBUG]                        ini/ini_augment.c ( 373) ini_aug_construct_list Entry
[DEBUG]                        ini/ini_augment.c ( 146) ini_aug_regex_prepare Entry
[DEBUG]                        ini/ini_augment.c ( 165) Pattern: ^[^\.].*\.conf$
[DEBUG]                        ini/ini_augment.c ( 210) ini_aug_regex_prepare Exit
[DEBUG]                        ini/ini_augment.c ( 416) Processing .
[DEBUG]                        ini/ini_augment.c ( 416) Processing ..
[DEBUG]                        ini/ini_augment.c ( 125) regex_cleanup Entry
[DEBUG]                        ini/ini_augment.c ( 128) regex_cleanup Exit
[DEBUG]                        ini/ini_augment.c ( 308) ini_aug_sort_list Entry
[DEBUG]                        ini/ini_augment.c ( 472) ini_aug_construct_list Exit
[DEBUG]                        ini/ini_augment.c ( 545) ini_aug_preprare Exit
[DEBUG]                        ini/ini_augment.c ( 666) ini_aug_apply Entry
[DEBUG]                        ini/ini_augment.c ( 679) ini_aug_apply Exit
[DEBUG]                        ini/ini_augment.c ( 953) ini_config_augment Exit
==================== 2 failed, 240 passed in 562.45 seconds ====================

Ah yes, of course the macros TRACE_LEVEL and TRACE_HOME should only be defined in builds where we want the debug messages to be generated (test and debug builds). We do not use them in normal production builds. I had the macros defined when I tested the patches to see if the messages are generated and it was a mistake to have them also in the patch.

I will open PR for this in ding-libs to remove the two #define lines. Good that we have integration tests in SSSD that discovered this.
Edited by mzidek

Here is the PR:
https://pagure.io/SSSD/ding-libs/pull-request/3184

@rharwood or @cipherboy Feel free to review the patch. The test is simple, it should not generate the debug messages (the fact that it was generated when the macros are defined were already tested :) )

The additional patch is here:
- a731d8c8c515e7e42a4fb448e0ecb6934d5bf99b

For the reference there are two patches associated with this ticket:
- a731d8c8c515e7e42a4fb448e0ecb6934d5bf99b
- be9ca3a2c26b061d1f22bd4a09009bba7a01f67b

Metadata Update from @mzidek:
- Issue close_status updated to: fixed
- Issue status updated to: Closed (was: Open)
Metadata
None
None
None
None
None
None
None
None
None
None
None
None
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.