based on the suggestion @bmw made in #10484, this moves nearly
everything from `certbot-apache` and `certbot-nginx` into subdirectories
in `certbot/src/certbot/_internal`, and corresponding "extra"
dependencies are made for the certbot distribution. in their place,
entrypoint shims are made in the old distributions.
this way, installing `certbot[nginx]` will pull in the extra
dependencies needed for the nginx code, and also pull in the shim in
`certbot-nginx`, letting our plugin discovery system work just as it did
before. ditto for apache.
note that this doesn't yet deprecate anything, which was one of the
primary goals of the original issue -- i spun out that work into #10521fixes#10484
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: ohemorange <erica@eff.org>
this is part of https://github.com/certbot/certbot/issues/10517
to update this description in response to the discussion below, i'd
recommend reviewing this PR by commit. the first commit just moves
ocsp.py under _internal making no other changes while the second commit
fixes everything else up. the diff really isn't as big here as it looks
i noticed this when reviewing jsha's upcoming blog post
this probably should have been done as part of
https://github.com/certbot/certbot/pull/10544, but we forgot to do it
then
i don't think this PR requires two reviews
A few largely unused functions/types have been deprecated in our effort
to remove our pyOpenSSL dependency:
* Deprecated: `certbot.crypto_util.get_sans_from_cert`
* Deprecated: `certbot.crypto_util.get_names_from_cert`
* Deprecated: `certbot.crypto_util.get_names_from_req`
* Deprecated: `certbot.crypto_util.import_csr_file` (and replaced by
`certbot.crypto_util.read_csr_file`)
* Deprecated: `acme.crypto_util.Format`
`read_csr_file` now always returns a PEM formatted CSR, since that's
what was happening in practice, and therefore lets us stop having to
return a `Format`, so we will be able to stop importing it.
first half of #10433
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
The manual plugin offers environment variables for its hook called
CERTBOT_DOMAIN and CERTBOT_ALL_DOMAINS. I added CERTBOT_IDENTIFIER and
CERTBOT_ALL_IDENTIFIERS, while keeping the old variables for backwards
compatibility. Certbot will pass IP addresses in the CERTBOT_DOMAIN
environment variable rather than erroring out.
Part of #10346
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
https://github.com/certbot/certbot/pull/10146 was supposed to do this,
but because of multiple code paths, it did not. This PR simplifies the
code by creating a single code path.
In particular:
- `hooks.renew_hook()` is removed. There are now only calls to
`hooks.deploy_hook()`, which is called during certonly, run, and renew,
and runs both cli and directory hooks.
- `cli_config.renew_hook` is removed. Both `--renew-hook` (hidden option
kept for backwards compatibility purposes and `--deploy-hook` now set
`cli_config.deploy_hook`, which is used internally. When either or both
flags are used multiple times, the last value is kept, which is the
argparse default.
- references to running a "renew hook" internally are changed to "deploy
hook"
- To maintain downgrade compatibility, `deploy_hook` is written out to
renewal config files as `renew_hook`. This is achieved by translating to
and from `renew_hook` in `storage.py` and changing
`renewal.STR_CONFIG_ITEMS` to contain `deploy_hook`.
This results in the following behavior changes:
- Directory hooks are now run when getting a new cert using certonly/run
- If someone set a renew hook on the cli using `--renew-hook`, it would
previously not be run when getting a new (non-renewed) cert, but now
will be. But this option is hidden and should no longer be used anyway.
- When using `certbot reconfigure`, if someone sets `--renew-hook`
certbot will now also ask if someone would like to do a test run of the
new hook, whereas before it would only do so for `--deploy-hook`.
---------
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
i'm creating this PR in response to
47c5c88fe1
where ohemorange manually deleted .DS_Store on jsha's PR
rather than doing that or having new certbot devs manually configure
their system to ignore these files, let's just do it for them
i don't think this PR requires two reviews
as i mentioned at
https://github.com/certbot/certbot/pull/10509#discussion_r2601282033, i
didn't love how the tests were using `_DomainsAction` when i think the
leading underscore suggests the class is internal to the cli/cl_utils
module
this PR fixes that
also, i don't think this PR requires two reviews
Cherry-picks #10509 which fixes#10506. This will eventually make its
way into the 5.2.2 point release
Co-authored-by: Jacob Hoffman-Andrews <github@hoffman-andrews.com>
Fixes#10506.
When --webroot-path was specified multiple times, Certbot was erroring
with `DNSName SAN compared to non-SAN`. That's because, in the
_WebrootPathAction that builds `namespace.webroot_path`, we were passing
`domain` (type `san.DNSName`) as the keys. The other code that modifies
or accesses `namespace.webroot_path` expects the keys to be of type
`str`. In particular `webroot.Authenticator._set_webroots` does:
```python
for achall in achalls:
self.conf("map").setdefault(achall.domain, webroot_path)
```
Where `achall.domain` is a `str`.
Two existing unittests would have caught this: `test_multiwebroot` and
`test_webroot_map_partial_without_perform`. However, they faked out the
parsing of the `--domains` flag, and that faked out code was not updated
in #10468. Since this bug is caused by an interaction between the types
produced by the `--domains` flag and those produced by the
`--webroot-path` flag, the tests failed to catch the problem. I've
updated the tests and confirmed that they fail before the fix is
applied.
This field is optional to maintain backwards compatibility. Note that
`AnnotatedChallenge` inherits from `jose.ImmutableMap`, which has a
[check in
__init__](4b74747670/src/josepy/util.py (L125-L131))
that all slots are provided. That check would not allow us to do a
backwards-compatible addition, so I implemented an `__init__` for each
of these subclasses that fills the fields without calling the parent
`__init__`, and so doesn't hit an error when `identifier` is absent.
I chose to use `acme.messages.Identifier` rather than
`certbot._internal.san.SAN` here because these are wrapped ACME types,
so they should use the ACME representation. Also, `AnnotatedChallenge`
is passed to plugins, so we need to pass a type that the plugins can
understand.
Additionally, `domain` is marked as deprecated.
Part of #10346
/cc @bmw, who noticed the issue with `AnnotatedChallenge`
[here](https://github.com/certbot/certbot/pull/10468#issuecomment-3403294394)
and provided additional feedback
[here](https://github.com/jsha/certbot/pull/2#issuecomment-3534895793).
Note that there's still some work to do to finish excising `domain`
assumptions from this portion of the code.
---------
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Closes#10348.
## 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.
- [ ] If the change being made is to a [distributed
component](https://certbot.eff.org/docs/contributing.html#code-components-and-layout),
add a description of your change to the `newsfragments` directory. This
should be a file called `<title>.<type>`, where `<title>` is either a
GitHub issue number or some other unique name starting with `+`, and
`<type>` is either `changed`, `fixed`, or `added`.
* For example, if you fixed a bug for issue number 42, create a file
called `42.fixed` and put a description of your change in that file.
- [ ] Add or update any documentation as needed to support the changes
in this PR.
- [ ] Include your name in `AUTHORS.md` if you like.
In #10478 we added a `san.SAN` class, with two subclasses `san.DNSName`
and `san.IPAddress`, so we can carry type information about identifiers
through the Certbot code. This PR plumbs through those types in most
Certbot-internal code. Note that this does not change the `acme` module,
which uses `messages.Identifier`. It also tries to leave alone the code
paths into plugins.
This does not add a CLI flag to request an IP address certificate. That
will be in a followup PR.
Part of #10346
Contains san.DNSName, san.IPAddress, and a parent class san.SAN.
Split out from #10468 as a standalone PR. To see examples of how it's
intended to be used, please see that PR.
The constructor for DNSName incorporates the same validation done in
`enforce_domain_sanity`, and the tests from `enforce_domain_sanity` are
copied here as well. The goal is to delete `enforce_domain_sanity`
entirely as part of #10468.
In support of #10346.
