Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow mod_post to consider existing objects for addition to MEP #1471

Closed
389-ds-bot opened this issue Sep 12, 2020 · 11 comments
Closed

Allow mod_post to consider existing objects for addition to MEP #1471

389-ds-bot opened this issue Sep 12, 2020 · 11 comments
Labels
closed: not a bug Migration flag - Issue
Milestone

Comments

@389-ds-bot
Copy link

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


MEP expects that the only time it will create a managed entry is on the addition of the entry. Modifying an existing entry and then having it retrospectively add the managed entry is not supported.

Currently, if one creates an object such as:

objectClass: top
objectClass: account
uid: foo
sn: bar

Followed by a mod operation to add posixAttributes, in a standard MEP with user -> group template, the group is not created, because the operation isn't considered unless it's an add.

This attached patch adds support and dynamic plugin tests to allow MEP to create managed entries if a mod operation would bring an entry into a state where it now satisfies the criteria of the template.

@389-ds-bot 389-ds-bot added the closed: not a bug Migration flag - Issue label Sep 12, 2020
@389-ds-bot 389-ds-bot added this to the 1.3.4 backlog milestone Sep 12, 2020
@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2015-03-28 06:43:07

Additionally, the patch has commented tests and locations in mep.c for the inverse, where when you take an entry that has a managed entry, and remove "enough" parts of it to fall out of the mep scope, that the managed entry should be removed also. This behaviour however, being more "destructive" should be controlled by a configuration item to state whether the current MEP behaviour or the new behaviour is followed:

  • Current -> If you remove the parts of an entry that would make it fall out of the MEP scope, the managed entry is not removed.
  • Proposed optional -> If you remove the parts of an entry that would make it fall out of the MEP scope, the managed entry is removed.

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2015-04-02 05:55:27

Hi William,

I reviewed the mep.c part. Your fix looks good.
I guess you are currently working on movig the test script to ds/dirsrvtests/suites/mep_plugin.
I could wait for your new patch with the change for another review to push your patch. Or if you split the patch into 2 -- one for mep.c and another for the test script, then I'd push the mep.c one to the git repo. Either way works for me.

Also, setting the milestone to 1.3.4.

Thank you so much for your contribution!
--noriko

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2015-04-02 06:02:14

Replying to [comment:1 Firstyear]:

  • Current -> If you remove the parts of an entry that would make it fall out of the MEP scope, the managed entry is not removed.
  • Proposed optional -> If you remove the parts of an entry that would make it fall out of the MEP scope, the managed entry is removed.

Regarding this proposal, once the origin entry gets out of scope of the mep plug-in, the managed entry can be manually removed, can it? (Please note that if it is in the scope, it is not allowed.)

Automated deletion sometimes causes unexpected surprise. To implement it, a new config parameter needs to be introduced and switch between reserving the current behavior and the new behavior.

But the proposal itself looks very useful. Could you please open a new ticket for the feature?

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2015-04-02 07:03:14

I'll tidy this up into two patches. One for the tests, one for the feature.

I will add another ticket for the deletion feature. I suspected that configuration option for this would be the preferred way forwards. The deletion is a bit more complicated to implement, and I have a skeleton version of it partially working already.

I'm currently investigating a few outstanding failures in the tests, and will update accordingly.

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2015-04-02 23:54:10

additional patch: it'll avoid the mod ops if it was called from the plug-in. but still my test does not pass... keep investigating...
0001-Ticket-48140-Allow-ME-creation-on-mod_post-for-exist.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2015-04-08 12:16:19

Patch to allow MEP to create managed entries if an object modification would cause the object to satisfy the mep criteria
0001-Allow-ME-creation-on-mod_post-for-existing-entries.patch

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2015-04-08 12:17:47

Updated patch with cleaned up tests. Passes full dynamic test suite and stress test.

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2015-07-07 02:13:55

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1240405

@389-ds-bot
Copy link
Author

Comment from firstyear (@Firstyear) at 2015-07-07 04:57:14

I would rather focus on http://directory.fedoraproject.org/docs/389ds/design/mep-rework.html and drop these changes. There were too many issues with my original approach.

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2015-07-07 05:15:40

Replying to [comment:10 Firstyear]:

I would rather focus on http://directory.fedoraproject.org/docs/389ds/design/mep-rework.html and drop these changes. There were too many issues with my original approach.

Thanks for the clarification, William. I'm going to close the associated bug.

@389-ds-bot
Copy link
Author

Comment from nhosoi (@nhosoi) at 2017-02-11 22:58:49

Metadata Update from @nhosoi:

  • Issue assigned to Firstyear
  • Issue set to the milestone: 1.3.4 backlog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed: not a bug Migration flag - Issue
Projects
None yet
Development

No branches or pull requests

1 participant