Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zerotier: split configuration #24773

Merged
merged 1 commit into from
Sep 17, 2024
Merged

zerotier: split configuration #24773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion net/zerotier/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ include $(TOPDIR)/rules.mk

PKG_NAME:=zerotier
PKG_VERSION:=1.14.0
PKG_RELEASE:=1
PKG_RELEASE:=2

PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
PKG_SOURCE_URL:=https://codeload.github.com/zerotier/ZeroTierOne/tar.gz/$(PKG_VERSION)?
Expand Down Expand Up @@ -71,6 +71,7 @@ define Package/zerotier/install
$(INSTALL_BIN) $(PKG_BUILD_DIR)/zerotier-one $(1)/usr/bin/
$(LN) zerotier-one $(1)/usr/bin/zerotier-cli
$(LN) zerotier-one $(1)/usr/bin/zerotier-idtool
$(INSTALL_DIR) $(1)/etc/uci-defaults

ifeq ($(CONFIG_ZEROTIER_ENABLE_SELFTEST),y)
$(INSTALL_BIN) $(PKG_BUILD_DIR)/zerotier-selftest $(1)/usr/bin/
Expand Down
45 changes: 32 additions & 13 deletions net/zerotier/files/etc/config/zerotier
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,20 +1,39 @@

config zerotier sample_config
config zerotier 'global'
# Sets whether ZeroTier is enabled or not
option enabled 0

# persistent configuration folder (for ZT controller mode)
# Sets the ZeroTier listening port (default 9993; set to 0 for random)
#option port '9993'
# Client secret (leave blank to generate a secret on first run)
option secret ''
# Path of the optional file local.conf (see documentation at
# https://docs.zerotier.com/config#local-configuration-options)
#option local_conf_path '/etc/zerotier.conf'
# Persistent configuration directory (to perform other configurations such
# as controller mode or moons, etc.)
#option config_path '/etc/zerotier'
# copy <config_path> to RAM to prevent writing to flash (for ZT controller mode)
# Copy the contents of the persistent configuration directory to memory
# instead of linking it, this avoids writing to flash
#option copy_config_path '1'

#option port '9993'

# path to the local.conf
#option local_conf '/etc/zerotier.conf'
# Network configuration, you can have as many configurations as networks you
# want to join (the network name is optional)
config network 'mynet'
# Identifier of the network you wish to join
option id '8056c2e21c000001'
# Network configuration parameters (all are optional, if not indicated the
# default values are set, see documentation at
# https://docs.zerotier.com/config/#network-specific-configuration)
option allow_managed '1'
option allow_global '0'
option allow_default '0'
option allow_dns '0'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If allowing network specific config is the goal of this pull request, I think it's possible to extend current uci config while not breaking backward compatibility. join is still an config option in zerotier config section, with network specific configuration in added network section. E.g.

config zerotier sample_config
    ...
    list join 8056c2e21c000001

config network 8056c2e21c000001
#	option allow_managed '1'
#	option allow_global '0'
#	option allow_default '0'
#	option allow_dns '0'

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea really goes beyond that. What we pursue is what is commented in the MR, to simplify the configuration and the startup script and to eliminate a possibility (the one of starting multiple zerotier daemons) that we know that it does not work correctly and that in addition it does not make too much sense.

Copy link
Member

@yousong yousong Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limiting number of zerotier daemon to 1 is reasonable if it's known fact that multiple instances do not work. It's a fix, not going to break anyone's expectation.

From a user point of view, I do not see how the current zerotier uci config is more complex than the new one. Rewriting the init script is another thing though. It's impl detail. Compat is my concern. As a user really I do not intend to check the code unless something breaks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the reduction of complexity is for the future and possible new configuration options that may be included in the file. Besides, this configuration is more similar to the one used in other packages, so I understand that being something “more known” it will be less complex for the user.

In any case with the uci-defaults script we have added the configuration migration is covered. I have tested it a few times and it works correctly so there should be no more problems.

Copy link
Contributor Author

@mwarning mwarning Aug 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Multiple Zerotier instances are not useful, since a single instance can connect to multiple networks as well.
Also the current configuration does not allow any settings on a per network basis. This chance is going to fix this.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yousong it is true that with the example that you comment the current configuration would work as it is without changing, but having the migration script that does exactly the same it is preferable to go to the new configuration and leave the package ready for future possibilities.

