nixos/nginx: run nginx-config-reload as the nginx user and group

The "nginx-config-reload" script previously ran `nginx -t` as root, causing nginx to create files in /var/spool/nginx as the "nobody" user (which is the default if no user/group is specified in the nginx config itself). As further investigation showed [1], `nginx -t` does not only create files, but also tries to bind to sockets - which is a terrible thing to do to while just checking the configuration, which is why we removed the -t command from the nginx-config-reload command entirely, effectively aliasing it to systemctl reload nginx.service. Instead of all that, we now invoke `nginx -s reload` in the `ExecReload` command of `nginx.service`, which will move the unit into a failed state (but not stop the webserver). These failures are currently not properly propagated to the switch-to-configuration script, but `systemctl is-failed nginx` will return 0, and monitoring should detect this. [1]: https://trac.nginx.org/nginx/ticket/1506 Reported-By: Vincent Ambo <[email protected]>
NixOS · Apr 29, 2020 · ef78ca2 · ef78ca2
1 parent 3dbd3f2
commit ef78ca2
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 21 deletions.
diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix
@@ -698,7 +698,7 @@ in
       '';
       serviceConfig = {
         ExecStart = execCommand;
-        ExecReload = "${pkgs.coreutils}/bin/kill -HUP $MAINPID";
+        ExecReload = "${execCommand} -s reload";
         Restart = "always";
         RestartSec = "10s";
         StartLimitInterval = "1min";
@@ -721,18 +721,14 @@ in
       wants = [ "nginx.service" ];
       wantedBy = [ "multi-user.target" ];
       restartTriggers = [ configFile ];
-      # commented, because can cause extra delays during activate for this config:
-      #      services.nginx.virtualHosts."_".locations."/".proxyPass = "http://blabla:3000";
-      # stopIfChanged = false;
-      serviceConfig.Type = "oneshot";
-      serviceConfig.TimeoutSec = 60;
-      script = ''
-        if ${pkgs.systemd}/bin/systemctl -q is-active nginx.service ; then
-          ${execCommand} -t && \
-            ${pkgs.systemd}/bin/systemctl reload nginx.service
-        fi
-      '';
-      serviceConfig.RemainAfterExit = true;
+      serviceConfig = {
+        Type = "oneshot";
+        TimeoutSec = 60;
+        ExecStart = [
+          "/run/current-system/systemd/bin/systemctl reload nginx.service"
+        ];
+        RemainAfterExit = true;
+      };
     };
 
     security.acme.certs = filterAttrs (n: v: v != {}) (

diff --git a/nixos/tests/nginx.nix b/nixos/tests/nginx.nix
@@ -114,16 +114,13 @@ import ./make-test-python.nix ({ pkgs, ... }: {
         webserver.wait_for_unit("nginx")
         webserver.succeed("journalctl -u nginx | grep -q -i stopped")
 
-    with subtest("nixos-rebuild --switch should fail when there are configuration errors"):
-        webserver.fail(
+    with subtest(
+        "When switching to a broken configuration, the nginx unit should fail to reload"
+    ):
+        webserver.succeed(
             "${reloadWithErrorsSystem}/bin/switch-to-configuration test >&2"
         )
-        webserver.succeed("[[ $(systemctl is-failed nginx-config-reload) == failed ]]")
+        webserver.succeed("journalctl -u nginx | grep -q -i 'reload failed'")
         webserver.succeed("[[ $(systemctl is-failed nginx) == active ]]")
-        # just to make sure operation is idempotent. During development I had a situation
-        # when first time it shows error, but stops showing it on subsequent rebuilds
-        webserver.fail(
-            "${reloadWithErrorsSystem}/bin/switch-to-configuration test >&2"
-        )
   '';
 })