Keep it mutable, add valkey.conf to explain the harm of modifying it

Signed-off-by: Binbin <[email protected]>
enjoy-binbin · Dec 6, 2024 · d90000d · d90000d
1 parent 6ed2540
commit d90000d
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 7 deletions.
diff --git a/src/config.c b/src/config.c
@@ -3385,7 +3385,7 @@ standardConfig static_configs[] = {
     createStringConfig("tls-ciphersuites", NULL, MODIFIABLE_CONFIG, EMPTY_STRING_IS_NULL, server.tls_ctx_config.ciphersuites, NULL, NULL, applyTlsCfg),
 
     /* Special configs */
-    createSpecialConfig("dir", NULL, IMMUTABLE_CONFIG, setConfigDirOption, getConfigDirOption, rewriteConfigDirOption, NULL),
+    createSpecialConfig("dir", NULL, MODIFIABLE_CONFIG | PROTECTED_CONFIG | DENY_LOADING_CONFIG, setConfigDirOption, getConfigDirOption, rewriteConfigDirOption, NULL),
     createSpecialConfig("save", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigSaveOption, getConfigSaveOption, rewriteConfigSaveOption, NULL),
     createSpecialConfig("client-output-buffer-limit", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigClientOutputBufferLimitOption, getConfigClientOutputBufferLimitOption, rewriteConfigClientOutputBufferLimitOption, NULL),
     createSpecialConfig("oom-score-adj-values", NULL, MODIFIABLE_CONFIG | MULTI_ARG_CONFIG, setConfigOOMScoreAdjValuesOption, getConfigOOMScoreAdjValuesOption, rewriteConfigOOMScoreAdjValuesOption, updateOOMScoreAdj),

diff --git a/tests/assets/default.conf b/tests/assets/default.conf
@@ -19,8 +19,6 @@ save ''
 
 rdbcompression yes
 dbfilename dump.rdb
-# Make sure it works to set --dir multiple times, some guy was doing this.
-dir ./
 dir ./
 
 slave-serve-stale-data yes

diff --git a/tests/unit/introspection.tcl b/tests/unit/introspection.tcl
@@ -793,7 +793,6 @@ start_server {tags {"introspection"}} {
 
     test {CONFIG SET set immutable} {
         assert_error "ERR *immutable*" {r config set daemonize yes}
-        assert_error "ERR *immutable*" {r config set dir "./"}
     }
 
     test {CONFIG GET hidden configs} {
@@ -994,7 +993,7 @@ start_server {tags {"introspection"}} {
 
 start_server {tags {"introspection external:skip"} overrides {enable-protected-configs {no} enable-debug-command {no}}} {
     test {cannot modify protected configuration - no} {
-        assert_error "ERR *protected*" {r config set dbfilename somedbfilename.rdb}
+        assert_error "ERR *protected*" {r config set dir somedir}
         assert_error "ERR *DEBUG command not allowed*" {r DEBUG HELP}
     } {} {needs:debug}
 }
@@ -1010,7 +1009,7 @@ start_server {config "minimal.conf" tags {"introspection external:skip"} overrid
         if {$myaddr != "" && ![string match {127.*} $myaddr]} {
             # Non-loopback client should fail
             set r2 [get_nonloopback_client]
-            assert_error "ERR *protected*" {$r2 config set dbfilename somedbfilename.rdb}
+            assert_error "ERR *protected*" {$r2 config set dir somedir}
             assert_error "ERR *DEBUG command not allowed*" {$r2 DEBUG HELP}
         }
     } {} {needs:debug}
@@ -1033,7 +1032,7 @@ test {config during loading} {
         assert_equal [lindex [r config get loglevel] 1] debug
 
         # verify some configs are forbidden during loading
-        assert_error {LOADING*} {r config set appendonly yes}
+        assert_error {LOADING*} {r config set dir asdf}
 
         # make sure it's still loading
         assert_equal [s loading] 1

diff --git a/valkey.conf b/valkey.conf
@@ -582,6 +582,9 @@ rdb-del-sync-files no
 
 # The working directory.
 #
+# The server log is written relative this directory, if the 'logfile'
+# configuration directive is a relative path.
+#
 # The DB will be written inside this directory, with the filename specified
 # above using the 'dbfilename' configuration directive.
 #
@@ -591,6 +594,9 @@ rdb-del-sync-files no
 # 'cluster-config-file' configuration directive is a relative path.
 #
 # Note that you must specify a directory here, not a file name.
+# Note that modifying 'dir' during runtime may have unexpected behavior,
+# for example when a child process is running, related file operations may
+# have unexpected effects.
 dir ./
 
 ################################# REPLICATION #################################