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

Add support for ACL commands #2838

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Conversation

atakavci
Copy link
Contributor

@atakavci atakavci commented Jan 14, 2025

This brings the support for REDIS ACL (short for Access Control List) set of commands which are long time existing on Redis (available since 6.0.0).
The list of commands included in the PR;

  • ACL CAT
  • ACL DELUSER
  • ACL GENPASS
  • ACL GETUSER
  • ACL LIST
  • ACL LOAD
  • ACL LOG
  • ACL SAVE
  • ACL SETUSER
  • ACL USERS
  • ACL WHOAMI

More details and doc;
👉 https://redis.io/docs/latest/commands/acl/
👉 https://redis.io/docs/latest/operate/oss_and_stack/management/security/acl/

@atakavci atakavci marked this pull request as draft January 14, 2025 11:08
@atakavci atakavci marked this pull request as ready for review February 3, 2025 14:16
@atakavci
Copy link
Contributor Author

atakavci commented Feb 4, 2025

hi @mgravell , @NickCraver
i believe this brings some good level of support for ACL commands. Only ACL DRYRUN is missing in the PR, and i plan to leave it since it has very limited use case.
please let me know what you think..

@mgravell
Copy link
Collaborator

mgravell commented Mar 5, 2025

We discussed and reviewed this in our regular meetings; after much discussion, we reached some thoughts:

  • this is a large PR, that contributes a large API surface into the core, for a relatively niche admin operation (rather than routine runtime)
  • the API surface may need to evolve over time, which is very difficult in the core
  • we think this would work better as a bolt-on additional package, rather than expanding the core (name TBD)
  • we also think this might serve as an ideal test-case for the upcoming new modules / custom commands API, rather than forcing everything to use blind Execute

Proposal: when the pre-reqs are in place, @mgravell to work with @atakavci to rework this (much appreciated) effort into that shape; this could sit as a Redis Ltd owned package, or we could absorb it into a satellite package here, with looser versioning rules than the core - TBD.

Thoughts?

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