Let's say that right now we do what you say, we leave a configuration that looks “weird” because I have to declare twice the network I want to join (first a “list join” and then a “network config”). In the next update we would have to change it again and again we would be at “square one”.

I fully understand the initial reluctance of not making the change without the migration script, but now that the /etc/uci-defaults/80-zt-migration is already included I don't see the problem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's say that right now we do what you say, we leave a configuration that looks “weird” because I have to declare twice the network I want to join (first a “list join” and then a “network config”). In the next update we would have to change it again and again we would be at “square one”.

Why "In the next update we would have to change it again"?

Looking at it from another angle, I see it as a convenience feature. Only those present in join list will be joined. We can easily join/leave a network by inserting/removing a #

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why "In the next update we would have to change it again"?

It is very likely that this will have to be done if new functionality is added.

Looking at it from another angle, I see it as a convenience feature. Only those present in join list will be joined. We can easily join/leave a network by inserting/removing a #

You can do the same by commenting out the only two mandatory lines when configuring a network, e.g:

#config network 'mynet'
#	option id '8056c2e21c000001'

The advantage of being able to name the networks is that they are easily identifiable by the user.

As I said before, having a migration script that adapts the current configuration to the new one, I don't see the problem.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we have not mentioned it, but one of the objectives we are looking for with this change is precisely the possibility that the user can name their networks and thus be able to identify them much more easily.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name option can be added to the network section if needed.

Well, concerns expressed and still I am not a fan of the incompat change. The migration script will be a unnecessary burden. That's it for me at the moment. Maintainers have the say.


# Generate secret on first start
option secret ''
# Example of a second network (unnamed as it is optional)
#config network
# option id '1234567890123456'
# option allow_managed '1'
# option allow_global '0'
# option allow_default '0'
# option allow_dns '0'

# Join a public network called Earth
list join '8056c2e21c000001'
#list join '<other_network>'
129 changes: 55 additions & 74 deletions net/zerotier/files/etc/init.d/zerotier
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,113 +7,94 @@ USE_PROCD=1
PROG=/usr/bin/zerotier-one
CONFIG_PATH=/var/lib/zerotier-one

