URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: opened
PR body: """ This just makes sss_ssh_knownhostsproxy work. There is no support for hostgroups (although hostgroups in `ipa` should continue working).
I've been using this for a few days with the `ldap` and `krb5` providers and I haven't noticed any regressions. I haven't tested `ipa` and `ad` but all tests seem to pass. """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-294803784
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-294803793
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
pbrezina commented: """ ok to test """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-295709734
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
jhrozek commented: """ Hi,
I'm sorry the review takes so long. We're swamped with fixing bugs at the moment and the Easter holidays didn't help either.
I think since the patch moves quite a bit of code to the generic layer, it could be a bit generic as well. The current approach that moves all the IPA code to LDAP provider has a side-effect of exposing pieces of interface that are only (to the best of my knowledge) available only in IPA to the LDAP provider. For example I'm not sure if any other LDAP schema exposes something like UUID or memberof for hosts.
So what do you think about not exposing the part that fetches the host groups outside the IPA provider? The `sdap_host_info_send/recv` request would only return the host. Then, in the IPA provider, there would be a `ipa_host_info_send/recv` request that would first call sdap_host_info and then proceed to fetch the hostgroups as well. This could also mean that the only host-related options that would be publicly exposed in the LDAP documentation would be the host class, name and the public key objectclass. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-295856665
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ UUIDs are exposed in LDAP for users and groups. Not exposing `memberOf` would require having completely different attribute maps in LDAP and IPA. I don't think this can be done easily without duplicating absolutely everything.
Separating `ipa_host_info_send/recv` will either result in more duplication or significantly more complicated code. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-295891519
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ UUIDs are exposed in LDAP for users and groups. Not exposing `memberOf` would require having completely different attribute maps in LDAP and IPA. I don't think this can be done easily without duplicating absolutely everything.
Separating `ipa_host_info_send/recv` will either result in more duplication or significantly more complicated code. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-295891519
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ On (20/04/17 13:20), Hristo Venev wrote:
UUIDs are exposed in LDAP for users and groups.
They are not exposed. They are just available in ldap provider. But do not have default value.
``` sh$ git grep -n ldap_.*_uuid -- src/providers/ src/providers/ad/ad_opts.c:196: { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL }, src/providers/ad/ad_opts.c:233: { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.c:181: { "ldap_user_uuid", "ipaUniqueID", SYSDB_UUID, NULL }, src/providers/ipa/ipa_opts.c:218: { "ldap_group_uuid", "ipaUniqueID", SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.c:157: { "ldap_user_uuid", NULL, SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.c:194: { "ldap_group_uuid", NULL, SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.c:215: { "ldap_user_uuid", NULL, SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.c:252: { "ldap_group_uuid", NULL, SYSDB_UUID, NULL },
src/providers/ldap/ldap_opts.c:273: { "ldap_user_uuid", "objectGUID", SYSDB_UUID, NULL }, src/providers/ldap/ldap_opts.c:310: { "ldap_group_uuid", "objectGUID", SYSDB_UUID, NULL }, ```
"""
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-295903444
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ I'm adding the "Changes requested" label as per @jhrozek's review.
Although it's an ongoing discussion it helps us to have a cleaner idea on what's new on pull-requests land. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-298598458
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ Sorry, I didn't mean to send this. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-299063465
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ This seems to be working for ldap. Can someone test ipa? """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-299480824
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ retest this, please """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318271653
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ Removing the "Changes requested" label as the patch has been updated (v2). """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318329098
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ When this patch is applied to 1.15.3, all tests are passing and things seem to be working fine when using ldap. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318501152
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ @hvenev. sorry, the "retest this, please" is a command that will trigger our CI to retest the patch set. :-) """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318569356
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ @fidencio tests very likely will not pass due to expired certificates in unit test. Which was fixed in 2ccfa9502abf52941d8b6e44b5f7cfdd13311a2d. Patches would need to rebased on current master (or at leas 1.15.3) otherwise it will fail in jenkins. And that would be the same for any older pull request. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318596855
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ @hvenev, please, why this PR got closed?
Please, I'll re-open it and rebase your patches on top of current git master. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318814583
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: reopened
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318815513
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318815516
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ Sorry, I must have pushed the wrong thing. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318815530
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ ok to test """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318815681
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
fidencio commented: """ @hvenev, thanks a lot for re-opening this PR. :-) """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-318815701
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ @hvenev, Could you rebase patch + also fix some issues. You can see POC https://github.com/lslebodn/sssd/tree/sdap-hostid_rebased
@jhrozek were your comments addressed in latesd version? https://github.com/SSSD/sssd/pull/237#issuecomment-295856665 """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-347607817
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
jhrozek commented: """ Yes, but I think we should also work on tests. I also haven't ran any automated tests at all. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-347648386
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ @hvenev do you plan to work on this PR. I know you were waiting for a long time for review and I am sorry for that. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-350078851
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
hvenev commented: """ I no longer have a real environment where I'm using sssd so I don't think I'll be working on this. """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-350090567
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """
Yes, but I think we should also work on tests. I also haven't ran any automated tests at all.
I rebased PR + ran automated test and there are not any regressions.
I'll push PR with your RB.
"""
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-364422213
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ master: * 60a715a0dd79873d2d2607eab8fdfaf0ffd2e7d3 """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-364422952
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
lslebodn commented: """ master: * 60a715a0dd79873d2d2607eab8fdfaf0ffd2e7d3 """
See the full comment at https://github.com/SSSD/sssd/pull/237#issuecomment-364423036
URL: https://github.com/SSSD/sssd/pull/237 Author: hvenev Title: #237: providers: Move hostid from ipa to sdap Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/237/head:pr237 git checkout pr237
URL: https://github.com/SSSD/sssd/pull/237 Title: #237: providers: Move hostid from ipa to sdap
Label: +Pushed
sssd-devel@lists.fedorahosted.org
-
centos-ci -
fidencio
-
hvenev
-
jhrozek
-
lslebodn
-
pbrezina