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

[Ondrej Hamada]

On 12/02/2011 04:16 PM, Rob Crittenden wrote:
> Ondrej Hamada wrote:
>> On 11/29/2011 10:33 PM, Rob Crittenden wrote:
>>> 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
>> corrected, corrected, changed, rebased
>>
>>
>>
>> In order to check presence of nss_ldap or nss-pam-ldapd when
>> installing client with '--no-sssd' option there was added
>> code intoipa-client-install. Checking is based on existence
>> of one 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 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.
>>
>> https://fedorahosted.org/freeipa/ticket/2063
>>
>
> Can you add a block header to nssldap_exists()? I think in particular 
> you need explain that it returns 1 and 0 because that value can 
> eventually be the return value of the installer itself (normally an 
> exists would return True/False).
I've changed it to return True/False and added comment
>
> Seeing a traceback:
>
> # ipa-client-install --no-sssd
>
> [ snip ]
>
> Enrolled in IPA realm EXAMPLE.COM
> Created /etc/ipa/default.conf
> Configured /etc/krb5.conf for IPA realm EXAMPLE.COM
> LDAP enabled
> Kerberos 5 enabled
> Traceback (most recent call last):
>   File "/usr/sbin/ipa-client-install", line 1294, in <module>
>     sys.exit(main())
>   File "/usr/sbin/ipa-client-install", line 1281, in main
>     rval = install(options, env, fstore, statestore)
>   File "/usr/sbin/ipa-client-install", line 1211, in install
>     (retcode, conf, filename) = configurer(fstore, cli_basedn, 
> cli_realm, cli_domain, cli_server, dnsok, options)
> TypeError: configure_ldap_conf() takes exactly 8 arguments (7 given)
>
> rob
corrected

-- 
Regards,

Ondrej Hamada
FreeIPA team
jabber: ohama at jabbim.cz
IRC: ohamada

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ohamada-3-4-Client-install-checks-for-nss_ldap.patch
Type: text/x-patch
Size: 7157 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111205/62aa2611/attachment.bin>