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

Failed Renews Do Not Provide Environment Variable to Scripts #9934

Open
Riolku opened this issue Apr 20, 2024 · 1 comment
Open

Failed Renews Do Not Provide Environment Variable to Scripts #9934

Riolku opened this issue Apr 20, 2024 · 1 comment

Comments

@Riolku
Copy link

Riolku commented Apr 20, 2024

My operating system is (include version):

Alpine 3.19.1

I installed Certbot with (snap, OS package manager, pip, certbot-auto, etc):

apk (alpine package manager)

certbot --version reports certbot 2.7.4

I ran this command and it produced this output:

certbot renew --dry-run

I intentionally made my renewal command fail in order to test out alerts for failed renewals. My post hook script looks like this:

#!/bin/sh
echo FAILED_DOMAINS = $FAILED_DOMAINS
echo RENEWED_DOMAINS = $RENEWED_DOMAINS

And I received:

Hook 'post-hook' ran with output:
 FAILED_DOMAINS =
 RENEWED_DOMAINS =

Certbot's behavior differed from what I expected because:

As per the documentation, FAILED_DOMAINS should contain a list of the domains that failed.

Here is a Certbot log showing the issue (if available):

2024-04-20 13:31:02,712:INFO:certbot.compat.misc:Running post-hook command: /etc/letsencrypt/renewal-hooks/post/10-notification.sh
2024-04-20 13:31:02,724:DEBUG:certbot._internal.display.obj:Notifying user: Hook 'post-hook' ran with output:
 FAILED_DOMAINS =
 RENEWED_DOMAINS =
2024-04-20 13:31:04,712:DEBUG:certbot._internal.log:Exiting abnormally:
Traceback (most recent call last):
  File "/usr/bin/certbot", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/certbot/main.py", line 19, in main
    return internal_main.main(cli_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^   
  File "/usr/lib/python3.11/site-packages/certbot/_internal/main.py", line 1873, in main
    return config.func(config, plugins)  
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/certbot/_internal/main.py", line 1642, in renew
    renewed_domains, failed_domains = renewal.handle_renewal_request(config)
                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  
  File "/usr/lib/python3.11/site-packages/certbot/_internal/renewal.py", line 568, in handle_renewal_request
    raise errors.Error(
certbot.errors.Error: 1 renew failure(s), 0 parse failure(s)
2024-04-20 13:31:04,713:ERROR:certbot._internal.log:1 renew failure(s), 0 parse failure(s)

The Bug

After browsing the source code, I believe I have found the issue.

In renew, if an error occurs, we run post hooks with renewed_domains = [] and failed_domains = [].

In handle_renewal_request, we throw an error if any renewal fails, which means we don't return the domains that failed.

Happy to file a PR, just not sure what the best course of action. One obvious solution is to not throw an error on renew failure, but I believe that will mess with the exit codes.

Another is to pass renewed_domains and failed_domains into handle_renewal_request so they can be modified. Any error can then be thrown, and handle_renewal_request can be changed to return void.

Finally, the error that is thrown could contain failed_domains and renewed_domains.

I think option 2 is probably cleanest but I'm happy to file a fix for any of them.

@fa1rid
Copy link

fa1rid commented May 8, 2024

I'm facing the same issue. Any updates on this?
Why this functionality is broken in first place?

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

No branches or pull requests

2 participants