nixos/nginx: report actual vhosts name in assertions/warnings

[SuperSandro2000]

Before assertions where not really helpful as they didn't show what vhost had the problem which is very essential to fix it.

- Options services.nginx.service.virtualHosts.<name>.enableACME and
services.nginx.virtualHosts.<name>.useACMEHost are mutually exclusive.

and now they do 🎉

- Options services.nginx.service.virtualHosts."example.org".enableACME and
services.nginx.virtualHosts."example.org".useACMEHost are mutually exclusive.

Actual assertion with mocked host name while I had the old assertions still in place:

       error:
       Failed assertions:
       - Options services.nginx.service.virtualHosts.<name>.enableACME and
       services.nginx.virtualHosts.<name>.useACMEHost are mutually exclusive.

       - Options services.nginx.service.virtualHosts."example.org".enableACME and
       services.nginx.virtualHosts."example.org".useACMEHost are mutually exclusiv

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[Izorkin]

Checked it on a test virtual machine, it works.

[SuperSandro2000]

Fixing the docs build was quite a bit more involved than I like it to be, especially of this nothing telling error:

lazy-options.json>        … from call site
lazy-options.json>          at /nix/store/n09xs0fxjvk13r3p0almbz6rmy2kl2w7-lib/modules.nix:490:7:
lazy-options.json>           489|       # shorthand syntax
lazy-options.json>           490|       throwIfNot (isAttrs m) "module ${file} (${key}) does not look like a module."
lazy-options.json>              |       ^
lazy-options.json>           491|       { _file = toString m._file or file;
lazy-options.json>        … while calling 'throwIfNot'
lazy-options.json>          at /nix/store/n09xs0fxjvk13r3p0almbz6rmy2kl2w7-lib/trivial.nix:859:22:
lazy-options.json>           858|   */
lazy-options.json>           859|   throwIfNot = cond: msg: if cond then x: x else throw msg;
lazy-options.json>              |                      ^
lazy-options.json>           860|
lazy-options.json>        error: module <unknown-file> (:anon-2) does not look like a module.
lazy-options.json> Cacheable portion of option doc build failed.
lazy-options.json> Usually this means that an option attribute that ends up in documentation (eg `default` or `description`) depends on the restricted module arguments `config` or `pkgs`.
lazy-options.json> Rebuild your configuration with `--show-trace` to find the offending location. Remove the references to restricted arguments (eg by escaping their antiquotations or adding a `defaultText`) or disable the sandboxed build for the failing module by setting `meta.buildDocsInSandbox = false`.
lazy-options.json>
error: builder for '/nix/store/cpsq5j49wgkh8dnfa6d4l7s52x0ny7hd-lazy-options.json.drv' failed with exit code 1

[Ma27]

@roberth @infinisil I'm curious, are there any best practices on importing submodules from other files? This seems kinda odd, but right now I don' thave a better idea.

[roberth]

My first thought would be to use importApply when calling this, but that barely improves anything; just the error message location.
Nicer improvements would be:

• use deferredModule so that it only needs to be invoked in one place, the main nginx module
• factor out the dependencies on services.nginx aka cfg into a separate module
  • make that separate module an anonymous module in the main nginx module
  • this file won't need extra paramaters, so it can be a regular module file, imported with imports = [ ./vhost-options.nix ]; or whatever the path expression is.

[roberth]

To clarify, importing into submodules is ok and lets you do interesting things, and it may even look like this, but this isn't the right use case for it, because option values shouldn't be forwarded en masse to become definitions for multiple options.

[Ma27]

shouldn't be forwarded en masse to become definitions for multiple options.

Agreed, that's why I opened #313412 and I even have a half-finished thing in my stash.

That said, this is out-of-scope here, it contains a nice improvement and as long as we don't break existing code here, we're good I'd say.

[roberth]

Worth trying the following suggestions.

Note that deferredModule adds the option name to its _file location for error messages, and "${options.foo}" gives you a fully qualified option path, including attrsOf names such as the vhost name.

[roberth]

A minor improvement for error messages would be:

Suggested change

	(import ../web-servers/nginx/vhost-options.nix { inherit config lib; }));
	(lib.modules.importApply ../web-servers/nginx/vhost-options.nix { inherit config lib; }));

