feat(acl): add acl keys to acl setuser command #2258
Conversation
|
@dranikpg I tested this well but I did after I implement acl get and acl list. All the tests will follow in the following PR's |
| size_t key_cap = std::accumulate( | ||
| msg->keys.key_globs.begin(), msg->keys.key_globs.end(), 0, [](auto acc, auto& str) { | ||
| return acc + (str.first.capacity() * sizeof(char)) + sizeof(str.second); | ||
| }); | ||
| return sizeof(AclUpdateMessage) + msg->username.capacity() * sizeof(char) + | ||
| msg->commands.capacity() * sizeof(uint64_t) + key_cap; |
There was a problem hiding this comment.
Great you added it, but it doesn't make much of a difference, I skipped CheckpointMessage likewise because memory usage negligible. You can just avoid bothering with it next time you make a change 😄
| void Connection::DispatchOperations::operator()(const AclUpdateMessage& msg) { | ||
| if (self->cntx()) { | ||
| for (size_t id = 0; id < msg.username.size(); ++id) { | ||
| if (msg.username[id] == self->cntx()->authed_username) { | ||
| self->cntx()->acl_categories = msg.categories[id]; | ||
| self->cntx()->acl_commands = msg.commands[id]; | ||
| } | ||
| if (msg.username == self->cntx()->authed_username) { | ||
| self->cntx()->acl_categories = msg.categories; | ||
| self->cntx()->acl_commands = msg.commands; | ||
| self->cntx()->keys = msg.keys; |
There was a problem hiding this comment.
Actually we can pass msg by value and move, can't we? It's owned by us after all (nit because the runtime cost is negligible again)
There was a problem hiding this comment.
It's either accept msg by value and move the msg.keys or accept my const ref and copy the msg.keys value (as is now). There is not really a difference so I chose the later because it's how the rest of overloads accept their parameters (and looks consistent)
There was a problem hiding this comment.
or accept my const ref and copy
I just meant there is no reason to because it's gonna be dropped after this call either way
src/server/acl/helpers.cc
Outdated
| if (absl::StartsWith(command, "%")) { | ||
| if (command.size() < 3) { | ||
| return {}; | ||
| } | ||
|
|
||
| if (!(command.substr(1, 2) == "RW")) { | ||
| if (command[1] == 'R') { | ||
| op = KeyOp::READ; | ||
| } else if (command[1] == 'W') { | ||
| op = KeyOp::WRITE; | ||
| } else { | ||
| return {}; | ||
| } | ||
| command = command.substr(2); | ||
| } else { | ||
| command = command.substr(3); | ||
| } | ||
| } |
There was a problem hiding this comment.
Maybe checking 3 times startswith would even be easier 😅 , because I had to think at the if (! (substr == "RW")) part 🤣 (!=)
There was a problem hiding this comment.
so much better now!
| struct ParseKeyResult { | ||
| std::string glob; | ||
| KeyOp op; | ||
| bool all_keys{false}; | ||
| bool reset_keys{false}; | ||
| }; |
There was a problem hiding this comment.
You have this type defined in the previous pr, you can just rebase on top of it
| @@ -138,7 +138,7 @@ const AclKeys& User::Keys() const { | |||
| return keys_; | |||
| } | |||
|
|
|||
| void User::SetKeyGlobs(std::vector<UpdateKey>&& keys) { | |||
| void User::SetKeyGlobs(std::vector<UpdateKey> keys) { | |||
There was a problem hiding this comment.
Did it here cause I wanted to merge the previous PR :)
PR2
part of #2229