section_enabled() {
config_get_bool enabled "$1" 'enabled' 0
[ $enabled -ne 0 ]
join_network() {
local section="${1}"
local id allow_managed allow_global allow_default allow_dns

config_get id "${section}" 'id'
config_get_bool allow_managed "${section}" 'allow_managed' 1
config_get_bool allow_global "${section}" 'allow_global' 0
config_get_bool allow_default "${section}" 'allow_default' 0
config_get_bool allow_dns "${section}" 'allow_dns' 0

if [ -n "${id}" ]; then
# an (empty) config file will cause ZT to join a network
touch "${CONFIG_PATH}"/networks.d/"${id}".conf
echo "allowManaged=${allow_managed}" > "${CONFIG_PATH}"/networks.d/"${id}".local.conf
echo "allowGlobal=${allow_global}" >> "${CONFIG_PATH}"/networks.d/"${id}".local.conf
echo "allowDefault=${allow_default}" >> "${CONFIG_PATH}"/networks.d/"${id}".local.conf
echo "allowDNS=${allow_dns}" >> "${CONFIG_PATH}"/networks.d/"${id}".local.conf
fi
}

start_instance() {
local cfg="$1"
local port secret config_path local_conf copy_config_path path
start_service() {
config_load zerotier
local enabled port secret local_conf_path config_path copy_config_path
local args=""

if ! section_enabled "$cfg"; then
config_get_bool enabled 'global' 'enabled' 0
config_get port 'global' 'port'
config_get secret 'global' 'secret'
config_get local_conf_path 'global' 'local_conf_path'
config_get config_path 'global' 'config_path'
config_get_bool copy_config_path 'global' 'copy_config_path' 0

if [ ${enabled} -eq 0 ]; then
echo "disabled in /etc/config/zerotier"
return 1
fi

config_get config_path $cfg 'config_path'
config_get port $cfg 'port'
config_get secret $cfg 'secret'
config_get local_conf $cfg 'local_conf'
config_get_bool copy_config_path $cfg 'copy_config_path' 0

path=${CONFIG_PATH}_$cfg

# Remove existing link or folder
rm -rf $path
rm -rf "${CONFIG_PATH}"

# Create link or copy files from CONFIG_PATH to config_path
if [ -n "$config_path" -a "$config_path" != "$path" ]; then
if [ ! -d "$config_path" ]; then
echo "ZeroTier config_path does not exist: $config_path" 1>&2
# Create link or copy files from config_path to CONFIG_PATH
if [ -n "${config_path}" ]; then
if [ ! -d "${config_path}" ]; then
echo "ZeroTier config_path does not exist: ${config_path}" 1>&2
return
fi

# ensure that the target exists
mkdir -p $(dirname $path)

if [ "$copy_config_path" = "1" ]; then
cp -r $config_path $path
if [ ${copy_config_path} -eq 1 ]; then
cp -r "${config_path}" "${CONFIG_PATH}"
else
ln -s $config_path $path
ln -s "${config_path}" "${CONFIG_PATH}"
fi
fi

mkdir -p $path/networks.d

# link latest default config path to latest config path
rm -f $CONFIG_PATH
ln -s $path $CONFIG_PATH
mkdir -p "${CONFIG_PATH}"/networks.d
config_foreach join_network network

if [ -n "$port" ]; then
args="$args -p${port}"
if [ -f "${local_conf_path}" ]; then
ln -s "${local_conf_path}" "${CONFIG_PATH}"/local.conf
fi

if [ -z "$secret" ]; then
echo "Generate secret - please wait..."
local sf="/tmp/zt.$cfg.secret"

zerotier-idtool generate "$sf" > /dev/null
[ $? -ne 0 ] && return 1

secret="$(cat $sf)"
rm "$sf"
if [ -n "${port}" ]; then
args="${args} -p${port}"
fi

uci set zerotier.$cfg.secret="$secret"
if [ -z "${secret}" ]; then
echo -n "Generating secret - please wait... "
secret="$(zerotier-idtool generate)"
[ ${?} -ne 0 ] && return 1
uci set zerotier.global.secret="${secret}"
uci commit zerotier
echo "done."
fi

if [ -n "$secret" ]; then
echo "$secret" > $path/identity.secret
if [ -n "${secret}" ]; then
echo "${secret}" > "${CONFIG_PATH}"/identity.secret
# make sure there is not previous identity.public
rm -f $path/identity.public
rm -f "${CONFIG_PATH}"/identity.public
fi

if [ -f "$local_conf" ]; then
ln -s "$local_conf" $path/local.conf
fi

add_join() {
# an (empty) config file will cause ZT to join a network
touch $path/networks.d/$1.conf
}

config_list_foreach $cfg 'join' add_join

procd_open_instance
procd_set_param command $PROG $args $path
procd_set_param command ${PROG} ${args}
procd_set_param stderr 1
procd_set_param respawn
procd_close_instance
}

start_service() {
config_load 'zerotier'
config_foreach start_instance 'zerotier'
}

stop_instance() {
local cfg="$1"

# Remove existing link or folder
rm -rf ${CONFIG_PATH}_${cfg}
}

stop_service() {
config_load 'zerotier'
config_foreach stop_instance 'zerotier'
rm -f ${CONFIG_PATH}
rm -rf "${CONFIG_PATH}"
}

reload_service() {
Expand Down
19 changes: 19 additions & 0 deletions net/zerotier/files/etc/uci-defaults/80-zt-migration
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Convert the join list into networks
nets=$(uci -q get zerotier.@zerotier[0].join)

if [ -n "$nets" ]; then
for net in ${nets}; do
sid=$(uci add zerotier network)
uci set zerotier.${sid}.id=${net}
done
uci delete zerotier.@zerotier[0].join

# Rename local conf (only if defined)
uci -q rename zerotier.@zerotier[0].local_conf='local_conf_path' || true

# Rename configuration to global
uci rename zerotier.@zerotier[0]='global'

# Commit all changes
uci commit zerotier
fi