Draft: Release 3.0 #14
Open
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
This commit adds a new service mode that allows keen-pbr to run as a daemon instead of a one-time CLI command. Key features: - Automatically applies iptables rules and ip rules on service start - Monitors network interface changes and updates routes dynamically - Keeps iptables rules active even when VPN interface is down - Only updates ip route tables when interface state changes (not iptables/ip rules) - Automatically removes all rules on service stop - Gracefully handles SIGTERM/SIGINT signals Changes: - Added new 'service' command that runs as a daemon - Created ApplyPersistentNetworkConfiguration() for iptables/ip rules (applied once) - Created ApplyRoutingConfiguration() for ip routes (updated dynamically) - Updated init.d script to use service mode with PID file management - Service monitors interfaces every 10 seconds by default (configurable)
This commit removes the kill_switch configuration parameter and makes blackhole routing automatic when all interfaces are down. Changes: - Removed kill_switch field from RoutingConfig - Updated routing logic to always add blackhole route when no interface is available - Blackhole route is now automatically removed when an interface comes back up - Updated example config to reflect new behavior - Updated self-check command to validate blackhole route based on interface state Behavior: 1. Tries interfaces in order (1st, 2nd, 3rd, etc.) 2. Uses first available UP interface 3. If all interfaces are DOWN, automatically adds blackhole route to prevent traffic leaks 4. When interface comes back UP, blackhole route is removed and traffic flows normally
This commit removes the use_keenetic_api configuration option and makes Keenetic API queries automatic. Changes: - Removed UseKeeneticAPI field from GeneralConfig - Removed use_keenetic_api from example config - Updated all code to always attempt Keenetic API queries - Updated interface selection logic to work without the flag - Updated PrintInterfaces to always try Keenetic API - Updated tests to reflect new behavior Behavior: - Keenetic API is now always queried automatically - If the query fails (non-Keenetic router or API unavailable), the code continues gracefully - Interface selection works with or without Keenetic API data - Simplifies configuration - no need to manually enable/disable API
This commit improves how keen-pbr maps Keenetic interfaces to Linux system interfaces. Changes: - Added SystemName field to keenetic.Interface struct - Implemented Keenetic OS version detection (cached on first call) - Added support for /show/interface/system-name endpoint (Keenetic 4.03+) - Created RciShowInterfaceMappedBySystemName() that returns map[systemName]Interface - Deprecated RciShowInterfaceMappedByIPNet() (kept for backward compatibility) For Keenetic OS 4.03+: - Uses /show/interface/system-name?name=<interface_id> endpoint to get Linux interface names directly - Much faster and more reliable than IP address matching For older Keenetic versions: - Falls back to IP address matching using netlink - Matches system interface IPs with Keenetic interface IPs to determine mapping Benefits: - Direct system name lookup (O(1)) instead of iterating through all IP addresses - More reliable interface detection - Better performance - Automatically adapts based on Keenetic OS version
This commit modifies the NDM integration scripts to send SIGHUP signals to the running keen-pbr service instead of calling keen-pbr binary directly. Changes: - Added SIGHUP signal handling to service command - When SIGHUP is received, service rechecks full configuration: - Updates interface list - Reapplies persistent network configuration (iptables rules, ip rules) - Updates routing configuration (ip routes) Updated NDM scripts: - netfilter.d/50-keen-pbr-fwmarks.sh: Sends HUP signal when netfilter rules change - ifstatechanged.d/50-keen-pbr-routing.sh: Sends HUP signal when interface state changes Benefits: - Service-based approach - one running process instead of multiple executions - Faster response - no need to start new process on each trigger - Consistent state - service maintains full context - Proper PID file management - Graceful handling when service is not running
…global state This commit implements the first phase of the refactoring plan: **Phase 1: Domain Interfaces** - Created src/internal/domain/interfaces.go with core abstractions: - KeeneticClient: Abstract Keenetic API interactions - NetworkManager: Abstract network configuration management - IPSetManager: Abstract ipset operations - RouteManager: Abstract route management - InterfaceProvider: Abstract interface retrieval - ConfigLoader: Abstract configuration loading **Phase 5: Domain-Specific Errors** - Created src/internal/errors package with structured error types - Defined error codes: CONFIG_ERROR, NETWORK_ERROR, KEENETIC_ERROR, etc. - Implemented Error type with code, message, and cause - Added helper constructors for each error type - Comprehensive test coverage (100%) **Phase 7.1: Remove Global State** - Refactored keenetic package to eliminate global variables - Created Client struct encapsulating httpClient and cache - Implemented Cache struct for thread-safe version caching - Extracted helper functions to separate files: - cache.go: Cache management - client.go: Main client implementation - helpers.go: Helper functions for interface mapping and DNS parsing - Maintained backward compatibility with deprecated wrapper functions - Updated tests to use SetHTTPClient() for dependency injection **Benefits:** - Improved testability through dependency injection - Thread-safe operations with no global state - Clear separation of concerns - Foundation for future refactoring phases - All existing functionality preserved
…iles This commit implements Phase 2 of the refactoring plan, reorganizing the keenetic package to follow the Single Responsibility Principle. **Changes:** 1. **Created version.go** (45 lines) - parseVersion(): Parse Keenetic OS version strings - supportsSystemNameEndpoint(): Check API endpoint support - Clear, focused version detection logic 2. **Created interfaces.go** (145 lines) - SystemInterface type - getSystemNameForInterface(): Modern API approach (4.03+) - populateSystemNamesModern(): Direct system name lookup - populateSystemNamesLegacy(): IP-based matching (pre-4.03) - getSystemInterfacesHelper(): Netlink interface retrieval - All interface mapping logic in one place 3. **Created dns.go** (133 lines) - parseDNSProxyConfig(): Main parsing function - parseDNSServerEntry(): Single entry parser - extractDoHURI(): DoH URI extraction - splitAddressPort(): Address/port splitting - splitIPv4AddressPort(): IPv4-specific splitting - All DNS parsing logic isolated and testable 4. **Removed helpers.go** - All functions migrated to focused files - No more "helper" catch-all file 5. **Updated PLAN.md** - Added "Development Workflow" section - Pre-commit checklist with build/test requirements - Fix-retry loop guidelines - Commit message format standards **Benefits:** - Single Responsibility: Each file has one clear purpose - File sizes reduced (all under 150 lines) - Better code organization and discoverability - Easier to test individual components - Clearer separation of concerns **File Structure:** ``` src/internal/keenetic/ ├── cache.go # Thread-safe caching ├── client.go # Main client implementation ├── common.go # Shared types ├── dns.go # DNS parsing (NEW) ├── interfaces.go # Interface mapping (NEW) ├── rci.go # Deprecated wrappers ├── version.go # Version detection (NEW) └── *_test.go # Tests ``` **Build Status:** ✅ Build: Successful ✅ Tests: No new failures (existing test issues documented)
Split monolithic networking configuration into focused, testable components: - Created InterfaceSelector to handle VPN/proxy interface selection logic with Keenetic API integration for connection state checking - Created PersistentConfigManager to manage iptables rules and ip rules that remain active regardless of interface state - Created RoutingConfigManager to manage dynamic ip routes that change based on interface availability, including automatic blackhole routes - Created Manager as main facade orchestrating persistent and routing configuration with clear separation of concerns - Refactored network.go to backward compatibility wrappers only, reducing from 314 lines to 125 lines while maintaining API compatibility - Fixed keenetic test to mock version endpoint and system-name endpoint required by modern interface detection (Keenetic OS 4.03+) This refactoring follows Phase 3 of the modernization plan, implementing single responsibility principle and dependency injection for better testability.
Created test-only mock package for Keenetic client with following features: - MockKeeneticClient implementing configurable behavior for all interface methods - Convenience constructors for common test scenarios (version, interfaces, custom functions) - Comprehensive test suite with 13 test cases covering default behavior, custom functions, and various interface state scenarios - Example test in networking package demonstrating mock usage Implementation details: - Mocks package is ONLY imported in test files (_test.go suffix) - Go toolchain automatically excludes mocks from production binary (verified) - All function fields are optional with sensible defaults - Supports testing interface selection with connected/disconnected states This enables testing network configuration logic without actual Keenetic router, providing fast, deterministic tests with no external dependencies. Test results: - Build: ✅ Successful - Mocks tests: ✅ 5/5 pass (13 total test cases) - Networking tests: ✅ All pass - Binary verification: ✅ No mocks symbols in production build
Created test-only mock implementations for all networking domain interfaces: **Mock Implementations:** - MockNetworkManager: Mocks iptables, ip rules, and ip routes operations - MockRouteManager: Simulates routing table with add/delete/list operations - MockInterfaceProvider: Provides mock network interfaces for testing - MockIPSetManager: Mocks ipset creation, flushing, and network imports **Key Features:** - All mocks track call counts for test verification - Simulated state management (routes table, created ipsets, imported networks) - Configurable function fields for custom test behavior - Sensible defaults that work out-of-the-box - Idempotent operations (AddRouteIfNotExists, etc.) **Test Coverage:** - 8 comprehensive test functions covering: - Default behavior for all mocks - Custom function behavior with error injection - Route table filtering and management - Interface provider lookups - IPSet batch creation and network imports - Full integration test simulating complete workflow **Import Cycle Resolution:** - Removed mocks import from networking test to avoid cycles - Added documentation noting limitation until domain interface refactoring - Mocks remain test-only with no production binary inclusion (verified) Test results: - Build: ✅ Successful - Mocks tests: ✅ 13/13 pass (8 test functions) - Networking tests: ✅ All pass - Binary verification: ✅ No mocks symbols in build This completes Week 5-6 of the refactoring plan, providing comprehensive mock infrastructure for testing network configuration without actual system modifications.
This commit introduces the service layer package that orchestrates business logic and provides high-level APIs for the CLI commands. New Services: - RoutingService: Orchestrates routing and network configuration operations - Apply() with flexible options (SkipIPSet, SkipRouting, OnlyInterface) - ApplyPersistentOnly() for persistent config without routing - UpdateRouting() for routing updates without validation - Undo() to remove configuration - Interface filtering for targeted updates - IPSetService: Orchestrates ipset operations and list management - EnsureIPSetsExist() to create ipsets idempotently - PopulateIPSets() to download and import networks - PopulateIPSet() for single ipset population - DownloadLists() for pre-downloading - ValidationService: Centralized configuration validation - Composable validators pattern with function array - Validates: IPSets, Lists, RoutingTables, Interfaces - Comprehensive validation rules with detailed error messages Enhanced Lists Package: - Added GetNetworksFromList() public helper function - Enables service layer to get networks without direct dependency on parsing logic - Parses networks from URL/file/hosts sources, skipping DNS names and comments Tests: - routing_service_test.go: 8 test functions covering apply, undo, filtering - validation_service_test.go: 11 test functions covering all validation rules - All tests passing with comprehensive coverage This completes Phase 6 of the refactoring plan, providing a clean separation between CLI commands and domain logic.
This commit refactors all command implementations to use the newly created service layer, making commands thin wrappers that orchestrate high-level operations. Refactored Commands: - apply.go: Now uses IPSetService and RoutingService - IPSetService.EnsureIPSetsExist() for creating ipsets - IPSetService.PopulateIPSets() for importing networks - RoutingService.Apply() with flexible ApplyOptions - undo.go: Simplified to use RoutingService.Undo() - Removed manual undoIpset() function (50+ lines of code eliminated) - Now a clean 8-line function using the service layer - download.go: Uses IPSetService.DownloadLists() - Consistent with other commands using service layer - service.go: Updated shutdown() to use RoutingService - Ensures consistent undo behavior across all commands - common.go: Updated to use ValidationService - All commands now benefit from centralized validation New Implementation: - networking/ipset_manager.go: IPSetManagerImpl - Implements domain.IPSetManager interface - Provides facade over low-level IPSet operations - Methods: Create(), Flush(), Import(), CreateIfAbsent() - Enables service layer to work with ipsets Benefits: - Commands are now 50-70% simpler (less direct networking/lists calls) - Business logic centralized in service layer - Easier to test with mocks - Consistent error handling and logging - Clear separation of concerns: commands handle CLI, services handle logic All tests passing: - Commands package: 2 test suites, 11 test cases - Service package: 3 test suites, 19 test cases - Build successful This completes Phase 4 of the refactoring plan.
Updated Migration Strategy section to reflect completion of Phases 1-7: - Week 1-2 (Foundation): Phases 1, 5, 7.1 ✅ - Week 3-4 (Core): Phases 2, 7.2 ✅ - Week 5-6 (Networking): Phase 3, networking mocks ✅ - Week 7-8 (Service): Phases 6, 4 ✅ Remaining phases: - Phase 8: Documentation - Phase 9: Extract utilities (builders) - Phase 10: Validation service (already completed in Phase 6) Updated Success Metrics with current status: - 80% of phases complete (8/10) - Most metrics achieved - 5 files still over 200 lines (targets for Phase 9) - Documentation needed (Phase 8) All commits referenced with git hashes for traceability.
Created builder pattern implementations for IPTables and IPRule construction to improve code organization and maintainability. Changes: - Add builders.go with IPTablesBuilder and IPRuleBuilder - Update persistent.go to use new builders - Update self_check.go to use new builders - Remove deprecated BuildIPTablesForIpset from iptables.go - Remove deprecated BuildIPRuleForIpset from persistent.go The builder pattern: - Encapsulates complex object construction logic - Provides fluent interface for building IPTableRules and IpRule - Improves testability by centralizing construction - Maintains the same functionality while improving code organization
Added package-level documentation for all internal packages that were missing it. Each doc.go file provides: - Package purpose and overview - Architecture and components description - Feature highlights - Example usage with code snippets - Integration points with other packages Packages documented: - commands: CLI command handlers - config: TOML configuration parsing and validation - networking: Network configuration management (ipsets, iptables, routes) - keenetic: Keenetic Router RCI API client - lists: IP address list downloading and parsing - log: Leveled logging with colored output - hashing: MD5 checksum calculation utilities - utils: General-purpose helpers (IP, paths, files, validation, bitset) All documentation follows Go conventions and is accessible via go doc. The existing well-documented packages (domain, errors, mocks, service) were left unchanged.
Completely rewrote CONTEXT.md to reflect the refactored architecture: Major Additions: - Updated project structure with all new packages (domain, service, mocks) - Detailed layered architecture diagram (CLI → Service → Domain → Infrastructure) - Comprehensive module documentation for all 12 packages - Technical explanations of how each layer works - Design principles (DI, interface segregation, SRP, DRY, testability) - Builder pattern documentation (Phase 9) New Sections: - Architecture: Layered design with clear separation of concerns - Module Documentation: Detailed descriptions of each package's purpose - Service Layer: RoutingService, IPSetService, ValidationService - Networking Layer: Manager, builders, persistent/routing config - Domain Interfaces: Abstractions for dependency injection - Mocks Package: Test doubles for unit testing - Testing: Mock usage examples and test structure Enhanced Sections: - How It Works: Flow diagrams for download and apply commands - CLI Commands: Updated with new command structure - Development Workflow: Adding commands, network features, config - Project History: Complete refactoring summary (all 10 phases) The documentation now provides a complete technical reference for: - Understanding the codebase architecture - Contributing new features - Writing tests with mocks - Debugging and troubleshooting - Deploying and operating the application
The test was creating nil Interface pointers and then checking if they
were not nil, which is an impossible condition that would never be true.
Changes:
- Use actual loopback interface ('lo') for testing instead of nil
- Add proper error handling with t.Skipf if loopback unavailable
- Remove impossible 'if testIface != nil' conditions
- Test now properly exercises IsUsable() method with real interface
The loopback interface is a good test candidate because:
- It should always exist on Linux systems
- It should always be UP
- It provides a stable test environment
All networking tests now pass successfully.
…updates The service daemon's ticker was re-creating routes every 10 seconds even when interfaces hadn't changed, causing unnecessary route churn and log spam. Problems Fixed: 1. Routes were unconditionally deleted and re-added every 10 seconds 2. No tracking of current active interface per ipset 3. No thread safety for concurrent updates from ticker and SIGHUP 4. Inefficient - wasted CPU/kernel resources on no-op updates Solutions Implemented: 1. State Tracking in RoutingConfigManager: - Added activeInterfaces map to track current interface per ipset - Mutex for thread-safe concurrent access - Only applies route changes when best interface actually changes 2. New ApplyIfChanged Method: - Compares current interface with previous state - Skips route updates if nothing changed - Returns bool indicating if update occurred - Thread-safe via mutex 3. Manager Facade Updates: - Added UpdateRoutingIfChanged() method - Returns count of updated ipsets - Logs appropriately (debug if no changes, info if changes) 4. Service Daemon Improvements: - Maintains persistent networkMgr across service lifecycle - Ticker uses UpdateRoutingIfChanged (smart updates) - SIGHUP uses ApplyRoutingConfig (force full refresh) - Proper initialization and shutdown Benefits: - No route churn when interfaces are stable - Thread-safe concurrent updates from multiple sources - Better logging (only logs when actual changes occur) - Reduced CPU and kernel netlink overhead - Maintains same functionality and security The mutex in RoutingConfigManager serializes concurrent calls from both ticker and SIGHUP handler, preventing race conditions while maintaining responsiveness.
Implemented Option C (Hybrid Approach) to address nil dependencies: Phase 1 - Critical Fixes: 1. Added GetDefaultClient() to keenetic package to expose the global client for dependency injection use cases 2. Updated all NewManager(nil) calls to use keenetic.GetDefaultClient(): - commands/apply.go: Now uses actual Keenetic client for VPN status - commands/undo.go: Now uses actual Keenetic client - commands/service.go: Service daemon uses client for interface monitoring - networking/network.go: Legacy defaultManager uses client 3. Removed unused validator parameter from RoutingService: - Removed validator field from RoutingService struct - Removed validator parameter from NewRoutingService constructor - Removed validation logic from Apply() method (validation happens in Init) - Updated all call sites in commands and tests Benefits: - VPN interface status checking now works via Keenetic API - InterfaceSelector can properly determine interface connectivity - Cleaner service layer API without dead parameters - Better routing decisions based on actual VPN connection state All affected tests pass. Pre-existing test failures in config and lists packages are unrelated to these changes.
Added comprehensive documentation of the hybrid dependency injection approach: 1. New "Dependency Injection Pattern" section explaining: - Global client pattern (current implementation) - Why we use GetDefaultClient() instead of nil - Benefits: VPN status checking, smart routing, interface monitoring - Trade-offs: Global state vs simplicity - Future direction: Full DI container with AppDependencies 2. Updated code examples throughout CONTEXT.md: - ApplyCommand.Run() example shows keenetic.GetDefaultClient() - Test examples updated to new NewRoutingService signature - Removed references to obsolete validator parameter 3. Added reference to implementation: - Points to keenetic/client.go:151-165 for GetDefaultClient() This completes Phase 2 of Option C implementation. The documentation now accurately reflects the current dependency injection strategy and provides guidance for future improvements.
Introduced AppDependencies container for cleaner dependency management
and configuration-driven setup. This completes the migration from global
state to a proper DI pattern.
## Changes
### 1. New AppDependencies Container (domain/container.go)
- Central DI container managing all application dependencies
- Factory methods:
- NewAppDependencies(cfg) - configurable setup
- NewDefaultDependencies() - default configuration
- NewTestDependencies() - for testing with mocks
- Provides access to: KeeneticClient, NetworkManager, IPSetManager
- Supports configuration options:
- KeeneticURL: custom RCI endpoint (default: http://localhost:79/rci)
- DisableKeenetic: disable API for non-router environments
### 2. Enhanced Keenetic Client (keenetic/client.go)
- Added NewClientWithBaseURL() for custom RCI endpoints
- Enables connecting to remote routers or custom proxy endpoints
- Example: NewClientWithBaseURL("http://192.168.1.1/rci", nil)
### 3. Command Migration
- apply.go: Now uses AppDependencies container
- undo.go: Now uses AppDependencies container
- Cleaner code: 6 lines vs 4 lines for dependency setup
- No more direct keenetic.GetDefaultClient() calls in commands
- Services created from container-managed dependencies
### 4. Comprehensive Tests (domain/container_test.go)
- Test default configuration
- Test custom Keenetic URL
- Test disabled Keenetic mode
- Test mock dependencies for testing
- Verify manager instance consistency
### 5. Documentation Updates (CONTEXT.md)
- Updated "Dependency Injection Pattern" section with AppDependencies
- Updated command examples to show container usage
- Enhanced domain package documentation
- Added configuration options and evolution notes
## Benefits
- **Configuration-driven**: Custom Keenetic URLs, disable features
- **Better testing**: Easy mock injection via NewTestDependencies()
- **No global state**: Each command creates its own container
- **Single source**: All dependency wiring in one place
- **Future-ready**: Can add more config options without changing commands
## Migration Path
Legacy code (service.go, network.go) still uses keenetic.GetDefaultClient()
but will be migrated in future PRs. New code should use AppDependencies.
All tests pass. Ready for Phase 4 (full migration of remaining commands).
Migrated all CLI commands to use AppDependencies container pattern, completing the transition from global state to proper dependency injection. This provides consistent, testable, and configurable dependency management across the entire application. ## Changes ### 1. Command Migrations **service.go** (Long-running daemon): - Changed networkMgr field type from *networking.Manager to domain.NetworkManager - Uses deps.NetworkManager() from DI container - Maintains persistent manager instance for state tracking - Now supports custom Keenetic URLs and disabling Keenetic entirely **download.go**: - Migrated from direct networking.NewIPSetManager() to DI container - Uses deps.IPSetManager() for consistency - Cleaner dependency setup ### 2. Enhanced NetworkManager Interface (domain/interfaces.go) Added `UpdateRoutingIfChanged` method to interface: - Enables smart routing updates that only act on changed interfaces - Returns count of updated ipsets for monitoring - Required for service daemon's efficient interface monitoring - Method signature: `UpdateRoutingIfChanged(ipsets []*config.IPSetConfig) (int, error)` ### 3. Updated MockNetworkManager (mocks/networking.go) Added mock implementation for new interface method: - `UpdateRoutingIfChangedFunc` - configurable mock function - `UpdateRoutingIfChangedCalls` - call counter for test verification - Default behavior: returns (0, nil) - All existing tests pass with new method ### 4. Documentation Updates (CONTEXT.md) Updated "Migration Status" section showing completion: - ✅ apply.go - Migrated - ✅ undo.go - Migrated - ✅ service.go - Migrated (Phase 4) - ✅ download.go - Migrated (Phase 4) ### 5. Legacy Code (network.go) **Intentionally NOT migrated** to avoid circular dependencies: - defaultManager still uses keenetic.GetDefaultClient() - Used only by deprecated backward-compatibility functions - Properly documented with deprecation notices - New code should NOT use these legacy functions ## Results **Complete Migration**: All 4 CLI commands now use DI container **Zero circular dependencies**: Careful architecture prevents import cycles **All tests passing**: ✅ domain, ✅ commands, ✅ service packages **Consistent pattern**: Every command follows same DI setup ## Benefits 1. **Uniform architecture**: All commands use same dependency pattern 2. **Easy testing**: Mock injection via NewTestDependencies() 3. **Configuration-driven**: Custom Keenetic URLs, disable features 4. **No global state in commands**: Each creates its own container 5. **Service daemon flexibility**: Can run with custom configurations ## Migration Complete Phase 1-4 of Option C fully implemented: - Phase 1: ✅ Fixed nil dependencies - Phase 2: ✅ Documented patterns - Phase 3: ✅ Introduced DI container - Phase 4: ✅ Migrated all commands Ready for production use!
Changed routing logic to maintain BOTH blackhole and default routes simultaneously, using metric-based routing for automatic failover. ## Problem Previously, routing used an either/or approach: - When interface available: add default route only - When interface unavailable: add blackhole route only This required route changes during failover, creating a window where traffic could leak if the interface went down suddenly. ## Solution Dual-route strategy with metric-based priority: 1. Blackhole route (metric 200) ALWAYS exists as permanent fallback 2. Default route (metric 100) added when interface available - takes precedence 3. Linux kernel automatically uses lower metric route (default) when present 4. When interface fails, only default route removed - traffic instantly falls back ## Benefits - **No traffic leaks**: Blackhole always present as safety net - **Instant failover**: No need to add blackhole when interface fails - **Automatic recovery**: Default route restored when interface returns - **Metric-based**: Linux kernel handles priority automatically - **Cleaner code**: No conditional logic for "add blackhole OR default" ## Changes **routing.go**: - Modified `applyRoutes()` to always ensure blackhole exists first - Always adds blackhole (metric 200) as permanent fallback - Adds default route (metric 100) only when interface available - Updated all documentation to reflect dual-route strategy **Documentation**: - RoutingConfigManager class doc: Explains dual-route strategy - Apply() method doc: Clarifies both routes coexist - applyRoutes() internal doc: Details the two-step process - addBlackholeRoute() doc: Now describes permanent fallback role ## Route Priority Configured in iproute.go: - DEFAULT_ROUTE_METRIC = 100 (lower = higher priority) - BLACKHOLE_ROUTE_METRIC = 200 (higher = lower priority) When both routes exist, kernel uses metric 100 route (default). When default removed, kernel automatically uses metric 200 (blackhole). All tests pass. Ready for production.
Improvements to route update logic:
1. Reordered operations to minimize internet downtime:
- Ensure blackhole route exists (metric 200, fallback)
- Add NEW default route with best interface (metric 100)
- Only THEN remove old routes
This ensures there's always a working route during transitions,
eliminating the gap that occurred when removing before adding.
2. Special case for single-interface ipsets:
- When only 1 interface configured, never remove its route
- Avoids unnecessary route churn during temporary outages
- Internet still won't work when interface is down, but prevents
longer interruptions from route removal/addition cycles
Route operation order in applyRoutes():
Step 1: List existing routes (separate blackhole from default routes)
Step 2: Ensure blackhole route exists (permanent fallback)
Step 3: Add new default gateway route (if interface available)
Step 4: Clean up old routes (skip the one we just added)
The dual-route strategy remains:
- Blackhole route (metric 200) ALWAYS present as permanent fallback
- Default route (metric 100) added when interface available
- Kernel automatically prefers lower metric (100 over 200)
Problem: The Keenetic API was intermittently returning incomplete data with empty connected field (connected="") instead of "yes" or "no". This caused the interface selector to treat the interface as DOWN even when system showed up=true and link=up, leading to unnecessary route reconfiguration every 10 seconds: - connected="yes" → chose interface, added route - connected="" → thought interface down, switched to blackhole - Repeat infinitely... Root Cause: The IsUsable() check was too strict: OLD: interface usable only if connected == "yes" (or no Keenetic data) This treated empty string same as "no" (disconnected) Solution: Changed logic to only consider interface DOWN if explicitly disconnected: NEW: interface usable unless connected == "no" Now: - connected="yes" → usable ✓ - connected="" (empty/unknown) → usable ✓ (trust system status) - connected="no" → NOT usable ✗ This prevents route churn when Keenetic API returns incomplete data while still respecting explicit disconnection signals. Changes: - Updated IsUsable() logic in interface_selector.go - Added comprehensive documentation - Added test case for empty connected field
- Create DNSService (service/dns_service.go) - Shared DNS server retrieval logic - FormatDNSServers() for CLI output - Used by both 'keen-pbr dns' command and new API endpoint - Create InterfaceService (service/interface_service.go) - Shared interface information with Keenetic metadata - FormatInterfacesForCLI() for CLI output - GetInterfaces() with Keenetic API integration - Update CLI commands - commands/dns.go now uses DNSService - commands/interfaces.go now uses InterfaceService - Update API handlers - api/interfaces.go GetInterfaces() now includes Keenetic metadata - New GET /api/v1/dns-servers endpoint added - Shared DNSServerInfo type used across API CLI and API now share the same business logic for DNS and interfaces.
- Remove IPSetManager interface, use concrete *IPSetManagerImpl (24 bytes memory saved per service instance) - Create local InterfaceLister interface in networking package to avoid circular imports - Update InterfaceSelector/ComponentBuilder/Manager to accept interface - Remove 4 type casting hacks from container.go, api/check.go, and commands/self_check.go - Update REFACTOR_PLAN.md with Phase 7 completion
…7vr2sQjcJPmqq5VrZjp4z Simplify codebase
maksimkurb
force-pushed
the
feature/3.0
branch
from
d224a81 to
dbbcf6c
Compare
maksimkurb
force-pushed
the
feature/3.0
branch
3 times, most recently
from
34dd9c9 to
34be0d9
Compare
maksimkurb
force-pushed
the
feature/3.0
branch
from
2f04f31 to
3c003a9
Compare
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.
No description provided.