Skip to content

Commit

Permalink
chore(acl): Implicit categories (#3411)
Browse files Browse the repository at this point in the history 
Most of our CO:: categories became meaningless with the introduction of acl. For example, CO::FAST literally doesn't mean anything, it's never read or used.

* add implicit categories

---------

Signed-off-by: Vladislav Oleshko <[email protected]>
  • Loading branch information
@dranikpg
dranikpg authored Nov 8, 2024
1 parent c9537bb commit b860b71
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 68 deletions.
53 changes: 47 additions & 6 deletions src/server/command_registry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,48 @@ using absl::StrAppend;
using absl::StrCat;
using absl::StrSplit;

CommandId::CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key,
int8_t last_key, uint32_t acl_categories)
: facade::CommandId(name, mask, arity, first_key, last_key, acl_categories) {
namespace {

uint32_t ImplicitCategories(uint32_t mask) {
if (mask & CO::ADMIN)
opt_mask_ |= CO::NOSCRIPT;
mask |= CO::NOSCRIPT;
return mask;
}

uint32_t ImplicitAclCategories(uint32_t mask) {
mask = ImplicitCategories(mask);
uint32_t out = 0;

if (mask & CO::WRITE)
out |= acl::WRITE;

if ((mask & CO::READONLY) && ((mask & CO::NOSCRIPT) == 0))
out |= acl::READ;

if (mask & CO::ADMIN)
out |= acl::ADMIN | acl::DANGEROUS;

// todo pubsub

if (mask & CO::FAST)
out |= acl::FAST;

if (mask & CO::BLOCKING)
out |= acl::BLOCKING;

if ((out & acl::FAST) == 0)
out |= acl::SLOW;

return out;
}

} // namespace

CommandId::CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key,
int8_t last_key, std::optional<uint32_t> acl_categories)
: facade::CommandId(name, ImplicitCategories(mask), arity, first_key, last_key,
acl_categories.value_or(ImplicitAclCategories(mask))) {
implicit_acl_ = !acl_categories.has_value();
}

