[SSSD] Re: [PATCH] SDAP: Fix settig paging attribute in sdap_get_generic_ext_send

2016-08-30 Thread Fabiano Fidêncio 
On Tue, Aug 30, 2016 at 5:26 PM, Lukas Slebodnik  wrote:
> ehlo,
>
> We should set pagging flag in state and not in local
> variable which is not read anywhere in the function.
>
> Found by clang static analyzer.
>
> Do we need this patch also to stable branch?
>
> LS
>
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>

Acked-by: Fabiano Fidêncio 
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCHES] Remove leftovers from diag_cmd and force_timeout

2016-08-30 Thread Petr Cech 

On 08/30/2016 06:35 PM, Fabiano Fidêncio wrote:

Seems that when I sent the v2 of 7579cf99 and ac35fe74 I attached the
wrong patches that ended up being pushed.
Those patches were incomplete as there are still some leftovers.

My bad, sorry :-\

See these 2 attached patches

Best Regards,
--
Fabiano Fidêncio


Hi Fabiano,

LGTM, make distcheck passed.
I am waiting for CI.

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: [PATCH] SYSDB: Fix error handling in sysdb_get_user_members_recursively

2016-08-30 Thread Petr Cech 

On 08/30/2016 05:24 PM, Lukas Slebodnik wrote:

ehlo,

We should not ignore return values of functions.

LS


Hi Lukas,

thanks for patch.
LGTM, I am waiting for CI.

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: [PATCH SET] SYSDB: Adding message to inform about cache

2016-08-30 Thread Petr Cech 

Bump.

--
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: [PATCH] knownhostsproxy: use all of the getaddrinfo()

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 21:22), Jakub Hrozek wrote:
>On Tue, Aug 30, 2016 at 06:51:43PM +0200, Lukas Slebodnik wrote:
>> On (30/08/16 13:01), Pavel Březina wrote:
>> >On 08/19/2016 02:03 PM, Jakub Hrozek wrote:
>> >> Hi,
>> >> 
>> >> I'm sending this patch on behalf of Nalin, who attached it to
>> >> https://bugzilla.redhat.com/show_bug.cgi?id=1063278#c9
>> >
>> >Ack.
>> >
>> >Solves:
>> >https://fedorahosted.org/sssd/ticket/1498
>> Denis wrote a comment into BZ
>> 
>>  in quick testing it works.  fall back is very slow, but it does appear
>>  to do  so.
>> 
>> Current version iterate over all addressed and we can contine with next one
>> only if the 1st failed.
>> 
>> Could we try to connect to all addressed at once and use the fastest
>> (pool should use the fastest connection)
>
>Feel free to write a patch. I think Nalin's patch is already an
>improvement over total breakage we have now.
The question is what is better.

The broken version or *very slow* version.
IMHO broken version.

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

[SSSD] [sssd PR#7] Fix initgroups with nested groups (+ack)

2016-08-30 Thread jhrozek 
sumit-bose's pull request #7: "Fix initgroups with nested groups" label *ack* 
has been added

See the full pull-request at https://github.com/SSSD/sssd/pull/7
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#7] Fix initgroups with nested groups (comment)

2016-08-30 Thread jhrozek 
jhrozek commented on a pull request

"""
OK, also looking at gdb confirms this is the right fix of course:

(gdb) frame 0
#0  sdap_initgr_nested_get_membership_diff (mem_ctx=0x1176190, sysdb=0x1120040, 
opts=0x1134d90, 
dom=0x110b610, group=0x1184b80, all_groups=0x1184a80, groups_count=5, 
_mdiff=0x7ffe14f1ed38)
at /sssd/src/providers/ldap/sdap_async_initgroups.c:1420
1420
opts->group_map[SDAP_AT_GROUP_NAME].name,
(gdb) list
1415
1416if (parents_count > 0) {
1417ret = sysdb_attrs_primary_fqdn_list(dom, tmp_ctx,
1418ldap_parentlist,
1419parents_count,
1420
opts->group_map[SDAP_AT_GROUP_NAME].name,
1421_parent_names_list);
1422if (ret != EOK) {
1423DEBUG(SSSDBG_CRIT_FAILURE,
1424  "sysdb_attrs_primary_name_list failed [%d]: %s\n",
(gdb) p *ldap_parentlist[0]
$4 = {  num = 8
{   flags = 0, name = 0x1182b20 
"originalDN", num_values = 1
{   data = 0x1183370 
"cn=group20,cn=groups,cn=accounts,dc=ipa,dc=test", length = 47 } }
{   flags = 0, name = 0x116d0e0 "name", 
num_values = 1
{   data = 0x11808e0 "group20", 
length = 7 } }
{   flags = 0, name = 0x115db10 
"gidNumber", num_values = 1
{   data = 0x1183c20 "935600011", 
length = 9 } }
{   flags = 0, name = 0x115dbc0 "member", 
num_values = 2
{   data = 0x1183e00 
"cn=group10,cn=groups,cn=accounts,dc=ipa,dc=test", length = 47 }
{   data = 0x1183ea0 
"cn=group11,cn=groups,cn=accounts,dc=ipa,dc=test", length = 47 } }
{   flags = 0, name = 0x1183d10 "uniqueID", 
num_values = 1
{   data = 0x11841c0 
"1f9a98ba-5961-11e6-a51b-525400f71478", length = 36 } }
{   flags = 0, name = 0x1184030 
"objectSIDString", num_values = 1
{   data = 0x1184400 
"S-1-5-21-925249755-300578800-2979754137-1011", length = 44 } }
{   flags = 0, name = 0x11842d0 
"originalModifyTimestamp", num_values = 1
{   data = 0x1175780 
"20160803100057Z", length = 15 } }
{   flags = 0, name = 0x1175880 "entryUSN", 
num_values = 1
{   data = 0x1183f40 "17299", 
length = 5 } } }
(gdb) 

name is unqualified and should be qualified.

ACK, but I won't push for now in case @lslebodn wants to run some tests before 
pushing..
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/7#issuecomment-243555414
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#7] Fix initgroups with nested groups (comment)

2016-08-30 Thread jhrozek 
jhrozek commented on a pull request

"""
On Tue, Aug 30, 2016 at 11:47:09AM -0700, lslebodn wrote:
> Please provide a test-case (probably a hierarchy of groups)

I was able to reproduce with:
$ ipa group-show group20
  Group name: group20
  GID: 935600011
  Member groups: group10, group11
  Indirect Member users: user1
$ ipa group-show group10
  Group name: group10
  GID: 93568
  Member users: user1
  Member of groups: group20
$ ipa group-show group11
  Group name: group11
  GID: 93569
  Member users: user1
  Member of groups: group20

Before the patch, group20 wasn't resolved, after the patch it was.

btw I had this group hierarhcy pre-created on my test IPA server which makes me 
wonder a bit how we didn't see this bug before, I'm sure I created it
for some reason. Also I'm surprised a lot none of the downstream tests we
were running caught the bug.

About the discussion I saw on #sssd in backscroll, the rfc2307bis schema
only uses the member attribute because IIRC the RFC doesn't talk about
memberof at all. But in IPA, we know the specifics on the schema, so we
are able to dereference the memberof attribute to get a complete list of
all groups with one call.

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/7#issuecomment-243553980
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] knownhostsproxy: use all of the getaddrinfo()

2016-08-30 Thread Jakub Hrozek 
On Tue, Aug 30, 2016 at 06:51:43PM +0200, Lukas Slebodnik wrote:
> On (30/08/16 13:01), Pavel Březina wrote:
> >On 08/19/2016 02:03 PM, Jakub Hrozek wrote:
> >> Hi,
> >> 
> >> I'm sending this patch on behalf of Nalin, who attached it to
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1063278#c9
> >
> >Ack.
> >
> >Solves:
> >https://fedorahosted.org/sssd/ticket/1498
> Denis wrote a comment into BZ
> 
>  in quick testing it works.  fall back is very slow, but it does appear
>  to do  so.
> 
> Current version iterate over all addressed and we can contine with next one
> only if the 1st failed.
> 
> Could we try to connect to all addressed at once and use the fastest
> (pool should use the fastest connection)

Feel free to write a patch. I think Nalin's patch is already an
improvement over total breakage we have now.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sss_override: improve --debug description

2016-08-30 Thread Justin Stephenson 

On 08/05/2016 11:45 AM, Lukas Slebodnik wrote:

On (15/03/16 12:40), Pavel Březina wrote:

On 12/09/2015 01:16 PM, Jakub Hrozek wrote:

On Wed, Dec 09, 2015 at 01:07:10PM +0100, Pavel Březina wrote:

https://fedorahosted.org/sssd/ticket/2813

I wanted to split include/debug_levels.xml into more files so we don't
duplicate information, but I didn't figure out how to use xi:include in
files that are already beeing included. I always managed to fail on dtd
validation. Maybe someone more familiar with docbook may chime in.


