[Freeipa-devel] [PATCH] client: include the directory with domain-realm mappings in krb5.conf

[Rob Crittenden]

Jakub Hrozek wrote:
> On Fri, Aug 17, 2012 at 12:20:27PM -0400, Simo Sorce wrote:
>>
>>
>> ----- Original Message -----
>>> Hi,
>>>
>>> the attached patches add the directory the SSSD writes domain-realm
>>> mappings as includedir to krb5.conf when installing the client.
>>>
>>> [PATCH 1/3] ipachangeconf: allow specifying non-default delimeter for
>>> options
>>> ipachangeconf only allows one delimeter between keys and values. This
>>> patch adds the possibility of also specifying "delim" in the option
>>> dictionary to override the default delimeter.
>>>
>>> On a slightly-unrelated note, we really should think about adopting
>>> Augeas. Changing configuration with home-grown scripts is getting
>>> tricky.
>>>
>>> [PATCH 2/3] Specify includedir in krb5.conf on new installs
>>> This patch utilizes the new functionality from the previous patch to
>>> add
>>> the includedir on top of the krb5.conf file
>>>
>>> [PATCH 3/3] Add the includedir to krb5.conf on upgrades
>>> This patch is completely untested and I'm only posting it to get
>>> opinions. At first I was going to use an upgrade script in %post but
>>> then I thought it would be overengineering when all we want to do is
>>> prepend one line.. Would a simple munging like this be acceptable or
>>> shall I write a full script?
>>
>> NACK, using a scriptlet is fine, but not the way you did, as it has a huge race condition where krb5.conf exists and has only one line in it (the include line).
>>
>> You should first create the new file: echo "include ..." > /etc/krb.conf.ipanew
>> Then cat the contents of the existing file in i:t cat /etc/krb.conf >> /etc/krb.conf.ipanew
>> And finally atomically rename it: mv /etc/krb.conf.ipanew /etc/krb.conf
>>
>> This method is also safe wrt something killing the yum process ...
>>
>> Simo.
>
> I'm attaching a new revision of the patches not even two months after
> the original nack.
>
> I also think it might be nice to have a more general way of upgrading
> the client config so I filed
> https://fedorahosted.org/freeipa/ticket/3149

I don't think grepping for a string is an effective way to determine if 
the client has been configured. Someone could have removed that line.

I'd prefer using /var/lib/ipa-client/sysrestore/sysrestore.index. If it 
exists and has more than 2 lines in it ([files] + one other file) then 
it is safe to say the client is configured, or at least partially 
configured.

rob