[Freeipa-devel] [PATCH 0051] Preserve already configured options in openldap conf

[Martin Kosek]

On 04/25/2013 12:29 PM, Jan Cholasta wrote:
> On 25.4.2013 08:51, Martin Kosek wrote:
>> On 04/24/2013 08:02 PM, Rob Crittenden wrote:
>>> Jan Cholasta wrote:
>>>> On 24.4.2013 14:54, Martin Kosek wrote:
>>>>> On 04/24/2013 02:51 PM, Rob Crittenden wrote:
>>>>>> Jan Cholasta wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> On 23.4.2013 12:28, Tomas Babej wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> We should respect already configured options present in
>>>>>>>> /etc/openldap/ldap.conf when generating our own configuration.
>>>>>>>> With this patch, we only rewrite URI, BASE and TLS_CACERT options.
>>>>>>>>
>>>>>>>> https://fedorahosted.org/freeipa/ticket/3582
>>>>>>>>
>>>>>>>
>>>>>>> the changeConf call will fail when the file does not exist, we might
>>>>>>> want to handle that gracefully.
>>>>>>>
>>>>>>> Honza
>>>>>>>
>>>>>>
>>>>>> We also need to handle the case where these items are already
>>>>>> defined. I'm
>>>>>> honestly not sure what the behavior should be: overwrite, warn and
>>>>>> overwrite,
>>>>>> fail.
>>>>>>
>>>>>> rob
>>>>>>
>>>>>
>>>>> I am also thinking that we may want to be more cautious before
>>>>> updating this
>>>>> file. AFAIK, we do not need the updated file for our function, its
>>>>> only updated
>>>>> for user convenience so that he can run ldapsearches more easily.
>>>>>
>>>>> I see several options here that could help this goal:
>>>>> 1) Update ldap.conf if BASE and URI and TLS_CACERT only if these
>>>>> options are
>>>>> not set. If the options are already set, we could just print a note
>>>>> that we
>>>>> skipped it. When I see my vanilla /etc/openldap/ldap.conf, it has
>>>>> these options
>>>>> commented out, so it should be possible to implement this check.
>>>>>
>>>>> 2) Do ldap.conf changes only if a new special option is passe (e.g.
>>>>> --configure-ldap-cong)
>>>>>
>>>>> 3) Do not update ldap.conf when a new special option is not passed (e.g.
>>>>> --no-ldap-conf
>>>>>
>>>>> Martin
>>>>>
>>>>
>>>> If we don't need the file for our function, we can just not configure it
>>>> at all IMO. We can document how to configure it for users who want it.
>>>
>>> It was an RFE that we create this file. It is handy to have pre-configured, I
>>> like having it actually.
>>>
>>> We just need to try to have a gentler touch than my first crack at it, which
>>> overwrote it completely. I think #1 is probably enough for now. I'm not sure I
>>> want to add two new options this late in the game, and the client already has a
>>> lot of knobs.
>>>
>>> rob
>>>
>>
>> Yeah, I also agree that 1) is enough. It will not add any more options and will
>> let us be more gentle and respectful to already existent custom user settings
>> in ldap.conf. So Tomas, this seems like the way to go :-)
>>
>> Martin
>>
>
> I don't see the point of updating only some of these values. What about

Not some of them - either all of them (BASE, URI, TLS_CACERT) when none of them 
is already configured or none at all.

>
> 4) Update BASE and URI and TLS_CACERT, comment any old settings out.
>
> ?

This would still break an existing user configuration, we would just tell user 
what we broke :-)

Martin

>
> Honza
>