feat: add socket address uniqueness validation for tracker configuration #258
Conversation
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
- Add HealthCheckApiConfig domain type with bind_address field - Add HealthCheckApiSection DTO with validation (rejects port 0) - Default bind address: 127.0.0.1:1313 - Update TrackerConfig and TrackerSection with health_check_api field - Add comprehensive unit tests for validation logic - Update all test fixtures and E2E config generators - Regenerate environment-config.json schema - Fix all doc test examples The Health Check API is now fully configurable throughout the application with proper validation and comprehensive test coverage.
- Convert config.rs into folder module (config/) - Extract TrackerCoreConfig → config/core.rs (2 tests) - Extract UdpTrackerConfig → config/udp.rs (3 tests) - Extract HttpTrackerConfig → config/http.rs (3 tests) - Extract HttpApiConfig → config/http_api.rs (3 tests) - Extract HealthCheckApiConfig → config/health_check_api.rs (3 tests) - Keep TrackerConfig in config/mod.rs (3 integration tests) - Improve code organization and maintainability - All 1582 tests passing
- Convert core.rs into folder module (config/core/) - Move database module into config/core/database/ - Update imports to reflect new structure - Database types now exported through config module hierarchy - Improves logical grouping (database is part of core config) - All 1582 tests passing - All linters passing
…p system - Add BindingAddress value object combining SocketAddr with Protocol - Add TrackerConfig::validate() method to detect socket address conflicts - Add TrackerConfigError enum with DuplicateSocketAddress variant - Implement tiered help system (brief Display + detailed .help() method) - Integrate validation at DTO boundary (TrackerSection.to_tracker_config) - Add comprehensive unit tests for validation logic - Add integration tests for DTO validation Follows error handling conventions from docs/contributing/error-handling.md
- Extract collect_bindings() private method from validate() - Improves Single Responsibility Principle compliance - Makes validate() method more focused on conflict detection - Better separation of concerns: collection vs validation
- Extract check_for_conflicts() private method from validate() - Further improves Single Responsibility Principle - Now validate() is a clean orchestrator of two focused methods - Three-level separation: collect → check → report error
- Extract register_binding() helper to remove repeated HashMap operations - Extract register_trackers() to handle multiple tracker instances generically - Add HasBindAddress trait for uniform tracker handling - Service names now declared once at call site (DRY principle) - Reduces code from ~30 lines to ~15 lines in collect_bindings() - Improves testability with focused helper methods
Member
Author
|
ACK dc153f0 |
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.
Summary
Implements socket address uniqueness validation for tracker configuration as described in issue #255. This prevents configuration errors where multiple services attempt to bind to the same socket address with the same protocol, which would result in runtime failures.
Changes
Core Implementation
BindingAddress value object (
src/domain/tracker/binding_address.rs)SocketAddrwithProtocolfor type-safe binding representationHashandEqfor HashMap-based duplicate detectionTrackerConfig validation (
src/domain/tracker/config/mod.rs)validate()- Main validation orchestratorcollect_bindings()- Gathers all socket bindings with service namescheck_for_conflicts()- Detects duplicate bindingsregister_binding()/register_trackers()- DRY helper methodsHasBindAddresstrait for generic tracker handlingTrackerConfigError with tiered help system
.help()method for troubleshootingdocs/contributing/error-handling.mdIntegration
TrackerSection.to_tracker_config())CreateConfigErrorValidation Rules
✅ Rejects: Same protocol + same socket address
✅ Allows: Different protocols sharing same port
Manual E2E Testing
Tested with real environment configurations to verify production behavior:
Test Case 1: TCP Conflict ❌ (Correctly Rejected)
Configuration: HTTP Tracker and HTTP API both on
0.0.0.0:7070(TCP){ "http_trackers": [{"bind_address": "0.0.0.0:7070"}], "http_api": {"bind_address": "0.0.0.0:7070", "admin_token": "..."} }Result:
Test Case 2: UDP Conflict ❌ (Correctly Rejected)
Configuration: Two UDP trackers on same address
{ "udp_trackers": [ {"bind_address": "0.0.0.0:6969"}, {"bind_address": "0.0.0.0:6969"} ] }Result:
Test Case 3: Valid Configuration ✅ (Correctly Accepted)
Configuration: All services on unique addresses
{ "udp_trackers": [{"bind_address": "0.0.0.0:6969"}], "http_trackers": [{"bind_address": "0.0.0.0:7070"}], "http_api": {"bind_address": "0.0.0.0:1212"}, "health_check_api": {"bind_address": "127.0.0.1:1313"} }Result: ✅ Environment created successfully
Test Case 4: Cross-Protocol ✅ (Correctly Accepted)
Configuration: UDP and TCP sharing same port (different protocols)
{ "udp_trackers": [{"bind_address": "0.0.0.0:7070"}], "http_trackers": [{"bind_address": "0.0.0.0:7070"}] }Result: ✅ Environment created successfully (UDP and TCP use separate port spaces)
Automated Testing
Code Quality Improvements
This PR includes several refactoring commits that improve code quality:
Benefits:
Documentation
docs/external-issues/tracker/udp-tcp-port-sharing-allowed.mdRelated