ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
On 06/23/2016 11:10 AM, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
I can not give an ack because I was involved in making these patches, but they LGTM and I just gave it a few runs and everything I tested seem to work. So LGTM.
I did not run CI though.
Michal
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");
Why can't we use localstatedir here instead of hardcoding /var?
- if (ret != EOK) {
DEBUG(SSSDBG_CRIT_FAILURE, "Failed to call validators\n");/* This is not fatal, continue */- }
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
Michal
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
On 06/23/2016 10:17 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
Btw the FIXME was not removed because it can be made configurable via command line option in the future.
But I think we will never do it, so the FIXME line can probably be removed as well.
Should I send a new patch without the FIXME?
Michal
On Thu, Jun 23, 2016 at 10:19:49PM +0200, Michal Židek wrote:
On 06/23/2016 10:17 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
Btw the FIXME was not removed because it can be made configurable via command line option in the future.
But I think we will never do it, so the FIXME line can probably be removed as well.
Should I send a new patch without the FIXME?
Yes please, can you also submit it to CI?
On 06/23/2016 10:22 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:19:49PM +0200, Michal Židek wrote:
On 06/23/2016 10:17 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
@@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, const char *config_dir, goto done; }
- /* FIXME: Do not hardcode the path */
- ret = sss_ini_call_validators(init_data,
"/var/lib/sss/cfg_rules.ini");Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
Btw the FIXME was not removed because it can be made configurable via command line option in the future.
But I think we will never do it, so the FIXME line can probably be removed as well.
Should I send a new patch without the FIXME?
Yes please, can you also submit it to CI?
Pushed to CI.
http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/4601/
Michal
On 06/23/2016 10:38 PM, Michal Židek wrote:
On 06/23/2016 10:22 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:19:49PM +0200, Michal Židek wrote:
On 06/23/2016 10:17 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote: > @@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, > const char *config_dir, > goto done; > } > > + /* FIXME: Do not hardcode the path */ > + ret = sss_ini_call_validators(init_data, > + "/var/lib/sss/cfg_rules.ini");
Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
Btw the FIXME was not removed because it can be made configurable via command line option in the future.
But I think we will never do it, so the FIXME line can probably be removed as well.
Should I send a new patch without the FIXME?
Yes please, can you also submit it to CI?
Pushed to CI.
CI passed.
http://sssd-ci.duckdns.org/logs/job/46/01/summary.html
Michal
On Fri, Jun 24, 2016 at 09:48:18AM +0200, Michal Židek wrote:
On 06/23/2016 10:38 PM, Michal Židek wrote:
On 06/23/2016 10:22 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:19:49PM +0200, Michal Židek wrote:
On 06/23/2016 10:17 PM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 10:12:21PM +0200, Michal Židek wrote:
On 06/23/2016 10:08 PM, Jakub Hrozek wrote: > On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote: > > @@ -217,6 +216,14 @@ int confdb_init_db(const char *config_file, > > const char *config_dir, > > goto done; > > } > > > > + /* FIXME: Do not hardcode the path */ > > + ret = sss_ini_call_validators(init_data, > > + "/var/lib/sss/cfg_rules.ini"); > > Why can't we use localstatedir here instead of hardcoding /var?
It is fixed in the second second patch. Together with some build system changes.
ah, OK.
Btw the FIXME was not removed because it can be made configurable via command line option in the future.
But I think we will never do it, so the FIXME line can probably be removed as well.
Should I send a new patch without the FIXME?
Yes please, can you also submit it to CI?
Pushed to CI.
CI passed.
I did another smoke-check which passed, so I pushed the patches to master.
Thank you for the teamwork.
* master: * 8b2a31634764168183506925a4b9f461afdba6f3 * c42ca36247022490ad65a33c453cb5e43900dbe9
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
From 76d0ab2784d341e5204d63ddebcfec2012f01016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 22 Jun 2016 19:11:42 +0200 Subject: [PATCH 1/2] confdb: Check for config file errors on sssd startup
ACK
From 0436bd95ceafed4ce1c9173fa001c5aee064b29e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Thu, 23 Jun 2016 08:52:18 +0200 Subject: [PATCH 2/2] Prepare ini schema with rules for validation
Resolves: https://fedorahosted.org/sssd/ticket/2028
Makefile.am | 5 +- contrib/sssd.spec.in | 1 + src/confdb/confdb_setup.c | 2 +- src/config/cfg_rules.ini | 615 ++++++++++++++++++++++++++++++++++++++++++++++
we need to allow entry_negative_timeout local_negative_timeout and get_domains_timeout for all responders. Also 'timeout' for all services (this one is more important, many users set timeout especially if they use enumeration).
user_attributes is also possible for the NSS responder and used to get attributes of trusted users. We also seem to be reading override_space from the monitor section.
Should I open a ticket so that we can fix these later and not delay the beta any longer?
On 06/24/2016 09:56 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
From 76d0ab2784d341e5204d63ddebcfec2012f01016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 22 Jun 2016 19:11:42 +0200 Subject: [PATCH 1/2] confdb: Check for config file errors on sssd startup
ACK
From 0436bd95ceafed4ce1c9173fa001c5aee064b29e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Thu, 23 Jun 2016 08:52:18 +0200 Subject: [PATCH 2/2] Prepare ini schema with rules for validation
Resolves: https://fedorahosted.org/sssd/ticket/2028
Makefile.am | 5 +- contrib/sssd.spec.in | 1 + src/confdb/confdb_setup.c | 2 +- src/config/cfg_rules.ini | 615 ++++++++++++++++++++++++++++++++++++++++++++++
we need to allow entry_negative_timeout local_negative_timeout and get_domains_timeout for all responders. Also 'timeout' for all services (this one is more important, many users set timeout especially if they use enumeration).
user_attributes is also possible for the NSS responder and used to get attributes of trusted users. We also seem to be reading override_space from the monitor section.
Should I open a ticket so that we can fix these later and not delay the beta any longer?
Yes, please do. Just copy the above to the ticket description. Maybe I will fix the ticket even today, but right now I am doing something else.
Michal
On 06/24/2016 10:09 AM, Michal Židek wrote:
On 06/24/2016 09:56 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
From 76d0ab2784d341e5204d63ddebcfec2012f01016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 22 Jun 2016 19:11:42 +0200 Subject: [PATCH 1/2] confdb: Check for config file errors on sssd startup
ACK
From 0436bd95ceafed4ce1c9173fa001c5aee064b29e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Thu, 23 Jun 2016 08:52:18 +0200 Subject: [PATCH 2/2] Prepare ini schema with rules for validation
Resolves: https://fedorahosted.org/sssd/ticket/2028
Makefile.am | 5 +- contrib/sssd.spec.in | 1 + src/confdb/confdb_setup.c | 2 +- src/config/cfg_rules.ini | 615 ++++++++++++++++++++++++++++++++++++++++++++++
we need to allow entry_negative_timeout local_negative_timeout and get_domains_timeout for all responders. Also 'timeout' for all services (this one is more important, many users set timeout especially if they use enumeration).
user_attributes is also possible for the NSS responder and used to get attributes of trusted users. We also seem to be reading override_space from the monitor section.
Should I open a ticket so that we can fix these later and not delay the beta any longer?
Yes, please do. Just copy the above to the ticket description. Maybe I will fix the ticket even today, but right now I am doing something else.
Michal
Btw. these patches should be pushed after the patch in "[PATCH] confd: Make it possible to use config snippets" is pushed.
Michal
On Fri, Jun 24, 2016 at 10:09:24AM +0200, Michal Židek wrote:
On 06/24/2016 09:56 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
From 76d0ab2784d341e5204d63ddebcfec2012f01016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 22 Jun 2016 19:11:42 +0200 Subject: [PATCH 1/2] confdb: Check for config file errors on sssd startup
ACK
From 0436bd95ceafed4ce1c9173fa001c5aee064b29e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Thu, 23 Jun 2016 08:52:18 +0200 Subject: [PATCH 2/2] Prepare ini schema with rules for validation
Resolves: https://fedorahosted.org/sssd/ticket/2028
Makefile.am | 5 +- contrib/sssd.spec.in | 1 + src/confdb/confdb_setup.c | 2 +- src/config/cfg_rules.ini | 615 ++++++++++++++++++++++++++++++++++++++++++++++
we need to allow entry_negative_timeout local_negative_timeout and get_domains_timeout for all responders. Also 'timeout' for all services (this one is more important, many users set timeout especially if they use enumeration).
user_attributes is also possible for the NSS responder and used to get attributes of trusted users. We also seem to be reading override_space from the monitor section.
Should I open a ticket so that we can fix these later and not delay the beta any longer?
Yes, please do. Just copy the above to the ticket description. Maybe I will fix the ticket even today, but right now I am doing something else.
https://fedorahosted.org/sssd/ticket/3068
I assigned it to you :-)
On 06/27/2016 07:04 PM, Jakub Hrozek wrote:
On Fri, Jun 24, 2016 at 10:09:24AM +0200, Michal Židek wrote:
On 06/24/2016 09:56 AM, Jakub Hrozek wrote:
On Thu, Jun 23, 2016 at 11:10:57AM +0200, Lukas Slebodnik wrote:
ehlo
The first patch is sligtly modified version of Michal's patch. It depends on patch for config snippet. Because config validation is optional if it isn't supported in libini_config. And detection for new libini_config is in patch for config snippets
You might see "typos" in sssd.log e.g. (Thu Jun 23 10:48:39:370079 2016) [sssd] [sss_ini_call_validators] (0x0020): [rule/allowed_domain_options]: Attribute 'ldapi_uri' is not allowed in section 'domain/example.com'. Check for typos.
BTW don't forget to build with ding-libs-0.6 (libini_config 1.3.0)
LS
From 76d0ab2784d341e5204d63ddebcfec2012f01016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michal=20=C5=BDidek?= mzidek@redhat.com Date: Wed, 22 Jun 2016 19:11:42 +0200 Subject: [PATCH 1/2] confdb: Check for config file errors on sssd startup
ACK
From 0436bd95ceafed4ce1c9173fa001c5aee064b29e Mon Sep 17 00:00:00 2001 From: Lukas Slebodnik lslebodn@redhat.com Date: Thu, 23 Jun 2016 08:52:18 +0200 Subject: [PATCH 2/2] Prepare ini schema with rules for validation
Resolves: https://fedorahosted.org/sssd/ticket/2028
Makefile.am | 5 +- contrib/sssd.spec.in | 1 + src/confdb/confdb_setup.c | 2 +- src/config/cfg_rules.ini | 615 ++++++++++++++++++++++++++++++++++++++++++++++
we need to allow entry_negative_timeout local_negative_timeout and get_domains_timeout for all responders. Also 'timeout' for all services (this one is more important, many users set timeout especially if they use enumeration).
user_attributes is also possible for the NSS responder and used to get attributes of trusted users. We also seem to be reading override_space from the monitor section.
Should I open a ticket so that we can fix these later and not delay the beta any longer?
Yes, please do. Just copy the above to the ticket description. Maybe I will fix the ticket even today, but right now I am doing something else.
https://fedorahosted.org/sssd/ticket/3068
I assigned it to you :-)
Ok. Thanks.
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Lukas Slebodnik -
Michal Židek