The plugins precedence is enforced in the opposite order for extop

[389-ds-bot]

Cloned from Pagure issue: https://pagure.io/389-ds-base/issue/48870

• Created at 2016-06-06 20:55:43 by tbordaz (@tbordaz)
• Closed as Fixed
• Assigned to firstyear (@Firstyear)

---

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

[389-ds-bot]

Comment from lkrispen (@elkris) at 2016-06-07 20:32:45

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

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2016-06-08 04:57:33

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.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2016-06-08 05:33:44

attachment
0001-Ticket-48870-Correct-plugin-execution-order-due-to-c.patch

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2016-06-08 17:06:47

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.

[389-ds-bot]

Comment from firstyear (@Firstyear) at 2016-06-09 04:30:25

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

[389-ds-bot]

Comment from tbordaz (@tbordaz) at 2017-02-11 22:54:40

Metadata Update from @tbordaz:

• Issue assigned to Firstyear
• Issue set to the milestone: 1.3.5.5