[Freeipa-devel] [PATCH] ipa-client-install with --no-sssd option should check for nss_ldap

[Rob Crittenden]

Ondrej Hamada wrote:
> On 11/11/2011 02:55 PM, Ondrej Hamada wrote:
>> https://fedorahosted.org/freeipa/ticket/2063
>>
>> In order to check presence of nss_ldap when installing client with
>> '--no-sssd' option there was added code into ipa-client-install. Check
>> is base on existence of nss_ldap configuration files. This
>> configuration could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
>> '/etc/libnss_ldap.conf'. Presence of any of these files is considered
>> as success otherwise failure.
>>
>>
>>
>> _______________________________________________
>> Freeipa-devel mailing list
>> Freeipa-devel at redhat.com
>> https://www.redhat.com/mailman/listinfo/freeipa-devel
> I've rewritten it. Additionally it checks for existence of nss-pam-ldapd
> and makes the results reusable by configure_{ldap|nslcd}_conf() functions.
>
> https://fedorahosted.org/freeipa/ticket/2063
>
> In order to check presence of nss_ldap or nss-pam-ldapd when installing
> client
> with '--no-sssd' option there was added code into ipa-client-install.
> Checking is based on existence of nss_ldap configuration files. This
> configuration could be in 'etc/ldap.conf', '/etc/nss_ldap.conf' or
> '/etc/libnss_ldap.conf'. Optionaly the nss_ldap could cooperate with
> pam_ldap
> module and hence the presence of it is checked by looking for
> 'pam_ldap.conf' file.
> Existence of nss-pam-ldapd is checked against existence of 'nslcd.conf'
> file.
> All this checking is done by function nssldap_exists().
> Because both main modules are maintained by two different functions, the
> function
> returns tuple containing return code and dictionary structure - its key
> is name
> of target function and value is list of existing configuration files.
> Files to check are specified inside the nssldap_exists() function.
>
> In order to fit the returned values, the functions
> configure_{ldap|nslcd}_conf()
> were slightly modified. They accept one more parameter which is list of
> existing files.
> They are not checking existence of above mentioned files anymore.

The patch looks good, just a couple of issues.

1. In the nslcd configurator you add ''.join(files). Did you mean 
','.join(files)?

2. The commit message lines wrap making it difficult to read. Can you 
limit the lines to ~70 chars per line?

3. I think the message printed when neither package is available can be 
simplified to:

One of these packages must be installed: nss_ldap or nss-pam-ldapd

It needs a rebase too.

rob