nginx module: clean up SSL/listen handling

2 files changed, 29 insertions, 33 deletions

diff --git a/nixos/modules/services/web-servers/nginx/default.nix b/nixos/modules/services/web-servers/nginx/default.nix
index 293fa77107e3..70c1d9942063 100644
--- a/nixos/modules/services/web-servers/nginx/default.nix
+++ b/nixos/modules/services/web-servers/nginx/default.nix
@@ -130,22 +130,23 @@ let
 
   vhosts = concatStringsSep "\n" (mapAttrsToList (vhostName: vhost:
     let
-        ssl = with vhost; addSSL || onlySSL || enableSSL;
-
-        defaultListen = with vhost;
-          if listen != [] then listen
-          else if onlySSL || enableSSL then
-               singleton                          { addr = "0.0.0.0"; port = 443; ssl = true;  }
-               ++ optional enableIPv6             { addr = "[::]";    port = 443; ssl = true;  }
-          else singleton                          { addr = "0.0.0.0"; port = 80;  ssl = false; }
-               ++ optional enableIPv6             { addr = "[::]";    port = 80;  ssl = false; }
-               ++ optional addSSL                 { addr = "0.0.0.0"; port = 443; ssl = true;  }
-               ++ optional (enableIPv6 && addSSL) { addr = "[::]";    port = 443; ssl = true;  };
+        onlySSL = vhost.onlySSL || vhost.enableSSL;
+        hasSSL = onlySSL || vhost.addSSL || vhost.forceSSL;
+
+        defaultListen =
+          if vhost.listen != [] then vhost.listen
+          else ((optionals hasSSL (
+            singleton                    { addr = "0.0.0.0"; port = 443; ssl = true; }
+            ++ optional enableIPv6 { addr = "[::]";    port = 443; ssl = true; }
+          )) ++ optionals (!onlySSL) (
+            singleton                    { addr = "0.0.0.0"; port = 80;  ssl = false; }
+            ++ optional enableIPv6 { addr = "[::]";    port = 80;  ssl = false; }
+          ));
 
         hostListen =
-          if !vhost.forceSSL
-            then defaultListen
-            else filter (x: x.ssl) defaultListen;
+          if vhost.forceSSL
+            then filter (x: x.ssl) defaultListen
+            else defaultListen;
 
         listenString = { addr, port, ssl, ... }:
           "listen ${addr}:${toString port} "
@@ -155,9 +156,6 @@ let
 
         redirectListen = filter (x: !x.ssl) defaultListen;
 
-        redirectListenString = { addr, ... }:
-          "listen ${addr}:80 ${optionalString vhost.default "default_server"};";
-
         acmeLocation = ''
           location /.well-known/acme-challenge {
             ${optionalString (vhost.acmeFallbackHost != null) "try_files $uri @acme-fallback;"}
@@ -175,7 +173,7 @@ let
       in ''
         ${optionalString vhost.forceSSL ''
           server {
-            ${concatMapStringsSep "\n" redirectListenString redirectListen}
+            ${concatMapStringsSep "\n" listenString redirectListen}
 
             server_name ${vhost.serverName} ${concatStringsSep " " vhost.serverAliases};
             ${optionalString vhost.enableACME acmeLocation}
@@ -191,9 +189,9 @@ let
           ${optionalString vhost.enableACME acmeLocation}
           ${optionalString (vhost.root != null) "root ${vhost.root};"}
           ${optionalString (vhost.globalRedirect != null) ''
-            return 301 http${optionalString ssl "s"}://${vhost.globalRedirect}$request_uri;
+            return 301 http${optionalString hasSSL "s"}://${vhost.globalRedirect}$request_uri;
           ''}
-          ${optionalString ssl ''
+          ${optionalString hasSSL ''
             ssl_certificate ${vhost.sslCertificate};
             ssl_certificate_key ${vhost.sslCertificateKey};
           ''}
@@ -478,18 +476,15 @@ in
       }
 
       {
-        assertion = all (conf: with conf; !(addSSL && (onlySSL || enableSSL))) (attrValues virtualHosts);
-        message = ''
-          Options services.nginx.service.virtualHosts.<name>.addSSL and
-          services.nginx.virtualHosts.<name>.onlySSL are mutually esclusive
-        '';
-      }
-
-      {
-        assertion = all (conf: with conf; forceSSL -> addSSL) (attrValues virtualHosts);
+        assertion = all (conf: with conf;
+          !(addSSL && (onlySSL || enableSSL)) &&
+          !(forceSSL && (onlySSL || enableSSL)) &&
+          !(addSSL && forceSSL)
+        ) (attrValues virtualHosts);
         message = ''
-          Option services.nginx.virtualHosts.<name>.forceSSL requires
-          services.nginx.virtualHosts.<name>.addSSL set to true.
+          Options services.nginx.service.virtualHosts.<name>.addSSL,
+          services.nginx.virtualHosts.<name>.onlySSL and services.nginx.virtualHosts.<name>.forceSSL
+          are mutually exclusive.
         '';
       }
     ];
diff --git a/nixos/modules/services/web-servers/nginx/vhost-options.nix b/nixos/modules/services/web-servers/nginx/vhost-options.nix
index 362f8ee90524..8a04e07eeeac 100644
--- a/nixos/modules/services/web-servers/nginx/vhost-options.nix
+++ b/nixos/modules/services/web-servers/nginx/vhost-options.nix
@@ -96,8 +96,9 @@ with lib;
       default = false;
       description = ''
         Whether to add a separate nginx server block that permanently redirects (301)
-        all plain HTTP traffic to HTTPS. This option needs <literal>addSSL</literal>
-        to be set to true.
+        all plain HTTP traffic to HTTPS. This will set defaults for
+        <literal>listen</literal> to listen on all interfaces on the respective default
+        ports (80, 443), where the non-SSL listens are used for the redirect vhosts.
       '';
     };