txacme
txacme copied to clipboard
Add .onstore scripts
This lets you run some arbitrary code after something's been stored, so you can rehash nginx or whatever.
This was the major blocker for me adopting txacme: some services need to be told to reload keys/certs from disk.
I should say this was mostly a proof of concept; I don't expect this to get merged as-is because it has no docs. But, how's it look?
Oops yeah I just remembered this is also the bit with the super crappy attempt at getting hypothesis and testtools to work together. That should definitely be split off into a PR for hypothesis.
This seems like a generally useful feature. Adding it to the endpoint as a kwarg is a bit tricky, since any kwargs we use for ourselves will mask kwargs in the endpoint we're wrapping... maybe @glyph has some thought about the Right Thing To Do here since I haven't thought about it too much myself yet.
We should add it to the docs, of course, and I see the tests are failing on Python 3 because of str/bytes issues (to nobody's surprise), but those should be easy things to fix.
A thought: Make the "on stored callback" (maybe this could be implemented with the same mechanism that Request.notifyFinish
uses, for multiple "observers" if necessary) a plain Python function, this would be a more generic interface with all the benefits that come with being able to invoke functions in the same process.
For the "run arbitrary scripts" idea you could always provide a Python function (it may even be in txacme itself) that implements this behaviour on top of the "on stored callback" function.
We already have a directory with certain enforced naming conventions. Perhaps just put a script in there, on-store
, which is executed as a hook, no need for an additional command line argument?
I was hoping to avoid checking argv[1]
in the script in general, but a single on-store
script does probably make the most sense, given the naming convention point.
No, I mean, put it in the certificates directory.
Right. Have a single {cert-dir}/on-store
script instead of {cert-dir}/{hostname}.onstore
.
Okay, so the hook script behaviour will be hardcoded for the endpoint parser, and "always on". I think @jonathanj is right that the underlying interface should just be DirectoryStore
taking a callable (let's say a 1-arg callable that takes the servername?), so If you need different hooks for some hosts, you can either just use different directories, or you can write some Python code.
Sounds like a good strategy to me.
Current coverage is 99.75%
- 5 files (not in diff) in
src/txacme/test
were modified. more- Misses
+1
- Partials
+1
- Hits
-2
- Misses
- 2 files (not in diff) in
src/txacme
were modified. more
@@ master #27 diff @@
=========================================
Files 20 20
Lines 1535 1579 +44
Methods 0 0
Messages 0 0
Branches 139 128 -11
=========================================
+ Hits 1535 1575 +40
- Misses 0 2 +2
- Partials 0 2 +2
Powered by Codecov. Last updated by 8c1a3cd...e73bd8e
I'm +1 on @jonathanj's suggestion that there should be a callback for certificate changes. For now I've implemented a wrapper for ICertificateStore
that triggers a reload of certificates in an external system when .store()
is called. Which I think should work fine, but it's not explicit anywhere that .store()
is only called when a certificate is issued/renewed, although this is probably a reasonable assumption to make if I expect you to have written this software well (this is maybe #35?).
Another problem with having .store()
as the point where things are notified of certificate changes is that it means it's tricky to add new domains/certificates to the store, as this is usually done by storing an empty certificate in the store and the empty certificate is likely to break whatever external thing using the certificates (#76).
+1 for callback on certificate updates for the default CertificateStore implementation.
I have implemented by own ICertificateStore
which will call/forward the server_name, pem_objects
each time something is stored in it.
For the file based CertificateStore I am expecting the callback to also include the full path to the updated file.