[SSSD] Re: [PATCH] Add winbind idmap plugin

2016-06-24 Thread Sumit Bose
On Tue, Jun 21, 2016 at 09:01:05AM -0400, Stephen Gallagher wrote:
> On 06/20/2016 05:48 AM, Sumit Bose wrote:
> > On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
> >> BTW we can add Requires/Recommends into pacakge sssd-ad for this 
> >> sub-pacakge.
> >> So it will be installed by default.
> > 
> > I think this is not needed. It is only needed for samba, not on an
> > general AD client. And even with samba people might want to select
> > between the idmap plugin and SSSD's libwbclient implementation.
> > 
> 
> What is the advantage of one over the other? This seems to me like one of 
> those
> situations where we have two solutions that people aren't realistically going 
> to
> know which to pick. In that case, we should probably consider using 
> dependencies
> to select the preferred one if neither is currently present.
> 
> We can do this by having both packages add a virtual `Provides:
> sssd-samba-id-mapping` and have sssd-ad add
> 
> Recommends: sssd-samba-id-mapping
> Suggests: sssd-libwbclient
> 
> (or Suggests: sssd-winbind-idmap whichever one is more feature-complete)
> 
> 
> What this will do is pull in the Suggests dependency unless the other one was
> specified manually (or already exists on the system) to satisfy the virtual
> Provides.
> 

But as said the packages are not needed on a general AD client. Only if
Samba and/or Winbind are used either sssd-libwbclient (winbind not
running) or sssd-winbind-idmap (winbind is running) are needed. So for
the time being I would prefer to not pull in one of the package
automatically to keep e.g. the container images small.

Please note that we are working with some Samba developers on how winbind
and SSSD can work better together in future. We have just started so it
is currently not clear what will be the results but of course it should
be easy for a user to configure a Samba server without having to think
about which package to install in which situation.

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


[SSSD] Re: [PATCH] confdb: Check for config file errors on sssd startup

2016-06-24 Thread Michal Židek

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?= 
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 
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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Michal Židek

On 06/24/2016 02:52 PM, Lukas Slebodnik wrote:

On (24/06/16 13:51), Lukas Slebodnik wrote:

On (24/06/16 13:49), Lukas Slebodnik wrote:

On (24/06/16 10:29), Michal Židek wrote:

On 06/24/2016 09:57 AM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:

On 06/23/2016 09:40 PM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:

On (23/06/16 18:59), Michal Židek wrote:

On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:

On (23/06/16 18:24), Michal Židek wrote:

On 06/23/2016 05:50 PM, Michal Židek wrote:

Lukas,

did you have some local patches related to
this patch, that you did not send here?

Because I can not apply the patch on current
master and the changes in the context of
your patch seem like you did something with
default config file path?

Slightly modified version that applies on
current master is attached (the changes
are the same, but the context differs).

Michal



I added the missing access check for
snippets. If the snippets does not
pass the access check it is skipped
and a loud debug message is logged.

I attach it as separate patch for
easier review, but it should be squashed
before pushing. I also attach the first
patch for convenience.

Michal



   From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 22 Mar 2016 14:09:34 +0100
Subject: [PATCH] confdb: Make it possible to use config snippets

Resolves:
https://fedorahosted.org/sssd/ticket/2247

Signed-off-by: Lukas Slebodnik 
---

LGTM.

It isn't ACK because I was involved in development
and I didn;t test last version but code is fine fore me.


I just ran a quick test and the patch works for me and the code reads OK
as well. I will push after CI finishes.

I also opened:
   https://fedorahosted.org/sssd/ticket/3062
because the patch is missing documentation and
   https://fedorahosted.org/sssd/ticket/3063
to add at least a simple integration test.


Actually, can we get rid of this warning?

Error: COMPILER_WARNING:
sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' 
[-Wunused-variable]
# const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
# ^
#  214|   int ret;
#  215|   #ifdef HAVE_LIBINI_CONFIG_V1
#  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
#  217|   const char *sections[] = { ".*", NULL };
#  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3

Error: COMPILER_WARNING:
sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config'
sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' 
[-Wunused-variable]
# const char *sections[] = { ".*", NULL };
# ^
#  215|   #ifdef HAVE_LIBINI_CONFIG_V1
#  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
#  217|-> const char *sections[] = { ".*", NULL };
#  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
#  219|   uint32_t i = 0;
___


New patch attached.

Michal



The patch is fine, (so ack also from me), but can someone check if the
attached rebase on top of Pavel's sssctl is OK?




I did not check the differences between the rebased and
non-rebased version, but I just tried branch with sssctl
patches + this rebased patch + startup validation patches
and it worked for me.

But the CI failed for me with the snippets in place.
Will take a look at what is happening there.


It failed beccause it bas broken with libini_config 1.0 .. 1.2
and current CI machines still have libini_config 1.2
or 1.1 (el6)

Updated patch is attached.


Ups,

ENOPATCH

fixed in this version :-)


