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
This PR is the first part of #6497 to ease the integration, following the new plan propose by @bmw here: #6497 (comment)
This step 1 refactor existing certbot.compat module into certbot.compat.misc, without any logic changed. Package certbot.compat will host the new modules that constitute the security model for Windows.
* Create the certbot.compat package. Move logic in certbot.compat.misc
* Add doc
* Fix lint
* Correct mypy
* Update client.py
This PR is a part of the tls-sni-01 removal plan described in #6849.
This PR removes --tls-sni-01-port, --tls-sni-01-address and tls-sni-01/tls-sni options from --preferred-challenges. They are replace by deprecation warning, indicating that these options will be removed soon.
This deprecation, instead of complete removal, is done to avoid certbot instances to hard fail if some automated scripts still use these flags for some users.
Once this PR lands, we can remove completely theses flags in one or two release.
* Remove tls-sni related flags in cli. Add a deprecation warning instead.
* Adapt tests to cli and renewal towards tls-sni flags deprecation
* Add https_port option. Make tls_sni_01_port show a deprecation warning, but silently modify https_port if set
* Migrate last items
* Fix lint
* Update certbot/cli.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Ensure to remove all occurences of tls-sni-01
* Remove unused parameter
* Revert modifications on cli-help.txt
* Use logger.warning instead of sys.stderr
* Update the logger warning message
* Remove standalone_supported_challenges option.
* Fix order of preferred-challenges
* Remove supported_challenges property
* Fix some tests
* Fix lint
* Fix tests
* Add a changelog
* Clean code, fix test
* Update CI
* Reload
* No hard date for tls-sni removal
* Remove useless cast to list
* Update certbot/tests/renewal_test.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Add entry to the changelog
* Add entry to the changelog
* Remove tls-sni from nginx config
* Add a dedicated configuration to define what is the HTTPS port for this certbot instance.
* Correct some tests
* Reestablish default vhost creation
* Clean tls references for nginx integration tests
* Associate https_port only to tests and nginx
This PR is a part of the tls-sni-01 removal plan described in #6849.
This PR removes the tls-sni-01 challenge tests during the integration tests. The approach I used here is not to remove completely the existing test code, but simply editing it to use a http-01 challenge. Indeed:
* the current integration tests are strongly coupled, and would require more modifications that it is worth, because ...
* the certbot-ci project, that has already no tls-sni tests, will soon replace completely the current integration tests code.
If you execute `tests/lock_test.py` or `tox -e integration` on a fairly recent machine, you will get the following error during tests executing against a live Nginx instance:
```
no "ssl_certificate" is defined in server listening on SSL port while SSL handshaking, client: x.x.x.x, server: y:y:y:y:z
```
Indeed, having no defined ssl certificate for a ssl port would inevitably lead to an error during the handshake SSL process between a client and this mis-configured nginx instance.
However it was not a problem one year before, because the handshake was not occurring in practice: the test just need to have a nginx started, and then immediately proceed to modify the configuration with a correct SSL setup. And nginx was able to start with a mis-configuration on SSL.
But then this fix has been done: https://trac.nginx.org/nginx/ticket/178
Basically with this, validation of the configuration is done during nginx startup, that will refuse to start with invalid configuration on SSL. Consequently, all related tests are failing with a sufficiently up-to-date nginx. For now, it is not seen on Travis because Ubuntu Trusty is used, with an old Nginx.
The PR fixes that, by generating on the fly self-signed certificates in the two impacted tests, and pushing the right parameters in the Nginx configuration.
* Fix nginx configuration with self-signed certificates generated on the fly
* Fix lint/mypy
* Fix old cryptography
* Unattended openssl
* Update lock_test.py
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
This PR fixes certbot-nginx and relevant tests to make them succeed on Windows.
Next step will be to enable integration tests through certbot-ci in a future PR.
* Fix tests and incompabilities in certbot-nginx for Windows
* Fix lint, fix oldest local dependencies
This PR reactivates tls-sni-01 challenges on recent Boulder versions checkout for integration tests. This allows to continue testing this challenge until it is officially dropped from server (Boulder) and client (Certbot).
Reverts #6679.
* Remove tls-sni-01 challenges in integration tests
* Remove the tls-sni test in the less invasive way
* Correct code coverage from tls-sni logic not been tested anymore.
* Update certbot-boulder-integration.sh
In response to #5342.
Currently, certbot will execute the operations necessary to validate a challenge even if the challenge has already been validated before against the acme ca server. This can occur for instance if a certificate is asked and issue correctly, then deleted locally, then asked again.
It is a corner case, but it will lead to some heavy operations (like updating a DNS zone, or creating an HTTP server) that are not needed.
This PR corrects this behavior by not executing challenges already validated, and use them directly instead to issue the certificate.
Fixes#5342
* Avoid to execute a given challenge that have been already validated by acme ca server.
* Execute tls challenge on a separate dns name, to avoid reusing the existing valid http challenge.
* Align with master
* Improve log
* Simplify the implementation
* Update changelog
* Add a unit test to ensure that validated challenges are not rerun
* flip challenge preference in Nginx
* Fix Nginx tests
* Flip challenge preference in Apache
* Flip challenge preference in standalone
* update changelog
* continue to run with tls-sni in integration tests for coverage
* feat(nginx): add and test new parsing abstractions
* chore(nginx parser): fix mypy and address small comments
* chore(nginx parser): clean up by removing context object
* fix integration test and lint
Stop caching the results of ipv6_info in http01.py. A call to choose_vhosts might change the ipv6 results of later calls. Add tests for this and default_listen_addresses more broadly.
Previously, Nginx did not allow `${` to start a variable name. Now it's allowed to. This means we'll be more permissible than Nginx when people are on older versions of Nginx, but it's unlikely anyone was relying on this to fail in the first place, so that's probably ok.
This makes errors more useful when Nginx can't be found or when Nginx's
configuration can't be found. Previously, a generic
`NoInstallationError` isn't descriptive enough to explain _what_ wasn't
installed or what failed without going digging into the source code.
* Exclude one-time use parameters. Fixes#6118
* Fix error.
* Delete items inplace, rather than creating new list.
* Fix stupid mistake.
* Use .index() for stability.
* Try previous idea while resetting the index.
* Shorter comment for pylint.
* More readable approach
* Fix whitespace
