Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: remove some manual acl categories #4088

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dranikpg
Copy link
Contributor

@dranikpg dranikpg commented Nov 8, 2024

Buids on #3411

Interestingly, redis modules don't expose any acl categories at all, not even their module πŸ€·πŸ»β€β™‚οΈ Not sure if we should do the same πŸ€”

Signed-off-by: Vladislav Oleshko <[email protected]>
@dranikpg
Copy link
Contributor Author

dranikpg commented Nov 8, 2024

  1. fixed a few fast/slow mistakes, everything is now in accordance with redis docs
  2. module commands now have properties like fast/slow, read/write - not sure if we should keep them
--- before.txt  2024-11-08 10:38:51.213266052 +0300
+++ after.txt   2024-11-08 16:21:54.789952448 +0300
@@ -260,12 +260,14 @@
     "last_key_pos": 0,
     "name": "AUTH",
     "step_count": 1
   },
   "BF.ADD": {
     "acl_categories": [
+      "@WRITE",
+      "@FAST",
       "@BLOOM"
     ],
     "arity": 3,
     "first_key_pos": 1,
     "flags": [
       "denyoom",
@@ -275,12 +277,14 @@
     "last_key_pos": 1,
     "name": "BF.ADD",
     "step_count": 1
   },
   "BF.EXISTS": {
     "acl_categories": [
+      "@READ",
+      "@FAST",
       "@BLOOM"
     ],
     "arity": 3,
     "first_key_pos": 1,
     "flags": [
       "fast",
@@ -289,12 +293,14 @@
     "last_key_pos": 1,
     "name": "BF.EXISTS",
     "step_count": 1
   },
   "BF.MADD": {
     "acl_categories": [
+      "@WRITE",
+      "@FAST",
       "@BLOOM"
     ],
     "arity": -3,
     "first_key_pos": 1,
     "flags": [
       "denyoom",
@@ -304,12 +310,14 @@
     "last_key_pos": 1,
     "name": "BF.MADD",
     "step_count": 1
   },
   "BF.MEXISTS": {
     "acl_categories": [
+      "@READ",
+      "@FAST",
       "@BLOOM"
     ],
     "arity": -3,
     "first_key_pos": 1,
     "flags": [
       "fast",
@@ -318,12 +326,14 @@
     "last_key_pos": 1,
     "name": "BF.MEXISTS",
     "step_count": 1
   },
   "BF.RESERVE": {
     "acl_categories": [
+      "@WRITE",
+      "@FAST",
       "@BLOOM"
     ],
     "arity": -4,
     "first_key_pos": 1,
     "flags": [
       "denyoom",
@@ -387,12 +397,13 @@
       "@BITMAP",
       "@FAST"
     ],
     "arity": -5,
     "first_key_pos": 1,
     "flags": [
+      "fast",
       "readonly"
     ],
     "last_key_pos": 1,
     "name": "BITFIELD_RO",
     "step_count": 1
   },
@@ -426,13 +437,13 @@
     "last_key_pos": 1,
     "name": "BITPOS",
     "step_count": 1
   },
   "BLMOVE": {
     "acl_categories": [
-      "@READ",
+      "@WRITE",
       "@LIST",
       "@SLOW",
       "@BLOCKING"
     ],
     "arity": 6,
     "first_key_pos": 1,
@@ -922,12 +933,13 @@
     "last_key_pos": 0,
     "name": "FLUSHDB",
     "step_count": 1
   },
   "FT.AGGREGATE": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -3,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -937,12 +949,14 @@
     "last_key_pos": 0,
     "name": "FT.AGGREGATE",
     "step_count": 1
   },
   "FT.ALTER": {
     "acl_categories": [
+      "@WRITE",
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -3,
     "first_key_pos": 0,
     "flags": [
       "global-trans",
@@ -951,12 +965,14 @@
     "last_key_pos": 0,
     "name": "FT.ALTER",
     "step_count": 1
   },
   "FT.CREATE": {
     "acl_categories": [
+      "@WRITE",
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -2,
     "first_key_pos": 0,
     "flags": [
       "global-trans",
@@ -965,12 +981,14 @@
     "last_key_pos": 0,
     "name": "FT.CREATE",
     "step_count": 1
   },
   "FT.DROPINDEX": {
     "acl_categories": [
+      "@WRITE",
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -2,
     "first_key_pos": 0,
     "flags": [
       "global-trans",
@@ -979,12 +997,13 @@
     "last_key_pos": 0,
     "name": "FT.DROPINDEX",
     "step_count": 1
   },
   "FT.INFO": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": 2,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -994,12 +1013,13 @@
     "last_key_pos": 0,
     "name": "FT.INFO",
     "step_count": 1
   },
   "FT.PROFILE": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -4,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -1009,12 +1029,13 @@
     "last_key_pos": 0,
     "name": "FT.PROFILE",
     "step_count": 1
   },
   "FT.SEARCH": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": -3,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -1024,12 +1045,13 @@
     "last_key_pos": 0,
     "name": "FT.SEARCH",
     "step_count": 1
   },
   "FT.TAGVALS": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": 3,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -1039,12 +1061,13 @@
     "last_key_pos": 0,
     "name": "FT.TAGVALS",
     "step_count": 1
   },
   "FT._LIST": {
     "acl_categories": [
+      "@SLOW",
       "@FT_SEARCH"
     ],
     "arity": 1,
     "first_key_pos": 0,
     "flags": [
       "custom-journal",
@@ -1235,13 +1258,12 @@
       "@STRING",
       "@SLOW"
     ],
     "arity": 4,
     "first_key_pos": 1,
     "flags": [
-      "fast",
       "readonly"
     ],
     "last_key_pos": 1,
     "name": "GETRANGE",
     "step_count": 1
   },
@@ -2034,13 +2056,13 @@
     "last_key_pos": 1,
     "name": "LINDEX",
     "step_count": 1
   },
   "LINSERT": {
     "acl_categories": [
-      "@READ",
+      "@WRITE",
       "@LIST",
       "@SLOW"
     ],
     "arity": 5,
     "first_key_pos": 1,
     "flags": [
@@ -2105,13 +2127,12 @@
       "@LIST",
       "@SLOW"
     ],
     "arity": -3,
     "first_key_pos": 1,
     "flags": [
-      "fast",
       "readonly"
     ],
     "last_key_pos": 1,
     "name": "LPOS",
     "step_count": 1
   },
@@ -2416,12 +2437,13 @@
       "@HYPERLOG",
       "@FAST"
     ],
     "arity": -3,
     "first_key_pos": 1,
     "flags": [
+      "fast",
       "write"
     ],
     "last_key_pos": 1,
     "name": "PFADD",
     "step_count": 1
   },
@@ -2770,13 +2792,12 @@
       "@SLOW"
     ],
     "arity": 3,
     "first_key_pos": 1,
     "flags": [
       "custom-journal",
-      "fast",
       "write"
     ],
     "last_key_pos": 2,
     "name": "RPOPLPUSH",
     "step_count": 1
   },
