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

Throughput limit exemptions for SR, PP #11692

Draft
wants to merge 12 commits into
base: dev
Choose a base branch
from
199 changes: 174 additions & 25 deletions src/v/config/tests/throughput_control_group_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "config/config_store.h"
#include "config/property.h"
#include "config/throughput_control_group.h"
#include "security/acl.h"

#include <seastar/core/sstring.hh>
#include <seastar/testing/thread_test_case.hh>
Expand All @@ -25,26 +26,50 @@
#include <exception>
#include <iterator>
#include <locale>
#include <optional>
#include <vector>

using namespace std::string_literals;

SEASTAR_THREAD_TEST_CASE(throughput_control_group_test) {
std::vector<config::throughput_control_group> tcgv;
tcgv.emplace_back();
struct test_config : public config::config_store {
config::property<std::vector<config::throughput_control_group>> cgroups;
test_config()
: cgroups(
*this, "cgroups", "", {.needs_restart = config::needs_restart::no}) {}

struct test_config : public config::config_store {
config::property<std::vector<config::throughput_control_group>> cgroups;
test_config()
: cgroups(*this, "cgroups", "") {}
auto get_match_index(std::optional<std::string_view> client_id) const
-> std::optional<size_t> {
if (const auto i = config::find_throughput_control_group(
cgroups().cbegin(), cgroups().cend(), client_id, nullptr);
i != cgroups().cend()) {
return std::distance(cgroups().cbegin(), i);
}
return std::nullopt;
};

auto get_match_index(
const security::principal_type type, ss::sstring principal_name) const
-> std::optional<size_t> {
const security::acl_principal p(type, std::move(principal_name));
if (const auto i = config::find_throughput_control_group(
cgroups().cbegin(), cgroups().cend(), std::nullopt, &p);
i != cgroups().cend()) {
return std::distance(cgroups().cbegin(), i);
}
return std::nullopt;
}
};

SEASTAR_THREAD_TEST_CASE(throughput_control_group_by_clientid_test) {
std::vector<config::throughput_control_group> tcgv;
tcgv.emplace_back();

auto cfg_node = YAML::Load(R"(
cgroups:
- name: ""
client_id: client_id-1
- client_id: cli.+_id-\d+
- client_id: another unnamed group indended to verify this passes validation
- client_id: another unnamed group intended to verify this passes validation
- name: match-nothing-group
client_id: ""
- name: unspecified client_id
Expand All @@ -58,7 +83,7 @@ SEASTAR_THREAD_TEST_CASE(throughput_control_group_test) {
BOOST_TEST(
YAML::Dump(config::to_yaml(cfg, config::redact_secrets{false}))
== YAML::Dump(cfg_node));
BOOST_TEST(cfg.cgroups().size() == 6);
BOOST_REQUIRE(cfg.cgroups().size() == 6);
for (auto& cg : cfg.cgroups()) {
BOOST_TEST(!cg.throughput_limit_node_in_bps);
BOOST_TEST(!cg.throughput_limit_node_out_bps);
Expand All @@ -72,22 +97,32 @@ SEASTAR_THREAD_TEST_CASE(throughput_control_group_test) {
BOOST_TEST(!validate_throughput_control_groups(
cfg.cgroups().cbegin(), cfg.cgroups().cend()));

// Matches
const auto get_match_index =
[&cfg](
std::optional<std::string_view> client_id) -> std::optional<size_t> {
if (const auto i = config::find_throughput_control_group(
cfg.cgroups().cbegin(), cfg.cgroups().cend(), client_id);
i != cfg.cgroups().cend()) {
return std::distance(cfg.cgroups().cbegin(), i);
// Equality
for (size_t k = 0; k != cfg.cgroups().size(); ++k) {
for (size_t l = 0; l != cfg.cgroups().size(); ++l) {
BOOST_TEST(
(cfg.cgroups()[k] == cfg.cgroups()[l]) == (k == l),
"k=" << k << " l=" << l);
}
return std::nullopt;
};
BOOST_TEST(get_match_index("client_id-1") == 0);
BOOST_TEST(get_match_index("clinet_id-2") == 1);
BOOST_TEST(get_match_index("") == 3);
BOOST_TEST(get_match_index(std::nullopt) == 4);
BOOST_TEST(get_match_index("nonclient_id") == 5);
}

// Matches
BOOST_TEST(cfg.get_match_index("client_id-1") == 0);
BOOST_TEST(cfg.get_match_index("clinet_id-2") == 1);
BOOST_TEST(cfg.get_match_index("") == 3);
BOOST_TEST(cfg.get_match_index(std::nullopt) == 4);
BOOST_TEST(cfg.get_match_index("nonclient_id") == 5);

// Copying
config::throughput_control_group p4 = cfg.cgroups()[0];
BOOST_TEST(p4 == cfg.cgroups()[0]);
BOOST_TEST(fmt::format("{}", p4) == fmt::format("{}", cfg.cgroups()[0]));

// Binding a property of
auto binding = cfg.cgroups.bind();
BOOST_TEST(binding() == cfg.cgroups());
BOOST_TEST(
fmt::format("{}", binding()) == fmt::format("{}", cfg.cgroups()));

// Failure cases

Expand All @@ -113,7 +148,7 @@ SEASTAR_THREAD_TEST_CASE(throughput_control_group_test) {
!cfg.read_yaml(YAML::Load(R"(cgroups: [{name: n, client_id: "*"}])"s))
.empty());

// Specify any throupghput limit
// Specify any throughput limit
BOOST_TEST(
!cfg
.read_yaml(YAML::Load(
Expand All @@ -137,3 +172,117 @@ SEASTAR_THREAD_TEST_CASE(throughput_control_group_test) {
.find("uplicate")
!= ss::sstring::npos);
}

SEASTAR_THREAD_TEST_CASE(throughput_control_group_by_principal_test) {
std::vector<config::throughput_control_group> tcgv;
tcgv.emplace_back();

auto cfg_node = YAML::Load(R"(
cgroups:
- name: empty principals list in useless as it matches nothing, but still a valid config
principals:
- name: match a specific user
principals:
- user: alpha
- name: match a list of users (principal 'user')
principals:
- user: beta
- user: gamma
- user: delta
# - name: (future) match a group (principal 'group')
# principals:
# - group: greek_letters
- name: match schema registry (principal 'ephemeral user')
principals:
- service: schema registry
Copy link
Member

Choose a reason for hiding this comment

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

Question: Why not keep this as security::principal_type? I.e., ephemeral_user rather than service?

I don't dislike it, but it adds an overloaded term.

Copy link
Contributor Author

@dlex dlex Jun 28, 2023

Choose a reason for hiding this comment

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

That would have exposed an implementation detail for the users. Currently the name of the ephemeral user used for SR/PP is not exposed anywhere, and is only a matter of the code. It can be e.g. a uuid as well, and it can be changed anytime.

- name: match PP (principal 'ephemeral user')
principals:
- service: panda proxy
- name: match heterogeneous set of principals
principals:
- user: servicebot
- service: schema registry
- service: panda proxy
- name: match _any_ authenticated user
principals:
- user: "*"
- name: catch-all - no "principal" matches anything
)");
cfg_node.SetStyle(YAML::EmitterStyle::Flow);

test_config cfg;
BOOST_TEST(cfg.read_yaml(cfg_node).empty());
BOOST_TEST(
YAML::Dump(config::to_yaml(cfg, config::redact_secrets{false}))
== YAML::Dump(cfg_node));
BOOST_REQUIRE(cfg.cgroups().size() == 8);
for (auto& cg : cfg.cgroups()) {
BOOST_TEST(!cg.throughput_limit_node_in_bps);
BOOST_TEST(!cg.throughput_limit_node_out_bps);
BOOST_TEST(cg.validate().empty());
}
BOOST_TEST(!validate_throughput_control_groups(
cfg.cgroups().cbegin(), cfg.cgroups().cend()));

// Equality
for (size_t k = 0; k != cfg.cgroups().size(); ++k) {
for (size_t l = 0; l != cfg.cgroups().size(); ++l) {
BOOST_TEST(
(cfg.cgroups()[k] == cfg.cgroups()[l]) == (k == l),
"k=" << k << " l=" << l);
}
}

// Matches
using pt = security::principal_type;
BOOST_TEST(cfg.get_match_index(std::nullopt) == 7);
BOOST_TEST(cfg.get_match_index(pt::user, "alpha") == 1);
BOOST_TEST(cfg.get_match_index(pt::user, "beta") == 2);
BOOST_TEST(cfg.get_match_index(pt::user, "gamma") == 2);
BOOST_TEST(cfg.get_match_index(pt::user, "delta") == 2);
BOOST_TEST(cfg.get_match_index(pt::user, "epsilon") == 6);
BOOST_TEST(cfg.get_match_index(pt::user, "zeta") == 6);
BOOST_TEST(
cfg.get_match_index(pt::ephemeral_user, "__schema_registry") == 3);
BOOST_TEST(cfg.get_match_index(pt::ephemeral_user, "__pandaproxy") == 4);
BOOST_TEST(cfg.get_match_index(pt::user, "servicebot") == 5);

// Copying
config::throughput_control_group p4 = cfg.cgroups()[1];
BOOST_TEST(p4 == cfg.cgroups()[1]);
BOOST_TEST(fmt::format("{}", p4) == fmt::format("{}", cfg.cgroups()[1]));

// Binding a property of
auto binding = cfg.cgroups.bind();
BOOST_TEST(binding() == cfg.cgroups());
BOOST_TEST(
fmt::format("{}", binding()) == fmt::format("{}", cfg.cgroups()));

// Failure cases

// Invalid keys in principal
BOOST_TEST(!cfg
.read_yaml(YAML::Load(
R"(cgroups: [{principals: [{invalid_key: value}]}])"s))
.empty());

// Invalid service name
BOOST_TEST(!cfg
.read_yaml(YAML::Load(
R"(cgroups: [{principals: [{service: sepulkarium}]}])"s))
.empty());

// Control characters
BOOST_TEST(!cfg
.read_yaml(YAML::Load(
"cgroups: [{principals: [{\auser: tarantoga}]}]"s))
.empty());
BOOST_TEST(!cfg
.read_yaml(YAML::Load(
"cgroups: [{principals: [{user: tarantoga\001}]}]"s))
.empty());
BOOST_TEST(!cfg
.read_yaml(YAML::Load(
"cgroups: [{principals: [{service: panda proxy\002}]}]"s))
.empty());
}
Loading