feat: configure peer relay port via GUI #53
Conversation
Signed-off-by: Derek Kaser <[email protected]>
WalkthroughAdds relay-server port configuration: frontend input and save action, backend handler for Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as Frontend (Tailscale.php)
participant Backend as Config.php
participant LocalAPI as LocalAPI
participant TS as Tailscale Daemon
User->>UI: enter port, click Save
UI->>UI: show spinner, disable controls
UI->>Backend: POST action='set-relay-port', port=value
Backend->>Backend: validate port (empty or 0–65535)
alt valid
Backend->>LocalAPI: set RelayServerPort preference
LocalAPI->>TS: apply/update setting
TS-->>LocalAPI: success
LocalAPI-->>Backend: success
Backend-->>UI: JSON success
UI->>Backend: request updated config
Backend->>LocalAPI: getPacketFilterRules()
LocalAPI-->>Backend: packet-filter rules
Backend-->>UI: config + relay approval/port
else invalid
Backend-->>UI: JSON error
UI->>UI: show error, re-enable controls
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php(1 hunks)src/usr/local/emhttp/plugins/tailscale/include/data/Config.php(2 hunks)src/usr/local/emhttp/plugins/tailscale/locales/en_US.json(3 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php(1 hunks)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/usr/local/emhttp/plugins/tailscale/include/data/Config.php (2)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (3)
getRelayServerPort(494-500)isApprovedPeerRelay(475-492)tr(66-69)src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
patchPref(124-131)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
getPacketFilterRules(104-107)
🪛 PHPMD (2.15.0)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php
481-481: Avoid unused local variables such as '$key'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (6)
src/usr/local/emhttp/plugins/tailscale/locales/en_US.json (1)
31-31: LGTM! Clean localization additions.The new localization keys follow existing naming conventions and are properly structured. The messages are clear and provide appropriate user guidance for the relay port feature.
Also applies to: 91-91, 107-108
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
494-500: LGTM! Clean type-safe implementation.The method correctly checks both existence and type of the RelayServerPort preference before returning. The union return type
int|falseprovides clear semantics for callers.src/usr/local/emhttp/plugins/tailscale/include/Pages/Tailscale.php (1)
124-129: LGTM! Consistent with established patterns.The function follows the same pattern as other feature toggle functions in this file (e.g.,
setFunnelPort(),setTailscaleExitNode()). The implementation correctly shows the spinner, disables controls during the operation, and refreshes the UI afterward.src/usr/local/emhttp/plugins/tailscale/include/data/Config.php (2)
102-119: LGTM! Well-designed UI with helpful feedback.The relay port UI implementation includes:
- Clear warning message when port is configured without ACL approval
- HTML5 validation with min/max constraints
- Disabled save button until user makes changes (good UX)
The UI properly integrates with the existing table structure and follows established patterns.
347-368: LGTM! Robust validation and error handling.The
set-relay-porthandler includes comprehensive validation:
- Allows empty string to disable relay (maps to null)
- Uses
ctype_digit()to ensure numeric input- Validates port range (0-65535)
- Logs changes for auditing
- Properly throws exceptions for invalid input
The implementation follows the same pattern as other action handlers and integrates well with existing error handling.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
104-107: Verify that using an undocumented debug endpoint is appropriate for this use case.The implementation follows the established pattern of other getter methods in this class. However, the
v0/debug-packet-filter-rulesendpoint lacks public documentation and is an internal debug endpoint in Tailscale's codebase, meaning it may change without notice. Confirm that using this endpoint is suitable for your needs and monitor Tailscale releases for any changes to its behavior or availability.
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php
Show resolved
Hide resolved
Signed-off-by: Derek Kaser <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (2)
477-481: Remove unnecessary array cast for clarity.Line 477 casts the result to
array, but the subsequent code accesses properties using object syntax ($rule->CapGrant). SincegetPacketFilterRules()returns\stdClass, remove the cast and iterate directly over the object properties.Apply this diff:
- $netmapRules = (array) $this->localAPI->getPacketFilterRules(); + $netmapRules = $this->localAPI->getPacketFilterRules(); // Parse the packet filter rules to see if peer relay is approved // TODO: Get a better way to do this from Tailscale - foreach ($netmapRules as $rule) { + foreach ((array) $netmapRules as $rule) {
484-484: Consider extracting the complex condition for readability.The deeply nested condition chain is difficult to parse. Consider extracting it into a helper method or breaking it into intermediate variables.
Example refactor:
private function hasRelayCapability($rule): bool { if (!isset($rule->CapGrant) || !is_array($rule->CapGrant)) { return false; } foreach ($rule->CapGrant as $capGrant) { if (is_object($capGrant) && isset($capGrant->CapMap) && is_object($capGrant->CapMap) && isset($capGrant->CapMap->{'tailscale.com/cap/relay'})) { return true; } } return false; } public function isApprovedPeerRelay(): bool { $netmapRules = (array) $this->localAPI->getPacketFilterRules(); foreach ($netmapRules as $rule) { if ($this->hasRelayCapability($rule)) { return true; } } return false; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/LocalAPI.php (1)
getPacketFilterRules(104-107)
🔇 Additional comments (1)
src/usr/local/php/unraid-tailscale-utils/unraid-tailscale-utils/Info.php (1)
494-500: The strict type checking is appropriate and well-founded.The
RelayServerPortis always set viaintval()(Config.php:357), which explicitly casts to integer before being patched to the API. When retrieved viajson_decode(), JSON integers are preserved as PHP integers in object properties. Theis_int()check ingetRelayServerPort()is correct and will reliably detect the expected type. No changes needed.
Summary by CodeRabbit