[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-08-01 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
ACK. I was able to reproduce the bug following the instructions provided above 
and the patch does fix it.

CI passed.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319381495
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-08-01 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
ACK. I was able to reproduce the bug following the instructions provided above 
and the patch does fix it.

CI:
http://vm-058-233.abc.idm.lab.eng.brq.redhat.com/logs/job/72/58/summary.html
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319381495
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-08-01 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
Thank you for the description. I see the bug now and the patch makes sense.
I sent the patch to CI.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319337655
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-31 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
And I've removed the "Changes requested" label as this PR is waiting to be 
reviewed.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319053597
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-31 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
**Issue**: UpdateMemberList fails in case there's no ghost attribute in the 
group

**Steps to reproduce**:
- In an IPA server, create a user:
```
[root@master x86_64]# ipa user-add tuser99 --first="Test" --last="User"

Added user "tuser99"

  User login: tuser99
  First name: Test
  Last name: User
  Full name: Test User
  Display name: Test User
  Initials: TU
  Home directory: /home/tuser99
  GECOS: Test User
  Login shell: /bin/bash
  Principal name: tuser99@IPA.EXAMPLE
  Principal alias: tuser99@IPA.EXAMPLE
  Email address: tuser99@ipa.example
  UID: 192788
  GID: 192788
  Password: False
  Member of groups: ipausers
  Kerberos keys available: False
```
- Create a new group:
```
[root@master x86_64]# ipa group-add tgroup99
--
Added group "tgroup99"
--
  Group name: tgroup99
  GID: 192789
```
- Add the newly created user to the newly created group: 
```
[root@master x86_64]# ipa group-add-member --users=tuser99 tgroup99
  Group name: tgroup99
  GID: 192789
  Member users: tuser99
-
Number of members added 1
```
- Checking the cache you'll notice that the newly created user and group are 
not there:
```
[root@master x86_64]# ldbsearch -H /var/lib/sss/db/cache_ipa.example.ldb
asq: Unable to register control with rootdse!
# record 1
dn: cn=views,cn=sysdb
viewName: default
distinguishedName: cn=views,cn=sysdb

# record 2
dn: cn=ipa.example,cn=sysdb
cn: ipa.example
realmName: IPA.EXAMPLE
distinguishedName: cn=ipa.example,cn=sysdb

# record 3
dn: cn=ranges,cn=sysdb
cn: ranges
distinguishedName: cn=ranges,cn=sysdb

# record 4
dn: cn=sudorules,cn=custom,cn=ipa.example,cn=sysdb
cn: sudorules
sudoLastFullRefreshTime: 1501501934
distinguishedName: cn=sudorules,cn=custom,cn=ipa.example,cn=sysdb

# record 5
dn: cn=users,cn=ipa.example,cn=sysdb
cn: Users
distinguishedName: cn=users,cn=ipa.example,cn=sysdb

# record 6
dn: cn=sysdb
cn: sysdb
description: base object
version: 0.18
distinguishedName: cn=sysdb

# record 7
dn: cn=groups,cn=ipa.example,cn=sysdb
cn: Groups
distinguishedName: cn=groups,cn=ipa.example,cn=sysdb

# record 8
dn: cn=certmap,cn=sysdb
cn: certmap
userNameHint: FALSE
distinguishedName: cn=certmap,cn=sysdb

# returned 8 records
# 8 entries
# 0 referrals
```
- Call the FindByName method for the group:
```
[root@master x86_64]# dbus-send --print-reply --system 
--dest=org.freedesktop.sssd.infopipe /org/freedesktop/sssd/infopipe/Groups 
org.freedesktop.sssd.infopipe.Groups.FindByName string:tgroup99
method return time=1501502395.330843 sender=:1.544 -> destination=:1.546 
serial=5 reply_serial=2
   object path "/org/freedesktop/sssd/infopipe/Groups/ipa_2eexample/192789"
```
- Checking the cache you'll notice that the group is there and the user is 
shown there in the ghost attribute:
```
[root@master x86_64]# ldbsearch -H /var/lib/sss/db/cache_ipa.example.ldb
asq: Unable to register control with rootdse!
# record 1
dn: name=tgroup99@ipa.example,cn=groups,cn=ipa.example,cn=sysdb
createTimestamp: 1501502395
gidNumber: 192789
name: tgroup99@ipa.example
objectClass: group
uniqueID: d51d90a0-75e6-11e7-b172-52540072ab99
isPosix: TRUE
originalDN: cn=tgroup99,cn=groups,cn=accounts,dc=ipa,dc=example
originalModifyTimestamp: 20170731115328Z
entryUSN: 5288
orig_member: uid=tuser99,cn=users,cn=accounts,dc=ipa,dc=example
ghost: tuser99@ipa.example
nameAlias: tgroup99@ipa.example
lastUpdate: 1501502395
dataExpireTimestamp: 1501507795
overrideDN: name=tgroup99@ipa.example,cn=groups,cn=ipa.example,cn=sysdb
distinguishedName: name=tgroup99@ipa.example,cn=groups,cn=ipa.example,cn=sysdb

# record 2
dn: cn=views,cn=sysdb
viewName: default
distinguishedName: cn=views,cn=sysdb

# record 3
dn: cn=ipa.example,cn=sysdb
cn: ipa.example
realmName: IPA.EXAMPLE
distinguishedName: cn=ipa.example,cn=sysdb

# record 4
dn: cn=ranges,cn=sysdb
cn: ranges
distinguishedName: cn=ranges,cn=sysdb

# record 5
dn: cn=sudorules,cn=custom,cn=ipa.example,cn=sysdb
cn: sudorules
sudoLastFullRefreshTime: 1501501934
distinguishedName: cn=sudorules,cn=custom,cn=ipa.example,cn=sysdb

# record 6
dn: cn=users,cn=ipa.example,cn=sysdb
cn: Users
distinguishedName: cn=users,cn=ipa.example,cn=sysdb

# record 7
dn: cn=sysdb
cn: sysdb
description: base object
version: 0.18
distinguishedName: cn=sysdb

# record 8
dn: cn=groups,cn=ipa.example,cn=sysdb
cn: Groups
distinguishedName: cn=groups,cn=ipa.example,cn=sysdb

# record 9
dn: cn=certmap,cn=sysdb
cn: certmap
userNameHint: FALSE
distinguishedName: cn=certmap,cn=sysdb

# returned 9 records
# 9 entries
# 0 referrals
```
- Call the UpdateMemberList method:
```
[root@master x86_64]# dbus-send --print-reply --system 
--dest=org.freedesktop.sssd.infopipe 
/org/freedesktop/sssd/infopipe/Groups/ipa_2eexample/192789 
org.freedesktop.sssd.infopipe.Groups.Group.UpdateMemberList

[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-31 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
So, after playing with this PR for some time I do believe that:
- The reproducer correct (although some info described may not be valid 
anymore), thus I'll provide a new reproducer;
- **IFP: Parse ghost name in Group.UpdateMemberList** is not needed;
- **IFP: ldb_msg_find_element empty result fix** is needed, but the commit 
message will have to be improved.

I'll update this PR with the new reproducer and the patch with the description. 
I'm giving the patch an ACK but I really would like that @mzidek-rh could check 
it, as he wasn't able to reproduce the issue at the first time and also provide 
his ACK/NACK no the patch.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319039688
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-31 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
I am Ok with closing this PR. Btw. I still do not think that what is described 
in the reproducer as buggy behavior is actually wrong. I think the requests to 
the IFP do not update/fill the cache by design, but I may be mistaken.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-319034733
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-30 Thread jhrozek
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

jhrozek commented:
"""
The ghost attribute is an optimization. Normally, when you are asked for a 
group (which, if you follow the getgr* interface) also includes a list of group 
members, you would create a user object for every member of the group and link 
the user object with member/memberof links in the cache.

But that's very slow, so instead, if we are only interested in the member name, 
we store the member name in the special 'ghost' attribute. When the user info 
is later requested (e.g. getent group developers returns fabiano, later someone 
asks for getent passwd fabiano), we remove the ghost attribute and instead 
store the full user entry and the proper links.

(I haven't checked the patch, just saying what the ghost is about).

And in general it's OK if there are no ghost attributes for a group. it just 
means that the group either has no members or all members have been previously 
fully resolved.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318923831
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-28 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
Actually, I'd like to hear from a more experienced developer the concept of a 
`ghost` object.

The error reported by @celestian happens because ldb_msg_find_element(group, 
SYSDB_GHOST) cannot find anything. What does it actually mean? Is it something 
expected?

In case it's something expected to happen, his patch is okay as this case won't 
be treated as an error. However, in case it's not expected (and that's my 
thought), his patch is just covering the root cause of the issue and it has to 
be properly investigated.

So, please, if someone with more experience in this area can give me some help 
here to understand this would be really nice.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318750940
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-28 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

lslebodn commented:
"""
On (28/07/17 16:45), fidencio wrote:
>Okay. I was able to reproduce the issue described in the bug report and the 
>patch does fix the issue.
>However, I do not think the fix is correct as it's not done in the right part 
>of the code.
>
>I'll dig a little bit more into the issue but, for now, I'd suggest to just 
>close this PR and a new one can be opened whenever someone actually has some 
>time allocated to work on the issue.
>
>@lslebodn, @mzidek-rh , do you guys (who were involved in this PR) agree with 
>closing this PR?
>

If you are sure that fix is not in correct part of code then +1 for closing.
Because celestian will not have a time to working on this due to focusing on
other cool project :-) :-) :-)

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318705110
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-28 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
Okay. I was able to reproduce the issue described in the bug report and the 
patch does fix the issue.
However, I do not think the fix is correct as it's not done in the right part 
of the code.

I'll dig a little bit more into the issue but, for now, I'd suggest to just 
close this PR and a new one can be opened whenever someone actually has some 
time allocated to work on the issue.

@lslebodn, @mzidek-rh , do you guys (who were involved in this PR) agree with 
closing this PR?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318704033
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-27 Thread celestian
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

celestian commented:
"""
The issue was that getent shows user test_user in test_group, but dbus call 
doesn't.

How I did it is described in my description. But I don't know if it is still 
valid. It was some time ago.

If I understand others comments right, it was try to fix method
`org.freedesktop.sssd.infopipe.Groups.Group.UpdateMemberList()`
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318345349
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-07-27 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
@fidencio , sorry but I do not know what is the issue. I was probably not clear 
in my previous comments, but IMO the reproducer is wrong because it does not 
describe any issue. I do not know what the issue was, so I do not know what the 
reproducer should be. And I did not see any difference in behavior before and 
after applying the patches (but it is a while since I tried them so I do not 
remember every detail).
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-318341344
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-05-05 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
@mzidek-rh, you pointed out the reproducer is not correct. Would you mind 
providing a correct reproducer?
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-299414693
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-05-02 Thread fidencio
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

fidencio commented:
"""
I'm adding "Changes requested" label by @mzidek-rh's review.

As @celestian is not working on the project anymore someone else will take this 
over at some point in the future.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-298600043
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-04-03 Thread celestian
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

celestian commented:
"""
IMO, this patch set fix method 
org.freedesktop.sssd.infopipe.Groups.Group.UpdateMemberList() which you need 
call if you would like to see members of group.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-291151435
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-03-31 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
On 03/31/2017 03:44 PM, lslebodn wrote:
> On (31/03/17 06:34), mzidek-rh wrote:
>>Actually, I looking at it again I do not think these patches solve the
> issue. The reproducer is not correct. After you update all the members
> in the group with dbus call in your reproducer, the user is added to the
> cache (as a normal user, not as a ghost user)... and this is what worked
> even before your paches... so it does not seem to be fixed when only
> ghost users are stored (and that is what the bug was about).
>>
>
> Does it solve another bug?
>
> LS
>

If yes, then I do not know which one. From the description, I though
the problem was with results from dbus when there were only ghost users
in the group. This is not fixed. The test scenario provided as a
reproducer does not seem to be related to the issue at all (it
does not test dbus call for properties in group that has ghost
users) so it works, but even without the patches.

Michal

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-290719881
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-03-31 Thread lslebodn
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

lslebodn commented:
"""
On (31/03/17 06:34), mzidek-rh wrote:
>Actually, I looking at it again I do not think these patches solve the issue. 
>The reproducer is not correct. After you update all the members in the group 
>with dbus call in your reproducer, the user is added to the cache (as a normal 
>user, not as a ghost user)... and this is what worked even before your 
>paches... so it does not seem to be fixed when only ghost users are stored 
>(and that is what the bug was about).
>

Does it solve another bug?

LS

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-290715921
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-03-31 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
Actually, I looking at it again I do not think these patches solve the issue. 
The reproducer is not correct. After you update all the members in the group 
with dbus call in your reproducer, the user is added to the cache (as a normal 
user, not as a ghost user)... and this is what worked even before your 
paches... so it does not seem to be fixed when only ghost users are stored (and 
that is what the bug was about).

Also there is a warning:

/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c: In function 
‘resolv_ghosts_step’:
/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c:600:37: warning: passing 
argument 3 of ‘sss_parse_internal_fqname’ from incompatible pointer type 
[-Wincompatible-pointer-types]
 _name, NULL);
 ^
In file included from 
/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c:24:0:
/home/user/gitrepo/sssd/src/util/util.h:284:9: note: expected ‘char **’ but 
argument is of type ‘const char **’
 errno_t sss_parse_internal_fqname(TALLOC_CTX *mem_ctx,
 ^

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-290713169
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-03-31 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
Actually, I looking at it again I do not think these patches solve the issue. 
The reproducer is not correct. After you update all the members in the group 
with dbus call in your reproducer, the user is added to the cache (as a normal 
user, not as a ghost user)... and this is what worked even before your 
paches... so it does not seem to be fixed when only ghost users are stored (and 
that is what the bug was about).

Also there is a warning:

/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c: In function 
‘resolv_ghosts_step’:
/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c:600:37: warning: passing 
argument 3 of ‘sss_parse_internal_fqname’ from incompatible pointer type 
[-Wincompatible-pointer-types]
 _name, NULL);
 ^
In file included from 
/home/user/gitrepo/sssd/src/responder/ifp/ifp_groups.c:24:0:
/home/user/gitrepo/sssd/src/util/util.h:284:9: note: expected ‘char **’ but 
argument is of type ‘const char **’
 errno_t sss_parse_internal_fqname(TALLOC_CTX *mem_ctx,
 ^

"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-290713169
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org


[SSSD] [sssd PR#202][comment] T3315 infopipe group users master

2017-03-31 Thread mzidek-rh
  URL: https://github.com/SSSD/sssd/pull/202
Title: #202: T3315 infopipe group users master

mzidek-rh commented:
"""
These patches fixed the issue for me. LGTM, if CI passes I will give you an ACK.
"""

See the full comment at 
https://github.com/SSSD/sssd/pull/202#issuecomment-290704624
___
sssd-devel mailing list -- sssd-devel@lists.fedorahosted.org
To unsubscribe send an email to sssd-devel-le...@lists.fedorahosted.org