Redis redis-cli cluster reshard and ACL (AUTH2)

The resharding redis-cli code uses the MIGRATE command, which does not implement the AUTH2 mode, required when ACL are on on the server and the default user isn't 'migrate' capable. I believe that this is incorrect.

Comment From: bsergean

I believe I have a fix for it, will send a PR soon. Here is the error I'm getting without my fix.

Node 127.0.0.1:11002 replied with error:
ERR Target instance replied with error: ERR AUTH <password> called without any password configured for the default user. Are you sure your configuration is correct?

Comment From: bsergean

Here is the fix, tested with an old school password and an ACL password. I'm trying hard to make a PR but git tells me to go away with a 403 error.

--- a/src/redis-cli.c
+++ b/src/redis-cli.c
@@ -3416,6 +3416,7 @@ static redisReply *clusterManagerMigrateKeysInReply(clusterManagerNode *source,
     size_t *argv_len = NULL;
     int c = (replace ? 8 : 7);
     if (config.auth) c += 2;
+    if (config.user) c += 1;
     size_t argc = c + reply->elements;
     size_t i, offset = 6; // Keys Offset
     argv = zcalloc(argc * sizeof(char *));
@@ -3442,12 +3443,24 @@ static redisReply *clusterManagerMigrateKeysInReply(clusterManagerNode *source,
         offset++;
     }
     if (config.auth) {
-        argv[offset] = "AUTH";
-        argv_len[offset] = 4;
-        offset++;
-        argv[offset] = config.auth;
-        argv_len[offset] = strlen(config.auth);
-        offset++;
+        if (config.user) {
+            argv[offset] = "AUTH2";
+            argv_len[offset] = 5;
+            offset++;
+            argv[offset] = config.user;
+            argv_len[offset] = strlen(config.user);
+            offset++;
+            argv[offset] = config.auth;
+            argv_len[offset] = strlen(config.auth);
+            offset++;
+        } else {
+            argv[offset] = "AUTH";
+            argv_len[offset] = 4;
+            offset++;
+            argv[offset] = config.auth;
+            argv_len[offset] = strlen(config.auth);
+            offset++;
+        }
     }
     argv[offset] = "KEYS";
     argv_len[offset] = 4;

Comment From: bsergean

I had this in my configuration file to disable the default user.

user default off nopass ~* +@all
user bob on >robert ~* +@all
masteruser bob
masterauth robert

Comment From: bsergean

@antirez / do you agree that there's something fishy here ?

I briefly looked at the unittest to try to see how easy it would be to add a new unittest but I got lazy. I have tested the fix in the associated PR by hacking redis-cli reshard mode to only migrate one hashslot from one cluster node to a second one, for a key that had been created on that hashslot (if no key is created, there is no bug when resharding).

I've been wondering whether, as a new feature, there there could be a new cluster migrate sub-command (or maybe different name "move-slot", or option for reshard), that only move one hashslot from one cluster node to another one. Was it intentional that this wasn't provided ? I have a resharding tool that does that manually, but maybe it could also support doing it by shelling out to redis-cli.

Comment From: bsergean

ps: I noticed that redis-benchmark is missing a --user option. Probably good to add if someone wants to benchmark an ACL protected redis instance.

Comment From: bsergean

Just made #7197 to the redis-benchmark thing.