Fix MagicDNS incompatibility with Home Assistant's DNS #455
Conversation
WalkthroughThis pull request introduces comprehensive changes to the Tailscale Home Assistant Community Add-on, focusing on DNS configuration and documentation improvements. The modifications include updating the documentation with clearer explanations of DNS settings, adding a new AppArmor profile, installing Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Actions performedReview triggered.
|
1 similar comment
✅ 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/DOCS.md (1)
438-469: Fix minor formatting and grammar issues in advanced DNS configuration.The advanced DNS configuration section has some minor issues:
Apply these corrections:
- Line 444: Remove comma before "because"
- Line 450: Remove duplicate "the"
- Line 458: Change "eg." to "e.g."
- Line 465: Remove comma before "because"
🧰 Tools
🪛 LanguageTool
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...(COMMA_COMPOUND_SENTENCE)
[formatting] ~444-~444: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...s global nameserver on the admin console, because you want to redirect all DNS queries wi...(COMMA_BEFORE_BECAUSE)
[duplication] ~450-~450: Possible typo: you repeated a word.
Context: ...f your tailnet that are configured on the the admin console above. This will preven...(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~455-~455: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...gure your DNS as the only DNS server (eg. IPv4: 127.0.0.1, IPv6: ::1). - In your...(E_G)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~458-~458: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...ailnet domain as upstream DNS server (eg. in case of AdGuard `[/tail1234.ts.net/]...(E_G)
[uncategorized] ~461-~461: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... can configure your normal DNS servers (eg. 192.168.1.1 or 1.1.1.1) at the second o...(E_G)
[formatting] ~465-~465: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...n Home Assistant's network configuration, because whenaccept_dnsoption is disabled, T...(COMMA_BEFORE_BECAUSE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
tailscale/DOCS.md(4 hunks)tailscale/Dockerfile(1 hunks)tailscale/apparmor.txt(1 hunks)tailscale/config.yaml(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run(2 hunks)tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh(1 hunks)tailscale/translations/en.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~368-~368: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...n name instead of only the device name, ie. `ping some-tailnet-device.tail1234.ts.n...
(I_E)
[grammar] ~385-~385: It appears that a pronoun is missing.
Context: ... in the add-on, is that Tailscale's DNS when can't resolve a query, instead of retur...
(WHERE_MD_VB)
[uncategorized] ~404-~404: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...0::53). Move your normal DNS servers (eg. 192.168.1.1 or 1.1.1.1) to lower positi...
(E_G)
[uncategorized] ~429-~429: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... can configure your normal DNS servers (eg. 192.168.1.1 or 1.1.1.1) at the second o...
(E_G)
[style] ~434-~434: The adverb ‘also’ is commonly used to connect clauses and isn’t usually used at the end of a phrase or before a conjunction. Consider replacing it with a more formal alternative.
Context: ... DNS will resolve non-tailnet addresses also. Whether you have your own DNS (like Ad...
(ALSO_AS_WELL)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...
(COMMA_COMPOUND_SENTENCE)
[formatting] ~444-~444: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...s global nameserver on the admin console, because you want to redirect all DNS queries wi...
(COMMA_BEFORE_BECAUSE)
[duplication] ~450-~450: Possible typo: you repeated a word.
Context: ...f your tailnet that are configured on the the admin console above. This will preven...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~455-~455: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...gure your DNS as the only DNS server (eg. IPv4: 127.0.0.1, IPv6: ::1). - In your...
(E_G)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~458-~458: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...ailnet domain as upstream DNS server (eg. in case of AdGuard `[/tail1234.ts.net/]...
(E_G)
[uncategorized] ~461-~461: The abbreviation “e.g.” (= for example) requires two periods.
Context: ... can configure your normal DNS servers (eg. 192.168.1.1 or 1.1.1.1) at the second o...
(E_G)
[formatting] ~465-~465: If the ‘because’ clause is essential to the meaning, do not use a comma before the clause.
Context: ...n Home Assistant's network configuration, because when accept_dns option is disabled, T...
(COMMA_BEFORE_BECAUSE)
🔇 Additional comments (13)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run (2)
8-8: LGTM! Good choice of IP address.The chosen IP address (127.52.52.52) is within the loopback range but uses an uncommon subnet to avoid conflicts with other services.
12-16: LGTM! Well-configured dummy DNS setup.The dnsmasq configuration is secure and properly isolated:
- Binds only to specific interface
- Runs in foreground for proper supervision
- Refuses all queries as intended
- Uses port 53 as required for resolv.conf
tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish (1)
15-25: LGTM! Robust exit handling implementation.The script properly manages service shutdown with appropriate exit code handling:
- Preserves signal-based exits (code 256)
- Special handling for SIGTERM (signal 15)
- Maintains container exit codes
tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run (2)
38-40: LGTM! Good DNS loop prevention strategy.The implementation correctly prevents DNS loops by isolating tailscaled's DNS configuration from the system.
52-62: Verify mount namespace isolation.While the mount namespace isolation is a good approach, ensure that:
- The mount namespace is properly cleaned up on service restart
- The file permissions are preserved during the move operation
tailscale/config.yaml (1)
24-24: Verify the necessity of SYS_ADMIN privilege.While SYS_ADMIN is required for mount namespace operations, it's a powerful capability. Consider if a more limited capability like CAP_SYS_CHROOT would suffice for the mount --bind operation.
✅ Verification successful
SYS_ADMIN capability is required for Tailscale's core functionality
The capability is necessary for network namespace operations and interface management, which are essential for Tailscale's VPN functionality. CAP_SYS_CHROOT would not be sufficient as Tailscale requires full network namespace management capabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the mount operation works with CAP_SYS_CHROOT # Note: This is a theoretical check as the actual test would require runtime capabilities capsh --print | grep cap_sys_adminLength of output: 132
Script:
#!/bin/bash # Search for mount-related operations and SYS_ADMIN references echo "=== Searching for mount operations ===" rg -i "mount" --type yaml --type go -C 2 echo -e "\n=== Searching for SYS_ADMIN references ===" rg -i "sys_admin" --type yaml --type go -C 2 echo -e "\n=== Checking README and docs ===" fd "README|\.md$" -t f -X catLength of output: 30735
tailscale/apparmor.txt (2)
3-9: LGTM! AppArmor profile correctly configured with necessary flags and base abstractions.The profile is properly set up with:
attach_disconnectedflag for handling disconnected pathsmediate_deletedflag for mediating deleted files- Required signal capabilities for process management
43-47: Verify mount capabilities for DNS fix.The mount capabilities and rules look correct for the MagicDNS fix:
sys_admincapability for mount operations- Root filesystem mount with
rprivatefor proper isolation- Bind mount for tailscaled resolver configuration
tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh (1)
8-15: LGTM! Correct service management logic for dnsmasq.The conditional block properly handles dnsmasq service based on configuration:
- Disables when userspace networking is enabled
- Disables when DNS acceptance is disabled
- Correctly removes both service and dependency files
tailscale/Dockerfile (1)
13-13: Verify dnsmasq package version for security vulnerabilities.The dnsmasq package version 2.90-r3 is added as a dependency.
tailscale/translations/en.yaml (1)
6-7: LGTM! Clear and accurate description of accept_dns option.The updated description correctly explains:
- Purpose of accepting DNS settings from tailnet
- Configuration location in admin console
tailscale/DOCS.md (2)
348-394: LGTM! Comprehensive explanation of DNS functionality.Excellent documentation of:
- DNS server functionality at 100.100.100.100
- MagicDNS components and limitations
- DNS resolution behavior
- Loop prevention mechanism
🧰 Tools
🪛 LanguageTool
[uncategorized] ~368-~368: The abbreviation “i.e.” (= that is) requires two periods.
Context: ...n name instead of only the device name, ie. `ping some-tailnet-device.tail1234.ts.n...(I_E)
[grammar] ~385-~385: It appears that a pronoun is missing.
Context: ... in the add-on, is that Tailscale's DNS when can't resolve a query, instead of retur...(WHERE_MD_VB)
397-415: LGTM! Clear instructions for basic DNS configuration.Well-documented scenario for resolving only tailnet devices:
- Correct DNS server configuration
- Proper order of DNS servers
- Fallback behavior
🧰 Tools
🪛 LanguageTool
[uncategorized] ~404-~404: The abbreviation “e.g.” (= for example) requires two periods.
Context: ...0::53). Move your normal DNS servers (eg. 192.168.1.1 or 1.1.1.1) to lower positi...(E_G)
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tailscale/DOCS.md (2)
88-93: Add a comma for better readability.Add a comma after "console" to improve readability.
-This option allows you to accept DNS settings of your tailnet that are -configured on the [DNS page][tailscale_dns] of the admin console. +This option allows you to accept DNS settings of your tailnet that are +configured on the [DNS page][tailscale_dns] of the admin console,🧰 Tools
🪛 LanguageTool
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...(AI_HYDRA_LEO_MISSING_COMMA)
348-469: Excellent documentation of DNS configuration!The new DNS section comprehensively addresses the MagicDNS incompatibility issue and provides clear guidance for different scenarios. The explanation of why MagicDNS's local DNS configuration manipulation is disabled (lines 384-390) is particularly valuable.
However, there are a few minor grammatical improvements needed:
- Line 385: "is that Tailscale's DNS when can't resolve" should be "is that when Tailscale's DNS can't resolve"
- Line 434: Move "also" before "resolve": "will also resolve non-tailnet addresses"
- Lines 418, 421, 440, 443: Add commas after "console" and before "your"
-The reason that Tailscale's "magical" local DNS configuration manipulation is -permanently disabled in the add-on, is that Tailscale's DNS when can't resolve +The reason that Tailscale's "magical" local DNS configuration manipulation is +permanently disabled in the add-on, is that when Tailscale's DNS can't resolve -Tailscale's DNS will resolve non-tailnet addresses also. +Tailscale's DNS will also resolve non-tailnet addresses. -On the [DNS page][tailscale_dns] of the admin console you already enabled +"On the [DNS page][tailscale_dns] of the admin console, you already enabled -In this scenario your Home Assistant device's tailnet IP +In this scenario, your Home Assistant device's tailnet IP🧰 Tools
🪛 LanguageTool
[grammar] ~385-~385: It appears that a pronoun is missing.
Context: ... in the add-on, is that Tailscale's DNS when can't resolve a query, instead of retur...(WHERE_MD_VB)
[uncategorized] ~418-~418: Possible missing comma found.
Context: ... [DNS page][tailscale_dns] of the admin console you already enabled "Override local DNS...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~421-~421: Possible missing comma found.
Context: ...l nameservers". Important: In this scenario your Home Assistant device's tailnet IP...(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~434-~434: The adverb ‘also’ is commonly used to connect clauses and isn’t usually used at the end of a phrase or before a conjunction. Consider replacing it with a more formal alternative.
Context: ... DNS will resolve non-tailnet addresses also. Whether you have your own DNS (like Ad...(ALSO_AS_WELL)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~440-~440: Possible missing comma found.
Context: ... [DNS page][tailscale_dns] of the admin console you already enabled "Override local DNS...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~443-~443: Possible missing comma found.
Context: ...l nameservers". Important: In this scenario your Home Assistant device's tailnet IP...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~385-~385: It appears that a pronoun is missing.
Context: ... in the add-on, is that Tailscale's DNS when can't resolve a query, instead of retur...
(WHERE_MD_VB)
[uncategorized] ~418-~418: Possible missing comma found.
Context: ... [DNS page][tailscale_dns] of the admin console you already enabled "Override local DNS...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~421-~421: Possible missing comma found.
Context: ...l nameservers". Important: In this scenario your Home Assistant device's tailnet IP...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~434-~434: The adverb ‘also’ is commonly used to connect clauses and isn’t usually used at the end of a phrase or before a conjunction. Consider replacing it with a more formal alternative.
Context: ... DNS will resolve non-tailnet addresses also. Whether you have your own DNS (like Ad...
(ALSO_AS_WELL)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~440-~440: Possible missing comma found.
Context: ... [DNS page][tailscale_dns] of the admin console you already enabled "Override local DNS...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~443-~443: Possible missing comma found.
Context: ...l nameservers". Important: In this scenario your Home Assistant device's tailnet IP...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (1)
tailscale/DOCS.md (1)
320-323: LGTM! Good cross-referencing.The addition provides helpful context about DNS resolution and appropriately directs users to the detailed DNS configuration section.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
394-394: Add a comma for better readability.-More information: [What is 100.100.100.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDNS][tailscale_info_magicdns], [Access a Pi-hole from anywhere][tailscale_info_pi_hole] +More information: [What is 100.100.100.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDNS][tailscale_info_magicdns], [Access a Pi-hole from anywhere][tailscale_info_pi_hole], Below we describe some typical scenarios for DNS configuration.🧰 Tools
🪛 LanguageTool
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~413-~413: Possible missing comma found.
Context: ...values remain in the DNS cache for some time even after the add-on is started and Ta...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~432-~432: Possible missing comma found.
Context: ...the second or lower positions. In this configuration Home Assistant (as any other general de...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...
(COMMA_COMPOUND_SENTENCE)
🔇 Additional comments (4)
tailscale/DOCS.md (4)
88-93: LGTM! Clear and accurate description of theaccept_dnsoption.The updated description precisely explains the option's purpose and provides helpful references to both the DNS configuration page and the detailed DNS section.
320-323: LGTM! Good addition about DNS resolution capabilities.The new text effectively explains the relationship between userspace networking and DNS resolution, with a helpful reference to the detailed DNS section.
348-394: LGTM! Excellent explanation of DNS functionality and configuration.The new section thoroughly explains the DNS configuration requirements and limitations, particularly addressing the MagicDNS incompatibility issue with Home Assistant's DNS. The root cause of the DNS loop issue is well-documented, helping users understand why certain configurations are necessary.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...(AI_HYDRA_LEO_MISSING_COMMA)
397-469: LGTM! Well-structured DNS configuration scenarios.The documentation provides clear, practical examples for common DNS configuration scenarios:
- Using Tailscale DNS for tailnet devices only
- Using Tailscale DNS for all resolution
- Using Tailscale DNS with local DNS services
Each scenario includes proper configuration steps and important notes about potential issues.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~413-~413: Possible missing comma found.
Context: ...values remain in the DNS cache for some time even after the add-on is started and Ta...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~432-~432: Possible missing comma found.
Context: ...the second or lower positions. In this configuration Home Assistant (as any other general de...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...(COMMA_COMPOUND_SENTENCE)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
91-91: Consider adding commas for improved readability.Minor formatting suggestions to improve readability:
- Line 91: Add comma after "console"
- Line 394: Add comma after "pi-hole]"
- Line 432: Add comma after "positions"
- Line 438: Add comma before "and"
- Line 457: Add comma after "::1)"
Also applies to: 394-394, 432-432, 438-438, 457-457
🧰 Tools
🪛 LanguageTool
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
tailscale/DOCS.md(4 hunks)tailscale/Dockerfile(1 hunks)tailscale/apparmor.txt(1 hunks)tailscale/config.yaml(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run(2 hunks)tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh(1 hunks)tailscale/translations/en.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- tailscale/config.yaml
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type
- tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
- tailscale/apparmor.txt
- tailscale/Dockerfile
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish
- tailscale/translations/en.yaml
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~432-~432: Possible missing comma found.
Context: ...the second or lower positions. In this configuration Home Assistant (as any other general de...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (5)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run (3)
8-9: LGTM! Well-structured log level variables.The readonly variables provide clear and informative messages about log suppression behavior.
40-50: LGTM! Clean implementation of the regular execution path.The code properly handles both debug and non-debug modes while maintaining consistent log suppression behavior.
52-62: Verify mount namespace isolation.The implementation uses
unshare -mto isolate the mount changes, which is crucial for preventing DNS loops. The mount binding of the fake resolv.conf appears correct.Let's verify the mount namespace isolation:
tailscale/DOCS.md (2)
348-394: LGTM! Excellent documentation of DNS behavior and limitations.The documentation clearly explains:
- The components of MagicDNS
- Why automatic DNS configuration is disabled
- The DNS loop issue and its solution
🧰 Tools
🪛 LanguageTool
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...(AI_HYDRA_LEO_MISSING_COMMA)
397-469: LGTM! Comprehensive coverage of DNS configuration scenarios.The documentation provides clear instructions for common scenarios:
- Using Tailscale DNS for tailnet devices only
- Using Tailscale DNS for all resolution
- Integration with local DNS services
🧰 Tools
🪛 LanguageTool
[uncategorized] ~432-~432: Possible missing comma found.
Context: ...the second or lower positions. In this configuration Home Assistant (as any other general de...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...(AI_HYDRA_LEO_MISSING_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
91-91: Consider adding commas for improved readability.Add commas in the following places:
- Line 91: "For more information, see the "DNS" section"
- Line 394: "Tailscale_info_pi_hole], Below we describe"
- Line 438: "resolve everything, and you have your own DNS"
- Line 448: "configuration, disable
accept_dns"- Line 457: "In your DNS, configure Tailscale DNS"
Also applies to: 394-394, 438-438, 448-448, 457-457
🧰 Tools
🪛 LanguageTool
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
tailscale/DOCS.md(4 hunks)tailscale/Dockerfile(1 hunks)tailscale/apparmor.txt(1 hunks)tailscale/config.yaml(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run(2 hunks)tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh(1 hunks)tailscale/translations/en.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- tailscale/config.yaml
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/type
- tailscale/Dockerfile
- tailscale/rootfs/etc/s6-overlay/scripts/stage2_hook.sh
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/finish
- tailscale/translations/en.yaml
- tailscale/apparmor.txt
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/dnsmasq/run
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~91-~91: Possible missing comma found.
Context: ...le_dns] of the admin console. For more information see the "DNS" section of this documenta...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~448-~448: Possible missing comma found.
Context: ...ning on this device. - In the add-on's configuration disable accept_dns option and restart...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (6)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run (4)
8-9: LGTM! Well-structured logging configuration.The readonly variables improve maintainability by centralizing log suppression messages.
38-40: LGTM! Effective DNS loop prevention.The implementation elegantly prevents DNS loops by mounting a separate resolv.conf only for tailscaled using namespace isolation.
41-50: LGTM! Clean execution path implementation.The code maintains a clear separation between debug and non-debug modes while implementing consistent log suppression.
52-62: LGTM! Safe namespace isolation implementation.The code safely isolates DNS configuration changes using mount namespace separation while maintaining consistent logging behavior.
tailscale/DOCS.md (2)
348-394: LGTM! Comprehensive DNS documentation.The new DNS section effectively explains:
- The DNS resolution mechanism
- Special considerations for the add-on
- The rationale behind disabling MagicDNS's local DNS configuration manipulation
🧰 Tools
🪛 LanguageTool
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...(AI_HYDRA_LEO_MISSING_COMMA)
397-469: LGTM! Well-documented DNS configuration scenarios.The documentation effectively covers:
- Using Tailscale DNS for tailnet devices only
- Using Tailscale DNS for all resolution
- Complex scenarios with local DNS services
🧰 Tools
🪛 LanguageTool
[uncategorized] ~438-~438: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...sing Tailscale DNS to resolve everything and you have your own DNS (like AdGuard) _o...(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~448-~448: Possible missing comma found.
Context: ...ning on this device. - In the add-on's configuration disableaccept_dnsoption and restart...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~457-~457: Possible missing comma found.
Context: ...IPv4: 127.0.0.1, IPv6: ::1). - In your DNS configure Tailscale DNS for your tailne...(AI_HYDRA_LEO_MISSING_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tailscale/DOCS.md (2)
88-93: Clarify theaccept_dnsOption DescriptionThe updated description now clearly states that enabling
accept_dnscauses the add-on to accept DNS settings as configured on the Tailnet’s DNS page. This improves the clarity over the previous version. Consider, however, whether mentioning consequences of disabling the option or linking to further configuration examples might be beneficial for less experienced users.
348-395: Comprehensive New DNS Section AddedThe new DNS section is a significant improvement. It thoroughly explains how Tailscale’s DNS operates in relation to the
userspace_networkingmode and details the pitfalls of the "magical" local DNS configuration manipulation that has been disabled in this add-on. The explanation about the dual components of MagicDNS is informative. A few minor grammatical tweaks (for example, ensuring sentences like “and completely disabled in this add-on” read as “and is completely disabled in this add-on”) and a potential use of bullet points to break down the key points could further enhance readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🔇 Additional comments (2)
tailscale/DOCS.md (2)
320-323: Detail on Userspace Networking ConfigurationThe additional lines clearly explain that disabling userspace networking results in the creation of a
tailscale0network interface and points users to the "DNS" section for further guidance on hostname resolution within the tailnet. This extra context is valuable for troubleshooting name resolution issues in Home Assistant.
548-557: Updated Reference Links VerificationSeveral reference links (e.g.,
[tailscale_info_dns],[tailscale_info_magicdns],[tailscale_info_pi_hole], and[tailscale_info_quad100]) have been updated. Please double-check that all URLs are correct and reflect the latest documentation on Tailscale’s website.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
tailscale/DOCS.md (3)
88-94: Clarifyaccept_dnsOption DescriptionThe updated description clearly states that this option accepts the DNS settings from your tailnet as configured on the admin console’s DNS page and directs users to the new "DNS" section for further details. Ensure that any related translations (for example, in
tailscale/translations/en.yaml) remain in sync with this updated wording.
320-323: Enhanced Clarity for Userspace Networking ExplanationThe additional lines now explicitly mention that disabling userspace networking creates a
tailscale0network interface on the host, and they refer users to the "DNS" section for details on addressing tailnet devices by name. This improves overall clarity. You might want to do a quick grammar check for smoothness.
348-441: Comprehensive New DNS Configuration SectionThis new section provides an in-depth explanation of how DNS is handled when userspace networking is disabled—detailing the dual nature of Tailscale’s DNS and the implications of both enabling and disabling MagicDNS and the
accept_dnsoption. The step-by-step scenarios for configuring DNS in Home Assistant are very useful.A few considerations:
- Verify that all reference links (e.g.,
[tailscale_dns],[tailscale_info_dns],[tailscale_info_magicdns]) point to the correct and current resources.- For even better readability, consider breaking up this long section into smaller, titled subsections (for example, separate blocks for “Using Tailscale DNS to resolve only tailnet devices” and “Using Tailscale DNS to resolve everything”).
- [Nitpick]: A static analysis hint flagged a possible missing comma near where the Pi-hole reference is mentioned (around lines 387–394). Please review the punctuation there to ensure that the sentence structure remains clear.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~394-~394: Possible missing comma found.
Context: ...from anywhere][tailscale_info_pi_hole] Below we describe some typical scenarios for ...
(AI_HYDRA_LEO_MISSING_COMMA)
This comment was marked as off-topic.
This comment was marked as off-topic.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
374-380: Refine the Recursive Loop Explanation
The explanation regarding the recursive DNS loop is informative but could be reworded for improved clarity and formality. For example, instead of the current phrasing suggesting “calls itself the originally configured DNS” and “This is a loop, that had to be cut in the add-on,” consider a revision that more formally describes the potential recursive resolution and its intentional prevention.Suggested diff:
- manipulation is permanently disabled in the add-on, is that when Tailscale's DNS - can't resolve a query, instead of returning an error and letting the operating - system call the next DNS, calls itself the originally configured DNS. But that - DNS is Home Assistant's DNS, where we configure, to call Tailscale's DNS... This - is a loop, that had to be cut in the add-on. + manipulation is permanently disabled in the add-on because when Tailscale's DNS + cannot resolve a query, it would otherwise fall back to the originally configured + DNS—which, in this setup, is Home Assistant's DNS—leading to a recursive resolution loop. + This recursive behavior is intentionally prevented by the add-on.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🔇 Additional comments (3)
tailscale/DOCS.md (3)
88-93: Clarify theaccept_dnsOption Description
The updated description clearly explains that this option enables acceptance of DNS settings from your tailnet as configured on the admin console. It also notes that the option is enabled by default, which is useful for users.
317-319: Enhance theuserspace_networkingExplanation
The additional sentences now clarify that disabling userspace networking creates atailscale0interface and also emphasizes that to leverage tailnet names (and not just tailnet IPs), one must refer to the "DNS" section. This extra context is very helpful for users setting up their network correctly.
348-373: Comprehensive New DNS Section
The newly added DNS section provides a thorough overview of Tailscale’s DNS behavior when theuserspace_networkingoption is disabled. It details how the DNS server works (at 100.100.100.100) and explains the need for proper manual configuration in Home Assistant’s network settings. This section clearly outlines the implications of disabling MagicDNS and offers practical guidance on how DNS queries are handled in various scenarios.
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
frenck
added
the
bugfix
|
Looking at this... this is a hot mess. Having to have so many notes in the docs and instruct to litterly work around stuff is just a huge big red flag. It is just utterly bullshit that we are running a dnsmasq to work around shit |
This is a big no-go. The OS should never hook into a DNS server provided by a container it is hosting/running itself. |
|
Deleted 80% of the DNS docs, they were explanations why/how it is different. Not needed.
I'm talking about hassio_dns, not the OS, the normal net config UI at Settings -> System -> Network. There is no other way to resolve anything within a VPN. And I've tested hassio_dns, it has no problem with an unreachable DNS, and even has 1.1.1.1 as a fallback when nothing works. |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tailscale/DOCS.md (2)
88-94: Clarify the "accept_dns" Option Description
The updated description clearly explains that this option lets users accept DNS settings from their tailnet (as configured on the admin console’s DNS page). For consistency and improved readability, consider inserting “the” before “DNS settings” (e.g. “accept the DNS settings of your tailnet”).
348-380: New DNS Section: Comprehensive and Clear with Minor Language Adjustments
The newly added “DNS” section greatly improves clarity by providing step-by-step guidance on how to configure Tailscale’s DNS when theuserspace_networkingoption is disabled. This addition directly supports the PR’s objective of ensuring proper name resolution and preventing DNS loops.Minor Language Suggestions:
- Line 350: Consider revising to “When the
userspace_networkingoption is disabled, Tailscale provides a DNS (at 100.100.100.100)…” so that the definite article “the” improves clarity.- Line 354: Review the punctuation in the list of links. A slight adjustment (such as ensuring commas or semicolons are used uniformly) could improve readability.
- Line 358: Update the instruction from “Check that
userspace_networkingoption is disabled.” to “Check that theuserspace_networkingoption is disabled.”- Line 375: Revise the text to: “Disable the
accept_dnsoption to prevent Tailscale DNS from redirecting queries from your device back to it, causing a loop.” This change improves the verb form and ensures proper use of articles.🧰 Tools
🪛 LanguageTool
[uncategorized] ~350-~350: You might be missing the article “the” here.
Context: ... port is used by default. ## DNS Whenuserspace_networkingoption is disabled...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~354-~354: A punctuation mark might be missing here.
Context: ...00.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDN...(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~358-~358: You might be missing the article “the” here.
Context: ...[tailscale_info_pi_hole] 1. Check thatuserspace_networkingoption is disabled...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~375-~375: You might be missing the article “the” here.
Context: ...of the admin console, then: 1. Disableaccept_dnsoption to prevent the Tailsc...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~375-~375: This verb may not be in the correct form. Consider using a different form for this context.
Context: ... option to prevent the Tailscale DNS to redirect queries from your device back to you...(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~350-~350: You might be missing the article “the” here.
Context: ... port is used by default. ## DNS When userspace_networking option is disabled...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~354-~354: A punctuation mark might be missing here.
Context: ...00.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDN...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
[uncategorized] ~358-~358: You might be missing the article “the” here.
Context: ...[tailscale_info_pi_hole] 1. Check that userspace_networking option is disabled...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~375-~375: You might be missing the article “the” here.
Context: ...of the admin console, then: 1. Disable accept_dns option to prevent the Tailsc...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~375-~375: This verb may not be in the correct form. Consider using a different form for this context.
Context: ... option to prevent the Tailscale DNS to redirect queries from your device back to you...
(AI_EN_LECTOR_REPLACEMENT_VERB_FORM)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tailscale/DOCS.md (2)
321-324: Enhanced Guidance for Userspace Networking
The newly added lines now advise users that disablinguserspace_networkingcreates atailscale0network interface and highlights that for full name resolution, they should refer to the "DNS" section. This addition is helpful and reinforces the connection between networking mode and DNS resolution.Suggestion: Consider rephrasing for slight clarity. For example, "To be able to address other clients on your tailnet by their names (and not only by IP addresses), see the 'DNS' section of this documentation."
349-366: New "DNS" Section: Well-Structured with Minor Punctuation Adjustment
This newly added "DNS" section clearly outlines the steps to configure Tailscale’s DNS whenuserspace_networkingis disabled. It offers practical instructions to set the first DNS server and to lower the positions of other normal DNS servers, aligning perfectly with the PR’s objectives to mitigate issues with MagicDNS.Suggestion: On line 355, the list of references could benefit from improved punctuation for enhanced readability. Consider revising as follows:
- More information: [What is 100.100.100.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDNS][tailscale_info_magicdns], [Access a Pi-hole from anywhere][tailscale_info_pi_hole] + More information: [What is 100.100.100.100][tailscale_info_quad100]; [DNS in Tailscale][tailscale_info_dns]; [MagicDNS][tailscale_info_magicdns]; [Access a Pi-hole from anywhere][tailscale_info_pi_hole].This adjustment makes the reference list clearer.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~355-~355: A punctuation mark might be missing here.
Context: ...00.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDN...(AI_EN_LECTOR_MISSING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/DOCS.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
tailscale/DOCS.md
[uncategorized] ~355-~355: A punctuation mark might be missing here.
Context: ...00.100][tailscale_info_quad100], [DNS in Tailscale][tailscale_info_dns], [MagicDN...
(AI_EN_LECTOR_MISSING_PUNCTUATION)
🔇 Additional comments (1)
tailscale/DOCS.md (1)
88-94: Clear and Concise Update for theaccept_dnsOption
The revised description succinctly explains that enabling this option accepts the tailnet DNS settings (as configured on the [DNS page][tailscale_dns]) and that the default behavior is enabled. This update provides the necessary clarity for users and aligns with the intended changes for handling MagicDNS.
Proposed Changes
This was a permanent returning issue.
Problem:
Not solutions:
Solution:
dns: resolver: forward: no upstream resolvers set, returning SERVFAILPositive side effects:
Negative side effects:
ping tailnet-device-namedoesn't work, onlyping tailnet-device-name.tailxxxx.ts.net, so without domain, only local.hass.io domain works, but this is the default HA behavior, so I think it is fineRelated Issues
Summary by CodeRabbit
Summary by CodeRabbit
Documentation
accept_dnsoption to reflect its role in accepting DNS settings.New Features
dnsmasqandbind-toolspackages to enhance DNS functionality.dnsmasqservice operations effectively.dnsmasqservice.Configuration
SYS_ADMINprivilege to expand service capabilities.Bug Fixes