* Fix TTL mismatch leading to HTTP 412
This PR is a follow up from #8521 where we address the
issue of potentially having a mismatch of TTL when executing
a DNS change (transaction = deletion + additions). Let's say
we have a record `foo.org 30 IN TXT foo-content` with TTL 30s,
when creating challenge or cleaning we might need to perform
a deletion operation in the transaction. Currently certbot
would ask Google API to delete the foo record like this:
`foo.org 60 in TXT foo-content` ignoring the record's original
TTL and using 60s instead. This leads to HTTP 412 as Google would
expect a perfect match of what we want to delete with what it is
on the DNS. See also #8523
* remove ttl from default data to avoid confusions
* Refactor tests and add a missing case
This commit adds a test that covers the case when we are
deleting a TXT record which contains a single rrdatas. Also,
refactoring a couple of tests.
* Make get_existing_txt_rrset documentation more precise about return value
* Add missing assertions in tests.
* fix linting issues
* Mention fix on changelog
* Explain fix around user impact
* Explain what happens when no records are returned
* Update certbot/CHANGELOG.md
* Update certbot/CHANGELOG.md
* update_account: print correct message for -m ""
When -m "" was passed on the CLI, Certbot would print that it updated
the email to '' (an empty string) rather than printing that it removed
the contact details.
This commit also refactors the update_account tests to be a bit more
modern.
* use addCleanup instead of tearDown in tests
* Fix fetch of existing records from Google DNS
There has been many complaints regarding `certbot_dns_google` plugin
failing with:
* HTTP 412 - Precondition not met
* HTTP 409 - Conflict
See #6036. This PR fixes that situation. The bug lies on how we
fetch the TXT records from google. For large amount of records
the Google API paginates the result but we ignore the subsequent
pages and assume that if the record is not in the first response then
it doesn't exist. This leads to either HTTP 409, or HTTP 412 or both.
In this PR we leverage the use of filters on the API to get exactly
the records we are looking for. Apart from fixing the problem stated
above, it has the extra benefit of making the process faster by
reducing the amount of API calls and it doesn't require us to handle
any pagination logic
* Explain changes on CHANGELOG
* Edit AUTHORS.md
* make execute static
* Update certbot/CHANGELOG.md
Being more specific for which plugin this fix bug is meant for.
Co-authored-by: alexzorin <alex@zor.io>
* Fix if expression to be more python-idiomatic
Co-authored-by: alexzorin <alex@zor.io>
* Sort AUTHORS.md
* Simplify tests
Make rrs_mock modeling simpler and refactor
* Revert "Simplify tests"
This reverts commit 9de9623ba7.
* Reimplement conditional mock
We still want to use a conditional mock by make it more
simple to understand by using MagicMock.
* Revert "Sort AUTHORS.md"
This reverts commit b3aa35bcf1.
* Add name in AUTHORS.md
Co-authored-by: alexzorin <alex@zor.io>
Fixes https://github.com/certbot/certbot/issues/8519.
I left the `certbot-auto` docs in `install.rst` to avoid breaking links and to help propagate information about our changes there. I moved it closer to the bottom of the doc though since I think our documentation about OS packages and Docker is more helpful to most people.
* clean up certbot-auto docs
* add more info to changelog
* remove more certbot-auto references
Although Certbot is a classic snap, it shouldn't load Python code from
the host system. This change prevents packages being loaded from the
"user site-packages directory" (PEP-370). i.e. Certbot will no longer
load DNS plugins installed via `pip install --user certbot-dns-*`.
* cli: clean up `certbot renew` summary
- Unduplicate output which was being sent to both stdout and stderr
- Don't use IDisplay.notification to buffer output
- Remove big "DRY RUN" guards above and below, instead change language
to "renewal" or "simulated renewal"
- Reword "Attempting to renew cert ... produced an unexpected error"
to be more concise.
* add newline to docstring
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Fixes https://github.com/certbot/certbot/issues/8495.
To further explain the problem here, `modify_kwargs_for_default_detection` as called in `add` is simplistic and doesn't always work. See https://github.com/certbot/certbot/issues/6164 for one other example.
In this case, were bitten by the code https://github.com/certbot/certbot/blob/d1e7404358c05734aaf436ef3c9d709029d62b09/certbot/certbot/_internal/cli/helpful.py#L393-L395
The action used for deprecated arguments isn't in `ZERO_ARG_ACTIONS` so it assumes that all deprecated flags take one parameter.
Rather than trying to fix this function (which I think can only realistically be fixed by https://github.com/certbot/certbot/issues/4493), I took the approach that was previously used in `HelpfulArgumentParser.add_deprecated_argument` of bypassing this extra logic entirely. I adapted that function to now call `HelpfulArgumentParser.add` as well for consistency and to make testing easier.
* Rename deprecated arg action class
* Skip extra parsing for deprecated arguments
* Add back test of --manual-public-ip-logging-ok
* Add changelog entry
(cherry picked from commit 5f73274390)
* nginx: fix py2 unicode sandwich
The nginx parser would crash when saving configuraitons containing
Unicode, because py2's `str` type does not support Unicode.
This change fixes that crash by ensuring that a string type supporting
Unicode is used in both Python 2 and Python 3.
* nginx: add unicode to the integration test config
* update CHANGELOG
Fixes https://github.com/certbot/certbot/issues/8134.
* Test on Python 3.9.
* Mention Python 3.9 support in changelog.
* s/\( *'Pro.*3\.\)8\(',\)/\18\2\n\19\2/
* undo changes to tox.ini
* Move more tests to Python 3.9
* Update PyYAML and packages which pinned it back
* Upgrade typed-ast
* Use <= to "pin" dnspython
* Fix lint by telling pylint it cannot be trusted
* Disable mypy on RFC plugin
* add comment about <= support
* cli: improve Obtaining/Renewing wording
* dont use logger, and use new phrasing
* .display_util.notify: dont wrap
As this function is supposed to be an analogue for print, we do not want
it to wrap by default.
Fixes#7717
This PR adds a `--dns-server` option to the `run_acme_server` test tool, in order to provide an arbitrary DNS server to Pebble or Boulder for the integration tests.
I also take this occasion to make `run_acme_server` a real CLI tool using argparse, and set the `--server-type` (default `pebble`) option as well.
* Set --dns-server flag in run_acme_server
* Default to pebble
* Add documentation
* Configure also Boulder
Fixes#8365
This PR adds a control when `certbot certonly` or `certbot run` are called for a certificate that already exists and would eventually be replaced. As described in #8365, this control is here to ensure that the user will not modify the key type of their certificate (eg. ECDSA to RSA) without an explicit approval (set explicitly `--cert-name` and `--key-type`), since RSA is the default if not specified.
* Handle unexpected key type migration.
* Update certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* tests: fix leaking patch in eff_test.py
* tests: PrintTest->NotifyTest in .display.util
The function was renamed during #8432. This change renames the test as
well.
* IDisplay.notification: add `decorate` param.
The flag allows the caller to control whether the message will be
printed in a decorated way (wrapped by hlines) or in an undecorated
way (similar to print).
It is set to true by default, to reflect the existing behavior of the
function.
* IDisplay.notification: write message to debug log
In the same vein as IReporter, this ensures that all notifications which
are shown to the user also make an appearance in the debug log, which
will aid in troubleshooting.
* restore accidentally deleted newline in decoration
* add helper function for printing status messages
* register: use notify rather than logger
Undoes the change in #8393 in favor of the new helper
* comment .display and ._internal.log
Describing when it is suitable to use each
* add more comments to log.py
* make IDisplay.notification decorate arg private
* rename notify->print and move to .display.util
* rename .display.print back to .display.notify
because linters complain about print being a redefined builtin
* Remove python_version from mypy.ini.
* Fix magic_typing
* Ignore msvcrt usage.
* make mypy happier
* clean up changes
* Add type for reporter queue
* More mypy fixes
* Fix pyrfc3339 str.
* Remove unused import.
* Make certbot.util mypy work in both Pythons
* Fix typo
This PR adds the following documentation improvements to fix https://github.com/certbot/certbot/issues/7958:
- Simplify building external plugins
- Separate out certbot snap instructions from plugin instructions
- Mention that dnsimple is just an example for the plugin instructions
- Mention remote build for other architectures
- Mention snap doc exists elsewhere in developer guide (`contributing.rst`)
* Set up generate_dnsplugins_all.sh for all files and parametrize snapcraft and postrefreshhook files
* Create constraints file in the generate_dnsplugins_all script
* Separate out plugin and certbot snaps and update instructions
* Add remote build instructions
* Add pointers to the README to contributing.rst
* Add timeout to DNS query function calls
* Modify tests to account for new timeout variable
* Add change to CHANGELOG
* Add `dns.exception.Timeout` to exception handler
* Move changelog to 1.10.0
