nixos/traefik: refactor

[SigmaSquadron]

@TheRealGramdalf's Traefik refactor, split off from #437073

Closes #471071

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

[SigmaSquadron]

@ofborg test simple

[TheRealGramdalf]

There's a couple things that need fixing, but they are mostly minor. Feel free to make the changes, I doubt I'll have a ton of time in the near future.

[SigmaSquadron]

Reorganised the module, and dropped dynamic.settings and extraFiles, so we just have dynamic.files instead.

[SigmaSquadron]

Everything should be ready now.

[TheRealGramdalf]

LGTM!

Thanks again for all the work to bring this over the finish line.

[SigmaSquadron]

Minor update to make sure an assertion actually fires and isn't smothered by another assertion.

[SigmaSquadron]

Fixed the logic of the docker group warning

[SigmaSquadron]

Thanks for the review, Sandro. Changes addressed.

[adamcstephens]

Are we good to go on this? Any final concerns @SuperSandro2000 ?

[SigmaSquadron]

Wrong lever!

[TheRealGramdalf]

Wrong lever!

"A llama? He's supposed to be dead!"

My only concern would be making a post to the unstable updates discourse thread. This shouldn't be a breaking change, but it's a fairly large overhaul with the potential for wacky edge cases.

[SigmaSquadron]

As soon as it's merged I can add a reply there.

[adamcstephens]

Let's do this.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/breaking-changes-announcement-for-unstable/17574/121

[MatthewCroughan]

This PR should not have been merged whilst the traefik.tests continued to fail, and was not updated in line with the module change.

nix-build -A traefik.tests
error:
       … while evaluating the attribute 'drvPath'
         at /home/matthew/git/nixpkgs/lib/customisation.nix:446:7:
          445|     // {
          446|       drvPath =
             |       ^
          447|         assert condition;

       … while calling the 'derivationStrict' builtin
         at «nix-internal»/derivation-internal.nix:37:12:
           36|
           37|   strict = derivationStrict drvAttrs;
             |            ^
           38|

       … while evaluating the option `nodes.traefik.system.build.toplevel':

       … while evaluating definitions from `/home/matthew/git/nixpkgs/nixos/modules/system/activation/top-level.nix':

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

       error:
       Failed assertions:
       - 'services.traefik.dynamic.files' requires the dynamic file provider to be set
       to a directory. Please set a path for 'services.traefik.dynamic.dir'.

[SigmaSquadron]

Followup in #490814.

[TheRealGramdalf]

I looked into this a little bit and I don't recognize this: https://github.com/NixOS/nixpkgs/pull/482512/changes#diff-023d14a18ce025c19e53d56dbf4a5f5ccb5d8e82676905b55cf31ffe385b5ac4R434-R444
I don't fully know what happened yet, but I do not think that code means what you think it means. I was quite intentional with how I implemented the changes, such that migration would be simple and that the new system would be powerful but not obtuse. I think I was too tolerant of changes, with the end result being a compromise between my initial idea and what others wanted.

Perhaps this is selfish, but I would propose we revert to my initial implementation of dynamic settings - I felt like it was a good way to add some quality of life without adding a black box that tries to do everything for you.

[SigmaSquadron]

If you believe that this hasn't been reviewed enough, I think it is best to revert this for now and try again later. The blast radius is thankfully limited to unstable.

[adamcstephens]

Sorry everyone. While I did put this through some of my own tests with my config and things looked ok, I clearly didn't validate enough. And also neglected the tests 😞

[jackrosenberg]

Followup in #490985