However, it seems that these modules are forwarding an arbitrary set of option values as config definitions, thereby losing dynamic behavior and potentially causing problems in services.nginx as priorities like mkForce are lost.
The right solution here would be to use a type that represents these definitions etc, without computing the config fixpoint or any definition merging before its use in services.nginx.

  type = lib.types.nullOr lib.types.deferredModule;

[SuperSandro2000]

It evaluates, no idea how to test this though. I don't have any of the modules in here set up.

Also no idea how to use deferredModule here. Just replacing submodule yields an error:

lazy-options.json>        error: attempt to call something which is not a function but a set
lazy-options.json>
lazy-options.json>        at /nix/store/54wv4qcvzlxg98v3n8asy27bfxviycca-nixos/modules/services/misc/radicle.nix:212:36:
lazy-options.json>
lazy-options.json>           211|           # Type of a single virtual host, or null.
lazy-options.json>           212|           type = lib.types.nullOr (lib.types.deferredModule
lazy-options.json>              |                                    ^
lazy-options.json>           213|             (lib.modules.importApply ../web-servers/nginx/vhost-options.nix { inherit config lib; }));
lazy-options.json> Cacheable portion of option doc build failed.
lazy-options.json> Usually this means that an option attribute that ends up in documentation (eg `default` or `description`) depends on the restricted module arguments `config` or `pkgs`.
lazy-options.json>
lazy-options.json> Rebuild your configuration with `--show-trace` to find the offending location. Remove the references to restricted arguments (eg by escaping their antiquotations or adding a `defaultText`) or disable the sandboxed build for the failing module by setting `meta.buildDocsInSandbox = false`.
lazy-options.json>
error: build of '/nix/store/kysl1q2mkcbq80gssnsxjbfw2m8qyyd8-lazy-options.json.drv' on 'ssh-ng:https://amy.dse.in.tum.de' failed: builder for '/nix/store/kysl1q2mkcbq80gssnsxjbfw2m8qyyd8-lazy-options.json.drv' failed with exit code 1;

and without sandbox:

 ➜ NIX_PATH=nixpkgs=$(pwd) nix-build --option restrict-eval true nixos/release.nix -A manual.x86_64-linux
error:
       … while calling the 'deepSeq' builtin
         at /home/sandro/src/nixpkgs3/lib/customisation.nix:415:7:
          414|     in if drv == null then null else
          415|       deepSeq drv' drv';
             |       ^
          416|

       … while evaluating the attribute 'outPath'
         at /home/sandro/src/nixpkgs3/lib/customisation.nix:404:13:
          403|           value = commonAttrs // {
          404|             outPath = output.outPath;
             |             ^
          405|             drvPath = output.drvPath;

       (stack trace truncated; use '--show-trace' to show the full, detailed trace)

       error: attempt to call something which is not a function but a set: { _type = "option-type"; check = «thunk»; deprecationMessage = null; description = «thunk»; descriptionClass = "noun"; emptyValue = «thunk»; functor = «thunk»; getSubModules = «thunk»; getSubOptions = «thunk»; merge = «thunk»; «4 attributes elided» }
       at /home/sandro/src/nixpkgs3/nixos/modules/services/misc/radicle.nix:212:36:
          211|           # Type of a single virtual host, or null.
          212|           type = lib.types.nullOr (lib.types.deferredModule
             |                                    ^
          213|             (lib.modules.importApply ../web-servers/nginx/vhost-options.nix { inherit config lib; }));

[roberth]

My first thought would be to use importApply when calling this, but that barely improves anything; just the error message location.
Nicer improvements would be:

• use deferredModule so that it only needs to be invoked in one place, the main nginx module
• factor out the dependencies on services.nginx aka cfg into a separate module
  • make that separate module an anonymous module in the main nginx module
  • this file won't need extra paramaters, so it can be a regular module file, imported with imports = [ ./vhost-options.nix ]; or whatever the path expression is.

[SuperSandro2000]

I did another rebase and applied the last 3 comments.

[Ma27]

This gives me error: cannot coerce a set to a string. I think you meant attrNames here?

[Ma27]

I always get a headache when staring at this

(no need to change that in this PR, just complaining :p)