Unverified Commit eef9c6b0 authored by Oran Agra's avatar Oran Agra Committed by GitHub
Browse files

New detailed key-spec flags (RO, RW, OW, RM, ACCESS, UPDATE, INSERT, DELETE) (#10122)

The new ACL key based permissions in #9974 require the key-specs (#8324) to have more
explicit flags rather than just READ and WRITE. See discussion in #10040

This PR defines two groups of flags:
One about how redis internally handles the key (mutually-exclusive).
The other is about the logical operation done from the user's point of view (3 mutually exclusive
write flags, and one read flag, all optional).
In both groups, if we can't explicitly flag something as explicit read-only, delete-only, or
insert-only, we flag it as `RW` or `UPDATE`.
here's the definition from the code:
```
/* Key-spec flags *
 * -------------- */
/* The following refer what the command actually does with the value or metadata
 * of the key, and not necessarily the user data or how it affects it.
 * Each key-spec may must have exaclty one of these. Any operation that's not
 * distinctly deletion, overwrite or read-only would be marked as RW. */
#define CMD_KEY_RO (1ULL<<0)     /* Read-Only - Reads the value of the key, but
                                  * doesn't necessarily returns it. */
#define CMD_KEY_RW (1ULL<<1)     /* Read-Write - Modifies the data stored in the
                                  * value of the key or its metadata. */
#define CMD_KEY_OW (1ULL<<2)     /* Overwrite - Overwrites the data stored in
                                  * the value of the key. */
#define CMD_KEY_RM (1ULL<<3)     /* Deletes the key. */
/* The follwing refer to user data inside the value of the key, not the metadata
 * like LRU, type, cardinality. It refers to the logical operation on the user's
 * data (actual input strings / TTL), being used / returned / copied / changed,
 * It doesn't refer to modification or returning of metadata (like type, count,
 * presence of data). Any write that's not INSERT or DELETE, would be an UPADTE.
 * Each key-spec may have one of the writes with or without access, or none: */
#define CMD_KEY_ACCESS (1ULL<<4) /* Returns, copies or uses the user data from
                                  * the value of the key. */
#define CMD_KEY_UPDATE (1ULL<<5) /* Updates data to the value, new value may
                                  * depend on the old value. */
#define CMD_KEY_INSERT (1ULL<<6) /* Adds data to the value with no chance of,
                                  * modification or deletion of existing data. */
#define CMD_KEY_DELETE (1ULL<<7) /* Explicitly deletes some content
                                  * from the value of the key. */
```

