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

feat: Change renew rc when cert is still valid #2366

Closed
wants to merge 1 commit into from

Conversation

m1cr0man
Copy link
Contributor

@m1cr0man m1cr0man commented Dec 1, 2024

Sometimes the lego Client may fail to contact the CA due to some temporary networking issue on the host. The certificate may otherwise still be valid, and a user may want to ignore this issue.

This change modifies the renew service so that if the lego client is nil, but the certificate is not expired, the exit code of the program will be 2 instead of 1. Crucially it does not change if lego exits, only the exit code itself.

In order to implement this, I had to change the setup function to remove a usage of log.Fatalf which in turn required me to introduce explicit os.Exit calls in some commands. Although this was the lowest patch delta solution, I also considered refactoring the setup entirely to remove log.Fatalf, and return setup errors instead.

As with my previous PR, I performed a full integration test using the NixOS ACME integration test suite. You can run that test locally with this command:

nix run github:m1cr0man/nixpkgs/lego-offline-renewal-test#nixosTests.acme

Here's some example log output with this change implemented, sampled from a system with the network disabled:

$ lego --accept-tos --path . -d http.example.test --email [email protected] --key-type ec256 --http --http.port :80 --server https://acme.test/dir renew --no-random-sleep --force-cert-domains --days 30
2024/11/30 23:33:00 [WARN] Could not create client: get directory at 'https://acme.test/dir': Get "https://acme.test/dir": dial tcp: lookup acme.test: Temporary failure in name resolution
2024/11/30 23:33:00 [http.example.test] The certificate expires in 1825 days, the number of days defined to perform the renewal is 30: no renewal.
2024/11/30 23:33:00 [WARN] [http.example.test] Failed to check ARI as there is no client. Check previous error messages for possible initialization issues.
$ echo $?
2

Sometimes the lego Client may fail to contact the CA due to some
temporary networking issue on the host. The certificate may otherwise
still be valid, and a user may want to ignore this issue.

This change modifies the renew service so that if the lego client
is nil, but the certificate is not expired, the exit code of the
program will be 2 instead of 1.

In order to implement this, I had to change the setup function to remove
a usage of log.Fatalf which in turn required me to introduce explicit
os.Exit calls in some commands. Although this was the lowest patch delta
solution, I also considered refactoring the setup entirely to remove
log.Fatalf, and return setup errors instead.
@ldez ldez self-requested a review December 1, 2024 02:14
@m1cr0man
Copy link
Contributor Author

m1cr0man commented Dec 1, 2024

To note, this at least allows users to resolve #1280 (which stems from NixOS/nixpkgs#85794 (comment)) and #1546 by checking the return code in their specific situations.

@ldez
Copy link
Member

ldez commented Dec 1, 2024

Hello,

I understand your problem but your proposal is not something I want to follow.
Can you open an issue?

@ldez ldez closed this Dec 1, 2024
@m1cr0man
Copy link
Contributor Author

m1cr0man commented Dec 1, 2024

Sure ok

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

Successfully merging this pull request may close these issues.

2 participants