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

[Simo Sorce]

On Mon, 2012-10-22 at 17:15 +0200, Martin Kosek wrote:
> 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.

Yeah that's right, we should have all sssd related changes under a
conditional that is true only when sssd is enabled.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York