[PATCH] Make vpn-select modular
Adrian Schmutzler
mail at adrianschmutzler.de
Sa Aug 8 01:55:36 CEST 2020
Hallo,
gefällt mir generell gut, ein paar Kommentare unten.
> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces at freifunk.net] On Behalf
> Of Robert Langhammer
> Sent: Freitag, 7. August 2020 01:33
> To: franken-dev at freifunk.net
> Subject: [PATCH] Make vpn-select modular
>
> vpn-select is an old relic and did not reflect the opportunities of our hoodfile.
> This rewrite makes vpn-select modular to easely add new vpn-protocols.
>
> The stuff dependent on the vpn-protocol is outsourced to files in /etc/vpn-
> select.d and comes in with the respective vpn package.
>
> vpn-stop is removed to use the protocol independent start/stop mechanism
> of vpn-select.
>
> Signed-off-by: Robert Langhammer <rlanghammer at web.de>
> ---
> .../fff-fastd/files/etc/vpn-select.d/fastd | 35 +++++++++
> .../fff-hoods/files/usr/sbin/configurehood | 2 +-
> .../fff-vpn-select/files/usr/sbin/vpn-select | 75 +++++++------------
> .../fff-vpn-select/files/usr/sbin/vpn-stop | 5 --
> 4 files changed, 62 insertions(+), 55 deletions(-) create mode 100644
> src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> delete mode 100755 src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
>
> diff --git a/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> new file mode 100644
> index 0000000..bd73761
> --- /dev/null
> +++ b/src/packages/fff/fff-fastd/files/etc/vpn-select.d/fastd
> @@ -0,0 +1,35 @@
> +protocol=fastd
> +
> +fastd_clear() {
> + rm /tmp/fastd_fff_peers/*
> +}
> +
> +fastd_addpeer() {
> + [ -d /tmp/fastd_fff_peers ] || mkdir /tmp/fastd_fff_peers
> +
> + # write fastd-config
> + json_get_var servername name
> + filename="/etc/fastd/fff/peers/$servername"
> + echo "#name \"${servername}\";" > "$filename"
> + json_get_var key key
> + echo "key \"${key}\";" >> "$filename"
> + json_get_var address address
> + json_get_var port port
> + echo "remote \"${address}\" port ${port};" >> "$filename"
> + echo "" >> "$filename"
> + echo "float yes;" >> "$filename"
> +}
> +
> +fastd_start_stop() {
Den Namen finde ich nicht optimal, mir fällt allerdings auch nichts ein, was jetzt substantiell besser wäre.
Ich frage mich zudem, ob ein vergleichbares Konzept für andere Dienste auch funktioniert (also ohne Trennung von start und stop).
> + /etc/init.d/fastd reload # does nothing if fastd was not running
> +
> + # fastd start/stop for various situations
> + # this is needed for first start and if fastd comes up or disappears in
> hoodfile
> + pidfile="/tmp/run/fastd.fff.pid"
> + if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
> + ([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
> /etc/init.d/fastd start
> + else
> + ([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
> /etc/init.d/fastd stop
> + fi
Hier bin ich mir nicht ganz sicher, ob das wirklich den vpn-stop immer ersetzt. Wenn ja, wäre das eine nette Code-Reduktion :-)
> +}
> +
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 3b92cbc..c84a8cc 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -207,7 +207,7 @@ if [ -s "$hoodfiletmp" ]; then
> if hasInternet ; then
> /usr/sbin/vpn-select "$hoodfiletmp"
> else
> - /usr/sbin/vpn-stop
> + /usr/sbin/vpn-select stop-VPN
Das finde ich nicht schön. Der erste Parameter ist ein Pfad.
Allerdings könnte man hier einfach den Parameter weglassen und würde effektiv zum gewünschten Ergebnis kommen.
> fi
>
> # now we load the prefix from the hoodfile and set this to br-mesh
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> index 30883f5..8f48f9a 100755
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> +++ b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-select
> @@ -1,65 +1,42 @@
> #!/bin/sh
>
> # Usage: vpn-select <path-to-hood-file>
> +# To add a new protocol, put a file with three functions to /etc/vpn-
> select.d/ .
> +# The file must start with protocol=name. It is most important to use the
> same name here and in hoodfile.
> +# The old config can be cleared in function ${protocol}_clear(). It is called
> once per installed protocol.
> +# The function ${protocol}_addpeer() is called for every peer in hoodfile.
> +# The function ${protocol}_start_stop() is called once per installed protocol.
>
> . /usr/share/libubox/jshn.sh
>
> hoodfile="$1"
>
> -make_config() {
> - # remove old config
> - rm /tmp/fastd_fff_peers/*
> +# source functions
> +for file in /etc/vpn-select.d/*; do
> + . "$file"
Hier sollte man vorher prüfen, ob der Ordner leer ist. Ist zwar unwahrscheinlich, aber ich möchte gerne ein
". /etc/vpn-select.d/*" vermeiden.
> + supported_protocols="$supported_protocols $protocol"
> +done
>
> - # prepare
> - Index=1
> +# clear old config
> +for protocol in $supported_protocols; do
> + "${protocol}_clear"
> +done
> +
> +# load hoodfile and add peers
> +if [ -s "$hoodfile" ] ; then
Hier sollte man prüfen, ob $hoodfile überhaupt einen String enthält. Prinzipiell sind hier zwei Ansätze möglich:
1. Man prüft [ -n $hoodfile] gleich am Anfang und beendet das Skript, wenn false.
2. Man prüft hier [ -n $hoofile ] && [ -s $hoodfile ], und erschlägt damit das vpn-stop Szenario gleich mit. Wird keine Argument übergeben (kein Hoodfile), würde also hier gelöscht und start-stop gemacht, aber keine Peers erzeugt. Problem gelöst.
Ich würde aber in jedem Fall den Check mit -n ergänzen, da ich nicht weiß, was für spannende Sachen mit [ -s "" ] passieren können.
> json_load "$(cat "$hoodfile")"
> json_select vpn
> -
> - # get fastd peers
> - while json_select "$Index" > /dev/null
> - do
> + index=1
> + while json_select "$index" > /dev/null ; do
Wenn wir den index nicht direkt brauchen (wie früher bei l2tp), dann geht das eleganter (habe ich hier https://github.com/openwrt/openwrt/blob/master/package/base-files/files/lib/upgrade/fwtool.sh#L61 gefunden):
json_select vpn
json_get_keys vpn_keys
for k in $vpn_keys; do
json_select $k
json_get_var protocol protocol
....
json_select ..
done
Damit werden wir den Index los und iterieren nur über die tatsächlichen Elemente des arrays (habs gerade auch lokal getestet).
Grüße
Adrian
> json_get_var protocol protocol
> - if [ "$protocol" = "fastd" ]; then
> - # set up fastd
> - json_get_var servername name
> - filename="/etc/fastd/fff/peers/$servername"
> - echo "#name \"${servername}\";" > "$filename"
> - json_get_var key key
> - echo "key \"${key}\";" >> "$filename"
> - json_get_var address address
> - json_get_var port port
> - echo "remote \"${address}\" port ${port};" >>
> "$filename"
> - echo "" >> "$filename"
> - echo "float yes;" >> "$filename"
> - fi
> + "${protocol}_addpeer" || echo "protocol $protocol
> unknown"
> json_select ".." # back to vpn
> - Index=$(( Index + 1 ))
> + index=$(( index + 1 ))
> done
> - json_select ".." # back to root
> -}
> +fi
>
> -# Only do something if file is there and not empty; otherwise exit 1 -if [ -s
> "$hoodfile" ]; then
> - if [ ! -d /tmp/fastd_fff_peers ]; then
> - # first run after reboot
> - mkdir /tmp/fastd_fff_peers
> - make_config
> - # start fastd only if there are some peers
> - [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ] &&
> /etc/init.d/fastd start
> - else
> - make_config
> - /etc/init.d/fastd reload
> +# start/restart/stop vpnservices
> +for protocol in $supported_protocols; do
> + "${protocol}_start_stop"
> +done
>
> - # fastd start/stop for various situations
> - pidfile="/tmp/run/fastd.fff.pid"
> - if [ "$(ls /etc/fastd/fff/peers/* 2>/dev/null)" ]; then
> - ([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) ||
> /etc/init.d/fastd start
> - else
> - ([ -s "$pidfile" ] && [ -d "/proc/$(cat "$pidfile")" ]) &&
> /etc/init.d/fastd stop
> - fi
> - fi
> - exit 0
> -else
> - echo "vpn-select: Hood file not found or empty!"
> - exit 1
> -fi
> diff --git a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> b/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> deleted file mode 100755
> index 03a160b..0000000
> --- a/src/packages/fff/fff-vpn-select/files/usr/sbin/vpn-stop
> +++ /dev/null
> @@ -1,5 +0,0 @@
> -#!/bin/sh
> -
> -rm /tmp/fastd_fff_peers/*
> -/etc/init.d/fastd stop
> -
> --
> 2.20.1
-------------- nächster Teil --------------
Ein Dateianhang mit Binärdaten wurde abgetrennt...
Dateiname : openpgp-digital-signature.asc
Dateityp : application/pgp-signature
Dateigröße : 834 bytes
Beschreibung: nicht verfügbar
URL : <https://lists.freifunk.net/pipermail/franken-dev-freifunk.net/attachments/20200808/5fc98228/attachment.sig>
Mehr Informationen über die Mailingliste franken-dev