Skip to content

Conversation

@a-spiker
Copy link
Contributor

Role Name Validation Enhancement

Description

Added validation for role names to ensure they only contain allowed characters. This helps prevent potential issues with invalid role names in the system.

Changes

  • Added is_valid_role_name() function in lib/utils/util.py that validates role names against a regex pattern
  • Added comprehensive test cases in test/unit/utils/test_util.py

Validation Rules

A valid role name can only contain:

  • Latin lowercase and uppercase letters (a-z, A-Z)
  • Digits (0-9)
  • Underscores (_)
  • Hyphens (-)
  • Dollar signs ($)

Test Coverage

Added test cases for:

  • Valid role names (e.g., "valid_role", "Valid-Role", "role$123")
  • Invalid role names with:
    • Spaces
    • Special characters (@, ,, ;)
    • Empty strings
    • Complex invalid cases

Impact

This change helps maintain data integrity by ensuring role names follow the correct format before they are used in the system.

Note

I have added this change to manage acl create role command and not other commands like grant role or create user grant roles, where it should be assumed that the role is already present. It requires enforcement from Server.

* add is_valid_role_name function to validate role names against specified criteria
* update ManageACLCreateRoleController to return early if the role name is invalid
* add unit tests for is_valid_role_name to cover valid and invalid cases
@a-spiker a-spiker requested a review from sud82 June 16, 2025 03:23
… function

* update error message to specify invalid role name and illegal characters
* improve documentation with argument and return type descriptions
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.56%. Comparing base (2d600a7) to head (6f5851f).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
lib/live_cluster/manage_controller.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #357      +/-   ##
==========================================
- Coverage   68.58%   68.56%   -0.02%     
==========================================
  Files          92       92              
  Lines       21255    21274      +19     
==========================================
+ Hits        14577    14587      +10     
- Misses       6678     6687       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@a-spiker a-spiker marked this pull request as draft June 16, 2025 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants