nixos/acme: execute a single lego command

[datafoo]

Motivation for this change

See #86184.

This implementation follows the workaround indicated go-acme/lego#693 (comment).

Things done

• Tested using NixOps
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

[flokli]

cc @maralorn

[maralorn]

Overall this change looks great. Thank you very much!

But I have not tested it.

[datafoo]

Overall this change looks great. Thank you very much!

I must say though that I am not a fan of my own work here.

We are introducing new dependencies on openssl and perl to fix a little something that should logically be done in lego (#216, #290, #693). 4 years and 3 issues later, that does not seem to be a priority for lego to fix this.

@m1cr0man: you introduced the change to Lego. Was Lego the only choice for what we need in NixOS?

[m1cr0man]

Hi datafoo! This is a nice solution, using openssl makes sense and probably doesn't hurt to make it a dependency. It's inevitable that it would be needed on a system using the acme module in the first place.

As for whether lego was the only choice, and why I chose it. Initially, my PR was based on another PR that used lego and that had received sufficient reviews and discussion to justify the selection of lego, and I certainly wasn't familiar enough with any other client to convince people otherwise. I simply continued the implementation and got it merged, having had it in production for a couple months beforehand.

However, I have been working on an update to the acme module as a whole.. I've been delayed just by being busy at work, but it does address this issue along with many, many others. I'll open a WIP PR now, which I should've done ages ago, and you can decide if you want to merge this PR regardless. If I don't get my work finished, this should definitely be added.

[flokli]

I think this PR is an improvement as well - let's merge it 👍

[mweinelt]

The acme update mentioned is here #91121.

[datafoo]

Is it good to merge then?

[m1cr0man]

Yeah go for it! :)

[datafoo]

Yeah go for it! :)

I believe I do not have such permission on this repo.

[m1cr0man]

Yeah go for it! :)

I believe I do not have such permission on this repo.

Ah, yes 😅 someone should be able to do it today for you.

[arianvp]

@GrahamcOfBorg test acme

[aanderse]

Merging entirely based on discussion.