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>
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.
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.
Fixes#10312. This is perhaps overly detailed, but I was hoping that by
giving a viable path forward it would forestall requests to change it
back, add a flag to ignore ari, or otherwise change the behavior. Very
open to suggestions on wording/content/length/etc.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Previously, we were constructing an ACME client for ARI checking that
used the global value for `server`, not the one recorded in a lineage's
renewal file.
This resulted in errors in the logs and failure to observe ARI for
lineages that used a non-default `--server` (e.g. staging or non-Let's
Encrypt CAs).
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Fixes#10266.
See example deprecation in
https://github.com/certbot/josepy/pull/207/files
I can add stacklevel=2, though I find that usually I just look at the
whole stack anyway when debugging, myself, so it doesn't really matter.
Fixes#10252.
See further discussion here: https://github.com/pypa/pip/issues/11457
We are doing option:
> Alternatively, enable the --use-pep517 pip option, possibly with
--no-build-isolation. The --use-pip517 flag will force pip to use the
modern mechanism for editable installs. --no-build-isolation may be
needed if your project has build-time requirements beyond setuptools and
wheel. By passing this flag, you are responsible for making sure your
environment already has the required dependencies to build your package.
Once the legacy mechanism is removed, --use-pep517 will have no effect
and will essentially be enabled by default in this context.
Major changes made here include:
- Add `--use-pep517` to use the modern mechanism, which will be the only
mechanism in future pip releases
- Change to `/src` layout to appease mypy, and because for editable
installs that really is the normal way these days.
- `cd acme && mkdir src && mv acme src/` etc.
- add `where='src'` argument to `find_packages` and add
`package_dir={'': 'src'},` in `setup.py`s
- update `MANIFEST.in` files with new path locations
- Update our many hardcoded filepaths
- Update `importlib-metadata` requirement to fix
double-plugin-entry-point problem in oldest tests
Fixes
https://github.com/certbot/certbot/issues/9835#issuecomment-2717096178,
where our `RewriteEngine on` directive inserted at the beginning of a
virtualhost was overridden a `RewriteEngine Off` directive later. This
PR does the easy thing of placing `RewriteEngine on` in our
post-insert.
Previously we defaulted to renewing at 30 days before expiry, and
allowed users to customize the config file to set a different value.
Instead, we should renew when 1/3 of the lifetime is left, or for
shorter certificates (<10 days), when 1/2 of the lifetime is left.
This still allows explicitly configured values to take precedence.
---------
Co-authored-by: Will Greenberg <ifnspifn@gmail.com>
We strongly encouraged providing an email address because we wanted
people to get expiration notices to ensure that even if their Certbot
install broke, they could fix it before their site goes down.
Now that Let's Encrypt is getting rid of expiration notices
(https://letsencrypt.org/2025/01/22/ending-expiration-emails/), we can
remove some of the encouragement, providing a smoother user experience.
Brad Warren