Streamline and reorganize Certbot's CLI output.
This change is a substantial command-line UX overhaul,
based on previous user research. The main goal was to streamline
and clarify output. To see more verbose output, use the -v or -vv flags.
---
* nginx,apache: CLI logging changes
- Add "Successfully deployed ..." message using display_util
- Remove IReporter usage and replace with display_util
- Standardize "... could not find a VirtualHost ..." error
This changes also bumps the version of certbot required by certbot-nginx
and certbot-apache to take use of the new display_util function.
* fix certbot_compatibility_test
since the http plugins now require IDisplay, we need to inject it
* fix dependency version on certbot
* use better asserts
* try fix oldest deps
because certbot 1.10.0 depends on acme>=1.8.0, we need to use
acme==1.8.0 in the -oldest tests
* cli: redesign output of new certificate reporting
Changes the output of run, certonly and certonly --csr. No longer uses
IReporter.
* cli: redesign output of failed authz reporting
* fix problem sorting to be stable between py2 & 3
* add some catch-all error text
* cli: dont use IReporter for EFF donation prompt
* add per-authenticator hints
* pass achalls to auth_hint, write some tests
* exclude static auth hints from coverage
* dont call auth_hint unless derived from .Plugin
* dns fallback hint: dont assume --dns-blah works
--dns-blah won't work for third-party plugins, they need to be specified
using --authenticator dns-blah.
* add code comments about the auth_hint interface
* renew: don't restart the installer for dry-runs
Prevents Certbot from superfluously invoking the installer restart
during dry-run renewals. (This does not affect authenticator restarts).
Additionally removes some CLI output that was reporting the fullchain
path of the renewed certificate.
* update CHANGELOG.md
* cli: redesign output when cert installation failed
- Display a message when certificate installation begins.
- Don't use IReporter, just log errors immediately if restart/rollback
fails.
- Prompt the user with a command to retry the installation process once
they have fixed any underlying problems.
* vary by preconfigured_renewal
and move expiry date to be above the renewal advice
* update code comment
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* update code comment
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* fix lint
* derve cert name from cert_path, if possible
* fix type annotation
* text change in nginx hint
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* print message when restarting server after renewal
* log: print "advice" when exiting with an error
When running in non-quiet mode.
* try fix -oldest lock_test.py
* fix docstring
* s/Restarting/Reloading/ when notifying the user
* fix test name
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* type annotations
* s/using the {} plugin/installer: {}/
* copy: avoid "plugin" where possible
* link to user guide#automated-renewals
when not running with --preconfigured-renewal
* cli: reduce default logging verbosity
* fix lock_test: -vv is needed to see logger.debug
* Change comment in log.py to match the change to default verbosity
* Audit and adjust logging levels in apache module
* Audit and adjust logging levels in nginx module
* Audit, adjust logging levels, and improve logging calls in certbot module
* Fix tests to mock correct methods and classes
* typo in non-preconfigured-renewal message
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* fix test
* revert acme version bump
* catch up to python3 changes
* Revert "revert acme version bump"
This reverts commit fa83d6a51c.
* Change ocsp check error to warning since it's non-fatal
* Update storage_test in parallel with last change
* get rid of leading newline on "Deploying [...]"
* shrink renewal and installation success messages
* print logfile rather than logdir in exit handler
* Decrease logging level to info for idempotent operation where enhancement is already set
* Display cert not yet due for renewal message when renewing and no other action will be taken, and change cert to certificate
* also write to logger so it goes in the log file
* Don't double write to log file; fix main test
* cli: remove trailing newline on new cert reporting
* ignore type error
* revert accidental changes to dependencies
* Pass tests in any timezone by using utcfromtimestamp
* Add changelog entry
* fix nits
* Improve wording of try again message
* minor wording change to changelog
* hooks: send hook stdout to CLI stdout
includes both --manual and --{pre,post,renew} hooks
* update docstrings and remove TODO
* add a pending deprecation on execute_command
* add test coverage for both
* update deprecation text
Co-authored-by: ohemorange <ebportnoy@gmail.com>
Co-authored-by: Alex Zorin <alex@zorin.id.au>
Co-authored-by: alexzorin <alex@zor.io>
Since Saturday the CI pipeline is failing due to several Sphinx errors. See https://dev.azure.com/certbot/certbot/_build/results?buildId=3928&view=logs&j=d74e04fe-9740-597d-e9fa-1d0400037dfd&t=dde413a4-f24c-59a0-9684-e33d79f9aa02
First, the build of certbot-dns-google is failing because of a particular configuration. It seems that this configuration has been written here to activate the support of the RST instruction `.. code-block:: json` in documentation. However, it does not seem to be necessary for a similar situation in certbot-dns-route53 documentation. So let's try to remove it and fix the Sphinx builds.
Second, Sphinx builds were not pinning dependencies, so Sphinx 4.x (that has been released yesterday) started to be used in the pipeline. Sadly this new version is not compatible with the plugin `repoze.sphinx.autointerface`, used to extract documentation from `zope.interface`. So I fixed the pinning and also explicitly pin Sphinx to 3.5.x for now.
Technically speaking the second action is sufficient to fix the first error, but I keep the dedicated solution because it improves the documentation in my opinion.
This situation could be fixed by not requiring `repoze.sphinx.autointerface`, but this is possible only if we remove `zope.interface` from Certbot. Luckily I started the work few days ago ;).
* Remove explicit lexer call in certbot-dns-google doc builds.
* Write a valid JSON file in the documentation
* Apply constraints to sphinx build environments
* Pin Sphinx to 3.5.4
* Update dependencies
* Pin traitlets
This is one of the things that newer versions of `pylint` complains about.
* git grep -l super\( | xargs sed -i 's/super([^)]*)/super()/g'
* fix spacing
Built on top of #8748, this PR reenables mypy strict mode and adds the appropriate corrections to pass the types checks.
* Upgrade mypy
* First step for acme
* Cast for the rescue
* Fixing types for certbot
* Fix typing for certbot-nginx
* Finalize type fixes, configure no optional strict check for mypy in tox
* Align requirements
* Isort
* Pylint
* Protocol for python 3.6
* Use Python 3.9 for mypy, make code compatible with Python 3.8<
* Pylint and mypy
* Pragma no cover
* Pythonic NotImplemented constant
* More type definitions
* Add comments
* Simplify typing logic
* Use vararg tuple
* Relax constraints on mypy
* Add more type
* Do not silence error if target is not defined
* Conditionally import Protocol for type checking only
* Clean up imports
* Add comments
* Align python version linting with mypy and coverage
* Just ignore types in an unused module
* Add comments
* Fix lint
* Work in progress
* Finish type control
* Isort
* Fix pylint
* Fix imports
* Fix cli subparser
* Some fixes
* Coverage
* Remove --no-strict-optional (obviously...)
* Update certbot-apache/certbot_apache/_internal/configurator.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update certbot/certbot/_internal/display/completer.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Cleanup dns_google
* Improve lock controls and fix subparser
* Use the expected interfaces
* Fix code
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Fixes https://github.com/certbot/certbot/issues/7913.
I only added the deprecation warning to `certbot.tests.util` because that's the only place where I think someone could be using the `mock` module through our API.
* remove external mock from acme
* update Certbot's mock usage
* remove mock dependency in plugins
* remove external mock from compatibility test
* add changelog entry
Fixes#8389#8584.
This PR makes the necessary modifications to officially drop Python 2 support in the Certbot project.
I did not remove the specific Python 2 compatibility branches that has been added in various places in the codebase, to reduce the size of this PR and this will be done in a future one
* Update classifiers and python_requires in setup.py
* Remove warnings about Python 2 deprecation
* Remove Azure jobs on Python 2.7
* Remove references to python 2 in documentation
* Pin dnspython to 2.1.0
* Update changelog
* Remove warning ignore
Fixes https://github.com/certbot/certbot/issues/8580.
With this PR, it should now be possible to run the oldest tests natively on Linux, at least when using an older version of Python 3, which hasn't been possible in a long time. Unfortunately, this isn't possible on macOS which I opened https://github.com/certbot/certbot/issues/8589 to track.
You can see the full test suite running with these changes at https://dev.azure.com/certbot/certbot/_build/results?buildId=3283&view=results.
I took the version numbers for the packages I updated by searching for the oldest version of the dependency I think we should try and support based on the updated comments at the top of `oldest_constraints.txt`. While kind of annoying, I think it'd be a good idea for the reviewer to double check that I didn't make a mistake with the versions I used here.
To find these versions, I used https://packages.ubuntu.com, https://packages.debian.org, and a CentOS 7 Docker image with EPEL 7 installed. For the latter, not all packages are available in Python 3 yet (which is something Certbot's EPEL package maintainers are working on) and in that case I didn't worry about the system because I think they can/will package the newest version available. If they end up hitting any issues here when trying to package Certbot on Python 3, we can always work with them to fix it.
* remove py27 from oldest name
* update min cryptography version
* remove run_oldest_tests.sh
* upgrade setuptools and pyopenssl
* update cffi, pyparsing, and idna
* expand oldest_constraints comments
* clarify oldest comment
* update min configobj version
* update min parsedatetime version
* quote tox env name
* use Python 3.6 in the oldest tests
* use Python 3.6 for oldest integration tests
* properly pin asn1crypto
* update min six version
* set basepython for a nicer error message
* remove outdated python 2 oldest constraints
* clean up some Sphinx warnings
* first attempt at a doc-test pipeline job
* fix formatting
* fix test name
* set env for bash
* try bash vs script
* maybe it didn't like me setting 'PATH'...derp
* drop use of venv
* sphinx-build isn't a py script
* try activating venv
* docs: remove unused html_static tags
* clean up final sphinx build errors for certbot
* clean up final sphinx build errors for acme
* better names for docs pipeline
* fix spelling
* add docs_extras to setup.py
* remove temp doc-testing pipeline; add template to main.yml
* rearrange pipeline execution; run sphinx builds in one job
* add documentation note to compat.os
* add uninstall.rst as a sub-toctree to avoid build error
* 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
* Added note to each DNS documentation index page to mention that plugins need to be installed and are not included as standard.
* Resolved issue with white space in doc files
* Changed wording as discussed in PR.
* Changing URL to new wildcard instructions link
* Update certbot-dns-cloudflare/certbot_dns_cloudflare/__init__.py
* 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>
This adds a 'Error parsing credentials file ...' wrapper to any errors
raised inside certbot-dns-google's usage of oauth2client, to make it
obvious to the user where the problem lies.
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
While reviewing https://github.com/certbot/certbot/pull/8404, it occurred to me that we're keeping both the generated files and the script used to generate them in `git`. Keeping both around seems unnecessary and is almost asking for the files to get out of sync at some point in the future. I fixed that by removing the files, adding them to `.gitignore`, and updating `build_remote.py` to generate them as needed.
* Remove generated files.
* Add generated files to gitignore.
* Reuse generate_dnsplugins_all.sh in build_remote
With more and more of our wildcard instructions on https://certbot.eff.org telling people to use these plugins, I think we should get ready to move our DNS plugins to the stable channel. This PR removes grade: devel so the snap store doesn't prevent us from doing that when we want to. See #8128 where we did this to the Certbot snap for more info.
You can see the snap tests passing with this change at https://dev.azure.com/certbot/certbot/_build/results?buildId=2797&view=results.
This reverts commit feca125437.
Since this change landed, ARM builds for many of the DNS plugins have failed every night. See https://dev.azure.com/certbot/certbot/_build?definitionId=5 or our public Mattermost channel.
I quickly tried to fix this myself and wasn't trivially able to do so. I tried setting `SNAPCRAFT_PYTHON_VENV_ARGS: --system-site-packages` and adding `python3-wheel` as a build dependency, but it didn't work for some reason. The `python3-wheel` package didn't seem to be installed.
I still suspect something like this is the approach we should take, however, I want to fix the failing tests now so things are no longer broken in `master` and those of us on the Certbot team at EFF stop getting spammed with 54 (!!) emails about failed builds from launchpad every night.
Unfortunately, while I was working on this the queue for ARM machines on Launchpad jumped up to an estimated ~20 hour wait, but I confirmed that this fixes the problem by building on an ARM AMI using the instructions at https://github.com/certbot/certbot/blob/master/tools/snap/README.md#use-testing-and-development. If whoever reviews this would like an ARM machine to test on themselves, please let me know.
Fixes#7585
This PR removes the specific configuration to configure the test runner included in `setuptools` to use pytest, the deprecated parameters related to setuptools testing in `setup.py`, and update the packaging guide to use `python -m pytest` instead of `python setup.py test`.
The farm test `test_sdist.sh` is also updated to use directly pytest. This test is designed to reproduce the steps used by OS integrators when they package `certbot`, and ensure that we are not breaking something that will impact their work. We discussed with integrators from RHEL/CentOS and Debian, and they are fine with us testing sdist directly with pytest.
One execution of the `test_sdist.sh` farm test with the modifications made by this PR can be seen here: https://dev.azure.com/certbot/certbot/_build/results?buildId=2606&view=results
* Remove setuptools deprecated features about testing
* Updating packaging guide
* Add changelog entry
Fixes#7863.
Connect command is `sudo snap connect certbot-dns-dnsimple:certbot-metadata certbot:certbot-metadata`
Logs are `cat /var/snap/certbot-dns-dnsimple/current/debuglog`
Echos in hook are only printed to terminal when it exits 0; otherwise, check logs in `debuglog` mentioned above.
Manual tests include all iterations of connected, unconnected, installed for the first, second time, etc, with passing and failing version checks.
* Make dnsimple not update if certbot is too old
* create an interface to read cb version
* add missing newline
* fix syntax
* trying to figure out the consumer syntax
* trying to figure out the consumer syntax, again
* only check post first install
* valid setting name
* test for first install differently
* snapctl doesn't error if it fails I guess
* time to do some print debugging
* continue playing with syntax
* once again, fooled by bash int vs string comparisons!
* debugging
* if we use post and pre together we can do this
* is this how content interface syntax works
* it's a directory?
* more debug
* what's that error message again?
* try other syntax
* if it's not documented just guess at syntax
* actually, I think this is the syntax
* oops didn't set for new hook
* test passing information along connection
* interface attributes can only be set during the execution of prepare hooks
* just do it with main connection
* undo last few test changes
* Add some printing to make sure we understand what's going on
* create empty directory to bind to
* put mkdir in the correct part
* let's inspect the environment
* it can't run bash directly.
* perhaps only directories can be shared via the contente interface
* update name of folder
* echo to debug log to understand what's going on exactly. we have file access though!
* update grep for new file
* more printing
* echo to the debug log
* ok NOW all print statements are going to the log
* why does echo need two >s
* remove unnecessary extra check, just check if the init file is available
* check if certbot version will be available post-refresh after all
* pre-refresh hook is not necessary to get certbot version
* update mkdir so we don't have to clean each time
* try comparing version numbers in python
* it's python3
* we need different prints for if we succeed or if we fail.
* improve bash syntax
* remove some debugging code
* Remove debug script
* remove spaces for clarity
* consolidate parts and remove more test code
* s/certbot-version/certbot-metadata/g
* use sys.exit instead of exit
* find and save certbot version on the certbot side
* change presence test to new file
* switch to using packaging.version.parse instead of LooseVersion
* switch to requiring certbot version >= plugin version
* add plugin snap changes to generate script
* Add comment to generation file saying not to edit generated files manually
* Create post-refresh hook for all plugins with script
* generate files using new script
* update snapcraft.yaml files for plugins
* bin/sh comes first
* Add packaging to install_requires
* Check that refresh is allowed in integration test
* switch plug and slot names in integration test
* Update tools/generate_dnsplugins_postrefreshhook.sh
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* small bash fixes
* Update snap readme with new instructions
* Run tools/generate_dnsplugins_postrefreshhook.sh
* Update tools/snap/generate_dnsplugins_postrefreshhook.sh
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Snapcraft has a feature name `remote-build`. It allows to compile snaps using the Canonical dedicated build architecture for several architectures. Compared to the QEMU-enabled Docker approach used currently, the remote build has several advantages:
* the builds are done on the native architecture, making them basically faster than what can be achieved on QEMU
* it avoids to depend on `adferrand/snapcraft` (which could be otherwise be fixed with the merge of https://github.com/snapcore/snapcraft/pull/3144, but this will not happen in the short term)
* when everything is good, all snaps build can be run in parallel and then can be orchestrated by one single Azure Pipeline job, since the heavy tasks are done remotely.
This PR makes the necessary ajustements to use the remote build feature instead of the QEMU-enabled docker approach.
One complex task was to be able to compile the `certbot` snap on `arm64` and `armhf`. Indeed on these architectures the pre-compiled wheel for `cffi` is not available. So it needs to be compiled during the snap build. Sadly, the current version of the python plugin in snapcraft is limited by the fact that `wheels` is not installed in the virtual environment set up to build the python packages, and there is no easy way to change that except by overridding the whole build process.
In the long term, I think I will open a PR on `snapcraft` Git repository to provide a consistent solution. But for the short term, I used the possibility to provide arguments to the `venv` module, to add the flag `--system-site-packages`. With it, the virtual environment can use the system site package, where `wheel` is available.
The other significant additions are in `tools/snap/build_remote.py` script. If invoking the remote build on a local machine is quite straight-forward, it is another story on the CI because we need build auditability and resiliency during these non-interactive actions. In particular we should avoid as possible inconsistent results on the nightly pipeline and the release pipeline.
So this script wraps the `snapcraft` call into a retry logic, and improves its logs in the context of parallel builds.
For the minor modifications, it is mainly about ensuring that plugins can be built (some of them also need `cffi` for instance), and simplify the Azure Pipeline since all snaps are retrieved in one go.
Please note that the `test-` branches still run only the `amd64` architecture. Indeed I noticed that builds on `arm64` and `armhf` are tending to be very slow to start (up to 40 min) while the `amd64` ones wait at max 10 mins, and usually 30 seconds only when the overall load on Canonical side is low.
To work on `certbot/certbot` repository, one secured file needs to be added, because `snapcraft` needs to be authenticated against Launchpad with credentials allowing remote builds. To do so, from a local machine that have this capability, one can extract the existing file at `$HOME/.local/share/snapcraft/provider/launchpad/credentials`, and register it as a secured file in Azure Pipeline with the name `snapcraftRemoteBuildCredentials`.
* Define scripts
* Setup pipeline to use remote builds
* Focus on packaging builds
* Set credentials
* Setup git
* Launch all builds in parallel
* Add dev dependencies to build cffi and cryptography
* Convert to a python logic
* Reorganize the pipeline
* Handle the fact that snap builds may be taken from cache
* Generate constraints
* Exit code
* Check existence
* Try to handle better non zero exit code
* Add --system-site-packages to get wheel in the venv
* Add executable permissions
* Troubleshoot
* Dynamic display, take the maximum timeout for snap build job
* Allow retries if the remote build does not start
* Trigger only amd64 builds for test branches
* Exit properly
* Update snapcraft.yaml
* Fix snap run
* Set secured file name
* Update .azure-pipelines/templates/jobs/packaging-jobs.yml
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update .azure-pipelines/templates/jobs/packaging-jobs.yml
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Update .azure-pipelines/templates/jobs/packaging-jobs.yml
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Move order in deps
* Reactivate all builds
* Use Manager() as a context manager
* Use Pool as a context manager
* Some nice refactorings
* Check snapcraft execution interruption with exit codes
* Use f-string and format expressions
* Start log
* Consistent use of single/double quotes
* Better loop to extract lines
* Retry on build failures
* Few optimizations
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Fixes#8103.
* Update the DNS plugin generator script to core20 syntax
* Generate new snapcraft.yamls for the DNS plugins
* Update certbot.wrapper to search for python3.8 paths
According to `distutils/version.py`, StrictVersion is pretty strict in
what version numbers to accept:
> A version number consists of two or three dot-separated numeric
> components, with an optional "pre-release" tag on the end. The
> pre-release tag consists of the letter 'a' or 'b' followed by a number.
This assumption already fails for some pretty basic python libraries
itself, like setuptools, also available in `46.1.3.post20200610`, a
completely valid version number according to
https://www.python.org/dev/peps/pep-0440/#post-releases.
There doesn't seem to be a particular reason on why StrictVersion has
been used here, so let's use LooseVersion, to be compatible with these
versions.
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
