nixos/nginx: use the directory given in the ACME configuration #29556
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrew-d
changed the title
|
Note that this is similar to the |
andrew-d
force-pushed
the
adunham/nginx-acme
branch
from
d9ebaf9
to
ba61c7d
Compare
|
cc @globin (last person to commit to this file) - thoughts? |
aneeshusa
reviewed
Sep 24, 2017
| @@ -13,8 +13,8 @@ let | |||
| vhostConfig // { | |||
| inherit serverName; | |||
| } // (optionalAttrs vhostConfig.enableACME { | |||
| sslCertificate = "/var/lib/acme/${serverName}/fullchain.pem"; | |||
| sslCertificateKey = "/var/lib/acme/${serverName}/key.pem"; | |||
| sslCertificate = "${cfg.acmeDir}/${serverName}/fullchain.pem"; | |||
There was a problem hiding this comment.
I haven't tested this, but I think you should be able to use config.security.acme.directory directly here instead of making an nginx-specific wrapper option.
There was a problem hiding this comment.
Yes, it appears that this does work - just pushed a commit to use that instead. Thanks for the suggestion! 😀
andrew-d
force-pushed
the
adunham/nginx-acme
branch
from
ba61c7d
to
8ae30a5
Compare
andrew-d
changed the title
This commit changes the default `/var/lib/acme` directory to instead be the directory that was specified in the acme config: `config.security.acme.directory`. This doesn't change anything by default, since the default value of that option is the same. This is useful in cases where we'd like to store the ACME certificates and keys in a more centralized location; for example, on a mounted EBS volume in Amazon, or to centralize secrets in a centralized directory for monitoring purposes.
andrew-d
force-pushed
the
adunham/nginx-acme
branch
from
8ae30a5
to
f01380b
Compare
|
@aneeshusa - Thanks for the review! Just rebased & force-pushed, and the build is now passing. |
|
This LGTM, but I don't have merge rights. |
orivej
approved these changes
Oct 8, 2017
8 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation for this change
This is useful in cases where we'd like to store the ACME certificates
and keys in a more centralized location; for example, on a mounted EBS
volume in Amazon, or to centralize secrets in a centralized directory
for monitoring purposes.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)