[SSSD] [sssd PR#202][comment] T3315 infopipe group users master
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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