[CORE-15198] security/role: Add group role member type#29217
Merged
nguyen-andrew merged 3 commits intoJan 12, 2026
Conversation
The original `role_store_test.cc` was a little complex, and a previous tree-wide change broke the original intent of some tests (e.g., the original `role_store_test` had a test for a "moved" role that was actually being copied). This commit refactors the role & role store tests to improve clarity and maintainability by minimizing each test to only validate a single behavior. Changes: - Extract role-specific tests into new role_test.cc file - Replace monolithic tests with focused, fixture-based tests - Use descriptive test names that clearly state expected behavior - Add detailed BOOST_CHECK_MESSAGE assertions Unit test coverage for roles/role store is maintained or improved while making the tests easier to understand, extend, and debug.
Adds role_member_type::group alongside the existing user type to support groups as role members. Includes the corresponding operator<< overload for string representation and updated related tests.
Maps principal_type::group to role_member_type::group in the member_type_for_principal_type function, allowing group principals to be converted to role members. Updates relevant tests to cover this new mapping.
nguyen-andrew
requested review from
IoannisRP and
Copilot
and removed request for
a team
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends Redpanda's internal role membership type system to support groups as role members, alongside the existing user support. This is preparatory infrastructure work that enables groups to be assigned to roles, though the admin API functionality to actually bind groups to roles is not yet exposed.
Changes:
- Added
groupas a newrole_member_typeenum value - Implemented formatting and conversion logic for group role members
- Added comprehensive unit tests for role member operations including group support
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/v/security/role.h | Adds group enum value to role_member_type and declares stream operator |
| src/v/security/role.cc | Implements group handling in principal type conversion and stream output |
| src/v/security/tests/role_test.cc | New test file covering role member creation, formatting, and type conversions for both users and groups |
| src/v/security/tests/role_store_test.cc | Refactors existing tests into fixture-based structure with improved clarity and coverage |
| src/v/security/tests/BUILD | Adds new role_test target and removes unused dependencies from role_store_test |
Comment on lines
+27
to
+30
| absl::node_hash_set<role_name> | ||
| make_role_name_set(const role_store::roles_range& range) { | ||
| return {range.begin(), range.end()}; | ||
| } |
There was a problem hiding this comment.
Add a docstring explaining the purpose of this helper function, as it is used throughout multiple test cases to convert ranges to sets for assertion checking.
Collaborator
CI test resultstest results on build#78845
|
IoannisRP
approved these changes
Jan 12, 2026
| BOOST_AUTO_TEST_CASE(role_format_includes_all_members) { | ||
| const role_member mem0{role_member_type::user, "member0"}; | ||
| const role_member mem1{role_member_type::user, "member1"}; | ||
| const role_member mem1{role_member_type::group, "group0"}; |
Contributor
There was a problem hiding this comment.
nit:
Instead of replacing member1 with group0, append group0 to the set.
Contributor
|
@nguyen-andrew |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds internal support for groups as role members in preparation for follow-on work to enable administrators to actually bind groups to roles.
The role system previously only supported users as role members. This PR extends the internal role membership type system to include groups, laying the groundwork for future admin API features that will allow administrators to assign groups to roles. While the admin API does not yet expose this functionality, the core role system can now represent and handle group memberships.
Backports Required
Release Notes