Merge pull request #85223 from arianvp/acme-fix-nginx-after

nixos/acme: Fix ordering of certificate requests (#81482)

3 files changed, 50 insertions, 17 deletions

diff --git a/nixos/modules/services/web-servers/apache-httpd/default.nix b/nixos/modules/services/web-servers/apache-httpd/default.nix
index 8abee7130d7..e1d1217943b 100644
--- a/nixos/modules/services/web-servers/apache-httpd/default.nix
+++ b/nixos/modules/services/web-servers/apache-httpd/default.nix
@@ -708,6 +708,7 @@ in
         wantedBy = [ "multi-user.target" ];
         wants = concatLists (map (hostOpts: [ "acme-${hostOpts.hostName}.service" "acme-selfsigned-${hostOpts.hostName}.service" ]) vhostsACME);
         after = [ "network.target" "fs.target" ] ++ map (hostOpts: "acme-selfsigned-${hostOpts.hostName}.service") vhostsACME;
+        before = map (hostOpts: "acme-${hostOpts.hostName}.service") vhostsACME;
 
         path = [ pkg pkgs.coreutils pkgs.gnugrep ];
 
diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix
index 8a015bb3556..4c4b7f39e6b 100644
--- a/nixos/modules/services/web-servers/nginx/default.nix
+++ b/nixos/modules/services/web-servers/nginx/default.nix
@@ -693,6 +693,10 @@ in
       wantedBy = [ "multi-user.target" ];
       wants = concatLists (map (vhostConfig: ["acme-${vhostConfig.serverName}.service" "acme-selfsigned-${vhostConfig.serverName}.service"]) acmeEnabledVhosts);
       after = [ "network.target" ] ++ map (vhostConfig: "acme-selfsigned-${vhostConfig.serverName}.service") acmeEnabledVhosts;
+      # Nginx needs to be started in order to be able to request certificates
+      # (it's hosting the acme challenge after all)
+      # This fixes https://github.com/NixOS/nixpkgs/issues/81842
+      before = map (vhostConfig: "acme-${vhostConfig.serverName}.service") acmeEnabledVhosts;
       stopIfChanged = false;
       preStart = ''
         ${cfg.preStart}
diff --git a/nixos/tests/acme.nix b/nixos/tests/acme.nix
index fc41dc1eb5f..a8188473721 100644
--- a/nixos/tests/acme.nix
+++ b/nixos/tests/acme.nix
@@ -48,10 +48,9 @@ in import ./make-test-python.nix ({ lib, ... }: {
       security.acme.certs."standalone.test" = {
         webroot = "/var/lib/acme/acme-challenges";
       };
-      systemd.targets."acme-finished-standalone.test" = {};
-      systemd.services."acme-standalone.test" = {
-        wants = [ "acme-finished-standalone.test.target" ];
-        before = [ "acme-finished-standalone.test.target" ];
+      systemd.targets."acme-finished-standalone.test" = {
+        after = [ "acme-standalone.test.service" ];
+        wantedBy = [ "acme-standalone.test.service" ];
       };
       services.nginx.enable = true;
       services.nginx.virtualHosts."standalone.test" = {
@@ -68,11 +67,9 @@ in import ./make-test-python.nix ({ lib, ... }: {
 
       # A target remains active. Use this to probe the fact that
       # a service fired eventhough it is not RemainAfterExit
-      systemd.targets."acme-finished-a.example.test" = {};
-      systemd.services."acme-a.example.test" = {
-        wants = [ "acme-finished-a.example.test.target" ];
-        before = [ "acme-finished-a.example.test.target" ];
-        after = [ "nginx.service" ];
+      systemd.targets."acme-finished-a.example.test" = {
+        after = [ "acme-a.example.test.service" ];
+        wantedBy = [ "acme-a.example.test.service" ];
       };
 
       services.nginx.enable = true;
@@ -89,11 +86,9 @@ in import ./make-test-python.nix ({ lib, ... }: {
       security.acme.server = "https://acme.test/dir";
 
       specialisation.second-cert.configuration = {pkgs, ...}: {
-        systemd.targets."acme-finished-b.example.test" = {};
-        systemd.services."acme-b.example.test" = {
-          wants = [ "acme-finished-b.example.test.target" ];
-          before = [ "acme-finished-b.example.test.target" ];
-          after = [ "nginx.service" ];
+        systemd.targets."acme-finished-b.example.test" = {
+          after = [ "acme-b.example.test.service" ];
+          wantedBy = [ "acme-b.example.test.service" ];
         };
         services.nginx.virtualHosts."b.example.test" = {
           enableACME = true;
@@ -104,6 +99,7 @@ in import ./make-test-python.nix ({ lib, ... }: {
           '';
         };
       };
+
       specialisation.dns-01.configuration = {pkgs, config, nodes, lib, ...}: {
         security.acme.certs."example.test" = {
           domain = "*.example.test";
@@ -115,10 +111,12 @@ in import ./make-test-python.nix ({ lib, ... }: {
           user = config.services.nginx.user;
           group = config.services.nginx.group;
         };
-        systemd.targets."acme-finished-example.test" = {};
+        systemd.targets."acme-finished-example.test" = {
+          after = [ "acme-example.test.service" ];
+          wantedBy = [ "acme-example.test.service" ];
+        };
         systemd.services."acme-example.test" = {
-          wants = [ "acme-finished-example.test.target" ];
-          before = [ "acme-finished-example.test.target" "nginx.service" ];
+          before = [ "nginx.service" ];
           wantedBy = [ "nginx.service" ];
         };
         services.nginx.virtualHosts."c.example.test" = {
@@ -132,6 +130,26 @@ in import ./make-test-python.nix ({ lib, ... }: {
           '';
         };
       };
+
+      # When nginx depends on a service that is slow to start up, requesting used to fail
+      # certificates fail.  Reproducer for https://github.com/NixOS/nixpkgs/issues/81842
+      specialisation.slow-startup.configuration = { pkgs, config, nodes, lib, ...}: {
+        systemd.services.my-slow-service = {
+          wantedBy = [ "multi-user.target" "nginx.service" ];
+          before = [ "nginx.service" ];
+          preStart = "sleep 5";
+          script = "${pkgs.python3}/bin/python -m http.server";
+        };
+        systemd.targets."acme-finished-d.example.com" = {
+          after = [ "acme-d.example.com.service" ];
+          wantedBy = [ "acme-d.example.com.service" ];
+        };
+        services.nginx.virtualHosts."d.example.com" = {
+          forceSSL = true;
+          enableACME = true;
+          locations."/".proxyPass = "http://localhost:8000";
+        };
+      };
     };
 
     client = {nodes, lib, ...}: {
@@ -207,5 +225,15 @@ in import ./make-test-python.nix ({ lib, ... }: {
           client.succeed(
               "curl --cacert /tmp/ca.crt https://c.example.test/ | grep -qF 'hello world'"
           )
+
+      with subtest("Can request certificate of nginx when startup is delayed"):
+          webserver.succeed(
+              "${switchToNewServer}"
+          )
+          webserver.succeed(
+              "/run/current-system/specialisation/slow-startup/bin/switch-to-configuration test"
+          )
+          webserver.wait_for_unit("acme-finished-d.example.com.target")
+          client.succeed("curl --cacert /tmp/ca.crt https://d.example.com/")
     '';
 })