Change options default value and name to align with stock Tailscale's platform-specific behavior - config is automatically updated!!!#585
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughDefault Tailscale configuration options changed from enabled to disabled ( ChangesConfiguration and Documentation Update
Sequence Diagram(s)(Skipped — changes focus on configuration defaults, documentation rewrites, and synchronous startup/migration script updates; no multi-component sequential interactions requiring visualization.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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
added
the
bugfix
|
There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions. |
github-actions
Bot
added
the
stale
lmagyar
added
no-stale
|
@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: 0
🧹 Nitpick comments (1)
tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh (1)
74-82: Consider combining the duplicate conditions.Both forwarding and mss-clamping services are disabled under the same condition (
userspace_networkingis true). These could be combined into a single conditional block for clarity.🔎 Suggested refactor
# Disable forwarding service when userspace-networking is enabled -if bashio::config.true "userspace_networking"; then +# Disable mss-clamping service when userspace-networking is enabled +if bashio::config.true "userspace_networking"; then rm /etc/s6-overlay/s6-rc.d/user/contents.d/forwarding -fi - -# Disable mss-clamping service when userspace-networking is enabled -if bashio::config.true "userspace_networking"; then rm /etc/s6-overlay/s6-rc.d/user/contents.d/mss-clamping fi
📜 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: 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.
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.
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`.
📚 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/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/DOCS.mdtailscale/config.yamltailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/translations/en.yamltailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
📚 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/tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/usr/bin/subnet-routes
📚 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/tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/usr/bin/subnet-routes
📚 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/DOCS.mdtailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/run
📚 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/DOCS.mdtailscale/translations/en.yamltailscale/rootfs/usr/bin/subnet-routes
📚 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-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/DOCS.mdtailscale/config.yaml
📚 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
📚 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/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/usr/bin/subnet-routes
🔇 Additional comments (23)
tailscale/config.yaml (2)
28-43: LGTM! Well-structured default configuration.The explicit defaults align with the PR objective of making
accept_routesdefault tofalse. The mandatory options with sensible defaults improve configuration clarity and align with the shift away from implicit behavior.
44-61: LGTM! Schema correctly enforces mandatory configuration.The transition from optional (
bool?) to required (bool) types ensures all configuration options are explicitly set. The regex patterns correctly validate the allowed values.tailscale/rootfs/etc/s6-overlay/s6-rc.d/share-homeassistant/run (1)
72-72: LGTM! Simplified port handling aligns with mandatory configuration.With
share_on_portnow a required field with a default value of443inconfig.yaml, the fallback logic is correctly removed. The configuration system guarantees a value will always be present.tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run (1)
26-29: LGTM! Explicit true check aligns with mandatory configuration.The change from a negated check to
bashio::config.trueis correct. Sinceuserspace_networkingis now mandatory with a default oftrueinconfig.yaml, this explicit check properly enables userspace networking mode. Based on learnings, this intentional deviation from stock Tailscale's default kernel/TUN networking is appropriate for Home Assistant accessibility.tailscale/translations/en.yaml (1)
1-108: LGTM! Translation descriptions accurately reflect new defaults.The updated descriptions consistently communicate the new mandatory configuration behavior. The
accept_routesdescription correctly states "disabled by default" which is the primary objective of this PR. All other option descriptions align with their respective defaults inconfig.yaml.tailscale/DOCS.md (3)
43-64: LGTM! Clear configuration guidance with updated defaults.The documentation correctly explains the new default behavior and provides step-by-step instructions for enabling features in the Tailscale admin console. The reference to the Machines page improves discoverability.
66-87: LGTM! Sample configuration matches new defaults.The example YAML correctly shows
accept_routes: falseand includeslocal_subnetsinadvertise_routes, aligning with the new configuration schema inconfig.yaml.
107-114: LGTM! Documentation accurately describes accept_routes default.The
accept_routessection correctly states "This option is disabled by default", which is the primary objective of this PR to align with stock Tailscale's platform-specific behavior.tailscale/rootfs/usr/bin/subnet-routes (2)
27-41: LGTM! Clean handling of local_subnets configuration.The logic correctly:
- Uses exact regex match (
^local_subnets$) to identify the special value- Recursively calls
subnet-routes localto collect local subnets- Warns if no local subnets are found (helpful for troubleshooting)
- Appends other configured addresses directly
The recursive call is safe since it only occurs when
$1 == "advertised"and the recursive call useslocal.
50-73: LGTM! Route extraction and deduplication logic is sound.The route extraction correctly:
- Skips link-local addresses (fe80:: and 169.254.x.x)
- Checks forwarding is enabled for the address family before including routes
- Uses ipcalc to compute network/prefix from addresses
- Deduplicates routes using
sort -utailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh (3)
61-66: LGTM! Correct logic for disabling protect-subnets service.The condition properly disables protect-subnets when:
- Userspace networking is enabled (no TUN interface to protect)
- OR accepting routes is disabled (no external routes to protect against)
This aligns with the PR's change to make
accept_routesdefault tofalse.
68-72: LGTM! Conditional local-network dependency based on local_subnets configuration.The grep pattern correctly checks if
local_subnetsis explicitly configured inadvertise_routes. When not configured, there's no need to wait for the local network to collect subnet information.
89-92: LGTM! Explicit check for 'disabled' value.The change from checking absence to explicitly checking for
'disabled'aligns with the mandatory configuration approach. Sinceshare_homeassistantnow defaults to'disabled'inconfig.yaml, this condition will correctly disable the service by default.tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (10)
19-24: LGTM!The explicit boolean check pattern for
accept_dnsis correct and consistent with the PR's goal of requiring explicit configuration values.
26-31: LGTM — aligns with stock Tailscale behavior.This is the core change of the PR. The
accept_routesoption now defaults to disabled and only enables when explicitly set totrue, matching Tailscale's platform-specific default behavior.
34-45: LGTM!The error check and explicit boolean handling for
advertise_exit_nodeare correct. The mutual exclusivity check between advertising and using an exit node is properly placed.
57-62: LGTM!Consistent explicit boolean handling pattern for
advertise_connector.
68-73: LGTM!Consistent explicit boolean handling pattern for
stateful_filtering.
75-80: LGTM!Consistent explicit boolean handling pattern for
snat_subnet_routes.
146-167: LGTM!The colliding routes warning condition correctly uses
bashio::config.true "accept_routes", consistent with the updated accept_routes handling. The warning only triggers when accept_routes is explicitly enabled.
169-178: LGTM!The userspace networking notice correctly triggers when
userspace_networkingis explicitly true. Based on learnings, this defaults totruein this add-on (intentional deviation from stock Tailscale), so users will be appropriately informed about the mode's limitations.
82-84: No action needed. Thetagsconfiguration has a default value of[]inconfig.yaml(line 41), so the removal of the// []fallback is safe. Callingjoin(",")on an empty array returns an empty string, which is the correct behavior.Likely an incorrect or invalid review comment.
65-66: No action required.The
login_serverconfiguration has a default value inconfig.yaml("https://controlplane.tailscale.com"), so the unconditional addition of--login-server="${login_server}"is correct and safe. Thebashio::configcall will always return a non-empty value (either the default or user-configured value).Likely an incorrect or invalid review comment.
lmagyar
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @tailscale/config.yaml:
- Around line 28-43: The advertise_routes setting currently lists local_subnets
which keeps advertising local subnets by default; change advertise_routes (the
key under the options block) to an empty list (i.e., no advertised routes) to
stop advertising subnets by default, and update any related docs/translations
that assume local_subnets is advertised (or explicitly document that advertising
local subnets is intentional if you prefer to keep it).
🧹 Nitpick comments (4)
tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh (3)
10-11: tags→advertise_tags migration: avoid clobbering + make failure handling consistent.If a user somehow has both
tagsandadvertise_tags, this will overwriteadvertise_tagsunconditionally. Consider only migrating whenadvertise_tagsis empty/unset, and wrapping the set intry(like share_on_port) to avoid a hard fail on invalid legacy values.Also applies to: 62-68
70-75: Userm -ffor s6 dependency/content removals to make upgrades resilient.A missing file (already removed by earlier migration / future layout change) would currently fail the hook under
set -e.Proposed change
- rm /etc/s6-overlay/s6-rc.d/post-tailscaled/dependencies.d/protect-subnets + rm -f /etc/s6-overlay/s6-rc.d/post-tailscaled/dependencies.d/protect-subnets ... - rm /etc/s6-overlay/s6-rc.d/post-tailscaled/dependencies.d/local-network + rm -f /etc/s6-overlay/s6-rc.d/post-tailscaled/dependencies.d/local-network ... - rm /etc/s6-overlay/s6-rc.d/user/contents.d/forwarding + rm -f /etc/s6-overlay/s6-rc.d/user/contents.d/forwarding ... - rm /etc/s6-overlay/s6-rc.d/user/contents.d/mss-clamping + rm -f /etc/s6-overlay/s6-rc.d/user/contents.d/mss-clamping ... - rm /etc/s6-overlay/s6-rc.d/user/contents.d/taildrop + rm -f /etc/s6-overlay/s6-rc.d/user/contents.d/taildrop ... - rm /etc/s6-overlay/s6-rc.d/user/contents.d/share-homeassistant + rm -f /etc/s6-overlay/s6-rc.d/user/contents.d/share-homeassistantAlso applies to: 80-81, 85-86, 90-91, 95-96, 100-101
77-81: Local-network gating forlocal_subnetslooks correct; prefer a “fixed string” grep.Minor robustness:
grep -Fxq local_subnetsavoids regex surprises and is a bit clearer.Proposed change
-if ! bashio::config "advertise_routes" | grep -Eq "^local_subnets$"; +if ! bashio::config "advertise_routes" | grep -Fxq "local_subnets"; then rm /etc/s6-overlay/s6-rc.d/post-tailscaled/dependencies.d/local-network fitailscale/DOCS.md (1)
43-47: Docs are consistent with the current defaults; verify they match the intended fix scope for #604.If you decide to change advertise_routes default to
[], these sections + the sample config will need to be updated to avoid reintroducing confusion.Also applies to: 66-87, 132-146, 160-169
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tailscale/DOCS.mdtailscale/config.yamltailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.shtailscale/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: 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.
📚 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/rootfs/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/DOCS.md
📚 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/scripts/stage2_hook.shtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 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/scripts/stage2_hook.shtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/runtailscale/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/etc/s6-overlay/scripts/stage2_hook.shtailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run
📚 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/DOCS.md
📚 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-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/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 (4)
tailscale/config.yaml (1)
45-48: Making booleans explicit/non-nullable should help avoid “presence-driven” surprises.This aligns well with the move in scripts to rely on explicit true/false checks. (And per add-on behavior, missing options should be filled from defaults anyway.)
Also applies to: 54-61
tailscale/translations/en.yaml (1)
8-8: Wording matches the new defaults; good alignment with the “explicit booleans” shift.Only callout: the advertise_routes text explicitly states local subnets are advertised by default—make sure that’s the intended long-term behavior (ties back to whether #604 should consider this fixed).
Also applies to: 14-15, 22-29, 35-42, 67-68, 75-76, 83-84, 90-91, 96-97, 104-105
tailscale/rootfs/etc/s6-overlay/s6-rc.d/post-tailscaled/run (2)
19-32: Good move to explicit true/false flags—should eliminate presence-driven defaults.This looks like the right direction for fixing the “toggle off but still enabled” class of issues.
Also applies to: 40-46, 57-63, 65-80, 146-149, 169-178
82-90: No issues found with Tailscale CLI compatibility.The pinned Tailscale version (v1.90.8) officially supports empty values for
--advertise-tags=to clear tags, and supports the--flag=falsesyntax used throughout the script (lines 23, 30, 44, 61, 72, 79). These are documented patterns in the CLI, and the code is compatible with v1.90.8 as-is.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
lmagyar
force-pushed
the
pr-accept-routes-default
branch
6 times, most recently
from
6605999 to
2102a4e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tailscale/DOCS.md`:
- Around line 150-151: Fix the grammar in the admin-console note by replacing
the incorrect phrase "you also has to enable them" with "you also have to enable
them" in the DOCS admin console note string (the sentence beginning "**Note:**
After you add subnets to this option, you also has to enable them on Tailscale's
admin console.").
d6494e3 to
f988139
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh`:
- Around line 63-74: The migration currently always writes advertise_tags from
tags which can overwrite a user-provided advertise_tags; change the logic in the
stage2_hook.sh block that reads tags to first check whether advertise_tags is
already set (e.g., get current value via bashio::addon.option 'advertise_tags'
or bashio::jq) and only call try bashio::addon.option 'advertise_tags'
"^${tags}" when advertise_tags is empty/unspecified; regardless of that
decision, still call bashio::addon.option 'tags' to remove the old key, and
preserve the existing TRY_ERROR handling and logging paths (log a message when
skipping the rename due to existing advertise_tags).
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
51-73: Make the example reflectadvertise_routesbeing disabled by default.
The snippet shows active subnets, which can be read as defaults now that other defaults are explicitly set tofalse. Consider using[]with commented examples.Suggested edit
-advertise_routes: - - local_subnets - - 192.168.1.0/24 - - fd12:3456:abcd::/64 +advertise_routes: [] +# - local_subnets +# - 192.168.1.0/24 +# - fd12:3456:abcd::/64
lmagyar
force-pushed
the
pr-accept-routes-default
branch
from
f988139 to
a72edeb
Compare
|
I mark it as draft to "prevent" it from merging. First we need a new release with the mandatory options PR copying the current default values to the config yaml. Then we have to wait a few months until most users update to that version, the missing default values are filled in, then we can change the defaults, this way we won't change the behaviour of most of the existing installations. |
…latform-specific behavior
lmagyar
force-pushed
the
pr-accept-routes-default
branch
from
ba8677f to
b4c7252
Compare
lmagyar
added
enhancement
# Conflicts: # tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
|
I think it's time to make it non-draft. |
2 participants
Proposed Changes
Default disabled:
Renamed:
General considerations to merge this several months after #541:
Related Issues
closes #604
coderabbitai in this comment/review
Summary by CodeRabbit
Configuration Changes
tag:prefix). The legacy tags option is removed and is replaced by advertise_tags.Documentation