This is an automated email from the ASF dual-hosted git repository.
twice pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 3b8ef09b chore(command): add admin flag for permission-required
commands (#2747)
3b8ef09b is described below
commit 3b8ef09bfe1fe4bea2d13336ca2be6807ea60abf
Author: Twice <[email protected]>
AuthorDate: Tue Jan 28 09:11:16 2025 +0800
chore(command): add admin flag for permission-required commands (#2747)
---
src/commands/cmd_cluster.cc | 13 ++------
src/commands/cmd_server.cc | 70 ++++++++++++------------------------------
src/commands/commander.h | 22 +++++++------
src/server/redis_connection.cc | 5 +++
src/storage/scripting.cc | 5 +++
5 files changed, 45 insertions(+), 70 deletions(-)
diff --git a/src/commands/cmd_cluster.cc b/src/commands/cmd_cluster.cc
index f8e63728..7a16ddd9 100644
--- a/src/commands/cmd_cluster.cc
+++ b/src/commands/cmd_cluster.cc
@@ -70,10 +70,6 @@ class CommandCluster : public Commander {
return {Status::RedisExecErr, "Cluster mode is not enabled"};
}
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
if (subcommand_ == "keyslot") {
auto slot_id = GetSlotIdFromKey(args_[2]);
*output = redis::Integer(slot_id);
@@ -249,10 +245,6 @@ class CommandClusterX : public Commander {
return {Status::RedisExecErr, "Cluster mode is not enabled"};
}
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
bool need_persist_nodes_info = false;
if (subcommand_ == "setnodes") {
Status s = srv->cluster->SetClusterNodes(nodes_str_, set_version_,
force_);
@@ -357,8 +349,9 @@ class CommandAsking : public Commander {
}
};
-REDIS_REGISTER_COMMANDS(Cluster, MakeCmdAttr<CommandCluster>("cluster", -2,
"no-script", NO_KEY, GenerateClusterFlag),
- MakeCmdAttr<CommandClusterX>("clusterx", -2,
"no-script", NO_KEY, GenerateClusterFlag),
+REDIS_REGISTER_COMMANDS(Cluster,
+ MakeCmdAttr<CommandCluster>("cluster", -2, "no-script
admin", NO_KEY, GenerateClusterFlag),
+ MakeCmdAttr<CommandClusterX>("clusterx", -2,
"no-script admin", NO_KEY, GenerateClusterFlag),
MakeCmdAttr<CommandReadOnly>("readonly", 1,
"no-multi", NO_KEY),
MakeCmdAttr<CommandReadWrite>("readwrite", 1,
"no-multi", NO_KEY),
MakeCmdAttr<CommandAsking>("asking", 1, "", NO_KEY), )
diff --git a/src/commands/cmd_server.cc b/src/commands/cmd_server.cc
index f47b8f2b..a8512936 100644
--- a/src/commands/cmd_server.cc
+++ b/src/commands/cmd_server.cc
@@ -64,10 +64,6 @@ class CommandAuth : public Commander {
class CommandNamespace : public Commander {
public:
Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn, std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
Config *config = srv->GetConfig();
std::string sub_command = util::ToLower(args_[1]);
if (config->repl_namespace_enabled && config->IsSlave() && sub_command !=
"get") {
@@ -158,10 +154,6 @@ class CommandFlushDB : public Commander {
class CommandFlushAll : public Commander {
public:
Status Execute(engine::Context &ctx, Server *srv, Connection *conn,
std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
if (srv->GetConfig()->cluster_enabled) {
if (srv->slot_migrator->IsMigrationInProgress()) {
srv->slot_migrator->SetStopMigrationFlag(true);
@@ -209,10 +201,6 @@ class CommandSelect : public Commander {
class CommandConfig : public Commander {
public:
Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn, std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
Config *config = srv->GetConfig();
std::string sub_command = util::ToLower(args_[1]);
if ((sub_command == "rewrite" && args_.size() != 2) || (sub_command ==
"get" && args_.size() != 3) ||
@@ -539,12 +527,8 @@ class CommandMonitor : public Commander {
class CommandShutdown : public Commander {
public:
- Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn,
+ Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
[[maybe_unused]] Connection *conn,
[[maybe_unused]] std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
if (!srv->IsStopped()) {
LOG(INFO) << "SHUTDOWN command received, stopping the server";
srv->Stop();
@@ -903,11 +887,8 @@ class CommandCompact : public Commander {
class CommandBGSave : public Commander {
public:
- Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn, std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
+ Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
[[maybe_unused]] Connection *conn,
+ std::string *output) override {
Status s = srv->AsyncBgSaveDB();
if (!s.IsOK()) return s;
@@ -919,11 +900,8 @@ class CommandBGSave : public Commander {
class CommandFlushBackup : public Commander {
public:
- Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn, std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
+ Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
[[maybe_unused]] Connection *conn,
+ std::string *output) override {
Status s = srv->AsyncPurgeOldBackups(0, 0);
if (!s.IsOK()) return s;
@@ -979,10 +957,6 @@ class CommandSlaveOf : public Commander {
return {Status::RedisExecErr, "slaveof doesn't work with disable_wal
option"};
}
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
if (host_.empty()) {
auto s = srv->RemoveMaster();
if (!s.IsOK()) {
@@ -1043,11 +1017,8 @@ static uint64_t GenerateConfigFlag(uint64_t flags, const
std::vector<std::string
class CommandLastSave : public Commander {
public:
- Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
Connection *conn, std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
+ Status Execute([[maybe_unused]] engine::Context &ctx, Server *srv,
[[maybe_unused]] Connection *conn,
+ std::string *output) override {
int64_t unix_sec = srv->GetLastBgsaveTime();
*output = redis::Integer(unix_sec);
return Status::OK();
@@ -1152,10 +1123,6 @@ class CommandRdb : public Commander {
}
Status Execute(engine::Context &ctx, Server *srv, Connection *conn,
std::string *output) override {
- if (!conn->IsAdmin()) {
- return {Status::RedisExecErr, errAdminPermissionRequired};
- }
-
redis::Database redis(srv->storage, conn->GetNamespace());
auto stream_ptr = std::make_unique<RdbFileStream>(path_);
@@ -1387,17 +1354,18 @@ REDIS_REGISTER_COMMANDS(Server,
MakeCmdAttr<CommandAuth>("auth", 2, "read-only o
MakeCmdAttr<CommandSelect>("select", 2, "read-only",
NO_KEY),
MakeCmdAttr<CommandInfo>("info", -1, "read-only
ok-loading", NO_KEY),
MakeCmdAttr<CommandRole>("role", 1, "read-only
ok-loading", NO_KEY),
- MakeCmdAttr<CommandConfig>("config", -2, "read-only",
NO_KEY, GenerateConfigFlag),
- MakeCmdAttr<CommandNamespace>("namespace", -3,
"read-only", NO_KEY),
+ MakeCmdAttr<CommandConfig>("config", -2, "read-only
admin", NO_KEY, GenerateConfigFlag),
+ MakeCmdAttr<CommandNamespace>("namespace", -3,
"read-only admin", NO_KEY),
MakeCmdAttr<CommandKeys>("keys", 2, "read-only slow",
NO_KEY),
MakeCmdAttr<CommandFlushDB>("flushdb", 1, "write
no-dbsize-check exclusive", NO_KEY),
- MakeCmdAttr<CommandFlushAll>("flushall", 1, "write
no-dbsize-check exclusive", NO_KEY),
+ MakeCmdAttr<CommandFlushAll>("flushall", 1, "write
no-dbsize-check exclusive admin", NO_KEY),
MakeCmdAttr<CommandDBSize>("dbsize", -1, "read-only",
NO_KEY),
MakeCmdAttr<CommandSlowlog>("slowlog", -2,
"read-only", NO_KEY),
MakeCmdAttr<CommandPerfLog>("perflog", -2,
"read-only", NO_KEY),
MakeCmdAttr<CommandClient>("client", -2, "read-only",
NO_KEY),
MakeCmdAttr<CommandMonitor>("monitor", 1, "read-only
no-multi no-script", NO_KEY),
- MakeCmdAttr<CommandShutdown>("shutdown", 1, "read-only
exclusive no-multi no-script", NO_KEY),
+ MakeCmdAttr<CommandShutdown>("shutdown", 1, "read-only
exclusive no-multi no-script admin",
+ NO_KEY),
MakeCmdAttr<CommandQuit>("quit", 1, "read-only",
NO_KEY),
MakeCmdAttr<CommandScan>("scan", -2, "read-only",
NO_KEY),
MakeCmdAttr<CommandRandomKey>("randomkey", 1,
"read-only", NO_KEY),
@@ -1411,15 +1379,15 @@ REDIS_REGISTER_COMMANDS(Server,
MakeCmdAttr<CommandAuth>("auth", 2, "read-only o
MakeCmdAttr<CommandRestore>("restore", -4, "write", 1,
1, 1),
MakeCmdAttr<CommandCompact>("compact", 1, "read-only
no-script", NO_KEY),
- MakeCmdAttr<CommandBGSave>("bgsave", 1, "read-only
no-script", NO_KEY),
- MakeCmdAttr<CommandLastSave>("lastsave", 1,
"read-only", NO_KEY),
- MakeCmdAttr<CommandFlushBackup>("flushbackup", 1,
"read-only no-script", NO_KEY),
- MakeCmdAttr<CommandSlaveOf>("slaveof", 3, "read-only
exclusive no-script", NO_KEY),
- MakeCmdAttr<CommandSlaveOf>("replicaof", 3, "read-only
exclusive no-script", NO_KEY),
+ MakeCmdAttr<CommandBGSave>("bgsave", 1, "read-only
no-script admin", NO_KEY),
+ MakeCmdAttr<CommandLastSave>("lastsave", 1, "read-only
admin", NO_KEY),
+ MakeCmdAttr<CommandFlushBackup>("flushbackup", 1,
"read-only no-script admin", NO_KEY),
+ MakeCmdAttr<CommandSlaveOf>("slaveof", 3, "read-only
exclusive no-script admin", NO_KEY),
+ MakeCmdAttr<CommandSlaveOf>("replicaof", 3, "read-only
exclusive no-script admin", NO_KEY),
MakeCmdAttr<CommandStats>("stats", 1, "read-only",
NO_KEY),
- MakeCmdAttr<CommandRdb>("rdb", -3, "write exclusive",
NO_KEY),
+ MakeCmdAttr<CommandRdb>("rdb", -3, "write exclusive
admin", NO_KEY),
MakeCmdAttr<CommandReset>("reset", 1, "ok-loading
bypass-multi no-script", NO_KEY),
MakeCmdAttr<CommandApplyBatch>("applybatch", -2,
"write no-multi", NO_KEY),
MakeCmdAttr<CommandDump>("dump", 2, "read-only", 1, 1,
1),
- MakeCmdAttr<CommandPollUpdates>("pollupdates", -2,
"read-only", NO_KEY), )
+ MakeCmdAttr<CommandPollUpdates>("pollupdates", -2,
"read-only admin", NO_KEY), )
} // namespace redis
diff --git a/src/commands/commander.h b/src/commands/commander.h
index a91a44f0..a4dd8b2d 100644
--- a/src/commands/commander.h
+++ b/src/commands/commander.h
@@ -63,26 +63,28 @@ enum CommandFlags : uint64_t {
kCmdReadOnly = 1ULL << 1,
// "ok-loading" flag, for any command that can be executed while
// the db is in loading phase
- kCmdLoading = 1ULL << 5,
+ kCmdLoading = 1ULL << 2,
// "bypass-multi" flag, for commands that can be executed in a MULTI scope,
// but these commands will NOT be queued and will be executed immediately
- kCmdBypassMulti = 1ULL << 6,
+ kCmdBypassMulti = 1ULL << 3,
// "exclusive" flag, for commands that should be executed execlusive globally
- kCmdExclusive = 1ULL << 7,
+ kCmdExclusive = 1ULL << 4,
// "no-multi" flag, for commands that cannot be executed in MULTI scope
- kCmdNoMulti = 1ULL << 8,
+ kCmdNoMulti = 1ULL << 5,
// "no-script" flag, for commands that cannot be executed in scripting
- kCmdNoScript = 1ULL << 9,
+ kCmdNoScript = 1ULL << 6,
// "no-dbsize-check" flag, for commands that can ignore the db size checking
- kCmdNoDBSizeCheck = 1ULL << 12,
+ kCmdNoDBSizeCheck = 1ULL << 7,
// "slow" flag, for commands that run slowly,
// usually with a non-constant number of rocksdb ops
- kCmdSlow = 1ULL << 13,
+ kCmdSlow = 1ULL << 8,
// "blocking" flag, for commands that don't perform db ops immediately,
// but block and wait for some event to happen before performing db ops
- kCmdBlocking = 1ULL << 14,
+ kCmdBlocking = 1ULL << 9,
// "auth" flag, for commands used for authentication
- kCmdAuth = 1ULL << 15,
+ kCmdAuth = 1ULL << 10,
+ // "admin" flag, for commands that require admin permission
+ kCmdAdmin = 1ULL << 11,
};
enum class CommandCategory : uint8_t {
@@ -335,6 +337,8 @@ inline uint64_t ParseCommandFlags(const std::string
&description, const std::str
flags |= kCmdAuth;
else if (flag == "blocking")
flags |= kCmdBlocking;
+ else if (flag == "admin")
+ flags |= kCmdAdmin;
else {
std::cout << fmt::format("Encountered non-existent flag '{}' in command
{} in command attribute parsing", flag,
cmd_name)
diff --git a/src/server/redis_connection.cc b/src/server/redis_connection.cc
index 62028651..3f8e0dd2 100644
--- a/src/server/redis_connection.cc
+++ b/src/server/redis_connection.cc
@@ -452,6 +452,11 @@ void Connection::ExecuteCommands(std::deque<CommandTokens>
*to_process_cmds) {
continue;
}
+ if ((cmd_flags & kCmdAdmin) && !IsAdmin()) {
+ Reply(redis::Error({Status::RedisExecErr, errAdminPermissionRequired}));
+ continue;
+ }
+
if (config->cluster_enabled) {
s = srv_->cluster->CanExecByMySelf(attributes, cmd_tokens, this);
if (!s.IsOK()) {
diff --git a/src/storage/scripting.cc b/src/storage/scripting.cc
index 40002827..b2c0eebc 100644
--- a/src/storage/scripting.cc
+++ b/src/storage/scripting.cc
@@ -807,6 +807,11 @@ int RedisGenericCommand(lua_State *lua, int raise_error) {
}
}
+ if ((cmd_flags & redis::kCmdAdmin) && !conn->IsAdmin()) {
+ PushError(lua, redis::errAdminPermissionRequired);
+ return raise_error ? RaiseError(lua) : 1;
+ }
+
if (config->slave_readonly && srv->IsSlave() && (cmd_flags &
redis::kCmdWrite)) {
PushError(lua, "READONLY You can't write against a read only slave.");
return raise_error ? RaiseError(lua) : 1;