Unrelated changes:
- generate-command-code.py is only compatible with python3 (modified the shabang)
- generate-command-code.py print file on json parsing error
- rename `shard_channel` key-spec flag to just `channel`.
- add INCOMPLETE flag in input spec of SORT and SORT_RO
parent d697daa7
......@@ -16,7 +16,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -22,7 +22,8 @@
"key_specs": [
{
"flags": [
"WRITE"
"RW",
"DELETE"
],
"begin_search": {
"index": {
......
......@@ -15,7 +15,8 @@
"key_specs": [
{
"flags": [
"WRITE"
"RW",
"DELETE"
],
"begin_search": {
"index": {
......
......@@ -15,7 +15,8 @@
"key_specs": [
{
"flags": [
"WRITE"
"RW",
"DELETE"
],
"begin_search": {
"index": {
......
......@@ -15,7 +15,8 @@
"key_specs": [
{
"flags": [
"WRITE"
"RW",
"DELETE"
],
"begin_search": {
"index": {
......
......@@ -20,7 +20,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -20,7 +20,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -26,7 +26,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -16,7 +16,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -16,7 +16,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -16,7 +16,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -16,7 +16,8 @@
"key_specs": [
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -17,7 +17,8 @@
"key_specs": [
{
"flags": [
"WRITE"
"OW",
"UPDATE"
],
"begin_search": {
"index": {
......@@ -34,7 +35,8 @@
},
{
"flags": [
"READ"
"RO",
"ACCESS"
],
"begin_search": {
"index": {
......
......@@ -838,9 +838,15 @@ int64_t commandKeySpecsFlagsFromString(const char *s) {
sds *tokens = sdssplitlen(s,strlen(s)," ",1,&count);
for (j = 0; j < count; j++) {
char *t = tokens[j];
if (!strcasecmp(t,"write")) flags |= CMD_KEY_WRITE;
else if (!strcasecmp(t,"read")) flags |= CMD_KEY_READ;
else if (!strcasecmp(t,"shard_channel")) flags |= CMD_KEY_SHARD_CHANNEL;
if (!strcasecmp(t,"RO")) flags |= CMD_KEY_RO;
else if (!strcasecmp(t,"RW")) flags |= CMD_KEY_RW;
else if (!strcasecmp(t,"OW")) flags |= CMD_KEY_OW;
else if (!strcasecmp(t,"RM")) flags |= CMD_KEY_RM;
else if (!strcasecmp(t,"ACCESS")) flags |= CMD_KEY_ACCESS;
else if (!strcasecmp(t,"INSERT")) flags |= CMD_KEY_INSERT;
else if (!strcasecmp(t,"UPDATE")) flags |= CMD_KEY_UPDATE;
else if (!strcasecmp(t,"DELETE")) flags |= CMD_KEY_DELETE;
else if (!strcasecmp(t,"channel")) flags |= CMD_KEY_CHANNEL;
else if (!strcasecmp(t,"incomplete")) flags |= CMD_KEY_INCOMPLETE;
else break;
}
......
......@@ -4128,9 +4128,15 @@ void addReplyDocFlagsForCommand(client *c, struct redisCommand *cmd) {
void addReplyFlagsForKeyArgs(client *c, uint64_t flags) {
replyFlagNames docFlagNames[] = {
{CMD_KEY_WRITE, "write"},
{CMD_KEY_READ, "read"},
{CMD_KEY_SHARD_CHANNEL, "shard_channel"},
{CMD_KEY_RO, "RO"},
{CMD_KEY_RW, "RW"},
{CMD_KEY_OW, "OW"},
{CMD_KEY_RM, "RM"},
{CMD_KEY_ACCESS, "access"},
{CMD_KEY_UPDATE, "update"},
{CMD_KEY_INSERT, "insert"},
{CMD_KEY_DELETE, "delete"},
{CMD_KEY_CHANNEL, "channel"},
{CMD_KEY_INCOMPLETE, "incomplete"},
{0,NULL}
};
......
......@@ -233,14 +233,37 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
#define ACL_CATEGORY_TRANSACTION (1ULL<<19)
#define ACL_CATEGORY_SCRIPTING (1ULL<<20)
/* Key argument flags. Please check the command table defined in the server.c file
* for more information about the meaning of every flag. */
#define CMD_KEY_WRITE (1ULL<<0) /* "write" flag */
#define CMD_KEY_READ (1ULL<<1) /* "read" flag */
#define CMD_KEY_SHARD_CHANNEL (1ULL<<2) /* "shard_channel" flag */
#define CMD_KEY_INCOMPLETE (1ULL<<3) /* "incomplete" flag (meaning that
* the keyspec might not point out
* to all keys it should cover) */
/* Key-spec flags *
* -------------- */
/* The following refer what the command actually does with the value or metadata
* of the key, and not necessarily the user data or how it affects it.
* Each key-spec may must have exactly one of these. Any operation that's not
* distinctly deletion, overwrite or read-only would be marked as RW. */
#define CMD_KEY_RO (1ULL<<0) /* Read-Only - Reads the value of the key, but
* doesn't necessarily returns it. */
#define CMD_KEY_RW (1ULL<<1) /* Read-Write - Modifies the data stored in the
* value of the key or its metadata. */
#define CMD_KEY_OW (1ULL<<2) /* Overwrite - Overwrites the data stored in
* the value of the key. */
#define CMD_KEY_RM (1ULL<<3) /* Deletes the key. */
/* The following refer to user data inside the value of the key, not the metadata
* like LRU, type, cardinality. It refers to the logical operation on the user's
* data (actual input strings / TTL), being used / returned / copied / changed,
* It doesn't refer to modification or returning of metadata (like type, count,
* presence of data). Any write that's not INSERT or DELETE, would be an UPDATE.
* Each key-spec may have one of the writes with or without access, or none: */
#define CMD_KEY_ACCESS (1ULL<<4) /* Returns, copies or uses the user data from
* the value of the key. */
#define CMD_KEY_UPDATE (1ULL<<5) /* Updates data to the value, new value may
* depend on the old value. */
#define CMD_KEY_INSERT (1ULL<<6) /* Adds data to the value with no chance of
* modification or deletion of existing data. */
#define CMD_KEY_DELETE (1ULL<<7) /* Explicitly deletes some content
* from the value of the key. */
/* Other flags: */
#define CMD_KEY_CHANNEL (1ULL<<8) /* PUBSUB shard channel */
#define CMD_KEY_INCOMPLETE (1ULL<<9) /* Means that the keyspec might not point
* out to all keys it should cover */
/* AOF states */
#define AOF_OFF 0 /* AOF is off */
......
......@@ -37,14 +37,14 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR;
RedisModuleCommand *legacy = RedisModule_GetCommand(ctx,"kspec.legacy");
if (RedisModule_AddCommandKeySpec(legacy,"read",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(legacy,"RO ACCESS",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(legacy,spec_id,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(legacy,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(legacy,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(legacy,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(legacy,spec_id,2) == REDISMODULE_ERR)
return REDISMODULE_ERR;
......@@ -56,14 +56,14 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR;
RedisModuleCommand *complex1 = RedisModule_GetCommand(ctx,"kspec.complex1");
if (RedisModule_AddCommandKeySpec(complex1,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex1,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchKeyword(complex1,spec_id,"STORE",2) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(complex1,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(complex1,"read",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex1,"RO ACCESS",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchKeyword(complex1,spec_id,"KEYS",2) == REDISMODULE_ERR)
return REDISMODULE_ERR;
......@@ -75,35 +75,35 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR;
RedisModuleCommand *complex2 = RedisModule_GetCommand(ctx,"kspec.complex2");
if (RedisModule_AddCommandKeySpec(complex2,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex2,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchKeyword(complex2,spec_id,"STORE",5) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(complex2,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(complex2,"read",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex2,"RO ACCESS",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(complex2,spec_id,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(complex2,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(complex2,"read",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex2,"RO ACCESS",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(complex2,spec_id,2) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysRange(complex2,spec_id,0,1,0) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(complex2,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex2,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(complex2,spec_id,3) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecFindKeysKeynum(complex2,spec_id,0,1,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_AddCommandKeySpec(complex2,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(complex2,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchKeyword(complex2,spec_id,"MOREKEYS",5) == REDISMODULE_ERR)
return REDISMODULE_ERR;
......
......@@ -33,7 +33,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR;
RedisModuleCommand *subcmd = RedisModule_GetCommand(ctx,"subcommands.bitarray|set");
if (RedisModule_AddCommandKeySpec(subcmd,"write",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(subcmd,"RW UPDATE",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(subcmd,spec_id,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
......@@ -44,7 +44,7 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
return REDISMODULE_ERR;
subcmd = RedisModule_GetCommand(ctx,"subcommands.bitarray|get");
if (RedisModule_AddCommandKeySpec(subcmd,"read",&spec_id) == REDISMODULE_ERR)
if (RedisModule_AddCommandKeySpec(subcmd,"RO ACCESS",&spec_id) == REDISMODULE_ERR)
return REDISMODULE_ERR;
if (RedisModule_SetCommandKeySpecBeginSearchIndex(subcmd,spec_id,1) == REDISMODULE_ERR)
return REDISMODULE_ERR;
......
......@@ -11,8 +11,8 @@ start_server {tags {"modules"}} {
assert_equal [lindex $reply 5] 1
# Verify key-specs
set keyspecs [lindex $reply 8]
assert_equal [lindex $keyspecs 0] {flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 1] {flags write begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
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}}}
}
test "Module key specs: Complex specs, case 1" {
......@@ -24,8 +24,8 @@ start_server {tags {"modules"}} {
# Verify key-specs
set keyspecs [lindex $reply 8]
assert_equal [lindex $keyspecs 0] {flags {} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 1] {flags write 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 read begin_search {type keyword spec {keyword KEYS startfrom 2}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}
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}}}
}
test "Module key specs: Complex specs, case 2" {
......@@ -36,11 +36,11 @@ start_server {tags {"modules"}} {
assert_equal [lindex $reply 5] 1
# Verify key-specs
set keyspecs [lindex $reply 8]
assert_equal [lindex $keyspecs 0] {flags write begin_search {type keyword spec {keyword STORE startfrom 5}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 1] {flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 2] {flags read begin_search {type index spec {index 2}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 3] {flags write begin_search {type index spec {index 3}} find_keys {type keynum spec {keynumidx 0 firstkey 1 keystep 1}}}
assert_equal [lindex $keyspecs 4] {flags write begin_search {type keyword spec {keyword MOREKEYS startfrom 5}} find_keys {type range spec {lastkey -1 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 0] {flags {RW update} begin_search {type keyword spec {keyword STORE startfrom 5}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}
assert_equal [lindex $keyspecs 1] {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 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}}}
}
test "Module command list filtering" {
......
......@@ -8,8 +8,8 @@ start_server {tags {"modules"}} {
set command_reply [r command info subcommands.bitarray]
set first_cmd [lindex $command_reply 0]
set subcmds_in_command [lsort [lindex $first_cmd 9]]
assert_equal [lindex $subcmds_in_command 0] {subcommands.bitarray|get -2 module 1 1 1 {} {} {{flags read begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}
assert_equal [lindex $subcmds_in_command 1] {subcommands.bitarray|set -2 module 1 1 1 {} {} {{flags write begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}
assert_equal [lindex $subcmds_in_command 0] {subcommands.bitarray|get -2 module 1 1 1 {} {} {{flags {RO access} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}
assert_equal [lindex $subcmds_in_command 1] {subcommands.bitarray|set -2 module 1 1 1 {} {} {{flags {RW update} begin_search {type index spec {index 1}} find_keys {type range spec {lastkey 0 keystep 1 limit 0}}}} {}}
# Verify that module subcommands are displayed correctly in COMMAND DOCS
set docs_reply [r command docs subcommands.bitarray]
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment