Handle key-spec flags with modules (#10237)

- add COMMAND GETKEYSANDFLAGS sub-command
- add RM_KeyAtPosWithFlags and GetCommandKeysWithFlags
- RM_KeyAtPos and RM_CreateCommand set flags requiring full access for keys
- RM_CreateCommand set VARIABLE_FLAGS
- expose `variable_flags` flag in COMMAND INFO key-specs
- getKeysFromCommandWithSpecs prefers key-specs over getkeys-api
- add tests for all of these

tests/unit/moduleapi/getkeys.tcl

@@ -16,32 +16,64 @@ start_server {tags {"modules"}} {
         r command getkeys getkeys.command arg1 arg2 key key1 arg3 key key2 key key3
     } {key1 key2 key3}
 
+    test {COMMAND GETKEYS correctly reports a movable keys module command using flags} {
+        r command getkeys getkeys.command_with_flags arg1 arg2 key key1 arg3 key key2 key key3
+    } {key1 key2 key3}
+
+    test {COMMAND GETKEYSANDFLAGS correctly reports a movable keys module command not using flags} {
+        r command getkeysandflags getkeys.command arg1 arg2 key key1 arg3 key key2
+    } {{key1 {RW access update}} {key2 {RW access update}}}
+
+    test {COMMAND GETKEYSANDFLAGS correctly reports a movable keys module command using flags} {
+        r command getkeysandflags getkeys.command_with_flags arg1 arg2 key key1 arg3 key key2 key key3
+    } {{key1 {RO access}} {key2 {RO access}} {key3 {RO access}}}
+
     test {RM_GetCommandKeys on non-existing command} {
-        catch {r getkeys.introspect non-command key1 key2} e
+        catch {r getkeys.introspect 0 non-command key1 key2} e
         set _ $e
     } {*ENOENT*}
 
     test {RM_GetCommandKeys on built-in fixed keys command} {
-        r getkeys.introspect set key1 value1
+        r getkeys.introspect 0 set key1 value1
     } {key1}
 
+    test {RM_GetCommandKeys on built-in fixed keys command with flags} {
+        r getkeys.introspect 1 set key1 value1
+    } {{key1 OW}}
+
     test {RM_GetCommandKeys on EVAL} {
-        r getkeys.introspect eval "" 4 key1 key2 key3 key4 arg1 arg2
+        r getkeys.introspect 0 eval "" 4 key1 key2 key3 key4 arg1 arg2
     } {key1 key2 key3 key4}
 
     test {RM_GetCommandKeys on a movable keys module command} {
-        r getkeys.introspect getkeys.command arg1 arg2 key key1 arg3 key key2 key key3
+        r getkeys.introspect 0 getkeys.command arg1 arg2 key key1 arg3 key key2 key key3
     } {key1 key2 key3}
 
     test {RM_GetCommandKeys on a non-movable module command} {
-        r getkeys.introspect getkeys.fixed arg1 key1 key2 key3 arg2
+        r getkeys.introspect 0 getkeys.fixed arg1 key1 key2 key3 arg2
     } {key1 key2 key3}
 
     test {RM_GetCommandKeys with bad arity} {
-        catch {r getkeys.introspect set key} e
+        catch {r getkeys.introspect 0 set key} e
         set _ $e
     } {*EINVAL*}
 
+    # user that can only read from "read" keys, write to "write" keys, and read+write to "RW" keys
+    r ACL setuser testuser +@all %R~read* %W~write* %RW~rw*
+
+    test "module getkeys-api - ACL" {
+        # legacy triple didn't provide flags, so they require both read and write
+        assert_equal "OK" [r ACL DRYRUN testuser getkeys.command key rw]
+        assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser getkeys.command key read]
+        assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser getkeys.command key write]
+    }
+
+    test "module getkeys-api with flags - ACL" {
+        assert_equal "OK" [r ACL DRYRUN testuser getkeys.command_with_flags key rw]
+        assert_equal "OK" [r ACL DRYRUN testuser getkeys.command_with_flags key read]
+        assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser getkeys.command_with_flags key write]
+    }
+
     test "Unload the module - getkeys" {
         assert_equal {OK} [r module unload getkeys]
     }

tests/unit/moduleapi/keyspecs.tcl

