-
-
Notifications
You must be signed in to change notification settings - Fork 15.4k
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
Add test for ACME #27683
Add test for ACME #27683
Conversation
These modules implement a way to test ACME based on a test instance of Letsencrypt's Boulder service. The service implementation is in letsencrypt.nix and the second module (resolver.nix) is a support-module for the former, but can also be used for tests not involving ACME. The second module provides a DNS server which hosts a root zone containing all the zones and /etc/hosts entries (except loopback) in the entire test network, so this can be very useful for other modules that need DNS resolution. Originally, I wrote these modules for the Headcounter deployment, but I've refactored them a bit to be generally useful to NixOS users. The original implementation can be found here: https://github.com/headcounter/deployment/tree/89e7feafb/modules/testing Quoting parts from the commit message of the initial implementation of the Letsencrypt module in headcounter/deployment@95dfb31110397567534f2: This module is going to be used for tests where we need to impersonate an ACME service such as the one from Letsencrypt within VM tests, which is the reason why this module is a bit ugly (I only care if it's working not if it's beautiful). While the module isn't used anywhere, it will serve as a pluggable module for testing whether ACME works properly to fetch certificates and also as a replacement for our snakeoil certificate generator. Also quoting parts of the commit where I have refactored the same module in headcounter/deployment@85fa481: Now we have a fully pluggable module which automatically discovers in which network it's used via the nodes attribute. The test environment of Boulder used "dns-test-srv", which is a fake DNS server that's resolving almost everything to 127.0.0.1. On our setup this is not useful, so instead we're now running a local BIND name server which has a fake root zone and uses the mentioned node attribute to automatically discover other zones in the network of machines and generate delegations from the root zone to the respective zones with the primaryIPAddress of the node. ... We want to use real letsencrypt.org FQDNs here, so we can't get away with the snakeoil test certificates from the upstream project but now roll our own. This not only has the benefit that we can easily pass the snakeoil certificate to other nodes, but we can (and do) also use it for an nginx proxy that's now serving HTTPS for the Boulder web front end. The Headcounter deployment tests are simulating a production scenario with real IPs and nameservers so it won't need to rely on networking.extraHost. However in this implementation we don't necessarily want to do that, so I've added auto-discovery of networking.extraHosts in the resolver module. Another change here is that the letsencrypt module now falls back to using a local resolver, the Headcounter implementation on the other hand always required to add an extra test node which serves as a resolver. I could have squashed both modules into the final ACME test, but that would make it not very reusable, so that's the main reason why I put these modules in tests/common. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
The test here is pretty basic and only tests nginx, but it should get us started to write tests for different webservers and different ACME implementations. Signed-off-by: aszlig <aszlig@redmoonstudios.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome work!
Can't we factor Boulder into a proper package and a NixOS service? Maybe not very general purpose for now but still -- putting everything into one test seems painful to me.
@abbradar: Probably a good idea, although I think we should factor the components out one by one. My original intent was also to make it a generic service, but it has a shitton of configuration options that we need to take care of. If we split it up into packages now, we'd also need tests for the boulder service itself, because then package bumps can break the service as well. What this does at the moment is taking the test configs and patching them, so that they closely resemble what they run in production (although we'd still need to switch to unbound, because that's what they're running). |
Hm, indeed we may not want to advertise boulder service (and leave it in tests instead) but shouldn't we at least factor out the packages? This way seems more proper and doesn't add much in terms of maintenance (think |
This is a rebased version of the pull request with small fixes due to changes in recent master. Original description from the pull request: Currently this is only a very basic test which gets certificates via the enableACME option of the nginx module. However the main reason why I'm not directly merging and putting this up for review is that the complexity here lies in the support-modules needed for the test. The support modules are for running a Boulder instance along with a DNS resolver (as a separate module). For details about the implementation, see the commit messages and the comments at the start of the respective support modules. I'm merging this first of all because other than @abbradar, none of the other requested reviewers did comment on the changes and second because the change here is adding a test, so even if the implementation would be so disgusting and crappy it's better than having no test at all. The comment of @abbradar was: Can't we factor Boulder into a proper package and a NixOS service? Maybe not very general purpose for now but still -- putting everything into one test seems painful to me. My objection to this is that the components are heavily patched and some of them don't even have a release, so I'm not sure whether infesting pkgs/ with them is really a good idea. Nevertheless, we can still do that later. Cc: @fpletz, @domenkozar, @bjornfor
Merged in 62711f4. |
Currently this is only a very basic test which gets certificates via the
enableACME
option of thenginx
module.However the main reason why I'm not directly merging and putting this up for review is that the complexity here lies in the support-modules needed for the test. The support modules are for running a Boulder instance along with a DNS resolver (as a separate module).
For details about the implementation, see the commit messages and the comments at the start of the respective support modules.
Cc: @qknight