[SSSD] Re: [PATCH] GPO: log specific ini parse error messages

[Lukas Slebodnik]

On (22/03/16 22:01), Lukas Slebodnik wrote:
>On (22/03/16 14:18), Michal Židek wrote:
>>On 03/22/2016 10:45 AM, Michal Židek wrote:
>>>On 03/21/2016 02:17 PM, Lukas Slebodnik wrote:
On (16/03/16 16:50), Michal Židek wrote:
>Hi,
>
>sorry, I do not have working AD so I did
>not test the patches. My testing
>was only SSSD compilation :)
>
>But I made a small ap that parses
>ini files and treats the errors
>the same way as in these patches
>and it worked fine.
>
>It prints the problematic line
>with short error description
>(like missing equal sign).
>
>The patch is quite simple. It does not
>solve any GPO issues, just logs found
>parsing errors for easier debugging.
>
>Michal

>From 0bff25243b18406f88910a88705397365e2e37f1 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>Date: Wed, 16 Mar 2016 16:38:34 +0100
>Subject: [PATCH] GPO: log specific ini parse error messages
>
>We should log error messages generated by
>libini if there are problems with parsing
>gpo files.
>---
>src/providers/ad/ad_gpo.c   | 19 +++
>src/providers/ad/ad_gpo_child.c | 19 +++
>2 files changed, 38 insertions(+)
>
>diff --git a/src/providers/ad/ad_gpo.c b/src/providers/ad/ad_gpo.c
>index 069196c..360aca5 100644
>--- a/src/providers/ad/ad_gpo.c
>+++ b/src/providers/ad/ad_gpo.c
>@@ -1131,8 +1131,27 @@ ad_gpo_store_policy_settings(struct
>sss_domain_info *domain,
>
> ret = ini_config_parse(file_ctx, INI_STOP_ON_NONE, 0, 0,
>ini_config);
> if (ret != 0) {
>+int lret;
>+char **errors;
>+
> DEBUG(SSSDBG_CRIT_FAILURE,
>   "ini_config_parse failed [%d][%s]\n", ret,
>strerror(ret));

Does it make sense to print also the filename?
ini_config_get_filename()
or is it logged somewhere else?
>>>
>>>It is not logged on the same level (and in gpo_child
>>>it is not logged at all) so I think it makes sense to
>>>add it to the debug message that informs about parse
>>>failure. I updated the patch.
>>>

BTW you might not be able to do it in gpo_child
because IIRC config file is parsed from memory (fmemopen)

>>>
>>>It is available in both cases. We call ini_config_file_open
>>>just few lines above.
>>>
>>>Michal
>>>
LS
>>>
>>>
>>
>>Lukas noticed that there is missing newline in
>>the debug message. I also shortened one of the
>>messages because it contained uninteresting info.
>>
>>New patch attached.
>>
>>Michal
>>
>
>>From e252f76364d3db5446b34c50379dfda31cbb6aa9 Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>>Date: Wed, 16 Mar 2016 16:38:34 +0100
>>Subject: [PATCH] GPO: log specific ini parse error messages
>>
>>We should log error messages generated by
>>libini if there are problems with parsing
>>gpo files.
>>---
>ACK
>
>http://sssd-ci.duckdns.org/logs/job/39/74/summary.html
>
>BTW I provided a test package with your patch
>to user who had such issue. I hope it will help.
>
I could see a extended error message in user logs.

master:
* dad416a9b0095e1c423b7da65db7c636fa69e614

sssd-1-13:
* 148c12d5e2a41280bc459fc686234fca67ffd411

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] UTIL: Allow to append new line in sss_vdebug_fn

[Lukas Slebodnik]

On (23/03/16 12:11), Lukas Slebodnik wrote:
>On (10/03/16 13:15), Lukas Slebodnik wrote:
>>On (10/03/16 13:10), Pavel Březina wrote:
>>>On 03/10/2016 01:05 PM, Lukas Slebodnik wrote:
On (10/03/16 12:58), Pavel Březina wrote:
>On 03/09/2016 05:39 PM, Lukas Slebodnik wrote:
>>ehlo,
>>
>>I read log files from latest 1.13 today and it was a small challenge
>>due to missing line feed after some ldb messages.
>>
>>LS
>
>Can we check if new line is present instead of always appending it?
We do not always append it. We append it only in libldb.
Because libldb is inconsistent. I sent patch to samba upstream
>>>
>>>That's why I ask for it.
>>>
but we cannot reply on specific version of libldb.
In future, we can conditionally remove this flag if we detect there is
recent enough libldb.

We could append it but it would be additional slowdown in debug messages
even in other parts of code which already have line feed in message
(sssd debug messages, libhbac ...)
>>>
>>>It doesn't need to be on sssd debug, but only in ldb_debug_messages. I don't
>>>think that if (fmt[strlen-1] '= '\n') is such a slow down, but I don'ẗ
>>>insist.
>>>
>>strlen is time consuming.
>>
>>But if there will be more developers who like idea with strlen.
>>Then we might consider to add it.
>>
>>>Ack to the patches then.
>>>
>>Thank you for review.
>>
>>I will wait few days
>>for other opinions before pushing.
>>
>http://sssd-ci.duckdns.org/logs/job/39/88/summary.html
>
>master:
>* 7c30eade4ae794ed809845f2ef70dda849b6e7c9
>* 558ec7d717735bb16c210c675c2cc5bee1da4576
>
and the same issue is in 1.13 branch therefore backported patches:
sssd-1-13:
* f07332dca37952b371936be5005c10319b3e32bd
* a7eba44333da1f711d027ef0f034f1ac6edb8403

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Jakub Hrozek]

> On 22 Mar 2016, at 16:19, Michal Židek  wrote:
> 
> On 03/22/2016 03:29 PM, Sumit Bose wrote:
>> On Tue, Mar 22, 2016 at 12:29:39PM +0100, Michal Židek wrote:
>>> Hi,
>>> 
>>> I would like to write a patch that will
>>> allow SSSD to use the config file merging
>>> feature from libini. But first I would like
>>> to ask developers for their opinions on how
>>> this  should be implemented.
>>> 
>>> My idea was that it could work like this.
>>> 
>>> The current /etc/sssd/sssd.conf would work
>>> as usual.
>>> 
>>> We would add new directory /etc/sssd/conf.d/
>>> and its content would be following:
>>> - README file that informs what the direcotory
>>> is for.
>>> - any number of files ending with .conf extension
>>>   that would contain additional configuration for
>>>   SSSD. These files would have higher prioriry
>>>   than the /etc/sssd/sssd.conf , meaning if
>>>   the same option is present in these files
>>>   it will override the /etc/sssd/sssd.conf
>>>   value.
>>> 
>>> SSSD would automatically pick up files ending
>>> in .conf from that direcory and use them. In
>>> order to disable the config file, the admin will
>>> have to rename the file ending (for example
>>> .conf.disabled). This way, we do not need to
>>> inspect the snippets for any special options
>>> like 'enable_this_snippet = true' which would
>>> just complicate the processing.
>>> 
>>> In order for SSSD to load a configuration, all
>>> the config snippets in /etc/sssd/conf.d/ and
>>> /etc/sssd/sssd.conf must in combination
>>> result in valid configuration. If there is an
>>> error in processing one of the config files,
>>> the whole configuration loading will be
>>> unsuccessful and there will be no way to
>>> skip problematic snippets (later we may add
>>> a fallback config, but that is different issue).
>>> 
>>> Of course sssd will have an cli option to
>>> use alternative directory for snippets (similar
>>> to what we use for config file now).
>>> 
>>> Could it be implemented this way?
>>> Any comments are welcome.
>> 
>> How will this interact with the python configuration API? If some
>> application will use the API to change the configuration the result will
>> be written to sssd.conf. But if there is a config snippets which sets
>> the same value the change via the config API is overridden which I guess
>> is unexpected by the application using the config API.

Would it be possible in this case for configAPI to load the resulting config 
file even with include files and if the value is different from the one set. 
then raise an exception? Maybe the admin could then pass a flag to configAPI to 
force-override the override?

>> 
>> bye,
>> Sumit
>> 
> 
> Good question. I was not thinking about this. We
> could change the config API to actually write to its
> own snippet that will be always applied last.
> 
> OTOH some admins may want to really override whatever
> other applications may set up using python config API.
> 
> If we decide to apply snippets in alphabetical
> order as Lukas suggested, we can give the snippet to which
> python API writes a name for example 990_pyapi.conf
> and if the admin decides to create snippet with starting
> number lower than 990 it will have lower pririty than
> python config API.
> 
> We should document such behavior in the README file
> that will be placed in the /etc/sssd/conf.d directory.
> 

Hmm, this could work, except dowgrades to a version that does not support 
include directory, in that case this pyapi override wouldn't be reachable..

Also, I'm not sure if this would solve the problem Sumit describes where the 
admin has some (opaque) config and calls the confiAPI to change a parameter..
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Jakub Hrozek]

