Implement a sunset mechanism in certbot-auto for systems not supported anymore#7587
Merged
adferrand merged 34 commits intocertbot:masterfrom Jan 8, 2020
Merged
Implement a sunset mechanism in certbot-auto for systems not supported anymore#7587adferrand merged 34 commits intocertbot:masterfrom
adferrand merged 34 commits intocertbot:masterfrom
Conversation
5d2d35f to
795137f
Compare
bmw
reviewed
Dec 4, 2019
Member
bmw
left a comment
There was a problem hiding this comment.
This isn't even close to a full review, but here's how far I got today.
|
|
||
| RPM_DIST_NAME=`(. /etc/os-release 2> /dev/null && echo $ID) || echo "unknown"` | ||
|
|
||
| if [ "$PYVER" -eq 26 -a "$RPM_DIST_NAME" != "rhel" -a $(arch) != 'x86_64' ]; then |
Member
There was a problem hiding this comment.
arch isn't part of the POSIX standard: https://pubs.opengroup.org/onlinepubs/9699919799/idx/utilities.html
Can we be confident it is always available?
Also, I suggested that SCL packages seem to be available for 32 bit RHEL 6 but were you able to verify this? I tried to spin up a 32 bit RHEL 6 image to play with and couldn't make SCL work. Not sure if that's because it's not available or I did something wrong.
If you haven't, I propose just removing that condition and also deprecating support for 32-bit RHEL 6. It's simpler, is kind of nice anyway because of cryptography's OpenSSL deprecations affecting 32-bit users, and the stats I pulled on 32-bit certbot-auto users a while ago didn't list any on RHEL 6: https://gist.github.com/bmw/5b47802666212e9c368ec7370b0cf445
Collaborator
Author
There was a problem hiding this comment.
I honestly could not find any way to get a 32 bit version of scl on RHEL and affiliates. I it is available, it is confidential. So I am quite OK to include RHEL in the deprecation.
For arch, well, uname -m will do the job!
Member
There was a problem hiding this comment.
Nice. uname -m is indeed in the POSIX standard. Is its output reliable though, at least on RHEL 6 based systems? I tried to find something stating possible outputs but couldn't find anything.
Collaborator
Author
There was a problem hiding this comment.
I got x86_64 for my machine for any system I could think of. All code source dumps I could find are stating x86_64 for 64 bits, and ix86 (with x=3,4,5,6) for 32 bits.
For a standard POSIX tool, I expect this behavior to be extremely consistent.
Member
There was a problem hiding this comment.
I don't love this. Everything I've seen about uname says its output is implementation defined and looking at the source code it seems its output is completely controlled by the kernel where I was unable to find any guarantees about its output across systems/versions.
With that said, I'm not aware of any better options. I considered using Python (which I think we can rely on being there because its used by yum), but we can't just check if the machine is 64-bit because RHEL 6 supports other 64-bit architectures and the other options I'm aware of all go back to uname.
I think this is probably fine, but I don't like uncertainty like this especially when dealing with certbot-auto.
Collaborator
Author
There was a problem hiding this comment.
Me neither, in term of solution. But I never saw anything except x86_64 for 64 bits on kernel architectures.
bmw
requested changes
Dec 4, 2019
Member
bmw
left a comment
There was a problem hiding this comment.
I didn't manually play with this yet, but in addition to my inline comments, I think we should add tests for this. I think there are a few ways we could do this so let me know if you want ideas.
| } | ||
|
|
||
| if [ "$1" = "--le-auto-phase2" ]; then | ||
| if [ "$1" = "--le-auto-phase2" -a "$DEPRECATED_OS" = 1 ]; then |
Member
There was a problem hiding this comment.
nit: What do you think about moving this as a case of the existing phase 2 logic? I personally find it a little simpler to understand than adding a 3rd high level case and it deduplicates some code.
If we do this, at the end of the DEPRECATED_OS = 1 case, we'll need to call exit.
| # In this situation, we bypass any bootstrap or certbot venv setup. | ||
| shift 1 | ||
|
|
||
| error "Your system is not supported by certbot-auto anymore, and so certbot cannot be installed or upgraded." |
Member
There was a problem hiding this comment.
What do you think about making the rest of the code in this case look something like:
error "Your system is no longer supported by certbot-auto."
if [ -f "$VENV_BIN/letsencrypt" ]; then
error "Certbot will no longer receive updates."
error "Please visit https://certbot.eff.org/ to check for other alternatives."
"$VENV_BIN/letsencrypt" "$@"
exit 0
else
error "Certbot cannot be installed."
error "Please visit https://certbot.eff.org/ to check for other alternatives."
exit 1
fi
Collaborator
Author
There was a problem hiding this comment.
Ah. With your previous comment, I was hesitating on the exit code value depending on the situation (certbot installed or not). Yes, it makes sense, and I add that.
| shift 1 | ||
|
|
||
| error "Your system is not supported by certbot-auto anymore, and so certbot cannot be installed or upgraded." | ||
| error "Please consult official Certbot site (https://certbot.eff.org/) to check other alternatives." |
Member
There was a problem hiding this comment.
If we point people here (which I think we should), I think we need to make sure we have something to tell these users.
Given the complexity of getting a new version of Python and OpenSSL and how few users are affected, I think we should probably just add a warning to the top of the CentOS and RHEL 6 instructions saying that only x86_64 systems are supported.
Collaborator
Author
There was a problem hiding this comment.
You mean, in the website?
Member
There was a problem hiding this comment.
Yeah. I created certbot/website#512 and #7632 to track this and I think we should get that PR ready before releasing this code.
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
Collaborator
Author
|
@bmw, I am very curious about how to trick |
Collaborator
Author
|
Oh, I have an idea! An alias to uname, set in a subshell like for venv environment variables setup! |
Collaborator
Author
No, not working, aliases are not transfered to child scripts... |
Collaborator
Author
|
I added tests to check the sunset mechanism. Full tests suite is running at https://travis-ci.com/certbot/certbot/builds/139610387 |
bmw
requested changes
Dec 6, 2019
Member
bmw
left a comment
There was a problem hiding this comment.
I have a few small suggestions.
I'm planning on taking the time to a bunch of additional testing including a mock release once the code LGTM.
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
bmw
previously requested changes
Dec 10, 2019
|
|
||
| if [ "$INSTALL_ONLY" = 1 ]; then | ||
| # If you're modifying any of the code after this point in this `if` block, you may | ||
| # need to update the "$DEPRECATED_OS" = 1 case at the beginning of phase 2 as well. |
Member
There was a problem hiding this comment.
I should have been more clear in my initial comment, but I was actually imagining we move this comment up a level and put it before the if [ "$INSTALL_ONLY" = 1 ]; then line. If we change anything after this point such as how we invoke Certbot, we probably need to update the deprecation case as well.
Collaborator
Author
There was a problem hiding this comment.
Indeed, it can also impact the non install-only mode.
| echo "On CentOS 32 bits, certbot-auto upgraded certbot." | ||
| exit 1 | ||
| fi | ||
|
|
Member
There was a problem hiding this comment.
Can we add the test for the --install-only case which I suggested at #7587 (comment)?
bmw
reviewed
Dec 10, 2019
bmw
reviewed
Dec 10, 2019
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
2b7c4e7 to
933f9d2
Compare
933f9d2 to
ab2b0cb
Compare
Collaborator
Author
|
During the development of this PR, I used the following pattern several times: #!/bin/bash
# Protect against environment variable modifications
(
export TEST="test"
if [ "foo" != "bar" ]; then
echo "Error!"
exit 1
fi
echo "Success"
)This pattern was already present in the test script, so it made sense to reuse it to enrich the tests here. Its intention is to make ephemeral modifications thanks to a subshell, execute some logic in it, and fails the script using a non zero exit code if the logic does not fullfill some expectation. However this pattern is not working as expected: invoking In practice it was not a problem until now for the test logic itself, because a failed assertion was equivalent to a missing requirement for the success of any following assertions. But with my modifications it is not the case (we test several parallel and independent situations) and nonetheless it is not a clean approach. To fix that, I did not put |
Collaborator
Author
|
Full test suite passing here: https://travis-ci.com/certbot/certbot/builds/142492386 |
Collaborator
Author
|
Relevant tests from the full tests suite are running here: https://travis-ci.com/certbot/certbot/builds/142931508 |
bmw
previously requested changes
Jan 2, 2020
Member
bmw
left a comment
There was a problem hiding this comment.
I have a few very small nitpicks but the code otherwise LGTM. After these changes are made and merge conflicts are resolved, we should run the full test suite again and I'm going to do a little more manual testing.
| echo "On CentOS 6 32 bits, certbot-auto failed to run installed certbot instance." | ||
| exit 1 | ||
| fi | ||
| if ! "$LE_AUTO" --version 2>&1 | grep -q "Certbot will no longer receive updates."; then |
Member
There was a problem hiding this comment.
nit: Instead of the extra call to certbot-auto above to test that it doesn't error here and below, what about something like:
set -o pipefail
if ! "$LE_AUTO" --version 2>&1 | grep -q "Certbot will no longer receive updates."; then
echo "On CentOS 6 32 bits, certbot-auto failed or upgraded installed certbot instance."
exit 1
fi
set +o pipefail
I think this is very unlikely to matter, but I think this is nice to help make sure the first run where we're not checking for "Certbot will no longer receive updates." doesn't do something like upgrade Certbot and not print this message.
Alternatively, you could flip the order of the calls to certbot-auto, but I personally like using pipefail better.
Collaborator
Author
There was a problem hiding this comment.
Indeed, it is better to test everything together. I used the set -o pipefail approach.
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
# Conflicts: # certbot/CHANGELOG.md
Collaborator
Author
|
Full tests suite passing here: https://travis-ci.com/certbot/certbot/builds/142976996 |
bmw
reviewed
Jan 3, 2020
Member
bmw
left a comment
There was a problem hiding this comment.
This LGTM!
I did some manual testing reusing the old venv and a mock release running test farm tests which passed.
Contributor
|
Did you change this back after making this comment? #7587 (comment) |
Member
Yes he did. Sorry. To speed things up, Adrien and I talked about this synchronously during our 1-1. I thought using |
ohemorange
approved these changes
Jan 8, 2020
Contributor
ohemorange
left a comment
There was a problem hiding this comment.
In this review, I focused on the following questions that I thought were most important to double-check:
- Could this break certbot-auto on non-centos systems?
- Does this work as expected if we have trouble with the python version available?
- Are we accidentally running any setup that we don't expect to be doing on a deprecated system, for both phases?
For all of these questions, this code looks as correct as possible ("as possible" because of the uname discussion), so lgtm!
adferrand
added a commit
to adferrand/certbot
that referenced
this pull request
Jan 8, 2020
Member
|
While reviewing #7519, I noticed this slightly awkward behavior on 32-bit CentOS 6: This will happen on any deprecated system when My initial reaction is this isn't worth fixing though. |
adferrand
added a commit
that referenced
this pull request
Jan 13, 2020
…tOS 6 users (#7519) * Revert "Add back Python 3.4 support (#7510)" This reverts commit 9b848b1. * Fix certbot-auto * Use a more consistent way to enable rh-python36 * Avoid to call CompareVersions unecessarily * Control rh-python36 exit code * Fix travis config * Remove vscode config * Ignore vscode * Fix merge conflicts regarding #7587 (#70) * Add changelog entry * Finish sentence * Update certbot/CHANGELOG.md Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update letsencrypt-auto-source/tests/centos6_tests.sh Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update letsencrypt-auto-source/tests/centos6_tests.sh Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update letsencrypt-auto-source/tests/centos6_tests.sh Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update letsencrypt-auto-source/tests/centos6_tests.sh Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update letsencrypt-auto-source/tests/centos6_tests.sh Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com> * Update comments * Improve warning message * Update changelog Co-authored-by: Joona Hoikkala <joohoi@users.noreply.github.com>
Tomoyuki-GH
added a commit
to Tomoyuki-GH/certbot
that referenced
this pull request
Apr 3, 2020
* Use dummy values for ancestor (#7462)
* [Apache v2] AugeasBlockNode find_comments() implementation (#7457)
* find_comments implementation and AugeasCommentNode creation
* Use dummy value for ancestor
* Add NotImplementedError when calling find_comments with exact parameter
* Remove parameter 'exact' from find_comments interface
* Fix comment
* Remove references to TLS-SNI-01 outside of ACME (#7479)
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
* Fix invalid escape sequence \. rebuild_dependencies.py (#7486)
Signed-off-by: Mickaël Schoentgen <contact@tiger-222.fr>
* Don't use dev version of 3.8. (#7485)
Now that Python 3.8 is out, we don't need to use the development version.
* Don't use acme.test_util outside of acme. (#7484)
`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.
* Use distro library for all OS version detection (#7467)
This pull request ensures that we use distro package in all the distribution version detection. It also replaces the custom systemd /etc/os-release parsing and adds a few version fingerprints to Apache override selection.
Fixes: #7405
* Revert "Try to use platform.linux_distribution() before distro equivalent (#7403)"
This reverts commit ca3077d0347aae12163a43bf74a0c8321284367e.
* Use distro for all os detection code
* Address review comments
* Add changelog entry
* Added tests
* Fix tests to return a consistent os name
* Do not crash on non-linux systems
* Minor fixes to distro compatibility checks
* Make the tests OS independent
* Update certbot/util.py
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Skip linux specific tests on other platforms
* Test fixes
* Better test state handling
* Lower the coverage target for Windows tests
* Remove changelog entry about unpackaged scripts. (#7490)
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.
* Deprecate more code related to TLS-SNI-01 (#7483)
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.
* Describe distributed Certbot components. (#7493)
* Clarify when the changelog should be modified (#7491)
* Dropped deprecated flags from commands (#7482)
This pull request addresses #7451 by removing the deprecated flags.
* Dropped deprecated flags from commands
* Updated changelog for dropped flags and deleted outdated tests
* removed init-script part of apache test
* Use fresh authorizations in dry runs (#7442)
* acme: re-populate uri in deactivate_authorization
* Use fresh authorizations in dry runs
--dry-run now deactivates 'valid' authorizations if it encounters them
when creating a new order.
Resolves #5116.
* remove unused code
* typo in local-oldest-requirements
* better error handling
* certbot-ci: AUTHREUSE to 100 + unskip dry-run test
* improve test coverage for error cases
* restore newline to local-oldest-requirements.txt
* Build Windows installers with pinned dependencies (#7498)
* Consume constraints file
* Independent pywin32 dependency definition in setup.py and construct.py
* Don't use --agree-dev-preview in tests. (#7501)
* Update changelog for 0.40.0 release
* Release 0.40.0
* Add contents to CHANGELOG.md for next version
* Bump version to 0.41.0
* Add back Python 3.4 support (#7510)
* Revert "Deprecation warnings for Python 3.4 (#7378)"
This reverts commit 6fcdfb0e5006be85500fad67a5a67b47befedb2a.
* Revert "Migrate certbot-auto users on CentOS 6 to Python 3.6 (#7268)"
This reverts commit e19b2e04c75b6df4e3f8a455700aa95fca79bcc3.
* add changelog entry
* keep mona in authors
* Add back Python 3.4 support (#7510) (#7511)
* Revert "Deprecation warnings for Python 3.4 (#7378)"
This reverts commit 6fcdfb0e5006be85500fad67a5a67b47befedb2a.
* Revert "Migrate certbot-auto users on CentOS 6 to Python 3.6 (#7268)"
This reverts commit e19b2e04c75b6df4e3f8a455700aa95fca79bcc3.
* add changelog entry
* keep mona in authors
(cherry picked from commit 9b848b1d65783000a13ef3f94ac5fe0e8c3879e7)
* Update changelog for 0.40.1 release
* Release 0.40.1
* Add contents to CHANGELOG.md for next version
* Bump version to 1.0.0
* Pin all build dependencies for the Windows installer (#7504)
This PR uses pipstrap to bootstrap the venv used to build Windows installers. This effectively pin all build dependencies, since pynsist is already installed through pip_install.py script.
* Use pipstrap
* Pin also NSIS version
* [Apache v2] Implement set_parameters() (#7461)
* find_comments implementation and AugeasCommentNode creation
* set_parameters implementation
* Change parameters to a property
* Remove parameters property setter
* More pythonic iteration handling
* Match our Travis logic in Azure. (#7514)
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.
* Deprecate config_changes (#7469)
Closes #7454
* Deprecate config_changes
* Error on config_changes
* Fix tests for main.py
* Fix CHANGELOG entry
* Remove remnants of config_changes
* Fix CHANGELOG and add removed functions
* dns-rfc2136: use TCP to query SOA records (#7503)
* Use tcp query on dns-rfc2136 plugin
To improve network robust; fixes #7502.
* Update CHANGELOG.md
* Fix dns-rfc2136 test cases
* Add UDP fallback to dns-rfc2136
* Remove tls sni common (#7527)
* fixes #7478
* add changelog entry
* remove get_systemd_os_info (#7526)
Fixes #7500.
* Make uncomplicated modules private (#7528)
* Create _internal package for Certbot's non-public modules
* Move account.py to _internal
* Move auth_handler.py to _internal
* Move cert_manager.py to _internal
* Move client.py to _internal
* Move error_handler.py to _internal
* Move lock.py to _internal
* Move main.py to _internal
* Move notify.py to _internal
* Move ocsp.py to _internal
* Move renewal.py to _internal
* Move reporter.py to _internal
* Move storage.py to _internal
* Move updater.py to _internal
* update apache and nginx oldest requirements
* Keep the lock file as certbot.lock
* nginx oldest tests still need to rely on newer certbot
* python doesn't have good dependency resolution, so specify the transitive dependency
* update required minimum versions in nginx setup.py
* Move log.py to _internal (#7531)
Part of #5775. Methodology similar to #7528, but slightly more manual.
* Move items in certbot/display to _internal (#7532)
* Move display/completer.py to _internal/
* Move display/dummy_readline.py to _internal/
* Move display/enhancements.py to _internal/
* Create __init__.py in _internal/display
* Move eff.py to _internal (#7530)
* Move eff.py to _internal
* missed a few certbot.effs in tests
* remove sublime autocompletion
* fix messy scripting
* [Apache v2] Adding nodes 1/3 : add_child_block() (#7497)
* Implement add_child_block()
* Add comments and example
* Check augas path inconsistencies in initialization
* Remove TLS-SNI objects in ACME (#7535)
* fixes #7214
* update changelog
* remove unused import
* Move items in certbot/plugins to _internal (#7533)
* Create and initialize _internal/plugins
* Move plugins/manual.py to _internal/
* Move plugins/disco.py to _internal/
* Move plugins/selection.py to _internal/
* Move plugins/webroot.py to _internal/
* Move plugins/null.py to _internal/
* Move plugins/standalone.py to _internal/
* add missed internalization
* shorten line
* Update outdated init comment
* Move constants.py to _internal (#7534)
* Don't call core constants from nginx plugin
* Move constants.py to _internal/
* Move ENHANCEMENTS from now-internal constants to public plugins.enhancements
* Update display.enhancements.ask from its 2015 comment
* fix docstring
* Remove python2 and certbot-auto references in how to set up a Certbot build environment. (#7549)
Fixes #7548.
This PR udpdates installation instructions to get rid of python2 and certbot-auto in the how-to explaining the Certbot development environment setup.
Instead, Python 3 is used, and appropriate instructions for APT and RPM based distributions are provided.
* Implement add_child_directive (#7517)
* [Apache v2] Implement save() and unsaved_files() (#7520)
* Implement save() and unsaved_files()
* Linter fix
* [Apache v2] Implement delete_child() (#7521)
* Implement delete_child
* Fix linter
* [Windows] Fix certbot renew task failure under NT AUTHORITY\SYSTEM account (#7536)
Turned out that the scheduled task that runs `certbot renew` twice a day, is failing. Without any kind of log of course, otherwise it would not be fun.
It can be revealed by opening a powershell under the `NT AUTHORITY\SYSTEM` account, under which the scheduled task is run. Under theses circumstances, the bug is revealed: Certbot breaks when trying to invoke `certbot.compat.filesystem._get_current_user()`. Indeed the logic there implied to call `win32api.GetUserNameEx(win32api.NameSamCompatible)` and this function does not return always a useful value.
For normal account, it will be typically `DOMAIN_OR_MACHINE_NAME\YOUR_USER_NAME` (e.g. `My Machine\Adrien Ferrand`). But for the account `NT AUTHORITY\SYSTEM`, it will return `MACHINE_NAME\DOMAIN$`, which is a nonsense and makes fail the resolution of the actual SID of the account at the end of `_get_current_user()`.
This PR fixes this behavior by using an explicit construction of the account name that works both for normal users and `SYSTEM`.
* Use a different way to resolve current user account, that works both for normal users and SYSTEM.
* Add a comment to run Certbot under NT AUTHORITY\SYSTEM
* [Windows] Avoid letsencrypt.log permissions error during scheduled certbot renew task (#7537)
While coding for #7536, I ran into another issue. It appears that Certbot logs generated during the scheduled task execution have wrong permissions that make them almost unusable: they do not have an owner, and their ACL contains nonsense values (non existant accounts name).
The class `logging.handler.RotatingFileHandler` is responsible for these logs, and become mad when it is in a Python process run under a scheduled task owned by `SYSTEM`. This is precisely our case here.
This PR avoids (but not fix) the issue, by changing the owner of the scheduled task from `SYSTEM` to the `Administrators` group, that appears to work fine.
* Use Administrators group instead of SYSTEM to run the certbot renew task
* Move configuration.py to _internal (#7542)
Part of #5775. Methodology similar to #7528. Also refactors NGINX test util to use certbot.tests.util.ConfigTestCase.
* refactor nginx tests to no longer rely on certbot.configuration internals
* Move configuration.py to _internal
* Deprecate certbot register --update-registration (#7556)
Closes #7452.
* Fix shebang in rebuild_deps (#7557)
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.
* Internalize modules called by internal plugins (#7543)
* Move hooks.py to _internal
* Move cli.py to _internal
* Update pinned dependencies (#7558)
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
* fixes #7553 (#7560)
* [Apache v2] Initial ApacheParser skeleton (#7559)
* Fix metadata & primary references in Augeas tests.
When performing actions only on one of the trees in DualNodeParser, the two
trees get out-of-sync. Similarly, we can't expect that the metadata between
the two trees will remain the same.
Did a pass over the tests to re-wire metadata and primary usage.
* Add ApacheParser skeleton.
Fix plumbing in configurator & dualparser to initialize ApacheParser
alongside AugeasParser.
* Silence coverage reports for now
* Implement add_child_comment (#7518)
* Remove unused apache docs (#7575)
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`
* Remove unused certbot-compatibility-test docs (#7577)
Part of #5775. We don't use these docs anywhere, so delete them.
Removes:
- `certbot-compatibility-test/readthedocs.org.requirements.txt`
- `certbot-compatibility-test/docs/` folder
- docs include in `MANIFEST.in`
- docs dependencies in `setup.py`
* Remove DNS plugin API docs. (#7578)
Replace DNS plugins' API documentation with a note that plugins adhere to certbot's plugin interface.
* remove unused route53 tools (#7586)
* Remove unused nginx docs (#7576)
Part of #5775. We don't use these docs anywhere, so delete them.
Removes:
- `certbot-nginx/readthedocs.org.requirements.txt`
- `certbot-nginx/docs/` folder
- docs include in `MANIFEST.in`
- docs dependencies in `setup.py`
* Remove unused nginx docs
* Add changelog entry about the removal
* Make the contents of the apache plugin private (#7579)
Part of #5775.
Tree:
```
certbot-apache/certbot_apache
├── __init__.py
├── _internal
│ ├── apache_util.py
│ ├── augeas_lens
│ │ ├── httpd.aug
│ │ └── README
│ ├── centos-options-ssl-apache.conf
│ ├── configurator.py
│ ├── constants.py
│ ├── display_ops.py
│ ├── entrypoint.py
│ ├── http_01.py
│ ├── __init__.py
│ ├── obj.py
│ ├── options-ssl-apache.conf
│ ├── override_arch.py
│ ├── override_centos.py
│ ├── override_darwin.py
│ ├── override_debian.py
│ ├── override_fedora.py
│ ├── override_gentoo.py
│ ├── override_suse.py
│ └── parser.py
└── tests
├── ...
```
* Create _internal folder for certbot_apache
* Move apache_util.py to _internal
* Move display_ops.py to _internal
* Move override_centos.py to _internal
* Move override_gentoo.py to _internal
* Move override_darwin.py to _internal
* Move override_suse.py to _internal
* Move override_debian.py to _internal
* Move override_fedora.py to _internal
* Move override_arch.py to _internal
* Move parser.py to _internal
* Move obj.py to _internal
* Move http_01.py to _internal
* Move entrypoint.py to _internal
* Move constants.py to _internal
* Move configurator.py to _internal
* Move augeas_lens to _internal
* Move options-ssl-apache.conf files to _internal
* move augeas_lens in MANIFEST
* Clean up some stray references to certbot_apache that could use _internal
* Correct imports and lint
* Make the contents of the DNS plugins private (#7580)
Part of #5775.
```
modify_item () {
mkdir certbot-dns-$1/certbot_dns_$1/_internal
git grep -l "from certbot_dns_$1 import dns_$1" | xargs sed -i "s/from certbot_dns_$1 import dns_$1/from certbot_dns_$1._internal import dns_$1/g"
git grep -l "certbot_dns_$1\.dns_$1" | xargs sed -i "s/certbot_dns_$1\.dns_$1/certbot_dns_$1._internal.dns_$1/g"
git checkout -- certbot-dns-$1/certbot_dns_$1/__init__.py
echo '"""Internal implementation of \`~certbot_dns_$1.dns_$1\` plugin."""' > certbot-dns-$1/certbot_dns_$1/_internal/__init__.py
mv certbot-dns-$1/certbot_dns_$1/dns_$1.py certbot-dns-$1/certbot_dns_$1/_internal
git checkout -- CHANGELOG.md
git status
git add -A
git commit -m "Move certbot-dns-$1 to _internal structure"
}
```
Structure now looks like this:
```
certbot-dns-cloudflare/
├── certbot_dns_cloudflare
│ ├── dns_cloudflare_test.py
│ ├── __init__.py
│ └── _internal
│ ├── dns_cloudflare.py
│ └── __init__.py
```
* Move certbot-dns-cloudflare to _internal structure
* Move certbot-dns-cloudxns to _internal structure
* Move certbot-dns-digitalocean to _internal structure
* Move certbot-dns-dnsimple to _internal structure
* Move certbot-dns-dnsmadeeasy to _internal structure
* Move certbot-dns-gehirn to _internal structure
* Move certbot-dns-google to _internal structure
* Move certbot-dns-linode to _internal structure
* Move certbot-dns-luadns to _internal structure
* Move certbot-dns-nsone to _internal structure
* Move certbot-dns-ovh to _internal structure
* Move certbot-dns-rfc2136 to _internal structure
* Move certbot-dns-sakuracloud to _internal structure
* Init file comments need to be comments
* Move certbot-dns-route53 to _internal structure
* Fix comment in route53 init
* Refactor certbot/ and certbot/tests/ to use the same structure as the other packages (#7544)
Summary of changes in this PR:
- Refactor files involved in the `certbot` module to be of a similar structure to every other package; that is, inside a directory inside the main repo root (see below).
- Make repo root README symlink to `certbot` README.
- Pull tests outside of the distributed module.
- Make `certbot/tests` not be a module so that `certbot` isn't added to Python's path for module discovery.
- Remove `--pyargs` from test calls, and make sure to call tests from repo root since without `--pyargs`, `pytest` takes directory names rather than package names as arguments.
- Replace mentions of `.` with `certbot` when referring to packages to install, usually editably.
- Clean up some unused code around executing tests in a different directory.
- Create public shim around main and make that the entry point.
New directory structure summary:
```
repo root ("certbot", probably, but for clarity all files I mention are relative to here)
├── certbot
│ ├── setup.py
│ ├── certbot
│ │ ├── __init__.py
│ │ ├── achallenges.py
│ │ ├── _internal
│ │ │ ├── __init__.py
│ │ │ ├── account.py
│ │ │ ├── ...
│ │ ├── ...
│ ├── tests
│ │ ├── account_test.py
│ │ ├── display
│ │ │ ├── __init__.py
│ │ │ ├── ...
│ │ ├── ... # note no __init__.py at this level
│ ├── ...
├── acme
│ ├── ...
├── certbot-apache
│ ├── ...
├── ...
```
* refactor certbot/ and certbot/tests/ to use the same structure as the other packages
* git grep -lE "\-e(\s+)\." | xargs sed -i -E "s/\-e(\s+)\./-e certbot/g"
* git grep -lE "\.\[dev\]" | xargs sed -i -E "s/\.\[dev\]/certbot[dev]/g"
* git grep -lE "\.\[dev3\]" | xargs sed -i -E "s/\.\[dev3\]/certbot[dev3]/g"
* Remove replacement of certbot into . in install_and_test.py
* copy license back out to main folder
* remove linter_plugin.py and CONTRIBUTING.md from certbot/MANIFEST.in because these files are not under certbot/
* Move README back into main folder, and make the version inside certbot/ a symlink
* symlink certbot READMEs the other way around
* move testdata into the public api certbot zone
* update source_paths in tox.ini to certbot/certbot to find the right subfolder for tests
* certbot version has been bumped down a directory level
* make certbot tests directory not a package and import sibling as module
* Remove unused script cruft
* change . to certbot in test_sdists
* remove outdated comment referencing a command that doesn't work
* Install instructions should reference an existing file
* update file paths in Dockerfile
* some package named in tox.ini were manually specified, change those to certbot
* new directory format doesn't work easily with pyargs according to http://doc.pytest.org/en/latest/goodpractices.html#tests-as-part-of-application-code
* remove other instance of pyargs
* fix up some references in _release.sh by searching for ' . ' and manual check
* another stray . in tox.ini
* fix paths in tools/_release.sh
* Remove final --pyargs call, and now-unnecessary call to modules instead of local files, since that's fixed by certbot's code being one layer deeper
* Create public shim around main and make that the entry point
* without pyargs, tests cannot be run from an empty directory
* Remove cruft for running certbot directly from main
* Have main shim take real arg
* add docs/api file for main, and fix up main comment
* Update certbot/docs/install.rst
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Fix comments in readthedocs requirements files to refer to current package
* Update .[docs] reference in contributing.rst
* Move plugins tests to certbot tests directory
* add certbot tests to MANIFEST.in so packagers can run python setup.py test
* move examples directory inside certbot/
* Move CHANGELOG into certbot, and create a top-level symlink
* Remove unused sys and logging from main shim
* nginx http01 test no longer relies on certbot plugins common test
* Make the contents of the nginx plugin private (#7589)
Part of #5775.
* Create _internal folder certbot-nginx
* Move configurator.py to _internal
* Move constants.py to _internal
* Move display_ops.py to _internal
* Move http_01.py to _internal
* Move nginxparser.py to _internal
* Move obj.py to _internal
* Move parser_obj.py to _internal
* Move parser.py to _internal
* Update location and references for tls_configs
* exclude parser_obj from coverage
* Update pull_request_template.md (#7596)
* Update pull_request_template.md
* Remove line breaks
Github seems to be keeping the line breaks rather than ignoring them, making it be formatted weirdly, so remove them.
* Fix refactor (#7597)
Clean up some places missed by #7544.
Found this when running test farm tests. They were working as of 5d90544, and I will truly shocked if subsequent changes (all to the windows installer) made them stop working.
* Release script needs to target new CHANGELOG location
* Clean up various other CHANGELOG path references
* Update windows paths for new certbot location
* Add certbot to packages list for windows installer
* Implement redirect by default (#7595)
* Change redirect default to yes so that it happens automatically in noninteractive mode
* Update changelog
* Refactor tests out of packaged module for dns plugins (#7599)
* Refactor tests out of module for certbot-dns-cloudflare
* Refactor tests out of module for certbot-dns-cloudxns
* Refactor tests out of module for certbot-dns-digitalocean
* Refactor tests out of module for certbot-dns-dnsimple
* Refactor tests out of module for certbot-dns-dnsmadeeasy
* Refactor tests out of module for certbot-dns-gehirn
* Refactor tests out of module for certbot-dns-google
* Refactor tests out of module for certbot-dns-linode
* Refactor tests out of module for certbot-dns-luadns
* Refactor tests out of module for certbot-dns-nsone
* Refactor tests out of module for certbot-dns-ovh
* Refactor tests out of module for certbot-dns-rfc2136
* Refactor tests out of module for certbot-dns-sakuracloud
* Refactor tests out of module for certbot-dns-route53
* Move certbot-dns-google testdata/ under tests/
* Use pytest for dns plugins
* Exclude pycache and .py[cod]
* Refactor tests out of packaged module for acme plugin (#7600)
* Move acme tests to tests/ directory outside of acme module
* Fix call to messages_test in client_test
* Move test_util.py and testdata/ into tests/
* Update manifest to package tests
* Exclude pycache and .py[cod]
* Exclude pycache and .py[cod] from certbot package (#7608)
* Refactor tests out of packaged module for nginx plugin (#7606)
* Refactor tests out of packaged module for nginx plugin
* Exclude pycache and .py[cod]
* Refactor tests out of packaged module for apache plugin (#7607)
Part of #7593.
* Refactor tests out of packaged module for apache plugin
* Exclude pycache and .py[cod]
* Change tests path in tox.ini
* Defines the RenewableCert API (#7603)
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
* [Apache v2] Implement find_ancestors (#7561)
* Implement find_ancestors
* Create the node properly and add assertions
* Update certbot-apache/certbot_apache/augeasparser.py
Co-Authored-By: ohemorange <ebportnoy@gmail.com>
* Remove comment
* Upgrade to pywin32>=227 (#7615)
Current version of pywin32 used in certbot (225) does not have wheels available for Python 3.8. Installing certbot for development in this case requires to build from source. On Windows, this implies a Visual Studio C++ environment up and ready, which is absolutely not fun.
Let's upgrade to pywin32 227, that provides these wheels for all Python versions from 3.5 up to current dev status of 3.9.
* [Apache v2] Move the apachectl parsing to apache_util (#7569)
* Move the Apache CLI parsing to apache_util
* Fix test mocks
* Address review comments
* Fix the parsernode metadata dictionary
* acme/setup.py: comment refers to "PyOpenSSL" not "mock" (#7619)
* Update changelog for 1.0.0 release
* Release 1.0.0
* Add contents to certbot/CHANGELOG.md for next version
* Bump version to 1.1.0
* document main (#7610)
I deleted the exceptions because I think it's not feasible to document the possible exceptions raised by all of Certbot.
* update external plugin (#7604)
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.
* Add full API documentation (#7614)
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`.
* fix bad links in docs (#7623)
This PR fixes the failures at https://travis-ci.com/certbot/website/builds/139193502#L1316.
Once this PR lands, I'll update certbot/website#508 to include this commit.
* Don't list DNS plugins as alpha quality. (#7624)
They should be considered production quality like our other packaged code.
* Don't list adding type annotations as a PR req. (#7627)
* Reorganize imports (#7616)
* Isort execution
* Fix pylint, adapt coverage
* New isort
* Fix magic_typing lint
* Second round
* Fix pylint
* Third round. Store isort configuration
* Fix latest mistakes
* Other fixes
* Add newline
* Fix lint errors
* [Apache v2] Implement parsed_files (#7562)
* Implement parsed_files
* Add parsed_files stub to ApacheParserNodes and fix assertions
* Update certbot-apache/certbot_apache/interfaces.py
Co-Authored-By: ohemorange <ebportnoy@gmail.com>
* Add more descriptive comments
* Update certbot-apache/certbot_apache/augeasparser.py
Co-Authored-By: ohemorange <ebportnoy@gmail.com>
* Update certbot-apache/certbot_apache/dualparser.py
Co-Authored-By: ohemorange <ebportnoy@gmail.com>
* Update certbot-apache/certbot_apache/interfaces.py
Co-Authored-By: ohemorange <ebportnoy@gmail.com>
* Lint certbot code on Python 3, and update Pylint to the latest version (#7551)
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
* Execute Windows installer integration tests on several Windows versions (#7641)
This PRs extends the installer tests on Azure Pipeline, in order to run the integration tests on a certbot instance installed with the Windows installer for several Windows versions, corresponding to the scope of supported versions on Certbot:
* Windows Server 2012 R2
* Windows Server 2016
* Windows Server 2019
One can see the result on: https://dev.azure.com/adferrand/certbot/_build/results?buildId=311
* Try specific installer-build step
* Install Python manually
* Add tests on windows 2019
* discourage dns plugins (#7639)
* Remove warning about dev preview (#7640)
* Add docker-compose as a requirement of certbot-ci (#7120)
Fixes #7110
This PR declares docker-compose as a requirement for certbot-ci. This way, a recent version of docker-compose is installed in the standard virtual environment set up by `tools/venv.py` and `tools/venv3.py`, and so is available to pytest integration tests from `tox` or in the virtual environment enabled.
* Add docker-compose as a dev dependency and declares it in certbot-ci requirements
* Update docker-compose 1.25.0
* Remove other 3.8-dev references. (#7646)
* Implement get_virtual_hosts() for ParserNode interfaces (#7564)
* How to uninstall certbot-auto (#7648)
* Include header files for compilation. (#7650)
* Remove POST-as-GET fallback to GET (#6994)
* Update CHANGELOG.md (#7659)
* Fix gating to ensure that no parsernode functionality is run unless explicitly requested (#7654)
* Modifications needed for merging to master
* [Apache v2] Add apacheconfig as a dependency (#7643)
* Add apacheconfig as a dependency.
* Change apacheconfig to a dev dependency
* Bump apacheconfig dep to 0.3.1
* Implement a sunset mechanism in certbot-auto for systems not supported anymore (#7587)
* Sunset mechanism
* Simplify code
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update template
* Deprecate for all RHEL/CentOS 6 32bits flavors
* Add a wrapper to uname to do tests on fake 32 bits versions
* Replace all occurences
* Add some tests about sunset mechanism
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Various corrections
* Recreate script
* Update comment position
* Test also install only
* Fix docker
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* What error command is doing here ?
* Fix permissions
* Rebuild script
* Add changelog
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Update changelog
* Trigger CI
* Handle old venv path
* Modify test
* Fix test error detection from subpaths
* Edit echo
* Use set -e
* Update letsencrypt-auto-source/letsencrypt-auto.template
Co-Authored-By: Brad Warren <bmw@users.noreply.github.com>
* Corrections
Co-authored-by: Brad Warren <bmw@users.noreply.github.com>
* improve help about supply selecting in delete command (#7673)
for #6625
* Do not document private members (#7675)
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.
* Fix certbot-auto regarding python 3.4 -> python 3.6 migration for CentOS 6 users (#7519)
* Revert "Add back Python 3.4 support (#7510)"
This reverts commit 9b848b1d65783000a13ef3f94ac5fe0e8c3879e7.
* Fix certbot-auto
* Use a more consistent way to enable rh-python36
* Avoid to call CompareVersions unecessarily
* Control rh-python36 exit code
* Fix travis config
* Remove vscode config
* Ignore vscode
* Fix merge conflicts regarding #7587 (#70)
* Add changelog entry
* Finish sentence
* Update certbot/CHANGELOG.md
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update letsencrypt-auto-source/tests/centos6_tests.sh
Co-Authored-By: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update comments
* Improve warning message
* Update changelog
Co-authored-by: Joona Hoikkala <joohoi@users.noreply.github.com>
* Update changelog for 1.1.0 release
* Release 1.1.0
* Add contents to certbot/CHANGELOG.md for next version
* Bump version to 1.2.0
* Add missing directory field (#7687)
Fixes #7683.
* Add missing directory field to error message
* Added change to CHANGELOG.md
* Do not list the name twice. (#7689)
* Cleanup disabled warnings list in pytest.ini. (#7690)
* Fix minimum certbot version in plugins (#7684)
Fixes the problem found at https://github.com/certbot/certbot/pull/7682#discussion_r367140415.
* Don't run some tests multiple times. (#7685)
* Include added/deleted TXT record name in RFC 2136 debug log (#7696)
* Spelling and grammar fixes (#7695)
* Downgrade NSIS and upgrade Python (#7702)
* Add --allow-downgrade to chocolatey command.
* Upgrade tests to use Python 3.8.1.
* Drop Travis tests for Python 3.4 (#7394)
* Minor release script improvements (#7697)
* Do not use git diff.
* Add a warning on exit.
* Don't run Python 3.5 tests twice. (#7704)
* unpin macos (#7705)
* fixes #1948 -- MD5 on FIPS systems (#7708)
* use MD5 in non-security mode to get around FIPS issue
* update CHANGELOG
* add myself to AUTHORS
* ignore hashlib params
* Update documentation files to remove claiming support for Python 3.4 (#7395)
* Fix collections.abc imports for Python 3.9 (#7707)
* Fix collections.abc imports for Python 3.9
* Update AUTHORS.md
* No longer ignore collections.abc deprecation warning
* Update changelog
* Remove outdated comment
* Disabling no-name-in-module not needed as linting is on Python 3
* Remove ECDHE-RSA-AES128-SHA from NGINX ciphers list (#7719)
As mentioned in https://github.com/certbot/certbot/pull/7712#discussion_r370419867, it's time to remove this ciphersuite now that Windows 2008 R2 and Windows 7 are EOLed.
* Remove ECDHE-RSA-AES128-SHA from NGINX ciphers list to celebrate Windows 2008 R2 deprecation
* Update changelog
* Drop Python 3.4 support (#7721)
Fixes #7393.
* Remove Python 3.4 classifiers
* Remove unneeded typing dependency
* Exclude Python 3.4 in python_requires
* Remove Python 3.4 deprecation warning
* update changelog
* Disable old SSL versions and ciphersuites to follow Mozilla recommendations in Apache (#7712)
Part of #7204.
Makes the smaller changes described at https://github.com/certbot/certbot/issues/7204#issuecomment-571838185 to disable many old ciphersuites and TLS versions < 1.2. Does not add checks for OpenSSL version or modify session tickets.
Since Apache uses TLS protocol blacklisting instead of whitelisting (as in NGINX), we additionally may not need to determine if the server supports TLS1.3 and turn it on or off based on Apache version.
* Update SSL versions and ciphersuites based on Mozilla intermediate recommendations for apache
* Update constants with hashes of new config files
* Update changelog
* Unpin Python 3.4 dependencies (#7709)
* Unpin dependencies pinned back for py3.4 support.
* update pinned packages
* run build.py
* Update boto3 and deps to work with requests
* Update dns-lexicon version. (#7723)
* dns-cloudflare: Implement limited-scope API Tokens (#7583)
A while ago Cloudflare added support for limited-scope API Tokens in place of using a global API key, but support for them in cloudflare/python-cloudflare took a while to get through.
In summary, this PR:
- Implements token functionality through the INI file parameter `dns_cloudflare_api_token` (in addition to the traditional `dns_cloudflare_email` and `dns_cloudflare_api_key`). This needed a more advanced parameter validator than the built in `required_variables` mechanism.
- Updates the docs to reflect the new option, needed token permissions, and version details of the `cloudflare` module
* Update python-cloudflare version
* Add Cloudflare API Token support to certbot-dns-cloudflare
* Add token-specific errors to certbot-dns-cloudflare
* Tidy up certbot-dns-cloudflare
* Implement Cloudflare API Tokens in testing for certbot-dns-cloudflare(needs work)
* Further tidying of certbot-dns-cloudflare
* Update CHANGELOG with Cloudflare API Tokens implementation
* Improve testing of certbot-dns-cloudflare
* Improve certbot-dns-cloudflare test formatting
* Further improve testing for certbot-dns-cloudflare
* Change needed permissions for token
* Add documentation regarding python-cloudflare version
* Fix changelog, references to python-cloudflare and docs
* Fix behaviour when domain does not match cloudflare root domain. Improve error handling.
* Improve testing
* Improve hints and error handling
* Add backwards compatibility docs (#7611)
Fixes #7463.
* Add backwards compatibility docs.
* Exclude certbot-auto
* Remove SSLCompression off line from all config options (#7726)
Based on discussion at https://github.com/certbot/certbot/pull/7712#discussion_r371451761.
* Remove SSLCompression off line from all config options
* Update changelog
* Add space between words.
* Update instructions about how to build docs (#7605)
* Turn off Travis notifications in test branches. (#7733)
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.
* Parse `$hostname` in `server_name`
* Add CHANGELOG entry
* Forgot to remove a `breakpoint()` statement
* Use unrestrictive umask for challenge directory
* Update changelog
* Update changelog for 1.2.0 release
* Release 1.2.0
* Add contents to certbot/CHANGELOG.md for next version
* Bump version to 1.3.0
* Wrap makedirs() within exception handelrs
* Missing import
* Windows installer integration tests (#7724)
As discussed in #7539, we need proper tests of the Windows installer itself in order to variety that all the logic contained in a production-grade runtime of Certbot on Windows is correctly setup by each version of the installer, and so for a variety of Windows OSes.
This PR handles this requirement. The new `windows_installer_integration_tests` module in `certbot-ci` will:
* run the given Windows installer
* check that Certbot is properly installed and working
* check that the scheduled renew task is set up
* check that the scheduled task actually launch the Certbot renew logic
The Windows nightly tests are updated accordingly, in order to have the tests run on Windows Server 2012R2, 2016 and 2019.
These tests will evolve as we add more logic on the installer.
* Configure an integration test testing the windows installer
* Write the test module
* Configurable installer path, prepare azure pipelines
* Fix option
* Update test_main.py
* Add confirmation for this destructive test
* Use regex to validate certbot --version output
* Explicit dependency on a log output
* Use an exception to ask confirmation
* Use --allow-persistent-changes
* Set recreate = true in tox.ini. (#7746)
Fixes #7745.
* Add triggers for only a single CI system (#7748)
* Configure travis-test to only run on Travis.
* Configure azure-test to only run on Azure.
* Add docs and comments to keep it up-to-date.
* restore CHANGELOG in root directory
* Add test for $hostname parsing
* Remove todo::
* Fixing existing tests
* Don't display todo comments in docs (#7753)
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.
* Remove text that certbot.tests.utils isn't public (#7754)
* Remove link to letsencrypt readthedocs (#7757)
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.
* Really remove old docs link from README (#7758)
* Move ocsp.py to public api (#7744)
We should move ocsp.py to public API, as an upcoming OCSP prefetching functionality in Apache plugin relies on it, and as the plugins are note released in lockstep with the Certbot core, we need to be careful when changing those APIs.
* Move ocsp.py to public api
* Fix type annotations, move to pointing to an interface and fix linting
* Add certbot.ocsp to documentation table of contents
* Modify tests to reflect the changes in ocsp.py
* Add changelog entry
* Fix notAfter mock for tests
* Print script output in case of a failure. (#7759)
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.
* Fix unpinned tests (#7760)
Our nightly tests failed last night due to a new release of `virtualenv` and `pip`'s lack of dependency resolution: https://travis-ci.com/certbot/certbot/jobs/285797857#L280. It looks like we were not the only ones affected by this problem: https://github.com/pypa/virtualenv/issues/1551
This fixes the problem by using `-I` to skip the logic where `pip` decides a dependency is already satisfied and has it reinstall/update the packages passed to `pip` and all of their dependencies.
You can see our nightly tests passing with this change at https://github.com/certbot/certbot/runs/439231061.
* Remove duplicate pyparsing pin
* update pyparsing comment
* Remove _internal from docstring.
* Remove useless pylint error suppression directives (#7657)
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
* more robustly stop patches (#7763)
* Remove letshelp-certbot (#7761)
* remove references to letshelp
* remove letshelp files
* Remove line continuation
Co-authored-by: ohemorange <ebportnoy@gmail.com>
* Fix spurious pylint errors. (#7780)
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.
* Correct AutoHSTS docs (#7767)
domains is a list of strings, not a single string.
* Correct AutoHSTS docs.
* Fix Apache enable_autohsts docs.
* add pgp key docs (#7765)
Fixes #7613.
* Move our macOS tests to Azure Pipelines (#7793)
[Our macOS tests are failing](https://travis-ci.com/certbot/certbot/builds/149965318) again this time due to the problem described at https://travis-ci.community/t/macos-build-fails-because-of-homebrew-bundle-unknown-command/7296/14.
I tried adding `update: true` to the Homebrew config as described in that thread, but [it didn't work](https://travis-ci.com/certbot/certbot/builds/150070374). I also tried updating the macOS image we use which [didn't work](https://travis-ci.com/certbot/certbot/builds/150072389).
Since we continue to have problems with macOS on Travis, let try moving the tests to Azure Pipelines.
* test macos
* Remove Travis macOS setup
* add displayName
* Refactor cli.py, splitting in it smaller submodules (#6803)
* Refactor cli.py into a package with submodules
* Added unit tests for helpful module in cli.
* Fixed linter errors
* Fixed pylint issues
* Updated changelog.md
* Fixed test failing and mypy error. Appeared a new pylint error (seems to be in conflict with mypy)
mypy require zope.interface to be imported but when imported it is not used and pylint throws an error.
* Fixed pylint errors
* Apply changes to cli since last merge from master (efc8d49806b14a31d88cfc0f1b6daca1dd373d8d)
* Fix lint
* Remaining lint errors
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
* Add test
* Use UTF-8 encoding for nginx plugin
* Use `io` module instead of `codecs`
See https://mail.python.org/pipermail/python-list/2015-March/687124.html
* Added test for valid/invalid unicode characters
* Fix lint problems with long lines
* Relpace deprecated `logger.warn()` with `logger.warning()`
* Remove `unicode_support/` path in test case
* Add test case for `_parse_ssl_options()`
* Add logging test for `_parse_files()`
* Trivial code clean-up
* Add my name to AUTHORS.md
:)
* Add this change to CHANGELOG.md
* Add `TestCase.assertLogs()` backport for Python 2.7
* Add simple comments
* acme: ignore params in content-type check (#7342)
* acme: ignore params in content-type check
Fixes the warning in #7339
* Suppress coverage complaint in test
* Update CHANGELOG
* Repair symlink
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
* Fix issue #7165 in _create_challenge_dirs(), attempt to fix pylint errors (#7568)
* fix issue #7165 by checking if directory exists before trying to create it, fix possible pylint issues in webroot.py
* fix get_chall_pref definition
* Update CHANGELOG.md
* Update CHANGELOG.md
Co-authored-by: Adrien Ferrand <adferrand@users.noreply.github.com>
* remove _internal docs (#7801)
* Fixed typo & some trivial documentation change
* Update comment in testdata file
* Update parser test to better assert logging output
* Split advanced pipeline (#7813)
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.
* update letstest reqs (#7809)
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.
* Remove unused notify code. (#7805)
This code is unused and hasn't been modified since 2015 except for various times our files have been renamed. Let's remove it.
* Change how _USE_DISTRO is set for mypy (#7804)
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.
* Fix tests on macOS Catalina (#7794)
This PR fixes the failures that can be seen at https://dev.azure.com/certbot/certbot/_build/results?buildId=1184&view=results.
You can see this code running on macOS Catalina at https://dev.azure.com/certbot/certbot/_build/results?buildId=1192&view=results.
* Remove references to deprecated flags in Certbot. (#7509)
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.
* Remove codecov (#7811)
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.
* Don't run advanced tests on PRs. (#7820)
When I wrote https://github.com/certbot/certbot/pull/7813, I didn't understand the default behavior for pull requests if you don't specify `pr` in the yaml file. According to https://docs.microsoft.com/en-us/azure/devops/pipelines/build/triggers?view=azure-devops&tabs=yaml#pr-triggers:
> If no pr triggers appear in your YAML file, pull request builds are automatically enabled for all branches...
This is not the behavior we want. This PR fixes the problem by disabling builds on PRs.
You should be able to see this working because the advanced tests should not run on this PR but they did run on https://github.com/certbot/certbot/pull/7811.
* Document safe and simple usage by services without root privileges (#7821)
Certificates are public information by design: they are provided by
web servers without any prior authentication required. In a public
key cryptographic system, only the private key is secret information.
The private key file is already created as accessible only to the root
user with mode 0600, and these file permissions are set before any key
content is written to the file. There is no window within which an
attacker with access to the containing directory would be able to read
the private key content.
Older versions of Certbot (prior to 0.29.0) would create private key
files with mode 0644 and rely solely on the containing directory
permissions to restrict access. We therefore cannot (yet) set the
relevant default directory permissions to 0755, since it is possible
that a user could install Certbot, obtain a certificate, then
downgrade to a pre-0.29.0 version of Certbot, then obtain another
certificate. This chain of events would leave the second
certificate's private key file exposed.
As a compromise solution, document the fact that it is safe for the
common case of non-downgrading users to change the permissions of
/etc/letsencrypt/{live,archive} to 0755, and explain how to use chgrp
and chmod to make the private key file readable by a non-root service
user.
This provides guidance on the simplest way to solve the common problem
of making keys and certificates usable by services that run without
root privileges, with no requirement to create a custom (and hence
error-prone) executable hook.
Remove the existing custom executable hook example, so that the
documentation contains only the simplest and safest way to solve this
very common problem.
Signed-off-by: Michael Brown <mbrown@fensystems.co.uk>
* Check OCSP as part of determining if the certificate is due for renewal (#7829)
Fixes #1028.
Doing this now because of https://community.letsencrypt.org/t/revoking-certain-certificates-on-march-4/.
The new `ocsp_revoked_by_paths` function is taken from https://github.com/certbot/certbot/pull/7649 with the optional argument removed for now because it is unused.
This function was added in this PR because `storage.py` uses `self.latest_common_version()` to determine which certificate should be looked at for determining renewal status at https://github.com/certbot/certbot/blob/9f8e4507ad0cb3dbedb726dda4c46affb1eb7ad3/certbot/certbot/_internal/storage.py#L939-L947
I think this is unnecessary and you can just look at the currently linked certificate, but I don't think we should be changing the logic that code has always had now.
* Check OCSP status as part of determining to renew
* add integration tests
* add ocsp_revoked_by_paths
* Update changelog for 1.3.0 release
* Release 1.3.0
* Add contents to certbot/CHANGELOG.md for next version
* Bump version to 1.4.0
* Fix issues with Azure Pipelines (#7838)
This PR fixes two issues.
First, it fixes #7814 by removing our tests on Windows Server 2012. I also added the sentence "Certbot supports Windows Server 2016 and Windows Server 2019." to https://community.letsencrypt.org/t/beta-phase-of-certbot-for-windows/105822.
Second, it fixes the test failures which can be seen at https://dev.azure.com/certbot/certbot/_build/results?buildId=1309&view=results by no longer manually installing…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
During review of #7519, we discovered that SCL was not available for 32 bits versions of CentOS 6, and so we could not migrate these uses on Python 3.6.
However, we need to handle that nicely, by giving a proper sunset mechanism that avoid certbot-auto to just break without warning. Also, it will be reused for Python 2.7 in general, then for every users as we will remove certbot-auto, so it is good to invest some time on it.
This PR implements that.
When certbot-auto detects a system that is not supported anymore:
Of course this mechanism is enabled for 32 bits CentOS 6 OSes and affiliates.
NB: Modifications in
letsencrypt-auto.templateL750-778 are taken directly from #7519 to make certbot-auto skip self-upgrade instead of breaking when no suitable version of Python is available.