YunoHost-Apps / garage_ynh

Garage package for YunoHost
https://garagehq.deuxfleurs.fr/
GNU Affero General Public License v3.0
1 stars 2 forks source link

Savagely editing the global nginx conf is "not cool" #4

Open alexAubin opened 1 year ago

alexAubin commented 1 year ago

cf https://github.com/YunoHost-Apps/garage_ynh/blob/master/scripts/install#L205

32238 ERROR Packagers /!\ This app manually modified some system configuration files! This should not happen! If you need to do so, you should implement a proper conf_regen hook. Those configuration were affected:
    - /etc/nginx/conf.d/sub.domain.tld.conf

The good way to do this is to have a regen conf hook, for example as done in

(not sure about having it in sources/hooks/conf_regen, maybe conf/ is simpler)

oiseauroch commented 1 year ago

Yeah, I saw the warning and then tried to create hook as explain in https://yunohost.org/en/packaging_apps_hooks. But the weird thing is that even if the hook is on the right place, it seems that its called only if regen_conf is called without argument (ex nginx) and the warning stay.

As I understand, I should better add the hook like in the postman app (and not as the doc say ?)

mrflos commented 1 year ago

Hello, I just got this very same warning about nginx conf changed manually, from the diagnosis email. I use this merged version 0.8.0~yhn4, and it's quite strange:

# yunohost tools regen-conf nginx --dry-run --with-diff
Warning: The configuration file '/etc/nginx/conf.d/garage.yeswiki.net.conf' has been manually modified and will not be updated
nginx: 
  applied: 
  pending: 
    /etc/nginx/conf.d/garage.yeswiki.net.conf: 
      diff: @@ -6,7 +6,7 @@
 server {
     listen 80;
     listen [::]:80;
-    server_name garage.yeswiki.net *.garage.yeswiki.net;
+    server_name garage.yeswiki.net;

     access_by_lua_file /usr/share/ssowat/access.lua;

@@ -35,7 +35,7 @@
 server {
     listen 443 ssl http2;
     listen [::]:443 ssl http2;
-    server_name garage.yeswiki.net *.garage.yeswiki.net;
+    server_name garage.yeswiki.net;

     include /etc/nginx/conf.d/security.conf.inc;
      status: modified

But after : # yunohost tools regen-conf nginx --force

# cat /etc/nginx/conf.d/garage.yeswiki.net.conf 
[..]
server_name garage.yeswiki.net *.garage.yeswiki.net;
[..]

It's like the dry-run doesn't wait for the hook to be applied and complains..

Thatoo commented 1 year ago

Is it possible to close this issue after merging https://github.com/YunoHost-Apps/garage_ynh/pull/5 ?

tituspijean commented 1 year ago

The hook still triggers the CI "error", but the test passes, so OK. :)

However you forgot to tweak the change_url script to update the hook (it has a __DOMAIN__ variable).