Repository navigation
Issue 6680 - instance read-only mode is broken - #6681
Conversation
droideck
left a comment
droideck
left a comment
There was a problem hiding this comment.
Besides the minor issues, LGTM!
| char *error = NULL; | ||
| const char *regexp = ""; | ||
|
|
||
| if (dsename) { |
There was a problem hiding this comment.
Should this be if (!dsename) { - or do I miss something?
There was a problem hiding this comment.
Good catch but now I wonder why the testcase is working ?
| if (ro) { | ||
| /* read-only mode and dse is up to date ==> Do not modify it. */ | ||
| return ro; | ||
| } |
There was a problem hiding this comment.
I think the tests worked because we return here.
I can't recall exactly the order config init operates on the start-up, but I think it has the config read into memory at this point.
And the core part of the fix is is_rejected_op func.
There was a problem hiding this comment.
The impact of the dsename bug is that is_readonly_set_in_dse always returns false, so ro variable stays false and internal operation get written in the dse.file.
I think that the reason the testcase pass is because it does not check that dse file is not modified after the restart.
I will improve that.
| inst.config.replace(attr, val) | ||
| inst.config.replace(RO_ATTR, 'off') | ||
| inst.restart | ||
| assert inst.config.get_attr_val_utf8(RO_ATTR) == "off" |
There was a problem hiding this comment.
Is it missing the step 7 - update a config attribute when RO is disabled
There was a problem hiding this comment.
Sorry, I do not understand what do you mean. Could you precise ?
|Oh, I guess you meant Step 8: Try to modify another config attribute
(That is now step 14. I will add it)
| static bool | ||
| is_readonly_set_in_dse(const char *dsename) | ||
| { | ||
| Slapi_Regex *re_config = recomp("^dn:\\s+cn=config\\s*$"); |
There was a problem hiding this comment.
Shouldn't it match only for a line ending with 'cn=config' ?
There was a problem hiding this comment.
Matching a line ending with 'cn=config' would also work because cn=config is always the first entry that match the ending with 'cn=config' condition
But since the function is searching for nsslapd-readonly in cn=config entry, doing an exact match is more logical.
FYI: nsslapd-readonly is also an attribute of the backend config entry and we should ignore it.
| } | ||
|
|
||
| /* Tombstone fixup are not rejected. */ | ||
| if (operation_is_flag_set(op, OP_FLAG_TOMBSTONE_FIXUP)) { |
There was a problem hiding this comment.
Should we also accept RUV update (OP_FLAG_REPL_RUV) ? As there are no update, RUV should not change so it is probably fine to ignore RUV.
Or it may be handle by later test that accept all internal operation.
There was a problem hiding this comment.
Internal op are accepted only on dse backendthat is not replicated and has no tombstone ...
Since old code was already only checking OP_FLAG_TOMBSTONE_FIXUP, I think that your first assumption is right: no write operation means no RUV nor tombstone change
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: 389ds#6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend. Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode) Issue: #6680 Reviewed by: @droideck, @tbordaz (thanks!)
3 participants
Read only mode is broken because some plugins fails to starts as they are not able to create/updates some entries in the dse backend.
Solution is to allow interrnal operations to write in dse.backend but not modify the dse.ldif (except for the special case when trying to modify nsslapd-readonly flags (to be allowed to set/unset the readonly mode)
Issue: #6680
Reviewed by: @droideck, @tbordaz (thanks!)