If nesting doesn't work, wouldn't it be better to have a separate file
with just the levels so that services would include the
how-to-debug-services.xml and then levels?

Either way, the new file must be added to src/man/po/po4a.cfg


Here is the original patch with po4a.cfg altered.




From fb91d0bb1a84e77c5900aae0f8ca8b634f9baea7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
Date: Wed, 9 Dec 2015 13:04:35 +0100
Subject: [PATCH] sss_override: improve --debug description

Resolves:
https://fedorahosted.org/sssd/ticket/2813
---

Bump for review


+ Critical failures. An error that doesn't kill the SSSD, but 
one that

+ indicates that at least one major feature is not going to work
+ properly.
+
+

ACK with one minor change.

s/kill the SSSD/kill SSSD/

Kind regards,
Justin Stephenson



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] [sssd PR#7] Fix initgroups with nested groups (comment)

2016-08-30 Thread lslebodn 
lslebodn commented on a pull request

"""
Please provide a test-case (probably a hierarchy of groups)
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/7#issuecomment-243540136
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] knownhostsproxy: use all of the getaddrinfo()

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 13:01), Pavel Březina wrote:
>On 08/19/2016 02:03 PM, Jakub Hrozek wrote:
>> Hi,
>> 
>> I'm sending this patch on behalf of Nalin, who attached it to
>> https://bugzilla.redhat.com/show_bug.cgi?id=1063278#c9
>
>Ack.
>
>Solves:
>https://fedorahosted.org/sssd/ticket/1498
Denis wrote a comment into BZ

 in quick testing it works.  fall back is very slow, but it does appear
 to do  so.

Current version iterate over all addressed and we can contine with next one
only if the 1st failed.

Could we try to connect to all addressed at once and use the fastest
(pool should use the fastest connection)

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

[SSSD] [PATCHES] Remove leftovers from diag_cmd and force_timeout

2016-08-30 Thread Fabiano Fidêncio 
Seems that when I sent the v2 of 7579cf99 and ac35fe74 I attached the
wrong patches that ended up being pushed.
Those patches were incomplete as there are still some leftovers.

My bad, sorry :-\

See these 2 attached patches

Best Regards,
--
Fabiano Fidêncio
From 44a4238a169fa885478a2b3189af1c0e0a7a3692 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 30 Aug 2016 18:17:46 +0200
Subject: [PATCH 1/2] MONITOR: Remove leftovers from diag_cmd
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seems that when I sent the v2 of 7579cf99 I attached the wrong patch
that ended up being pushed.
That patch was incomplete as there are still some leftovers.

Related:
https://fedorahosted.org/sssd/ticket/3051

Signed-off-by: Fabiano Fidêncio 
---
 src/config/SSSDConfig/__init__.py.in | 1 -
 src/config/SSSDConfigTest.py | 1 -
 src/config/cfg_rules.ini | 9 -
 src/config/etc/sssd.api.conf | 1 -
 4 files changed, 12 deletions(-)

diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac..fb07127 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -51,7 +51,6 @@ option_strings = {
 'reconnection_retries' : _('Number of times to attempt connection to Data Providers'),
 'fd_limit' : _('The number of file descriptors that may be opened by this responder'),
 'client_idle_timeout' : _('Idle time before automatic disconnection of a client'),
-'diag_cmd' : _('The command to run when a service ping times out'),
 
 # [sssd]
 'services' : _('SSSD Services to start'),
diff --git a/src/config/SSSDConfigTest.py b/src/config/SSSDConfigTest.py
index 8fcd1a5..575a124 100755
--- a/src/config/SSSDConfigTest.py
+++ b/src/config/SSSDConfigTest.py
@@ -309,7 +309,6 @@ class SSSDConfigTestSSSDService(unittest.TestCase):
 'reconnection_retries',
 'fd_limit',
 'client_idle_timeout',
-'diag_cmd',
 'description',
 'certificate_verification',
 'override_space']
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538..a2c3fa2 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -25,7 +25,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Monitor service
 option = services
@@ -57,7 +56,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Name service
 option = user_attributes
@@ -96,7 +94,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # Authentication service
 option = offline_credentials_expiration
@@ -130,7 +127,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # sudo service
 option = sudo_timed
@@ -152,7 +148,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # autofs service
 option = autofs_negative_timeout
@@ -173,7 +168,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # ssh service
 option = ssh_hash_known_hosts
@@ -196,7 +190,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # PAC responder
 option = allowed_uids
@@ -218,7 +211,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 # InfoPipe responder
 option = allowed_uids
@@ -239,7 +231,6 @@ option = fd_limit
 option = client_idle_timeout
 option = force_timeout
 option = description
-option = diag_cmd
 
 #Available provider types
 option = id_provider
diff --git a/src/config/etc/sssd.api.conf b/src/config/etc/sssd.api.conf
index 5e69414..b2f20c5 100644
--- a/src/config/etc/sssd.api.conf
+++ b/src/config/etc/sssd.api.conf
@@ -15,7 +15,6 @@ fd_limit = int, None, false
 client_idle_timeout = int, None, false
 force_timeout = int, None, false
 description = str, None, false
-diag_cmd = str, None, false
 
 [sssd]
 # Monitor service
-- 
2.7.4

From 19ab71baeacfcbe45d28deaeae82012551c05968 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= 
Date: Tue, 30 Aug 2016 18:25:21 +0200
Subject: [PATCH 2/2] MONITOR: Remove leftovers from kill_service
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Seems that wen I sent the v2 of ac35fe74 I attached the wrong pacth that
ended up being pushed.
The patch was incomplete as there are still some leftovers.

The .po and sssd-docs.pot were not touched as I do believe they are
autogenerated from Zanata.

Related:

[SSSD] [sssd PR#7] Fix initgroups with nested groups (opened)

2016-08-30 Thread sumit-bose 
sumit-bose's pull request #7: "Fix initgroups with nested groups" was opened

PR body:
"""
I think this is a leftover from the change to use fully-qualified names in
sysdb. To verify this you can create a nested group in IPA. Without this patch
the id command will only show the groups the user is a direct member of. With
the patch the indirect groups memberships should be shown as well.
"""

See the full pull-request at https://github.com/SSSD/sssd/pull/7
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/7/head:pr7
git checkout pr7


sssd-pr-7.patch
Description: application/text/diff
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [PATCH] SDAP: Fix settig paging attribute in sdap_get_generic_ext_send

2016-08-30 Thread Lukas Slebodnik 
ehlo,

We should set pagging flag in state and not in local
variable which is not read anywhere in the function.

Found by clang static analyzer.

Do we need this patch also to stable branch?

LS
>From 6fb265fc7a41f037e3a380a4af7c60d513f48df4 Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Tue, 30 Aug 2016 16:39:49 +0200
Subject: [PATCH 2/2] SDAP: Fix settig paging attribute in
 sdap_get_generic_ext_send

We should set pagging flag in state and not in local
variable which is not read anywhere in the function.

Found by clang static analyzer.
---
 src/providers/ldap/sdap_async.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/providers/ldap/sdap_async.c b/src/providers/ldap/sdap_async.c
index 
4195ba95d911f3956f8cca665310b4b92091e6cd..e9ce2d5fd7c835919fff615e7b553d95f72d65a7
 100644
--- a/src/providers/ldap/sdap_async.c
+++ b/src/providers/ldap/sdap_async.c
@@ -1254,7 +1254,7 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
  */
 if (scope == LDAP_SCOPE_BASE && (flags & SDAP_SRCH_FLG_PAGING)) {
 /* Disable paging */
-flags &= ~SDAP_SRCH_FLG_PAGING;
+state->flags &= ~SDAP_SRCH_FLG_PAGING;
 DEBUG(SSSDBG_TRACE_FUNC,
   "WARNING: Disabling paging because scope is set to base.\n");
 }
@@ -1267,7 +1267,7 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
 serverctrls,
 NULL);
 if (control) {
-flags |= SDAP_SRCH_FLG_PAGING;
+state->flags |= SDAP_SRCH_FLG_PAGING;
 }
 
 /* ASQ */
@@ -1275,7 +1275,7 @@ sdap_get_generic_ext_send(TALLOC_CTX *memctx,
 serverctrls,
 NULL);
 if (control) {
-flags |= SDAP_SRCH_FLG_PAGING;
+state->flags |= SDAP_SRCH_FLG_PAGING;
 }
 
 for (state->nserverctrls=0;
-- 
2.9.3

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

[SSSD] [PATCH] SYSDB: Fix error handling in sysdb_get_user_members_recursively

2016-08-30 Thread Lukas Slebodnik 
ehlo,

We should not ignore return values of functions.

LS
>From f60edd21fd40a2b3d5d7ca0ed0fb9918268f72fb Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Tue, 30 Aug 2016 15:37:43 +0200
Subject: [PATCH 1/2] SYSDB: Fix error handling in
 sysdb_get_user_members_recursively

We ignored failures from sysdb_search_entry
---
 src/db/sysdb_ops.c   | 3 +++
 src/db/sysdb_views.c | 5 -
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/db/sysdb_ops.c b/src/db/sysdb_ops.c
index 
44fb5b70e6d33fffbca5824f831a3229254ecb57..e4c8e1e285e3bc49710f71c896ba9a30c742d4fa
 100644
--- a/src/db/sysdb_ops.c
+++ b/src/db/sysdb_ops.c
@@ -4738,6 +4738,9 @@ errno_t sysdb_get_user_members_recursively(TALLOC_CTX 
*mem_ctx,
 
 ret = sysdb_search_entry(tmp_ctx, dom->sysdb, base_dn, LDB_SCOPE_SUBTREE,
  filter, attrs, , );
+if (ret != EOK) {
+goto done;
+}
 
 res = talloc_zero(tmp_ctx, struct ldb_result);
 if (res == NULL) {
diff --git a/src/db/sysdb_views.c b/src/db/sysdb_views.c
index 
79f513d13ba41212a6cd84e1d9e609df6acba29c..9dc48f5b6c414bbc7c64bcd1fe73553f388588bd
 100644
--- a/src/db/sysdb_views.c
+++ b/src/db/sysdb_views.c
@@ -1374,7 +1374,10 @@ errno_t sysdb_add_group_member_overrides(struct 
sss_domain_info *domain,
 
 ret = sysdb_get_user_members_recursively(tmp_ctx, domain, obj->dn,
  _members);
-if (ret != EOK) {
+if (ret == ENOENT) {
+ret = EOK;
+goto done;
+} else if (ret != EOK) {
 DEBUG(SSSDBG_OP_FAILURE,
   "sysdb_get_user_members_recursively failed.\n");
 goto done;
-- 
2.9.3

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

[SSSD] Re: [PATCH SYSDB: Fix handling of version in sysdb_cache_connect_helper

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 16:59), Fabiano Fidêncio wrote:
>Lukaš,
>
>On Tue, Aug 30, 2016 at 4:54 PM, Lukas Slebodnik  wrote:
>> ehlo,
>>
>> Clang static analyzer assume that ldb_search can found
>> 0 entries in the tree "cn=sysdb". Thenvariable version
>> could be used uninitialized.
>>
>> We cannot get to such state in sssd but we already handle
>> a case for more then one entry.
>
>I don't think this is the right approach as res->count == 0 seems to
>be a valid case for a newly created database (please, correct me if
>I'm wrong).
>
Agree

I should have tried to run unit test before sending a patch

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

[SSSD] [PATCH SYSDB: Fix handling of version in sysdb_cache_connect_helper

2016-08-30 Thread Lukas Slebodnik 
ehlo,

Clang static analyzer assume that ldb_search can found
0 entries in the tree "cn=sysdb". Thenvariable version
could be used uninitialized.

We cannot get to such state in sssd but we already handle
a case for more then one entry.

LS
>From 88da8ca30fbf50c320cd2bf4d79d84925cb0883c Mon Sep 17 00:00:00 2001
From: Lukas Slebodnik 
Date: Tue, 30 Aug 2016 15:16:31 +0200
Subject: [PATCH 1/3] SYSDB: Fix handling of version in
 sysdb_cache_connect_helper

Clang static analyzer assume that ldb_search can found
0 entries in the tree "cn=sysdb". Thenvariable version
could be used uninitialized.

We cannot get to such state in sssd but we already handle
a case for more then one entry.
---
 src/db/sysdb_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/db/sysdb_init.c b/src/db/sysdb_init.c
index 
d110aa7a2878e47650db177cfd342d0ac32248ab..0a794899d029d8e45dc133d9bb45444f4b083771
 100644
--- a/src/db/sysdb_init.c
+++ b/src/db/sysdb_init.c
@@ -572,7 +572,7 @@ static errno_t sysdb_cache_connect_helper(TALLOC_CTX 
*mem_ctx,
 ret = EIO;
 goto done;
 }
-if (res->count > 1) {
+if (res->count != 1) {
 ret = EIO;
 goto done;
 }
-- 
2.9.3

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

[SSSD] [sssd PR#6] Applications should never define _USE_GNU themselves, but rely on _GNU_SOURCE (closed)

2016-08-30 Thread jhrozek 
jhrozek's pull request #6: "Applications should never define _USE_GNU 
themselves, but rely on _GNU_SOURCE" was closed

See the full pull-request at https://github.com/SSSD/sssd/pull/6
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/6/head:pr6
git checkout pr6
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#6] Applications should never define _USE_GNU themselves, but rely on _GNU_SOURCE (+pushed)

2016-08-30 Thread jhrozek 
jhrozek's pull request #6: "Applications should never define _USE_GNU 
themselves, but rely on _GNU_SOURCE" label *pushed* has been added

See the full pull-request at https://github.com/SSSD/sssd/pull/6
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#6] Applications should never define _USE_GNU themselves, but rely on _GNU_SOURCE (-ack)

2016-08-30 Thread jhrozek 
jhrozek's pull request #6: "Applications should never define _USE_GNU 
themselves, but rely on _GNU_SOURCE" label *ack* has been removed

See the full pull-request at https://github.com/SSSD/sssd/pull/6
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#6] Applications should never define _USE_GNU themselves, but rely on _GNU_SOURCE (comment)

2016-08-30 Thread jhrozek 
jhrozek commented on a pull request

"""
* master: 1384d0ce6ea741aefb56b0006b6268d76e6cc2c2
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/6#issuecomment-243453157
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#6] Applications should never define _USE_GNU themselves, but rely on _GNU_SOURCE (+ack)

2016-08-30 Thread fidencio 
jhrozek's pull request #6: "Applications should never define _USE_GNU 
themselves, but rely on _GNU_SOURCE" label *ack* has been added

See the full pull-request at https://github.com/SSSD/sssd/pull/6
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sudo man page: say that we support IPA schema

2016-08-30 Thread Justin Stephenson 


On 08/30/2016 04:24 AM, Lukas Slebodnik wrote:

On (30/08/16 10:14), Jakub Hrozek wrote:

On Mon, Aug 29, 2016 at 11:28:44AM -0400, Justin Stephenson wrote:

On 08/10/2016 04:33 PM, Dan Lavu wrote:

I asked Lukas this but he wasn't positive, is the objectClasses different when 
adding 'ldap_sudo_search_base' ? Or is it just location?

Eitherway, I think this is going to be a little more concise,

"When SSSD is configured and using the IPA provider, sudo is automatically enabled. 
The sudo search base is cn=sudo,ou=sudoers,$DC. If a different search base is defined in 
sssd.conf, it will use the value from the configuration file. (e.g. ou=sudoers,$DC 
generated by compat plugin)."


Hello Dan/Pavel,

I tried to combine some of your suggestions, Please see attached.

I also thought that $SUFFIX makes the root suffix more clear than $DC but
that is just my personal opinion.

Kind regards,
Justin Stephenson


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




From f639386298d40013e2c2d915b9ed4a72e1c09868 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Mon, 29 Aug 2016 11:20:00 -0400
Subject: [PATCH] MAN: sssd-sudo manual update IPA native LDAP tree support

Update sssd-sudo man page to reflect native IPA sudo support

Resolves:
https://fedorahosted.org/sssd/ticket/3145
---
 src/man/sssd-sudo.5.xml | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/man/sssd-sudo.5.xml b/src/man/sssd-sudo.5.xml
index 
de276ad2d7647da9b7d510bf00fdf8fb58aed1c7..845d1699bd8c3739b401a09eeca0b06861c2e86b
 100644
--- a/src/man/sssd-sudo.5.xml
+++ b/src/man/sssd-sudo.5.xml
@@ -109,9 +109,12 @@ ldap_sudo_search_base = ou=sudoers,dc=example,dc=com
 
 
 
-When the SSSD is configured to use IPA as the ID provider,
-the sudo provider is automatically enabled. The sudo search base
-is configured to use the compat tree (ou=sudoers,$DC).
+When SSSD is configured to use IPA as the ID provider, the
+sudo provider is automatically enabled. The sudo search base is
+configured to use the IPA native LDAP 
tree(cn=sudo,ou=sudoers,$SUFFIX).

   ^^^
I thought it is either (ou=sudoers,$SUFFIX)
or (cn=sudo,$SUFFIX)


Hi, the manpage builds and the text reads good to me. I would just like
to put a whitespace between "tree" and the opening "(". If you agree, I
can fix this before pushing the patch, no need to re-send it..


Yes, please go ahead.




IMHO, It deserves a new patch :-)


