registry
registry copied to clipboard
feat: add regex validation to server name in API types (#471)
Summary
Following @domdomegg's suggestion in #476, this PR started as adding regex validation to the server name field but evolved based on extensive reviewer feedback into a comprehensive validation improvement.
Changes
Initial Implementation
- Added pattern validation to the
Namefield inpkg/api/v0/types.go - Updated tests to expect HTTP 422 for validation errors
Based on Review Feedback
- Removed regex from API annotations - Moved validation to handler code to maintain HTTP 400 status (more familiar than 422) as suggested by @joelverhagen
-
Updated regex pattern - Changed from
^[^/]+/[^/]+$to the more specific^[a-zA-Z0-9.-]+/[a-zA-Z0-9._-]+$to match the schema definition -
Moved regex patterns to
utils.go- Centralized all validation regexes in one place as suggested by @Avish34 -
Added error constants to
constants.go- CreatedErrInvalidNamespaceCharactersandErrInvalidNameCharactersfor consistent error messages - Improved error messages - Now provides clearer messages like "server name must match pattern..." instead of generic validation errors
-
Removed redundant validation - Eliminated unnecessary
strings.Contains(name, "//")check that was already covered by slash count validation
Key Decisions
- Chose handler validation over schema validation to maintain HTTP 400 status and provide better error messages
- Kept case-sensitive validation allowing uppercase letters (discussion with @joelverhagen about potentially enforcing lowercase for future consideration)
- Centralized validation logic following the existing codebase patterns
Context
This PR demonstrates the value of code review - what started as a simple regex addition evolved into a better implementation that:
- Maintains backward compatibility with HTTP 400 status codes
- Provides clearer, more actionable error messages
- Follows the codebase's existing patterns for validation
- Makes the code more maintainable by centralizing regex patterns and error messages
Related to #471 Builds on #476
Testing
- All existing tests updated and passing
- Added comprehensive test cases for the new validation pattern
- Verified error messages are clear and helpful
- Tested various edge cases discovered during review
Note: Special thanks to @joelverhagen, @Avish34, and @domdomegg for their thorough reviews that significantly improved this implementation.