[syncthing] Improve configuration

[salim-b]

Changes:

• Use environment variables instead of CLI arguments where possible and properly separate config and database/state directories (related to make default folder persistent #447 (comment)).

  With this change, the add-on now mounts the new addon_config instead of config and stores Syncthing's

  • config in /config which maps to /mnt/data/supervisor/addon_configs/243ffc37_syncthing/ on the host (I think?).
  • internal database state in /data which maps to /mnt/data/supervisor/addons/data/243ffc37_syncthing/ on the host.

• Set HOME=/share as default folder path fallback, thus fix make default folder persistent #447.

• Slightly improve the logging in the s6 run script.

• Avoid creating a default folder when generating an initial configuration / starting for the first time by setting STNODEFAULTFOLDER=1. Syncthing shouldn't actually attempt to create a default folder since we set STCONFDIR and STDATADIR to paths that already exist. But in case anything goes south, STNODEFAULTFOLDER=1 ensures we don't start writing to arbitrary paths.

Note that I haven't actually tested this PR yet 😬

[salim-b]

Besides, I'm wondering whether we should limit the mounts (map in config.yaml) by

• mounting ssl read-only, i.e. remove the :rw
• or even completely removing ssl. Why does Syncthing need access to custom certs in the first place? We have Ingress for that, no?
• removing addons:rw. What is the use case for Syncthing having access to local add-ons? Not even the official File editor add-on has access to addons since it's deemed confusing for non-developers.

[tomaszduda23]

• /ssl seems to have other certificates as well e.g. zerotier
• it is confusing to have so many folders. I cannot find any documentation what is the purpose of each of them. It is hard at the beginning to understand which one is right to use
• On the other hand samba seems to expose all of them so maybe there is the reason to have access to those https://github.com/home-assistant/addons/blob/133709534a0bc853ee93a2315657b59caa67ca0e/samba/config.yaml#L18

After the change the default configuration will just work for most of the users. I would keep all the folder just in case if someone needs it.

[salim-b]

• it is confusing to have so many folders. I cannot find any documentation what is the purpose of each of them. It is hard at the beginning to understand which one is right to use

Yeah, I feel the same. The official add-on development documentation only states about the map config key:

List of Home Assistant directories to bind mount into your container. Possible values: homeassistant_config, addon_config, ssl, addons, backup, share, media, and all_addon_configs. Defaults to ro, which you can change by adding :rw to the end of the name.

The documentation of the official Samba add-on gives some more info about the purpose of these folders:

Directory	Description
addons	This is for your local add-ons.
backup	This is for your snapshots.
config	This is for your Home Assistant configuration.
media	This is for local media files.
share	This is for your data that is shared between add-ons and Home Assistant.
ssl	This is for your SSL certificates.

Note that config has been deprecated in favor of the new, more granular homeassistant_config, addon_config and all_addon_configs directories.

---

• On the other hand samba seems to expose all of them so maybe there is the reason to have access to those https://github.com/home-assistant/addons/blob/133709534a0bc853ee93a2315657b59caa67ca0e/samba/config.yaml#L18

The Samba add-on has access to all of these folders to provide users with a convenient way to edit config files, not because Samba itself needs access.

---

I would keep all the folder just in case if someone needs it.

I'd rather limit access to only the folders for which there's an actual Syncthing use case, i.e. backups, media and share. This would be less confusing for users and better security-wise.

[tomaszduda23]

I'd rather limit access to only the folders for which there's an actual Syncthing use case, i.e. backups, media and share. This would be less confusing for users and better security-wise.

syncthing can be also used to check/edit configuration like smb. You could just sync ha config, change it locally and reload.

[tomaszduda23]

btw is migration script needed? If it stop working after update it will be pretty annoying.

[salim-b]

syncthing can be also used to check/edit configuration like smb. You could just sync ha config, change it locally and reload.

Yeah, I thought about such a use case. But honestly, I don't think it is a good idea. Way too easy to accidentally delete your HA config. People should just use the official File editor or Studio Code Server add-ons or SSH into HA to directly edit config files.

Plus, mapping config/all_addon_configs also means it's very easy for users to accidentally expose sensitive stuff to their Syncthing remotes. To cite the blog post announcing the new addon_config mount:

Add-ons that map config have far more access than they should since config includes all secrets and credentials used in your Home Assistant integrations.

---

btw is migration script needed? If it stop working after update it will be pretty annoying.

Ideally yes. But I didn't look into it yet (didn't even test the PR yet).

I think all we need to do for proper migration is to check if there's a /data/config folder before launching Syncthing and if yes,

1. move config.xml, cert.pem, key.pem, https-cert.pem and https-key.pem to /config/, and

2. move everything else to /data/ (i.e. one directory level up).

Update: See 289da16

[Poeschl]

Thanks a lot guys for making those changes. I hope I can find some time this week to go through the MRs 💟

[Poeschl]

Due the merge conflicts I patched the changes manually ;) See #452

[salim-b]

Due the merge conflicts I patched the changes manually ;) See #452

@Poeschl Cool, thanks!

Did you actually test the changes by this PR? Because I didn't... 😬

[salim-b]

@Poeschl I just updated my HA instance to the latest add-on version 1.18.0 and noticed the config migration didn't work as expected because of an unsupported mv CLI flag, see #453.

This means the 1.18.0 version breaks people's Syncthing config. Merging #453 should resolve it, so please have a look ASAP. Thanks!