Hi Lukas, I can resubmit the patch if you'd like.


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: [PATCH] Fix typo

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 13:41), Lukas Slebodnik wrote:
>On (30/08/16 13:13), Pavel Březina wrote:
>>On 08/05/2016 05:24 PM, Lukas Slebodnik wrote:
>>> On (04/08/16 14:11), Pavel Březina wrote:
>>> > Simple one liner attached.
>>> 
>>> > From 6c3d8d6dba969960590d88bdd9a4ec2e9b4a1bb2 Mon Sep 17 00:00:00 2001
>>> > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
>>> > Date: Thu, 4 Aug 2016 14:10:09 +0200
>>> > Subject: [PATCH] Fix typo in debug message
>>> > 
>>> > ---
>>> > src/providers/ipa/ipa_init.c | 2 +-
>>> > 1 file changed, 1 insertion(+), 1 deletion(-)
>>> > 
>>> > diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c
>>> > index 
>>> > ca99200a1ba1d9508ac0aecaa08149552fee..7dec4d1fb8541a48470d4e44f10838e5bea67ad5
>>> >  100644
>>> > --- a/src/providers/ipa/ipa_init.c
>>> > +++ b/src/providers/ipa/ipa_init.c
>>> > @@ -200,7 +200,7 @@ static errno_t ipa_init_dyndns(struct be_ctx *be_ctx,
>>> >  enabled = dp_opt_get_bool(ipa_options->dyndns_ctx->opts,
>>> >DP_OPT_DYNDNS_UPDATE);
>>> >  if (!enabled) {
>>> > -DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates are of.\n");
>>> > +DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates are off.\n");
>>> 
>>> It would be good to have the same message as in ad provider.
>>> Or change it also there :-)
>>> 
>>> sh$ git grep "Dynamic DNS updates"
>>> src/config/cfg_rules.ini:# Dynamic DNS updates
>>> src/config/etc/sssd.api.conf:# Dynamic DNS updates
>>> src/providers/ad/ad_common.h:/* Dynamic DNS updates */
>>> src/providers/ad/ad_dyndns.c:DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic 
>>> DNS updates not set\n");
>>> src/providers/ad/ad_dyndns.c:  "Dynamic DNS updates are on. 
>>> Checking for nsupdate..\n");
>>> src/providers/be_dyndns.c:  "%s does not exist. Dynamic DNS 
>>> updates disabled\n",
>>> src/providers/ipa/ipa_init.c:DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic 
>>> DNS updates are of.\n");
>>> src/providers/ipa/ipa_init.c:  "Dynamic DNS updates are on. 
>>> Checking for nsupdate...\n");
>>> 
>>> LS
>>> 
>>
>
>>From 40850397be1e8be3b38437a6a93f4c1772c2f6af Mon Sep 17 00:00:00 2001
>>From: =?UTF-8?q?Pavel=20B=C5=99ezina?= 
>>Date: Thu, 4 Aug 2016 14:10:09 +0200
>>Subject: [PATCH] dyndns: fix typo and unify ipa with ad debug message when off
>>
>>---
>> src/providers/ad/ad_dyndns.c | 2 +-
>> src/providers/ipa/ipa_init.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>>diff --git a/src/providers/ad/ad_dyndns.c b/src/providers/ad/ad_dyndns.c
>>index 
>>e3f1812837f7cee9d18ef001233871e0fcc16b4c..00190485e8f0ca7362ed60b2df022c74c53988c9
>> 100644
>>--- a/src/providers/ad/ad_dyndns.c
>>+++ b/src/providers/ad/ad_dyndns.c
>>@@ -47,7 +47,7 @@ errno_t ad_dyndns_init(struct be_ctx *be_ctx,
>> 
>> if (dp_opt_get_bool(ad_opts->dyndns_ctx->opts,
>> DP_OPT_DYNDNS_UPDATE) == false) {
>>-DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates not set\n");
>>+DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates are off.\n");
>> return EOK;
>> }
>> 
>>diff --git a/src/providers/ipa/ipa_init.c b/src/providers/ipa/ipa_init.c
>>index 
>>ca99200a1ba1d9508ac0aecaa08149552fee..7dec4d1fb8541a48470d4e44f10838e5bea67ad5
>> 100644
>>--- a/src/providers/ipa/ipa_init.c
>>+++ b/src/providers/ipa/ipa_init.c
>>@@ -200,7 +200,7 @@ static errno_t ipa_init_dyndns(struct be_ctx *be_ctx,
>> enabled = dp_opt_get_bool(ipa_options->dyndns_ctx->opts,
>>   DP_OPT_DYNDNS_UPDATE);
>> if (!enabled) {
>>-DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates are of.\n");
>>+DEBUG(SSSDBG_CONF_SETTINGS, "Dynamic DNS updates are off.\n");
>> return EOK;
>ACK
>
master:
* b3851e86af91dc1aa6e265d5b2e4279b2611ff43

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

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Petr Cech 

On 08/30/2016 01:21 PM, Pavel Březina wrote:

On 08/30/2016 01:06 PM, Lukas Slebodnik wrote:

On (30/08/16 13:03), Petr Cech wrote:

On 08/30/2016 12:42 PM, Pavel Březina wrote:

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]:
%s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then
1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable
value (10?).


Hello Pavel,

max_children is set on the next line as:
auth_ctx->max_children = max_children;


No it is not.


+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}
+auth_ctx->max_children = max_children;


if max_children < 1, you report an error and goto done, keeping ret =
EOK from previous successful call. Either we consider it as an error,
and set ret to e.g. EINVAL, or we set default value and continue.


I see, good point. Thank you.

I chose go to done. It prevents our users to have wrong configuration.



I use temporary variable max_children,
because there is issue with signed/unsigned
integer value.

10 was original value. I increase it to 50.
And I use constant OPT_MAX_CHILDREN_DEFAULT now.


10 is a reasonable default and works for most of users.
I do not think we need to increase default value.
This is a purpose of the new option


Agree.


--
Petr^4 Čech
>From 3783aed0c7127358e1708d5657773af50fd136a0 Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 12 
 src/providers/proxy/proxy_init.c  | 21 +++--
 6 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac26309bb5b518fb87cd0dae2962e853179..9076dd2c4bf630626d6d8eaef0e7ab67c0ac93f5 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -430,6 +430,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538dee4a547a1b1af62a4cfe37b89e236b18..1b3c840199d64fe1a9088147c9c5c836216b25eb 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -323,6 +323,7 @@ option = base_directory
 option = proxy_lib_name
 option = proxy_fast_alias
 option = proxy_pam_target
+option = proxy_max_children
 
 # simple access provider specific options
 option = simple_allow_users
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml

[SSSD] [sssd PR#5] Miscellanous patches for the sssd-secrets responder (-ack)

2016-08-30 Thread jhrozek 
jhrozek's pull request #5: "Miscellanous patches for the sssd-secrets 
responder" label *ack* has been removed

See the full pull-request at https://github.com/SSSD/sssd/pull/5
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#5] Miscellanous patches for the sssd-secrets responder (+pushed)

2016-08-30 Thread jhrozek 
jhrozek's pull request #5: "Miscellanous patches for the sssd-secrets 
responder" label *pushed* has been added

See the full pull-request at https://github.com/SSSD/sssd/pull/5
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#5] Miscellanous patches for the sssd-secrets responder (closed)

2016-08-30 Thread jhrozek 
jhrozek's pull request #5: "Miscellanous patches for the sssd-secrets 
responder" was closed

See the full pull-request at https://github.com/SSSD/sssd/pull/5
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/5/head:pr5
git checkout pr5
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 13:03), Petr Cech wrote:
>On 08/30/2016 12:42 PM, Pavel Březina wrote:
>> On 08/25/2016 01:43 PM, Petr Cech wrote:
>> > -/* FIXME: get max_children from configuration file */
>> > -auth_ctx->max_children = 10;
>> > +ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
>> > + CONFDB_PROXY_MAX_CHILDREN, 10,
>> > + _children);
>> > +if (ret != EOK) {
>> > +DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
>> > +   ret, sss_strerror(ret));
>> > +goto done;
>> > +}
>> > +if (max_children < 1) {
>> > +DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
>> > +   CONFDB_PROXY_MAX_CHILDREN);
>> > +goto done;
>> > +}
>> 
>> You need to either set ret here, or set max_children to some reasonable
>> value (10?).
>
>Hello Pavel,
>
>max_children is set on the next line as:
>auth_ctx->max_children = max_children;
>
>I use temporary variable max_children,
>because there is issue with signed/unsigned
>integer value.
>
>10 was original value. I increase it to 50.
>And I use constant OPT_MAX_CHILDREN_DEFAULT now.
>
10 is a reasonable default and works for most of users.
I do not think we need to increase default value.
This is a purpose of the new option

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

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Petr Cech 

