[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Tue, Sep 6, 2016 at 8:49 PM, lslebodn
 wrote:
> lslebodn commented on a pull request
>
> """
> IMHO, it might be better to close this PR.
> If we decide to dor support for libini_config < 1.1 or 1.2
> then it will be a different patch anyway. @see my previous comment
> """

Please, take a look on Jakub's comment and see what he proposed.

Anyways, I'm fine with closing this PR and opening a new one when we
can drop support to libini_config < 1.3.

Just for the record, from this whole discussion I could see that
dropping support to augeas in order to use libini is something that
shouldn't and is not going to happen any time soon as well and for the
very same reasons that this patch wasn't accepted. We could, in the
best (or worst?) case scenario support/depend on both due to
compatibility with elder systems, but I can't see any advantage on
that.

Best Regards,
-- 
Fabiano Fidêncio
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[lslebodn]

lslebodn commented on a pull request

"""
IMHO, it might be better to close this PR.
If we decide to dor support for libini_config < 1.1 or 1.2
then it will be a different patch anyway. @see my previous comment
"""

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

[SSSD] [sssd PR#16] TOOLS: sss_groupshow did not work (comment)

[lslebodn]

lslebodn commented on a pull request

"""
On (06/09/16 10:25), mzidek-rh wrote:
>This PR includes some fixes for local provider. Namely jhrozek's patch for 
>sss_groupadd and fix for sss_groupshow. I will add more fixes (sss_usermod 
>etc.) in another PR tomorrow.
>
Try to run pep8 src/tests/intg/test_local_domain.py
before submiting patches :-)

LS

"""

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

[SSSD] [sssd PR#16] TOOLS: sss_groupshow did not work (synchronize)

[mzidek-rh]

mzidek-rh's pull request #16: "TOOLS: sss_groupshow did not work" was 
synchronize

See the full pull-request at https://github.com/SSSD/sssd/pull/16
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/16/head:pr16
git checkout pr16
From 0bcb9ce46d91129516ab0b1d4862856e0e2af88f Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 6 Sep 2016 12:13:08 +0200
Subject: [PATCH 1/3] TOOLS: Fix a typo in groupadd()

Resolves:
https://fedorahosted.org/sssd/ticket/3173
---
 src/tools/sss_sync_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/sss_sync_ops.c b/src/tools/sss_sync_ops.c
index a23a0b8..39ef5be 100644
--- a/src/tools/sss_sync_ops.c
+++ b/src/tools/sss_sync_ops.c
@@ -657,7 +657,7 @@ int groupadd(struct ops_ctx *data)
 int ret;
 
 data->sysdb_fqname = sss_create_internal_fqname(data,
-data->sysdb_fqname,
+data->name,
 data->domain->name);
 if (data->sysdb_fqname == NULL) {
 return ENOMEM;

From 48449165abbbfb807dc928085cce4d6f50af96df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 6 Sep 2016 13:46:53 +0200
Subject: [PATCH 2/3] TOOLS: sss_groupshow did not work

sss_groupshow used shortname to search
in sysdb database. We have to u e sysdb_fqname
(aka internal_fqname) format for all sysdb
oprations.

Resolves:
https://fedorahosted.org/sssd/ticket/3175
---
 src/tools/sss_groupshow.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/tools/sss_groupshow.c b/src/tools/sss_groupshow.c
index 41d7475..5870cc8 100644
--- a/src/tools/sss_groupshow.c
+++ b/src/tools/sss_groupshow.c
@@ -318,7 +318,7 @@ int group_show(TALLOC_CTX *mem_ctx,
struct sysdb_ctx *sysdb,
struct sss_domain_info *domain,
bool   recursive,
-   const char *name,
+   const char *shortname,
struct group_info **res)
 {
 struct group_info *root;
@@ -326,11 +326,20 @@ int group_show(TALLOC_CTX *mem_ctx,
 struct ldb_message *msg = NULL;
 const char **group_members = NULL;
 int nmembers = 0;
+char *sysdb_fqname = NULL;
 int ret;
 int i;
 
+sysdb_fqname = sss_create_internal_fqname(mem_ctx,
+  shortname,
+  domain->name);
+if (sysdb_fqname == NULL) {
+return ENOMEM;
+}
+
 /* First, search for the root group */
-ret = sysdb_search_group_by_name(mem_ctx, domain, name, attrs, );
+ret = sysdb_search_group_by_name(mem_ctx, domain, sysdb_fqname, attrs,
+ );
 if (ret) {
 DEBUG(SSSDBG_OP_FAILURE,
   "Search failed: %s (%d)\n", strerror(ret), ret);

From d81f24e86b9e4b0bff730b86afd355bdcfd29be2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20=C5=BDidek?= 
Date: Tue, 6 Sep 2016 17:37:14 +0200
Subject: [PATCH 3/3] TESTS: sss_groupadd/groupshow regressions

Adds regression CI test for ticket #3173
and #3175.

Resolves:
https://fedorahosted.org/sssd/ticket/3173
https://fedorahosted.org/sssd/ticket/3175
---
 src/tests/intg/test_local_domain.py | 24 
 1 file changed, 24 insertions(+)

diff --git a/src/tests/intg/test_local_domain.py b/src/tests/intg/test_local_domain.py
index b83e56d..acae43a 100644
--- a/src/tests/intg/test_local_domain.py
+++ b/src/tests/intg/test_local_domain.py
@@ -19,11 +19,13 @@
 import os
 import stat
 import pwd
+import grp
 import time
 import config
 import signal
 import subprocess
 import pytest
+import ent
 from util import unindent
 
 
@@ -89,6 +91,10 @@ def assert_nonexistent_user(name):
 with pytest.raises(KeyError):
 pwd.getpwnam(name)
 
+def assert_nonexistent_group(name):
+with pytest.raises(KeyError):
+grp.getgrnam(name)
+
 
 def test_wrong_LC_ALL(local_domain_only):
 """
@@ -107,3 +113,21 @@ def test_wrong_LC_ALL(local_domain_only):
 subprocess.check_call(["sss_userdel", "foo", "-R"])
 assert_nonexistent_user("foo")
 os.environ["LC_ALL"] = oldvalue
+
+def test_sss_group_add_show_del(local_domain_only):
+"""
+Regression test for tickets
+https://fedorahosted.org/sssd/ticket/3173
+https://fedorahosted.org/sssd/ticket/3175
+"""
+
+subprocess.check_call(["sss_groupadd", "foo", "-g", "10001"])
+
+"This should not raise KeyError"
+ent.assert_group_by_name("foo", dict(name="foo", gid=10001))
+
+"sss_grupshow should return 0 with existing group name"
+subprocess.check_call(["sss_groupshow", "foo"])
+
+subprocess.check_call(["sss_groupdel", "foo"])
+assert_nonexistent_group("foo")
___
sssd-devel mailing list

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

[Petr Cech]

On 09/06/2016 05:11 PM, Justin Stephenson wrote:

On 09/06/2016 10:57 AM, Petr Cech wrote:

On 09/06/2016 04:17 PM, Justin Stephenson wrote:



On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

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
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything
that
was requested during the review process. However, I'm not
comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to
have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

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







Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone
...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is
useful for larger environments" may work - whichever you prefer.

Kind regards,
Justin Stephenson


Hello,

new version attached. Description is:

This option specifies the number of pre-forked
proxy children. It is useful for high-load SSSD
environments where sssd may run out of available
child slots, which would cause some issues due to
the requests being queued.

After hint from Michal I replaced introduces by specifies.

Is this version linguistic right?


This description looks good to me.

-Justin


Thanks, Justin.

--
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] PROXY: Adding proxy_max_children option

[Justin Stephenson]

On 09/06/2016 10:57 AM, Petr Cech wrote:

On 09/06/2016 04:17 PM, Justin Stephenson wrote:



On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

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
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to
have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

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






Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone
...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is
useful for larger environments" may work - whichever you prefer.

Kind regards,
Justin Stephenson


Hello,

new version attached. Description is:

This option specifies the number of pre-forked
proxy children. It is useful for high-load SSSD
environments where sssd may run out of available
child slots, which would cause some issues due to
the requests being queued.

After hint from Michal I replaced introduces by specifies.

Is this version linguistic right?


This description looks good to me.

-Justin



Regards



___
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] PROXY: Adding proxy_max_children option

[Justin Stephenson]

On 09/05/2016 10:20 AM, Petr Cech wrote:

On 09/05/2016 04:05 PM, Fabiano Fidêncio wrote:

Petr,

On Mon, Sep 5, 2016 at 3:43 PM, Petr Cech  wrote:

On 09/05/2016 09:57 AM, Fabiano Fidêncio wrote:


Petr,

I see you have updated the OPT_MAX_CHILDREN_DEFAULT to 10 instead of
50. However, you haven't update the value on sssd.conf.5.xml:

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
   here: ^^^

+
+
+
+
 
 

Apart from this minor the patch seems to be following everything that
was requested during the review process. However, I'm not comfortable
with the text used to describe the new option, so adding there a bit
more information would be super. Like, I don't know what's the
influence of the preforked proxy children to the rest of the code
(probably because I'm a newbie here ;-)), but would be nice to have it
clear in the documentation (for newbies like myself ;-)).

Best Regards,
--
Fabiano Fidêncio



Hi Fabiano,

thanks for code review. I fixed the default value in man page and I
reformulated description. Is it better?

Regards

--
Petr^4 Čech

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




Looking at the changes in the manual ...

+In busy environments it is possible sssd
runs out of
+available child slots and starts queuing
requests
+in proxy mode. This option introduces the
number of
+preforked proxy children.

I personally go for something like:

"This option introduces the number of pre-forked proxy children. It's
useful for busy environments* where sssd may run out of available
child slots, which would cause some issues due to the requests being
queued".

*: Not sure whether busy environments is something clear for everyone ...

IMO the patch is good to go as soon as we have this part done/reviewed
by a native speaker.
Maybe Justin can help us here?

Best Regards,
--
Fabiano Fidêncio


Fabiano,

I took your suggestion, thanks. I don't know right term for 'busy
environments'. I will be glad if native speaker help me with the right
formulation of description.


Something like "it is useful for high-load SSSD environments" or "it is 
useful for larger environments" may work - whichever you prefer.


Kind regards,
Justin Stephenson


Regards



___
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#15] Avoid returning System Error on clock skew (comment)

[jhrozek]

jhrozek commented on a pull request

"""
good idea
"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (-ack)

[jhrozek]

jhrozek's pull request #15: "Avoid returning System Error on clock skew" label 
*ack* has been removed

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

[SSSD] [sssd PR#16] TOOLS: sss_groupshow did not work (+nack)

[jhrozek]

mzidek-rh's pull request #16: "TOOLS: sss_groupshow did not work" label *nack* 
has been added

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (+nack)

[jhrozek]

jhrozek's pull request #15: "Avoid returning System Error on clock skew" label 
*nack* has been added

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (comment)

[sumit-bose]

sumit-bose commented on a pull request

"""
On Tue, Sep 06, 2016 at 04:51:26AM -0700, Jakub Hrozek wrote:
> Thanks for the ack, I would also like to ask @sumit-bose if he agrees with 
> the change.

Since we already handle other clock skew related error codes the same
way I think it is ok. But I wonder if a clock-skew is something which
qualifies for a syslog messages to make the admin aware that there is
something which needs fixing?

> 
> -- 
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub:
> https://github.com/SSSD/sssd/pull/15#issuecomment-244927163

"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (comment)

[sumit-bose]

sumit-bose commented on a pull request

"""
On Tue, Sep 06, 2016 at 05:37:14AM -0700, Pavel Březina wrote:
> On 09/06/2016 02:21 PM, Jakub Hrozek wrote:
> > On Tue, Sep 06, 2016 at 05:10:07AM -0700, Pavel Březina wrote:
> >  > On 09/06/2016 01:51 PM, Jakub Hrozek wrote:
> >  > > Thanks for the ack, I would also like to ask @sumit-bose
> >  > >  if he agrees with the change.
> >  >
> >  > Btw since clock skew is not fatal anymore, is it possible for us to
> >  > actually perform online authentication?
> >
> > The TGT times are generated on the server and the error usually happens
> > only when the client attempts to use the TGT for something like FAST
> > tunnel establishment or TGT validation. See Sumit's reply here:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1373427#c4
> >
> > (that's why I wanted him to confirm this is a good idea)
> 
> AFAIK kinit's magic applies clock skew to the timestamp in the ticket 
> and compares times within the client's range. I'm just asking if there 
> is something similar we can do in SSSD:

iirc this magic is applied to the timestamps used in the "default"
pre-authentication method where encrypted timestamps are send around.
This does not related to the timestamps in the tickets.

> 
> 
> 
> -- 
> You are receiving this because you were mentioned.
> Reply to this email directly or view it on GitHub:
> https://github.com/SSSD/sssd/pull/15#issuecomment-244936798

"""

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

[SSSD] [sssd PR#16] TOOLS: sss_groupshow did not work (comment)

[lslebodn]

lslebodn commented on a pull request

"""
On (06/09/16 05:40), mzidek-rh wrote:
>sss_groupshow used shortname to search
>in sysdb database. We have to u e sysdb_fqname
>(aka internal_fqname) format for all sysdb
>oprations.
>
>Resolves:
>https://fedorahosted.org/sssd/ticket/3175
I miss an integration test in src/tests/intg/test_local_domain.py

LS

"""

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

[SSSD] Re: [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[Fabiano Fidêncio]

On Tue, Sep 6, 2016 at 2:30 PM, jhrozek
 wrote:
> jhrozek commented on a pull request
>
> """
> On Tue, Sep 06, 2016 at 05:16:00AM -0700, fidencio wrote:
>> The distributions that would break with this patch are:
>> - RHEL/CentOS 5 and older
>
> I don't think we care about RHEL-5 with master and I'm not sure sssd
> master even builds there.
>
>> - Debian Wheezy (from 2013) and older
>
> OK, this one I think we care about for the basic functionality:
> 7.0 Wheezy May 4th 2013 April 26th 2016 (full) / May 2018 (LTS)
> But LTS means bug fixes and for those I think sssd-1-13 would be OK.
>
>> - Ubuntu 12.04 LTS and older
>
> 12.04 is often used (for example travis-ci still offers only this
> distro) and ends its life in 2017.
>
> On one hand, it's unlikely that users of LTS distributions will run
> master, they will probably only run sssd-1-13 or some PPAs, on the other
> hand, I don't see too much value except for cleaner code.
>
> So all in all I suggest we nack this patchset for now and push it when
> Ubuntu 12.04 goes EOL.
>
> Does that sound like a reasonable compromise?

Works for me.

>
> """
>
> See the full comment at 
> https://github.com/SSSD/sssd/pull/10#issuecomment-244935329
>
> ___
> 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#15] Avoid returning System Error on clock skew (comment)

[pbrezina]

pbrezina commented on a pull request

"""
On 09/06/2016 02:21 PM, Jakub Hrozek wrote:
> On Tue, Sep 06, 2016 at 05:10:07AM -0700, Pavel Březina wrote:
>  > On 09/06/2016 01:51 PM, Jakub Hrozek wrote:
>  > > Thanks for the ack, I would also like to ask @sumit-bose
>  > >  if he agrees with the change.
>  >
>  > Btw since clock skew is not fatal anymore, is it possible for us to
>  > actually perform online authentication?
>
> The TGT times are generated on the server and the error usually happens
> only when the client attempts to use the TGT for something like FAST
> tunnel establishment or TGT validation. See Sumit's reply here:
> https://bugzilla.redhat.com/show_bug.cgi?id=1373427#c4
>
> (that's why I wanted him to confirm this is a good idea)

AFAIK kinit's magic applies clock skew to the timestamp in the ticket 
and compares times within the client's range. I'm just asking if there 
is something similar we can do in SSSD:


"""

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

[SSSD] [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[jhrozek]

jhrozek commented on a pull request

"""
On Tue, Sep 06, 2016 at 05:16:00AM -0700, fidencio wrote:
> The distributions that would break with this patch are:
> - RHEL/CentOS 5 and older

I don't think we care about RHEL-5 with master and I'm not sure sssd
master even builds there.

> - Debian Wheezy (from 2013) and older

OK, this one I think we care about for the basic functionality:
7.0 Wheezy May 4th 2013 April 26th 2016 (full) / May 2018 (LTS)
But LTS means bug fixes and for those I think sssd-1-13 would be OK.

> - Ubuntu 12.04 LTS and older

12.04 is often used (for example travis-ci still offers only this
distro) and ends its life in 2017.

On one hand, it's unlikely that users of LTS distributions will run
master, they will probably only run sssd-1-13 or some PPAs, on the other
hand, I don't see too much value except for cleaner code.

So all in all I suggest we nack this patchset for now and push it when
Ubuntu 12.04 goes EOL.

Does that sound like a reasonable compromise?

"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (comment)

[jhrozek]

jhrozek commented on a pull request

"""
On Tue, Sep 06, 2016 at 05:10:07AM -0700, Pavel Březina wrote:
> On 09/06/2016 01:51 PM, Jakub Hrozek wrote:
> > Thanks for the ack, I would also like to ask @sumit-bose
> >  if he agrees with the change.
> 
> Btw since clock skew is not fatal anymore, is it possible for us to 
> actually perform online authentication?

The TGT times are generated on the server and the error usually happens
only when the client attempts to use the TGT for something like FAST
tunnel establishment or TGT validation. See Sumit's reply here:
https://bugzilla.redhat.com/show_bug.cgi?id=1373427#c4

(that's why I wanted him to confirm this is a good idea)

"""

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

[SSSD] [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[fidencio]

fidencio commented on a pull request

"""
The distributions that would break with this patch are:
- RHEL/CentOS 5 and older
- Debian Wheezy (from 2013) and older
- Ubuntu 12.04 LTS and older

I was not able to find what's the version of the package on SLES
"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (comment)

[pbrezina]

pbrezina commented on a pull request

"""
On 09/06/2016 01:51 PM, Jakub Hrozek wrote:
> Thanks for the ack, I would also like to ask @sumit-bose
>  if he agrees with the change.

Btw since clock skew is not fatal anymore, is it possible for us to 
actually perform online authentication?

"""

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

[SSSD] [sssd PR#13] MEMBEROF: Don't resolve members if they are removed (comment)

[jhrozek]

jhrozek commented on a pull request

"""
On Fri, Sep 02, 2016 at 08:37:07AM -0700, celestian wrote:
> I wrote the conditions in fully way for clarity. Yes, it is possible to write 
> it in condensed way.
> 
> My question in note is about which type of our CI I can use. It is in 
> memberof plugin. I don't see any test for memberof plugin at all.

Look into src/tests/sysdb-tests.c, search for "tc_memberof". There is
already quite a few tests for the memberof plugin.

"""

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

[SSSD] [sssd PR#13] MEMBEROF: Don't resolve members if they are removed (+nack)

[jhrozek]

celestian's pull request #13: "MEMBEROF: Don't resolve members if they are 
removed" label *nack* has been added

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

[SSSD] [sssd PR#10] UTIL: Remove support to libini older than 1.0.0 (comment)

[jhrozek]

jhrozek commented on a pull request

"""
On Thu, Sep 01, 2016 at 09:01:54AM -0700, lslebodn wrote:
> On (01/09/16 08:35), fidencio wrote:
> >On Thu, Sep 1, 2016 at 2:10 PM, lslebodn  wrote:
> >
> >> On (01/09/16 04:32), Jakub Hrozek wrote:
> >> >On Thu, Sep 01, 2016 at 03:21:06AM -0700, fidencio wrote:
> >> >> On Thu, Sep 1, 2016 at 11:54 AM, lslebodn 
> >> wrote:
> >> >> >
> >> >> > On (31/08/16 23:18), fidencio wrote:
> >> >> > >libini 1.0.0 is part of ding-libs 0.3.0 and has been around since
> >> 2013.
> >> >> > >Even the old systems that we have to support already have a newer
> >> >> > >version of the library. RHEL6, for instance, has ding-libs 0.4.0
> >> which
> >> >> > >provides libinit 1.1.0.
> >> >> > >
> >> >> > >By removing this code we also can stop depending on libcollection.
> >> >> > >
> >> >> > >Signed-off-by: Fabiano Fidêncio 
> >> >> > >You can view, comment on, or merge this pull request online at:
> >> >> > >
> >> >> > > https://github.com/SSSD/sssd/pull/10
> >> >> > >
> >> >> > >-- Commit Summary --
> >> >> > >
> >> >> > > * UTIL: Remove support to libini older than 1.0.0
> >> >> > >
> >> >> > >-- File Changes --
> >> >> > >
> >> >> > > M configure.ac (1)
> >> >> > > M contrib/ci/deps.sh (1)
> >> >> > > M contrib/sssd.spec.in (1)
> >> >> > > D src/external/libcollection.m4 (9)
> >> >> > > M src/util/sss_ini.c (97)
> >> >> > >
> >> >> > >-- Patch Links --
> >> >> > >
> >> >> > >https://github.com/SSSD/sssd/pull/10.patch
> >> >> > >https://github.com/SSSD/sssd/pull/10.diff
> >> >> > >
> >> >> > OpenSUSE LEAP has just ding-libs 0.3.0.1 in official repositories.
> >> >> > http://software.opensuse.org/package/ding-libs?search_term=ding-libs
> >> >>
> >> >> Yep. OpenSUSE LEAP has 0.3.0.1, Debian Stable (Jessie) has 0.4.0,
> >> >> latest Ubuntu LTS has 0.5.0.
> >> >> And all of them would be able to build SSSD with my patch without any
> >> issues.
> >> >
> >> >Two questions:
> >> > 1) how long until the distributions with too old ding-libs go out of
> >> > support?
> >>
> >
> >Hmm. Unfortunately I don't have an answer for you.
> >What are the major distributions that we want to support? Debian, Ubuntu
> >LTS, RHEL, SLES ...?
> >
> I would prefer if limited version of sssd (ldap + krb5 provider)
> could be compiled almost anywhere. (even old distributions)
> It is not only about major distributions.
> 
> >
> >> > 2) since we will be (likely) supporting sssd-1-13 for the lifetime
> >> > of RHEL-6, can we say that the old distributions just use sssd-1-13?
> >> >
> >> libini_config-1.0 does not provide any new functionality
> >> which is not in libini_config < 1.0
> >>
> >
> >> They have just a different API (and moreover libini_config-1.0
> >> still provides old API) (at least from sssd POV)
> >>
> >
> >There's no new functionality, true. But there's a quite good API
> >simplification
> I agree that API is better.
> But feature wise are the same. So if we wanted to drop
> support for libini_config < 1.0 then we could
> drop support for libini_config-1.0.
> It would simplify more things in sssd
> 
> However, OpenSUSE LEAP has just a libini_config-1.0

This PR has stalled somehow. Can we either move it forward or reject?

I admit I got a bit lost in the comments, which distributions would we
break with this patch? Are any of them supported/will be supported in th
near future?

"""

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (comment)

[jhrozek]

jhrozek commented on a pull request

"""
On Tue, Sep 06, 2016 at 04:54:29AM -0700, lslebodn wrote:
> On (06/09/16 03:14), Jakub Hrozek wrote:
> >The log said:
> >(Tue Sep  6 15:18:31:935105 2016) [sss_groupadd] 
> >[sysdb_timestamp_cache_connect] (0x0400): No timestamp cache for LOCAL
> >(Tue Sep  6 15:18:31:935255 2016) [sss_groupadd] [sss_names_init_from_args] 
> >(0x0100): Using re [(?P[^@]+)@?(?P[^@]*$)].
> >(Tue Sep  6 15:18:31:935277 2016) [sss_groupadd] [sss_fqnames_init] 
> >(0x0100): Using fq format [%1$s@%2$s].
> >(Tue Sep  6 15:18:31:935392 2016) [sss_groupadd] [parse_name_domain] 
> >(0x0200): Parsed username: group1
> >(Tue Sep  6 15:18:31:935418 2016) [sss_groupadd] [ldb] (0x4000): start ldb 
> >transaction (nesting: 0)
> >(Tue Sep  6 15:18:31:935504 2016) [sss_groupadd] [ldb] (0x4000): cancel ldb 
> >transaction (nesting: 0)
> >(Tue Sep  6 15:18:31:935629 2016) [sss_groupadd] [main] (0x0020): sysdb 
> >operation failed (12)[Cannot allocate memory]
> 
> NACK.
> 
> I miss an integration test.
> We already have some for localprovider in src/tests/intg/test_local_domain.py

Fair enough, I will add it later.

"""

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (+nack)

[jhrozek]

jhrozek's pull request #14: "Attempting to create a qualified name from a NULL 
pointer broke sss_groupadd" label *nack* has been added

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (comment)

[lslebodn]

lslebodn commented on a pull request

"""
On (06/09/16 03:14), Jakub Hrozek wrote:
>The log said:
>(Tue Sep  6 15:18:31:935105 2016) [sss_groupadd] 
>[sysdb_timestamp_cache_connect] (0x0400): No timestamp cache for LOCAL
>(Tue Sep  6 15:18:31:935255 2016) [sss_groupadd] [sss_names_init_from_args] 
>(0x0100): Using re [(?P[^@]+)@?(?P[^@]*$)].
>(Tue Sep  6 15:18:31:935277 2016) [sss_groupadd] [sss_fqnames_init] (0x0100): 
>Using fq format [%1$s@%2$s].
>(Tue Sep  6 15:18:31:935392 2016) [sss_groupadd] [parse_name_domain] (0x0200): 
>Parsed username: group1
>(Tue Sep  6 15:18:31:935418 2016) [sss_groupadd] [ldb] (0x4000): start ldb 
>transaction (nesting: 0)
>(Tue Sep  6 15:18:31:935504 2016) [sss_groupadd] [ldb] (0x4000): cancel ldb 
>transaction (nesting: 0)
>(Tue Sep  6 15:18:31:935629 2016) [sss_groupadd] [main] (0x0020): sysdb 
>operation failed (12)[Cannot allocate memory]

NACK.

I miss an integration test.
We already have some for localprovider in src/tests/intg/test_local_domain.py

"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (comment)

[jhrozek]

jhrozek commented on a pull request

"""
Thanks for the ack, I would also like to ask @sumit-bose if he agrees with the 
change.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/15#issuecomment-244927163
___
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

[Petr Cech]

On 09/06/2016 01:15 PM, Petr Cech wrote:

On 09/05/2016 02:31 PM, Fabiano Fidêncio wrote:

On Mon, Sep 5, 2016 at 11:59 AM, Fabiano Fidêncio
 wrote:

Petr,

I went through your patches and in general they look good to me.
However, I haven't done any tests yet with your patches (and I'll do
it after lunch).


I've done some tests and I've been able to see the ldif changes in the
domain log. So, I assume it's working.
For sure it's a good improvement! Would be worth to link some
documentation about ldiff as it may be confusing for someone who is
not used to it.

I'll wait for a new version of the patches and go through them again.

I really would like to have someone's else opinion on this series.



Please, below you can see a few comments. Feel completely free to
ignore the first one if you feel like doing it, it's just a minor :-)
For the other comments, I'd like to understand a few changes you have
done.


Patch 0001: SYSDB: Adding message to inform which cache is used

About the following part of the patch:
+static const char *get_attr_storage(int state_mask)
+{
+const char *storage = "";
+
+if (state_mask == SSS_SYSDB_BOTH_CACHE ) {
+storage = "cache, ts_cache";
+} else if (state_mask == SSS_SYSDB_TS_CACHE) {
+storage = "ts_cache";
+} else if (state_mask == SSS_SYSDB_CACHE) {
+storage = "cache";
+}
+
+return storage;
+}

I personally don't like this kind of comparison done with flags. I'd
go for something like: if ((state_mask & SSS_SYSDB_BOTH_CACHE) != 0)
...
But this is a really minor and feel free to ignore it.


Patch 0002: SYSDB: Adding message about reason why cache changed

LGTM


Patch 0003: SYSDB: Adding wrappers for ldb_* operations

About the following parts of the patch:

On src/db/sysdb_ldb_wrapper.c

+#define ERR_FN_ENOMEM (-1 * ENOMEM)
+#define ERR_FN_ENOENT (-1 * ENOENT)

Why? I failed to understand why you're doing this here.

+if (print_ctx == NULL) {
+return -1;
+return ERR_FN_ENOMEM;
+}

I guess the return -1 is a leftover :-)

+if (print_ctx->ldif == NULL) {
+return -2;
+return ERR_FN_ENOENT;
+}

I guess the return -2 is also a leftover :-)

+if (ret < 0) {
+DEBUG(SSSDBG_MINOR_FAILURE, "ldb_ldif_write() failed with
[%d][%s].\n",
+-1 * ret, sss_strerror(-1 * ret));
+goto done;
+}

And here again this dance multiplying by -1 that I don't understand
the reason :-\

+done:
+if (ldb_print_ctx != NULL && ldb_print_ctx->ldif != NULL) {
+talloc_free(ldb_print_ctx->ldif);
+}
+talloc_free(ldb_print_ctx);

AFAIU talloc_free can gracefully handle NULL. Considering that's the
case I'd just check for (if ldb_print_ctx != NULL)
talloc_free(ldb_print_ctx->ldif);
Considering it doesn't, we may have some issues on trying to free
(ldb_print_ctx)

On src/db/sysdb_ldb_wrapper.h:

+int sss_ldb_rename(struct ldb_context *ldb,
+   struct ldb_dn * olddn,
+   struct ldb_dn *newdn);

Just a really minor codying style change here, remove the extra space
between * and olddn: struct ldb_dn * olddn,  ->  struct ldb_dn *olddn,


Patch0004: SYSDB: ldb_add --> sss_ldb_add in sysdb
Patch0005: SYSDB: ldb_delete --> sss_ldb_delete in sysdb
Patch0006: SYSDB: ldb_modify --> sss_ldb_modify in sysdb
Patch0007: SYSDB: ldb_rename --> sss_ldb_rename in sysdb

LGTM


Best Regards,
--
Fabiano Fidêncio


Hello,


there is new patch set attached.
I replaced all ldb_* to new wrapper in whole code.


I wondered if my VM could push patches to our CI.
I will link the CI results after they finish.

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (comment)

[mzidek-rh]

mzidek-rh commented on a pull request

"""
I see the error with the sss_groupshow. Will send patch in another PR.
"""

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (comment)

[mzidek-rh]

mzidek-rh commented on a pull request

"""
The patch works. I pushed it to CI, waiting for results.

As I tested the local provider now, I found that the sss_groupshow does not 
seem to work. Will check why. But it is not related to this patch.
"""

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (+ack)

[fidencio]

jhrozek's pull request #14: "Attempting to create a qualified name from a NULL 
pointer broke sss_groupadd" label *ack* has been added

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

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (comment)

[fidencio]

fidencio commented on a pull request

"""
Patch works as expected but I wasn't able to run CI tests on it* due to some 
connection issue to the CI machine.

*: Well, I ran it locally and it passed.

Please, if possible, run the CI as feel free to push as soon as it passes.

"""

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

[SSSD] [sssd PR#15] Avoid returning System Error on clock skew (opened)

[jhrozek]

jhrozek's pull request #15: "Avoid returning System Error on clock skew" was 
opened

PR body:
"""
Rather return ERR_NETWORK_IO and at least let the user authenticate offline.
"""

See the full pull-request at https://github.com/SSSD/sssd/pull/15
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/15/head:pr15
git checkout pr15
From 939d731fba1d367324cf128ef9596d530a289f21 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 6 Sep 2016 12:27:51 +0200
Subject: [PATCH] KRB5: Return ERR_NETWORK_IO on clock skew

Adds two more return codes to the list of codes we translate to
ERR_NETWORK_IO.

Resolves:
https://fedorahosted.org/sssd/ticket/3174
---
 src/providers/krb5/krb5_child.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index a0a0f74..8252299 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -1374,6 +1374,8 @@ static errno_t map_krb5_error(krb5_error_code kerr)
 
 case KRB5_KDCREP_SKEW:
 case KRB5KRB_AP_ERR_SKEW:
+case KRB5KRB_AP_ERR_TKT_EXPIRED:
+case KRB5KRB_AP_ERR_TKT_NYV:
 case KRB5_KDC_UNREACH:
 case KRB5_REALM_CANT_RESOLVE:
 case KRB5_REALM_UNKNOWN:
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (synchronize)

[jhrozek]

jhrozek's pull request #14: "Attempting to create a qualified name from a NULL 
pointer broke sss_groupadd" was synchronize

See the full pull-request at https://github.com/SSSD/sssd/pull/14
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/14/head:pr14
git checkout pr14
From c9a8540e4542db1e763bd9c995afae169334cb62 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 6 Sep 2016 12:13:08 +0200
Subject: [PATCH] TOOLS: Fix a typo in groupadd()

Resolves:
https://fedorahosted.org/sssd/ticket/3173
---
 src/tools/sss_sync_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/sss_sync_ops.c b/src/tools/sss_sync_ops.c
index a23a0b8..39ef5be 100644
--- a/src/tools/sss_sync_ops.c
+++ b/src/tools/sss_sync_ops.c
@@ -657,7 +657,7 @@ int groupadd(struct ops_ctx *data)
 int ret;
 
 data->sysdb_fqname = sss_create_internal_fqname(data,
-data->sysdb_fqname,
+data->name,
 data->domain->name);
 if (data->sysdb_fqname == NULL) {
 return ENOMEM;
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org

[SSSD] [sssd PR#14] Attempting to create a qualified name from a NULL pointer broke sss_groupadd (opened)

[jhrozek]

jhrozek's pull request #14: "Attempting to create a qualified name from a NULL 
pointer broke sss_groupadd" was opened

PR body:
"""
The log said:
(Tue Sep  6 15:18:31:935105 2016) [sss_groupadd] 
[sysdb_timestamp_cache_connect] (0x0400): No timestamp cache for LOCAL
(Tue Sep  6 15:18:31:935255 2016) [sss_groupadd] [sss_names_init_from_args] 
(0x0100): Using re [(?P[^@]+)@?(?P[^@]*$)].
(Tue Sep  6 15:18:31:935277 2016) [sss_groupadd] [sss_fqnames_init] (0x0100): 
Using fq format [%1$s@%2$s].
(Tue Sep  6 15:18:31:935392 2016) [sss_groupadd] [parse_name_domain] (0x0200): 
Parsed username: group1
(Tue Sep  6 15:18:31:935418 2016) [sss_groupadd] [ldb] (0x4000): start ldb 
transaction (nesting: 0)
(Tue Sep  6 15:18:31:935504 2016) [sss_groupadd] [ldb] (0x4000): cancel ldb 
transaction (nesting: 0)
(Tue Sep  6 15:18:31:935629 2016) [sss_groupadd] [main] (0x0020): sysdb 
operation failed (12)[Cannot allocate memory]
"""

See the full pull-request at https://github.com/SSSD/sssd/pull/14
... or pull the PR as Git branch:
git remote add ghsssd https://github.com/SSSD/sssd
git fetch ghsssd pull/14/head:pr14
git checkout pr14
From f0af466ae5071552797e95a21a04cfd231b6c609 Mon Sep 17 00:00:00 2001
From: Jakub Hrozek 
Date: Tue, 6 Sep 2016 12:13:08 +0200
Subject: [PATCH] TOOLS: Fix a typo in groupadd()

(cherry picked from commit 6110fe053063863d2309adeef3228d2edcc159b4)
---
 src/tools/sss_sync_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/sss_sync_ops.c b/src/tools/sss_sync_ops.c
index a23a0b8..39ef5be 100644
--- a/src/tools/sss_sync_ops.c
+++ b/src/tools/sss_sync_ops.c
@@ -657,7 +657,7 @@ int groupadd(struct ops_ctx *data)
 int ret;
 
 data->sysdb_fqname = sss_create_internal_fqname(data,
-data->sysdb_fqname,
+data->name,
 data->domain->name);
 if (data->sysdb_fqname == NULL) {
 return ENOMEM;
___
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org