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
Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=781529
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.
a typo "cofig" in the comment. :) Fix Description: Store the origin attrs during the cofig processing. Then in the mod-post-op only perform
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.
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.
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
Revised fix... 0001-Ticket-159-Managed-Entry-Plugin-runs-against-managed.patch
Changes have been made, testing was good, new patch was just attached.
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
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.
subscribe
Thank you for understanding. We apologize for all inconvenience.
Metadata Update from @spichugi: - Issue close_status updated to: wontfix (was: Fixed)
Login to comment on this ticket.