-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: use the js_periodic directive to check/issue/renew certs #38
Conversation
Please ensure integration tests are updated and tests both triggers "periodic" and regular HTTP request. it would require updating nginx config in integration tests |
3. When started initially, nginx would not have certificates at all (`$njs_acme_dir`, e.g. `/etc/nginx/njs-acme/`), so we can issue a new one by sending an HTTP request to a location with the `js_content` handler: | ||
|
||
curl -vik --resolve proxy.nginx.com:8000:127.0.0.1 http://proxy.nginx.com:8000/acme/auto | ||
3. When started initially, nginx will not have certificates at all. If you use the [example config](examples/nginx.conf), you will need to wait one minute for the `js_periodic` directive to invoke `acme.clientAutoMode` to create the certificate. |
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.
Something I don't understand: Can we have the ability to kickstart via a request as well as having the periodic process run? It seems like perhaps the js_periodic
invocation could be in a normal location block that kicks it off and we could have both? Very possible that there's a situation I'm missing here but this change seems to take away some flexibility.
Should we add documentation here to show how to do it the old way?
js_content acme.clientAutoModeHTTP;
It could be that some users could prefer to invoke the check using an outside source as should in the deleted documentation.
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.
Oh that's an interesting idea!
So we could do:
location = /acme/auto {
js_periodic acme.clientAutoMode 1m;
js_content acme.clientAutoModeHTTP;
}
Then people who were impatient could just hit it with curl. Ya?
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.
Oh interesting I had not considered combining the two. Kind of strange the js_periodic
needs to be in a location
block to work since it's not tied to a location necessarily but 🤷 .
Yes I think this accomplishes the goal I was thinking of. Gives the user's automation the power to kickstart things on their desired cadence while still maintaining the automatic operation.
If we get requests for entirely manual management we could introduce a config option but until then this seems like a reasonable compromise 👍
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.
Sweet. I've documented it as an option in the README.md
. LMK what you think...
… on regular intervals
Co-authored-by: Ryan Davis <[email protected]>
…v var. Will update README.md if this looks like a good path.
…, remove advanced endpoints from example conf
Co-authored-by: Daniel Edgar <[email protected]>
…ModeWeb to clientAutoModeHTTP; fix unused var warning
… examples; move some util functions to utils.ts
Uses the new directive
js_periodic
in njs >= 0.8.2 to invokeclientAutoMode(r)
on regular intervals. This eliminates the need to set up a cron or use docker healthchecks.Proposed changes
js_periodic
to invokeclientAutoMode()
periodically.Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTING
documentREADME.md
andCHANGELOG.md
)