> On 23 Mar 2016, at 11:11, Michal Židek  wrote:
> 
> On 03/23/2016 11:02 AM, Lukas Slebodnik wrote:
>> On (23/03/16 10:53), Michal Židek wrote:
>>> On 03/22/2016 09:10 PM, Jakub Hrozek wrote:
 
> On 22 Mar 2016, at 12:29, Michal Židek  wrote:
> 
> Hi,
> 
> I would like to write a patch that will
> allow SSSD to use the config file merging
> feature from libini. But first I would like
> to ask developers for their opinions on how
> this  should be implemented.
> 
> My idea was that it could work like this.
> 
> The current /etc/sssd/sssd.conf would work
> as usual.
> 
> We would add new directory /etc/sssd/conf.d/
> and its content would be following:
> - README file that informs what the direcotory
> is for.
> - any number of files ending with .conf extension
>  that would contain additional configuration for
>  SSSD. These files would have higher prioriry
>  than the /etc/sssd/sssd.conf , meaning if
>  the same option is present in these files
>  it will override the /etc/sssd/sssd.conf
>  value.
> 
> SSSD would automatically pick up files ending
> in .conf from that direcory and use them. In
> order to disable the config file, the admin will
> have to rename the file ending (for example
> .conf.disabled). This way, we do not need to
> inspect the snippets for any special options
> like 'enable_this_snippet = true' which would
> just complicate the processing.
> 
> In order for SSSD to load a configuration, all
> the config snippets in /etc/sssd/conf.d/ and
> /etc/sssd/sssd.conf must in combination
> result in valid configuration. If there is an
> error in processing one of the config files,
> the whole configuration loading will be
> unsuccessful and there will be no way to
> skip problematic snippets (later we may add
> a fallback config, but that is different issue).
> 
 
 I guess for now this is acceptable, because the snippet is similar to 
 editing the conf file (as Sumit noted) but it would be nice to at least 
 note in the design page of the fallback config and this merging feature 
 that they are interconnected..
 
 