On 08/30/2016 12:42 PM, Pavel Březina wrote:

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable
value (10?).


Hello Pavel,

max_children is set on the next line as:
auth_ctx->max_children = max_children;

I use temporary variable max_children,
because there is issue with signed/unsigned
integer value.

10 was original value. I increase it to 50.
And I use constant OPT_MAX_CHILDREN_DEFAULT now.

Fixed patch is attached.

Regards

--
Petr^4 Čech
>From fb5818026adf0bb8dde45ccae5d94a6cb6331bec Mon Sep 17 00:00:00 2001
From: Petr Cech 
Date: Wed, 24 Aug 2016 14:41:09 +0200
Subject: [PATCH] PROXY: Adding proxy_max_children option

The new option 'proxy_max_children' is applicable
in domain section. Default value is 10.

Resolves:
https://fedorahosted.org/sssd/ticket/3153
---
 src/confdb/confdb.h   |  1 +
 src/config/SSSDConfig/__init__.py.in  |  3 +++
 src/config/cfg_rules.ini  |  1 +
 src/config/etc/sssd.api.d/sssd-proxy.conf |  1 +
 src/man/sssd.conf.5.xml   | 12 
 src/providers/proxy/proxy_init.c  | 20 ++--
 6 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 401e5fbf7ed6bb9e8d7158dfab378c8159aa03db..9b5c7bc04bb8297842aa9a0ef50f239c50302757 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -218,6 +218,7 @@
 #define CONFDB_PROXY_LIBNAME "proxy_lib_name"
 #define CONFDB_PROXY_PAM_TARGET "proxy_pam_target"
 #define CONFDB_PROXY_FAST_ALIAS "proxy_fast_alias"
