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

[Alexander Bokovoy]

On Wed, 18 Jun 2014, Nathaniel McCallum wrote:
>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?
Yes, that seems a likely cause. At least, I cannot reproduce it again.

>
>The new attached version fixes this bug. Everything appears to work...
Thanks, ACK.

-- 
/ Alexander Bokovoy