> Of course sssd will have an cli option to
> use alternative directory for snippets (similar
> to what we use for config file now).
> 
 
 Does anyone use the -c option actually (maybe except tests?)
 
 If not, do we need to add a new option? I'm cautious here, because any new 
 option needs to be supported (almost) forever..
 
>>> 
>>> Ok, we can do it without an option and add the option if there is a
>>> demand for it. It will be easy change (cca 15 lines + doc).
>>> 
> Could it be implemented this way?
> Any comments are welcome.
> 
 
 Would the admin be able to discover from debug message where an option 
 comes from (conf file or snippet) ? Does libini provide that info?
>>> 
>>> Libini does not provide that info. I was thinking about creating a
>>> tool (not sure if Python or C, I think Python will be easier for
>>> this) that would be called sss_showconf and it would print
>>> the actual configuration to stdout. With
>>> 
>>> $ sss_showconf -s
>>> or
>>> $ sss_showconf --show-source
>>> 
>> or sssctl config :-)
> 
> That is another option :)
> 

I would prefer sssctl config, too.

>> 
>> It looks like we implement config file merging on two places
>> python-sssdconfig and libini. I do not like it but we probably cannot avoid 
>> it.
>> 
>> Therefore it would be good to add test that both implementation will result
>> in the same "result". I would prefer to do it with libini_config
>> because sssd daemon will use it.
>> * dump config with libini_config (ini_config_save_as)
>> * dump config from python-sssdconfig + import with libini and
>>   save ini_config_save_as.
> 
> Yes, such test makes sense.
> 
> But Jakub wanted to know if there is a way to provide
> info about where the option comes from (sssd.conf or which snippet).
> libini_config currently can not do that and I think it will be easier
> to do it in Python.

btw I don't think the ability to see which config comes from which file is a 
blocker, just a nice to have. Would libini at least let admin know that it's 
processing some include file? I'm just concerned about admins banging their 
heads against the wall when they configured foo=bar in sssd.conf yet sssd would 
keep using foo=baz :)

> 
>> 
>> The result shoudl be equivalent. IIRC libini will not dump duplicates
>> because we use last match win.
>> 
>> Or try to use different test.
>> 
>> LS
>> ___
>> sssd-devel mailing list
>> sssd-devel@lists.fedorahosted.org
>> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>> 
> ___
> 

[SSSD] Re: Config file merging in SSSD

[Lukas Slebodnik]

On (22/03/16 15:44), Simo Sorce wrote:
>On Tue, 2016-03-22 at 16:19 +0100, Michal Židek wrote:
>> On 03/22/2016 03:29 PM, Sumit Bose wrote:
>> > On Tue, Mar 22, 2016 at 12:29:39PM +0100, Michal Židek wrote:
>> >> Hi,
>> >>
>> >> I would like to write a patch that will
>> >> allow SSSD to use the config file merging
>> >> feature from libini. But first I would like
>> >> to ask developers for their opinions on how
>> >> this  should be implemented.
>> >>
>> >> My idea was that it could work like this.
>> >>
>> >> The current /etc/sssd/sssd.conf would work
>> >> as usual.
>> >>
>> >> We would add new directory /etc/sssd/conf.d/
>> >> and its content would be following:
>> >> - README file that informs what the direcotory
>> >> is for.
>> >> - any number of files ending with .conf extension
>> >>that would contain additional configuration for
>> >>SSSD. These files would have higher prioriry
>> >>than the /etc/sssd/sssd.conf , meaning if
>> >>the same option is present in these files
>> >>it will override the /etc/sssd/sssd.conf
>> >>value.
>> >>
>> >> SSSD would automatically pick up files ending
>> >> in .conf from that direcory and use them. In
>> >> order to disable the config file, the admin will
>> >> have to rename the file ending (for example
>> >> .conf.disabled). This way, we do not need to
>> >> inspect the snippets for any special options
>> >> like 'enable_this_snippet = true' which would
>> >> just complicate the processing.
>> >>
>> >> In order for SSSD to load a configuration, all
>> >> the config snippets in /etc/sssd/conf.d/ and
>> >> /etc/sssd/sssd.conf must in combination
>> >> result in valid configuration. If there is an
>> >> error in processing one of the config files,
>> >> the whole configuration loading will be
>> >> unsuccessful and there will be no way to
>> >> skip problematic snippets (later we may add
>> >> a fallback config, but that is different issue).
>> >>
>> >> Of course sssd will have an cli option to
>> >> use alternative directory for snippets (similar
>> >> to what we use for config file now).
>> >>
>> >> Could it be implemented this way?
>> >> Any comments are welcome.
>> >
>> > How will this interact with the python configuration API? If some
>> > application will use the API to change the configuration the result will
>> > be written to sssd.conf. But if there is a config snippets which sets
>> > the same value the change via the config API is overridden which I guess
>> > is unexpected by the application using the config API.
>> >
>> > bye,
>> > Sumit
>> >
>> 
>> Good question. I was not thinking about this. We
>> could change the config API to actually write to its
>> own snippet that will be always applied last.
>> 
>> OTOH some admins may want to really override whatever
>> other applications may set up using python config API.
>> 
>> If we decide to apply snippets in alphabetical
>> order as Lukas suggested, we can give the snippet to which
>> python API writes a name for example 990_pyapi.conf
>> and if the admin decides to create snippet with starting
>> number lower than 990 it will have lower pririty than
>> python config API.
>> 
>> We should document such behavior in the README file
>> that will be placed in the /etc/sssd/conf.d directory.
>
>If you make sssd.conf always win over snippets then you do not need to
>change anything, though.
>
If you make sssd.conf always win then you will not be able to override
there anything. An int's not usual in other projects.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] URI in HBAC - design page

