nixos/nginx: make sslCertificate and sslCertificateKey nullable

[lukateras]

This doesn't change any behavior, but extraordinarily simplifies virtualHost override via options.services.nginx.virtualHosts.apply.

[xeji]

One drawback is a confusing error message when the user enables SSL but doesn't set the certificate paths. Most users wouldn't understand what's wrong here:

error: cannot coerce null to a string, at /home/uli/nixpkgs/nixos/modules/services/web-servers/nginx/default.nix:217:35

Before this PR, the message was much clearer:

error: The option `services.nginx.virtualHosts.0.my.test.sslCertificate' is used but not defined.

[xeji]

Maybe the error message can be improved by adding an assertion.

[lukateras]

That comes at a price of not being able to refer to these options in apply function. It would show the same error message, but even if I do builtins.tryEval (lib.hasAttr "sslCertificate" vhost) it shows the error anyway.

[lukateras]

To be clear, here's what I'm doing:

{ config, lib, ... }:

with lib;

let
  inherit (config.networking) hostName;

  overrideVirtualHost = name: vhost: vhost // ({
    forceSSL = true;
  }) // (optionalAttrs (vhost.sslCertificate == null) {
    enableACME = true;
  }) // (optionalAttrs (vhost.serverName == null) {
    serverName = "${name}.${hostName}";
  });
in

{
  options.services.nginx.virtualHosts = mkOption {
    apply = vhosts: mapAttrs overrideVirtualHost vhosts;
  };
}

[dotlambda]

Can't you use !(vhost ? sslCertificate) instead of vhost.sslCertificate == null?

[lukateras]

@dotlambda I don't think that would work:

nix-repl> (a ? "hi")
error: undefined variable 'a' at (string):1:2

[dotlambda]

Why not?

nix-repl> vhost = { }

nix-repl> !(vhost ? sslCertificate)
true

nix-repl> vhost = { sslCertificate = ""; }

nix-repl> !(vhost ? sslCertificate) 
false

[lukateras]

Huh, that worked. Thank you a lot!

[lukateras]

Hm, actually it did not:

error: The option `services.nginx.virtualHosts.aaaaaDefault.sslCertificate' is used but not defined.
(use '--show-trace' to show detailed location information)

I just changed (optionalAttrs (vhost.sslCertificate == null)) above to (optionalAttrs !(vhost ? sslCertificate)). And tryEval also doesn't work, this seems to evaluate weirdly.

[luzpaz]

Any progress on this ?

[mmahut]

Are there any updates on this pull request, please?

[mmahut]

Closing due to inactivity.