On (12/07/16 15:16), Michal Židek wrote: >On 07/12/2016 01:28 PM, Lukas Slebodnik wrote: >> On (11/07/16 07:44), Michal Zidek wrote: >> > Ok, I split the patches (one per option). >> > >> > Michal >> >> > From 4c11e6cfcfee3cad801d513d75e136e4bd3bd598 Mon Sep 17 00:00:00 2001 >> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >> > Date: Mon, 11 Jul 2016 13:03:28 +0200 >> > Subject: [PATCH 1/4] config: Allow timeout for all sevices >> > >> > Fixes: >> > https://fedorahosted.org/sssd/ticket/3068 >> > >> > Allow option "timeout" for all sevices. >> > --- >> > src/config/cfg_rules.ini | 7 +++++++ >> > src/config/etc/sssd.api.conf | 2 +- >> > 2 files changed, 8 insertions(+), 1 deletion(-) >> > >> Almost ACK >> >> I realized that there is an unused macro CONFDB_SERVICE_TIMEOUT >> Could you remove it in this patch? >> This macro is not used since commit 31d97bce8f113276bf73c7d4349f720cd5edbcb8 >> (3+ years) >> >> >> >> > From 851e274f5a8067f10b2fd29acc6a3bfc8da49cd3 Mon Sep 17 00:00:00 2001 >> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >> > Date: Mon, 11 Jul 2016 13:11:41 +0200 >> > Subject: [PATCH 2/4] config: override_space is monitor's option >> > >> > Fixes: >> > https://fedorahosted.org/sssd/ticket/3068 >> > >> > We read override_space from [sssd] not >> > [nss] section. >> > --- >> ACK >> >> > From c478a9440bb50c56c6004da806c0cdf8e9bbcc56 Mon Sep 17 00:00:00 2001 >> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >> > Date: Mon, 11 Jul 2016 13:23:40 +0200 >> > Subject: [PATCH 3/4] config: Fix user_attributes >> > >> > Fixes: >> > https://fedorahosted.org/sssd/ticket/3068 >> > >> > Option user_attributes is also available in >> > NSS responder, but not in PAC responder. >> > --- >> >> ACK >> >> > From ee4449c7b5c6154bfb079725e62874948c42124d Mon Sep 17 00:00:00 2001 >> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >> > Date: Mon, 11 Jul 2016 13:34:03 +0200 >> > Subject: [PATCH 4/4] config: Add config_file_version to schema >> > >> > Fixes: >> > https://fedorahosted.org/sssd/ticket/3068 >> > --- >> > src/config/cfg_rules.ini | 1 + >> > 1 file changed, 1 insertion(+) >> > >> > diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini >> > index 5c8d05a..635c078 100644 >> > --- a/src/config/cfg_rules.ini >> > +++ b/src/config/cfg_rules.ini >> > @@ -39,6 +39,7 @@ option = user >> > option = default_domain_suffix >> > option = certificate_verification >> > option = override_space >> > +option = config_file_version >> > >> > [rule/allowed_nss_options] >> > validator = ini_allowed_options >> > -- >> > 1.8.3.1 >> > >> >> Python API schema is not generated yet >> therefore we should add this option also >> to src/config/etc/sssd.api.conf. >> It was probably removed with the change from default 1 -> 2 >> >> BTW. We need to also allow section + add default options for >> "secrets" service. So you will need to modify the 1st patch. >> >> I will push acked patches after CI. >> >> LS > >Sending the patches that were not acked + patch >that adds the 'secrets' service. > >Michal >
>From 42a3038b68452cf92b2f87ae0875f4e3b8b1f051 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >Date: Mon, 11 Jul 2016 13:03:28 +0200 >Subject: [PATCH 1/3] config: Allow timeout for all sevices > >Fixes: >https://fedorahosted.org/sssd/ticket/3068 > >Allow option "timeout" for all sevices. >Also remove unused macro CONFDB_SERVICE_TIMEOUT. ACK >From cacd9f84e702c2aa7f5c41d0d257eb5ce8c77a12 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >Date: Mon, 11 Jul 2016 13:34:03 +0200 >Subject: [PATCH 2/3] config: Add config_file_version to schema > >Fixes: >https://fedorahosted.org/sssd/ticket/3068 >--- > src/config/SSSDConfigTest.py | 1 + > src/config/cfg_rules.ini | 1 + > src/config/etc/sssd.api.conf | 1 + > 3 files changed, 3 insertions(+) > >diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py >index 5fa9bce..332d870 100755 >--- a/src/config/SSSDConfigTest.py >+++ b/src/config/SSSDConfigTest.py >@@ -289,6 +289,7 @@ class SSSDConfigTestSSSDService(unittest.TestCase): > > options = service.list_options() > control_list = [ >+ 'config_file_version', > 'services', > 'domains', > 'timeout', >diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini >index 5c8d05a..635c078 100644 >--- a/src/config/cfg_rules.ini >+++ b/src/config/cfg_rules.ini >@@ -39,6 +39,7 @@ option = user > option = default_domain_suffix > option = certificate_verification > option = override_space >+option = config_file_version > > [rule/allowed_nss_options] > validator = ini_allowed_options >diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf >index e4011a3..737f0e1 100644 >--- a/src/config/etc/sssd.api.conf >+++ b/src/config/etc/sssd.api.conf >@@ -19,6 +19,7 @@ diag_cmd = str, None, false > > [sssd] > # Monitor service >+config_file_version = int, None, false > services = list, str, true, nss, pam > domains = list, str, true > sbus_timeout = int, None, false >-- >2.5.0 > LGTM. I haven't tested yet. >From 279677774d4eca57972512f145033b253d0dbf29 Mon Sep 17 00:00:00 2001 >From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzi...@redhat.com> >Date: Tue, 12 Jul 2016 15:05:16 +0200 >Subject: [PATCH 3/3] config: Allow 'secrets' section > >Fixes: >https://fedorahosted.org/sssd/ticket/3068 > >Allow the 'secrets' section in config file >schema. >--- > src/config/SSSDConfigTest.py | 6 ++++-- > src/config/cfg_rules.ini | 21 +++++++++++++++++++++ > src/config/etc/sssd.api.conf | 4 ++++ > 3 files changed, 29 insertions(+), 2 deletions(-) > >diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py >index 332d870..4748ecb 100755 >--- a/src/config/SSSDConfigTest.py >+++ b/src/config/SSSDConfigTest.py >@@ -1351,7 +1351,8 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): > 'autofs', > 'ssh', > 'pac', >- 'ifp'] >+ 'ifp', >+ 'secrets'] > for section in control_list: > self.assertTrue(sssdconfig.has_section(section), > "Section [%s] missing" % >@@ -1444,7 +1445,8 @@ class SSSDConfigTestSSSDConfig(unittest.TestCase): > 'autofs', > 'ssh', > 'pac', >- 'ifp'] >+ 'ifp', >+ 'secrets'] > service_list = sssdconfig.list_services() > for service in control_list: > self.assertTrue(service in service_list, >diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini >index 635c078..603c0ed 100644 >--- a/src/config/cfg_rules.ini >+++ b/src/config/cfg_rules.ini >@@ -224,6 +224,27 @@ option = diag_cmd > option = allowed_uids > option = user_attributes > >+[rule/allowed_secrets_options] >+validator = ini_allowed_options >+section_re = ^secrets/.*$ >+ >+option = timeout >+option = debug >+option = debug_level >+option = debug_timestamps >+option = debug_microseconds >+option = debug_to_files >+option = command >+option = reconnection_retries >+option = fd_limit >+option = client_idle_timeout >+option = force_timeout >+option = description >+option = diag_cmd >+ >+# secrets responder >+option = provider >+ I think you need to also update "rule/allowed_sections" maybe you could run tour tool "sssctl config-check" before sending patches :-) And there are another related question to this topic. Should we add undocumented option to the list? We already have "command" in schema. Should we add other as well? IMHO, no. LS _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