[Lukáš Hellebrandt]

I created a design page for the feature:

http://www.freeipa.org/page/URI-based-HBAC-design

-- 
Lukas Hellebrandt
Associate Quality Engineer
lhell...@redhat.com
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Lukas Slebodnik]

On (23/03/16 12:16), Michal Židek wrote:
>On 03/23/2016 11:04 AM, Michal Židek wrote:
>>On 03/22/2016 09:16 PM, Jakub Hrozek wrote:
>>>
On 22 Mar 2016, at 14:46, Lukas Slebodnik  wrote:
>>
>>SSSD would automatically pick up files ending
>>in .conf from that direcory and use them. In
>>order to disable the config file, the admin will
>>have to rename the file ending (for example
>>.conf.disabled). This way, we do not need to
>>inspect the snippets for any special options
>>like 'enable_this_snippet = true' which would
>>just complicate the processing.
>>
Another, way how to ignore snippet is to ignore
any file which start with dot ".".
"hiddent files". It would avoid adding suffix to every file.

BTW logrotate and crond do the same
/etc/logrotate.d/
/etc/cron.d/

>>>
>>>+1 I would expect any decent software to ignore hidden files. The
>>>question is, should sssd ignore them or should libini_config?
>>
>>libini does not provide API to merge config objects in bulk, so we need
>>to do it file by file and than call merge function on each.
>>
>>We could add function to libini that reads all config files
>>in a directory in alphabetical order and merges them in that order.
>>Such function could have a parameter to ignore hidden files.
>>With current libini API we would need to do it in SSSD.
>>
>>Should we add such function to libini?
>
>I am taking it back, there is a function in libini
>that merges files in bulk. So no need to add a new one.
>The function does all we need.
>
>Sorry for incorrect info.
>
Of course :-)

https://pagure.io/gssproxy/f2385558dd9542ca0a88f3386aa5833a23e5eb8b

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Michal Židek]

On 03/23/2016 11:04 AM, Michal Židek wrote:

On 03/22/2016 09:16 PM, Jakub Hrozek wrote:



On 22 Mar 2016, at 14:46, Lukas Slebodnik  wrote:


SSSD would automatically pick up files ending
in .conf from that direcory and use them. In
order to disable the config file, the admin will
have to rename the file ending (for example
.conf.disabled). This way, we do not need to
inspect the snippets for any special options
like 'enable_this_snippet = true' which would
just complicate the processing.


Another, way how to ignore snippet is to ignore
any file which start with dot ".".
"hiddent files". It would avoid adding suffix to every file.

BTW logrotate and crond do the same
/etc/logrotate.d/
/etc/cron.d/



+1 I would expect any decent software to ignore hidden files. The
question is, should sssd ignore them or should libini_config?


libini does not provide API to merge config objects in bulk, so we need
to do it file by file and than call merge function on each.

We could add function to libini that reads all config files
in a directory in alphabetical order and merges them in that order.
Such function could have a parameter to ignore hidden files.
With current libini API we would need to do it in SSSD.

Should we add such function to libini?


I am taking it back, there is a function in libini
that merges files in bulk. So no need to add a new one.
The function does all we need.

Sorry for incorrect info.

Michal
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] UTIL: Allow to append new line in sss_vdebug_fn

[Lukas Slebodnik]

On (10/03/16 13:15), Lukas Slebodnik wrote:
>On (10/03/16 13:10), Pavel Březina wrote:
>>On 03/10/2016 01:05 PM, Lukas Slebodnik wrote:
>>>On (10/03/16 12:58), Pavel Březina wrote:
On 03/09/2016 05:39 PM, Lukas Slebodnik wrote:
>ehlo,
>
>I read log files from latest 1.13 today and it was a small challenge
>due to missing line feed after some ldb messages.
>
>LS

Can we check if new line is present instead of always appending it?
>>>We do not always append it. We append it only in libldb.
>>>Because libldb is inconsistent. I sent patch to samba upstream
>>
>>That's why I ask for it.
>>
>>>but we cannot reply on specific version of libldb.
>>>In future, we can conditionally remove this flag if we detect there is
>>>recent enough libldb.
>>>
>>>We could append it but it would be additional slowdown in debug messages
>>>even in other parts of code which already have line feed in message
>>>(sssd debug messages, libhbac ...)
>>
>>It doesn't need to be on sssd debug, but only in ldb_debug_messages. I don't
>>think that if (fmt[strlen-1] '= '\n') is such a slow down, but I don'ẗ
>>insist.
>>
>strlen is time consuming.
>
>But if there will be more developers who like idea with strlen.
>Then we might consider to add it.
>
>>Ack to the patches then.
>>
>Thank you for review.
>
>I will wait few days
>for other opinions before pushing.
>
http://sssd-ci.duckdns.org/logs/job/39/88/summary.html

master:
* 7c30eade4ae794ed809845f2ef70dda849b6e7c9
* 558ec7d717735bb16c210c675c2cc5bee1da4576

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Michal Židek]

