#48870 The plugins precedence is enforced in the opposite order for extop
Closed: wontfix None Opened 8 years ago by tbordaz.

test on DS master.

The fix https://fedorahosted.org/389/ticket/48770 changes the way to select the plugin for a given extop OID.

plugin_determine_exop_plugins when it finds a plugins handling a given OID, does not select/return that plugin but continue the loop to find if it exists an other one.

The consequence is that, for example in freeipa, "Password Modify extended operation plugin" (core server) being after "IPA Password Extended Operation plugin" in the list.
"IPA Password Extended Operation plugin" is found first, but the loop continues and finally "Password Modify extended operation plugin" is selected and returned

This was done differently in plugin_call_exop_plugins where the first plugin handling a given OID was returned.

For example if we assign nsslapd-pluginprecedence: 51 (above default 51) "IPA Password Extended Operation plugin" is selected because it then appears after "Password Modify extended operation plugin" for the same OID


William, IPA is now aware of this - and they are getting nrevous the fix will be ready for 7.3

I think the fault really lies here:

{{{
add_plugin_to_list()
...
if (plugin->plg_precedence < (tmp)->plg_precedence)
...
plugin->plg_next =
tmp;
}}}

So if two plugins have the same precedence, the one that is added last, get's pushed to the end.

Then, the code in plugin_determine_exop_plugins() appears to take the last matching value, rather than the first.

So we can fix it in determine, or in add_plugin_to_list(). I think determine as we limit the scope of the change, and limit the impact.

Replying to [comment:4 firstyear]:

I think the fault really lies here:

{{{
add_plugin_to_list()
...
if (plugin->plg_precedence < (tmp)->plg_precedence)
...
plugin->plg_next =
tmp;
}}}

So if two plugins have the same precedence, the one that is added last, get's pushed to the end.

Then, the code in plugin_determine_exop_plugins() appears to take the last matching value, rather than the first.

So we can fix it in determine, or in add_plugin_to_list(). I think determine as we limit the scope of the change, and limit the impact.

I successfully tested https://fedorahosted.org/389/attachment/ticket/48870/0001-Ticket-48870-Correct-plugin-execution-order-due-to-c.patch within freeipa environment

Setting precedence selects the appropriate extop according to the default value (50) of passwd_modify_extop.

About your remark on add_plugin_to_list, I agree with you that on the same precedence we are adding the plugin to the end.
But I was surprised to see (without your fix) that passwd_modify_extop was selected instead of ipapwd_extop (both handling the same OID and having the same precedence 50). That would mean that in the list we had [..,ipapwd_extop,...,passwd_modify_extop] that is strange because passwd_modify_extop is added before ipapwd_extop and so with add_plugin_to_list doing add_to_the_end we should rather have the opposite order.

well, the question becomes "what is adding before" even mean? How are you determining this order? I don't think it's DSE.ldif order, I suspect it's alphabetical, but I would need to investigate ....

Anyway,

commit 7f8c64f
Writing objects: 100% (6/6), 1.04 KiB | 0 bytes/s, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
dcec327..7f8c64f master -> master

Metadata Update from @tbordaz:
- Issue assigned to firstyear
- Issue set to the milestone: 1.3.5.5

7 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/1929

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)

4 years ago

Log in to comment on this ticket.

Metadata