Add support for Taildrive #509
Conversation
WalkthroughAdds Taildrive share support: config.yaml gains new share map entries, an options.taildrive block, and schema.taildrive booleans; English translations and docs updated; and an s6 oneshot service (type/up/run) added to reconcile configured Taildrive shares with the Tailscale drive list at startup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant HA as Add‑on config
participant Run as taildrive run (oneshot)
participant TSCli as /opt/tailscale CLI
HA->>Run: provide `options.taildrive` config
Run->>TSCli: /opt/tailscale status + capability check
alt Taildrive unavailable & shares configured
Run-->>HA: log error + exit non‑zero
else Taildrive available
Run->>TSCli: /opt/tailscale drive list
loop each drive name
alt configured & not active
Run->>TSCli: drive share "<name>" "/<name>"
else active & not configured & managed_by_addon
Run->>TSCli: drive unshare "<name>"
else active & not managed_by_addon
Run-->>HA: log skip (do not unshare)
end
end
end
Run-->>HA: exit (success)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tailscale/translations/en.yaml (1)
93-98: Fix typo in the description.The translation content is good and informative, but there's a small typo that should be corrected.
Apply this diff to fix the typo:
taildrive: name: Taildrive Shares description: >- - This option allows you to specify which Homeassistant directories you + This option allows you to specify which Home Assistant directories you want to share with other Tailscale nodes using Taildrive. The listed values are the only allowed values.tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (1)
20-20: Consider improving array comparison logic.The string-based array comparison using
[[ ! " ${array[*]} " =~ " ${item} " ]]works but could be more robust for edge cases with special characters in directory names.Consider using a more robust comparison approach:
# Function to check if array contains element contains_element() { local element="$1" shift local array=("$@") for item in "${array[@]}"; do [[ "$item" == "$element" ]] && return 0 done return 1 } # Usage: if ! contains_element "$dir" "${shared_dirs[@]}"; then # Share the directory fiAlso applies to: 28-28
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up(1 hunks)tailscale/translations/en.yaml(1 hunks)
🔇 Additional comments (6)
tailscale/config.yaml (1)
27-34: LGTM: Comprehensive directory mappings added.The new directory mappings provide good coverage of Home Assistant directories for Taildrive sharing. The read-write permissions are appropriate for the use case.
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up (1)
1-1: Clarify the purpose and content of this file.This file appears to only contain a file path reference. If this is meant to be an s6 service script, it should contain executable commands or be properly configured as a symlink/reference to the actual run script.
Please verify if this file should:
- Be empty (if it's just a placeholder)
- Contain actual service startup commands
- Be a symlink to the main run script
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type (1)
1-1: LGTM: Correct service type configuration.The
oneshotservice type is appropriate for the Taildrive synchronization script that needs to run and complete its task rather than staying as a daemon.tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (3)
1-7: LGTM: Proper script header and documentation.The script header with shebang, shellcheck directive, and descriptive comments is well-structured and follows good practices.
8-9: LGTM: Configuration reading implementation.The use of
bashio::configwithmapfileto read the configured directories into an array is correct and follows the add-on patterns.
18-24: Verify command path consistency and add error handling.The script uses
/opt/tailscale drive sharehere but used justtaildriveearlier. The path should be consistent throughout.Apply this diff to add error handling:
for dir in "${configured_dirs[@]}"; do if [[ ! " ${shared_dirs[*]} " =~ " ${dir} " ]]; then # Share the directory - /opt/tailscale drive share "${dir}" "/${dir}" + if ! /opt/tailscale drive share "${dir}" "/${dir}" 2>/dev/null; then + bashio::log.error "Failed to share directory: ${dir}" + else + bashio::log.info "Successfully shared directory: ${dir}" + fi fi doneAlso verify that the directory paths are correct - should they be
/config,/media, etc. or the actual mounted paths?
ananthb
force-pushed
the
taildrive
branch
2 times, most recently
from
d9d88c5 to
a64cf18
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (2)
26-32:⚠️ Potential issueFix command path error and inverted logic.
This segment has two critical issues: incorrect command path and inverted conditional logic.
Apply this diff to fix both issues:
for dir in "${shared_dirs[@]}"; do - if printf '%s\0' "${configured_dirs[@]}" | grep -Fxqz -- "${dir}"; then + if ! printf '%s\0' "${configured_dirs[@]}" | grep -Fxqz -- "${dir}"; then # Unshare the directory - /opt/taildrive drive unshare "${dir}" + if ! /opt/tailscale drive unshare "${dir}" 2>/dev/null; then + bashio::log.error "Failed to unshare directory: ${dir}" + else + bashio::log.info "Successfully unshared directory: ${dir}" + fi fi doneThe command path should be
/opt/tailscale drive unsharenot/opt/taildrive drive unshare, and the logic should unshare directories that are NOT in the configured list.
11-16: 🛠️ Refactor suggestionAdd error handling for command execution.
While the command path appears correct, the script should handle cases where the
tailscale drive listcommand fails.Add error handling:
-mapfile -t shared_dirs < <(/opt/tailscale drive list | tail -n +3 | awk '{print $1}') +if ! mapfile -t shared_dirs < <(/opt/tailscale drive list 2>/dev/null | tail -n +3 | awk '{print $1}'); then + bashio::log.warning "Failed to retrieve current Taildrive shares" + shared_dirs=() +fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tailscale/config.yaml
🔇 Additional comments (2)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (2)
1-7: LGTM - Well-structured script header.The shebang and header comments are properly formatted and provide clear context about the script's purpose.
8-9: LGTM - Configuration reading looks correct.The script properly reads the configured directories from the add-on configuration using bashio.
|
I've made several fixes to this PR, pushed it to ananthb's repo:
|
|
@lmagyar I'm not seeing it in my repo yet. Feel free to push your changes to the taildrive branch on my repo. |
|
This should be the PR: ananthb#1 |
|
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
|
not stale |
github-actions
bot
removed
the
stale
|
@ananthb I've fixed a bug that broke startup, please see PR in your repo. |
|
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
|
Not stale |
github-actions
bot
removed
the
stale
|
Sure let me fix it |
|
Just a note on the conflict, you don't need the funnel section in the docs, and the end of the docs should look like this: |
ananthb
force-pushed
the
taildrive
branch
2 times, most recently
from
822b079 to
8bceec6
Compare
ananthb
force-pushed
the
taildrive
branch
2 times, most recently
from
ad01f3d to
ad24eb4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tailscale/config.yaml (2)
27-34: Security check: confirm intent to ship writable backup and all_addon_configs mounts.
- Exposing
/backupand/all_addon_configsover Taildrive could leak sensitive data (system backups, secrets from other add-ons).- If inclusion is intentional, call this out in DOCS and translations, and consider a strong warning.
- If not, drop the mounts or keep them unmapped until UI/ACL guidance is finalized.
I can generate a minimal doc snippet and warnings for these two entries if you confirm the desired behavior.
53-54: Schema looks aligned; decide on all_addon_configs allowance and optionality.
- The validator matches: homeassistant_config|addon_config|ssl|addons|backup|share|media. This intentionally excludes
all_addon_configs(mapped above). If you want to allow it, add it to the regex; if not, document why.- Confirm whether
taildriveshould be considered optional. Current pattern follows howadvertise_routesis modeled (property optional; items validated when present), which is OK.Possible update if you decide to allow it:
- - "match(^(?i:(homeassistant_config|addon_config|ssl|addons|backup|share|media))$)" + - "match(^(?i:(homeassistant_config|addon_config|ssl|addons|backup|share|media|all_addon_configs))$)"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tailscale/DOCS.md(1 hunks)tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up(1 hunks)tailscale/translations/en.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up
- tailscale/translations/en.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
PR: hassio-addons/addon-tailscale#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/taildrive/run
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
PR: hassio-addons/addon-tailscale#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/taildrive/run
🪛 markdownlint-cli2 (0.17.2)
tailscale/DOCS.md
440-440: Link and image reference definitions should be needed
Unused link or image reference definition: "tailscale_info_taildrive"
(MD053, link-image-reference-definitions)
441-441: Link and image reference definitions should be needed
Duplicate link or image reference definition: "tailscale_info_taildrop"
(MD053, link-image-reference-definitions)
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (1)
36-46: Incorrect unshare path and same canonicalization bug
- Uses
/opt/taildriveinstead of/opt/tailscale.- Same normalization issue as above.
- Add error handling.
-# If a directory is shared but not configured, unshare it. -for share_name in "${active_share_names[@]}"; do - if ! printf '%s\0' "${configured_share_names[@]}" | grep -Fxqz -- "${share_name}"; then - # Rewrite homeasstant_config to config - if [[ "${share_name}" == "homeassistant_config" ]]; then - share_name="config" - fi - - # Unshare the directory - /opt/taildrive drive unshare "${share_name}" - fi -done +# If a directory is shared but not configured, unshare it. +for name in "${active_canon[@]}"; do + if ! printf '%s\0' "${configured_canon[@]}" | grep -Fxqz -- "${name}"; then + if ! /opt/tailscale drive unshare "${name}" 2>/dev/null; then + bashio::log.error "Failed to unshare: ${name}" + else + bashio::log.info "Unshared: ${name}" + fi + fi +doneNote: If your environment requires
drive share unsharesubcommand form, switch the unshare line accordingly. Please verify with the installed Tailscale CLI.
🧹 Nitpick comments (2)
tailscale/DOCS.md (1)
440-441: Fix unused and duplicate link refs
- 440:
tailscale_info_taildriveis defined but never used.- 441: Duplicate
tailscale_info_taildropdefinition.Either use the Taildrive ref in a short “Option: taildrive” section or drop the unused/duplicate refs. Minimal fix below removes the duplicate and keeps Taildrive available for future use.
[tailscale_info_userspace_networking]: https://tailscale.com/kb/1112/userspace-networking [tailscale_info_taildrive]: https://tailscale.com/kb/1369/taildrive -[tailscale_info_taildrop]: https://tailscale.com/kb/1106/taildropOptionally, add a small section describing the new
taildriveoption and link it to[tailscale_info_taildrive]. I can draft that if you want.tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (1)
1-1: Ensure the s6 service is in the user bundleAdd
taildriveto the user bundle; otherwise this oneshot won’t run.+# /etc/s6-overlay/s6-rc.d/user/contents.d/taildrive +taildrive
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tailscale/DOCS.md(1 hunks)tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up(1 hunks)tailscale/translations/en.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type
- tailscale/translations/en.yaml
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
PR: hassio-addons/addon-tailscale#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/taildrive/run
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
PR: hassio-addons/addon-tailscale#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/taildrive/run
🪛 markdownlint-cli2 (0.17.2)
tailscale/DOCS.md
440-440: Link and image reference definitions should be needed
Unused link or image reference definition: "tailscale_info_taildrive"
(MD053, link-image-reference-definitions)
441-441: Link and image reference definitions should be needed
Duplicate link or image reference definition: "tailscale_info_taildrop"
(MD053, link-image-reference-definitions)
🔇 Additional comments (1)
tailscale/config.yaml (1)
53-55: Maketaildriveoptional and includeall_addon_configsUpdate the schema entry in
tailscale/config.yamlto marktaildriveoptional and supportall_addon_configs:- taildrive: - - "match(^(?i:(homeassistant_config|addon_config|ssl|addons|backup|share|media))$)" + taildrive: + - "match(^(?i:(homeassistant_config|addon_config|all_addon_configs|ssl|addons|backup|share|media))$)?"
|
This became a total mess. The only 2 merge conflict resolution were needed were the 2 thing I mentioned above. This version got back to your first version, that has a lot of issues. See my comment: #509 (comment) You should have rebased your branch, instead you cherry picked your first commit, and lost all other commits. |
|
A rebase would look like this: https://github.com/lmagyar/homeassistant-addon-tailscale/tree/pr-taildrive-ananthb |
|
I've:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (5)
1-7: Enable strict mode to avoid masked failures (pipefail, unset vars).Without strict mode, failures in pipelines (e.g., drive list) can go unnoticed and lead to incorrect share/unshare behavior.
#!/command/with-contenv bashio # shellcheck shell=bash +set -Eeuo pipefail
13-14: Track add-on–managed shares to avoid unsharing user-created ones.Introduce a small state file for shares created by this add-on.
declare share_name + +# Track shares created by this add-on +STATE_FILE="/data/taildrive-managed.txt" +touch "${STATE_FILE}"
52-63: Only unshare add-on–managed entries; don’t touch user shares.Checking against allowed keys isn’t sufficient; use the managed state file.
for share_name in ${active_share_names}; do if ! grep -Fxq -- "${share_name}" <<< "${configured_share_names}"; then - if grep -Fxq -- "${share_name}" <<< "${all_share_names}"; then + if grep -Fxq -- "${share_name}" "${STATE_FILE}"; then bashio::log.info "Taildrive: unsharing /${share_name}" - /opt/tailscale drive unshare "${share_name}" + if /opt/tailscale drive unshare "${share_name}" 2>/dev/null; then + # remove from state file + sed -i "/^${share_name//\//\\/}\$/d" "${STATE_FILE}" + else + bashio::log.error "Taildrive: failed to unshare /${share_name}" + fi else - bashio::log.info "Taildrive: skipping to unshare '${share_name}'" + bashio::log.info "Taildrive: skipping unshare of '${share_name}' (not managed by add-on)" fi fi doneCorrect unshare subcommand is ‘tailscale drive unshare ’. (tailscale.com)
42-50: Add error handling on share and record managed shares.Log failures (ACL, missing path), and persist managed names for safe unshare later.
for share_name in ${configured_share_names}; do if ! grep -Fxq -- "${share_name}" <<< "${active_share_names}"; then bashio::log.info "Taildrive: sharing /${share_name}" - /opt/tailscale drive share "${share_name}" "/${share_name}" + if ! /opt/tailscale drive share "${share_name}" "/${share_name}" 2>/dev/null; then + bashio::log.error "Taildrive: failed to share /${share_name} (check 'drive:share' ACL)." + else + bashio::log.info "Taildrive: shared /${share_name}" + grep -Fxq -- "${share_name}" "${STATE_FILE}" || echo "${share_name}" >> "${STATE_FILE}" + fi else bashio::log.info "Taildrive: /${share_name} is already shared" fi done‘drive share’ usage is per CLI docs. (tailscale.com)
31-38: Preserve exit status from ‘tailscale drive list’ and degrade gracefully.Current pipeline masks errors; on ACL denial or other failures, you’ll treat it as “no shares” and proceed silently.
- if ! active_share_names=$(/opt/tailscale drive list | tail -n +3 | awk '{print $1}'); then - bashio::exit.nok "Error reading shared Taildrive directories from Tailscale." - fi + list_out="$(/opt/tailscale drive list 2>&1)"; rc=$? + if (( rc != 0 )); then + bashio::log.warning "Taildrive: 'drive list' failed (rc=${rc}): ${list_out}" + active_share_names="" + else + active_share_names="$(printf '%s\n' "${list_out}" | tail -n +3 | awk '{print $1}')" + fiReference for CLI availability and subcommands. (tailscale.com)
🧹 Nitpick comments (2)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (2)
22-30: Quote when counting configured shares.Avoid word-splitting bugs if the variable is empty.
- if (( 0 < $(wc -w <<< ${configured_share_names}) )); then + if (( $(wc -w <<< "${configured_share_names:-}") > 0 )); thenThe capability check for drive:share is the right signal before attempting share ops. (tailscale.com)
40-40: Don’t overwrite /config; guard symlink creation./config typically exists in HA add-ons; unguarded ln -s can fail or override expected mounts.
- ln -s "/homeassistant" "/config" + if [[ ! -e "/config" ]] && [[ -d "/homeassistant" ]]; then + ln -s "/homeassistant" "/config" + fi
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tailscale/DOCS.md(3 hunks)tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up(1 hunks)tailscale/translations/en.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type
- tailscale/DOCS.md
- tailscale/config.yaml
- tailscale/translations/en.yaml
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-09T10:32:14.110Z
Learnt from: mikz
PR: hassio-addons/addon-tailscale#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/taildrive/run
📚 Learning: 2024-11-15T17:38:56.576Z
Learnt from: lmagyar
PR: hassio-addons/addon-tailscale#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/taildrive/run
🔇 Additional comments (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (1)
57-57: CLI usage is correct here.Keep ‘/opt/tailscale drive unshare "${share_name}"’; that’s the documented subcommand.
Reference. (tailscale.com)
|
@lmagyar you can take over this if you want to. I'm running home assistant as a nix service nowadays, so I'm not invested in this add-on anymore. |
|
OK, I will review the coderabbitai's suggestions and mark as ready for review. |
|
I've reviewed all the AI reviews, it's ready for human review. |
|
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
stale
|
The last commit (adding |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (3)
28-28: Consider using string non-emptiness check instead of word count.Using
wc -won a newline-separated list works but is less semantically clear. A simpler check would be:- if (( 0 < $(wc -w <<< ${configured_share_names}) )); then + if [[ -n "${configured_share_names}" ]]; thenThis directly checks if any shares are configured without relying on word-counting behavior.
43-51: Add error handling for share operations.The share command could fail (e.g., permission denied, network issues) without clear indication to the user.
Apply this diff to add error handling:
for share_name in ${configured_share_names}; do if ! grep -Fxq -- "${share_name}" <<< "${active_share_names}"; then bashio::log.info "Taildrive: sharing /${share_name}" - /opt/tailscale drive share "${share_name}" "/${share_name}" + if ! /opt/tailscale drive share "${share_name}" "/${share_name}"; then + bashio::log.error "Failed to share /${share_name}" + fi else bashio::log.info "Taildrive: /${share_name} is already shared" fi done
53-63: Add error handling for unshare operations.Similar to the share loop, the unshare command could fail without clear user notification.
Apply this diff to add error handling:
for share_name in ${active_share_names}; do if ! grep -Fxq -- "${share_name}" <<< "${configured_share_names}"; then if grep -Fxq -- "${share_name}" <<< "${all_share_names}"; then bashio::log.info "Taildrive: unsharing /${share_name}" - /opt/tailscale drive unshare "${share_name}" + if ! /opt/tailscale drive unshare "${share_name}"; then + bashio::log.error "Failed to unshare /${share_name}" + fi else bashio::log.info "Taildrive: skipping to unshare '${share_name}'" fi fi done
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
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.
📚 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/taildrive/run
📚 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/taildrive/run
📚 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/s6-rc.d/taildrive/run
📚 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/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run
Share homeassistant directories to your Tailnet using Taildrive.
Fix parsing Tailscale drive data. Fix homeassistant share config.
I went with exit.nok, let it fail as taildrop fails on startup.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (1)
28-28: Quoteconfigured_share_namesin thewctest for robustnessLine 28 currently relies on unquoted expansion:
if (( 0 < $(wc -w <<< ${configured_share_names}) )); thenSafer and more idiomatic:
if (( 0 < $(wc -w <<< "${configured_share_names}") )); thenThis avoids any future surprises from word splitting or globbing if keys ever change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
tailscale/DOCS.md(3 hunks)tailscale/config.yaml(2 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/type(1 hunks)tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up(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/taildrive/type
🚧 Files skipped from review as they are similar to previous changes (3)
- tailscale/translations/en.yaml
- tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/up
- tailscale/config.yaml
🧰 Additional context used
🧠 Learnings (7)
📚 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/taildrive/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/s6-rc.d/taildrive/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/s6-rc.d/taildrive/run
📚 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/s6-rc.d/taildrive/run
📚 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/rootfs/etc/s6-overlay/s6-rc.d/taildrive/runtailscale/DOCS.md
📚 Learning: 2025-11-06T11:15:56.108Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 509
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run:41-41
Timestamp: 2025-11-06T11:15:56.108Z
Learning: In Home Assistant Docker add-ons, runtime changes to the container filesystem (such as symlinks, created files, or directory modifications) are not persisted between container restarts. The filesystem starts fresh on each run, so operations like `ln -s` don't need idempotent checks for pre-existing targets.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run
📚 Learning: 2025-09-15T22:13:04.962Z
Learnt from: lmagyar
Repo: hassio-addons/addon-tailscale PR: 455
File: tailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/run:54-54
Timestamp: 2025-09-15T22:13:04.962Z
Learning: In Home Assistant Docker add-ons, files like `/etc/resolv.dnsmasq.conf` are ephemeral and get deleted/recreated on container restart, so using `mv` instead of `cp` for one-time startup operations is appropriate since the source file will be regenerated on the next restart.
Applied to files:
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: workflows / Lint Add-on
🔇 Additional comments (4)
tailscale/rootfs/etc/s6-overlay/s6-rc.d/taildrive/run (2)
16-39: Taildrive capability and config parsing look correctReading
taildrivevia jq and gating on.Self.CapMap["drive:share"]before callingdrive listis coherent with the schema and avoids hitting Taildrive when the capability is absent. The error paths andbashio::exit.nokusage are also appropriate for a oneshot sync service.
41-67: Share/unshare logic correctly limits scope to add-on–managed sharesThe loops over
configured_share_namesandactive_share_namescorrectly:
- Share configured-but-inactive names, and
- Unshare active-but-unconfigured names only when present in
all_share_names,so user-created Taildrive shares are preserved while add-on-managed ones are reconciled. Logging is clear and should make behavior easy to debug.
tailscale/DOCS.md (2)
77-84: Config snippet fortaildrivematches implementationThe new
taildriveblock in the sample configuration uses the same keys the run script and schema expect and defaults everything tofalse, which is a sensible, non-surprising default.
331-339: Taildrive option section and link are clear and consistentThe
Option: \taildrive`section succinctly explains the feature, and the new[tailscale_info_taildrive]` reference correctly points to the upstream docs and is actually used, so markdownlint should be happy. Structure is consistent with the other option sections.Also applies to: 482-482
This doesn't protect against word splitting here, wc doesn't care, but it is OK for But this is a typo in the for loops, already fixed in the last commit. |
3 participants
Proposed Changes
Share homeassistant directories to your Tailnet using Taildrive.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.