mwarning / zerotier-openwrt

A OpenWrt package for ZeroTier One - Pull requests are welcome!
669 stars 140 forks source link

Improves configuration #120

Closed ogarcia closed 1 month ago

ogarcia commented 1 month ago

As discussed in #119 it may be time to change the ZeroTier configuration file slightly to support a number of global options and a number of per-network options. This way the startup script is slightly simplified and more configuration items can be specified per network.

Note that although this makes the current configurations incompatible, it is not a problem since you are supposed to review the configuration changes with each update. It also greatly improves the understanding of the configuration itself and opens up new possibilities in the future.

generalinterest commented 1 month ago

Hi ya,

Just following up here as I think that #119 could be allowed to continue dependently of this as the "allow" vars are network specific and not global?

My only thought on a Global section is that it only becomes worthwhile when there are multiple zerotier configs. My understanding is there are a limited number of reasons to have to set the global options to begin with. I would be hopeful that the "allow" changes would take care of a lot of it.

If there are multiple configs it then follows there will be multiple zerotier-one instances. In these setups there may well be different network configuration required which suggests they would need their own set of configuration files!

That's just my 2 cents, onwards and upwards!

ogarcia commented 1 month ago

@generalinterest I reply here

ogarcia commented 1 month ago

Analysis of the implications of this change

Actually, the main implications are twofold:

  1. The configuration is incompatible with the previous configuration.
  2. The ability to configure more than one instance of ZeroTier is lost since only one (global) is allowed to be configured.

Let's analyze both.

Configuration incompatibility

When switching from a multi-instance model to a single instance the current configuration no longer works. This should not be a problem since, unless the user has called global to its configuration, what will happen is that the current configuration will be ignored. In this case the message disabled in /etc/config/zerotier will also appear, which will already give an important clue to the user that his configuration is not working.

In the hypothetical case that the user has a configuration called global two things can happen.

  1. If the user has all the configuration in his own config_path he will not notice anything. It will boot without problems and will work (this is the only 100% backwards compatible case).
  2. Otherwise, the normal case is that the user has a list join 'XXXXX', as this is not read what will happen is that ZT will boot but will not join any network.

In any case an update of the ZeroTier package in OpenWrt usually only occurs when the router firmware is updated and it is the user's duty to check at each update whether the configuration is still compatible or not.

Multiple instances

Currently the configuration has one or more zerotier sections and each of them that is correctly configured starts a ZeroTier daemon. This ability is lost with the new configuration that only allows to have a single zerotier configuration (and therefore start a single daemon).

In my personal experience starting more than one ZT daemon in OpenWrt is problematic, it is possible that in very powerful computers it works correctly, but in all the ones I have tried it, it has not worked correctly. Besides ZT is designed so that it is not necessary to start more than one daemon, so IMHO this option is not necessary.

Conclusions

Clearly we must decide which course to take. Personally I think this is the most correct one because it makes the configuration and the scripts much easier and gives us the possibility to add new configurations (such as the moons theme) in the future. But obviously this is just my opinion.

mwarning commented 1 month ago

Thank you for the analysis and the work you have put in here. I agree with your conclusion. Let me gives this a quick test and then we can merge if there are not futher issues.

mwarning commented 1 month ago

ok, let's get this upstream.