[Freeipa-devel] [PATCH] 0004 (2) Make proper LDAP configuration reporting for ipa-cli-install

[Rob Crittenden]

Alexander Bokovoy wrote:
> On 29.07.2011 14:13, Martin Kosek wrote:
>> On Fri, 2011-07-29 at 13:09 +0300, Alexander Bokovoy wrote:
>>> Hi,
>>>
>>> another attempt to refine error/configuration reporting when configuring
>>> means to access LDAP on a client. Previous one tried to use rpm to find
>>> out package name but this approach is avoiding package names. Instead,
>>> it tries to tell configuration file.
>>>
>>> Ticker https://fedorahosted.org/freeipa/ticket/1369
>>
>> NACK.
>>
>> 1) Return info from LDAP config functions gets overwritten:
>>
>> if not options.sssd:
>>          (retcode, conf, filename) = configure_ldap_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options)
>>          if retcode:
>>              return 1
>>          (retcode, conf, filename) = configure_nslcd_conf(fstore, cli_basedn, cli_realm, cli_domain, cli_server, dnsok, options)
>>          if retcode:
>>              return 1
>>
>> Only one function will do the real configuration, in my case it was the
>> configure_ldap_conf (nslcd was not installed). Due to the overwrite, my
>> ipa-client-install reported invalid information:
> Yes, fixed.
>
>> # ipa-client-install --server=vm-059.idm.lab.bos.redhat.com --domain=idm.lab.bos.redhat.com --no-sssd
>> ...
>> LDAP enabled
>> Kerberos 5 enabled
>> NSLCD configured using configuration file /etc/nslcd.conf<<<<
>> Unable to use DNS discovery! Recognized configuration: NSLCD
>> Changing configuration of /etc/ldap.conf to use hardcoded server name: vm-059.idm.lab.bos.redhat.com
>> NTP enabled
>> Client configuration complete.
>>
>> We need to indicate in the return triple that the service was not
>> configured so that we output correct information.
> I did this now by returning None: return (0, None, None).
>
>> 2) Returning tuple instead of triple (will raise exception when used):
>>
>> -            return 1
>> +            return (1, 'nslcd')
>>
>> Plus, NSLCD is referred in upper case in other return statements.
> Fixed.
>
> Version 3 attached.

nack, we shouldn't reference /etc/ldap.conf directly because this file 
may not actually exist or be used. nslcd uses /etc/nslcd.conf, for 
example. We would need to collect the files that get updated display 
them all, I guess. Or stick with the generic message.

rob