Make always use DERP option configurable#592
Conversation
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
lmagyar
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
Fix all issues with AI Agents 🤖
In @tailscale/translations/en.yaml:
- Around line 37-42: Correct the typo in the translation entry for the
always_use_derp option: change the value of the name field from "Alwas use DERP
connection" to "Always use DERP connection" so the label reads correctly; update
only the name string under the always_use_derp key (leave description
unchanged).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
tailscale/DOCS.mdtailscale/config.yamltailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/usr/bin/subnet-routestailscale/translations/en.yaml
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:30-37
Timestamp: 2025-09-16T23:47:20.987Z
Learning: In the hassio-addons/addon-tailscale project, the advertise_routes configuration uses `local_subnets` (with underscore) as the default value, and the runtime regex patterns use `local.subnets` (with dot) which correctly matches the underscore variant because '.' in regex matches any single character including '_'.
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 509
File: tailscale/config.yaml:0-0
Timestamp: 2025-09-17T23:53:19.467Z
Learning: In the Tailscale add-on config.yaml, there is an intentional naming mismatch between map entries and schema field names: map entries use actual Home Assistant directory names (like `homeassistant_config`, `all_addon_configs`) while schema fields use user-friendly names (like `config`, `addon_configs`). This allows correct directory mapping while presenting cleaner option names in the UI.
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 419
File: tailscale/rootfs/command/with-contenv-merge:8-12
Timestamp: 2024-11-15T17:38:56.576Z
Learning: The script `with-contenv-merge` in `tailscale/rootfs/command/` is directly copied from s6 and should not be modified.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/rootfs/usr/bin/subnet-routestailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
📚 Learning: 2025-11-06T10:55:55.062Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 566
File: tailscale/rootfs/usr/bin/subnet-routes:1-2
Timestamp: 2025-11-06T10:55:55.062Z
Learning: In the hassio-addons/addon-tailscale project, utility scripts in `/usr/bin/` (like `subnet-routes`, `protect-subnet-routes`, `unprotect-subnet-routes`) are invoked from s6-overlay service scripts or NetworkManager dispatchers that already use `#!/command/with-contenv bashio`. These utilities inherit the environment from their parent processes and therefore use `#!/usr/bin/env bashio` rather than `#!/command/with-contenv bashio`.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/rootfs/usr/bin/subnet-routestailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
Repo: hassio-addons/addon-tailscale PR: 407
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/init-nginx/run:24-29
Timestamp: 2024-10-09T10:32:14.110Z
Learning: In the Tailscale add-on for Home Assistant, the `tailscale/rootfs` directory is copied into the container's root directory via the Dockerfile, ensuring that files like `/etc/nginx/templates/homeassistant.gtpl` are present at runtime.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/DOCS.md
📚 Learning: 2025-09-17T23:53:19.467Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 509
File: tailscale/config.yaml:0-0
Timestamp: 2025-09-17T23:53:19.467Z
Learning: In the Tailscale add-on config.yaml, there is an intentional naming mismatch between map entries and schema field names: map entries use actual Home Assistant directory names (like `homeassistant_config`, `all_addon_configs`) while schema fields use user-friendly names (like `config`, `addon_configs`). This allows correct directory mapping while presenting cleaner option names in the UI.
Applied to files:
tailscale/config.yamltailscale/DOCS.md
📚 Learning: 2025-11-23T00:06:06.013Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:29-30
Timestamp: 2025-11-23T00:06:06.013Z
Learning: In the Home Assistant Tailscale add-on (hassio-addons/addon-tailscale), userspace_networking defaults to true because TUN networking can cause problems and is not needed for basic Home Assistant accessibility. This is an intentional deviation from stock Tailscale's default kernel/TUN networking.
Applied to files:
tailscale/config.yamltailscale/translations/en.yamltailscale/DOCS.mdtailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
📚 Learning: 2025-09-16T23:47:20.987Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 541
File: tailscale/config.yaml:30-37
Timestamp: 2025-09-16T23:47:20.987Z
Learning: In the hassio-addons/addon-tailscale project, the advertise_routes configuration uses `local_subnets` (with underscore) as the default value, and the runtime regex patterns use `local.subnets` (with dot) which correctly matches the underscore variant because '.' in regex matches any single character including '_'.
Applied to files:
tailscale/rootfs/usr/bin/subnet-routestailscale/DOCS.md
📚 Learning: 2025-10-29T20:47:02.818Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 562
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run:120-128
Timestamp: 2025-10-29T20:47:02.818Z
Learning: In the hassio-addons/addon-tailscale project, when checking for IPv6 addresses in bash scripts, use the regex format `if [[ "${variable}" =~ .*:.* ]]; then` for consistency with other IPv6 tests in the codebase.
Applied to files:
tailscale/rootfs/usr/bin/subnet-routestailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
📚 Learning: 2025-07-28T00:02:02.296Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 445
File: tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh:24-38
Timestamp: 2025-07-28T00:02:02.296Z
Learning: In Home Assistant add-ons, mandatory configuration options are automatically filled with default values by the supervisor, so there's no need to guard against overwriting user-configured values during migration since the option will always have a value.
Applied to files:
tailscale/DOCS.md
📚 Learning: 2025-06-11T20:37:01.152Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 455
File: tailscale/rootfs/usr/bin/magicdns-ingress-proxy-forwarding:100-106
Timestamp: 2025-06-11T20:37:01.152Z
Learning: In the Home Assistant Tailscale add-on, DNATing DNS packets to port 0 (e.g. `--to-destination 127.0.0.1:0`) is an intentional technique to drop traffic. iptables accepts port 0, rewrites the destination port to 0, and the packet is rejected by the stack, effectively discarding it. This is valid and should not be flagged as an error in future reviews.
Applied to files:
tailscale/DOCS.md
🔇 Additional comments (33)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run (2)
26-29: LGTM - More explicit configuration check.The change from implicit default handling to an explicit
truecheck improves clarity and aligns with the PR's goal of making configuration options explicit.
31-35: LGTM - Correct implementation of always_use_derp feature.The implementation correctly:
- Checks the configuration option
- Exports the appropriate Tailscale environment variable
TS_DEBUG_ALWAYS_USE_DERP- Notifies the user about the routing behavior change
tailscale/translations/en.yaml (1)
8-110: LGTM - Improved documentation clarity.The addition of explicit default state descriptions ("This option is enabled/disabled by default") across all configuration options improves user understanding. The updated
advertise_routesdescription correctly documents thelocal_subnetsfunctionality.tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh (5)
61-66: LGTM - More explicit configuration check.The change to explicitly check for
userspace_networkingbeingtrueimproves code clarity and consistency across the codebase.
68-72: LGTM - Improved precision for local_subnets handling.The change from checking mere existence of
advertise_routesto checking for the specificlocal_subnetsvalue is more precise and aligns with the new sentinel-based approach for route handling.
74-77: LGTM - Consistent with explicit configuration pattern.
79-82: LGTM - Consistent with explicit configuration pattern.
89-92: LGTM - More precise service disabling logic.The change to check for an explicit
'disabled'value rather than absence ensures the service is only disabled when intentionally configured to be disabled.tailscale/rootfs/usr/bin/subnet-routes (3)
27-48: LGTM - Correct implementation of local_subnets sentinel handling.The logic correctly:
- Detects the
local_subnetssentinel value- Recursively calls
subnet-routes localto collect actual subnet routes- Warns users if no local subnets are found
- Falls back to using configured addresses otherwise
The recursive call is safe (no risk of infinite recursion) and the exact pattern match
^local_subnets$is appropriate.
50-70: LGTM - Proper filtering of link-local addresses and forwarding checks.The implementation correctly:
- Filters out IPv6 link-local addresses (fe80::/10)
- Filters out IPv4 link-local addresses (169.254.0.0/16)
- Verifies IP forwarding is enabled for the appropriate address family before advertising routes
- Uses the consistent IPv6 detection pattern
.*:.*as per codebase conventions
72-77: LGTM - Clear deduplication and caching logic.The comment update accurately describes the deduplication process, and the implementation correctly uses
sort -uto remove duplicates before caching.tailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/run (1)
72-72: Theshare_on_portconfiguration already has a default value defined inconfig.yaml(line 39:share_on_port: 443). The removal of the inline default fallback is safe because the configuration system provides the default value automatically. No action is needed.Likely an incorrect or invalid review comment.
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (12)
19-24: LGTM! Explicit boolean handling is correct.The refactoring from implicit defaults to explicit true/false checks for
accept_dnsimproves clarity and ensures consistent behavior regardless of how bashio handles config defaults.
26-31: LGTM! Consistent explicit boolean pattern.The
accept_routeshandling follows the same clear pattern asaccept_dns, ensuring predictable behavior.
34-38: LGTM! Correct mutual exclusivity check.The validation correctly prevents advertising as an exit node while simultaneously using another exit node, which would be a configuration error.
40-45: LGTM! Consistent pattern for exit node advertisement.The explicit boolean handling for
advertise_exit_nodeis correct and consistent.
47-55: LGTM! Explicit exit node configuration is correct.The logic properly handles both cases:
- When an exit node is specified: configures it and enables LAN access (required for internal hassio/docker network access)
- When not specified: explicitly clears the exit node and disables LAN access
This ensures deterministic behavior on every run.
57-62: LGTM! Correct app connector advertisement handling.The explicit boolean pattern for
advertise_connectoris consistent with the refactoring approach.
65-66: LGTM! Unconditional login_server configuration is correct.Since
login_servernow has a mandatory default value in config.yaml, it's correct to always retrieve and apply it without conditional checks.
68-73: LGTM! Explicit stateful filtering configuration.The boolean handling for
stateful_filteringfollows the consistent pattern.
75-80: LGTM! Explicit SNAT configuration is correct.The boolean handling for
snat_subnet_routesmaintains the consistent pattern throughout the file.
83-83: LGTM! Simplified tags retrieval.The direct jq path expression for tags is cleaner and functionally equivalent.
147-148: LGTM! Correct condition for colliding routes check.Checking for colliding routes only when
accept_routesis true makes logical sense—if routes aren't being accepted, there's no possibility of collision.
170-170: LGTM! Consistent boolean check for userspace networking notice.The explicit true check for
userspace_networkingaligns with the refactoring pattern.tailscale/config.yaml (2)
28-44: LGTM! Default values align with documented behavior.The options block correctly defines defaults that match the documentation:
- Boolean options have appropriate true/false defaults
advertise_exit_node: trueadvertises the capability but still requires admin console approval to function- New
always_use_derp: falseoption is disabled by default as documentedadvertise_routes: [local_subnets]matches the documented default behaviorAll defaults are consistent with the documentation in DOCS.md.
45-63: LGTM! Schema definitions are correct and properly constrained.The schema block properly validates all configuration options:
- Boolean types are non-nullable as intended
advertise_routespattern correctly acceptslocal_subnetsor valid CIDR notation (based on learnings, the regex dot matches underscore)exit_nodeis correctly optional with comprehensive pattern matchingshare_on_portis correctly restricted to the three ports allowed by Tailscale (443, 8443, 10000)tagspattern enforces valid Tailscale tag format- All enum-style options use appropriate list() constraints
tailscale/DOCS.md (7)
43-64: LGTM! Clear configuration instructions.The updated instructions correctly describe the workflow where the add-on advertises capabilities by default, which can then be enabled through the Tailscale admin console. The steps are clear and actionable.
66-88: LGTM! YAML example accurately reflects defaults.The configuration example correctly shows all options with their default values, including the new
always_use_derp: falseoption andlocal_subnetsin the advertise_routes list.
101-101: LGTM! Consistent default statements.The "This option is enabled by default" statements for
accept_dns,accept_routes,advertise_exit_node, andadvertise_connectorall correctly match theirtruedefaults in config.yaml.Also applies to: 115-115, 126-126, 146-146
161-162: LGTM! Correct default behavior documented.The documentation correctly states that advertise_routes includes
local_subnetsby default, matching the config.yaml default.
235-235: LGTM! All default statements are accurate.The default statements throughout the documentation correctly reflect the values in config.yaml:
share_homeassistant: disabled by default ✓share_on_port: 443 by default ✓snat_subnet_routes: enabled by default ✓stateful_filtering: disabled by default ✓taildrop: enabled by default ✓userspace_networking: enabled by default ✓Also applies to: 311-311, 318-318, 337-337, 352-352, 362-362
490-490: LGTM! Correct link reference added.The new
[tailscale_machines]link correctly points to the Tailscale admin console machines page and is properly referenced earlier in the document at line 56.
164-177: Thealways_use_derpconfiguration is properly implemented in the tailscaled startup script.The verification confirms that the documentation in DOCS.md is accurate and the implementation is complete:
- The
always_use_derpoption is read from the configuration intailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run(lines 31-34)- When enabled, it correctly exports
TS_DEBUG_ALWAYS_USE_DERP=trueto the tailscaled process- Appropriate logging is in place to notify users when DERP routing is active
- The default value (
false) in config.yaml matches the documentationThe implementation properly integrates the configuration option throughout the addon (config schema, startup script, documentation, and translations).
lmagyar
force-pushed
the
pr-always-use-derp
branch
4 times, most recently
from
794ebe4 to
d744df2
Compare
lmagyar
force-pushed
the
pr-always-use-derp
branch
from
d744df2 to
f5a96b2
Compare
3 participants
Proposed Changes
Handles a maddening issue, when connections to Home Assistant are constantly dropping, while the device is always ping-able.
The root cause can be ISP routing misconfiguration (dropping certain UDP packets).
My hunch is that this issue is more regular, but nobody figures out the root cause. This made me mad when I experienced, but the working ping missled me to think "it's a HA web server issue". Until investigating #569 finally the root cause is identified.
Related Issues
fixes #569
Summary by CodeRabbit
Release Notes
New Features
always_use_derpconfiguration option to force peer communication over DERP by disabling UDP. Disabled by default.Documentation
Changes