Skip to content

Support internal commands#56

Merged
MeirShpilraien merged 4 commits intomasterfrom
use_internal_commands
Feb 5, 2025
Merged

Support internal commands#56
MeirShpilraien merged 4 commits intomasterfrom
use_internal_commands

Conversation

@MeirShpilraien
Copy link

@MeirShpilraien MeirShpilraien commented Jan 22, 2025

The PR makes changes to use the internal command capabilities introduced on: redis/redis#13740.
The main idea is to use the new redismodule API that provides internal secret for internal shard communication.

This means that the following is no longer needed:

  1. The new user configuration.
  2. The internal ACL category

All the commands are marked with the internal flag (if we see that the internal commands feature is supported on the current Redis version we run on). In addition, the connection between the shards uses INTERNALAUTH with the given internal secret so it can run all the internal commands. Regular user can not run any internal command.

The PR require the following PR's to be merge:

  1. Add internal connection and command mechanism redis/redis#13740
  2. Added RM_GetInternalSecret to redismodule.h RedisLabsModules/redismodule-rs#407

The tests was modified to avoid giving password to the test module in case it detects that the internal commands feature exists. In such case, avoiding giving the password to the module, make sure that LibMR actually using the INTERNALAUTH command with the internal secret to authenticate.

todo:

@MeirShpilraien MeirShpilraien requested a review from iddm January 22, 2025 19:57
@MeirShpilraien
Copy link
Author

unstable CI is expected to pass once the relevant PR's will be merge.

Copy link
Contributor

@ephraimfeldblum ephraimfeldblum left a comment

Choose a reason for hiding this comment

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

LGTM

@MeirShpilraien MeirShpilraien merged commit 3506693 into master Feb 5, 2025
7 checks passed
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