[Freeipa-devel] [PATCH 0044] Periodically refresh global ipa-kdb configuration

[Simo Sorce]

On Mon, 2014-05-05 at 20:08 -0400, Dmitri Pal wrote:
> On 05/02/2014 02:52 PM, Simo Sorce wrote:
> > On Thu, 2014-05-01 at 16:22 -0400, Nathaniel McCallum wrote:
> >> On Tue, 2014-03-11 at 11:09 -0400, Simo Sorce wrote:
> >>> On Tue, 2014-03-11 at 16:05 +0200, Alexander Bokovoy wrote:
> >>>> On Tue, 11 Mar 2014, Jan Pazdziora wrote:
> >>>>> On Mon, Feb 24, 2014 at 02:26:27PM -0500, Nathaniel McCallum wrote:
> >>>>>> Before this patch, ipa-kdb would load global configuration on startup
> >>>>>> and never update it. This means that if global configuration is changed,
> >>>>>> the KDC never receives the new configuration until it is restarted.
> >>>>>>
> >>>>>> This patch enables caching of the global configuration with a timeout of
> >>>>>> 60 seconds.
> >>>>>>
> >>>>>> https://fedorahosted.org/freeipa/ticket/4153
> >>>>>> >From 7daeae56671d7b3049b0341aad66c96877431bbe Mon Sep 17 00:00:00 2001
> >>>>>> From: Nathaniel McCallum <npmccallum at redhat.com>
> >>>>>> Date: Mon, 24 Feb 2014 14:19:13 -0500
> >>>>>> Subject: [PATCH] Periodically refresh global ipa-kdb configuration
> >>>>>>
> >>>>>> Before this patch, ipa-kdb would load global configuration on startup and
> >>>>>> never update it. This means that if global configuration is changed, the
> >>>>>> KDC never receives the new configuration until it is restarted.
> >>>>>>
> >>>>>> This patch enables caching of the global configuration with a timeout of
> >>>>>> 60 seconds.
> >>>>>>
> >>>>>> https://fedorahosted.org/freeipa/ticket/4153
> >>>>> I have only read the code and it looks sane, so depending on how much
> >>>>> you consider my word about code-reading worth, ack.
> >>>>>
> >>>>> However, my gut feeling is that my preferred way of handling the issue
> >>>>> (without knowing much about the background of the ticket) would
> >>>>> probably be a HUP signal handler or something similar, rather than
> >>>>> polling for current values with the value timeout. This patch
> >>>>> introduces small nondeterminism to the behaviour when the usage of the
> >>>>> new values cannot really be enfoced by the admin (without the daemon
> >>>>> restart).
> >>>> Thing is, we need the update to happen when other, non-root process
> >>>> makes the changes -- in our case when IPA server running under httpd
> >>>> privileges performs series of MS-RPC calls towards smbd which lead to
> >>>> creating certain LDAP objects. httpd couldn't send SIGHUP to a process
> >>>> not owned by httpd's effective user (non-root).
> >>> Yes but this is not really the way to go.
> >>>
> >>> The proper fix is to use syncrepl/persistent search to get a
> >>> notification of when we need to reload the configuration.
> >>>
> >>> On the patch itself I have a NACK due to this syntax used in various
> >>> places: function()->attribute
> >>>
> >>> don't. do. that. ever.
> >>>
> >>> assign whatever come from the function to a local variable and *check*
> >>> it is not null, *then* use it.
> >> Attached patch fixes the NACK issue, but does not address the question
> >> of the larger approach. Do we need to take a different approach? If so,
> >> what is it?
> > LGTM
> > I might prefer slightly more explicit names for those temp vars, but
> > that's not a big deal.
> >
> > As for the approach, moving to something like syncrepl would be a good
> > idea. As it would allow us to avoid useless polling and changes would be
> > applaied as soon as they become available as the syncrepl agreement
> > would push them to our client. It does mean we need a way to check that
> > there aren't pending changes coming down the syncrepl pipe, but we can
> > do that synchronously at every entry point in the KDB. After all we do
> > not need to care about a change until the KDC needs something from the
> > KDB.
> >
> > Simo.
> >
> Do we need a ticket for that then?

Turns out we already have one:
https://fedorahosted.org/freeipa/ticket/1302

Simo.

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