#71 [RFE] Permit 'Delete' operation for Managed Entry Config entries
Closed: wontfix None Opened 9 years ago by mkosek.

https://bugzilla.redhat.com/show_bug.cgi?id=660399

Description of problem:
Cannot delete a Managed Entry Config after it has been inserted into the
directory.


How reproducible:
Add a new Managed Entry Config, or attempt to delete an existing one.
e.g.  "cn=UPG Definition,cn=Managed Entries,cn=plugins,cn=config"

Steps to Reproduce:
1. Create ldif
2. Run ldapdelete against ldif
3. Error 53 Returned

Actual results:
ldap_delete: Server is unwilling to perform (53)
        additional info: Not a valid operation.

Expected results:
Delete object

Additional info:

Replying to [ticket:71 mkosek]:

https://bugzilla.redhat.com/show_bug.cgi?id=660399

{{{
Description of problem:
Cannot delete a Managed Entry Config after it has been inserted into the
directory.

How reproducible:
Add a new Managed Entry Config, or attempt to delete an existing one.
e.g. "cn=UPG Definition,cn=Managed Entries,cn=plugins,cn=config"

Steps to Reproduce:
1. Create ldif
2. Run ldapdelete against ldif
3. Error 53 Returned

Actual results:
ldap_delete: Server is unwilling to perform (53)
additional info: Not a valid operation.

Expected results:
Delete object

Additional info:
}}}

A fix has been built and tested.

Added a check for delete operations so that the operation would not get rejected. The postop correctly handles the delete.

Also converted all the char DN's to Slapi_DN's

Patch is attached.

Sending revised fix out for review...

{{{
2064 config_sdn = slapi_sdn_new_dn_byref(((struct configEntry *)list)->dn);
2065 if (slapi_sdn_compare(config_sdn, sdn) == 0) {
2066 found = 1;
2067 break;
}}}
config_sdn will be leaked at line 2067 - need to free it

Alternately, we should store the Slapi_DN instead of the char dn in configEntry so we don't have to convert it back to a Slapi_DN* for comparison.

It'd be a good thing to add running valgrind to the devel testing for finding out the memory problems including leaks.

I usually put this string before ns-slapd in /usr/sbin/start-dirsrv (and add "-d 0" at the end of the command line to run ns-slapd foreground).
"valgrind --num-callers=32 --tool=memcheck --leak-check=full --leak-resolution=high"

But there could be a better/easier way to run the server with valgrind. Rich, could there be any doc for that?

You don't need to check if the config entry being deleted actually exists in the in-memory config list at the pre-op stage. If the entry is not in the config list for some reason, it's fine to let the delete through. Since this check isn't needed, you don't need to add the new mep_get_config_entry_by_dn() function.

You also don't want to call mep_delete_configEntry() at the pre-op stage either, as there is still a possibility that the delete operation will fail (rejected by an ACI, etc.). In mep_pre_op(), you should simply allow delete operations for config entries (anything within the mep_dn_is_config(sdn) block). The pre-op stage is really just used for validation here, and there's no validation needed for a delete.

The mep_del_post_op() function already deals with deleting the in-memory config with this code:

{{{
/ Reload config if a config entry was deleted. /
if ((sdn = mep_get_sdn(pb))) {
if (mep_dn_is_config(sdn))
mep_load_config();
} else {
}}}

If you look at the mep_load_config() function, it deletes all of the in-memory config and then reloads it all from the config tree. Since the delete operation already went through, the deleted config won't exist and won't be loaded.

Pushed to master on behalf of Mark.

$ git merge trac71
Updating 77fdecc..c43a508
Fast-forward
ldap/servers/plugins/mep/mep.c | 105 +++++++++++++++++----------------------
ldap/servers/plugins/mep/mep.h | 4 +-
2 files changed, 48 insertions(+), 61 deletions(-)

$ git push
Counting objects: 15, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 1.78 KiB, done.
Total 8 (delta 5), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
77fdecc..c43a508 master -> master

Added initial screened field value.

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

4 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/71

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)

9 months ago

Login to comment on this ticket.

Metadata