search

YunoHost-Apps / vpnclient_ynh

VPN Client app for YunoHost
GNU Affero General Public License v3.0
41 stars 25 forks source link

Generate config during upgrade #113

Closed hidrarga closed 10 months ago

hidrarga commented 1 year ago

Problem

As explained in #112, the config file isn't generated during the upgrade, so it's difficult to improve the config, as I did with the hook scripts introduced in #107

Solution

I propose to store the uploaded config file as client.cube or client.ovpn file, and generate the config again each time the app is upgraded.

As the file wasn't stored before these changes, I'm simply copying the client.conf to client.ovpn and starting from there. As a side effect, custom changes made by the user won't be overwritten for them.

The config file will be overwritten only for uploaded config file starting from this PR.

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)

hidrarga commented 1 year ago

!testme

yunohost-bot commented 1 year ago

:rocket: Test Badge

hidrarga commented 1 year ago

!testme

yunohost-bot commented 1 year ago

:sunflower: Test Badge

alexAubin commented 1 year ago

Hmmm not sure to understand all the "tenants et aboutissants" ;P I should sit and think carefully about this but naively i trust you on this ...

@zamentur : any opinion on this ?

hidrarga commented 1 year ago

I can try to explain, just tell me what is unclear :p

For now, I find it difficult to maintain an app where I can't be sure the changes I made to the config template will be propagated... For instance, the hook scripts I introduced earlier, well it's useless if people don't update their openvpn config… So probably the hotspot won't work correctly anymore, etc. But I'd be ok with another solution I guess ^^

On a side note, I'm not sure about the naming of "client.ovpn", because I'm a bit worried that this could confuse users, as this is close to "client.conf"… So I'd be happy with another name :)

hidrarga commented 10 months ago

@alexAubin @zamentur What should we do about this PR?

alexAubin commented 10 months ago

LGTM, let's merge to testing and then in master in a few days if nobody complains ;P