+#define CONFDB_PROXY_MAX_CHILDREN "proxy_max_children"
 
 /* Secrets Service */
 #define CONFDB_SEC_CONF_ENTRY "config/secrets"
diff --git a/src/config/SSSDConfig/__init__.py.in b/src/config/SSSDConfig/__init__.py.in
index b3f04ac26309bb5b518fb87cd0dae2962e853179..9076dd2c4bf630626d6d8eaef0e7ab67c0ac93f5 100644
--- a/src/config/SSSDConfig/__init__.py.in
+++ b/src/config/SSSDConfig/__init__.py.in
@@ -430,6 +430,9 @@ option_strings = {
 'default_shell' : _('Default shell, /bin/bash'),
 'base_directory' : _('Base for home directories'),
 
+# [provider/proxy]
+'proxy_max_children' : _('The number of preforked proxy children.'),
+
 # [provider/proxy/id]
 'proxy_lib_name' : _('The name of the NSS library to use'),
 'proxy_fast_alias' : _('Whether to look up canonical group name from cache if possible'),
diff --git a/src/config/cfg_rules.ini b/src/config/cfg_rules.ini
index df10538dee4a547a1b1af62a4cfe37b89e236b18..1b3c840199d64fe1a9088147c9c5c836216b25eb 100644
--- a/src/config/cfg_rules.ini
+++ b/src/config/cfg_rules.ini
@@ -323,6 +323,7 @@ option = base_directory
 option = proxy_lib_name
 option = proxy_fast_alias
 option = proxy_pam_target
+option = proxy_max_children
 
 # simple access provider specific options
 option = simple_allow_users
diff --git a/src/config/etc/sssd.api.d/sssd-proxy.conf b/src/config/etc/sssd.api.d/sssd-proxy.conf
index 89a6503f9b84b7eab5fb3b0dd591dea905b43adb..09bf82affcb4263de3abbb67d1d484f6b01a1824 100644
--- a/src/config/etc/sssd.api.d/sssd-proxy.conf
+++ b/src/config/etc/sssd.api.d/sssd-proxy.conf
@@ -1,4 +1,5 @@
 [provider/proxy]
+proxy_max_children = int, None, false
 
 [provider/proxy/id]
 proxy_lib_name = str, None, true
diff --git a/src/man/sssd.conf.5.xml b/src/man/sssd.conf.5.xml
index ae291e0fc8f2f9afabcdf32f18a5ec12252f..1bf3e799047d9c722487be8657bbee5cfd479cdd 100644
--- a/src/man/sssd.conf.5.xml
+++ b/src/man/sssd.conf.5.xml
@@ -2464,6 +2464,18 @@ subdomain_inherit = ldap_purge_cache_timeout
 
 
 
+
+proxy_max_children (integer)
+
+
+The number of preforked proxy children.
+
+
+Default: 50
+
+
+
+
 
 
 
diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c
index 1edf4fd64e54f4f0df7a78a9e56eb232a1d3e948..ed5e825a730c2db9072de4a47f5d91720f4d 100644
--- a/src/providers/proxy/proxy_init.c
+++ b/src/providers/proxy/proxy_init.c
@@ -29,6 +29,8 @@
 
 #define NSS_FN_NAME "_nss_%s_%s"
 
+#define

[SSSD] Re: [PATCH] knownhostsproxy: use all of the getaddrinfo()

2016-08-30 Thread Pavel Březina 

On 08/19/2016 02:03 PM, Jakub Hrozek wrote:

Hi,

I'm sending this patch on behalf of Nalin, who attached it to
https://bugzilla.redhat.com/show_bug.cgi?id=1063278#c9


Ack.

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

[SSSD] [sssd PR#5] Miscellanous patches for the sssd-secrets responder (comment)

2016-08-30 Thread pbrezina 
pbrezina commented on a pull request

"""
Ack.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/5#issuecomment-243402484
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] PROXY: Adding proxy_max_children option

2016-08-30 Thread Pavel Březina 

On 08/25/2016 01:43 PM, Petr Cech wrote:

-/* FIXME: get max_children from configuration file */
-auth_ctx->max_children = 10;
+ret = confdb_get_int(be_ctx->cdb, be_ctx->conf_path,
+ CONFDB_PROXY_MAX_CHILDREN, 10,
+ _children);
+if (ret != EOK) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Unable to read confdb [%d]: %s\n",
+   ret, sss_strerror(ret));
+goto done;
+}
+if (max_children < 1) {
+DEBUG(SSSDBG_CRIT_FAILURE, "Option %s must be bigger then 1\n",
+   CONFDB_PROXY_MAX_CHILDREN);
+goto done;
+}


You need to either set ret here, or set max_children to some reasonable 
value (10?).

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

[SSSD] Re: [PATCH] sudo man page: say that we support IPA schema

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 10:14), Jakub Hrozek wrote:
>On Mon, Aug 29, 2016 at 11:28:44AM -0400, Justin Stephenson wrote:
>> On 08/10/2016 04:33 PM, Dan Lavu wrote:
>> > I asked Lukas this but he wasn't positive, is the objectClasses different 
>> > when adding 'ldap_sudo_search_base' ? Or is it just location?
>> > 
>> > Eitherway, I think this is going to be a little more concise,
>> > 
>> > "When SSSD is configured and using the IPA provider, sudo is automatically 
>> > enabled. The sudo search base is cn=sudo,ou=sudoers,$DC. If a different 
>> > search base is defined in sssd.conf, it will use the value from the 
>> > configuration file. (e.g. ou=sudoers,$DC generated by compat plugin)."
>> 
>> Hello Dan/Pavel,
>> 
>> I tried to combine some of your suggestions, Please see attached.
>> 
>> I also thought that $SUFFIX makes the root suffix more clear than $DC but
>> that is just my personal opinion.
>> 
>> Kind regards,
>> Justin Stephenson
>> 
>> > ___
>> > sssd-devel mailing list
>> > sssd-devel@lists.fedorahosted.org
>> > https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
>> > 
>
>> From f639386298d40013e2c2d915b9ed4a72e1c09868 Mon Sep 17 00:00:00 2001
>> From: Justin Stephenson 
>> Date: Mon, 29 Aug 2016 11:20:00 -0400
>> Subject: [PATCH] MAN: sssd-sudo manual update IPA native LDAP tree 
>> support
>> 
>> Update sssd-sudo man page to reflect native IPA sudo support
>> 
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/3145
>> ---
>>  src/man/sssd-sudo.5.xml | 9 ++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>> 
>> diff --git a/src/man/sssd-sudo.5.xml b/src/man/sssd-sudo.5.xml
>> index 
>> de276ad2d7647da9b7d510bf00fdf8fb58aed1c7..845d1699bd8c3739b401a09eeca0b06861c2e86b
>>  100644
>> --- a/src/man/sssd-sudo.5.xml
>> +++ b/src/man/sssd-sudo.5.xml
>> @@ -109,9 +109,12 @@ ldap_sudo_search_base = ou=sudoers,dc=example,dc=com
>>  
>>  
>>  
>> -When the SSSD is configured to use IPA as the ID provider,
>> -the sudo provider is automatically enabled. The sudo search base
>> -is configured to use the compat tree (ou=sudoers,$DC).
>> +When SSSD is configured to use IPA as the ID provider, the
>> +sudo provider is automatically enabled. The sudo search base is
>> +configured to use the IPA native LDAP 
>> tree(cn=sudo,ou=sudoers,$SUFFIX).
   ^^^
I thought it is either (ou=sudoers,$SUFFIX)
or (cn=sudo,$SUFFIX)

>Hi, the manpage builds and the text reads good to me. I would just like
>to put a whitespace between "tree" and the opening "(". If you agree, I
>can fix this before pushing the patch, no need to re-send it..
>
IMHO, It deserves a new patch :-)

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

[SSSD] Re: [PATCH] sudo man page: say that we support IPA schema

2016-08-30 Thread Pavel Březina 

On 08/30/2016 10:14 AM, Jakub Hrozek wrote:

On Mon, Aug 29, 2016 at 11:28:44AM -0400, Justin Stephenson wrote:

On 08/10/2016 04:33 PM, Dan Lavu wrote:

I asked Lukas this but he wasn't positive, is the objectClasses different when 
adding 'ldap_sudo_search_base' ? Or is it just location?

Eitherway, I think this is going to be a little more concise,

"When SSSD is configured and using the IPA provider, sudo is automatically enabled. 
The sudo search base is cn=sudo,ou=sudoers,$DC. If a different search base is defined in 
sssd.conf, it will use the value from the configuration file. (e.g. ou=sudoers,$DC 
generated by compat plugin)."


Hello Dan/Pavel,

I tried to combine some of your suggestions, Please see attached.

I also thought that $SUFFIX makes the root suffix more clear than $DC but
that is just my personal opinion.

Kind regards,
Justin Stephenson


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




 From f639386298d40013e2c2d915b9ed4a72e1c09868 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Mon, 29 Aug 2016 11:20:00 -0400
Subject: [PATCH] MAN: sssd-sudo manual update IPA native LDAP tree support

 Update sssd-sudo man page to reflect native IPA sudo support

 Resolves:
 https://fedorahosted.org/sssd/ticket/3145
---
  src/man/sssd-sudo.5.xml | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/man/sssd-sudo.5.xml b/src/man/sssd-sudo.5.xml
index 
de276ad2d7647da9b7d510bf00fdf8fb58aed1c7..845d1699bd8c3739b401a09eeca0b06861c2e86b
 100644
--- a/src/man/sssd-sudo.5.xml
+++ b/src/man/sssd-sudo.5.xml
@@ -109,9 +109,12 @@ ldap_sudo_search_base = ou=sudoers,dc=example,dc=com
  
  
  
-When the SSSD is configured to use IPA as the ID provider,
-the sudo provider is automatically enabled. The sudo search base
-is configured to use the compat tree (ou=sudoers,$DC).
+When SSSD is configured to use IPA as the ID provider, the
+sudo provider is automatically enabled. The sudo search base is
+configured to use the IPA native LDAP 
tree(cn=sudo,ou=sudoers,$SUFFIX).


Hi, the manpage builds and the text reads good to me. I would just like
to put a whitespace between "tree" and the opening "(". If you agree, I
can fix this before pushing the patch, no need to re-send it..


+If any other search base is defined in sssd.conf, this value will 
be
+used instead. The compat tree(ou=sudoers,$SUFFIX) is no longer
+required for IPA sudo functionality.
  
  

--
2.7.4


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

[SSSD] Re: [PATCH] PROXY: Use right name in ldap filter

2016-08-30 Thread Jakub Hrozek 
On Fri, Aug 26, 2016 at 03:10:39PM +0200, Lukas Slebodnik wrote:
> ehlo,
> 
> We used internal fq name in ldap filter
> with id_provider proxy to files and auth provider
> ldap
> 

ACK

thank you for catching this.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [PATCH] sudo man page: say that we support IPA schema

2016-08-30 Thread Jakub Hrozek 
On Mon, Aug 29, 2016 at 11:28:44AM -0400, Justin Stephenson wrote:
> On 08/10/2016 04:33 PM, Dan Lavu wrote:
> > I asked Lukas this but he wasn't positive, is the objectClasses different 
> > when adding 'ldap_sudo_search_base' ? Or is it just location?
> > 
> > Eitherway, I think this is going to be a little more concise,
> > 
> > "When SSSD is configured and using the IPA provider, sudo is automatically 
> > enabled. The sudo search base is cn=sudo,ou=sudoers,$DC. If a different 
> > search base is defined in sssd.conf, it will use the value from the 
> > configuration file. (e.g. ou=sudoers,$DC generated by compat plugin)."
> 
> Hello Dan/Pavel,
> 
> I tried to combine some of your suggestions, Please see attached.
> 
> I also thought that $SUFFIX makes the root suffix more clear than $DC but
> that is just my personal opinion.
> 
> Kind regards,
> Justin Stephenson
> 
> > ___
> > sssd-devel mailing list
> > sssd-devel@lists.fedorahosted.org
> > https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
> > 

> From f639386298d40013e2c2d915b9ed4a72e1c09868 Mon Sep 17 00:00:00 2001
> From: Justin Stephenson 
> Date: Mon, 29 Aug 2016 11:20:00 -0400
> Subject: [PATCH] MAN: sssd-sudo manual update IPA native LDAP tree support
> 
> Update sssd-sudo man page to reflect native IPA sudo support
> 
> Resolves:
> https://fedorahosted.org/sssd/ticket/3145
> ---
>  src/man/sssd-sudo.5.xml | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/src/man/sssd-sudo.5.xml b/src/man/sssd-sudo.5.xml
> index 
> de276ad2d7647da9b7d510bf00fdf8fb58aed1c7..845d1699bd8c3739b401a09eeca0b06861c2e86b
>  100644
> --- a/src/man/sssd-sudo.5.xml
> +++ b/src/man/sssd-sudo.5.xml
> @@ -109,9 +109,12 @@ ldap_sudo_search_base = ou=sudoers,dc=example,dc=com
>  
>  
>  
> -When the SSSD is configured to use IPA as the ID provider,
> -the sudo provider is automatically enabled. The sudo search base
> -is configured to use the compat tree (ou=sudoers,$DC).
> +When SSSD is configured to use IPA as the ID provider, the
> +sudo provider is automatically enabled. The sudo search base is
> +configured to use the IPA native LDAP 
> tree(cn=sudo,ou=sudoers,$SUFFIX).

Hi, the manpage builds and the text reads good to me. I would just like
to put a whitespace between "tree" and the opening "(". If you agree, I
can fix this before pushing the patch, no need to re-send it..

> +If any other search base is defined in sssd.conf, this value 
> will be
> +used instead. The compat tree(ou=sudoers,$SUFFIX) is no longer
> +required for IPA sudo functionality.
>  
>  
>  
> -- 
> 2.7.4
> 

> ___
> 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: [SSSD} [PATCH] Remove no longer used code

2016-08-30 Thread Lukas Slebodnik 
On (30/08/16 09:41), Petr Cech wrote:
>On 08/30/2016 08:47 AM, Petr Cech wrote:
>> 
>> 
>> On 08/30/2016 08:28 AM, Fabiano Fidêncio wrote:
>> > On Tue, Aug 30, 2016 at 8:23 AM, Petr Cech  wrote:
>> > > On 08/15/2016 02:58 PM, Fabiano Fidêncio wrote:
>> > > > 
>> > > > Those 3 patches are from Jakub and I've just done some minor
>> > > > adjustments and add myself as co-author of the first 2 patches.
>> > > > 
>> > > > CI has passed: http://sssd-ci.duckdns.org/logs/job/51/55/summary.html
>> > > > 
>> > > > Best Regards,
>> > > > --
>> > > > Fabiano Fidêncio
>> > > 
>> > > 
>> > > Hello,
>> > > 
>> > > CI passed:
>> > > http://sssd-ci.duckdns.org/logs/job/52/71/summary.html
>> > > 
>> > > > 0001-MONITOR-Remove-the-no-longer-used-diag_cmd-command.patch
>> > > > 
>> > > > 
>> > > > From aa6204816cde0a7d75b9303916d038ed06e467ba Mon Sep 17 00:00:00 2001
>> > > > From: Jakub Hrozek 
>> > > > Date: Sun, 8 May 2016 14:41:35 +0200
>> > > > Subject: [PATCH 1/3] MONITOR: Remove the no longer used diag_cmd
>> > > > command
>> > > > MIME-Version: 1.0
>> > > > Content-Type: text/plain; charset=UTF-8
>> > > > Content-Transfer-Encoding: 8bit
>> > > > 
>> > > > After introducing the watchdog, the diag_cmd is longer used and
>> > > > makes no
>> > > > sense trying to make it usable by watchdog as the result of "pstack %p"
>> > > > seems next to useless in this context.
>> > > > 
>> > > > Co-author: Fabiano Fidêncio 
>> > > > 
>> > > > Related:
>> > > > https://fedorahosted.org/sssd/ticket/3051
>> > > > ---
>> > > 
>> > > 
>> > > ACK
>> > > 
>> > > 
>> > > > 0002-MONITOR-Remove-the-no-longer-used-kill_service-comma.patch
>> > > > 
>> > > > 
>> > > > From 7954e0254752d0a830a0501f23a6a93d0345e5ce Mon Sep 17 00:00:00 2001
>> > > > From: Jakub Hrozek 
>> > > > Date: Sun, 8 May 2016 14:46:25 +0200
>> > > > Subject: [PATCH 2/3] MONITOR: Remove the no longer used kill_service
>> > > > command
>> > > > MIME-Version: 1.0
>> > > > Content-Type: text/plain; charset=UTF-8
>> > > > Content-Transfer-Encoding: 8bit
>> > > > 
>> > > > After introducing the watchdog, the force_timeout option is no longer
>> > > > used.
>> > > > 
>> > > > Co-author: Fabiano Fidêncio 
>> > > > 
>> > > > Resolves:
>> > > > https://fedorahosted.org/sssd/ticket/3052
>> > > > ---
>> > > 
>> > > 
>> > > ACK
>> > > 
>> > > 
>> > > > 0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch
>> > > > 
>> > > > 
>> > > > From 1302c5a95ac36dd674c8795cda0082b84d30978d Mon Sep 17 00:00:00 2001
>> > > > From: Jakub Hrozek 
>> > > > Date: Mon, 15 Aug 2016 12:54:20 +0200
>> > > > Subject: [PATCH 3/3] WATCHDOG: define and use _MAX_TICKS as 3
>> > > > 
>> > > > Instead of using the number 3 directly, let's introduce and use
>> > > > WATCHDOG_MAX_TICKS.
>> > > > --
>> > > 
>> > > 
>> > > This patch is unfortunately inapplicable on top of master
>> > > (after two previous patches):
>> > > 
>> > > pcech@albireo ~/sssd: (master) $ git am
>> > > ../patch/0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch
>> > > Applying: WATCHDOG: define and use _MAX_TICKS as 3
>> > > error: patch failed: src/util/util_watchdog.c:38
>> > > error: src/util/util_watchdog.c: patch does not apply
>> > > Patch failed at 0001 WATCHDOG: define and use _MAX_TICKS as 3
>> > > 
>> > > Regards
>> > 
>> > Rebase was quite simple.
>> > See the v2 attached (the only change in v2 was the rebase).
>> 
>> Thanks, Fabiano,
>> 
>> I pushed patches to CI so quickly I missed
>> that the 3rd patch is LGTM and almost ACK :-)
>> 
>> I would like to wait to CI anyway.
>
>CI passed:
>http://sssd-ci.duckdns.org/logs/job/52/73/summary.html
>
>=> ACK
>
master:
* d7075a255a1f28e890539072e06d0140ffe0927c
* fa93cd0f0fc75a6d635079e67788f8a9fe183c3c
* 1620f435dbe7013f985128dcdf001e9158cb00e3

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

[SSSD] Re: MONITOR: Add disable_netlink sssd.conf option

2016-08-30 Thread Jakub Hrozek 
On Sat, Aug 27, 2016 at 12:54:53PM -0400, Justin Stephenson wrote:
> Hello,
> 
> The attached patches resolve https://fedorahosted.org/sssd/ticket/3142
> 
> However, I am having difficult with the man page addition to
> 'src/man/sssd.conf.5.xml' for this new option. I have stared at the open and
> close xml tags(for far too long) and it looks correct but when I build sssd
> I never see the sssd.conf man page inclusion. Could anyone tell me what I am
> missing here?
> 
> If you feel there is better wording for the description please let me know.
> 
> Kind regards,
> Justin Stephenson

> From 0552c199dd37c7e280304b9bc92ff44a8a1a6d57 Mon Sep 17 00:00:00 2001
> From: Justin Stephenson 
> Date: Fri, 26 Aug 2016 15:15:32 -0400
> Subject: [PATCH 1/2] MONITOR: Remove --disable-netlink command-line option

I'm not sure I like removing the netlink option w/o letting admins who
use it at least know what happened. Could we keep the option in the popt
option list, but use the HIDDEN argument so that it doesn't show up in
--help output and print a loud warning that the option was removed in
favor of a sssd.conf option?

I already know of two people from sssd-users list who might be using
this feature. On the other hand, it was just introduced in the last
version and not in any enterprise distro, so just printing a warning and
removing even that warning in the next version would be fine for me..

> From c52c0c1a520cdf8509bac00fa3c7bec0dd73 Mon Sep 17 00:00:00 2001
> From: Justin Stephenson 
> Date: Fri, 26 Aug 2016 17:43:25 -0400
> Subject: [PATCH 2/2] MONITOR: Add disable_netlink option

LGTM, untested, though.
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] Re: [SSSD} [PATCH] Remove no longer used code

2016-08-30 Thread Petr Cech 



On 08/30/2016 08:28 AM, Fabiano Fidêncio wrote:

On Tue, Aug 30, 2016 at 8:23 AM, Petr Cech  wrote:

On 08/15/2016 02:58 PM, Fabiano Fidêncio wrote:


Those 3 patches are from Jakub and I've just done some minor
adjustments and add myself as co-author of the first 2 patches.

CI has passed: http://sssd-ci.duckdns.org/logs/job/51/55/summary.html

Best Regards,
--
Fabiano Fidêncio



Hello,

CI passed:
http://sssd-ci.duckdns.org/logs/job/52/71/summary.html


0001-MONITOR-Remove-the-no-longer-used-diag_cmd-command.patch


From aa6204816cde0a7d75b9303916d038ed06e467ba Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Sun, 8 May 2016 14:41:35 +0200
Subject: [PATCH 1/3] MONITOR: Remove the no longer used diag_cmd command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After introducing the watchdog, the diag_cmd is longer used and makes no
sense trying to make it usable by watchdog as the result of "pstack %p"
seems next to useless in this context.

Co-author: Fabiano Fidêncio 

Related:
https://fedorahosted.org/sssd/ticket/3051
---



ACK



0002-MONITOR-Remove-the-no-longer-used-kill_service-comma.patch


From 7954e0254752d0a830a0501f23a6a93d0345e5ce Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Sun, 8 May 2016 14:46:25 +0200
Subject: [PATCH 2/3] MONITOR: Remove the no longer used kill_service
command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After introducing the watchdog, the force_timeout option is no longer
used.

Co-author: Fabiano Fidêncio 

Resolves:
https://fedorahosted.org/sssd/ticket/3052
---



ACK



0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch


From 1302c5a95ac36dd674c8795cda0082b84d30978d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Mon, 15 Aug 2016 12:54:20 +0200
Subject: [PATCH 3/3] WATCHDOG: define and use _MAX_TICKS as 3

Instead of using the number 3 directly, let's introduce and use
WATCHDOG_MAX_TICKS.
--



This patch is unfortunately inapplicable on top of master
(after two previous patches):

pcech@albireo ~/sssd: (master) $ git am
../patch/0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch
Applying: WATCHDOG: define and use _MAX_TICKS as 3
error: patch failed: src/util/util_watchdog.c:38
error: src/util/util_watchdog.c: patch does not apply
Patch failed at 0001 WATCHDOG: define and use _MAX_TICKS as 3

Regards


Rebase was quite simple.
See the v2 attached (the only change in v2 was the rebase).


Thanks, Fabiano,

I pushed patches to CI so quickly I missed
that the 3rd patch is LGTM and almost ACK :-)

I would like to wait to CI anyway.

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: MONITOR: Add disable_netlink sssd.conf option

2016-08-30 Thread Petr Cech 

On 08/27/2016 06:54 PM, Justin Stephenson wrote:

Hello,

The attached patches resolve https://fedorahosted.org/sssd/ticket/3142

However, I am having difficult with the man page addition to
'src/man/sssd.conf.5.xml' for this new option. I have stared at the open
and close xml tags(for far too long) and it looks correct but when I
build sssd I never see the sssd.conf man page inclusion. Could anyone
tell me what I am missing here?

If you feel there is better wording for the description please let me know.

Kind regards,
Justin Stephenson


Hello Justin,

CI passed:
http://sssd-ci.duckdns.org/logs/job/52/72/summary.html

I have one little comment about coding style. See below.



0001-MONITOR-Remove-disable-netlink-command-line-option.patch


From 0552c199dd37c7e280304b9bc92ff44a8a1a6d57 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Fri, 26 Aug 2016 15:15:32 -0400
Subject: [PATCH 1/2] MONITOR: Remove --disable-netlink command-line option

Removing monitor command-line option, to be superceded by
sssd.conf option
---


ACK



0002-MONITOR-Add-disable_netlink-option.patch


From c52c0c1a520cdf8509bac00fa3c7bec0dd73 Mon Sep 17 00:00:00 2001
From: Justin Stephenson 
Date: Fri, 26 Aug 2016 17:43:25 -0400
Subject: [PATCH 2/2] MONITOR: Add disable_netlink option

Adding a new monitor boolean option to disable netlink support.
This will give users more control over sssd state changes without
having to modify systemd unit files.

Resolves:
https://fedorahosted.org/sssd/ticket/3142
---

[...]


 /* Set up the environment variable for the Kerberos Replay Cache */
@@ -2471,14 +2472,28 @@ static int monitor_process_init(struct mt_ctx *ctx,
 return ret;
 }

-ret = setup_netlink(ctx, ctx->ev, network_status_change_cb,
-ctx, >nlctx);
+ret = confdb_get_bool(ctx->cdb,
+  CONFDB_MONITOR_CONF_ENTRY,
+  CONFDB_MONITOR_DISABLE_NETLINK,
+  false, _netlink);
+
 if (ret != EOK) {
 DEBUG(SSSDBG_OP_FAILURE,
-  "Cannot set up listening for network notifications\n");
+"Failed to read disable_netlink from confdb: [%d] %s\n",

 ^ --- this is right indentation

+ret, sss_strerror(ret));

 ^ --- this is right indentation

Please, fix this little nitpicking.

I am not native speaker, I am not able check
text in man page. (I guess you are.)

The first patch ACKed, the second needs
little work.

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: [SSSD} [PATCH] Remove no longer used code

2016-08-30 Thread Fabiano Fidêncio 
On Tue, Aug 30, 2016 at 8:23 AM, Petr Cech  wrote:
> On 08/15/2016 02:58 PM, Fabiano Fidêncio wrote:
>>
>> Those 3 patches are from Jakub and I've just done some minor
>> adjustments and add myself as co-author of the first 2 patches.
>>
>> CI has passed: http://sssd-ci.duckdns.org/logs/job/51/55/summary.html
>>
>> Best Regards,
>> --
>> Fabiano Fidêncio
>
>
> Hello,
>
> CI passed:
> http://sssd-ci.duckdns.org/logs/job/52/71/summary.html
>
>> 0001-MONITOR-Remove-the-no-longer-used-diag_cmd-command.patch
>>
>>
>> From aa6204816cde0a7d75b9303916d038ed06e467ba Mon Sep 17 00:00:00 2001
>> From: Jakub Hrozek 
>> Date: Sun, 8 May 2016 14:41:35 +0200
>> Subject: [PATCH 1/3] MONITOR: Remove the no longer used diag_cmd command
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> After introducing the watchdog, the diag_cmd is longer used and makes no
>> sense trying to make it usable by watchdog as the result of "pstack %p"
>> seems next to useless in this context.
>>
>> Co-author: Fabiano Fidêncio 
>>
>> Related:
>> https://fedorahosted.org/sssd/ticket/3051
>> ---
>
>
> ACK
>
>
>> 0002-MONITOR-Remove-the-no-longer-used-kill_service-comma.patch
>>
>>
>> From 7954e0254752d0a830a0501f23a6a93d0345e5ce Mon Sep 17 00:00:00 2001
>> From: Jakub Hrozek 
>> Date: Sun, 8 May 2016 14:46:25 +0200
>> Subject: [PATCH 2/3] MONITOR: Remove the no longer used kill_service
>> command
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> After introducing the watchdog, the force_timeout option is no longer
>> used.
>>
>> Co-author: Fabiano Fidêncio 
>>
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/3052
>> ---
>
>
> ACK
>
>
>> 0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch
>>
>>
>> From 1302c5a95ac36dd674c8795cda0082b84d30978d Mon Sep 17 00:00:00 2001
>> From: Jakub Hrozek 
>> Date: Mon, 15 Aug 2016 12:54:20 +0200
>> Subject: [PATCH 3/3] WATCHDOG: define and use _MAX_TICKS as 3
>>
>> Instead of using the number 3 directly, let's introduce and use
>> WATCHDOG_MAX_TICKS.
>> --
>
>
> This patch is unfortunately inapplicable on top of master
> (after two previous patches):
>
> pcech@albireo ~/sssd: (master) $ git am
> ../patch/0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch
> Applying: WATCHDOG: define and use _MAX_TICKS as 3
> error: patch failed: src/util/util_watchdog.c:38
> error: src/util/util_watchdog.c: patch does not apply
> Patch failed at 0001 WATCHDOG: define and use _MAX_TICKS as 3
>
> Regards

Rebase was quite simple.
See the v2 attached (the only change in v2 was the rebase).

>
> ---
>
> Petr^4 Čech
> ___
> sssd-devel mailing list
> sssd-devel@lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
From 7579cf9982c86978500e9249ad3e82124867fc90 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Sun, 8 May 2016 14:41:35 +0200
Subject: [PATCH v2 1/3] MONITOR: Remove the no longer used diag_cmd command

After introducing the watchdog, the diag_cmd is longer used and makes no
sense trying to make it usable by watchdog as the result of "pstack %p"
seems next to useless in this context.

Related:
https://fedorahosted.org/sssd/ticket/3051
---
 src/confdb/confdb.h   |   1 -
 src/monitor/monitor.c | 163 --
 2 files changed, 164 deletions(-)

diff --git a/src/confdb/confdb.h b/src/confdb/confdb.h
index 72adbd8..58a085b 100644
--- a/src/confdb/confdb.h
+++ b/src/confdb/confdb.h
@@ -73,7 +73,6 @@
 #define CONFDB_MONITOR_DEFAULT_DOMAIN "default_domain_suffix"
 #define CONFDB_MONITOR_OVERRIDE_SPACE "override_space"
 #define CONFDB_MONITOR_USER_RUNAS "user"
-#define CONFDB_MONITOR_PRE_KILL_CMD "diag_cmd"
 #define CONFDB_MONITOR_CERT_VERIFICATION "certificate_verification"
 
 /* Both monitor and domains */
diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 7a9ef56..f97b2a9 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -112,7 +112,6 @@ struct mt_svc {
 char *identity;
 pid_t pid;
 
-char *diag_cmd;
 int kill_time;
 
 struct tevent_timer *kill_timer;
@@ -373,77 +372,6 @@ static int add_svc_conn_spy(struct mt_svc *svc)
 return EOK;
 }
 
-static char *expand_diag_cmd(struct mt_svc *svc,
- const char *template)
-{
-TALLOC_CTX *tmp_ctx = NULL;
-char *copy;
-char *p_copy;
-char *n;
-char *result = NULL;
-char action;
-char *res = NULL;
-
-if (template == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE, "Missing template.\n");
-return NULL;
-}
-
-tmp_ctx = talloc_new(NULL);
-if (!tmp_ctx) return NULL;
-
-copy = talloc_strdup(tmp_ctx, template);
-if (copy == NULL) {
-DEBUG(SSSDBG_CRIT_FAILURE, "talloc_strdup

[SSSD] Re: [SSSD} [PATCH] Remove no longer used code

2016-08-30 Thread Petr Cech 

On 08/15/2016 02:58 PM, Fabiano Fidêncio wrote:

Those 3 patches are from Jakub and I've just done some minor
adjustments and add myself as co-author of the first 2 patches.

CI has passed: http://sssd-ci.duckdns.org/logs/job/51/55/summary.html

Best Regards,
--
Fabiano Fidêncio


Hello,

CI passed:
http://sssd-ci.duckdns.org/logs/job/52/71/summary.html


0001-MONITOR-Remove-the-no-longer-used-diag_cmd-command.patch


From aa6204816cde0a7d75b9303916d038ed06e467ba Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Sun, 8 May 2016 14:41:35 +0200
Subject: [PATCH 1/3] MONITOR: Remove the no longer used diag_cmd command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After introducing the watchdog, the diag_cmd is longer used and makes no
sense trying to make it usable by watchdog as the result of "pstack %p"
seems next to useless in this context.

Co-author: Fabiano Fidêncio 

Related:
https://fedorahosted.org/sssd/ticket/3051
---


ACK



0002-MONITOR-Remove-the-no-longer-used-kill_service-comma.patch


From 7954e0254752d0a830a0501f23a6a93d0345e5ce Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Sun, 8 May 2016 14:46:25 +0200
Subject: [PATCH 2/3] MONITOR: Remove the no longer used kill_service command
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After introducing the watchdog, the force_timeout option is no longer
used.

Co-author: Fabiano Fidêncio 

Resolves:
https://fedorahosted.org/sssd/ticket/3052
---


ACK



0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch


From 1302c5a95ac36dd674c8795cda0082b84d30978d Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Mon, 15 Aug 2016 12:54:20 +0200
Subject: [PATCH 3/3] WATCHDOG: define and use _MAX_TICKS as 3

Instead of using the number 3 directly, let's introduce and use
WATCHDOG_MAX_TICKS.
--


This patch is unfortunately inapplicable on top of master
(after two previous patches):

pcech@albireo ~/sssd: (master) $ git am 
../patch/0003-WATCHDOG-define-and-use-_MAX_TICKS-as-3.patch

Applying: WATCHDOG: define and use _MAX_TICKS as 3
error: patch failed: src/util/util_watchdog.c:38
error: src/util/util_watchdog.c: patch does not apply
Patch failed at 0001 WATCHDOG: define and use _MAX_TICKS as 3

Regards

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