Several of these have been fixed, so let's update the requirement if
necessary and remove the warning catching.
`python-dateutil 2.9.0` was released Feb 29, 2024, so it's not widely
packaged in non-EOL major distros yet.
`pytest-cov 4.1.0` was released May 24, 2023.
Our pinned versions were already higher than these requirements.
Alternatively, we could just remove the warnings and not update the
minimum requirement, but I think it's nicer to note it in requirements
for anyone running our tests, like packagers.
We already require `poetry-plugin-export>=1.9.0`. `1.7.0` updated its
`requests-toolbelt` requirement to `>=1.0.0`, which is greater than the
minimum version needed to remove the warning.
It's a [drop-in
replacement](https://docs.astral.sh/uv/pip/compatibility/) that speeds
things up. I don't see any reason why not.
`--use-pep517` is [set by default](
https://docs.astral.sh/uv/pip/compatibility/#pep-517-build-isolation),
so we don't need it.
`--disable-pip-version-check` also does nothing on uv.
`uv` [uses
separate](https://docs.astral.sh/uv/pip/compatibility/#build-constraints)
`UV_BUILD_CONSTRAINT` and `UV_CONSTRAINT`. I just added it to both to do
the simplest thing here. We could split them.
We probably don't actually need to pipstrap pip anymore, I could take
that out.
What's happening with `parsedatetime` and `python-digitalocean` is that
they were always secretly wrong. Since `pip` compiles bytecode by
default, it was suppressing the errors. If you add the
`--compile-bytecode` flag to `uv`, it passes, but I don't think we
should do that. You can see the failure happen on main by passing
`--no-compile` to the pip args and running `certbot -r -e oldest`.
Now what I don't understand is that some places seem to say the `'\/'`
error from `parsedatetime` only started in python 3.12, whereas others
see it on earlier python. Perhaps pytest is vendorizing python or
something. Not too worried about that, needed to get updated anyway, and
it's an accurate oldest version based on our oldest OSes.
`python-digitalocean` is techincally newer than debian 11, but we've
made that decision before so it seems fine to me.
This uses the new `storage.atomic_rewrite` function to write only the
ARI-related value to the config file, leaving other values the same.
Updates `storage.atomic_rewrite` to handle the case where the
`"renewalparams"` section might be empty (which occurs in the new
unittests), and adds some comments.
Note: `atomic_rewrite` is mocked out as a no-op in
`test_resilient_ari_directory_fetches` and `test_renew_before_expiry`
because those tests have a simplistic config object on their
mock_renewable_cert that doesn't include a filename. If we try to write
the config in those tests we'd get an error (trying to write to `None`).
Since those tests aren't intended to test the "store and obey
Retry-After" behavior, I figured it's reasonable to skip it in the name
of testing; but of course, open to idea about the best way to navigate
this.
Part of #10355
Alternative to https://github.com/certbot/certbot/pull/10408/ and
https://github.com/certbot/certbot/pull/10415/ that fixes production
code for account meta and puts autouse fixtures in certbot and acme
tests. Overrides all `time.sleep` calls while we're at it.
Fixes the production code where it's simple/clean, and fixes the tests
for HTTPServer-based code because we just don't have that many mac users
using standalone.
The responsibility for atomic updates to a config file was previously
spread across different functions. This moves the atomic update to the
lowest level of the call graph.
Also, factor out the code that creates a ConfigObj based on various
inputs (archive dir, cert locations, and renewal params). This allows
cleanly reusing it across the "update" and "create new" paths.
Now the "create new config" code path doesn't have to do any renaming,
and the "update config" code path can assume the input file exists (and
error if it doesn't).
This will make it easier and cleaner to reuse the config-writing code in
#10377.
Part of #10355
this is another part of https://github.com/certbot/certbot/issues/10195
the first commit was done automatically with the command:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot/src/certbot/_internal
```
this was unfortunately insufficient as it left a line in webroot.py with
over 100 characters. i fixed this manually in my second commit
this is another part of https://github.com/certbot/certbot/issues/10195
these changes were done automatically with the commands:
```
ruff check --fix --extend-select UP006 --unsafe-fixes certbot
git checkout certbot/src/certbot/_internal
```
fixing up the files under certbot/src/certbot/_internal will be done in
another PR
This sets up a `pyproject.toml` file for certbot, initially generated
[using](https://hatch.pypa.io/latest/intro/#existing-project) `hatch new
--init` and modifying from there. Since we dynamically require acme of a
matching version, I kept that around in `setup.py` to do the simplest
thing in this PR.
Other possible (future) implementations:
- setuptools has a beta implementation to read from a
`requirements.txt`. we could generate one of those.
- we could just hardcode it and update at release time. I like not
having to keep the version up to date in various places but maybe it's
actually fine
- something something integration with poetry pinning?
I think the syntax for setting version dynamically in `pyproject.toml`
is much nicer than what we do in `setup.py`. It's a little silly to do
it there after we've bothered to calculate it, but I put it there in the
hopes of being able to remove it from `setup.py` someday/somehow.
It would be nice to access the version dynamically set in
`pyproject.toml` in `setup.py`, but I do not think it is likely to be
possible.
Here are some useful links regarding this migrations:
- [How to modernize a `setup.py` based
project?](https://packaging.python.org/en/latest/guides/modernize-setup-py-project/)
- [Writing your
`pyproject.toml`](https://packaging.python.org/en/latest/guides/writing-pyproject-toml/)
- [Configuring setuptools using `pyproject.toml` files](
https://setuptools.pypa.io/en/latest/userguide/pyproject_config.html)
- [`pyproject.toml`
specification](https://packaging.python.org/en/latest/specifications/pyproject-toml/)
- [Platform compatibility
tags](https://packaging.python.org/en/latest/specifications/platform-compatibility-tags/)
Alternative implementation for #7908.
In this PR:
- set up ruff in CI (add to `tox.ini`, mark dep in `certbot/setup.py`)
- add a `ruff.toml` that ignores particularly annoying errors. I think
line length isn't actually necessary to set with this workflow since
we're not checking it but putting it there for future usage.
- either fix or ignore the rest of the errors that come with the default
linting configuration. fixed errors are mostly unused variables. ignored
are usually where we're doing weird import things for a specific reason.
Part of #10355.
This allows combining the NamespaceConfig object with a cache of ACME
clients, so we don't have to make the whole large NamespaceConfig object
available all the way down the renewal call stack.
The new AriClientPool is responsible for instantiating ACME clients for
the purpose of ARI fetching. It provides only a simple user agent,
listing the Certbot version. The only CLI flag it observes is
--no-verify-ssl.
i wrote this in response to erica's thread at
https://opensource.eff.org/eff-open-source/pl/xtuemgdti78xfx1hn9jwbrfrjy
this hopefully helps some, but i think our logic here is unfortunately
fairly complicated which is reflected in the code comments. feel free to
suggest alternate wording or even just close this if you think our
comments currently in main are good enough
blast from the past! resurrects
https://github.com/certbot/certbot/pull/9803 with all of @bmw's changes.
i figured instead of force-pushing a basically brand new branch and
obliterating the old review, i'd just start from a clean slate
fixes#8272
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@eff.org>
fixes https://github.com/certbot/certbot/issues/10336 and fixes
https://github.com/certbot/certbot/issues/10357 using the plan at
https://github.com/certbot/certbot/issues/10336#issuecomment-3109192677
while this PR makes the renewal_time function slightly less nice, i
think us catching and handling the exceptions in certbot makes the most
sense so we can do exactly what we want around terminal and file logging
with this change, a output from a failed `sudo certbot renew` run looks
like
```
$ sudo certbot renew
Saving debug log to /var/log/letsencrypt/letsencrypt.log
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Processing /etc/letsencrypt/renewal/example.org.conf
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
An error occurred requesting ACME Renewal Information (ARI). If this problem persists and you think it's a bug in Certbot, please open an issue at https://github.com/certbot/certbot/issues/new/choose.
Certificate not yet due for renewal
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
The following certificates are not due for renewal yet:
/etc/letsencrypt/live/example.org/fullchain.pem expires on 2025-10-23 (skipped)
No renewals were attempted.
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
```
`sudo certbot renew -q` produces no output and the relevant messages in
the log file look like:
```
2025-07-30 19:51:13,267:WARNING:certbot._internal.renewal:An error occurred requesting ACME Renewal Information (ARI). If this problem persists and you think it's a bug in Certbot, please open an issue at https://github.com/certbot/certbot/issues/new/choose.
2025-07-30 19:51:13,267:DEBUG:certbot._internal.renewal:Error while requesting ARI was:
Traceback (most recent call last):
File "/home/brad/certbot/acme/src/acme/client.py", line 366, in renewal_time
raise ValueError('im some error')
ValueError: im some error
The above exception was the direct cause of the following exception:
Traceback (most recent call last):
File "/home/brad/certbot/certbot/src/certbot/_internal/renewal.py", line 351, in _ari_renewal_time
return acme.renewal_time(cert_pem)[0]
File "/home/brad/certbot/acme/src/acme/client.py", line 370, in renewal_time
raise errors.ARIError(error_msg, now + default_retry_after) from e
acme.errors.ARIError: ('failed to fetch renewal_info URL https://acme-staging-v02.api.letsencrypt.org/acme/renewal-info/oXQaBm1Qt4YtSizBfrSNiElszRY.LMjTHFS4HPbSRMOzLrA9OZId', datetime.datetime(2025, 7, 31, 1, 51, 13, 267088))
```
something weird happened to the changelog in
https://github.com/certbot/certbot/pull/10319. a 4.2.0 entry was added
below the entry for `5.0.0 - main` despite 4.2.0 not having been
released. since it's sounding like we're expecting our next release to
be 4.2.0 and not 5.0, i merged these two changelog entries into one for
4.2.0
i also modified our setup.py files to use 4.2.0.dev0 instead of
5.0.0.dev0 altho this isn't strictly necessary because our release
script will automatically set all version numbers to whatever version we
give it on the command line before building the release
Fixes https://github.com/certbot/certbot/issues/10342
When doing ARI checks in acme.renewal_time, we catch RequestException
and return a default value. That's so an unavailable ARI server doesn't
cause issues.
Before we get to acme.renewal_time, we have to create an ACME client,
and in the process fetch a directory. We should make the directory fetch
similarly resilient.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
When making an ARI request, use the server listed in the cert's renewal
conf rather than the one passed in certbot's config.
Fixes#10339
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Another token of gratitude for a super useful tool and service.
More about codespell: https://github.com/codespell-project/codespell .
I personally introduced it to dozens if not hundreds of projects already
and so far only positive feedback.
CI workflow has 'permissions' set only to 'read' so also should be safe.
---------
Signed-off-by: Yaroslav O. Halchenko <debian@onerussian.com>
Fixes https://github.com/certbot/certbot/issues/10259
This PR moves post-hook execution from `main.renew` to
`renewal.handle_renewal_request` so that failed and renewed domains
actually get passed into post-hook execution as promised, even when
failures happened.
I suspect the original PR was being overly cautious by putting the whole
thing into a try/finally so that post-hooks definitely happen, but
`handle_renewal_request` is already full of exception catching. I
understand the worry about executing a pre-hook and then failing to
execute its matching post-hook, but the code really is already
structured to make sure that that won't happen. And then when we added
`FAILED_DOMAINS` and `RENEWED_DOMAINS`, we both kept that
overly-cautious hooks execution location, but also kept the error so we
have a summary at the end...which meant that if failures happened, the
env vars were never set.
If we really want to keep the `hooks.run_saved_post_hooks` call on the
outside of everything in main, we can, but then we will have to do one
of the following:
- pass in the output lists to be filled out during execution. not my
favorite pattern
- throw the output lists in the error object or make a wrapper error,
not sure, haven't looked at `errors.py` too closely
- stop raising that final error where we report failures at the very
bottom. it's a little outdated maybe but I do like it and I think people
are used to it
- raise that error in main, returning the number of parse and renewal
failures. this is my favorite of the options, but I still like it less
than what I've implemented here.
Here's the integration/regression test failing on main:
https://dev.azure.com/certbot/certbot/_build/results?buildId=9237&view=logs&j=fca58cec-e7ce-563a-f36f-5c233894d750
You can see here that that branch just has the integration test without
the fix (and removing other tests for efficiency):
https://github.com/certbot/certbot/compare/main...test-fail-env-on-main
It's the default, but just to be clear, this should definitely have two
reviewers.
certbot's standalone code contains confusing references to things like
`SSLSocket` which we were hoping to deprecate in
https://github.com/certbot/certbot/issues/10284. are they relevant?
they're sure not!
certbot's standalone plugin only supports HTTP-01 so comments about
things like `ACMETLSServer` and the completely unused `certs` variable
can be deleted
furthermore, the type of the different variables named things like
`http_01_resources` were wrong in multiple places. as can be seen in
certbot's standalone code, the type is
`Set[acme_standalone.HTTP01RequestHandler.HTTP01Resource]`. this is also
[the type used in acme.standalone's
tests](https://github.com/certbot/certbot/blob/723fe64d4dbc778f1e8fd0b93320c1a30dc6fa95/acme/src/acme/_internal/tests/standalone_test.py#L78-L81)
despite the file's type annotations saying it takes a different type. i
think the incorrect type annotations were never caught because mypy
can't fully make sense of our overly complex server classes here
finally, `from __future__ import annotations` was added to make [forward
references in type
annotations](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#forward-references)
easier
This is one solution to https://github.com/certbot/certbot/issues/10327.
It won't test an ARI check during a dry run, since it will just avoid
the mismatch problem by checking for dry run first and returning before
checking ARI. This PR will make the big error (actually a warning, but
red and scary) go away though.
I thought https://github.com/certbot/certbot/pull/9804/ was abandoned
but the author just missed my comment. I would like to accept that PR to
get it in, but in the process of updating the PR I wrote a nicer
changelog entry, so I would like to add that.
When a CA fails to issue a certificate after finalisation Certbot
currently prints the following unhelpful message:
```
An unexpected error occurred:
acme.errors.IssuanceError
```
This PR makes Certbot print the ACME error object from the order, as
such
```
An unexpected error occurred:
CAA error :: Invalid CAA: CAA prohibits issuance
```
## Pull Request Checklist
- [ ] The Certbot team has recently expressed interest in reviewing a PR
for this. If not, this PR may be closed due our limited resources and
need to prioritize how we spend them.
- [x] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
edit the `master` section of `certbot/CHANGELOG.md` to include a
description of the change being made.
- [x] Add or update any documentation as needed to support the changes
in this PR.
- [x] Include your name in `AUTHORS.md` if you like.
Brad Warren