Previous version had an issue with SELinux and blocking
access to conf.d.

Hopefully last version is attached.

LS



Thanks, this version works for all CI machines.

Here is result of the CI:
http://sssd-ci.duckdns.org/logs/job/46/08/summary.html

There are two failures that are not related
to these patches (One is "device is busy" error during
mock and the other was that ldap server failed to start
for one set of tests).
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


[SSSD] Re: release notes for the 1.14 beta release

2016-06-24 Thread Lukas Slebodnik
On (23/06/16 22:21), Jakub Hrozek wrote:
>Hi,
>
>I prepared release notes for the beta release:
>https://fedorahosted.org/sssd/wiki/Releases/Notes-1.14.0beta
>I wanted to release the tarball today, but I would also like to push the
>secrets responder and wait for CI to finish..let's try tomorrow.
Do not forget to change location of rules for config validation

rpm -qf /usr/share/sssd/cfg_rules.ini
sssd-common-1.13.91-0.20160624.1443.git21e2677.snippet.fc24.x86_64

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


[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Lukas Slebodnik
On (24/06/16 13:51), Lukas Slebodnik wrote:
>On (24/06/16 13:49), Lukas Slebodnik wrote:
>>On (24/06/16 10:29), Michal Židek wrote:
>>>On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
 On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
 > On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
 > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
 > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
 > > > > On (23/06/16 18:59), Michal Židek wrote:
 > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
 > > > > > > On (23/06/16 18:24), Michal Židek wrote:
 > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote:
 > > > > > > > > Lukas,
 > > > > > > > > 
 > > > > > > > > did you have some local patches related to
 > > > > > > > > this patch, that you did not send here?
 > > > > > > > > 
 > > > > > > > > Because I can not apply the patch on current
 > > > > > > > > master and the changes in the context of
 > > > > > > > > your patch seem like you did something with
 > > > > > > > > default config file path?
 > > > > > > > > 
 > > > > > > > > Slightly modified version that applies on
 > > > > > > > > current master is attached (the changes
 > > > > > > > > are the same, but the context differs).
 > > > > > > > > 
 > > > > > > > > Michal
 > > > > > > > > 
 > > > > > > > 
 > > > > > > > I added the missing access check for
 > > > > > > > snippets. If the snippets does not
 > > > > > > > pass the access check it is skipped
 > > > > > > > and a loud debug message is logged.
 > > > > > > > 
 > > > > > > > I attach it as separate patch for
 > > > > > > > easier review, but it should be squashed
 > > > > > > > before pushing. I also attach the first
 > > > > > > > patch for convenience.
 > > > > > > > 
 > > > > > > > Michal
 > > > > > > 
 > > > > > > >   From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 
 > > > > > > > 00:00:00 2001
 > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
 > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100
 > > > > > > > Subject: [PATCH] confdb: Make it possible to use config 
 > > > > > > > snippets
 > > > > > > > 
 > > > > > > > Resolves:
 > > > > > > > https://fedorahosted.org/sssd/ticket/2247
 > > > > > > > 
 > > > > > > > Signed-off-by: Lukas Slebodnik 
 > > > > > > > ---
 > > > > LGTM.
 > > > > 
 > > > > It isn't ACK because I was involved in development
 > > > > and I didn;t test last version but code is fine fore me.
 > > > 
 > > > I just ran a quick test and the patch works for me and the code 
 > > > reads OK
 > > > as well. I will push after CI finishes.
 > > > 
 > > > I also opened:
 > > >   https://fedorahosted.org/sssd/ticket/3062
 > > > because the patch is missing documentation and
 > > >   https://fedorahosted.org/sssd/ticket/3063
 > > > to add at least a simple integration test.
 > > 
 > > Actually, can we get rid of this warning?
 > > 
 > > Error: COMPILER_WARNING:
 > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 
 > > 'patterns' [-Wunused-variable]
 > > # const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
 > > # ^
 > > #  214|   int ret;
 > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
 > > #  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
 > > #  217|   const char *sections[] = { ".*", NULL };
 > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
 > > 
 > > Error: COMPILER_WARNING:
 > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 
 > > 'sss_ini_get_config'
 > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 
 > > 'sections' [-Wunused-variable]
 > > # const char *sections[] = { ".*", NULL };
 > > # ^
 > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
 > > #  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
 > > #  217|-> const char *sections[] = { ".*", NULL };
 > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
 > > #  219|   uint32_t i = 0;
 > > ___
 > 
 > New patch attached.
 > 
 > Michal
 > 
 
 The patch is fine, (so ack also from me), but can someone check if the
 attached rebase on top of Pavel's sssctl is OK?
 
 
>>>
>>>I did not check the differences between the rebased and
>>>non-rebased version, but I just tried branch with sssctl
>>>patches + this rebased patch + startup validation patches
>>>and it worked for me.
>>>
>>>But the CI failed for me with the snippets in place.
>>>Will take a look at what is happening there.
>>>
>>It failed beccause it bas broken with libini_config 1.0 .. 1.2
>>and current CI machines still have libini_config 1.2
>>or 1.1 (el6)

[SSSD] Re: [PATCH] Pavel's sssctl work

2016-06-24 Thread Lukas Slebodnik
On (24/06/16 13:07), Pavel Březina wrote:
>On 06/23/2016 08:14 PM, Jakub Hrozek wrote:
>> On Thu, Jun 23, 2016 at 01:51:53PM +0200, Jakub Hrozek wrote:
>> > > Change to src/lib/sifp/sss_simpleifp.exports is wrong.
>> > > Current version:
>> > > We cannot add new function to already released version
>> > > They shoudl be added to new version.
>> > > @see attached diff
>> > > 
>> > > diff --git a/src/lib/sifp/sss_simpleifp.exports 
>> > > b/src/lib/sifp/sss_simpleifp.exports
>> > > index 3d598fb..9fa6ecd 100644
>> > > --- a/src/lib/sifp/sss_simpleifp.exports
>> > > +++ b/src/lib/sifp/sss_simpleifp.exports
>> > > @@ -7,6 +7,7 @@ SSS_SIMPLEIFP_0.0 {
>> > >   sss_sifp_init_ex;
>> > >   sss_sifp_get_last_io_error_name;
>> > >   sss_sifp_get_last_io_error_message;
>> > > +sss_sifp_strerr;
>> > >   sss_sifp_create_message;
>> > >   sss_sifp_send_message;
>> > >   sss_sifp_send_message_ex;
>> > > @@ -14,7 +15,9 @@ SSS_SIMPLEIFP_0.0 {
>> > >   sss_sifp_fetch_all_attrs;
>> > >   sss_sifp_fetch_object;
>> > >   sss_sifp_invoke_list;
>> > > +sss_sifp_invoke_list_ex;
>> > >   sss_sifp_invoke_find;
>> > > +sss_sifp_invoke_find_ex;
>> > >   sss_sifp_find_attr_as_bool;
>> > >   sss_sifp_find_attr_as_int16;
>> > >   sss_sifp_find_attr_as_uint16;
>> > > 
>> > > 
>> > > I didn't check rest of patches and I do not plant to do it :-)
>> > > 
>> > > LS
>> > 
>> > > diff --git a/src/lib/sifp/sss_simpleifp.exports 
>> > > b/src/lib/sifp/sss_simpleifp.exports
>> > > index 9fa6ecd..f491092 100644
>> > > --- a/src/lib/sifp/sss_simpleifp.exports
>> > > +++ b/src/lib/sifp/sss_simpleifp.exports
>> > > @@ -7,7 +7,6 @@ SSS_SIMPLEIFP_0.0 {
>> > >   sss_sifp_init_ex;
>> > >   sss_sifp_get_last_io_error_name;
>> > >   sss_sifp_get_last_io_error_message;
>> > > -sss_sifp_strerr;
>> > >   sss_sifp_create_message;
>> > >   sss_sifp_send_message;
>> > >   sss_sifp_send_message_ex;
>> > > @@ -15,9 +14,7 @@ SSS_SIMPLEIFP_0.0 {
>> > >   sss_sifp_fetch_all_attrs;
>> > >   sss_sifp_fetch_object;
>> > >   sss_sifp_invoke_list;
>> > > -sss_sifp_invoke_list_ex;
>> > >   sss_sifp_invoke_find;
>> > > -sss_sifp_invoke_find_ex;
>> > >   sss_sifp_find_attr_as_bool;
>> > >   sss_sifp_find_attr_as_int16;
>> > >   sss_sifp_find_attr_as_uint16;
>> > > @@ -49,3 +46,11 @@ SSS_SIMPLEIFP_0.0 {
>> > >   local:
>> > >   *;
>> > >   };
>> > > +
>> > > +SSS_SIMPLEIFP_0.1 {
>> > > +# public functions
>> > > +global:
>> > > +sss_sifp_strerr;
>> > > +sss_sifp_invoke_list_ex;
>> > > +sss_sifp_invoke_find_ex;
>> > > +} SSS_SIMPLEIFP_0.0;
>
>^ shouldn't this line say 0.1?
No.

SSS_SIMPLEIFP_0.1 {
^
here is a name of new version
then there are newly added functions
 } SSS_SIMPLEIFP_0.0;
   ^
and here is version which is extended.
Usually, previous version.

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


[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Lukas Slebodnik
On (24/06/16 13:49), Lukas Slebodnik wrote:
>On (24/06/16 10:29), Michal Židek wrote:
>>On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
>>> On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
>>> > On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
>>> > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
>>> > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
>>> > > > > On (23/06/16 18:59), Michal Židek wrote:
>>> > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
>>> > > > > > > On (23/06/16 18:24), Michal Židek wrote:
>>> > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote:
>>> > > > > > > > > Lukas,
>>> > > > > > > > > 
>>> > > > > > > > > did you have some local patches related to
>>> > > > > > > > > this patch, that you did not send here?
>>> > > > > > > > > 
>>> > > > > > > > > Because I can not apply the patch on current
>>> > > > > > > > > master and the changes in the context of
>>> > > > > > > > > your patch seem like you did something with
>>> > > > > > > > > default config file path?
>>> > > > > > > > > 
>>> > > > > > > > > Slightly modified version that applies on
>>> > > > > > > > > current master is attached (the changes
>>> > > > > > > > > are the same, but the context differs).
>>> > > > > > > > > 
>>> > > > > > > > > Michal
>>> > > > > > > > > 
>>> > > > > > > > 
>>> > > > > > > > I added the missing access check for
>>> > > > > > > > snippets. If the snippets does not
>>> > > > > > > > pass the access check it is skipped
>>> > > > > > > > and a loud debug message is logged.
>>> > > > > > > > 
>>> > > > > > > > I attach it as separate patch for
>>> > > > > > > > easier review, but it should be squashed
>>> > > > > > > > before pushing. I also attach the first
>>> > > > > > > > patch for convenience.
>>> > > > > > > > 
>>> > > > > > > > Michal
>>> > > > > > > 
>>> > > > > > > >   From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 
>>> > > > > > > > 00:00:00 2001
>>> > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>>> > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100
>>> > > > > > > > Subject: [PATCH] confdb: Make it possible to use config 
>>> > > > > > > > snippets
>>> > > > > > > > 
>>> > > > > > > > Resolves:
>>> > > > > > > > https://fedorahosted.org/sssd/ticket/2247
>>> > > > > > > > 
>>> > > > > > > > Signed-off-by: Lukas Slebodnik 
>>> > > > > > > > ---
>>> > > > > LGTM.
>>> > > > > 
>>> > > > > It isn't ACK because I was involved in development
>>> > > > > and I didn;t test last version but code is fine fore me.
>>> > > > 
>>> > > > I just ran a quick test and the patch works for me and the code reads 
>>> > > > OK
>>> > > > as well. I will push after CI finishes.
>>> > > > 
>>> > > > I also opened:
>>> > > >   https://fedorahosted.org/sssd/ticket/3062
>>> > > > because the patch is missing documentation and
>>> > > >   https://fedorahosted.org/sssd/ticket/3063
>>> > > > to add at least a simple integration test.
>>> > > 
>>> > > Actually, can we get rid of this warning?
>>> > > 
>>> > > Error: COMPILER_WARNING:
>>> > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 
>>> > > 'patterns' [-Wunused-variable]
>>> > > # const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>>> > > # ^
>>> > > #  214|   int ret;
>>> > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
>>> > > #  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>>> > > #  217|   const char *sections[] = { ".*", NULL };
>>> > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
>>> > > 
>>> > > Error: COMPILER_WARNING:
>>> > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 
>>> > > 'sss_ini_get_config'
>>> > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 
>>> > > 'sections' [-Wunused-variable]
>>> > > # const char *sections[] = { ".*", NULL };
>>> > > # ^
>>> > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
>>> > > #  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>>> > > #  217|-> const char *sections[] = { ".*", NULL };
>>> > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
>>> > > #  219|   uint32_t i = 0;
>>> > > ___
>>> > 
>>> > New patch attached.
>>> > 
>>> > Michal
>>> > 
>>> 
>>> The patch is fine, (so ack also from me), but can someone check if the
>>> attached rebase on top of Pavel's sssctl is OK?
>>> 
>>> 
>>
>>I did not check the differences between the rebased and
>>non-rebased version, but I just tried branch with sssctl
>>patches + this rebased patch + startup validation patches
>>and it worked for me.
>>
>>But the CI failed for me with the snippets in place.
>>Will take a look at what is happening there.
>>
>It failed beccause it bas broken with libini_config 1.0 .. 1.2
>and current CI machines still have libini_config 1.2
>or 1.1 (el6)
>
>Updated patch is attached.
>
Ups,

ENOPATCH

fixed in this version :-)

LS
>From 5102ee3a9ad53a1cdde8086e0186fd8a7a6b4101 Mon Sep 17 00:00:00 2001
From: =?

[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Lukas Slebodnik
On (24/06/16 10:29), Michal Židek wrote:
>On 06/24/2016 09:57 AM, Jakub Hrozek wrote:
>> On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
>> > On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
>> > > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
>> > > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
>> > > > > On (23/06/16 18:59), Michal Židek wrote:
>> > > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
>> > > > > > > On (23/06/16 18:24), Michal Židek wrote:
>> > > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote:
>> > > > > > > > > Lukas,
>> > > > > > > > > 
>> > > > > > > > > did you have some local patches related to
>> > > > > > > > > this patch, that you did not send here?
>> > > > > > > > > 
>> > > > > > > > > Because I can not apply the patch on current
>> > > > > > > > > master and the changes in the context of
>> > > > > > > > > your patch seem like you did something with
>> > > > > > > > > default config file path?
>> > > > > > > > > 
>> > > > > > > > > Slightly modified version that applies on
>> > > > > > > > > current master is attached (the changes
>> > > > > > > > > are the same, but the context differs).
>> > > > > > > > > 
>> > > > > > > > > Michal
>> > > > > > > > > 
>> > > > > > > > 
>> > > > > > > > I added the missing access check for
>> > > > > > > > snippets. If the snippets does not
>> > > > > > > > pass the access check it is skipped
>> > > > > > > > and a loud debug message is logged.
>> > > > > > > > 
>> > > > > > > > I attach it as separate patch for
>> > > > > > > > easier review, but it should be squashed
>> > > > > > > > before pushing. I also attach the first
>> > > > > > > > patch for convenience.
>> > > > > > > > 
>> > > > > > > > Michal
>> > > > > > > 
>> > > > > > > >   From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 
>> > > > > > > > 00:00:00 2001
>> > > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
>> > > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100
>> > > > > > > > Subject: [PATCH] confdb: Make it possible to use config 
>> > > > > > > > snippets
>> > > > > > > > 
>> > > > > > > > Resolves:
>> > > > > > > > https://fedorahosted.org/sssd/ticket/2247
>> > > > > > > > 
>> > > > > > > > Signed-off-by: Lukas Slebodnik 
>> > > > > > > > ---
>> > > > > LGTM.
>> > > > > 
>> > > > > It isn't ACK because I was involved in development
>> > > > > and I didn;t test last version but code is fine fore me.
>> > > > 
>> > > > I just ran a quick test and the patch works for me and the code reads 
>> > > > OK
>> > > > as well. I will push after CI finishes.
>> > > > 
>> > > > I also opened:
>> > > >   https://fedorahosted.org/sssd/ticket/3062
>> > > > because the patch is missing documentation and
>> > > >   https://fedorahosted.org/sssd/ticket/3063
>> > > > to add at least a simple integration test.
>> > > 
>> > > Actually, can we get rid of this warning?
>> > > 
>> > > Error: COMPILER_WARNING:
>> > > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 
>> > > 'patterns' [-Wunused-variable]
>> > > # const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>> > > # ^
>> > > #  214|   int ret;
>> > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
>> > > #  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>> > > #  217|   const char *sections[] = { ".*", NULL };
>> > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
>> > > 
>> > > Error: COMPILER_WARNING:
>> > > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 
>> > > 'sss_ini_get_config'
>> > > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 
>> > > 'sections' [-Wunused-variable]
>> > > # const char *sections[] = { ".*", NULL };
>> > > # ^
>> > > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
>> > > #  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
>> > > #  217|-> const char *sections[] = { ".*", NULL };
>> > > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
>> > > #  219|   uint32_t i = 0;
>> > > ___
>> > 
>> > New patch attached.
>> > 
>> > Michal
>> > 
>> 
>> The patch is fine, (so ack also from me), but can someone check if the
>> attached rebase on top of Pavel's sssctl is OK?
>> 
>> 
>
>I did not check the differences between the rebased and
>non-rebased version, but I just tried branch with sssctl
>patches + this rebased patch + startup validation patches
>and it worked for me.
>
>But the CI failed for me with the snippets in place.
>Will take a look at what is happening there.
>
It failed beccause it bas broken with libini_config 1.0 .. 1.2
and current CI machines still have libini_config 1.2
or 1.1 (el6)

Updated patch is attached.

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


[SSSD] Re: [PATCH] Pavel's sssctl work

2016-06-24 Thread Pavel Březina

On 06/23/2016 08:14 PM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 01:51:53PM +0200, Jakub Hrozek wrote:

Change to src/lib/sifp/sss_simpleifp.exports is wrong.
Current version:
We cannot add new function to already released version
They shoudl be added to new version.
@see attached diff

diff --git a/src/lib/sifp/sss_simpleifp.exports 
b/src/lib/sifp/sss_simpleifp.exports
index 3d598fb..9fa6ecd 100644
--- a/src/lib/sifp/sss_simpleifp.exports
+++ b/src/lib/sifp/sss_simpleifp.exports
@@ -7,6 +7,7 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_init_ex;
  sss_sifp_get_last_io_error_name;
  sss_sifp_get_last_io_error_message;
+sss_sifp_strerr;
  sss_sifp_create_message;
  sss_sifp_send_message;
  sss_sifp_send_message_ex;
@@ -14,7 +15,9 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_fetch_all_attrs;
  sss_sifp_fetch_object;
  sss_sifp_invoke_list;
+sss_sifp_invoke_list_ex;
  sss_sifp_invoke_find;
+sss_sifp_invoke_find_ex;
  sss_sifp_find_attr_as_bool;
  sss_sifp_find_attr_as_int16;
  sss_sifp_find_attr_as_uint16;


I didn't check rest of patches and I do not plant to do it :-)

LS



diff --git a/src/lib/sifp/sss_simpleifp.exports 
b/src/lib/sifp/sss_simpleifp.exports
index 9fa6ecd..f491092 100644
--- a/src/lib/sifp/sss_simpleifp.exports
+++ b/src/lib/sifp/sss_simpleifp.exports
@@ -7,7 +7,6 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_init_ex;
  sss_sifp_get_last_io_error_name;
  sss_sifp_get_last_io_error_message;
-sss_sifp_strerr;
  sss_sifp_create_message;
  sss_sifp_send_message;
  sss_sifp_send_message_ex;
@@ -15,9 +14,7 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_fetch_all_attrs;
  sss_sifp_fetch_object;
  sss_sifp_invoke_list;
-sss_sifp_invoke_list_ex;
  sss_sifp_invoke_find;
-sss_sifp_invoke_find_ex;
  sss_sifp_find_attr_as_bool;
  sss_sifp_find_attr_as_int16;
  sss_sifp_find_attr_as_uint16;
@@ -49,3 +46,11 @@ SSS_SIMPLEIFP_0.0 {
  local:
  *;
  };
+
+SSS_SIMPLEIFP_0.1 {
+# public functions
+global:
+sss_sifp_strerr;
+sss_sifp_invoke_list_ex;
+sss_sifp_invoke_find_ex;
+} SSS_SIMPLEIFP_0.0;


^ shouldn't this line say 0.1?



Ugh, thanks, that's because the first version just broke the API, I
didn't the fix the version exports properly after fixing the API..


Attached are patches I intend to push once CI and Coverity finish.



___
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: [PATCH] Pavel's sssctl work

2016-06-24 Thread Michal Židek

On 06/23/2016 08:14 PM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 01:51:53PM +0200, Jakub Hrozek wrote:

Change to src/lib/sifp/sss_simpleifp.exports is wrong.
Current version:
We cannot add new function to already released version
They shoudl be added to new version.
@see attached diff

diff --git a/src/lib/sifp/sss_simpleifp.exports 
b/src/lib/sifp/sss_simpleifp.exports
index 3d598fb..9fa6ecd 100644
--- a/src/lib/sifp/sss_simpleifp.exports
+++ b/src/lib/sifp/sss_simpleifp.exports
@@ -7,6 +7,7 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_init_ex;
  sss_sifp_get_last_io_error_name;
  sss_sifp_get_last_io_error_message;
+sss_sifp_strerr;
  sss_sifp_create_message;
  sss_sifp_send_message;
  sss_sifp_send_message_ex;
@@ -14,7 +15,9 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_fetch_all_attrs;
  sss_sifp_fetch_object;
  sss_sifp_invoke_list;
+sss_sifp_invoke_list_ex;
  sss_sifp_invoke_find;
+sss_sifp_invoke_find_ex;
  sss_sifp_find_attr_as_bool;
  sss_sifp_find_attr_as_int16;
  sss_sifp_find_attr_as_uint16;


I didn't check rest of patches and I do not plant to do it :-)

LS



diff --git a/src/lib/sifp/sss_simpleifp.exports 
b/src/lib/sifp/sss_simpleifp.exports
index 9fa6ecd..f491092 100644
--- a/src/lib/sifp/sss_simpleifp.exports
+++ b/src/lib/sifp/sss_simpleifp.exports
@@ -7,7 +7,6 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_init_ex;
  sss_sifp_get_last_io_error_name;
  sss_sifp_get_last_io_error_message;
-sss_sifp_strerr;
  sss_sifp_create_message;
  sss_sifp_send_message;
  sss_sifp_send_message_ex;
@@ -15,9 +14,7 @@ SSS_SIMPLEIFP_0.0 {
  sss_sifp_fetch_all_attrs;
  sss_sifp_fetch_object;
  sss_sifp_invoke_list;
-sss_sifp_invoke_list_ex;
  sss_sifp_invoke_find;
-sss_sifp_invoke_find_ex;
  sss_sifp_find_attr_as_bool;
  sss_sifp_find_attr_as_int16;
  sss_sifp_find_attr_as_uint16;
@@ -49,3 +46,11 @@ SSS_SIMPLEIFP_0.0 {
  local:
  *;
  };
+
+SSS_SIMPLEIFP_0.1 {
+# public functions
+global:
+sss_sifp_strerr;
+sss_sifp_invoke_list_ex;
+sss_sifp_invoke_find_ex;
+} SSS_SIMPLEIFP_0.0;


Ugh, thanks, that's because the first version just broke the API, I
didn't the fix the version exports properly after fixing the API..


Attached are patches I intend to push once CI and Coverity finish.



When I have badly configured SSSD (for example
domains = nonexistent.com), I can not run sssctl
command at all. It complains that there are no properly
confiured domains even if all I wanted is

sssctl -?

to print help. Should I open a ticket to fix this
later? We may have more functionality that does
not necessarily need properly confiured SSSD in the
future.

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


[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Michal Židek

On 06/24/2016 09:57 AM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:

On 06/23/2016 09:40 PM, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:

On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:

On (23/06/16 18:59), Michal Židek wrote:

On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:

On (23/06/16 18:24), Michal Židek wrote:

On 06/23/2016 05:50 PM, Michal Židek wrote:

Lukas,

did you have some local patches related to
this patch, that you did not send here?

Because I can not apply the patch on current
master and the changes in the context of
your patch seem like you did something with
default config file path?

Slightly modified version that applies on
current master is attached (the changes
are the same, but the context differs).

Michal



I added the missing access check for
snippets. If the snippets does not
pass the access check it is skipped
and a loud debug message is logged.

I attach it as separate patch for
easier review, but it should be squashed
before pushing. I also attach the first
patch for convenience.

Michal



  From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 22 Mar 2016 14:09:34 +0100
Subject: [PATCH] confdb: Make it possible to use config snippets

Resolves:
https://fedorahosted.org/sssd/ticket/2247

Signed-off-by: Lukas Slebodnik 
---

LGTM.

It isn't ACK because I was involved in development
and I didn;t test last version but code is fine fore me.


I just ran a quick test and the patch works for me and the code reads OK
as well. I will push after CI finishes.

I also opened:
  https://fedorahosted.org/sssd/ticket/3062
because the patch is missing documentation and
  https://fedorahosted.org/sssd/ticket/3063
to add at least a simple integration test.


Actually, can we get rid of this warning?

Error: COMPILER_WARNING:
sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' 
[-Wunused-variable]
# const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
# ^
#  214|   int ret;
#  215|   #ifdef HAVE_LIBINI_CONFIG_V1
#  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
#  217|   const char *sections[] = { ".*", NULL };
#  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3

Error: COMPILER_WARNING:
sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 'sss_ini_get_config'
sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' 
[-Wunused-variable]
# const char *sections[] = { ".*", NULL };
# ^
#  215|   #ifdef HAVE_LIBINI_CONFIG_V1
#  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
#  217|-> const char *sections[] = { ".*", NULL };
#  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
#  219|   uint32_t i = 0;
___


New patch attached.

Michal



The patch is fine, (so ack also from me), but can someone check if the
attached rebase on top of Pavel's sssctl is OK?




I did not check the differences between the rebased and
non-rebased version, but I just tried branch with sssctl
patches + this rebased patch + startup validation patches
and it worked for me.

But the CI failed for me with the snippets in place.
Will take a look at what is happening there.

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


[SSSD] Re: [PATCH SET} A new Secrets service

2016-06-24 Thread Jakub Hrozek
On Thu, Jun 23, 2016 at 04:23:12PM -0400, Simo Sorce wrote:
> On Thu, 2016-06-23 at 21:36 +0200, Jakub Hrozek wrote:
> > On Thu, Apr 21, 2016 at 09:21:17AM +0200, Jakub Hrozek wrote:
> > > Given that Lukas says "http_parser can provide pkgconfig in future", I
> > > read his mail as a preference to keep the pkg-check test. And actually I
> > > agree, it doesn't hurt, let's keep it in.
> > 
> > I wanted to push these patches:
> > https://github.com/jhrozek/sssd/tree/secrets-review
> > but..I can't find http_parser_strict on Debian, only http-parser and the
> > patches seem to require the _strict version. I really don't know the
> > difference between the two, can we fallback to the non-strict?
> 
> If it not too hard to detect if strict is present I would try to use it
> and fallback to not strict only of not available.

Thanks, I will prepare a fallback patch. I agree about the preference,
but was wondering if we should require strict or not.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


[SSSD] Re: [PATCH] confdb: Check for config file errors on sssd startup

2016-06-24 Thread Michal Židek

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?= 
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 
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
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


[SSSD] Re: [PATCH] confd: Make it possible to use config snippets

2016-06-24 Thread Jakub Hrozek
On Thu, Jun 23, 2016 at 10:04:21PM +0200, Michal Židek wrote:
> On 06/23/2016 09:40 PM, Jakub Hrozek wrote:
> > On Thu, Jun 23, 2016 at 09:10:34PM +0200, Jakub Hrozek wrote:
> > > On Thu, Jun 23, 2016 at 07:29:13PM +0200, Lukas Slebodnik wrote:
> > > > On (23/06/16 18:59), Michal Židek wrote:
> > > > > On 06/23/2016 06:52 PM, Lukas Slebodnik wrote:
> > > > > > On (23/06/16 18:24), Michal Židek wrote:
> > > > > > > On 06/23/2016 05:50 PM, Michal Židek wrote:
> > > > > > > > Lukas,
> > > > > > > > 
> > > > > > > > did you have some local patches related to
> > > > > > > > this patch, that you did not send here?
> > > > > > > > 
> > > > > > > > Because I can not apply the patch on current
> > > > > > > > master and the changes in the context of
> > > > > > > > your patch seem like you did something with
> > > > > > > > default config file path?
> > > > > > > > 
> > > > > > > > Slightly modified version that applies on
> > > > > > > > current master is attached (the changes
> > > > > > > > are the same, but the context differs).
> > > > > > > > 
> > > > > > > > Michal
> > > > > > > > 
> > > > > > > 
> > > > > > > I added the missing access check for
> > > > > > > snippets. If the snippets does not
> > > > > > > pass the access check it is skipped
> > > > > > > and a loud debug message is logged.
> > > > > > > 
> > > > > > > I attach it as separate patch for
> > > > > > > easier review, but it should be squashed
> > > > > > > before pushing. I also attach the first
> > > > > > > patch for convenience.
> > > > > > > 
> > > > > > > Michal
> > > > > > 
> > > > > > >  From 6299c0c9e2d07f0764be80e73334e3cdc2ba2b12 Mon Sep 17 
> > > > > > > 00:00:00 2001
> > > > > > > From: =?UTF-8?q?Michal=20=C5=BDidek?= 
> > > > > > > Date: Tue, 22 Mar 2016 14:09:34 +0100
> > > > > > > Subject: [PATCH] confdb: Make it possible to use config snippets
> > > > > > > 
> > > > > > > Resolves:
> > > > > > > https://fedorahosted.org/sssd/ticket/2247
> > > > > > > 
> > > > > > > Signed-off-by: Lukas Slebodnik 
> > > > > > > ---
> > > > LGTM.
> > > > 
> > > > It isn't ACK because I was involved in development
> > > > and I didn;t test last version but code is fine fore me.
> > > 
> > > I just ran a quick test and the patch works for me and the code reads OK
> > > as well. I will push after CI finishes.
> > > 
> > > I also opened:
> > >  https://fedorahosted.org/sssd/ticket/3062
> > > because the patch is missing documentation and
> > >  https://fedorahosted.org/sssd/ticket/3063
> > > to add at least a simple integration test.
> > 
> > Actually, can we get rid of this warning?
> > 
> > Error: COMPILER_WARNING:
> > sssd-1.13.91/src/util/sss_ini.c:216:17: warning: unused variable 'patterns' 
> > [-Wunused-variable]
> > # const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
> > # ^
> > #  214|   int ret;
> > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
> > #  216|-> const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
> > #  217|   const char *sections[] = { ".*", NULL };
> > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
> > 
> > Error: COMPILER_WARNING:
> > sssd-1.13.91/src/util/sss_ini.c: scope_hint: In function 
> > 'sss_ini_get_config'
> > sssd-1.13.91/src/util/sss_ini.c:217:17: warning: unused variable 'sections' 
> > [-Wunused-variable]
> > # const char *sections[] = { ".*", NULL };
> > # ^
> > #  215|   #ifdef HAVE_LIBINI_CONFIG_V1
> > #  216|   const char *patterns[] = { "^[^\\.].*\\.conf", NULL };
> > #  217|-> const char *sections[] = { ".*", NULL };
> > #  218|   #ifdef HAVE_LIBINI_CONFIG_V1_3
> > #  219|   uint32_t i = 0;
> > ___
> 
> New patch attached.
> 
> Michal
> 

The patch is fine, (so ack also from me), but can someone check if the
attached rebase on top of Pavel's sssctl is OK?
>From bfd51b6bff4df2c6e78b1718fa11623d249841e8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 22 Mar 2016 14:09:34 +0100
Subject: [PATCH] confdb: Make it possible to use config snippets

Resolves:
https://fedorahosted.org/sssd/ticket/2247

Signed-off-by: Lukas Slebodnik 
---
 Makefile.am   |  4 ++-
 contrib/sssd.spec.in  |  1 +
 src/confdb/confdb.h   |  1 +
 src/confdb/confdb_setup.c | 44 +--
 src/confdb/confdb_setup.h |  1 +
 src/external/libini_config.m4 | 12 +++
 src/monitor/monitor.c |  6 ++--
 src/tools/common/sss_tools.c  |  4 ++-
 src/util/sss_ini.c| 81 +++
 src/util/sss_ini.h|  3 +-
 10 files changed, 109 insertions(+), 48 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 
152fdbfc435179dfaf2dec1c248bd83a037e2420..d87896df403147acb69b7b50db779af6ad6e6162
 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3849,6 +3849,7 @@ SSSD_USER_DIRS = \
 $(DESTDIR)$(pubconfpath)/krb5.include.d \
 $(DESTDIR)$(gpocachepath) \
 $(DESTDIR)$(sssdconfdir) \
+$(DESTDIR)$(s

[SSSD] Re: [PATCH] confdb: Check for config file errors on sssd startup

2016-06-24 Thread Jakub Hrozek
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?= 
> 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 
> 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?
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org


[SSSD] Re: [PATCH] confdb: Check for config file errors on sssd startup

2016-06-24 Thread Michal Židek

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.

http://sssd-ci.idm.lab.eng.brq.redhat.com:8080/job/ci/4601/


CI passed.

http://sssd-ci.duckdns.org/logs/job/46/01/summary.html

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