389-ds-base

#50333 Ticket 50327 - Add replication conflict entry support to lib389/CLI
Closed by spichugi. Opened by mreynolds.
mreynolds/389-ds-base ticket50327  into  master

Download 50333.patch

Description:

Added Conflict Entry and Glue entry classes to lib389, and updated dsconf to allow for conflict entry management.

Made some other minor changes to mapped objects:

            -  Added an attribute list option to display()
            -  Added a recursive delete option to delete()
            -  _gen_filter - had to prevent escape_filter_chars()
               from escaping "*" as it would convert it to /02A
               and the server would not process pres filters
               correctly.

https://pagure.io/389-ds-base/issue/50327

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

IIRC there is already a ressurct method, and already a conflict type?

Have you see lib389/tombstone.py? Maybe this is similar and could be helpful?

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?

It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

@lkrispen What do you think here? there are some functions to revive/convert conflict entries and glue entries. I think if we want to revive these it would be safer to make new entries and copy the attributes to the new entry rather than deleting/modifying the existing conflict/glue. What do you think about this?

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

Hmm this does looks wrong, I'll look into it tomorrow

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?

It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

This is how the admin guide describes how to handle these conflict & glue entries. Just following the documented design...

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?

Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.

What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

@mreynolds It would be nice to see the intended usage by the CLI to see what the user can ar has to provide as input

IIRC there is already a ressurct method, and already a conflict type?

for this and the following comments: please do not confuse conflicts with tombstones. Conflicts are "real" entries, now hidden (and more consistently created), but all the managent operations like cleanup as described in the admin guide do still apply.

There is no need to copy/delete these entries, they can be handled like any other entry with modrdn and modify operations

It may not be a good idea to remove these, given they are part of the replication machinery, it could be better to leave them alone?
It may be safer to duplicate the entry content back to a new entry rather than trying to revive this as it's part of the replication machinery.

This is how the admin guide describes how to handle these conflict & glue entries. Just following the documented design...

correct, I just don't see the usage in the cli, and for glue entries, if you want to delete them
- you have to delete all their children as well, but
- you could do this with a normal recursive delete, don't know if we need a special function

@lkrispen What do you think here? there are some functions to revive/convert conflict entries and glue entries. I think if we want to revive these it would be safer to make new entries and copy the attributes to the new entry rather than deleting/modifying the existing conflict/glue. What do you think about this?

no, no need to create new entries first and it would probably also have unexpected side effects. if you create and copy you get a new entry with a different nsuniqueid, but you want to keep the entry, just rename it, or delete some attribute.

Conflicts are not really "part of the replication machinery", they come int existence by replication, buit once created they are "normal", valid entries

If you decided to format it properly, could you please check the pep8 and use it then? :)
https://legacy.python.org/dev/peps/pep-0008/#indentation

isn't it objectClass=ldapsubentry and nsds5replconflict? Atm this would pick up old COS defs that had ldapsubentry on them.

Hmm this does looks wrong, I'll look into it tomorrow

as far as I see you are using it in BASE searche, so this would only require the objectclass filter jut to be able to see the entry if it is an ldapsubentry

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

@lkrispen here is an example of using the CLI (as requested).

List conflict entries:

dsconf slapd-localhost repl-conflict list

Compare conflict entry with its "valid" entry counterpart

dsconf slapd-localhost repl-conflict compare <DN of conflict entry>

--> List lists two entries, the conflict and the valid entry so you can see what is different so you can decided if you want to swap them, or "resurrect", for lack of a better term, the conflict entry.

To swap the conflict entry with the "valid" entry do this

dsconf slapd-localhost repl-conflict swap <DN of conflict entry>

To resurrect conflict (meaning we keep the valid entry as well). We use this command with a new rdn value

dsconf slapd-localhost repl-conflict  resurrect <DN of conflict>  --new-rdn "cn=new rdn"

To delete conflict entry run:

dsconf slapd-localhost repl-conflict delete <DN of conflict entry>

Now for Glue entries...

List Glue entries

dsconf slapd-localhost repl-conflict list-glue

Delete Glue entry and its children

dsconf slapd-localhost repl-conflict delete-glue <DN of glue entry>

Convert Glue entry to "normal" entry

dsconf slapd-localhost repl-conflict convert-glue <DN of Glue entry>
Edited by mreynolds

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

That was the plan, but it was never started and we were still adding CLI tests to the old location. So until it was ported to /dirsrvtests I was updating everything in one location.

If you decided to format it properly, could you please check the pep8 and use it then? :)
https://legacy.python.org/dev/peps/pep-0008/#indentation

Sure I'll check this out!

rebased onto 01f907b1783ffd73f4dfc5905ba26c0293b71b89

rebased onto cbec098cfaadee1c329214084eb9c853282113f8

rebased onto baff11cafda98107e76e503350e629d191d21f42

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?
Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"

I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.
What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

Then why are we using it for internal classes @firstyear?

