Second part of #4443. Built on #4444. Fixes#3148.
This fixes an old problem with code logging messages before logging has been set up. How this works is explained in the docstring of certbot.log.pre_arg_setup.
* add memory handler
* Add exit_with_log_path
* add new_except_hook
* pre_arg_parse_setup++ and remove old except_hook
* Rewrite post_arg_setup
* test restricted permissions
* move changes to main
* Use .name of NamedTemporaryFile
* use better assertions
* set exc_info in except_hook
* Make post_arg_setup more robust
* final cleanup
* Add TempHandler
* undo main_test changes
* improve documentation
* use decorators instead of with for mock.patch
* add inline comment about logging.shutdown
In some sense, certbot.client.Client is an ACME client, but it's the not the client in the ACME library and this leads to confusion. Let's make what this is clear.
* call certbot client cb_client rather than acme_client
* update tests
* add fasteners as a dependency
* add LOCK_FILE constant
* Add lock file to Certbot
* Move code to _run_subcommand
* move lock file path into CLI_CONSTANTS
* add --lock-path flag
* move locking code to separate function
* Add TestAcquireFileLock
* assert we log
* test lock contention
* add fasteners to certbot-auto
* Use a different lock file for each test in MainTest
* remove instructions about removed Vagrantfile
* rewrite docker instructions
* say where docker-compose can be run
* give better intro for macOS devs
* prompt people for permission to install OS packages
* reword awkward sentence
* Change WORKDIR to /opt/certbot/src
This change is OK because all paths used in Dockerfile-dev are absolute paths.
* remove 'cd src' instructions for Dockerfile-dev
* Improve docker-compose testing instructions
* stop conditionally pinning mock version in acme
* stop conditionally pinning mock version in certbot
* stop conditionally pinning mock version in apache
* stop conditionally pinning mock version in nginx
* stop conditionally pinning mock version in letshelp
* stop conditionally pinning mock version in compatibility-test
* add get_certbot_version.sh
* Use get_certbot_version.sh in build.py
* make test_leauto_upgrades.sh more robust
* auto upgrades break tests
* OCSP experimental is not needed anymore
* Add test_sdists.sh
* Use LE_AUTO_VERSION, not repo version
* install OS deps earlier
* use readlink not realpath
* undo changes to build.py
* Factor out version code from build.py
* Use version.py in test_sdists
* Remove get_certbot_version
* workaround setuptools breakage
* use challenge type strings, not objectS
* Factor out parse_preferred_challenges
* restore pref_challs
* save pref_challs
* Make CheckCertCount more flexible
* improve integration tests
* Make pref_challs more flexible
* add py36 to tox
* Add Python 3.6 tests to Travis
* Provide real path to python stdlib during tests
* set logs_dir in config_test
* set *_dirs in DetermineAccountTest
* Fix TLSSNI01Test
* Fix RenewalTest
* fix test_ancient_webroot_renewal_conf
Fixes#3996.
I'm pretty confident this PR solves the problem. I've audited all calls to IDisplay methods and the assertions done in certbot.display.util are now done in all our unit tests.
With that said, it wouldn't hurt to have someone else double check I didn't miss anything. The easiest way to do this is to grep for IDisplay in our code and ensure all calls to IDisplay methods are valid. This means every method call other than notification (because a notification call is always OK) either provides a value for default or force_interactive. This is defined in interfaces.py.
I've also been considering removing the assertion that's been causing us trouble here from our release. The only argument I have for not doing so is it may hinder 3rd party plugin development. When they use IDisplay, they have the same problem as we do with prompting users without a TTY. Not keeping this assertion in makes it more likely they won't notice the issue and Certbot will crash on an unsuspecting user.
With that said, none of our known 3rd party plugins use IDisplay at all.
* Provide force_interactive in _get_certname
* Use force_interactive when asking for webroot
* Factor IDisplay assertion into it's own function
* Add util.patch_get_utility()
* Allow custom path to patch_get_utiity
* Change GetEmailTest to use patch_get_utility
* Use new_callable to create new objects
* Modify tests to use patch_get_utility
* Improve FreezableMock documentation
* Add user facing error to TTY magic
* Comment out assert_valid_call
* Add test_input_assertion_fail2()
* Remove incorrect hook message
* Add generalized msg about skipping hooks
* Properly report when hooks were skipped
* Also print message about renew hook
* Change quotes to help OCD
* Start of combined manual/script plugin
* Return str from hooks.execute, not bytes
* finish manual/script rewrite
* delete old manual and script plugins
* manually specify we want chall.token
* use consistent quotes
* specify chall for uri
* s/script/hook
* fix spacing on instructions
* remove unneeded response argument
* make achall more helpful
* simplify perform
* remove old test files
* add start of manual_tests
* fix ParseTest.test_help
* stop using manual_test_mode in cli tests
* Revert "make achall more helpful"
This reverts commit 54b01cea30.
* use bad response/validation methods on achalls
* simplify perform and cleanup environment
* finish manual tests
* Add HTTP manual hook integration test
* add manual http scripts
* Add manual DNS script integration test
* remove references to the script plugin
* they're hooks, not scripts
* add --manual-public-ip-logging-ok to integration tests
* use --pref-chall for dns integration
* does dns work?
* validate hooks
* test hook validation
* Revert "does dns work?"
This reverts commit 1224cc2961.
* busy wait in manual-http-auth
* remove DNS script test for now
* Fix challenge prefix and add trailing .
* Add comment about universal_newlines
* Fix typo from 0464ba2c4
* fix nits and typos
* Generalize HookCOmmandNotFound error
* Add verify_exe_exists
* Don't duplicate code in hooks.py
* Revert changes to hooks.py
* Use consistent hook error messages
* correctly match * and ? in ServerAlias directives
* update Apache wildcard test
* Consolidate wildcard matching and remove bad test
* Test Apache vhost selection with wildcards
* Added few more tests to proof vhost selection
* CLI flag for forcing interactivity
* add --force-interactive
* Add force_interactive error checking and tests
* Add force_interactive parameter to FileDisplay
* add _can_interact
* Add _return_default
* Add **unused_kwargs to NoninteractiveDisplay
* improve _return_default assertion
* Change IDisplay calls and write tests
* Document force_interactive in interfaces.py
* Don't force_interactive with a new prompt
* Warn when skipping an interaction for the first time
* add specific logger.debug message
* Stop passing around config and refactor tests
* Refactor and warn during enhance_config
* Use mock.ANY to make new Pythons happy
* Remove verbose enhance_config from test names
* Fix spacing in warning
* pin requests version in py26-oldest
* Determine requests security deps dynamically
Starting with requests 2.12, pyasn1 and ndg-httpsclient are no longer
needed to inject pyopenssl into urllib3. This change allows us to
determine whether or not these dependencies are required at install
time. If an older version of requests is used, these packages are
still installed. If a new version of requests is used, they are not
reducing the number of dependencies we have.
* Bump requests version in certbot-auto
* Use pkg_resources in activate test
Due to pip's lack of dependency resolution, the change to use
requests[extras] causes errors in acme.util_test because pkg_resources
accurately detects the "missing" dependency.
There isn't a real problem here. The problem comes from a brand new
requests and ancient pyopenssl as well as a unit test for
functionality we plan to remove in our next release. I modified
the unit test to fix the problem for now.
* Use six instead of pkg_resources for test
* Require requests<=2.11.1 in py27-oldest test
If we don't do this, we get test failures for the certbot package
which is actually a good thing! pkg_resources is catching the
unlikely but possible problem I describe in #3803 and erroring out
saying it is missing the necessary dependencies to run certbot.
Good job package resources.
* Undo changes to acme.util_test
* Changed informational messages because of confusing message on reinstallation.
Certbot prompts the user when it detects that an appropriately fresh certificate
is already available:
You have an existing certificate that contains exactly the same domains you requested and isn't close to expiry.
(ref: <path>)
What would you like to do?
-------------------------------------------------------------------------------
1: Attempt to reinstall this existing certificate
2: Renew & replace the cert (limit ~5 per 7 days)
-------------------------------------------------------------------------------
Select the appropriate number [1-2] then [enter] (press 'c' to cancel): 1
On selecting '1' (reinstall), the resulting message is:
-------------------------------------------------------------------------------
Your existing certificate has been successfully reinstalled, and the new
certificate has been installed.
The new certificate covers the following domains: https://<whatever>
You should test your configuration at:
https://www.ssllabs.com/ssltest/analyze.html?d=<whatever>
-------------------------------------------------------------------------------
"Your existing certificate has been successfully reinstalled" <-- Okay
"and the new certificate has been installed." <-- Wait, what?
The issue appears to come from assumptions in certbot/certbot/main.py
It uses `len(lineage.available_versions("cert"))` to determine if this was a
fresh install or renewal, and then calls either `display_ops.success_renewal()`
(which produces the "existing certificate ... and the new certificate" language)
or `display_ops.success_installation()` (which has no messaging about existing
vs. new certificates).
The len(lineage) test isn't the right way to make this choice. The certificate's
lineage length doesn't imply anything about whether we've just obtained a new
certificate, because there is no new certificate in the case of a "reinstall"
action.
The new logic calls `display_ops.success_installation()` on all "reinstall"
actions, and otherwise employs the existing `len(lineage)` test.
Additionally the `display_ops.success_installation()` has been enhanced to
accept an action parameter, and has the message reworded slightly to make
sense regardless of the action passed. The messaging is mostly unchanged if it's
called without the action parameter:
Original message:
-------------------------------------------------------------------------------
Congratulations! You have successfully enabled https://<whatever>
You should test your configuration at:
https://www.ssllabs.com/ssltest/analyze.html?d=<whatever>
-------------------------------------------------------------------------------
New message on initial install:
-------------------------------------------------------------------------------
Congratulations! You have successfully installed a certificate for
https://<whatever>
You should test your configuration at:
https://www.ssllabs.com/ssltest/analyze.html?d=<whatever>
-------------------------------------------------------------------------------
New message on re-install:
-------------------------------------------------------------------------------
Congratulations! You have successfully reinstalled a certificate for
https://<whatever>
You should test your configuration at:
https://www.ssllabs.com/ssltest/analyze.html?d=<whatever>
-------------------------------------------------------------------------------
* Typo in display message.
* Typo, characters transposed.
* undo changes to certbot/display/ops.py
* remove invalid todos
* Test success_installation() called for reinstall
* Simplify display_ops.success* functions
* refactor and expand run() tests
* Allow running constants_test.py individually
* Mock until tests pass
Mock out both functions used to determine the OS in
certbot_apache.tests.constants_test.
* Add and test activate function to acme.
This function can be used to check if our optional dependencies are
available and they meet our version requirements.
* use activate in dns_resolver
* use activate in dns_available() in challenges_test
* Use activate in dns_resolver_test
* Use activate in certbot.plugins.util_test
* Use acme.util.activate for psutil
* Better testing and handling of missing deps
* Factored out *_available() code into a common function
* Delayed exception caused from using acme.dns_resolver without
dnspython until the function is called. This makes both
production and testing code simpler.
* Make a common subclass for already_listening tests
* Simplify mocking of USE_PSUTIL in tests
* only verify port is available when you actually need it
* refactor code to create achalls
* Test port checks are based on achall
* test that only the port for the requested challenge is checked in standalone
* Create symlinks at runtime in cli_test.py
* use test_util.vector_path rather than hardcoding path
* Reference #2716 in comment about too many lines in cli.py
* Add and test activate function to acme.
This function can be used to check if our optional dependencies are
available and they meet our version requirements.
* use activate in dns_resolver
* use activate in dns_available() in challenges_test
* Use activate in dns_resolver_test
* Use activate in certbot.plugins.util_test
* Use acme.util.activate for psutil
* Better testing and handling of missing deps
* Factored out *_available() code into a common function
* Delayed exception caused from using acme.dns_resolver without
dnspython until the function is called. This makes both
production and testing code simpler.
* Make a common subclass for already_listening tests
* Simplify mocking of USE_PSUTIL in tests
* only verify port is available when you actually need it
* refactor code to create achalls
* Test port checks are based on achall
* test that only the port for the requested challenge is checked in standalone
* Create symlinks at runtime in cli_test.py
* use test_util.vector_path rather than hardcoding path
* Reference #2716 in comment about too many lines in cli.py
* fix requirements.txt surgery in response to shipping certbot-nginx (#3585)
* Make --quiet reduce the logging level (#3593)
* reduce logging level and ignore verbose flags in quiet mode
* Simplify setup_logging parameters
The extra parameters were there in the past when the letsencrypt-renewer was a
separate executable that also used this function. This is cruft that can be
removed.
* Add basic tests for setup_logging
* Release 0.9.1
* Bump version to 0.10.0
* reduce logging level and ignore verbose flags in quiet mode
* Simplify setup_logging parameters
The extra parameters were there in the past when the letsencrypt-renewer was a
separate executable that also used this function. This is cruft that can be
removed.
* Add basic tests for setup_logging
* reduce logging level and ignore verbose flags in quiet mode
* Simplify setup_logging parameters
The extra parameters were there in the past when the letsencrypt-renewer was a
separate executable that also used this function. This is cruft that can be
removed.
* Add basic tests for setup_logging
* move skipUnless to test_util
* add skip_unless to acme test_util
* Make dns_resolver_tests work with and without dnspython
* make acme.challenges_test pass when dns is unavailable
* remove unhelpful question about servernames and default vhosts
* add prefix about names found in config files
* test we include configuration files prefix
* Tell the user what kind of conf files were missing domains
* Revert "Tell the user what kind of conf files were missing domains"
This reverts commit 1066a88dae.
