Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nixos/acme: Allow using lego's built-in web server #125256

Merged
merged 1 commit into from Dec 11, 2021

Conversation

deviant
Copy link
Member

@deviant deviant commented Jun 1, 2021

Motivation for this change

Currently, we hardcode the use of --http.webroot, even if no webroot is
configured. This has the effect of disabling the built-in server.

Things done
  • 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/)
  • Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

@deviant
Copy link
Member Author

deviant commented Jun 1, 2021

This isn't a very complex change, but mentioning @NixOS/acme regardless since I don't think that happens automatically.

@emilazy
Copy link
Member

emilazy commented Jun 4, 2021

see also #88531 (thanks(?) stale bot)

@deviant
Copy link
Member Author

deviant commented Jun 4, 2021

see also #88531 (thanks(?) stale bot)

Ugh, I swear I searched for this but couldn't find anything related

@deviant deviant closed this Jun 4, 2021
@emilazy
Copy link
Member

emilazy commented Jun 4, 2021

It’d be good to include the AmbientCapabilities change and I’m ambivalent about the difference in configuration, so I didn’t mean you should close this one, merely wanted to link the two :)

@m1cr0man
Copy link
Contributor

m1cr0man commented Jun 4, 2021

Infact Deviant, if you were to pull the proposed listenHTTP from that PR into this one we could see about getting it merged since you're active (at least right now). I'm not of any first-in, first-out ruling here.

@deviant
Copy link
Member Author

deviant commented Jun 4, 2021

Yep, I'll look at combining the two in a bit once I'm not busy

@deviant deviant reopened this Jun 4, 2021
Currently, we hardcode the use of --http.webroot, even if no webroot is
configured. This has the effect of disabling the built-in server.

Co-authored-by: Chris Forno <jekor@jekor.com>
@deviant
Copy link
Member Author

deviant commented Jun 5, 2021

There. The patch now incorporates the functionality of #88531, and thus supersedes it.

Heads up: I've not tested this version, although I believe it should work.

@deviant
Copy link
Member Author

deviant commented Sep 27, 2021

Does this require further changes, or is it okay as-is?

Copy link
Contributor

@m1cr0man m1cr0man left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to double check what --http.port accepts but this looks good :) Nice one on the AmbientCapabilities too.

@deviant
Copy link
Member Author

deviant commented Dec 11, 2021

@mweinelt @m1cr0man @emilazy bump

@m1cr0man
Copy link
Contributor

I'm still good for a merge on this and infact I'm gonna say it'll block #147784 . This PR has been around long enough that the work to rebase should fall on me in that other PR. I'll also add a test too.

@mweinelt mweinelt merged commit e675946 into NixOS:master Dec 11, 2021
@deviant deviant deleted the acme-standalone branch December 11, 2021 21:07
@deviant
Copy link
Member Author

deviant commented Dec 11, 2021

Thanks!

@m1cr0man m1cr0man mentioned this pull request Dec 18, 2021
13 tasks
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/problem-with-security-acme-and-listenhttp-option/16790/5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants