[Freeipa-devel] [PATCH 0064] Create ipa-otp-decrement 389DS plugin

[Nathaniel McCallum]

On Thu, 2014-10-09 at 18:48 +0200, thierry bordaz wrote:
> On 10/09/2014 05:51 PM, Nathaniel McCallum wrote:
> 
> > On Thu, 2014-10-09 at 11:44 +0200, thierry bordaz wrote:
> > > On 10/09/2014 12:15 AM, Nathaniel McCallum wrote:
> > > 
> > > > On Wed, 2014-10-08 at 17:19 -0400, Simo Sorce wrote:
> > > > > On Wed, 08 Oct 2014 15:53:39 -0400
> > > > > Nathaniel McCallum <npmccallum at redhat.com> wrote:
> > > > > 
> > > > > > As I understand my code, all servers will have csnD. Some servers will
> > > > > > have valueB and others will have valueD, but valueB == valueD.
> > > > > > 
> > > > > > We *never* discard a CSN. We only discard the counter/watermark mods
> > > > > > in the replication operation.
> > > > > What Thierry is saying is that the individual attributes in the entry
> > > > > have associate the last CSN that modified them. Because you remove the
> > > > > mods when ValueD == ValueB the counter attribute will not have the
> > > > > associated CSN changed. But it doesn't really matter because the plugin
> > > > > will always keep things consistent.
> > > > Attached is a new version. It removes this optimization. If server X has
> > > > valueB/csnB and receives valueD/csnD and valueB == valueD, the
> > > > replication will be applied without any modification. However, if valueB
> > > > > valueD and csnD > csnB, the counter mods will still be stripped.
> > > > It also collapses the error check from betxnpre to bepre now that we
> > > > have a fix for https://fedorahosted.org/389/ticket/47919 committed. The
> > > > betxnpre functions are completely removed. Also, a dependency on 389
> > > > 1.3.3.4 (not yet released) is added.
> > > > 
> > > > Nathaniel
> > > Hello Nathaniel,
> > > 
> > >         For me the code is fine. Ack.
> > New attached patch.
> > 
> > >         I have two minor comments:
> > >               * in preop_mod, when a direct update moves the counter
> > >                 backward you send UNWILLING to perform with a message.
> > >                 The message is allocated with slapi_ch_smprintf, you
> > >                 may free it with slapi_ch_free_string (rather than
> > >                 'free').
> > Fixed.
> > 
> > >               * About this message, for example when you have these
> > >                 MODS (I admit they make no sens):
> > >                 
> > >                 changetype: modify
> > >                 ipatokenHOTPcounter: MOD_DELETE
> > >                 -
> > >                 ipatokenHOTPcounter: MOD_INCREMENT
> > >                 
> > >                 The returned message will be "Will not decrement
> > >                 ipatokenHOTPcounter", because 'simulate' will return
> > >                 'COUNTER_UNSET+1'.
> > >                 Is it the message you expected ?
> > I changed the logic in simulate(). Please review it.
> > 
> > Nathaniel
> > 
> Hello Nathaniel,
> 
> 
>         The patch is ok for me. Ack.

Since the ACK, the upstream 389 fix actually landed in 1.3.3.5. This
patch changes nothing except the dependency version. I have tested it
against the 1.3.3.5 build.

Nathaniel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-npmccallum-0064.8-Create-ipa-otp-counter-389DS-plugin.patch
Type: text/x-patch
Size: 34602 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20141014/f66ab1f0/attachment.bin>