Issue 4987 - imporvement to slapo-constraint
Summary: imporvement to slapo-constraint
Status: VERIFIED FIXED
Alias: None
Product: OpenLDAP
Classification: Unclassified
Component: contrib (show other issues)
Version: unspecified
Hardware: All All
: --- normal
Target Milestone: ---
Assignee: OpenLDAP project
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-05-29 06:30 UTC by manu@openldap.org
Modified: 2014-08-01 21:05 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description manu@openldap.org 2007-05-29 06:30:04 UTC
Full_Name: Emmanuel Dreyfus
Version: 4.4.4
OS: NetBSD
URL: ftp://ftp.openldap.org/incoming/manu-070529.ext
Submission from: (NULL) (213.41.141.172)


I modified slapo-constraint so that it can verify that an attribute value 
is bound to the existing values of another attribute. The purpose is to 
make referential integrity easier to obtain.

Attached is a draft patch for review.

Then there is the UI question. For now it's configured in slapd.conf like
this:
database        bdb
suffix          "dc=example,dc=net"
overlay         constraint
constraint_attribute title key netExampleTitle

Which means that add and modify on title will fail if the new value is
not an existing netExampleTitle value. I used the keyword "key" with
RDBMS referential integrity in mind, but I'm not sure it's that clear
in this context. Suggestions are welcome.

The update to man page is missing yet. I know.

Also, I had trouble understanding the style used in these sources. Is there
an official style guide for OpenLDAP?

Comment 1 ando@openldap.org 2007-06-02 09:35:31 UTC
moved from Incoming to Contrib
Comment 2 Gavin Henry 2007-07-06 09:00:06 UTC
Hi,

Did you see http://www.openldap.org/devel/contributing.html

Gavin.
Comment 3 manu@openldap.org 2007-07-07 13:30:16 UTC
> Did you see http://www.openldap.org/devel/contributing.html

Right, but before moving foward with appropriate style, documentation,
and so on, I'd like to have some input about the config syntax and the
way it's implemented. I had no feedback at all yet on that.

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@netbsd.org

Comment 4 Gavin Henry 2007-07-07 19:13:00 UTC
<quote who="manu@netbsd.org">
>> Did you see http://www.openldap.org/devel/contributing.html
>
> Right, but before moving foward with appropriate style, documentation,
> and so on, I'd like to have some input about the config syntax and the
> way it's implemented. I had no feedback at all yet on that.

It would be best to move this dicussion to openldap-devel@openldap.org now.

Try raising a question again there.

Gavin.

>
> --
> Emmanuel Dreyfus
> http://hcpnet.free.fr/pubz
> manu@netbsd.org
>
>
>

Comment 5 manu@openldap.org 2007-07-07 20:15:37 UTC
Gavin Henry <ghenry@suretecsystems.com> wrote:

> It would be best to move this dicussion to openldap-devel@openldap.org now.

Let's go...

Summarry: I've added a new feature to slapo-constraint for constraining
a value attribute the the existing values of another attribute. The idea
is that you can have a catalog of allowed values (eg: for titles: Mr,
Mrs, Miss) and constraint an attribute to these values

Before working further on this patch (style, dod), I'd like some feeback
on:
1) the way it's implemented: is there rought bugs, or is the logic fine?
2) configuration syntax: do we keep this one or do we swtich to
something else?

Could someone comment on the patch? 

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@netbsd.org

Comment 6 manu@openldap.org 2007-09-19 03:24:31 UTC
Here are improvements:

1) Instead of matching against an individual attribute, match against a
set of values returned by an LDAP URI:
ftp://ftp.openldap.org/incoming/manu-070918.patch

2) a man page for slapo-constraint(5)
ftp://ftp.openldap.org/incoming/manu-pkg-070919.tgz

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@netbsd.org

Comment 7 Kurt Zeilenga 2007-11-27 15:14:29 UTC
changed notes
Comment 8 Kurt Zeilenga 2007-11-27 15:32:48 UTC
Emmanuel,

Your submission does not contain the necessary IPR notices.  In particular, it
does not contain either a 
notice of origin nor a complete rights statement.  See
http://www.openldap.org/devel/contributing.html 
for discussion and suggestions in this area. 

Please upload a new patch file which contains both of these notices and then
your diff (updated if 
appropriate), and then send an email update to the ITS referencing the new 
patch file.

Thanks, Kurt
Comment 9 manu@openldap.org 2007-12-02 20:41:40 UTC
> Please upload a new patch file which contains both of these notices and then
> your diff (updated if 
> appropriate), and then send an email update to the ITS referencing the new
> patch file.

Here we are:
ftp://ftp.openldap.org/incoming/manu-071202.patch
ftp://ftp.openldap.org/incoming/manu-pkg-071202.tgz

-- 
Emmanuel Dreyfus
http://hcpnet.free.fr/pubz
manu@netbsd.org

Comment 10 Howard Chu 2007-12-15 04:16:03 UTC
In reviewing the patch, I see several small details that need to be fixed. 
E.g., you added a config keyword but re-used an existing OID for your new 
attribute. OIDs must never be re-used, and certainly you must not use two 
different attributes with the same OID.

You use strlen/strncat, which are heavily discouraged in our code. (We never 
use strcat/strncat; we use strlen only when it's unavoidable.) (Some uses of 
strlen already existed in constraint.c; we just didn't get around to 
eliminating them.) In general we use bervals instead of flat strings, and 
there's almost never a reason to count string lengths at runtime.

I got 3 rejects when applying the patch to my source tree (current HEAD).

I think these points can all be cleaned up pretty easily. Let me know if you 
need any particular pointers.
-- 
   -- Howard Chu
   Chief Architect, Symas Corp.  http://www.symas.com
   Director, Highland Sun        http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP     http://www.openldap.org/project/

Comment 11 Howard Chu 2007-12-15 04:39:46 UTC
hyc@symas.com wrote:
> In reviewing the patch, I see several small details that need to be fixed.
> E.g., you added a config keyword but re-used an existing OID for your new
> attribute. OIDs must never be re-used, and certainly you must not use two
> different attributes with the same OID.

Oh never mind, I see the confusion. You're just defining a new 2nd argument to 
the existing keyword.

> You use strlen/strncat, which are heavily discouraged in our code. (We never
> use strcat/strncat; we use strlen only when it's unavoidable.) (Some uses of
> strlen already existed in constraint.c; we just didn't get around to
> eliminating them.) In general we use bervals instead of flat strings, and
> there's almost never a reason to count string lengths at runtime.
> 
> I got 3 rejects when applying the patch to my source tree (current HEAD).
> 
> I think these points can all be cleaned up pretty easily. Let me know if you
> need any particular pointers.

I'm working on getting this to build now.
-- 
   -- Howard Chu
   Chief Architect, Symas Corp.  http://www.symas.com
   Director, Highland Sun        http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP     http://www.openldap.org/project/

Comment 12 Howard Chu 2007-12-15 10:13:14 UTC
changed notes
changed state Open to Test
Comment 13 Kurt Zeilenga 2008-02-08 22:48:31 UTC
changed notes
Comment 14 Quanah Gibson-Mount 2008-02-12 20:08:52 UTC
changed notes
changed state Test to Release
Comment 15 Quanah Gibson-Mount 2008-02-20 02:26:29 UTC
changed notes
changed state Release to Closed
Comment 16 Howard Chu 2009-02-17 07:00:14 UTC
moved from Contrib to Archive.Contrib
Comment 17 OpenLDAP project 2014-08-01 21:05:17 UTC
in HEAD
in 2.4.8