This PR removes all zope dependencies from plugins configuration.
It also lets Sphinx upgrade to the next major version by removing the plugin dedicated to zope interfaces documentation. As a consequence, the deprecated zope interfaces are not documented anymore.
* Cleanup zope dependencies in plugins and upgrade sphinx
* Update pinnings
1.10.0 was a bad release and this breaks our oldest Boulder tests.
I bumped the version to 1.10.0 in #8852 to get access to a new public display_util API, but that was the release with the broken deprecation of `--manual-public-ip-logging-ok`. So let's bump it to 1.10.1.
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>
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
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
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
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>
This PR gets its root from an observation I did on current version of Certbot (1.3.0): the `renewal-hooks` directory in Certbot configuration directory is created on Windows with write permissions to everybody.
I thought it was a critical bug since this directory contains hooks that are executed by Certbot, and you certainly do not want this folder to be open to any malicious hook that could be inserted by everyone, then executed with administrator privileges by Certbot.
Turns out for this specific problem that the bug is not critical for the hooks, because the scripts are expected to be in subdirectories of `renewal-hooks` (namely `pre`, `post` and `deploy`), and these subdirectories have proper permissions because we set them explicitly when Certbot is starting.
Still, there is a divergence here between Linux and Windows: on Linux all Certbot directories without explicit permissions have at maximum `0o755` permissions by default, while on Windows it is a `0o777` equivalent. It is not an immediate security risk, but it is definitly error-prone, not expected, and so a potential breach in the future if we forget about it.
Root cause is that umask is not existing in Windows. Indeed under Linux the umask defines the default permissions when you create a file or a directory. Python takes that into account, with an API for `os.open` and `os.mkdir` that expose a `mode` parameter with default value of `0o777`. In practice it is never `0o777` (either you the the `mode` explictly or left the default one) because the effective mode is masked by the current umask value in the system: on Linux it is `0o022`, so files/directories have a maximum mode of `0o755` if you did not set the umask explicitly, and it is what it is observed for Certbot.
However on Windows, the `mode` value passed (and got from default) to the `open` and `mkdir` of `certbot.compat.filesystem` module is taken verbatim, since umask does not exit, and then is used to calculate the DACL of the newly created file/directory. So if the mode is not set explicitly, we end up with files and directories with `0o777` permissions.
This PR fixes this problem by implementing a umask behavior in the `certbot.compat.filesystem` module, that will be applied to any file or directory created by Certbot since we forbid to use the `os` module directly.
The implementation is quite straight-forward. For Linux the behavior is not changed. On Windows a `mask` parameter is added to the function that calculates the DACL, to be invoked appropriately when file or directory are created. The actual value of the mask is taken from an internal class of the `filesystem` module: its default value is `0o755` to match default umasks on Linux, and can be changed with the new method `umask` that have the same behavior than the original `os.umask`. Of course `os.umask` becomes a forbidden function and `filesystem.umask` must be used instead.
Existing code that is impacted have been updated, and new unit tests are created for this new function.
* Implement umask for Windows
* Set umask at the beginning of tests
* Fix lint, update local oldest requirements
* Update certbot-apache/setup.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Improve tests
* Adapt filesystem.makedirs for Windows
* Fix
* Update certbot-apache/setup.py
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* Changelog entries
* Fix lint
* Update certbot/CHANGELOG.md
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
Part of #7886.
This PR conditionally installs mock in `apache/setup.py` based on setuptools version and python version, when possible. It then updates `apache` tests to use `unittest.mock` when `mock` isn't available.
* Conditionally install mock in apache
* error out on newer python and older setuptools
* error when trying to build wheels with old setuptools
* use unittest.mock when third-party mock isn't available in apache, with no cover and type ignore
* Load apacheconfig dependency, gate behind flag
* Bump apacheconfig dependency to latest version and install dev version of apache for coverage tests
* Move augeasnode_test tests to more generic parsernode_test
* Revert "Move augeasnode_test tests to more generic parsernode_test"
This reverts commit 6bb986ef78.
* Mock AugeasNode into DualNode's place, and run augeasnode tests exclusively on AugeasNode
* Don't calculate coverage for skeleton functions
* clean up helper function in augeasnode_test
Part of #7550
This PR makes appropriate corrections to run pylint on Python 3.
Why not keeping the dependencies unchanged and just run pylint on Python 3?
Because the old version of pylint breaks horribly on Python 3 because of unsupported version of astroid.
Why updating pylint + astroid to the latest version ?
Because this version only fixes some internal errors occuring during the lint of Certbot code, and is also ready to run gracefully on Python 3.8.
Why upgrading mypy ?
Because the old version does not support the new version of astroid required to run pylint correctly.
Why not upgrading mypy to its latest version ?
Because this latest version includes a new typshed version, that adds a lot of new type definitions, and brings dozens of new errors on the Certbot codebase. I would like to fix that in a future PR.
That said so, the work has been to find the correct set of new dependency versions, then configure pylint for sane configuration errors in our situation, disable irrelevant lintings errors, then fixing (or ignoring for good reason) the remaining mypy errors.
I also made PyLint and MyPy checks run correctly on Windows.
* Start configuration
* Reconfigure travis
* Suspend a check specific to python 3. Start fixing code.
* Repair call_args
* Fix return + elif lints
* Reconfigure development to run mainly on python3
* Remove incompatible Python 3.4 jobs
* Suspend pylint in some assertions
* Remove pylint in dev
* Take first mypy that supports typed-ast>=1.4.0 to limit the migration path
* Various return + else lint errors
* Find a set of deps that is working with current mypy version
* Update local oldest requirements
* Remove all current pylint errors
* Rebuild letsencrypt-auto
* Update mypy to fix pylint with new astroid version, and fix mypy issues
* Explain type: ignore
* Reconfigure tox, fix none path
* Simplify pinning
* Remove useless directive
* Remove debugging code
* Remove continue
* Update requirements
* Disable unsubscriptable-object check
* Disable one check, enabling two more
* Plug certbot dev version for oldest requirements
* Remove useless disable directives
* Remove useless no-member disable
* Remove no-else-* checks. Use elif in symetric branches.
* Add back assertion
* Add new line
* Remove unused pylint disable
* Remove other pylint disable
Part of #5775. We don't use these docs anywhere, so delete them.
Removes:
- `certbot-apache/readthedocs.org.requirements.txt`
- `certbot-apache/docs/` folder
- docs include in `MANIFEST.in`
- docs dependencies in `setup.py`
Fixes#7368.
When updating the changelog, I replaced the line about running tests on Python 3.8 because I personally think that support for Python 3.8 is the most relevant information for our users/packagers about our changes in this area.
* List support for Python 3.8.
* Update changelog.
Fixes#7212
This PR forbid os.stat and os.fstat, and fix or provide alternatives to avoid its usage in certbot outside of certbot.compat.filesystem.
* Reimplement private key mode propagation
* Remove other os.stat
* Remove last call of os.stat in certbot package
* Forbid stat and fstat
* Implement mode comparison checks
* Add unit tests
* Update certbot/compat/filesystem.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update certbot/compat/filesystem.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Handle case where multiple ace concerns a given SID in has_min_permissions
* Add a new test scenario
* Add a simple test for has_same_ownership
* Fix name function
* Add a comment explaining an ACE structure
* Move a test in its dedicated class
* Improve a message error
* Calculate has_min_permission result using effective permission rights to be more generic.
* Change an exception message
* Add comments, avoid to skip a test.
* Update certbot/compat/filesystem.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
Fixes#7115
This PR creates a `realpath` method in `filesystem`, whose goal is to replace any call to `os.path.realpath` in Certbot. The reason is that `os.path.realpath` is broken on some versions of Python for Windows. See https://bugs.python.org/issue9949. The function created here works consistently across Linux and Windows.
As for the other forbidden functions in `os` module, our `certbot.compat.os` will raise an exception if its `path.realpath` function is invoked, and using the `os` module from Python is forbidden from the pylint check implemented in our CI.
Every call to `os.path.realpath` is corrected in `certbot` and `certbot-apache` modules.
* Forbid os.path.realpath
* Finish implementation
* Use filesystem.realpath
* Control symlink loops also for Linux
* Add a test for forbidden method
* Import a new object from os.path module
* Use same approach of wrapping than certbot.compat.os
* Correct errors
* Fix dependencies
* Make path module internal
This PR implements the filesystem.chmod method from #6497.
* Implement filesystem.chmod
* Conditionally add pywin32 on setuptools versions that support environment markers.
* Update apache plugin requirements
* Use a try/except import approach similar to lock
* Add comments about well-known SIDs
* Add main command
* Call filesystem.chmod in tests, remove one test
* Add test for os module
* Update environment marker
* Ensure we are not building wheels using an old version of setuptools
* Added a link to list of NTFS rights
* Simplify sid comparison
* Enable coverage
* Sometimes, double-quote is the solution
* Add entrypoint
* Add unit tests to filesystem
* Resolve recursively the link, add doc
* Move imports to the top of the file
* Remove string conversion of the ACL, fix setup
* Ensure admins have all permissions
* Simplify dacl comparison
* Conditionally raise for windows temporary workaround
* Add a test to check filesystem.chown is protected against symlink loops
This PR is the second part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)
This PR creates the module certbot.compat.os, that delegates everything to os, and that will be the safeguard against problematic methods of the standard module. On top of that, a quality check wrapper is called in the lint tox environment. This wrapper calls pylint and ensures that standard os module is no used directly in the certbot codebase.
Finally local oldest requirements are updated to ensure that tests will take the new logic when running.
* Add executable permissions
* Add the delegate certbot.compat.os module, add check coding style to enforce usage of certbot.compat.os instead of standard os
* Load certbot.compat.os instead of os
* Move existing compat test
* Update local oldest requirements
* Import sys
* Update account_test.py
* Update os.py
* Update os.py
* Update local oldest requirements
* Implement the new linter_plugin
* Fix local oldest for nginx
* Remove check coding style
* Update linter_plugin.py
* Add several comments
* Update the setup.py
* Add documentation
* Update acme dependencies
* Update certbot/compat/os.py
* Update docs/contributing.rst
* Update linter_plugin.py
* Handle os.path. Simplify checker.
* Add a comment to a reference implementation
* Update changelog
* Fix module registering
* Update docs/contributing.rst
* Update config and changelog
So merging the study from @bmw and me, here is what happened.
Each invocation of `certbot.logger.post_arg_parse_setup` create a file handler on `letsencrypt.log`. This function also set an atexit handler invoking `logger.shutdown()`, that have the effect to close all logger file handler not already closed at this point. This method is supposed to be called when a python process is close to exit, because it makes all logger unable to write new logs on any handler.
Before #6667 and this PR, for tests, the atexit handle would be triggered only at the end of the pytest process. It means that each test that launches `certbot.logger.post_arg_parse_setup` add a new file handler. These tests were typically connecting the file handler on a `letsencrypt.log` located in a temporary directory, and this directory and content was wipped out at each test tearDown. As a consequence, the file handles, not cleared from the logger, were accumulating in the logger, with all of them connected to a deleted file log, except the last one that was just created by the current test. Considering the number of tests concerned, there were ~300 file handler at the end of pytest execution.
One can see that, on prior #6667, by calling `print(logger.getLogger().handlers` on the `tearDown` of these tests, and see the array growing at each test execution.
Even if this represent a memory leak, this situation was not really a problem on Linux: because a file can be deleted before it is closed, it was only meaning that a given invocation of `logger.debug` for instance, during the tests, was written in 300 log files. The overhead is negligeable. On Windows however, the file handlers were failing because you cannot delete a file before it is closed.
It was one of the reason for #6667, that added a call to `logging.shutdown()` at each test tearDown, with the consequence to close all file handlers. At this point, Linux is not happy anymore. Any call to `logger.warn` will generate an error for each closed file handler. As a file handler is added for each test, the number of errors grows on each test, following an arithmetical suite divergence.
On `test_sdists.py`, that is using the bare setuptools test suite without output capturing, we can see the damages. The total output takes 216000 lines, and 23000 errors are generated. A decent machine can support this load, but a not a small AWS instance, that is crashing during the execution. Even with pytest, the captured output and the memory leak become so large that segfaults are generated.
On the current PR, the problem is solved, by resetting the file handlers array on the logging system on each test tearDown. So each fileHandler is properly closed, and removed from the stack. They do not participate anymore in the logging system, and can be garbage collected. Then we stay on always one file handler opened at any time, and tests can succeed on AWS instances.
For the record, here is all the places where the logging system is called and fail if there is still file handlers closed but not cleaned (extracted from the original huge output before correction):
```
Logged from file account.py, line 116
Logged from file account.py, line 178
Logged from file client.py, line 166
Logged from file client.py, line 295
Logged from file client.py, line 415
Logged from file client.py, line 422
Logged from file client.py, line 480
Logged from file client.py, line 503
Logged from file client.py, line 540
Logged from file client.py, line 601
Logged from file client.py, line 622
Logged from file client.py, line 750
Logged from file cli.py, line 220
Logged from file cli.py, line 226
Logged from file crypto_util.py, line 101
Logged from file crypto_util.py, line 127
Logged from file crypto_util.py, line 147
Logged from file crypto_util.py, line 261
Logged from file crypto_util.py, line 283
Logged from file crypto_util.py, line 307
Logged from file crypto_util.py, line 336
Logged from file disco.py, line 116
Logged from file disco.py, line 124
Logged from file disco.py, line 134
Logged from file disco.py, line 138
Logged from file disco.py, line 141
Logged from file dns_common_lexicon.py, line 45
Logged from file dns_common_lexicon.py, line 61
Logged from file dns_common_lexicon.py, line 67
Logged from file dns_common.py, line 316
Logged from file dns_common.py, line 64
Logged from file eff.py, line 60
Logged from file eff.py, line 73
Logged from file error_handler.py, line 105
Logged from file error_handler.py, line 110
Logged from file error_handler.py, line 87
Logged from file hooks.py, line 248
Logged from file main.py, line 1071
Logged from file main.py, line 1075
Logged from file main.py, line 1189
Logged from file ops.py, line 122
Logged from file ops.py, line 325
Logged from file ops.py, line 338
Logged from file reporter.py, line 55
Logged from file selection.py, line 110
Logged from file selection.py, line 118
Logged from file selection.py, line 123
Logged from file selection.py, line 176
Logged from file selection.py, line 231
Logged from file selection.py, line 310
Logged from file selection.py, line 66
Logged from file standalone.py, line 101
Logged from file standalone.py, line 88
Logged from file standalone.py, line 97
Logged from file standalone.py, line 98
Logged from file storage.py, line 52
Logged from file storage.py, line 59
Logged from file storage.py, line 75
Logged from file util.py, line 56
Logged from file webroot.py, line 165
Logged from file webroot.py, line 186
Logged from file webroot.py, line 187
Logged from file webroot.py, line 204
Logged from file webroot.py, line 223
Logged from file webroot.py, line 234
Logged from file webroot.py, line 235
Logged from file webroot.py, line 237
Logged from file webroot.py, line 91
```
* Reapply #6667
* Make setuptools delegates tests execution to pytest, like in acme module.
* Clean handlers at each tearDown to avoid memory leaks.
* Update changelog
