search

valkey-io / valkeymodule-rs

Rust valkey SDK for modules
BSD 3-Clause "New" or "Revised" License
32 stars 10 forks source link

Adding capabilty for creating custom ACL categories as well as setting custom and existing ACL categories #120

Closed zackcam closed 2 weeks ago

zackcam commented 4 weeks ago

Overview

This pr is in relation to issue #96.

This is designed to allow the capability of creating and setting custom ACL categories. This also allows setting existing ACL categories to commands. This is not a breaking change as all fields that have been created in the macros are optional which means that developers will only need to add the field at the end of the commands if they want to add acl categories and can leave it as it was if they don't.

Testing

Tested by amending ACL example. This involved creating three new commands with the sole purpose of assigning ACL categories to them. In the setup we define two new custom ACL categories called custom_acl_one and custom_acl_two. The next important part is that in the command definition in the valkey_module macro we assign the desired ACL's to the commands. We test that these ACL's are assigned correctly by adding a new test in integration.rs where we get the list of commands associated with the ACL's and determine if the new commands show up correctly.

KarthikSubbarao commented 3 weeks ago

Looks like the DCO check needs to be handled with the right signoff

Regarding the min compatibility, it could be good to create an issue where we document a better approach in general within the SDK for specifying min compat on functions / macros. Right now, looks like we need to specify multiple versions. This should be simplified to only require the min compatibile version and not the versions afterwards which still support it

dmitrypol commented 3 weeks ago

@zackcam - can you add required-features = ["min-redis-compatibility-version-7-2"] to Cargo.toml for call example?

zackcam commented 3 weeks ago

@zackcam - can you add required-features = ["min-redis-compatibility-version-7-2"] to Cargo.toml for call example? Since

Updated so we now have both 8-0 and 7-2 in the feature flags as a temporary workaround while the feature flags dont take into account version below them

dmitrypol commented 2 weeks ago

Let's add min-valkey-compatibility-version-8-0 in not(any above pub fn register_commands( on line 495 in commands.rs.

Also could you please run cargo fmt --all to format it all nicely.

zackcam commented 2 weeks ago

Latest push: ran cargo fmt --all and added min-valkey-compatibility-version-8-0 for pub fn register_commands(