URL: https://github.com/SSSD/sssd/pull/372 Author: amitkumar50 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary Action: opened
PR body: """ IPA and AD providers default to userCertificate;binary for the ldap_user_certificate option. It will be good to default that value also for the generic LDAP provider.
Resolves: https://pagure.io/SSSD/sssd/issue/3499 """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/372/head:pr372 git checkout pr372
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327468371
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
centos-ci commented: """ Can one of the admins verify this patch? """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327468376
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
sumit-bose commented: """ ok to test """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327470431
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
jhrozek commented: """ The code part of this patch looks good to me, thank you. But please also change the sssd-ldap man page, currently it still says that the default value is unset for providers other than IPA. """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327709142
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
amitkumar50 commented: """ @jhrozek `man sssd-ldap` ldap_group_external_member (string) The LDAP attribute that references group members that are defined in an external domain. At the moment, only IPA's external members are supported. Default: ipaExternalMember in the IPA provider, otherwise unset.
Do you want to change this entry, as per you what suits best? """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327782819
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
jhrozek commented: """ No, I want to change the description of the option you are changing :-)
On my machine it reads like this: ``` ldap_user_certificate (string) Name of the LDAP attribute containing the X509 certificate of the user.
Default: no set in the general case, userCertificate;binary for IPA ```
The default should just say userCertificate;binary """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-327807629
URL: https://github.com/SSSD/sssd/pull/372 Author: amitkumar50 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/372/head:pr372 git checkout pr372
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
amitkumar50 commented: """ @jhrozek Done Changes. Thanks """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-328067357
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
jhrozek commented: """ Actually, I'm sorry, I noticed one more nitpick. We need to remove the mention of `ldap_user_certificate` from `src/man/include/ipa_modified_defaults.xml` as well. """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-329179123
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: +Changes requested
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
amitkumar50 commented: """ @jhrozek Thanks for review. On latest master Its: ` <para> ldap_user_certificate = userCertificate;binary </para> ` I believe this is what we want? """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-330186693
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
jhrozek commented: """ Yes, binary is what we want, but since it's the default now everywhere, we do not want to document that as a deviation from default in `src/man/include/ipa_modified_defaults.xml`. What needs to be done in this PR is to just drop the line that documents `dap_user_certificate` in modified_defaults. It is enough to document the default in the regular man pages. """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-330495528
URL: https://github.com/SSSD/sssd/pull/372 Author: amitkumar50 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/372/head:pr372 git checkout pr372
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
amitkumar50 commented: """ @jhrozek dropped the line that documents ldap_user_certificate in modified_defaults. Please have a look. Thanks """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-330823052
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
jhrozek commented: """ Thank you, this is what I wanted, but please squash the two patches together so we can push them as one (I think all the changes constitute one logical change, so it should be OK to push them both as one patch) """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-331258415
URL: https://github.com/SSSD/sssd/pull/372 Author: amitkumar50 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/372/head:pr372 git checkout pr372
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
amitkumar50 commented: """ @jhrozek Done Thanks.. """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-331838532
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: -Changes requested
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
lslebodn commented: """ master: * d1d6f3a7f08cd1dc5128105eb6ad7ec311f281b8 """
See the full comment at https://github.com/SSSD/sssd/pull/372#issuecomment-334065997
URL: https://github.com/SSSD/sssd/pull/372 Author: amitkumar50 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/372/head:pr372 git checkout pr372
URL: https://github.com/SSSD/sssd/pull/372 Title: #372: ldap: Change ldap_user_certificate to userCertificate;binary
Label: +Pushed
sssd-devel@lists.fedorahosted.org
-
amitkumar50 -
centos-ci
-
fidencio
-
jhrozek
-
lslebodn
-
sumit-bose