On 03/23/2016 11:02 AM, Lukas Slebodnik wrote:

On (23/03/16 10:53), Michal Židek wrote:

On 03/22/2016 09:10 PM, Jakub Hrozek wrote:



On 22 Mar 2016, at 12:29, Michal Židek  wrote:

Hi,

I would like to write a patch that will
allow SSSD to use the config file merging
feature from libini. But first I would like
to ask developers for their opinions on how
this  should be implemented.

My idea was that it could work like this.

The current /etc/sssd/sssd.conf would work
as usual.

We would add new directory /etc/sssd/conf.d/
and its content would be following:
- README file that informs what the direcotory
is for.
- any number of files ending with .conf extension
  that would contain additional configuration for
  SSSD. These files would have higher prioriry
  than the /etc/sssd/sssd.conf , meaning if
  the same option is present in these files
  it will override the /etc/sssd/sssd.conf
  value.

SSSD would automatically pick up files ending
in .conf from that direcory and use them. In
order to disable the config file, the admin will
have to rename the file ending (for example
.conf.disabled). This way, we do not need to
inspect the snippets for any special options
like 'enable_this_snippet = true' which would
just complicate the processing.

In order for SSSD to load a configuration, all
the config snippets in /etc/sssd/conf.d/ and
/etc/sssd/sssd.conf must in combination
result in valid configuration. If there is an
error in processing one of the config files,
the whole configuration loading will be
unsuccessful and there will be no way to
skip problematic snippets (later we may add
a fallback config, but that is different issue).



I guess for now this is acceptable, because the snippet is similar to editing 
the conf file (as Sumit noted) but it would be nice to at least note in the 
design page of the fallback config and this merging feature that they are 
interconnected..



Of course sssd will have an cli option to
use alternative directory for snippets (similar
to what we use for config file now).



Does anyone use the -c option actually (maybe except tests?)

If not, do we need to add a new option? I'm cautious here, because any new 
option needs to be supported (almost) forever..



Ok, we can do it without an option and add the option if there is a
demand for it. It will be easy change (cca 15 lines + doc).


Could it be implemented this way?
Any comments are welcome.



Would the admin be able to discover from debug message where an option comes 
from (conf file or snippet) ? Does libini provide that info?


Libini does not provide that info. I was thinking about creating a
tool (not sure if Python or C, I think Python will be easier for
this) that would be called sss_showconf and it would print
the actual configuration to stdout. With

$ sss_showconf -s
or
$ sss_showconf --show-source


or sssctl config :-)


That is another option :)



It looks like we implement config file merging on two places
python-sssdconfig and libini. I do not like it but we probably cannot avoid it.

Therefore it would be good to add test that both implementation will result
in the same "result". I would prefer to do it with libini_config
because sssd daemon will use it.
* dump config with libini_config (ini_config_save_as)
* dump config from python-sssdconfig + import with libini and
   save ini_config_save_as.


Yes, such test makes sense.

But Jakub wanted to know if there is a way to provide
info about where the option comes from (sssd.conf or which snippet).
libini_config currently can not do that and I think it will be easier
to do it in Python.



The result shoudl be equivalent. IIRC libini will not dump duplicates
because we use last match win.

Or try to use different test.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Michal Židek]

On 03/22/2016 09:16 PM, Jakub Hrozek wrote:



On 22 Mar 2016, at 14:46, Lukas Slebodnik  wrote:


SSSD would automatically pick up files ending
in .conf from that direcory and use them. In
order to disable the config file, the admin will
have to rename the file ending (for example
.conf.disabled). This way, we do not need to
inspect the snippets for any special options
like 'enable_this_snippet = true' which would
just complicate the processing.


Another, way how to ignore snippet is to ignore
any file which start with dot ".".
"hiddent files". It would avoid adding suffix to every file.

BTW logrotate and crond do the same
/etc/logrotate.d/
/etc/cron.d/



+1 I would expect any decent software to ignore hidden files. The question is, 
should sssd ignore them or should libini_config?


libini does not provide API to merge config objects in bulk, so we need 
to do it file by file and than call merge function on each.


We could add function to libini that reads all config files
in a directory in alphabetical order and merges them in that order.
Such function could have a parameter to ignore hidden files.
With current libini API we would need to do it in SSSD.

Should we add such function to libini?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Lukas Slebodnik]

