Fix bugs in CONFIG REWRITE, omitting rename-command and include lines, and inserting comments around module and acl configs (#10761)

A regression from #10285 (redis 7.0).
CONFIG REWRITE would put lines with: `include`, `rename-command`,
`user`,  `loadmodule`, and any module specific config in a comment.

For ACL `user`, `loadmodule` and module specific configs would be
re-inserted at the end (instead of updating existing lines), so the only
implication is a messy config file full of comments.

But for `rename-command` and `include`, the implication would be that
they're now missing, so a server restart would lose them.

Co-authored-by: Oran Agra <oran@redislabs.com>
This commit is contained in:
zhugezy 2022-06-02 13:36:55 +08:00 committed by GitHub
parent c81b5e5594
commit cf3323dba4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 37 additions and 4 deletions

View File

@ -1142,10 +1142,21 @@ struct rewriteConfigState *rewriteConfigReadOldFile(char *path) {
/* Not a comment, split into arguments. */
argv = sdssplitargs(line,&argc);
if (argv == NULL || (!server.sentinel_mode && !lookupConfig(argv[0]))) {
/* Apparently the line is unparsable for some reason, for
* instance it may have unbalanced quotes, or may contain a
* config that doesn't exist anymore. Load it as a comment. */
if (argv == NULL ||
(!lookupConfig(argv[0]) &&
/* The following is a list of config features that are only supported in
* config file parsing and are not recognized by lookupConfig */
strcasecmp(argv[0],"include") &&
strcasecmp(argv[0],"rename-command") &&
strcasecmp(argv[0],"user") &&
strcasecmp(argv[0],"loadmodule") &&
strcasecmp(argv[0],"sentinel")))
{
/* The line is either unparsable for some reason, for
* instance it may have unbalanced quotes, may contain a
* config that doesn't exist anymore, for instance a module that got
* unloaded. Load it as a comment. */
sds aux = sdsnew("# ??? ");
aux = sdscatsds(aux,line);
if (argv) sdsfreesplitres(argv, argc);

View File

@ -933,3 +933,13 @@ start_server [list overrides [list "dir" $server_path "aclfile" "user.acl"] tags
assert_match {*Duplicate user*} $err
} {} {external:skip}
}
start_server {overrides {user "default on nopass ~* +@all -flushdb"} tags {acl external:skip}} {
test {ACL from config file and config rewrite} {
assert_error {NOPERM *} {r flushdb}
r config rewrite
restart_server 0 true false
assert_error {NOPERM *} {r flushdb}
}
}

View File

@ -582,3 +582,15 @@ test {config during loading} {
exec kill [srv 0 pid]
}
} {} {external:skip}
test {CONFIG REWRITE handles rename-command properly} {
start_server {tags {"introspection"} overrides {rename-command {flushdb badger}}} {
assert_error {ERR unknown command*} {r flushdb}
r config rewrite
restart_server 0 true false
assert_error {ERR unknown command*} {r flushdb}
}
} {} {external:skip}