bool CommandId::IsTransactional() const {
Expand Down Expand Up @@ -152,6 +189,9 @@ CommandRegistry& CommandRegistry::operator<<(CommandId cmd) {
}

cmd.SetFamily(family_of_commands_.size() - 1);
if (acl_category_)
cmd.SetAclCategory(*acl_category_);

if (!is_sub_command || absl::StartsWith(cmd.name(), "ACL")) {
cmd.SetBitIndex(1ULL << bit_index_);
family_of_commands_.back().push_back(std::string(k));
Expand All @@ -165,9 +205,10 @@ CommandRegistry& CommandRegistry::operator<<(CommandId cmd) {
return *this;
}

void CommandRegistry::StartFamily() {
family_of_commands_.push_back({});
void CommandRegistry::StartFamily(std::optional<uint32_t> acl_category) {
family_of_commands_.emplace_back();
bit_index_ = 0;
acl_category_ = acl_category;
}

std::string_view CommandRegistry::RenamedOrOriginal(std::string_view orig) const {
Expand Down
11 changes: 9 additions & 2 deletions src/server/command_registry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class CommandId : public facade::CommandId {
// NOTICE: name must be a literal string, otherwise metrics break! (see cmd_stats_map in
// server_state.h)
CommandId(const char* name, uint32_t mask, int8_t arity, int8_t first_key, int8_t last_key,
uint32_t acl_categories);
std::optional<uint32_t> acl_categories = std::nullopt);

CommandId(CommandId&&) = default;

Expand Down Expand Up @@ -146,7 +146,13 @@ class CommandId : public facade::CommandId {
return command_stats_[thread_index];
}

void SetAclCategory(uint32_t mask) {
if (implicit_acl_)
acl_categories_ |= mask;
}

private:
bool implicit_acl_;
std::unique_ptr<CmdCallStats[]> command_stats_;
Handler handler_;
ArgValidator validator_;
Expand Down Expand Up @@ -194,7 +200,7 @@ class CommandRegistry {
}
}

void StartFamily();
void StartFamily(std::optional<uint32_t> acl_category = std::nullopt);

std::string_view RenamedOrOriginal(std::string_view orig) const;

Expand All @@ -212,6 +218,7 @@ class CommandRegistry {

FamiliesVec family_of_commands_;
size_t bit_index_;
std::optional<uint32_t> acl_category_; // category of family currently being built
};

} // namespace dfly
15 changes: 13 additions & 2 deletions src/server/main_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2487,8 +2487,8 @@ void Service::Command(CmdArgList args, Transaction* tx, SinkReplyBuilder* builde
});

auto* rb = static_cast<RedisReplyBuilder*>(builder);
auto serialize_command = [&rb](string_view name, const CommandId& cid) {
rb->StartArray(6);
auto serialize_command = [&rb, this](string_view name, const CommandId& cid) {
rb->StartArray(7);
rb->SendSimpleString(cid.name());
rb->SendLong(cid.arity());
rb->StartArray(CommandId::OptCount(cid.opt_mask()));
Expand All @@ -2504,6 +2504,17 @@ void Service::Command(CmdArgList args, Transaction* tx, SinkReplyBuilder* builde
rb->SendLong(cid.first_key_pos());
rb->SendLong(cid.last_key_pos());
rb->SendLong(cid.opt_mask() & CO::INTERLEAVED_KEYS ? 2 : 1);

{
const auto& table = acl_family_.GetRevTable();
vector<string> cats;
for (uint32_t i = 0; i < 32; i++) {
if (cid.acl_categories() & (1 << i)) {
cats.emplace_back("@" + table[i]);
}
}
rb->SendSimpleStrArr(cats);
}
};

// If no arguments are specified, reply with all commands
Expand Down
84 changes: 26 additions & 58 deletions src/server/string_family.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1530,68 +1530,36 @@ void StringFamily::ClThrottle(CmdArgList args, Transaction* tx, SinkReplyBuilder

#define HFUNC(x) SetHandler(&StringFamily::x)

namespace acl {
constexpr uint32_t kSet = WRITE | STRING | SLOW;
constexpr uint32_t kSetEx = WRITE | STRING | SLOW;
constexpr uint32_t kPSetEx = WRITE | STRING | SLOW;
constexpr uint32_t kSetNx = WRITE | STRING | FAST;
constexpr uint32_t kAppend = WRITE | STRING | FAST;
constexpr uint32_t kPrepend = WRITE | STRING | FAST;
constexpr uint32_t kIncr = WRITE | STRING | FAST;
constexpr uint32_t kDecr = WRITE | STRING | FAST;
constexpr uint32_t kIncrBy = WRITE | STRING | FAST;
constexpr uint32_t kIncrByFloat = WRITE | STRING | FAST;
constexpr uint32_t kDecrBy = WRITE | STRING | FAST;
constexpr uint32_t kGet = READ | STRING | FAST;
constexpr uint32_t kGetDel = WRITE | STRING | FAST;
constexpr uint32_t kGetEx = WRITE | STRING | FAST;
constexpr uint32_t kGetSet = WRITE | STRING | FAST;
constexpr uint32_t kMGet = READ | STRING | FAST;
constexpr uint32_t kMSet = WRITE | STRING | SLOW;
constexpr uint32_t kMSetNx = WRITE | STRING | SLOW;
constexpr uint32_t kStrLen = READ | STRING | FAST;
constexpr uint32_t kGetRange = READ | STRING | SLOW;
constexpr uint32_t kSubStr = READ | STRING | SLOW;
constexpr uint32_t kSetRange = WRITE | STRING | SLOW;
// ClThrottle is a module in redis. Therefore we introduce a new extension
// to the category. We should consider other defaults as well
constexpr uint32_t kClThrottle = THROTTLE;
} // namespace acl

void StringFamily::Register(CommandRegistry* registry) {
constexpr uint32_t kMSetMask =
CO::WRITE | CO::DENYOOM | CO::INTERLEAVED_KEYS | CO::NO_AUTOJOURNAL;

registry->StartFamily();
*registry
<< CI{"SET", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, -3, 1, 1, acl::kSet}.HFUNC(Set)
<< CI{"SETEX", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, 4, 1, 1, acl::kSetEx}.HFUNC(
SetEx)
<< CI{"PSETEX", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, 4, 1, 1, acl::kPSetEx}.HFUNC(
PSetEx)
<< CI{"SETNX", CO::WRITE | CO::DENYOOM, 3, 1, 1, acl::kSetNx}.HFUNC(SetNx)
<< CI{"APPEND", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1, acl::kAppend}.HFUNC(Append)
<< CI{"PREPEND", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1, acl::kPrepend}.HFUNC(Prepend)
<< CI{"INCR", CO::WRITE | CO::FAST, 2, 1, 1, acl::kIncr}.HFUNC(Incr)
<< CI{"DECR", CO::WRITE | CO::FAST, 2, 1, 1, acl::kDecr}.HFUNC(Decr)
<< CI{"INCRBY", CO::WRITE | CO::FAST, 3, 1, 1, acl::kIncrBy}.HFUNC(IncrBy)
<< CI{"INCRBYFLOAT", CO::WRITE | CO::FAST, 3, 1, 1, acl::kIncrByFloat}.HFUNC(IncrByFloat)
<< CI{"DECRBY", CO::WRITE | CO::FAST, 3, 1, 1, acl::kDecrBy}.HFUNC(DecrBy)
<< CI{"GET", CO::READONLY | CO::FAST, 2, 1, 1, acl::kGet}.HFUNC(Get)
<< CI{"GETDEL", CO::WRITE | CO::FAST, 2, 1, 1, acl::kGetDel}.HFUNC(GetDel)
<< CI{"GETEX", CO::WRITE | CO::DENYOOM | CO::FAST | CO::NO_AUTOJOURNAL, -2, 1, 1, acl::kGetEx}
.HFUNC(GetEx)
<< CI{"GETSET", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1, acl::kGetSet}.HFUNC(GetSet)
<< CI{"MGET", CO::READONLY | CO::FAST | CO::IDEMPOTENT, -2, 1, -1, acl::kMGet}.HFUNC(MGet)
<< CI{"MSET", kMSetMask, -3, 1, -1, acl::kMSet}.HFUNC(MSet)
<< CI{"MSETNX", kMSetMask, -3, 1, -1, acl::kMSetNx}.HFUNC(MSetNx)
<< CI{"STRLEN", CO::READONLY | CO::FAST, 2, 1, 1, acl::kStrLen}.HFUNC(StrLen)
<< CI{"GETRANGE", CO::READONLY | CO::FAST, 4, 1, 1, acl::kGetRange}.HFUNC(GetRange)
<< CI{"SUBSTR", CO::READONLY | CO::FAST, 4, 1, 1, acl::kSubStr}.HFUNC(
GetRange) // Alias for GetRange
<< CI{"SETRANGE", CO::WRITE | CO::FAST | CO::DENYOOM, 4, 1, 1, acl::kSetRange}.HFUNC(SetRange)
<< CI{"CL.THROTTLE", CO::WRITE | CO::DENYOOM | CO::FAST, -5, 1, 1, acl::kClThrottle}.HFUNC(
ClThrottle);
registry->StartFamily(acl::STRING);
*registry << CI{"SET", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, -3, 1, 1}.HFUNC(Set)
<< CI{"SETEX", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, 4, 1, 1}.HFUNC(SetEx)
<< CI{"PSETEX", CO::WRITE | CO::DENYOOM | CO::NO_AUTOJOURNAL, 4, 1, 1}.HFUNC(PSetEx)
<< CI{"SETNX", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1}.HFUNC(SetNx)
<< CI{"APPEND", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1}.HFUNC(Append)
<< CI{"PREPEND", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1}.HFUNC(Prepend)
<< CI{"INCR", CO::WRITE | CO::FAST, 2, 1, 1}.HFUNC(Incr)
<< CI{"DECR", CO::WRITE | CO::FAST, 2, 1, 1}.HFUNC(Decr)
<< CI{"INCRBY", CO::WRITE | CO::FAST, 3, 1, 1}.HFUNC(IncrBy)
<< CI{"INCRBYFLOAT", CO::WRITE | CO::FAST, 3, 1, 1}.HFUNC(IncrByFloat)
<< CI{"DECRBY", CO::WRITE | CO::FAST, 3, 1, 1}.HFUNC(DecrBy)
<< CI{"GET", CO::READONLY | CO::FAST, 2, 1, 1}.HFUNC(Get)
<< CI{"GETDEL", CO::WRITE | CO::FAST, 2, 1, 1}.HFUNC(GetDel)
<< CI{"GETEX", CO::WRITE | CO::DENYOOM | CO::FAST | CO::NO_AUTOJOURNAL, -2, 1, 1}.HFUNC(
GetEx)
<< CI{"GETSET", CO::WRITE | CO::DENYOOM | CO::FAST, 3, 1, 1}.HFUNC(GetSet)
<< CI{"MGET", CO::READONLY | CO::FAST | CO::IDEMPOTENT, -2, 1, -1}.HFUNC(MGet)
<< CI{"MSET", kMSetMask, -3, 1, -1}.HFUNC(MSet)
<< CI{"MSETNX", kMSetMask, -3, 1, -1}.HFUNC(MSetNx)
<< CI{"STRLEN", CO::READONLY | CO::FAST, 2, 1, 1}.HFUNC(StrLen)
<< CI{"GETRANGE", CO::READONLY, 4, 1, 1}.HFUNC(GetRange)
<< CI{"SUBSTR", CO::READONLY, 4, 1, 1}.HFUNC(GetRange) // Alias for GetRange
<< CI{"SETRANGE", CO::WRITE | CO::DENYOOM, 4, 1, 1}.HFUNC(SetRange)
<< CI{"CL.THROTTLE", CO::WRITE | CO::DENYOOM | CO::FAST, -5, 1, 1, acl::THROTTLE}.HFUNC(
ClThrottle);
}

} // namespace dfly

0 comments on commit b860b71

Please sign in to comment.