After getting a +1 from everyone on the team, this PR removes the use of `codecov` from the Certbot repo because we keep having problems with it.
Two noteworthy things about this PR are:
1. I left the text at https://github.com/certbot/certbot/blob/4ea98d830bcc3d1b980a4055243c6a6a25d8dc54/.azure-pipelines/INSTALL.md#add-a-secret-variable-to-a-pipeline-like-codecov_token because I think it's useful to document how to set up a secret variable in general.
2. I'm not sure what the text "Option -e makes sure we fail fast and don't submit to codecov." in `tox.cover.py` refers to but it seems incorrect since `-e` isn't accepted or used by the script so I just deleted the line.
As part of this, I said I'd open an issue to track setting up coveralls (which seems to be the only real alternative to codecov) which is at https://github.com/certbot/certbot/issues/7810.
With my change, failure output looks something like:
```
$ tox -e py27-cover
...
Name Stmts Miss Cover Missing
------------------------------------------------------------------------------------------
certbot/certbot/__init__.py 1 0 100%
certbot/certbot/_internal/__init__.py 0 0 100%
certbot/certbot/_internal/account.py 191 4 98% 62-63, 206, 337
...
certbot/tests/storage_test.py 530 0 100%
certbot/tests/util_test.py 374 29 92% 211-213, 480-484, 489-499, 504-511, 545-547, 552-554
------------------------------------------------------------------------------------------
TOTAL 14451 647 96%
Command '['/path/to/certbot/dir/.tox/py27-cover/bin/python', '-m', 'coverage', 'report', '--fail-under', '100', '--include', 'certbot/*', '--show-missing']' returned non-zero exit status 2
Test coverage on certbot did not meet threshold of 100%.
ERROR: InvocationError for command /Users/bmw/Development/certbot/certbot/.tox/py27-cover/bin/python tox.cover.py (exited with code 1)
_________________________________________________________________________________________________________________________________________________________ summary _________________________________________________________________________________________________________________________________________________________
ERROR: py27-cover: commands failed
```
I printed the exception just so we're not throwing away information.
I think it's also possible we fail for a reason other than the threshold not meeting the percentage, but I've personally never seen this, `coverage report` output is not being captured so hopefully that would inform devs if something else is going on, and saying something like "Test coverage probably did not..." seems like overkill to me personally.
* remove codecov
* remove unused variable group
* remove codecov.yml
* Improve tox.cover.py failure output.
Related to https://github.com/certbot/certbot/pull/7482, this removes some references to deprecated options in Certbot.
The only references I didn't remove were:
* In `certbot/tests/testdata/sample-renewal*` which contains a lot of old values and I think there's even some value in keeping them so we know if we make a change that suddenly causes old renewal configuration files to error.
* In the Apache and Nginx plugins and I created https://github.com/certbot/certbot/issues/7508 to resolve that issue.
If you run `mypy --platform darwin certbot/certbot/util.py` you'll get:
```
certbot/certbot/util.py:303: error: Name 'distro' is not defined
certbot/certbot/util.py:319: error: Name 'distro' is not defined
certbot/certbot/util.py:369: error: Name 'distro' is not defined
```
This is because mypy's logic for handling platform specific code is pretty simple and can't figure out what we're doing with `_USE_DISTRO` here. See https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks for more info.
Setting `_USE_DISTRO` to the result of `sys.platform.startswith('linux')` solves the problem without changing the overall behavior of our code here though.
This fixes part of https://github.com/certbot/certbot/issues/7803, but there's more work to be done on Windows.
I don't fully understand why, but since I updated my macbook to macOS Catalina, the test script currently fails to run for me with the versions of our dependencies we have pinned. Updating the dependencies solves the problem though and you can see Travis also successfully running tests with these new dependencies at https://travis-ci.com/certbot/certbot/builds/150573696.
I want to do what I did in https://github.com/certbot/certbot/pull/7733 to our Azure Pipelines setup, but unfortunately this isn't currently possible. The only filters available for service hooks for the "build completed" trigger are the pipeline and build status. See
To accomplish this, I propose splitting the "advanced" pipeline into two cases. One is for builds on protected branches where we want to be notified if they fail while the other is just used to manually run tests on certain branches.
This fixes (part of) the problem identified in https://github.com/certbot/certbot/pull/7657#issuecomment-586506340.
When I tested our pylint setup on Python 3.5.9, 3.6.9, or 3.6.10, tests failed with:
```
************* Module acme.challenges
acme/acme/challenges.py:57:15: E1101: Instance of 'UnrecognizedChallenge' has no 'jobj' member (no-member)
************* Module acme.jws
acme/acme/jws.py:28:16: E1101: Class 'Signature' has no '_orig_slots' member (no-member)
```
These errors did not occur for me on Python 3.6.7 or Python 3.7+.
You also cannot run our lint setup on Python 2.7 because our pinned version of pylint's dependency `asteroid` does not support Python 2. Because of this, `pylint` is not installed in the virtual environment created by `tools/venv.py` and our [`lint` environment in tox specifies that Python 3 should be used](https://github.com/certbot/certbot/blob/fd64c8c33b2176e6569d64d30776bd5fc9fd3820/tox.ini#L132).
I tried updating pylint and its dependencies to fix the problem, but they still occur so I think adding back these disable checks on these lines again is the best fix for now.
These tests failed at https://travis-ci.com/certbot/certbot/jobs/285202481 but do not include any output from the script about what went wrong because the string created from `subprocess.CalledProcessError` does not include value of output.
This PR fixes that by printing these values which `pytest` will include in the output if the test fails.
After a brief discussion in Mattermost, I shut down letsencrypt.readthedocs.io. Turns out we were linking to it in our README here so let's remove the broken link.
I didn't update the link to point to one of the readthedocs projects we still have because are main Certbot docs are self-hosted rather than being on readthedocs.
Currently if you go to https://certbot.eff.org/docs/api/certbot.crypto_util.html, there is a todo comment displayed at the top of the page. These todos were written for developers, not users, so I do not think they should be shown from our documentation.
This PR makes the quick and easy fix of configuring Sphinx not to show these todo items. I created #7752 to track removing all of these todos from our docstrings and disabling the Sphinx todo extension.
* Set todo_include_todos=False in sphinx-quickstart
* Remove todos from existing docs.
When I want to manually run the full test suite to test something, I've been manually deleting our notification setup from `.travis.yml` to avoid spamming IRC with my personal test failures.
This PR sets this behavior up to happen automatically by turning off IRC notifications in test branches. You can see this working by noticing the IRC notification section in the bottom of the config for this PR at https://travis-ci.com/certbot/certbot/builds/146827907/config and the fact that it is absent from a `test-` branch based on this one at https://travis-ci.com/certbot/certbot/jobs/282059094/config.
It looks like we're currently documenting functions that are marked private (prefixed with an underscore) such as https://certbot.eff.org/docs/api/certbot.crypto_util.html#certbot.crypto_util._load_cert_or_req. I do not think we should do this because the functionality is private, should not be used, and including it in our docs just adds visual noise.
This PR stops us from documenting private code and fixes up `tools/sphinx-quickstart.sh` so we don't document it in future modules.
* Do not document private code.
* Don't document private members in the future.
A lot of Certbot's files don't have API documentation which is fixed by this PR. To do this, from the top level certbot directory I ran:
```
sphinx-apidoc -Me -o docs/api certbot
```
I then merged the resulting `modules.rst` file with `docs/api.rst`.
The old plugin at https://github.com/marcan/certbot-external says it's obsolete and points people to https://github.com/EnigmaBridge/certbot-external-auth. The new plugin is also an installer.
I also removed the reference to #2782 about us adding similar functionality since that's been done for a long time. We could reference our manual plugin instead, but I think that devalues their plugin a bit which I don't think is necessary or correct as it has different features.
This is my proposed fix for #7540. I would ideally like this to be included in our 1.0 release.
I came up with this design by adding all attributes used either in our own plugins, 3rd party plugins listed at https://certbot.eff.org/docs/using.html#third-party-plugins, or our public API code.
Despite me thinking that zope is unneeded nowadays, I initially tried to use it to define this interface since we have it and it gives us a way to define expected attributes, but it doesn't work because zope interface objects also have a method called `names` which conflict with the API.
I talked about this with Adrien out of band and did some of my own research and there are some minor benefits with this new approach of using properties:
1. It's more conventional.
2. If you also change the implementation to inherit from the class, Python will error if all properties aren't defined.
3. The PEP 526 style type annotations with mypy seem to (currently) only be used to validate code using the class, not the class implementation itself. You can add a type annotation saying the class needs to have this attribute, never define it, and mypy won't complain.
With this new approach, I had to fix `names` because pylint was complaining that the arguments differed, however, we never used the optional parameter to `names` outside of tests so I just deleted the code altogether.
* fixes#7540
* move to properties
Fixes#7184.
I updated #7358 to track the issue of unpinning all of these dependencies.
* pin back configargparse
* Pin back zope packages.
* update deps
* Add changelog entry.
* run build.py
When you try to run this script, it crashes with:
```
standard_init_linux.go:211: exec user process caused "exec format error"
```
This is caused by the script being written to have the contents:
```
\
#!/bin/sh
set -e
...
```
This fixes the problem by removing the slash and moving the shebang to the first line of the string.
In Travis, the full test suite doesn't run on PRs for point release branches, just on commits for them. I think this behavior makes sense because what we actually want to test before a point release is the exact commit we want to release after any squashing/merging has been done. This PR modifies Azure to match this behavior.
After this PR lands, I need to update the tests required to pass on GitHub.
I tried to finish up #7214 by removing the code in acme but we can't really do that until #7478 is resolved which we cannot do until we release 0.40.0.
Since we have to wait, this PR adds deprecation warnings for code that uses the TLS-SNI-01 code or was only used by the long deprecated TLS-SNI-01 code.
I'd like this PR to land before our next release.
* Deprecate more code related to TLS-SNI-01.
* Assert about warning message.
We don't package rebuild_dependencies.py so I don't think we need to mention changes to it in our changelog which is primarily read by users and packagers.
`certbot-compatibility-test` is using code in `acme` that I proposed making private and not trivially importable in https://github.com/certbot/certbot/issues/5775.
To fix it, I switched to using Certbot's test utilities which I proposed keeping public to help with writing tests for plugins. When doing this I had to change the name of the key because `rsa1024_key.pem` doesn't exist in Certbot.
I also deleted the keys in `certbot-compatibility-test`'s testdata because because they are unused.
This is a big part of #7214. It removes all references to TLS-SNI-01 outside of acme (and pytest.ini). Those changes will come in a subsequent PR. I thought this one was getting big enough.
* Remove references to TLS-SNI-01 in Apache plugin
* Remove references to TLS-SNI-01 from certbot-nginx
* Remove references to TLS-SNI from Certbot.
* Remove TLS-SNI reference from docs
* add certbot changelog
* Clarify test behavior
I wanted to polish the changelog a bit. Changes made are:
* We don't ship our test farm tests so including info about them in our changelog seems unnecessary.
* I combined and expanded the info about the deprecation of Python 3.4.
While working on #7214, I noticed that certbot.plugins.common.TLSSNI01 wasn't printing a deprecation warning and it was still being used in our Apache plugin. This PR fixes that.
Over the weekend, nightly tests on Windows failed for certbot-dns-google: https://dev.azure.com/certbot/web/build.aspx?pcguid=74ef9c03-9faf-405b-9d03-9acf8c43e8d6&builduri=vstfs%3a%2f%2f%2fBuild%2fBuild%2f72
The error occurred inside `oauth2client`'s locking code and the failure seems spurious as it did not reproduce this morning: https://dev.azure.com/certbot/certbot/_build/results?buildId=73
I could not find a relevant changelog entry in `oauth2client` saying they've fixed the problem, but the problematic code no longer exists in `oauth2client>=4.0`. This PR updates our minimum dependency required in an attempt to avoid spurious failures for us in the future. The only downside I am aware of is it'll make it harder for certbot-dns-google to be packaged in Debian Old Stable or Ubuntu 16.04, but I don't expect either of those things to happen anytime soon.
* bump oauth2client dep
* Update dev_constraints.txt.
* Add changelog entry for packagers.
When setting up Azure Pipelines, I didn't like having to define codecov_token for each pipeline. This works around it by using a shared variable group.
You can see this working successfully at https://dev.azure.com/certbot/certbot/_build/results?buildId=3.
* Use certbot-common.
* update instructions
The repo description for the [3rd party Icecast plugin](https://github.com/e00E/lets-encrypt-icecast) says that the plugin isn't currently working and the repository hasn't been updated since 2017. Since it seems broken and unmaintained, let's remove it from the list of third party plugins.
I would happily add it again to the list of third party plugins if people fix and maintain it.
The README for the [3rd party heroku plugin](https://github.com/gboudreau/certbot-heroku) says it has been deprecated. Because of this, let's remove it from the list of third party plugins.
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.
See https://community.letsencrypt.org/t/ssl-error-after-cert-renew/99430.
The first commit of this PR is a simple, clean revert of #7191. Subsequent commits add back pieces of that PR we want to keep.
I also reverted #7299 which landed in a separate PR, but needs to be reverted to keep including the TLS config files in the certbot-apache package when it is built.
I tested this on Ubuntu 18.04 by installing a cert to Apache using Certbot master and then running certbot renew with this branch. I watched the Apache plugin update the configuration file to remove SSLSessionTickets off.
* Revert "Disable TLS session tickets for Apache 2.4.11+ (#7191)"
This reverts commit 9174c631d9.
* Keep hashes with TLS session tickets disabled.
* dont delete changelog entries
* add changelog entry
* Revert "Clean the useless entries in MANIFEST.in (#7299)"
This reverts commit f4d17d9a6b.
(cherry picked from commit 120137eb8d)
See https://community.letsencrypt.org/t/ssl-error-after-cert-renew/99430.
The first commit of this PR is a simple, clean revert of #7191. Subsequent commits add back pieces of that PR we want to keep.
I also reverted #7299 which landed in a separate PR, but needs to be reverted to keep including the TLS config files in the certbot-apache package when it is built.
I tested this on Ubuntu 18.04 by installing a cert to Apache using Certbot master and then running certbot renew with this branch. I watched the Apache plugin update the configuration file to remove SSLSessionTickets off.
* Revert "Disable TLS session tickets for Apache 2.4.11+ (#7191)"
This reverts commit 9174c631d9.
* Keep hashes with TLS session tickets disabled.
* dont delete changelog entries
* add changelog entry
* Revert "Clean the useless entries in MANIFEST.in (#7299)"
This reverts commit f4d17d9a6b.
