mirror of
https://github.com/certbot/certbot.git
synced 2026-06-05 06:42:10 -04:00
Merge branch 'cli-failed-to-install-reporting' into ux
This commit is contained in:
commit
dd3348b1e2
4 changed files with 93 additions and 30 deletions
|
|
@ -9,7 +9,6 @@ from cryptography.hazmat.backends import default_backend
|
|||
from cryptography.hazmat.primitives.asymmetric.rsa import generate_private_key # type: ignore
|
||||
import josepy as jose
|
||||
import OpenSSL
|
||||
import zope.component
|
||||
|
||||
from acme import client as acme_client
|
||||
from acme import crypto_util as acme_crypto_util
|
||||
|
|
@ -18,7 +17,6 @@ from acme import messages
|
|||
import certbot
|
||||
from certbot import crypto_util
|
||||
from certbot import errors
|
||||
from certbot import interfaces
|
||||
from certbot import util
|
||||
from certbot._internal import account
|
||||
from certbot._internal import auth_handler
|
||||
|
|
@ -30,6 +28,7 @@ from certbot._internal import storage
|
|||
from certbot._internal.plugins import selection as plugin_selection
|
||||
from certbot.compat import os
|
||||
from certbot.display import ops as display_ops
|
||||
from certbot.display import util as display_util
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -517,10 +516,11 @@ class Client:
|
|||
|
||||
return abs_cert_path, abs_chain_path, abs_fullchain_path
|
||||
|
||||
def deploy_certificate(self, domains, privkey_path,
|
||||
def deploy_certificate(self, cert_name, domains, privkey_path,
|
||||
cert_path, chain_path, fullchain_path):
|
||||
"""Install certificate
|
||||
|
||||
:param str cert_name: name of the certificate lineage (optional)
|
||||
:param list domains: list of domains to install the certificate
|
||||
:param str privkey_path: path to certificate private key
|
||||
:param str cert_path: certificate file path (optional)
|
||||
|
|
@ -534,7 +534,13 @@ class Client:
|
|||
|
||||
chain_path = None if chain_path is None else os.path.abspath(chain_path)
|
||||
|
||||
msg = ("Unable to install the certificate")
|
||||
display_util.notify("Deploying certificate")
|
||||
|
||||
msg = f"Failed to install the certificate (installer: {self.config.installer})."
|
||||
if cert_name:
|
||||
msg += (" Try again by running:\n\n"
|
||||
f" {cli.cli_constants.cli_command} install --cert-name {cert_name}\n")
|
||||
|
||||
with error_handler.ErrorHandler(self._recovery_routine_with_msg, msg):
|
||||
for dom in domains:
|
||||
self.installer.deploy_cert(
|
||||
|
|
@ -625,7 +631,7 @@ class Client:
|
|||
logger.warning("Enhancement %s was already set.",
|
||||
enhancement)
|
||||
except errors.PluginError:
|
||||
logger.warning("Unable to set enhancement %s for %s",
|
||||
logger.error("Unable to set enhancement %s for %s",
|
||||
enhancement, dom)
|
||||
raise
|
||||
|
||||
|
|
@ -638,8 +644,7 @@ class Client:
|
|||
|
||||
"""
|
||||
self.installer.recovery_routine()
|
||||
reporter = zope.component.getUtility(interfaces.IReporter)
|
||||
reporter.add_message(success_msg, reporter.HIGH_PRIORITY)
|
||||
display_util.notify(success_msg)
|
||||
|
||||
def _rollback_and_restart(self, success_msg):
|
||||
"""Rollback the most recent checkpoint and restart the webserver
|
||||
|
|
@ -648,19 +653,18 @@ class Client:
|
|||
|
||||
"""
|
||||
logger.critical("Rolling back to previous server configuration...")
|
||||
reporter = zope.component.getUtility(interfaces.IReporter)
|
||||
try:
|
||||
self.installer.rollback_checkpoints()
|
||||
self.installer.restart()
|
||||
except:
|
||||
reporter.add_message(
|
||||
logger.error(
|
||||
"An error occurred and we failed to restore your config and "
|
||||
"restart your server. Please post to "
|
||||
"https://community.letsencrypt.org/c/help "
|
||||
"with details about your configuration and this error you received.",
|
||||
reporter.HIGH_PRIORITY)
|
||||
"with details about your configuration and this error you received."
|
||||
)
|
||||
raise
|
||||
reporter.add_message(success_msg, reporter.HIGH_PRIORITY)
|
||||
display_util.notify(success_msg)
|
||||
|
||||
|
||||
def validate_key_csr(privkey, csr=None):
|
||||
|
|
|
|||
|
|
@ -838,7 +838,19 @@ def _install_cert(config, le_client, domains, lineage=None):
|
|||
path_provider = lineage if lineage else config
|
||||
assert path_provider.cert_path is not None
|
||||
|
||||
le_client.deploy_certificate(domains, path_provider.key_path,
|
||||
cert_name: Optional[str] = None
|
||||
if isinstance(path_provider, storage.RenewableCert):
|
||||
cert_name = path_provider.lineagename
|
||||
elif path_provider.certname:
|
||||
cert_name = path_provider.certname
|
||||
else:
|
||||
# Check if the cert path happens to be part of an existing lineage
|
||||
try:
|
||||
cert_name = cert_manager.cert_path_to_lineage(config)
|
||||
except errors.Error:
|
||||
pass
|
||||
|
||||
le_client.deploy_certificate(cert_name, domains, path_provider.key_path,
|
||||
path_provider.cert_path, path_provider.chain_path, path_provider.fullchain_path)
|
||||
le_client.enhance_config(domains, path_provider.chain_path)
|
||||
|
||||
|
|
|
|||
|
|
@ -578,6 +578,10 @@ class CertPathToLineageTest(storage_test.BaseRenewableCertTest):
|
|||
self._archive_files(x, 'fullchain')]
|
||||
self.assertEqual('example.org', self._call(self.config))
|
||||
|
||||
def test_only_path(self):
|
||||
self.config.cert_path = self.fullchain
|
||||
self.assertEqual('example.org', self._call(self.config))
|
||||
|
||||
|
||||
class MatchAndCheckOverlaps(storage_test.BaseRenewableCertTest):
|
||||
"""Tests for certbot._internal.cert_manager.match_and_check_overlaps w/o overlapping
|
||||
|
|
|
|||
|
|
@ -515,15 +515,16 @@ class ClientTest(ClientTestCommon):
|
|||
|
||||
shutil.rmtree(tmp_path)
|
||||
|
||||
def test_deploy_certificate_success(self):
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_success(self, mock_util):
|
||||
self.assertRaises(errors.Error, self.client.deploy_certificate,
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
|
||||
installer = mock.MagicMock()
|
||||
self.client.installer = installer
|
||||
|
||||
self.client.deploy_certificate(
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
installer.deploy_cert.assert_called_once_with(
|
||||
cert_path=os.path.abspath("cert"),
|
||||
chain_path=os.path.abspath("chain"),
|
||||
|
|
@ -533,46 +534,81 @@ class ClientTest(ClientTestCommon):
|
|||
self.assertEqual(installer.save.call_count, 2)
|
||||
installer.restart.assert_called_once_with()
|
||||
|
||||
def test_deploy_certificate_failure(self):
|
||||
@mock.patch('certbot._internal.client.display_util.notify')
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_failure(self, mock_util, mock_notify):
|
||||
installer = mock.MagicMock()
|
||||
self.client.installer = installer
|
||||
self.config.installer = "foobar"
|
||||
|
||||
installer.deploy_cert.side_effect = errors.PluginError
|
||||
self.assertRaises(errors.PluginError, self.client.deploy_certificate,
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
installer.recovery_routine.assert_called_once_with()
|
||||
|
||||
def test_deploy_certificate_save_failure(self):
|
||||
mock_notify.assert_any_call('Deploying certificate')
|
||||
mock_notify.assert_any_call(
|
||||
'Failed to install the certificate (installer: foobar). '
|
||||
'Try again by running:\n\n certbot install --cert-name foo.bar\n'
|
||||
)
|
||||
|
||||
@mock.patch('certbot._internal.client.display_util.notify')
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_failure_no_certname(self, mock_util, mock_notify):
|
||||
installer = mock.MagicMock()
|
||||
self.client.installer = installer
|
||||
self.config.installer = "foobar"
|
||||
|
||||
installer.deploy_cert.side_effect = errors.PluginError
|
||||
self.assertRaises(errors.PluginError, self.client.deploy_certificate,
|
||||
None, ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
installer.recovery_routine.assert_called_once_with()
|
||||
|
||||
mock_notify.assert_any_call('Deploying certificate')
|
||||
mock_notify.assert_any_call(
|
||||
'Failed to install the certificate (installer: foobar).'
|
||||
)
|
||||
|
||||
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_save_failure(self, mock_util):
|
||||
installer = mock.MagicMock()
|
||||
self.client.installer = installer
|
||||
|
||||
installer.save.side_effect = errors.PluginError
|
||||
self.assertRaises(errors.PluginError, self.client.deploy_certificate,
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
installer.recovery_routine.assert_called_once_with()
|
||||
|
||||
@mock.patch('certbot._internal.client.display_util.notify')
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_restart_failure(self, mock_get_utility):
|
||||
def test_deploy_certificate_restart_failure(self, mock_get_utility, mock_notify):
|
||||
installer = mock.MagicMock()
|
||||
installer.restart.side_effect = [errors.PluginError, None]
|
||||
self.client.installer = installer
|
||||
|
||||
self.assertRaises(errors.PluginError, self.client.deploy_certificate,
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
self.assertEqual(mock_get_utility().add_message.call_count, 1)
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
mock_notify.assert_called_with(
|
||||
'We were unable to install your certificate, however, we successfully restored '
|
||||
'your server to its prior configuration.')
|
||||
installer.rollback_checkpoints.assert_called_once_with()
|
||||
self.assertEqual(installer.restart.call_count, 2)
|
||||
|
||||
@mock.patch('certbot._internal.client.logger')
|
||||
@test_util.patch_get_utility()
|
||||
def test_deploy_certificate_restart_failure2(self, mock_get_utility):
|
||||
def test_deploy_certificate_restart_failure2(self, mock_get_utility, mock_logger):
|
||||
installer = mock.MagicMock()
|
||||
installer.restart.side_effect = errors.PluginError
|
||||
installer.rollback_checkpoints.side_effect = errors.ReverterError
|
||||
self.client.installer = installer
|
||||
|
||||
self.assertRaises(errors.PluginError, self.client.deploy_certificate,
|
||||
["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
self.assertEqual(mock_get_utility().add_message.call_count, 1)
|
||||
"foo.bar", ["foo.bar"], "key", "cert", "chain", "fullchain")
|
||||
self.assertEqual(mock_logger.error.call_count, 1)
|
||||
self.assertIn(
|
||||
'An error occurred and we failed to restore your config',
|
||||
mock_logger.error.call_args[0][0])
|
||||
installer.rollback_checkpoints.assert_called_once_with()
|
||||
self.assertEqual(installer.restart.call_count, 1)
|
||||
|
||||
|
|
@ -659,7 +695,7 @@ class EnhanceConfigTest(ClientTestCommon):
|
|||
def test_enhance_failure(self):
|
||||
self.client.installer = mock.MagicMock()
|
||||
self.client.installer.enhance.side_effect = errors.PluginError
|
||||
self._test_error()
|
||||
self._test_error(enhance_error=True)
|
||||
self.client.installer.recovery_routine.assert_called_once_with()
|
||||
|
||||
def test_save_failure(self):
|
||||
|
|
@ -685,12 +721,19 @@ class EnhanceConfigTest(ClientTestCommon):
|
|||
self._test_error()
|
||||
self.assertIs(self.client.installer.restart.called, True)
|
||||
|
||||
def _test_error(self):
|
||||
def _test_error(self, enhance_error=False, restart_error=False):
|
||||
self.config.redirect = True
|
||||
with test_util.patch_get_utility() as mock_gu:
|
||||
with mock.patch('certbot._internal.client.logger') as mock_logger, \
|
||||
test_util.patch_get_utility() as mock_gu:
|
||||
self.assertRaises(
|
||||
errors.PluginError, self._test_with_all_supported)
|
||||
self.assertEqual(mock_gu().add_message.call_count, 1)
|
||||
|
||||
if enhance_error:
|
||||
self.assertEqual(mock_logger.error.call_count, 1)
|
||||
self.assertIn('Unable to set enhancement', mock_logger.error.call_args_list[0][0][0])
|
||||
if restart_error:
|
||||
mock_logger.critical.assert_called_with(
|
||||
'Rolling back to previous server configuration...')
|
||||
|
||||
def _test_with_all_supported(self):
|
||||
if self.client.installer is None:
|
||||
|
|
|
|||
Loading…
Reference in a new issue