Fixes#8427
This PR converts the Python 2 types hints into Python 3 types annotations. I have used the project https://github.com/ilevkivskyi/com2ann which has been designed for that specific purpose and did that very well.
The only remaining things to do were to fix broken type hints that became wrong code after migration, and to fix lines too long with the new syntax.
* Raw execution of com2ann
* Fixing broken type annotations
* Cleanup imports
Fixes https://github.com/certbot/certbot/issues/8162.
I had to update the base of the Dockerfile to get a new enough version of Python 3. I also simplified things a lot and removed a lot of the comments that were essentially just describing how Dockerfiles work.
The most complicated changes here are in `testdata`. You can find a diff of the changes to `nginx.tar.gz` at https://gist.github.com/c7727db0cecf3f15f02439f085c73848.
The first problem was that there were some complaints from the new Apache/nginx/OpenSSL version about the 1024 bit RSA key so I updated `empty_cert.pem` both inside and outside of the tarball as well as the corresponding private key in the tarball to use a 2048 bit key.
The 2nd problem is trickier to understand. If you look at the output from nginx after loading the config from `lots/` you'll see it complaining about conflicting `server_name` directives for the directives I deleted. See https://dev.azure.com/certbot/certbot/_build/results?buildId=2578&view=logs&j=250aa146-b243-5f8f-bf86-17a529c9fb7e&t=9baa2014-9673-5e78-8f4f-7a463caf2bfa&l=1516.
After switching the tests to Python 3, tests on that domain started failing. What I believe to be happening is we were just lucky these tests were passing to begin with. In both the Apache and Nginx plugin, if there are conflicting virtual hosts like this, we just arbitrarily pick one. The relevant code here for nginx is 575092d603/certbot-nginx/certbot_nginx/_internal/configurator.py (L455)
I played around with a debugger and confirmed that before I removed the conflicting server names, there were two exact matches for the domain we were searching for here.
I think all that's going on is with the switch to Python 3, the vhost we happen to choose changes and "breaks" the test. I suspect this to be due to something like getting values out of a dict somewhere where the order of items in a dict while iterating over it is different between Python 2 and 3. I didn't track where this difference happens down, but I personally don't think it's a good use of time since I think the real problem here is that the nginx config being tested was invalid with conflicting `server` blocks.
I removed all references to the `server_name` causing conflicts in that nginx configuration because both server blocks had other domains that are being tested, but I could add either back if you prefer. You can see the `nginx_compat` test passing with these changes at https://dev.azure.com/certbot/certbot/_build/results?buildId=2587&view=logs&j=250aa146-b243-5f8f-bf86-17a529c9fb7e.
* update Dockerfile
* Fix apache_compat on py3.
* Update empty_cert.pem.
The command used here was `openssl req -key
certbot/certbot/tests/testdata/rsa2048_key.pem -new -subj '/CN=example.com'
-x509 >
certbot-compatibility-test/certbot_compatibility_test/testdata/empty_cert.pem`.
* update nginx.tar.gz
* Remove conflicting server_names
* Fix dangerous default argument
* Remove unused imports
* Remove unnecessary comprehension
* Use literal syntax to create data structure
* Use literal syntax instead of function calls to create data structure
Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
As pylint is evolving, it improves its accuracy, and several pylint error suppression (`# pylint: disable=ERROR) added in certbot codebase months or years ago are not needed anymore to make it happy.
There is a (disabled by default) pylint error to detect the useless suppressions (pylint-ception: `useless-suppression`). It is not working perfectly (it has also false-positives ...) but it is a good start to clean the codebase.
This PR removes several of these useless suppressions as detected by the current pylint version we use.
* Remove useless suppress
* Remove useless lines
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
* Reconfigure compatibility tests to use http challenge
* Correct simple test
* Add a fake DNS resolution for HTTP simple_verify
* Debug
* More subtle approach: we monkey patch urllib3 to fake a dns resolution to the target IP, allowing every host header to be preserved.
* Private package
* Relaxed permissions on certbot temp working dir
* Move the fake DNS logic in compatibility test, to avoid degrading the acme coverage
* Fix lint
* Update certbot-compatibility-test/certbot_compatibility_test/configurators/common.py
Co-Authored-By: adferrand <adferrand@users.noreply.github.com>
* Remove unsupported pylint disable options
* star-args removed in Pylint 1.4.3
* abstract-class-little-used removed in Pylint 1.4.3
* Fixes new lint errors
* Copy dummy-variable-rgx expression to new ignored-argument-names expression to ignore unused funtion arguments
* Notable changes
* Refactor to satisfy Pylint no-else-return warning
* Fix Pylint inconsistent-return-statements warning
* Refactor to satisfy consider-iterating-dictionary
* Remove methods with only super call to satisfy useless-super-delegation
* Refactor too-many-nested-statements where possible
* Suppress type checked errors where member is dynamically added (notably derived from josepy.JSONObjectWithFields)
* Remove None default of func parameter for ExitHandler and ErrorHandler
Resolves#5973
* Highlight failures more with asterisks
* Filter out wildcard names from all_names
* Only test -ai, not -aie (no redirects)
* Modified versions of almost all of 79 configs corpus
* Re-enable now-working stanza with 301 redirect
* Change another redirect to go to :443