ACL RFC for Search module #18
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Allen Samuels <[email protected]>
Adding gaps relative to RediSearch section Signed-off-by: yairgott <[email protected]>
Fixing heading of the 'Unsupported knobs and control' section Signed-off-by: yairgott <[email protected]>
removal of links to redis.io Signed-off-by: yairgott <[email protected]>
Signed-off-by: Allen Samuels <[email protected]>
yairgott
suggested changes
Jan 24, 2025
PrefixACL.md
Outdated
| VALKEYMODULE_PREFIX_CHECK_READ_ACCESS = 1, | ||
| } | ||
|
|
||
| int ValkeyModule_AclCheckKeyPrefixPermissions(ValkeyModuleContext *ctx, const char *prefix, size_t bytes, size_t flags) |
There was a problem hiding this comment.
- using ValkeyModuleString* data type eliminates the need for bytes. Also, it seems more aligned with ValkeyModule_ACLCheckKeyPermissions.
- We need to support multiple prefixes and with this interface the caller needs to loop call the this API. Consider extending the API to support variadic arguments where each argument must be a ValkeyModuleString*. .e.g.: int ValkeyModule_AclCheckKeyPrefixPermissions(ValkeyModuleContext *ctx, size_t flags, ...)
There was a problem hiding this comment.
Agree that passing in multiple ValkeyModuleStrings so that there need be only a single call is a good idea. A variadic function is difficult to use, rather i'll propose an array with count.
| @@ -0,0 +1,518 @@ | |||
| ## RFC: 8 | |||
Signed-off-by: Allen Samuels <[email protected]>
yairgott
reviewed
Jan 27, 2025
There was a problem hiding this comment.
should this reside under valkey-search/rfc/ ?
There was a problem hiding this comment.
Since this is essentially a proposal to augment the Module API, I assumed that the core team would be more interested in it. (IMO, even though the module call fallback method is fully functional, I'm skeptical that it's really a production solution due to performance concerns).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As requested, an RFC to describe the security model for the search module.
This also proposes a new module API to make the security model enforcement efficient.