On (23/03/16 10:53), Michal Židek wrote:
>On 03/22/2016 09:10 PM, Jakub Hrozek wrote:
>>
>>>On 22 Mar 2016, at 12:29, Michal Židek  wrote:
>>>
>>>Hi,
>>>
>>>I would like to write a patch that will
>>>allow SSSD to use the config file merging
>>>feature from libini. But first I would like
>>>to ask developers for their opinions on how
>>>this  should be implemented.
>>>
>>>My idea was that it could work like this.
>>>
>>>The current /etc/sssd/sssd.conf would work
>>>as usual.
>>>
>>>We would add new directory /etc/sssd/conf.d/
>>>and its content would be following:
>>>- README file that informs what the direcotory
>>>is for.
>>>- any number of files ending with .conf extension
>>>  that would contain additional configuration for
>>>  SSSD. These files would have higher prioriry
>>>  than the /etc/sssd/sssd.conf , meaning if
>>>  the same option is present in these files
>>>  it will override the /etc/sssd/sssd.conf
>>>  value.
>>>
>>>SSSD would automatically pick up files ending
>>>in .conf from that direcory and use them. In
>>>order to disable the config file, the admin will
>>>have to rename the file ending (for example
>>>.conf.disabled). This way, we do not need to
>>>inspect the snippets for any special options
>>>like 'enable_this_snippet = true' which would
>>>just complicate the processing.
>>>
>>>In order for SSSD to load a configuration, all
>>>the config snippets in /etc/sssd/conf.d/ and
>>>/etc/sssd/sssd.conf must in combination
>>>result in valid configuration. If there is an
>>>error in processing one of the config files,
>>>the whole configuration loading will be
>>>unsuccessful and there will be no way to
>>>skip problematic snippets (later we may add
>>>a fallback config, but that is different issue).
>>>
>>
>>I guess for now this is acceptable, because the snippet is similar to editing 
>>the conf file (as Sumit noted) but it would be nice to at least note in the 
>>design page of the fallback config and this merging feature that they are 
>>interconnected..
>>
>>
>>>Of course sssd will have an cli option to
>>>use alternative directory for snippets (similar
>>>to what we use for config file now).
>>>
>>
>>Does anyone use the -c option actually (maybe except tests?)
>>
>>If not, do we need to add a new option? I'm cautious here, because any new 
>>option needs to be supported (almost) forever..
>>
>
>Ok, we can do it without an option and add the option if there is a
>demand for it. It will be easy change (cca 15 lines + doc).
>
>>>Could it be implemented this way?
>>>Any comments are welcome.
>>>
>>
>>Would the admin be able to discover from debug message where an option comes 
>>from (conf file or snippet) ? Does libini provide that info?
>
>Libini does not provide that info. I was thinking about creating a
>tool (not sure if Python or C, I think Python will be easier for
>this) that would be called sss_showconf and it would print
>the actual configuration to stdout. With
>
>$ sss_showconf -s
>or
>$ sss_showconf --show-source
>
or sssctl config :-)

It looks like we implement config file merging on two places
python-sssdconfig and libini. I do not like it but we probably cannot avoid it.

Therefore it would be good to add test that both implementation will result
in the same "result". I would prefer to do it with libini_config
because sssd daemon will use it.
* dump config with libini_config (ini_config_save_as)
* dump config from python-sssdconfig + import with libini and
  save ini_config_save_as.

The result shoudl be equivalent. IIRC libini will not dump duplicates
because we use last match win.

Or try to use different test.

LS
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Michal Židek]

On 03/22/2016 09:10 PM, Jakub Hrozek wrote:



On 22 Mar 2016, at 12:29, Michal Židek  wrote:

Hi,

I would like to write a patch that will
allow SSSD to use the config file merging
feature from libini. But first I would like
to ask developers for their opinions on how
this  should be implemented.

My idea was that it could work like this.

The current /etc/sssd/sssd.conf would work
as usual.

We would add new directory /etc/sssd/conf.d/
and its content would be following:
- README file that informs what the direcotory
is for.
- any number of files ending with .conf extension
  that would contain additional configuration for
  SSSD. These files would have higher prioriry
  than the /etc/sssd/sssd.conf , meaning if
  the same option is present in these files
  it will override the /etc/sssd/sssd.conf
  value.

SSSD would automatically pick up files ending
in .conf from that direcory and use them. In
order to disable the config file, the admin will
have to rename the file ending (for example
.conf.disabled). This way, we do not need to
inspect the snippets for any special options
like 'enable_this_snippet = true' which would
just complicate the processing.

In order for SSSD to load a configuration, all
the config snippets in /etc/sssd/conf.d/ and
/etc/sssd/sssd.conf must in combination
result in valid configuration. If there is an
error in processing one of the config files,
the whole configuration loading will be
unsuccessful and there will be no way to
skip problematic snippets (later we may add
a fallback config, but that is different issue).



I guess for now this is acceptable, because the snippet is similar to editing 
the conf file (as Sumit noted) but it would be nice to at least note in the 
design page of the fallback config and this merging feature that they are 
interconnected..



Of course sssd will have an cli option to
use alternative directory for snippets (similar
to what we use for config file now).



Does anyone use the -c option actually (maybe except tests?)

If not, do we need to add a new option? I'm cautious here, because any new 
option needs to be supported (almost) forever..



Ok, we can do it without an option and add the option if there is a
demand for it. It will be easy change (cca 15 lines + doc).


Could it be implemented this way?
Any comments are welcome.



Would the admin be able to discover from debug message where an option comes 
from (conf file or snippet) ? Does libini provide that info?


Libini does not provide that info. I was thinking about creating a
tool (not sure if Python or C, I think Python will be easier for
this) that would be called sss_showconf and it would print
the actual configuration to stdout. With

$ sss_showconf -s
or
$ sss_showconf --show-source

it would label each line with prefix that would say, where this line 
comes from. I think this would be easy to do in Python. We could

start working on the tool once the merging is finished.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Design document - sssctl

[Pavel Reichl]

On 03/23/2016 03:42 AM, Stephen Gallagher wrote:

On 03/22/2016 07:42 AM, Pavel Reichl wrote:

Hello,

Pavel Březina and I have prepared the 1st draft of design document. We mostly
focused on summing up its future functionality and its interface.

Please comment if you miss some essential functionality or if you would prefer
some different interface.

Thanks!

https://fedorahosted.org/sssd/wiki/DesignDocs/sssctl




