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
* Use greater than or equal to in requirements.
This changes the existing requirements using strictly greater than to greater
than or equal to so that they're more conventional.
* Use >= for certbot-postfix.
Despite it previously saying 'certbot>0.23.0', certbot-postfix/local-oldest-requirements.txt was pinned to 0.23.0 so let's just use certbot>=0.23.0.
This PR adds the functionality to enhance Apache configuration to include HTTP Strict Transport Security header with a low initial max-age value.
The max-age value will get increased on every (scheduled) run of certbot renew regardless of the certificate actually getting renewed, if the last increase took place longer than ten hours ago. The increase steps are visible in constants.AUTOHSTS_STEPS.
Upon the first actual renewal after reaching the maximum increase step, the max-age value will be made "permanent" and will get value of one year.
To achieve accurate VirtualHost discovery on subsequent runs, a comment with unique id string will be added to each enhanced VirtualHost.
* AutoHSTS code rebased on master
* Fixes to match the changes in master
* Make linter happy with metaclass registration
* Address small review comments
* Use new enhancement interfaces
* New style enhancement changes
* Do not allow --hsts and --auto-hsts simultaneuously
* MyPy annotation fixes and added test
* Change oldest requrements to point to local certbot core version
* Enable new style enhancements for run and install verbs
* Test refactor
* New test class for main.install tests
* Move a test to a correct test class
* Remove unneeded sys import.
Once upon a time we needed this in some of these setup.py files because we were
using sys in the file, but we aren't anymore so let's remove the import.
* use setuptools instead of distutils
* resolved mypy untyped defs in parser.py
* resolved mypy untyped defs in obj.py
* removed unused imports
* resolved mypy untyped defs in http_01.py
* resolved mypy untyped defs in tls_sni_01.py
* resolved mypy untyped defs in configurator.py
* address mypy too-many-arguments error in override_centos.py
* resolved mypy untyped defs in http_01_test.py
* removed unused 'conf' argument that was causing mypy method assignment error
* address mypy error where same variable reassigned to different type
* address pylint and coverage issues
* one character space change for formatting
* fix required acme version for certbot-apache
Fixes#5490.
There's a lot of possibilities discussed in #5490, but I'll try and explain what I actually did here as succinctly as I can. Unfortunately, there's a fair bit to explain. My goal was to break lockstep and give us tests to ensure the minimum specified versions are correct without taking the time now to refactor our whole test setup.
To handle specifying each package's minimum acme/certbot version, I added a requirements file to each package. This won't actually be included in the shipped package (because it's not in the MANIFEST).
After creating these files and modifying tools/pip_install.sh to use them, I created a separate tox env for most packages (I kept the DNS plugins together for convenience). The reason this is necessary is because we currently use a single environment for each plugin, but if we used this approach for these tests we'd hit issues due to different installed plugins requiring different versions of acme/certbot. There's a lot more discussion about this in #5490 if you're interested in this piece. I unfortunately wasted a lot of time trying to remove the boilerplate this approach causes in tox.ini, but to do this I think we need negations described at complex factor conditions which hasn't made it into a tox release yet.
The biggest missing piece here is how to make sure the oldest versions that are currently pinned to master get updated. Currently, they'll stay pinned that way without manual intervention and won't be properly testing the oldest version. I think we should solve this during the larger test/repo refactoring after the release because the tests are using the correct values now and I don't see a simple way around the problem.
Once this lands, I'm planning on updating the test-everything tests to do integration tests with the "oldest" versions here.
* break lockstep between packages
* Use per package requirements files
* add local oldest requirements files
* update tox.ini
* work with dev0 versions
* Install requirements in separate step.
* don't error when we don't have requirements
* install latest packages in editable mode
* Update .travis.yml
* Add reminder comments
* move dev to requirements
* request acme[dev]
* Update pip_install documentation
