lbd modules and recursion

[Andrew Bartlett]

On Tue, 2005-12-27 at 10:33 +0100, Simo Sorce wrote:
> On Tue, 2005-12-27 at 20:06 +1100, Andrew Bartlett wrote:
> 
> > I'm very worried that in the real world, with potentially arbitrary
> > modules, that such loops would be unavoidable, if modules were to call
> > back up the stack.
> 
> Only if modules programmers do not make a rule of preventing them.

Or the infrastructure it setup to be less painful.

> > Thanks Simo!  Yes, I agree that order in ldb modules is absolutely
> > critical, and that very bad things happen if the order is abused or
> > setup incorrectly.  However I disagree with your conclusion: that an
> > arbitrary ldb module should never call ldb_next_request() for new
> > operations:
> > 
> > For your straw man you drew this stack: 
> > > API entry point
> > > - paged_results
> > > - sort
> > > - rdn_name
> > > - operational
> > > - schema
> > > - <your brand new module>
> > >__ ldb_tdb
> > 
> > Indeed, if you add a new module last, then it may violate the schema,
> > regardless of if it called ldb_next_request() with a modification of an
> > existing add/modify/delete request, or if it set off a new request.
> 
> In case of ldb_next_request() being called directly you will have a
> modification that violate the schema, if you start from top, the schema
> module will return your module an error and you will not be allowed to
> break the tree. That's a big difference.

You make a big point of the schema violation prevention.  However you
miss my point.  If a module can do 'bad things' attached to the original
request, which clearly must go to ldb_next_request(), what do you gain
by saying it cannot issue new requests at that layer?

> > My straw man is a slightly different stack:
> > 
> > API entry point
> >  - paged_results
> >  - DN mapping (dc=example,dc=com -> dc=samba,dc=org)
> >  - rdn_name
> >  - operational
> >  - <your brand new module>
> > __ ldb_ldb
> 
> aside, but probably that will not be the right place for the DN mapping
> module.
> 
> > Now, in my new module, I am watching for any operations that change the
> > unicodePwd, and need to update some values, including the
> > msDS-KeyVersionNumber attribute.  (I am updating keys, password
> > histories and the like).
> > 
> > Because the caller doesn't touch msDS-KeyVersionNumber I must search for
> > it (and atomically increment it).
> > 
> > In the above example, if I were to issue an LDAP search on the DN I see
> > at my new module, it would not exist.  I would be searching on
> > uid=abartlet,dc=samba,dc=org but the API entry point is expecting names
> > in the form of uid=abartlet,dc=example,dc=com.  
> 
> the API entry point do not expect much, I'd like to remember that ldb is
> DN agnostic. If you have a problem like this I'd suggest you modify the
> DN mapping module (or simply put your module above it) to accept direct
> calls to mapped tree. Such a mapping module will probably break other
> existing modules like the schema module anyway.

My point is that a module should have a consistant world view.  It is
making modifications to what it can see of the database below it.
Therefore, all searches/modifications/deletes should be in reference to
what it can see below it.  What an application may see form above is
irrelevant.  

> > Furthermore, when in the call handler I go to modify the user's record.
> > I do not wish to be recursively called by my own modification, even if I
> > have to structure it as an additional operation.
> 
> you should use a token system of some sort inside your modules
> private_data to avoid recursions, but from what you say your
> modification will not catch your code as you are not touching
> userPassword yourself, so that's not a problem, the second call will
> pass through your module and recursion is avoided.

I think this is creating more work, and I don't see the gain.

> > As such, I suggest that the ordering of partitions, schema and other
> > modules in the stack is completely critical, but that new modules should
> > not recurse back to the top of the stack.  They should call
> > ldb_next_request(), so that they always have a consistent view of what
> > is below them. 
> 
> No, as said, that would make more problems than what it solves.
> 
> You cannot add entries and not pass through rdn_name and operational
> modules (and other we may or may not have yet introduced) as you
> suggest. That may break the tree, and by doing this you do make
> assumptions about other modules. You do not know which modules will be
> added in front of yours or behind yours. 

I think it is quite sane to have modules which depend on other modules.
I also expect we will consolidate many of these modules.  

> You MUST be prepared to handle recursion because the module you call
> with ldb_next_request() may call a new ldb_modify() operation.

I think this is the part that is creating problems.  If you remove this
MUST (which does not currently exist in real code), I think we have a
simpler system.

> It may not be a direct call it can be any sequence of calls.

Indeed!

> As a general rule calling the stack from the start is the right thing.
> In some special case we may consider about breaking this rule, but I do
> not see as this being the case to do so, please provide more details if
> you think your case cannot be solved taking in account new operations
> are made from start each time.

I feel very strongly that a module should not have more than one 'view'
of the database.  Each database should see the world as presented by
those modules below it.  Anything more is going to simply cause
confusion. 

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org
Student Network Administrator, Hawker College  http://hawkerc.net
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : http://lists.samba.org/archive/samba-technical/attachments/20051227/af113b5e/attachment.bin