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

fixes #50 - purge cached cert/key when a replacement is received #51

Merged
merged 1 commit into from
Mar 25, 2024

Conversation

zsteinkamp
Copy link
Contributor

@zsteinkamp zsteinkamp commented Mar 22, 2024

Proposed changes

Fixes Issue #50 - Purge cached cert/key from shared dictionary zone (if applicable) when we receive a replacement. This allows for on-the-fly certificate reconfiguration (e.g. adding a domain) by using nginx -s reload.

Thank you @NetForce1 for raising the issue and your clarity in describing the symptoms and repro case.

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING document
  • If applicable, I have added tests that prove my fix is effective or that my feature works
  • If applicable, I have checked that any relevant tests pass after adding my changes
  • I have updated any relevant documentation (README.md and CHANGELOG.md)

Confirmation of fix

Before: 2 hosts in certificate
Screenshot 2024-03-22 at 1 34 53 PM

Reload nginx (via docker compose exec nginx bash)
Screenshot 2024-03-22 at 1 37 35 PM

nginx-1   | 2024/03/22 20:34:35 [info] 24#24: js: njs-acme: [auto] Renewing certificate because the hostnames in the certificate (proxy.nginx.com, proxy2.nginx.com) do not match the configured njs_acme_server_names (proxy.nginx.com)

After: 1 host in certificate
Screenshot 2024-03-22 at 1 35 37 PM

@zsteinkamp zsteinkamp assigned zsteinkamp and unassigned zsteinkamp Mar 22, 2024
@zsteinkamp zsteinkamp requested review from 4141done and ryepup March 22, 2024 20:46
Copy link
Collaborator

@ryepup ryepup left a comment

Choose a reason for hiding this comment

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

LGTM. Do we want to bump version numbers in package.json and CHANGELOG?

zsteinkamp added a commit that referenced this pull request Mar 25, 2024
@zsteinkamp
Copy link
Contributor Author

I'll bump the versions after the next PR. Thanks @ryepup !

@zsteinkamp zsteinkamp merged commit c0342f8 into main Mar 25, 2024
4 checks passed
@zsteinkamp zsteinkamp deleted the zsteinkamp/50-purge-cache branch March 25, 2024 18:30
@NetForce1
Copy link

NetForce1 commented Mar 25, 2024

Thanks for the swift actions @zsteinkamp

This allows for on-the-fly certificate reconfiguration (e.g. adding a domain) by using nginx -s reload.

I justed wanted to add (for future searchers :-) ) that this was not only a problem for adding a domain, but also for the renewal.

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

Successfully merging this pull request may close these issues.

3 participants