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

[Nathaniel McCallum]

On Wed, 2014-06-04 at 18:47 +0300, Alexander Bokovoy wrote:
> On Thu, 01 May 2014, 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?
> 
> I've tested the patch but was unable to spot any activity to refresh the
> configuration after I've updated it with 
>     ipa config-mod --ipaconfigstring=KDC:Disable\ Lockout
> 
> At least, dirsrv logs didn't show me any attempt to re-read IPA config
> past the change.

I spent extensive time testing this today. I am unable to reproduce a
failure to reload data from LDAP. If I make the change using "ipa
config-mod", wait a minute and run a kinit the KDC always gets the new
values. So it works for me.

However, I did uncover a subtle bug with regard to ipaconfigstring which
would cause it to never to be unset once set. Perhaps you were running
into this?

The new attached version fixes this bug. Everything appears to work...

Nathaniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0044.2-Periodically-refresh-global-ipa-kdb-configuration.patch
Type: text/x-patch
Size: 11637 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20140618/f7e42897/attachment.bin>