Well I'll just hardcoded the filter, but I really don't like that we are using these filter generation functions that will cause substring and presence filters to silently fail. They don't belong in our internal API IMO. Anyway it's no biggie, I worked around it by hardcoding the filter.

@spichugi - fixed pep8 errors in mapped_object.py

@lkrispen - I provided the CLI examples in the previous comment.

Please review...

@lkrispen - I provided the CLI examples in the previous comment.
Please review...

thanks for the examples, they look good and useful, with one exception. I would not use "resurrect", it is used for tombstones and will be confusing. What you want to do is to make the conflict entry a valid entry with a newrdn, so I would use something like "rename", "make -valid", "keep-as"

in general I think this is very helpful to deal with conflicts, the procedures in the admin guide are good, but complicated.

I let you settle the lib389/python arguments with the pythonians :-)

@lkrispen - I provided the CLI examples in the previous comment.
Please review...

thanks for the examples, they look good and useful, with one exception. I would not use "resurrect", it is used for tombstones and will be confusing. What you want to do is to make the conflict entry a valid entry with a newrdn, so I would use something like "rename", "make -valid", "keep-as"

Agreed, the more I look at this the more I hate "resurrect", originally it was "convert", but I changed it. I think "rename" is the more accurate argument name so I will change it, but I'm open to other name suggestions :-)

In fact I do like "convert" , it indicates that it is a bit more than rename, eg under the hood there is the removal of th ldapsubentry and nsds5replconflict. So I think your initial choice was the best

rebased onto 9f55a3c0cd2a94bcfbef9def8ca1fa0d822a3a7c

In fact I do like "convert" , it indicates that it is a bit more than rename, eg under the hood there is the removal of th ldapsubentry and nsds5replconflict. So I think your initial choice was the best

Hahaha, I thought same thing after the fact, and the rebase uses "convert". :-D It is also more consistent with the glue function argument names

As far as I recall, we try to get rid of lib389 tests and move them to dirsrvtests/tests/suites/lib389/

That was the plan, but it was never started and we were still adding CLI tests to the old location. So until it was ported to /dirsrvtests I was updating everything in one location.

@spichugi - FYI - looks like this is still being debated as well:

https://pagure.io/389-ds-base/issue/49911

Either way we should move the tests via a different ticket, not this one...

@spichugi - FYI - looks like this is still being debated as well:
https://pagure.io/389-ds-base/issue/49911
Either way we should move the tests via a different ticket, not this one...

Agree.
I think we should decide it sooner though because CLI is growing and more test will appear.
But I think the decision does belong more to @vashirov . So I'd wait for his return.
For now, I think it is okay to have it like this. :)

This is actually the whole point of this here is to prevent terms like * and () because there are security risks with allowing them to be input by users. If you need a * search, then there are probably other ways to construct it?
Well I need to construct a filter like "(&(objectclass=ldapsubentry)(nsds5replconflict=*))"
I agree, to find conflict entries we need this filter, the nsds5replconflict attribute is a string encoding where the conflict was coming from ADD/MODRDN and what teh dn of the conflicting entry was ..... So the presence search is the only really prdictable ay to address these entries.
What William said was to prevent filters like these entered by users, but I think this is not the case, it is an internally generated filter.

Then why are we using it for internal classes @firstyear?

Because we don't know where input could come from because python so we can't exactly do tracking of input data, so we assume everything is untrusted.

The way you get around this, is something like:

    def _get_objectclass_filter(self):
        return '(objectclass=ldapsubentry)(nsds5conflict=*)'

Which then will work in this case. All the gen instructions do is yield strings, so there are cases where you can just return them, and they should work just fine.

Well I'll just hardcoded the filter, but I really don't like that we are using these filter generation functions that will cause substring and presence filters to silently fail. They don't belong in our internal API IMO. Anyway it's no biggie, I worked around it by hardcoding the filter.

Which is exactly the point :) if we hardcode it internally, it's internal, we can do this safely. But we have to always un-trust all other input. So in fact, hardcoding the filter is correct here, but so is leaving the safety-escaping alone :)

@spichugi - fixed pep8 errors in mapped_object.py
@lkrispen - I provided the CLI examples in the previous comment.
Please review...

Which is exactly the point :) if we hardcode it internally, it's internal, we can do this safely. But we have to always un-trust all other input. So in fact, hardcoding the filter is correct here, but so is leaving the safety-escaping alone :)

It's all good, I agree!

So, Is that an ack then? :-)

So I think all of @lkrispen's comments are addressed (it sounded like my misunderstanding anyway, but always safer to ask than assume), and I think @spichugi's comments are addressed, so ack here.

rebased onto 4f7c05e2879cee7d205531edb64b19ad799e20bd

Pull-Request has been merged by mreynolds

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 pull request has been cloned to Github as issue and is available here:
- https://github.com/389ds/389-ds-base/issues/3392

If you want to continue to work on the PR, please navigate to the github issue,
download the patch from the attachments and file a new pull request.

Thank you for understanding. We apologize for all inconvenience.

Pull-Request has been closed by spichugi
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.