"This tool will communicate with InfoPipe responder through its public D-Bus
interface."

Presumably this means that the sssctl CLI will be a thin presentation-layer shim
over the D-BUS API (meaning that the CLI would contain no logic outside of
argument processing)? This would be the ideal case, since it would also allow
plugging in other front-ends for this (such as Cockpit).



Yes, this is current plan.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: NSS responder should negatively cache local users for a longer time

[Petr Cech]

On 03/22/2016 08:58 PM, Jakub Hrozek wrote:

>On 22 Mar 2016, at 20:35, Simo Sorce  wrote:
>
>On Sun, 2016-03-20 at 21:28 +0100, Jakub Hrozek wrote:

>>>On 16 Mar 2016, at 13:45, Petr Cech  wrote:
>>>
>>>Hi,
>>>
>>>I will work on $subject [1] and I have discussed this topic with

>>Jakub a week ago. There are some open questions, so I will be glad if
>>you say your opinion.

>>>
>>>There could be heavy traffic between SSSD client and server coused

>>by local users. We need longer timeout in negative cache for local
>>users.

>>>
>>>Questions are:
>>>
>>>a) Is better hack negative_cache or responder?
>>>

>>
>>I would say that this solution should be reusable by other responders
>>like ifp as well. Therefore I would say either negcache (but there I
>>would say a new function, not extend the generic one) or a reusable
>>function in responder/common.
>>

>>>b) Is better set timeout = 0 (it means permanently in negative

>>cache) or set something really big like 12 hours?

>>>* We couldn't remove local users from permanent negative cache (only

>>by restart).

>>>* Is timeout = 12 hours means some kind of network peak?
>>>

>>
>>I guess some long timeout is slightly more flexible for cases where
>>the admin would add the local user to LDAP groups. A couple of hours
>>should be enough, as long as the negative entries are cached across
>>all clients, then if a single client queries the server once a couple
>>of hours, that should not bring the server down..

>
>12 hours is a lot, if you made a mistake and want to correct it (eg some
>software install created a local user by mistake that the admin removes
>because they want it in LDAP) you do not want to wait for hours.
>
>The main issue with the initgroups calls is not the load o the server,
>but the slowdown of the call which ends up contacting a network (slower)
>store for a local user.
>
>I would use a smaller timeout here, like 10 minutes, but potentially add
>a midway cache check, like we do for positive results. so after 5 min,

The admins actually complained about a load on the servers from users like 
postfix IIRC. That's the reason I suggested a long timeout.

But I guess even a short timeout would work, but I would vote for defaulting to 
the entry_cache_timeout then (with a midway check perhaps..). I guess the 
biggest gain for admins would be to be able to specify a timeout for all passwd 
users at once and avoid putting them one-by-one into filter_users/filter_groups.


>if a request comes in we do an asynchronous positive check to see if we
>still need to extend the negative cache for another 10 minutes. If the
>local user disappeared we drop the negative cache.
>

This is a good idea.


>>btw do you think this feature should be enabled or disabled by
>>default?

>
>Good q. how often does this problem happen ?
>

>>>c) Is it enough to do it only for initgroups?

>>
>>Hmm, not sure, by convention initgroups is the most frequent example
>>(maybe there will be some users of the new libc merge feature), but at
>>the same time special-casing initgroups doesn't gain much..
>>
>>I guess I would personally do this for all lookups that the NSS
>>interface can do (by name, by id) but I'm not 100% for or against
>>either..

>
>I agree, keep it generic.
>
>Simo.
>
>--
>Simo Sorce * Red Hat, Inc * New York

Thank you for taking the time to review the design!


Hi,

I would like to recap the conclusions:


a) Is better hack negative_cache or responder?

  Jakub suggested hack new functions into negcache
  or new reusable functions in responder/common.


b) Is better set timeout = 0 (it means permanently
in negative cache) or set something really big
like 12 hours?

  Discussion crystallized into smaller values.
  For example set
  entry_negative_timeout = entry_cache_timeout

  And maybe adding midway check.
  Note: I am not sure how midway check exactly works.


c) Is it enough to do it only for initgroups?

  We do such negatively cache of local users for all
  lookups that the NSS interface can do.


And there is one new question:

Do you think this feature should be enabled
or disabled by default?

  My suggestion is add option for enabling/disabling
  and maybe another option for setting
  local_entry_negative_timeout (default entry_cache_timeout)

  If this is better behaviour for all our users, it should
  be enabled by default. If it is only feature for some
  of them, it should be disabling.
  I have no concrete opinion yet.


Thank you both for discussion.

Regards

--
Petr^4 Čech
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: Config file merging in SSSD

[Lukas Slebodnik]

On (22/03/16 15:43), Simo Sorce wrote:
>On Tue, 2016-03-22 at 15:15 +0100, Michal Židek wrote:
>> On 03/22/2016 02:46 PM, Lukas Slebodnik wrote:
>> > On (22/03/16 14:30), Michal Židek wrote:
>> >> On 03/22/2016 12:29 PM, Michal Židek wrote:
>> >>> Hi,
>> >>>
>> >>> I would like to write a patch that will
>> >>> allow SSSD to use the config file merging
>> >>> feature from libini. But first I would like
>> >>> to ask developers for their opinions on how
>> >>> this  should be implemented.
>> >>>
>> >>> My idea was that it could work like this.
>> >>>
>> >>> The current /etc/sssd/sssd.conf would work
>> >>> as usual.
>> >>>
>> >>> We would add new directory /etc/sssd/conf.d/
>> >>> and its content would be following:
>> >>> - README file that informs what the direcotory
>> >>> is for.
>> >>> - any number of files ending with .conf extension
>> >>>that would contain additional configuration for
>> >>>SSSD. These files would have higher prioriry
>> >>>than the /etc/sssd/sssd.conf , meaning if
>> >>>the same option is present in these files
>> >>>it will override the /etc/sssd/sssd.conf
>> >>>value.
>
>What's the rational behind 'snippets vs main conf' makes snippet a
>winner ?
>
We already have a policy "the last one win" even in current
version of sssd.conf. QE use it very often in testing.
So it would not ne a huge change.

