Unverified Commit b3a76d49 authored by Leona Maroni's avatar Leona Maroni
Browse files

nixos/nginx: allow adding new ACME certificates without nginx restart 

Currently, nginx gets restarted when adding a new ACME certificate, even
when `services.nginx.enableReload = true` because of changes in the
Wants/After/Before sections of `nginx.service`.
This change moves these dependencies to `nginx-config-reload.service` and
the respective ACME systemd units.
parent b76c36dc

nixos/modules/services/web-servers/nginx/default.nix

+187 −149
Original line number Diff line number Diff line
@@ -1491,17 +1491,22 @@ in 
      };

    };



    systemd.services.nginx = {

    systemd.services = {

      nginx = {

        description = "Nginx Web Server";

        wantedBy = [ "multi-user.target" ];

      wants = concatLists (map (certName: [ "acme-${certName}.service" ]) vhostCertNames);

        wants = lib.optionals (!cfg.enableReload) (

          concatLists (map (certName: [ "acme-${certName}.service" ]) vhostCertNames)

        );

        after = [

          "network.target"

        ]

        # Ensure nginx runs with baseline certificates in place.

      ++ map (certName: "acme-${certName}.service") vhostCertNames;

        ++ lib.optionals (!cfg.enableReload) (map (certName: "acme-${certName}.service") vhostCertNames);

        # Ensure nginx runs (with current config) before the actual ACME jobs run

      before = map (certName: "acme-order-renew-${certName}.service") vhostCertNames;

        before = lib.optionals (!cfg.enableReload) (

          map (certName: "acme-order-renew-${certName}.service") vhostCertNames

        );

        stopIfChanged = false;

        preStart = ''

          ${cfg.preStart}
@@ -1589,26 +1594,25 @@ in 
        };

      };



    environment.etc."nginx/nginx.conf" = mkIf cfg.enableReload {

      source = configFile;

    };



      # This service waits for all certificates to be available

      # before reloading nginx configuration.

      # sslTargets are added to wantedBy + before

      # which allows the acme-order-renew-$cert.service to signify the successful updating

      # of certs end-to-end.

    systemd.services.nginx-config-reload =

      nginx-config-reload =

        let

          sslServices = map (certName: "acme-${certName}.service") vhostCertNames;

          sslOrderRenewServices = map (certName: "acme-order-renew-${certName}.service") vhostCertNames;

        in

        mkIf (cfg.enableReload || vhostCertNames != [ ]) {

          wants = optionals cfg.enableReload [ "nginx.service" ];

        wantedBy = sslOrderRenewServices ++ [ "multi-user.target" ];

        # XXX Before the finished targets, after the renew services.

        # This service might be needed for HTTP-01 challenges, but we only want to confirm

        # certs are updated _after_ config has been reloaded.

        after = sslOrderRenewServices;

          # Reload config directly after the self-signed certificates have been requested

          # This is required for HTTP-01 ACME challenges, as the vHost with `.well-known/acme-challenge`

          # must already exist. Another reload with the actual certificate is triggered

          # with `security.acme.certs.<...>.reloadServices`

          wantedBy = [ "multi-user.target" ] ++ optionals cfg.enableReload sslServices;

          after = optionals cfg.enableReload sslServices;

          before = optionals cfg.enableReload sslOrderRenewServices;

          restartTriggers = optionals cfg.enableReload [ configFile ];

          # Block reloading if not all certs exist yet.

          # Happens when config changes add new vhosts/certs.
@@ -1629,15 +1633,43 @@ in 
            ExecStart = "/run/current-system/systemd/bin/systemctl reload nginx.service";

          };

        };

    }

    # When reload is enabled, add the systemd dependency to the acme unit to prevent restarts

    # of the nginx.service unit.

    # This needs to be here, because of how switch-to-configuration works in the case that nginx.service

    # is not started at the moment where new certificates are requested.

    # Configuring this relationship in nginx.service would lead to s-t-c restarting nginx.service

    # when a new certificate is added, as it will restart a unit when their direct unit properties,

    # including After and Wants, change.

    // lib.optionalAttrs cfg.enableReload (

      lib.listToAttrs (

        map (

          name:

          lib.nameValuePair "acme-${name}" {

            before = [ "nginx.service" ];

            wantedBy = [ "nginx.service" ];

          }

        ) vhostCertNames

      )

    );



    environment.etc."nginx/nginx.conf" = mkIf cfg.enableReload {

      source = configFile;

    };



    security.acme.certs =

      let

        acmePairs = map (

        # Here are two cases:

        # - when no `useACMEHost` is set, the `serverName` acme certificate is the primary name and we need to configure it

        # - when `useACMEHost` is set, this is also the primary name and we only need to configure the reloadServices property

        acmePairs =

          map (

            vhostConfig:

            let

              hasRoot = vhostConfig.acmeRoot != null;

            in

            nameValuePair vhostConfig.serverName {

              reloadServices = [ "nginx.service" ];

              group = mkDefault cfg.group;

              # if acmeRoot is null inherit config.security.acme

              # Since config.security.acme.certs.<cert>.webroot's own default value
@@ -1648,7 +1680,13 @@ in 
              extraDomainNames = vhostConfig.serverAliases;

              # Filter for enableACME-only vhosts. Don't want to create dud certs

            }

        ) (filter (vhostConfig: vhostConfig.useACMEHost == null) acmeEnabledVhosts);

          ) (filter (vhostConfig: vhostConfig.useACMEHost == null) acmeEnabledVhosts)

          ++ map (

            vhostConfig:

            nameValuePair vhostConfig.useACMEHost {

              reloadServices = [ "nginx.service" ];

            }

          ) (filter (vhostConfig: vhostConfig.useACMEHost != null) acmeEnabledVhosts);

      in

      listToAttrs acmePairs;

nixos/tests/acme/webserver.nix

+14 −1
Original line number Diff line number Diff line
@@ -226,5 +226,18 @@ 
          # Ensure the timer works, due to our shenanigans with

          # RemainAfterExit=true

          webserver.wait_until_succeeds(f"journalctl --cursor-file=/tmp/cursor | grep 'Starting Order (and renew) ACME certificate for zeroconf3.{domain}...'")

    ''

    +

      lib.optionalString

        (config.nodes.webserver.services.nginx.enable && config.nodes.webserver.services.nginx.enableReload)

        ''

          with subtest("Ensure that adding a second vhost does not restart nginx"):

              switch_to(webserver, "zeroconf")

              webserver.wait_for_unit("renew-triggered.target")

              webserver.succeed("journalctl --cursor-file=/tmp/cursor")

              switch_to(webserver, "zeroconf3")

              webserver.wait_for_unit("renew-triggered.target")

              output = webserver.succeed("journalctl --cursor-file=/tmp/cursor")

              t.assertNotIn("Stopping Nginx Web Server...", output)

        '';

}

nixos/tests/all-tests.nix

+4 −1
Original line number Diff line number Diff line
@@ -183,7 +183,10 @@ in 
  # keep-sorted start case=no numeric=no block=yes

  _3proxy = runTest ./3proxy.nix;

  aaaaxy = runTest ./aaaaxy.nix;

  acme = import ./acme/default.nix { inherit runTest; };

  acme = import ./acme/default.nix {

    inherit runTest;

    inherit (pkgs) lib;

  };

  acme-dns = runTest ./acme-dns.nix;

  activation = pkgs.callPackage ../modules/system/activation/test.nix { };

  activation-etc-overlay-immutable = runTest ./activation/etc-overlay-immutable.nix;