[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder lslebodn commented: """ On (10/11/16 13:38), Jakub Hrozek wrote: >Since there is no ticket, I only pushed the patches to master: >682c9c3467055c2149af28826f7458b857b0f8c4 >da8801c363716533f60bc78e10f3a2100cebc3a1 > Such version should have been pushed as part of https://fedorahosted.org/sssd/ticket/3207. I noticed issues lately due to holidays. Therefore pushed to 1.14 as well sssd-1-14: * 9d4cc96f2951412f647223dfe59060fa1e2b7b14 * 2535993d81c7d0dbbd6c6fab6f45b338845535cf LS """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259918798 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder jhrozek commented: """ Since there is no ticket, I only pushed the patches to master: 682c9c3467055c2149af28826f7458b857b0f8c4 da8801c363716533f60bc78e10f3a2100cebc3a1 """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259814966 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder jhrozek commented: """ Thanks, ACK """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259810451 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder fidencio commented: """ Change done and patchset updated. Just to make things easier for the reviewer, here is what the fix up looks like: ``` [ffidenci@cat sssd]$ git diff diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini index 8a5290e..882a185 100644 --- a/src/config/cfg_rules.ini +++ b/src/config/cfg_rules.ini @@ -228,7 +228,6 @@ option = reconnection_retries option = fd_limit option = client_idle_timeout option = description -option = provider option = containers_nest_level option = max_secrets @@ -237,6 +236,7 @@ validator = ini_allowed_options section_re = ^secrets/users/[0-9]\+$ # Secrets service - proxy +option = provider option = proxy_url option = auth_type option = auth_header_name ``` """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259799269 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder jhrozek commented: """ On Tue, Nov 08, 2016 at 05:40:21AM -0800, lslebodn wrote: > On (08/11/16 05:15), fidencio wrote: > >@jhrozek: > >For the first patch the tests are correct. @lslebodn also complained that > >[secrets/users/] could be a valid case in the way the code is in git right > >now and it's also fixed by my patch. In any case, seems that we don't allow > >any config section to end with "/". > > > >For the second test, I guess that good tests are adding configuration > >options that are only allowed for [secrets] into the [secrets/users/123] > >section and vice-versa. > > > >Example of a config that should fail: > >``` > >[secrets] > >proxy_url = foo > > > >[secrets/users/123] > >timeout = 10 > >``` > > > >Example of a config that should not fail: > >``` > >[secrets] > >debug_level = 9 > > > >[secrets/users/123] > >proxy_url = foo > >``` > >@lslebodn, does it make sense for you? > > > I am fine with the 1st patch. But I am not very familiar with > the secrets code therefore It would take me much more time to review > 2nd patch. I prefer if @jhrozek could review it. (Sorry, I accidentally replied to sssd-devel instead of here). The provider= option should be moved to the per-uid section, because currently only the user sections can select a provider, the global secrets section is always 'local'. Otherwise LGTM. """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259797538 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder lslebodn commented: """ On (08/11/16 05:15), fidencio wrote: >@jhrozek: >For the first patch the tests are correct. @lslebodn also complained that >[secrets/users/] could be a valid case in the way the code is in git right now >and it's also fixed by my patch. In any case, seems that we don't allow any >config section to end with "/". > >For the second test, I guess that good tests are adding configuration options >that are only allowed for [secrets] into the [secrets/users/123] section and >vice-versa. > >Example of a config that should fail: >``` >[secrets] >proxy_url = foo > >[secrets/users/123] >timeout = 10 >``` > >Example of a config that should not fail: >``` >[secrets] >debug_level = 9 > >[secrets/users/123] >proxy_url = foo >``` >@lslebodn, does it make sense for you? > I am fine with the 1st patch. But I am not very familiar with the secrets code therefore It would take me much more time to review 2nd patch. I prefer if @jhrozek could review it. LS """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259138537 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder fidencio commented: """ @jhrozek: For the first patch the tests are correct. @lslebodn also complained that [secrets/users/] could be a valid case in the way the code is in git right now and it's also fixed by my patch. In any case, seems that we don't allow any config section to end with "/". For the second test, I guess that good tests are adding configuration options that are only allowed for [secrets] into the [secrets/users/123] section and vice-versa. Example of a config that should fail: ``` [secrets] proxy_url = foo [secrets/users/123] timeout = 10 ``` Example of a config that should not fail: ``` [secrets] debug_level = 9 [secrets/users/123] proxy_url = foo ``` @lslebodn, does it make sense for you? @jhrozek: and I really would like to be sure that the options that I put into secrets section in the second patch are **only** valid for that section or whether those options should be inherited and also allowed to [secrets/users/123] """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-259133301 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder jhrozek commented: """ Sorry for the delay in reviewing. I tried: ``` [secrets/foo] [secrets/123] ``` as an invalid test and sssd noticed them as invalid. It also didn't complain about a positive test: ``` [secrets/users/123] ``` What other tests should I run, especially wrt the second patch? """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-257296593 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder lslebodn commented: """ Jakub is more familiar with that code. """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-254771806 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder jhrozek commented: """ On Wed, Oct 19, 2016 at 03:07:28AM -0700, fidencio wrote: > @jhrozek I'm removing "Changes requested" label as the last patchset (from 2 > days ago) already contains the changes requested by Lukaš. Ah, sorry about that, I didn't notice it does. """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-254770870 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder fidencio commented: """ @jhrozek I'm removing "Changes requested" label as the last patchset (from 2 days ago) already contains the changes requested by Lukaš. """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-254770287 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
[SSSD] [sssd PR#53][comment] Fixes in the config API related to secrets responder
URL: https://github.com/SSSD/sssd/pull/53 Title: #53: Fixes in the config API related to secrets responder fidencio commented: """ And CI has passed: http://sssd-ci.duckdns.org/logs/job/55/24/summary.html """ See the full comment at https://github.com/SSSD/sssd/pull/53#issuecomment-254306475 ___ sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org
