On 10/08/2012 08:27 PM, Rob Crittenden wrote:
> 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
> 

I just found one more issue. What if ipa-client-install is run with --no-sssd
option? In that case I assume we should not include the SSSD's krb5.include.d,
right? The same would also appy for upgrades, we would need to check if client
was actually configured with SSSD before mangling their krb5.conf.

Martin

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to