nixos/bluemap: fix defaults issue with services.bluemap.host

[philiptaron]

Description of changes

The default for the services.bluemap.host value depends on config.networking.domain, which is typed as types.nullOr types.str in nixos/modules/tasks/network-interfaces.nix

As a result, the default for services.bluemap.host either has to be types.nullOr types.str, or we need to drop the default.

Based on PR feedback, this commit drops the default and requires configuration through the services.bluemap.host option.

While this is a breaking change, since the module is a month old, there should be very few users so far.

I discovered this while testing out #313497.

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.

[D4ndellion]

Woops! Thanks for noticing!

A problem with this approach is that you now have to read the module source to find out that it's hosted on bluemap.${domain}. You could fix this by describing it in defaultText maybe?

A bigger issue is probably that you can't now refer to config.services.bluemap.host to get the domain since this would be null - even though the nginx host is listening on bluemap.${domain}

Maybe we could just remove the default entirely?

[philiptaron]

Maybe we could just remove the default entirely?

I started with that, but went the way I currently did in order to not have a breaking change. If you're OK with breaking any users that were relying on bluemap.${config.networking.domain}, I'll gladly update the PR.

[pbsds]

perhaps retarget this to 24.05 and drop the default on unstable? Up to the maintainers to decide

[D4ndellion]

I think there's no good solution here. If someone has this for example:

services.nginx.virtualHosts."${config.services.bluemap.host}" = {
  enableACME = true;
  forceSSL = true;
};

their nginx config will break at runtime completely silently in a probably hard to debug way. This is very relevant since I expect setting these kinds of settings would be very common.

I dont think either solution has a large impact so none of this is really important. The module is less than a month old, and has probably very very few users. But I'm erring towards just cleanly breaking at build time and ripping the band-aid off.

I could probably easily be swayed either way

[philiptaron]

Since it's a month old, that makes my concern about existing users pretty minor. I'll simplify and remove the bluemap.${domain} default.

[SuperSandro2000]

LGTM

[D4ndellion]

These are some completely non-blocking nitpicks!

[philiptaron]

Hold off on merging: I haven't actually built a VM with these settings set.

[philiptaron]

Now it's a super-simple change. Thanks for the feedback; I learned a lot about how defaults (or lack thereof) work in the module system. It's nice that a variable which is used but not defined blows up with a good error.