@@ -8,12 +8,13 @@ start_server {tags {"modules"}} {
         # Verify (first, last, step) and not movablekeys
         assert_equal [lindex $reply 2] {module}
         assert_equal [lindex $reply 3] 1
-        assert_equal [lindex $reply 4] 3
+        assert_equal [lindex $reply 4] -1
         assert_equal [lindex $reply 5] 2
         # Verify key-spec auto-generated from the legacy triple
         set keyspecs [lindex $reply 8]
         assert_equal [llength $keyspecs] 1
-        assert_equal [lindex $keyspecs 0] {flags {} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 2 keystep 2 limit 0}}}
+        assert_equal [lindex $keyspecs 0] {flags {RW access update variable_flags} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey -1 keystep 2 limit 0}}}
+        assert_equal [r command getkeys kspec.none key1 val1 key2 val2] {key1 key2}
     }
 
     test "Module key specs: Two ranges" {
@@ -27,6 +28,7 @@ start_server {tags {"modules"}} {
         set keyspecs [lindex $reply 8]
         assert_equal [lindex $keyspecs 0] {flags {RO access} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
         assert_equal [lindex $keyspecs 1] {flags {RW update} begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
+        assert_equal [r command getkeys kspec.tworanges foo bar baz quux] {foo bar}
     }
 
     test "Module key specs: Keyword-only spec clears the legacy triple" {
@@ -39,6 +41,7 @@ start_server {tags {"modules"}} {
         # Verify key-specs
         set keyspecs [lindex $reply 8]
         assert_equal [lindex $keyspecs 0] {flags {RO access} begin_search {type keyword spec {keyword KEYS startfrom 1}} find_keys {type range spec {lastkey -1 keystep 1 limit 0}}}
+        assert_equal [r command getkeys kspec.keyword foo KEYS bar baz] {bar baz}
     }
 
     test "Module key specs: Complex specs, case 1" {
@@ -53,6 +56,7 @@ start_server {tags {"modules"}} {
         assert_equal [lindex $keyspecs 0] {flags RO begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
         assert_equal [lindex $keyspecs 1] {flags {RW update} begin_search {type keyword spec {keyword STORE startfrom 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
         assert_equal [lindex $keyspecs 2] {flags {RO access} begin_search {type keyword spec {keyword KEYS startfrom 2}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}
+        assert_equal [r command getkeys kspec.complex1 foo dummy KEYS 1 bar baz STORE quux] {foo quux bar}
     }
 
     test "Module key specs: Complex specs, case 2" {
@@ -69,12 +73,39 @@ start_server {tags {"modules"}} {
         assert_equal [lindex $keyspecs 2] {flags {RO access} begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
         assert_equal [lindex $keyspecs 3] {flags {RW update} begin_search {type index spec {index 3}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}
         assert_equal [lindex $keyspecs 4] {flags {RW update} begin_search {type keyword spec {keyword MOREKEYS startfrom 5}} find_keys {type range spec {lastkey -1 keystep 1 limit 0}}}
+        assert_equal [r command getkeys kspec.complex2 foo bar 2 baz quux banana STORE dst dummy MOREKEYS hey ho] {dst foo bar baz quux hey ho}
     }
 
     test "Module command list filtering" {
         ;# Note: we piggyback this tcl file to test the general functionality of command list filtering
         set reply [r command list filterby module keyspecs]
         assert_equal [lsort $reply] {kspec.complex1 kspec.complex2 kspec.keyword kspec.none kspec.tworanges}
+        assert_equal [r command getkeys kspec.complex2 foo bar 2 baz quux banana STORE dst dummy MOREKEYS hey ho] {dst foo bar baz quux hey ho}
+    }
+
+    test {COMMAND GETKEYSANDFLAGS correctly reports module key-spec without flags} {
+        r command getkeysandflags kspec.none key1 val1 key2 val2
+    } {{key1 {RW access update variable_flags}} {key2 {RW access update variable_flags}}}
+
+    test {COMMAND GETKEYSANDFLAGS correctly reports module key-spec flags} {
+        r command getkeysandflags kspec.keyword keys key1 key2 key3
+    } {{key1 {RO access}} {key2 {RO access}} {key3 {RO access}}}
+
+    # user that can only read from "read" keys, write to "write" keys, and read+write to "RW" keys
+    r ACL setuser testuser +@all %R~read* %W~write* %RW~rw*
+
+    test "Module key specs: No spec, only legacy triple - ACL" {
+        # legacy triple didn't provide flags, so they require both read and write
+        assert_equal "OK" [r ACL DRYRUN testuser kspec.none rw val1]
+        assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser kspec.none read val1]
+        assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser kspec.none write val1]
+    }
+
+    test "Module key specs: tworanges - ACL" {
+        assert_equal "OK" [r ACL DRYRUN testuser kspec.tworanges read write]
+        assert_equal "OK" [r ACL DRYRUN testuser kspec.tworanges rw rw]
+        assert_equal "This user has no permissions to access the 'read' key" [r ACL DRYRUN testuser kspec.tworanges rw read]
+        assert_equal "This user has no permissions to access the 'write' key" [r ACL DRYRUN testuser kspec.tworanges write rw]
     }
 
     test "Unload the module - keyspecs" {