Openvpn hooks #118
Openvpn hooks #118
Conversation
|
!testme |
|
Alrighty! |
|
For the firewall issue, the trick is to use I needed that for upgrading the app… But otherwise it's not needed since I'm adding the VPN rules manually, which is faster than reloading the firewall. Unless we don't want updating firewall rules that way? |
scripts/backup
Outdated
| ynh_backup --src_path="/etc/openvpn/scripts/route-up.d/10-set-firewall" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-up.d/20-set-dns" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-up.d/30-set-server-ipv6-route" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-up.d/40-set-ipv6" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-down.d/10-unset-firewall" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-down.d/20-unset-dns" | ||
| ynh_backup --src_path="/etc/openvpn/scripts/route-down.d/30-unset-server-ipv6-route" |
There was a problem hiding this comment.
hmm, don't we want to somehow backup/restore the entire folder ? to also include user-provided hook ? Or would that create funky stuff with hotspot maybe ?
There was a problem hiding this comment.
Yes exactly :/
As a workaround, we could follow some naming scheme, for instance XX-vpnclient-function and backup everything that contains vpnclient? or backup everything except when it matches another app name?
There was a problem hiding this comment.
@alexAubin What are your thoughts about this?
There was a problem hiding this comment.
No strong opinion, if the current code works let's go, if you think having the -vpnclient- intermediate prefix is better design, feel free to implement it 👍
There was a problem hiding this comment.
I'll implement the prefix, it will be easier to improve the backup later on.
Although for the hotspot, I didn't use a prefix, so we will need to change that 🤷
There was a problem hiding this comment.
Hmm I don't know how I can restore the files… I can't restore the whole hook dirs because it will erase the hotspot script… and I can't restore each file separately because I was using the pattern *-vpnclient-* to backup them.
Sounds like an interesting twist ... I don't have enough insight to tell if there was a good reason for the initial design ... to me naively it sounds like using ping @zamentur in case you have time and insight about this ... |
|
if i understand, the post-iptables hook is keeped in order to manage the case when the admin reload the firewall, and in more we have openvpn hooks. LGTM Thanks to all people who work on this :) |
|
!testme |
|
🎠 |
|
!testme |
|
Alrighty! |
|
!testme |
|
Fingers crossed! |
Problem
As said in #116, I'm moving what's doing ynh-vpnclient service to OpenVPN hooks.
Solution
There are a lot of changes, sorry :/
I had to remove the
yunohost firewall reloadstuff because there are lock issues with it. Instead I'm adding / removing rules manually. This isn't too complex I think, and it's faster, so why not?I'm also improving some stuff, such as the
.ynh-vpnclient-startedfile which wasn't properly erased on error, or which wasn't checked by the ynh-vpnclient-checker timer…I'm removing the
ynh-vpnclient status, and I moved it's content inside the hooks. Honestly I don't even know if this used, or maybe for debugging purpose?Finally, I changed the priority of the hook
20-set-ipv6in order to run it after the firewall rules and the DNS configuration.PR Status
Automatic tests
Automatic tests can be triggered on https://ci-apps-dev.yunohost.org/ after creating the PR, by commenting "!testme", "!gogogadgetoci" or "By the power of systemd, I invoke The Great App CI to test this Pull Request!". (N.B. : for this to work you need to be a member of the Yunohost-Apps organization)