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?= <mzi...@redhat.com>
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 <lsleb...@redhat.com>
---
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

Reply via email to