Skip to content

Commit d435ce3

Browse files
Merge pull request #30978 from nguyen-andrew/sl-role-sync-scale
tests: scale coverage for shadow-link role sync
2 parents e76764e + f8b7d82 commit d435ce3

6 files changed

Lines changed: 247 additions & 4 deletions

File tree

src/v/cluster/security_manager.cc

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,10 @@ security_manager::fill_snapshot(controller_snapshot& controller_snap) const {
211211

212212
snapshot.acls = co_await _authorizer.local().all_bindings();
213213

214-
auto roles = _roles.local().range([](const auto&) { return true; });
214+
auto roles = co_await _roles.local().all_roles_with_members();
215215
snapshot.roles.reserve(roles.size());
216-
for (const auto& role : roles) {
217-
security::role_name name{role};
218-
snapshot.roles.emplace_back(name, *_roles.local().get(name));
216+
for (auto& rwm : roles) {
217+
snapshot.roles.emplace_back(std::move(rwm.name), std::move(rwm.role));
219218
}
220219

221220
co_return;

src/v/security/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,7 @@ redpanda_cc_library(
178178
"//src/v/serde:named_type",
179179
"//src/v/serde:optional",
180180
"//src/v/serde:variant",
181+
"//src/v/ssx:async_algorithm",
181182
"//src/v/ssx:future_util",
182183
"//src/v/ssx:sformat",
183184
"//src/v/ssx:thread_worker",

src/v/security/role.cc

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212

1313
#include "container/chunked_hash_map.h"
1414
#include "security/role_store.h"
15+
#include "ssx/async_algorithm.h"
16+
17+
#include <seastar/core/future.hh>
1518

1619
#include <algorithm>
1720
#include <ranges>
@@ -121,4 +124,39 @@ chunked_vector<role_with_members> role_store::roles_with_members(
121124
return result;
122125
}
123126

127+
ss::future<chunked_vector<role_with_members>>
128+
role_store::all_roles_with_members() const {
129+
// IMPORTANT: intended solely for security_manager::fill_snapshot.
130+
131+
// One counter spans all three phases so yield accounting is continuous.
132+
ssx::async_counter counter;
133+
134+
chunked_hash_map<role_name_view, role::container_type> by_role;
135+
by_role.reserve(_roles.size());
136+
co_await ssx::async_for_each_counter(
137+
counter, _roles, [&by_role](const role_name& rn) {
138+
by_role.try_emplace(role_name_view{rn});
139+
});
140+
141+
for (const auto& [member, role_names] : _members_store) {
142+
co_await ssx::async_for_each_counter(
143+
counter, role_names, [&by_role, &member](const role_name_view& rn) {
144+
if (auto it = by_role.find(rn); it != by_role.end()) {
145+
it->second.insert(member);
146+
}
147+
});
148+
}
149+
150+
chunked_vector<role_with_members> result;
151+
result.reserve(by_role.size());
152+
co_await ssx::async_for_each_counter(counter, by_role, [&result](auto& e) {
153+
auto& [name_view, members] = e;
154+
result.push_back(
155+
role_with_members{
156+
.name = role_name{ss::sstring{name_view()}},
157+
.role = role{std::move(members)}});
158+
});
159+
co_return result;
160+
}
161+
124162
} // namespace security

src/v/security/role_store.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "security/role.h"
1919
#include "security/types.h"
2020

21+
#include <seastar/core/future.hh>
2122
#include <seastar/util/noncopyable_function.hh>
2223

2324
#include <boost/range/iterator_range.hpp>
@@ -145,6 +146,19 @@ class role_store {
145146
chunked_vector<role_with_members>
146147
roles_with_members(const std::function<bool(const role_name&)>& pred) const;
147148

149+
// IMPORTANT: intended solely for security_manager::fill_snapshot. This
150+
// suspends mid-iteration, which is safe ONLY while the caller holds
151+
// mux_state_machine's _apply_mtx: that mutex serializes against command
152+
// application, so _roles/_members_store can't mutate across a yield. A
153+
// caller without that lock risks iterating a container that changes
154+
// underfoot.
155+
//
156+
// Like roles_with_members, but enumerates every role with its members in a
157+
// single pass and yields periodically, so snapshotting a very large role
158+
// store doesn't stall the reactor.
159+
ss::future<chunked_vector<role_with_members>>
160+
all_roles_with_members() const;
161+
148162
static constexpr auto name_prefix_filter =
149163
[](const role_accessor& e, std::string_view filter) {
150164
const auto& name = e.first;

tests/rptest/tests/rpk_shadow_link_test.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ def test_shadow_link_full_sync(self):
4848
expected_acl = self._seed_source_acl()
4949
source_offsets = self._seed_source_consumer_group(topic)
5050
subject = self._seed_source_schema()
51+
expected_role = self._seed_source_role()
5152

5253
assert not self.topic_exists_in_target(topic.name), (
5354
f"{topic.name} unexpectedly present on the shadow cluster before linking"
@@ -88,6 +89,13 @@ def test_shadow_link_full_sync(self):
8889
retry_on_exc=True,
8990
err_msg="schema registry subject was not synced to the shadow cluster",
9091
)
92+
wait_until(
93+
lambda: self._role_synced(rpk, expected_role),
94+
timeout_sec=90,
95+
backoff_sec=2,
96+
retry_on_exc=True,
97+
err_msg="role was not synced to the shadow cluster",
98+
)
9199

92100
def _full_link_config(self) -> dict[str, Any]:
93101
"""
@@ -132,6 +140,16 @@ def _full_link_config(self) -> dict[str, Any]:
132140
"schema_registry_sync_options": {
133141
"shadow_schema_registry_topic": {},
134142
},
143+
"role_sync_options": {
144+
"interval": "1s",
145+
"role_name_filters": [
146+
{
147+
"pattern_type": "PREFIX",
148+
"filter_type": "INCLUDE",
149+
"name": "synced-",
150+
}
151+
],
152+
},
135153
}
136154

137155
def _seed_source_topic(self) -> TopicSpec:
@@ -191,6 +209,16 @@ def _seed_source_schema(self) -> str:
191209
self.source_cluster_rpk.create_schema_from_str(subject, schema)
192210
return subject
193211

212+
def _seed_source_role(self) -> dict[str, str]:
213+
"""Create a role with a member on the source cluster, and return the
214+
role name and member principal expected to appear on the shadow
215+
cluster. The name matches the link's PREFIX 'synced-' role filter."""
216+
role = "synced-test-role"
217+
member = "role-member"
218+
self.source_cluster_rpk.create_role(role)
219+
self.source_cluster_rpk.assign_role(role, [member])
220+
return {"role": role, "member": member}
221+
194222
def _assert_link_listed(self, rpk: RpkTool) -> None:
195223
links = rpk.shadow_list()
196224
self.logger.debug(f"rpk shadow list: {links}")
@@ -224,6 +252,17 @@ def _schema_synced(self, subject: str) -> bool:
224252
names = [s["subject"] if isinstance(s, dict) else s for s in subjects]
225253
return subject in names
226254

255+
def _role_synced(self, rpk: RpkTool, expected: dict[str, str]) -> bool:
256+
roles = rpk.list_roles().get("roles", [])
257+
self.logger.debug(f"target roles: {roles}")
258+
if expected["role"] not in roles:
259+
return False
260+
members = [
261+
m["name"] for m in rpk.describe_role(expected["role"]).get("members", [])
262+
]
263+
self.logger.debug(f"target role {expected['role']} members: {members}")
264+
return expected["member"] in members
265+
227266
def _wait_link_active(self, rpk: RpkTool) -> None:
228267
def link_active() -> bool:
229268
status = rpk.shadow_status(self.LINK_NAME)

tests/rptest/tests/shadow_link_role_sync_test.py

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77
# the Business Source License, use of this software will be governed
88
# by the Apache License, Version 2.0
99

10+
import time
1011
from typing import Callable
1112

1213
from connectrpc.errors import ConnectError, ConnectErrorCode
1314

1415
import google.protobuf.duration_pb2 as duration_pb2
1516
import google.protobuf.field_mask_pb2 as field_mask_pb2
1617

18+
from ducktape.mark import parametrize
1719
from ducktape.tests.test import TestContext
1820
from ducktape.utils.util import wait_until
1921

@@ -548,3 +550,153 @@ def test_permission_grant_activates_sync(self):
548550
backoff_sec=1,
549551
err_msg="role did not mirror after the permission grant",
550552
)
553+
554+
555+
class ShadowLinkRoleSyncScaleTest(RoleSyncTestBase):
556+
"""Scale characterization for the roles migrator. Each parametrized point
557+
runs the full create -> update -> delete lifecycle (exercising all three
558+
migrator apply loops at volume) while pushing one cost axis: role count,
559+
total membership, or a single role's member set.
560+
"""
561+
562+
PREFIX = "synced-"
563+
564+
def __init__(self, test_context: TestContext):
565+
super().__init__(
566+
test_context=test_context,
567+
num_brokers=3,
568+
secondary_cluster_args=SecondaryClusterArgs(),
569+
)
570+
571+
def setUp(self):
572+
super().setUp()
573+
self._src = AdminV2RoleWrapper(AdminV2(self.source_cluster_service))
574+
575+
def _retry_transient(
576+
self, fn: Callable[..., object], *args: object, **kwargs: object
577+
) -> None:
578+
"""Run a source mutation, retrying the transient controller-backpressure
579+
errors -- raft not_leader/timeout, surfaced as a ConnectError 'internal'
580+
-- that thousands of rapid role mutations can provoke. Other error codes
581+
and the final failure propagate."""
582+
last: ConnectError | None = None
583+
for attempt in range(6):
584+
try:
585+
fn(*args, **kwargs)
586+
return
587+
except ConnectError as e:
588+
if e.code != ConnectErrorCode.INTERNAL:
589+
raise
590+
last = e
591+
self.logger.debug(f"retrying transient source mutation: {e}")
592+
time.sleep(0.5 * (attempt + 1))
593+
assert last is not None
594+
raise last
595+
596+
def _seed_source_roles(self, num_roles: int, members_per_role: int) -> set[str]:
597+
"""Create num_roles in-scope roles on the source, each with
598+
members_per_role distinct user members; return the role names. Members
599+
are free-form principal names that need no backing user accounts -- the
600+
migrator mirrors them as data."""
601+
names: set[str] = set()
602+
for i in range(num_roles):
603+
name = f"{self.PREFIX}role-{i}"
604+
members = [_user(f"u-{i}-{m}") for m in range(members_per_role)]
605+
self._retry_transient(self._src.create_role, role=name, members=members)
606+
names.add(name)
607+
return names
608+
609+
def _synced_role_names(self) -> set[str]:
610+
return {n for n in self._dst.list_role_names() if n.startswith(self.PREFIX)}
611+
612+
@cluster(num_nodes=6)
613+
@parametrize(num_roles=5000, members_per_role=1)
614+
@parametrize(num_roles=50, members_per_role=100)
615+
@parametrize(num_roles=5, members_per_role=1000)
616+
def test_role_sync_at_scale(self, num_roles: int, members_per_role: int) -> None:
617+
# create + update + delete each write ~1 controller record per role, on
618+
# both the source (seed/modify/delete) and the target (mirror), so each
619+
# controller log grows by ~3*num_roles over the lifecycle. Raise the
620+
# guard above its 1000 default to admit that while still catching runaway
621+
# writes.
622+
max_controller_records = 1000 + num_roles * 3
623+
self.redpanda.set_expected_controller_records(max_controller_records)
624+
self.source_cluster_service.set_expected_controller_records(
625+
max_controller_records
626+
)
627+
628+
first = f"{self.PREFIX}role-0"
629+
last = f"{self.PREFIX}role-{num_roles - 1}"
630+
mirror_timeout_sec = 240
631+
632+
# CREATE: seed the source, then mirror it to the destination.
633+
seed_start = time.time()
634+
expected = self._seed_source_roles(num_roles, members_per_role)
635+
self.logger.info(
636+
f"seeded {num_roles} source roles x {members_per_role} members "
637+
f"in {time.time() - seed_start:.1f}s"
638+
)
639+
create_start = time.time()
640+
self._create_link_with_role_sync()
641+
wait_until(
642+
lambda: self._synced_role_names() >= expected,
643+
timeout_sec=mirror_timeout_sec,
644+
backoff_sec=2,
645+
err_msg=f"{num_roles} roles did not fully mirror within "
646+
f"{mirror_timeout_sec}s",
647+
)
648+
self.logger.info(
649+
f"role sync created {num_roles} roles in {time.time() - create_start:.1f}s"
650+
)
651+
# Membership integrity at the range ends (a full per-role check would add
652+
# num_roles RPCs); the first and last seeded roles bound the range.
653+
for i in sorted({0, num_roles - 1}):
654+
name = f"{self.PREFIX}role-{i}"
655+
want = {f"u-{i}-{m}" for m in range(members_per_role)}
656+
assert self._dst_role_members(name) == want, (
657+
f"membership mismatch for {name} after create"
658+
)
659+
660+
# UPDATE: change every role's membership -- drop one member from each --
661+
# so all num_roles roles flow through the update apply loop.
662+
update_start = time.time()
663+
for i in range(num_roles):
664+
self._retry_transient(
665+
self._src.remove_role_members,
666+
role=f"{self.PREFIX}role-{i}",
667+
members=[_user(f"u-{i}-0")],
668+
)
669+
wait_until(
670+
lambda: (
671+
"u-0-0" not in self._dst_role_members(first)
672+
and f"u-{num_roles - 1}-0" not in self._dst_role_members(last)
673+
),
674+
timeout_sec=mirror_timeout_sec,
675+
backoff_sec=2,
676+
err_msg=f"membership update did not mirror for {num_roles} roles",
677+
)
678+
self.logger.info(
679+
f"role sync updated {num_roles} roles in {time.time() - update_start:.1f}s"
680+
)
681+
682+
# DELETE: drop every source role and confirm the destination is pruned.
683+
delete_start = time.time()
684+
for name in expected:
685+
self._retry_transient(self._src.delete_role, name, delete_acls=False)
686+
wait_until(
687+
lambda: len(self._synced_role_names()) == 0,
688+
timeout_sec=mirror_timeout_sec,
689+
backoff_sec=2,
690+
err_msg=f"{num_roles} roles were not pruned from the destination",
691+
)
692+
self.logger.info(
693+
f"role sync deleted {num_roles} roles in {time.time() - delete_start:.1f}s"
694+
)
695+
696+
# The full lifecycle ran fault-free -> the task settles ACTIVE.
697+
wait_until(
698+
lambda: self._roles_task_state() == shadow_link_pb2.TASK_STATE_ACTIVE,
699+
timeout_sec=30,
700+
backoff_sec=1,
701+
err_msg="roles task did not settle ACTIVE after the lifecycle",
702+
)

0 commit comments

Comments
 (0)