@@ -3018,12 +3039,13 @@
       "@FAST"
     ],
     "arity": 3,
     "first_key_pos": 1,
     "flags": [
       "denyoom",
+      "fast",
       "write"
     ],
     "last_key_pos": 1,
     "name": "SETNX",
     "step_count": 1
   },
@@ -3034,13 +3056,12 @@
       "@SLOW"
     ],
     "arity": 4,
     "first_key_pos": 1,
     "flags": [
       "denyoom",
-      "fast",
       "write"
     ],
     "last_key_pos": 1,
     "name": "SETRANGE",
     "step_count": 1
   },
@@ -3111,13 +3132,13 @@
     "step_count": 1
   },
   "SISMEMBER": {
     "acl_categories": [
       "@READ",
       "@SET",
-      "@SLOW"
+      "@FAST"
     ],
     "arity": 3,
     "first_key_pos": 1,
     "flags": [
       "fast",
       "readonly"
@@ -3182,12 +3203,13 @@
       "@SET",
       "@FAST"
     ],
     "arity": -3,
     "first_key_pos": 1,
     "flags": [
+      "fast",
       "readonly"
     ],
     "last_key_pos": 1,
     "name": "SMISMEMBER",
     "step_count": 1
   },
@@ -3227,13 +3249,13 @@
     "step_count": 1
   },
   "SPOP": {
     "acl_categories": [
       "@WRITE",
       "@SET",
-      "@SLOW"
+      "@FAST"
     ],
     "arity": -2,
     "first_key_pos": 1,
     "flags": [
       "custom-journal",
       "fast",
@@ -3341,13 +3363,12 @@
       "@STRING",
       "@SLOW"
     ],
     "arity": 4,
     "first_key_pos": 1,
     "flags": [
-      "fast",
       "readonly"
     ],
     "last_key_pos": 1,
     "name": "SUBSTR",
     "step_count": 1
   },

@dranikpg dranikpg marked this pull request as ready for review November 10, 2024 10:04
@romange romange requested a review from kostasrim November 11, 2024 06:25
@kostasrim
Copy link
Contributor

module commands now have properties like fast/slow, read/write - not sure if we should keep them

as long as the module commands have the same ACL's then it's fine. For example SCRIPT LOAD has @slow @scripting so @slow is fine here.

Which commands diverged ?

Copy link
Contributor

@kostasrim kostasrim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plz see my comment about module commands and if they diverged before you merged.

For scripts yes, the module itself does not have a way to report/attach ACL's to it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants