In terms of UX, dns-persist-01 functions nearly identically to dns-01,
except once the TXT record has been made, no further action is required
from the user.
This is just wholesale copied from certbot's utils. Maybe we could
DRY it up by having certbot rely on acme's, but IMO it's not a complex
enough function to justify that.
this is in response to
https://github.com/certbot/certbot/security/dependabot/126
as you can see by examining the github status checks on this PR, i ran
the full test suite and everything passed
i also don't think this PR requires two reviews
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>
Fixes https://github.com/certbot/certbot/issues/10180.
So first of all, the core issue here is that [pyca deliberately
chose](https://github.com/pyca/cryptography/blob/ec80c1c2894320d30fd674ea2c6103d91b4e777e/src/cryptography/utils.py#L15-L18)
to override the default python functionality and make deprecation
warnings appear by default. This isn't common. If they'd actually used a
`DeprecationWarning`, it wouldn't have shown up to users, at least. That
being said, we should still try to catch it, as we do in fact want to
know about deprecation warnings for our own updates.
To do that, this PR searches upwards for a `pytest.ini` file from the
file's location. If found, it reads the warnings from the file, and
passes them using the `PYTHONWARNINGS` env variable. It also explicitly
sets warnings to `error` always in case we can't find the `pytest.ini`,
and ignores the subsequent unverified-https-on-localhost warning. It
also fixes a warning in our test nginx config that seemed reasonable to
address.
I tested this by adding a temporary warning, which I then removed, but
since it turned out there were two other warnings, that wasn't actually
necessary.
Options I considered and rejected:
- Switch from `atexit` to calling `main` directly. To do this, we'd have
to switch our `main` function to something like a try-finally. That's
complicated by the fact that we call `atexit` from other places in the
code. Also, `exc_info` isn't availabe in `finally` while it is in
`at_exit`, so it's not as versatile. But mostly if we wanted to do this,
we'd have to implement a custom atexit handler, basically, and that
seems worse than this option.
- Looking into pytest-forked. It's apparently buggy and not being
maintained. Not even sure this is what it's for anyway.
- Multiple
[-W](https://docs.python.org/3/using/cmdline.html#cmdoption-W) options
can be given instead of an env variable. The env version seemed cleaner.
- More closely mimicking [how pytest finds ini
files](https://docs.pytest.org/en/stable/reference/customize.html#finding-the-rootdir).
It seemed unnecessary to me.
Potential drawbacks:
- If we move or rename the `pytest.ini` file and for some reason don't
do a reasonable grep for `pytest.ini`, we will no longer catch any
additional `ignore`s in there. But imo we're likely to do that grep, and
also a missing ignore will then show up when we run the tests.
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
i just updated this credential in CI and created a calendar event to
help us remember to update it
with the calendar event, i don't think we need the code comment here. it
wasn't updated last time and it's one less thing for us to remember to
do next year
i don't think this PR requires two reviews
with the changes i made to CI and our calendar, i think i can say this
fixes#10563
Will Greenberg