#159 Managed Entry Plugin runs against managed entries upon any update without validating
Closed: wontfix None Opened 10 years ago by jraquino.

Managed Entries needs a method of validating that it is updating the managed entry due to some effected modification, rather than just 'any' modification to the parent object.


batch move to milestone 1.2.10.a7

Wrote a fix to check what attributes were modified against the attributes in the managed entry template. If there is no match, then do not modified the managed entry.

I also fixed a small memory leak.

Sending fix out for review...

Adding a performance improvement, should have a new patch tomorrow.

Mark

Mostly looks good. 2 minor things.

  1. a typo "cofig" in the comment. :)
    Fix Description: Store the origin attrs during the cofig processing. Then in the mod-post-op only perform

  2. 997 attr = slapi_ch_strdup(origin_type);
    998 charray_add(&origin_attrs,attr);
    999 slapi_ch_free_string(&origin_type);
    Since mep_parse_mapped_origin_attr returns internally allocated origin_type and the caller mep_extract_origin_attrs is responsible to free it, you could just pass origin_type to charray_add and let it consume? This way we could skip one strdup and free. (not a big thing here, but we'd like to avoid unnecessary malloc/free as much as possible...)

Actually we need to copy the attribute type, because it gets freed in mep_parse_mapped_origin_attr(). Regardless where it is done, an allocation will still be needed for charray update.

I did change the comment to fix the typo.

Replying to [comment:10 mreynolds]:

Actually we need to copy the attribute type, because it gets freed in mep_parse_mapped_origin_attr().
If you don't want to make mep_parse_mapped_origin_attr free origin_type, you could set NULL to the value before passing it (&origin_type) to mep_parse_mapped_origin_attr, couldn't you?

Regardless where it is done, an allocation will still be needed for charray update.

I did change the comment to fix the typo.

Noriko, you're right setting it to NULL is fine. I was looking at other examples of charray, etc. I attached a new patch. Thanks.

Thanks, Mark! Now, I've applied your patch and rebuilt the server on my box.
I see these compiler warnings.
ldap/servers/plugins/mep/mep.c: In function 'mep_free_config_entry':
ldap/servers/plugins/mep/mep.c:793:9: warning: implicit declaration of function 'slapi_ch_free_charray'
==> I cannot find slapi_ch_free_charray api, but only charray_free. As other plugins do, you have to include slapi-private.h and replace slapi_ch_free_charray with charray_free...

ldap/servers/plugins/mep/mep.c: In function 'mep_extract_origin_attrs':
ldap/servers/plugins/mep/mep.c:994:17: warning: implicit declaration of function 'charray_add'
==> This will be cleared by including slapi-private.h

ldap/servers/plugins/mep/mep.c:986:11: warning: unused variable 'attr_type'
==> You could just remove this variable...

Interesting. I never got any compiler warnings after running make. Also I meant to use slapi_ch_array_free() in the config free function. I'll look into the charray_add() tomorrow...

Thanks,
Mark

Ah, I see. That's better.
Replace slapi_ch_free_charray with slapi_ch_array_free, and
charray_add with slapi_ch_array_add.
Both are declared in slapi-plugin.h, so you don't have to include slapi-private.h.
Thank YOU!
--noriko

Changes have been made, testing was good, new patch was just attached.

Mark

Psuhed to master:

[mareynol@mareynol mep]$ git merge ticket159
Merge made by recursive.
ldap/servers/plugins/mep/mep.c | 248 +++++++++++++++++++++++++++++++++++++++-
ldap/servers/plugins/mep/mep.h | 1 +
2 files changed, 245 insertions(+), 4 deletions(-)
[mareynol@mareynol mep]$ git push
Counting objects: 24, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (13/13), done.
Writing objects: 100% (13/13), 2.72 KiB, done.
Total 13 (delta 8), reused 0 (delta 0)
To ssh://mreynolds@git.fedorahosted.org/git/389/ds.git
165518b..3e845fa master -> master

Hi Mark, could you provide steps to verify the bug on Bug 781529?
Thanks!

I will add this to the bugzilla as well:

[1] Create the template entry:

dn: cn=UPG Template,dc=example,dc=com
objectclass: mepTemplateEntry
cn: UPG Template
mepRDNAttr: cn
mepStaticAttr: objectclass: posixGroup
mepMappedAttr: cn: $uid
mepMappedAttr: gidNumber: $gidNumber
mepMappedAttr: description: User private group for $uid

[3] Create the group:

 cn=groups,dc=example,dc=com

[3] Set up mep config

dn: cn=UPG Definition,cn=Managed Entries,cn=plugins,cn=config
objectclass: extensibleObject
cn: UPG Definition
originScope: dc=example,dc=com
originFilter: objectclass=posixAccount
managedBase: cn=groups,dc=example,dc=com
managedTemplate: cn=UPG Template,dc=example,dc=com

[4] Create a user with objectclass=posixAccount (uid=original, dc=example,dc=com)

[5] Check the modifytimestamp on the new entry (cn=original, cn=groups, dc=example,dc=com)

[6] Modify the original entry(uid=original,dc=example,dc=com)- changing the "uidNumber" or any attribute that is not listed in the mep template(e.g. gidNumber or uid).

[7] The modifytimestamp should not be updated, as the fix is to stop unnecessary changes to mep entries

Added initial screened field value.

Metadata Update from @mreynolds:
- Issue assigned to mreynolds
- Issue set to the milestone: 1.2.10.a7

5 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/159

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)

2 years ago

Login to comment on this ticket.

Metadata