nixos/navidrome: error: attribute 'Port' missing

[gepbird]

Describe the bug

Evaluation failure when setting services.navidrome.settings.* and services.navidrome.openfirewall = true.
Moreover setting any services.navidrome.settings.* will "remove" the default values for services.navidrome.settings.Address and services.navidrome.settings.Port

Steps To Reproduce

1. Build your system with these options:

  services.navidrome = {
    enable = true;
    openFirewall = true;
    settings = {
      BaseUrl = "/navidrome";
    };
  };

2. Observe the failure:

       error: attribute 'Port' missing

       at /nix/store/0aavdx9m5ms1cj5pb1dx0brbrbigy8ij-source/nixos/modules/services/audio/navidrome.nix:135:69:

          134|
          135|       networking.firewall.allowedTCPPorts = mkIf cfg.openFirewall [ cfg.settings.Port ];
             |                                                                     ^
          136|     };

Expected behavior

System builds, firewall is opened for the default 4533 port.

Additional context

This bug happens because the default value is defined for settings as a whole rather than for each individual setting.

Notify maintainers

@nu-nu-ko

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

[user@system:~]$ nix-shell -p nix-info --run "nix-info -m"
 - system: `"aarch64-linux"`
 - host os: `Linux 6.1.63, NixOS, 24.05 (Uakari), 24.05.20240517.4a6b83b`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.2`
 - channels(root): `"nixos-23.11.1115.5de0b32be6e8"`
 - nixpkgs: `/nix/store/0aavdx9m5ms1cj5pb1dx0brbrbigy8ij-source`

---

Add a 👍 reaction to issues you find important.

[nu-nu-ko]

Evaluation failure when setting services.navidrome.settings.* and services.navidrome.openfirewall = true.

the Issue here is the settings.port option being null, you can use these options together if settings.port is supplied. if you encountered a different eval failure to this please attach it.

Moreover setting any services.navidrome.settings.* will "remove" the default values for services.navidrome.settings.Address and services.navidrome.settings.Port

Yes this is because those options values are supplied as apart of one default of .settings.* as a whole, I am not aware of a nice way to give them a default independent of the rest of the settings being set or not.

would giving this port value a catch for being null be an alright solution?
e.g.. if cfg.settings.Port is not set, then use the upstream default ( of 4533 I believe )

I don't really want to separate out these values from the .settings. option so this wont address the removal of default values when setting them yourself to something different, I think this is expected enough behavior.

I think fixing this eval failure condition with simply supplying the default port where .settings.port isn't supplied is reasonable enough, as anyone who wasnt going to set that in the first place is unlikely to care if its the same value in function either way.

In fact i may just remove the settings. default entirely and simply handle the case of no supplied .settings. value by assuming the default port (of 4533 ) in the openFirewall rule.

I'll PR these changes once I get the time