Describe the bug
Simplified config:
redis.conf
include /etc/redis/redis-inc.conf
redis-inc.conf
user default on nopass sanitize-payload allcommands allkeys allchannels
or (doesnt matter which format, both behave the same)
user default on nopass sanitize-payload ~* &* +@all
when we call CONFIG REWRITE the user directive is again resaved into redis.conf
This prevent redis from starting after service restart or system reboot with error
Error in user declaration 'default': Duplicate user found. A user can only be defined once in config files
Comment From: enjoy-binbin
thanks for the report. i see the problem, @madolson Do you have any suggestions?
from redis.conf:
# Include one or more other config files here. This is useful if you
# have a standard template that goes to all Redis servers but also need
# to customize a few per-server settings. Include files can include
# other files, so use this wisely.
#
# Note that option "include" won't be rewritten by command "CONFIG REWRITE"
# from admin or Redis Sentinel. Since Redis always uses the last processed
# line as value of a configuration directive, you'd better put includes
# at the beginning of this file to avoid overwriting config change at runtime.
#
# If instead you are interested in using includes to override configuration
# options, it is better to use include as the last line.
Comment From: oranagra
CONFIG REWRITE doesn't really play well with include files. for most configs, it'll just add duplicate lines, and the last one simply overrides the earlier ones (as noted by the above quote), but for ACL we recently traded that with an error, see #9330.
since the rewrite code doesn't have any visibility of the include files (and will certainly not modify it), i think we should just argue that ACL should be managed by an aclfile, and if not then at least the plain config file, and mixing it in include files and config rewrite is unsupported.
Comment From: marekhanzlik
Thanks for the aclfile tip, i will use this as a workaround.
Regarding the REWRITE and include files: Then this documentation line does not make much sense
The CONFIG REWRITE command rewrites the redis.conf file the server was started with,
applying the minimal changes needed to make it reflect the configuration currently used by the server
It implies that rewrite will write out only changes that are not already in redis.conf (and in this case even included files, cause it's not clear how is include handled)
Comment From: oranagra
i'm not sure what you mean. feel free to make a PR to suggest a better text.
Comment From: kbcmdba
I see this was closed above, but I'm looking for a version this is fixed in...
Comment From: oranagra
@kbcmdba we didn't fix anything, config rewrite doesn't work well with include, for ACL you have a better option which is to use the aclfile config.
if your problem wasn't with ACL, maybe add some more details..
the conclusions was that maybe the docs can be improve to warn users, feel free to make a PR where you think that can be effective.
Comment From: kbcmdba
IMNSHO, rewrite shouldn't cause configuration problems that would prevent Redis from running in the future. To me, that's a bug. As such, I believe this should be reopened.
Comment From: oranagra
i agree that in a perfect world it shouldn't (cause configuration problems), but the fact is that it's complicated and not a priority.
maybe we should remove the include feature instead, then we won't have that problem anymore :smirk:
the way i see it, the include feature is meant to be used in certain types of use cases, and the rewrite is for others, and they should not be mixed.
that said, if someone submits a PR to solves the problem, we'll certainly consider merging it.
Comment From: enjoy-binbin
maybe we can consider this? https://github.com/redis/redis/issues/12010#issuecomment-1501308896