We can change it if there is a use case.

>> >>> SSSD would automatically pick up files ending
>> >>> in .conf from that direcory and use them. In
>> >>> order to disable the config file, the admin will
>> >>> have to rename the file ending (for example
>> >>> .conf.disabled). This way, we do not need to
>> >>> inspect the snippets for any special options
>> >>> like 'enable_this_snippet = true' which would
>> >>> just complicate the processing.
>> >>>
>> > Another, way how to ignore snippet is to ignore
>> > any file which start with dot ".".
>> > "hiddent files". It would avoid adding suffix to every file.
>> >
>> > BTW logrotate and crond do the same
>> > /etc/logrotate.d/
>> > /etc/cron.d/
>
>logrotate also adds .old in var/log, in general you should not
>have .conf files in conf.d if you do not want them used.
>
>Ignoring .something files should always be true as it makes it hard to
>understand what is going on if you just do ls and can't see a snippet.
>
>> >>> In order for SSSD to load a configuration, all
>> >>> the config snippets in /etc/sssd/conf.d/ and
>> >>> /etc/sssd/sssd.conf must in combination
>> >>> result in valid configuration. If there is an
>> >>> error in processing one of the config files,
>> >>> the whole configuration loading will be
>> >>> unsuccessful and there will be no way to
>> >>> skip problematic snippets (later we may add
>> >>> a fallback config, but that is different issue).
>> >>>
>> > I do not remember the lib_iniconfig API.
>> > But it might be already solved there.
>
>It is, question though: do you want to allow to override anything in a
>snippet ?
>At some point I had the idea the snippet would only allow to
>define/override a single domain and the snippet name would have to be
>domain_NAME.conf, but that may be overly restrictive given today we are
>adding stuff like secrets which has a different way to deal with section
>names.
>
Adding domains as a separate snippets is "kind of" a different use case.
Because you also amend a "domains" option in the "[sssd]" section.

It's solvable. Just add option "domain_enabled" into domain secition.
Do we want to do it or do we want to keep it simple (current state)?


>> >>> Of course sssd will have an cli option to
>> >>> use alternative directory for snippets (similar
>> >>> to what we use for config file now).
>> >>>
>> >>> Could it be implemented this way?
>> >>> Any comments are welcome.
>> >>>
>> >>> Michal
>> >>
>> >> I forgot to notice one more thing.
>> >>
>> >> There can be conflicts in configuration
>> >> among the snippets in /etc/sssd/conf.d directory.
>> >> IMO the best thing to do in this situation is
>> >> to end with an error. Defining priorities among
>> >> the snippets could cause situations where people
>> >> will write difficult to debug configurations.
>> >>
>> > In most, project you use numbers in the begining of files.
>> > It guarantee the order and last value win. It's current behaviour
>> > in sssd.conf.
>> 
>> So should we rely on alphabetical order? I personally
>> think it will add a little chaos to the configuration
>> but maybe not.
>> 
>> If we decide to rely on alphabetical order it may
>> be nice to have a tool that will print the actual
>> configuration in stdout skipping all the overriden options.
>> This may be good for troubleshooting SSSD.
>
>We need an order so that admins can understand why a conflict happened,
>alphabetical is as good as any.
>
+1

BTW logrotate was not ideal example because they do not care about order.

>> >
>> > We can also provide something like "systemctl cat name.service"
>> > or we can even save parsed/merged.. 

[SSSD] Re: Design document - sssctl

[Petr Cech]

On 03/22/2016 12:42 PM, Pavel Reichl wrote:

Hello,

Pavel Březina and I have prepared the 1st draft of design document. We
mostly focused on summing up its future functionality and its interface.

Please comment if you miss some essential functionality or if you would
prefer some different interface.

Thanks!

https://fedorahosted.org/sssd/wiki/DesignDocs/sssctl


Hi,

I would like to give a litle comment to:

"""
Uses cases

Removing cache - Removing SSSD cache seems to be often misused act done 
by administrators as there are few real needs for that. Nevertheless, if 
administrator decides to remove the cache it would be better to do this 
using the tool instead of crude removing directories that might contain 
other useful data and could lead to serious problems. Q: Is this what 
was requested as 'force reload'? Q: Should this rather be part of 
sss_cache tool?

"""

There are two slightly related tickets:
[1] sss_cache: invalidate sudo rules
[2] Trigger full refresh of sudo rules on desire

We have patch on list (tests remains to finish) for the first. The 
second is on my opinion candidate for sssctl. Reason is that is not only 
action of sss cache to obtain new updated data.


Regards

--
Petr^4 Čech

[1] https://fedorahosted.org/sssd/ticket/2081
[2] https://fedorahosted.org/sssd/ticket/2884
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org