crypt/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-01-26 07:41:33 +03:00
Code Activity

cli: miscellaneous IReporter removals (#8436)

Browse Source 
* certbot delete: use undecorated print

* certbot revoke: use undecorated print

* certbot revoke: remove ireporter usages

* eff: remove IReporter usages

* certbot unregister: remove IReporter usage

* certbot update_account: remove IReporter usages

* certbot run: remove IReporter in duplicate prompt

* fix test_revoke_multiple_lineages
This commit is contained in:
alexzorin
2020-11-10 10:31:27 +11:00
committed by GitHub
parent 11a4882128
commit bf20f39ceb
9 changed files with 59 additions and 63 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
certbot-ci/certbot_integration_tests/certbot_tests/test_main.py
View File

@@ -544,7 +544,8 @@ def test_revoke_multiple_lineages(context):
'revoke', '--cert-path', join(context.config_dir, 'live', cert1, 'cert.pem')
])
assert 'Not deleting revoked certs due to overlapping archive dirs' in output
with open(join(context.workspace, 'logs', 'letsencrypt.log'), 'r') as f:
assert 'Not deleting revoked certs due to overlapping archive dirs' in f.read()
def test_wildcard_certificates(context):

4
certbot/certbot/_internal/cert_manager.py
View File

@@ -100,8 +100,8 @@ def delete(config):
return
for certname in certnames:
storage.delete_files(config, certname)
disp.notification("Deleted all files relating to certificate {0}."
.format(certname), pause=False)
display_util.notify("Deleted all files relating to certificate {0}."
.format(certname))
###################
# Public Helpers

4
certbot/certbot/_internal/eff.py
View File

@@ -7,6 +7,7 @@ import zope.component
from acme.magic_typing import Optional # pylint: disable=unused-import
from certbot import interfaces
from certbot.display import util as display_util
from certbot._internal import constants
from certbot._internal.account import Account # pylint: disable=unused-import
from certbot._internal.account import AccountFileStorage
@@ -133,5 +134,4 @@ def _report_failure(reason=None):
msg.append(' because ')
msg.append(reason)
msg.append('. You can try again later by visiting https://act.eff.org.')
reporter = zope.component.getUtility(interfaces.IReporter)
reporter.add_message(''.join(msg), reporter.LOW_PRIORITY)
display_util.notify(''.join(msg))

23
certbot/certbot/_internal/main.py
View File

@@ -163,8 +163,7 @@ def _handle_subset_cert_request(config, domains, cert):
cli_flag="--expand",
force_interactive=True):
return "renew", cert
reporter_util = zope.component.getUtility(interfaces.IReporter)
reporter_util.add_message(
display_util.notify(
"To obtain a new certificate that contains these names without "
"replacing your existing certificate for {0}, you must use the "
"--duplicate option.{br}{br}"
@@ -172,8 +171,7 @@ def _handle_subset_cert_request(config, domains, cert):
existing,
sys.argv[0], " ".join(sys.argv[1:]),
br=os.linesep
),
reporter_util.HIGH_PRIORITY)
))
raise errors.Error(USER_CANCELLED)
@@ -542,7 +540,6 @@ def _delete_if_appropriate(config):
archive dir is found for the specified lineage, etc ...
"""
display = zope.component.getUtility(interfaces.IDisplay)
reporter_util = zope.component.getUtility(interfaces.IReporter)
attempt_deletion = config.delete_after_revoke
if attempt_deletion is None:
@@ -552,7 +549,6 @@ def _delete_if_appropriate(config):
force_interactive=True, default=True)
if not attempt_deletion:
reporter_util.add_message("Not deleting revoked certs.", reporter_util.LOW_PRIORITY)
return
# config.cert_path must have been set
@@ -570,9 +566,8 @@ def _delete_if_appropriate(config):
cert_manager.match_and_check_overlaps(config, [lambda x: archive_dir],
lambda x: x.archive_dir, lambda x: x)
except errors.OverlappingMatchFound:
msg = ('Not deleting revoked certs due to overlapping archive dirs. More than '
'one lineage is using {0}'.format(archive_dir))
reporter_util.add_message(''.join(msg), reporter_util.MEDIUM_PRIORITY)
logger.warning("Not deleting revoked certs due to overlapping archive dirs. More than "
"one certificate is using %s", archive_dir)
return
except Exception as e:
msg = ('config.default_archive_dir: {0}, config.live_dir: {1}, archive_dir: {2},'
@@ -625,7 +620,6 @@ def unregister(config, unused_plugins):
"""
account_storage = account.AccountFileStorage(config)
accounts = account_storage.find_all()
reporter_util = zope.component.getUtility(interfaces.IReporter)
if not accounts:
return "Could not find existing account to deactivate."
@@ -647,7 +641,7 @@ def unregister(config, unused_plugins):
# delete local account files
account_files.delete(config.account)
reporter_util.add_message("Account deactivated.", reporter_util.MEDIUM_PRIORITY)
display_util.notify("Account deactivated.")
return None
@@ -698,8 +692,6 @@ def update_account(config, unused_plugins):
# exist or not.
account_storage = account.AccountFileStorage(config)
accounts = account_storage.find_all()
reporter_util = zope.component.getUtility(interfaces.IReporter)
add_msg = lambda m: reporter_util.add_message(m, reporter_util.MEDIUM_PRIORITY)
if not accounts:
return "Could not find an existing account to update."
@@ -724,10 +716,11 @@ def update_account(config, unused_plugins):
account_storage.update_regr(acc, cb_client.acme)
if config.email is None:
add_msg("Any contact information associated with this account has been removed.")
display_util.notify("Any contact information associated "
"with this account has been removed.")
else:
eff.prepare_subscription(config, acc)
add_msg("Your e-mail address was updated to {0}.".format(config.email))
display_util.notify("Your e-mail address was updated to {0}.".format(config.email))
return None

10
certbot/certbot/display/ops.py
View File

@@ -262,17 +262,15 @@ def success_renewal(domains):
def success_revocation(cert_path):
"""Display a box confirming a certificate has been revoked.
"""Display a message confirming a certificate has been revoked.
:param list cert_path: path to certificate which was revoked.
"""
z_util(interfaces.IDisplay).notification(
display_util.notify(
"Congratulations! You have successfully revoked the certificate "
"that was located at {0}{1}{1}".format(
cert_path,
os.linesep),
pause=False)
"that was located at {0}.".format(cert_path)
)
def _gen_https_names(domains):

6
certbot/tests/cert_manager_test.py
View File

@@ -114,16 +114,20 @@ class DeleteTest(storage_test.BaseRenewableCertTest):
cert_manager.delete(self.config)
@test_util.patch_get_utility()
@mock.patch('certbot.display.util.notify')
@mock.patch('certbot._internal.cert_manager.lineage_for_certname')
@mock.patch('certbot._internal.storage.delete_files')
def test_delete_from_config_yes(self, mock_delete_files, mock_lineage_for_certname,
mock_util):
mock_notify, mock_util):
"""Test delete"""
mock_lineage_for_certname.return_value = self.test_rc
mock_util().yesno.return_value = True
self.config.certname = "example.org"
self._call()
mock_delete_files.assert_called_once_with(self.config, "example.org")
mock_notify.assert_called_once_with(
"Deleted all files relating to certificate example.org."
)
@test_util.patch_get_utility()
@mock.patch('certbot._internal.cert_manager.lineage_for_certname')

12
certbot/tests/display/ops_test.py
View File

@@ -384,16 +384,14 @@ class SuccessRevocationTest(unittest.TestCase):
success_revocation(path)
@test_util.patch_get_utility("certbot.display.ops.z_util")
def test_success_revocation(self, mock_util):
mock_util().notification.return_value = None
@mock.patch("certbot.display.util.notify")
def test_success_revocation(self, mock_notify, unused_mock_util):
path = "/path/to/cert.pem"
self._call(path)
mock_util().notification.assert_called_once_with(
mock_notify.assert_called_once_with(
"Congratulations! You have successfully revoked the certificate "
"that was located at {0}{1}{1}".format(
path,
os.linesep), pause=False)
self.assertTrue(path in mock_util().notification.call_args[0][0])
"that was located at {0}.".format(path)
)
class ValidatorTests(unittest.TestCase):

35
certbot/tests/eff_test.py
View File

@@ -43,11 +43,12 @@ class PrepareSubscriptionTest(SubscriptionTest):
prepare_subscription(self.config, self.account)
@test_util.patch_get_utility()
def test_failure(self, mock_get_utility):
@mock.patch("certbot._internal.eff.display_util.notify")
def test_failure(self, mock_notify, mock_get_utility):
self.config.email = None
self.config.eff_email = True
self._call()
actual = mock_get_utility().add_message.call_args[0][0]
actual = mock_notify.call_args[0][0]
expected_part = "because you didn't provide an e-mail address"
self.assertTrue(expected_part in actual)
self.assertIsNone(self.account.meta.register_to_eff)
@@ -121,6 +122,10 @@ class SubscribeTest(unittest.TestCase):
self.json = {'status': True}
self.response = mock.Mock(ok=True)
self.response.json.return_value = self.json
self.mock_notify = mock.patch("certbot._internal.eff.display_util.notify").start()
def tearDown(self):
self.mock_notify.stop()
@mock.patch('certbot._internal.eff.requests.post')
def _call(self, mock_post):
@@ -139,42 +144,38 @@ class SubscribeTest(unittest.TestCase):
self.assertFalse(data is None)
self.assertEqual(data.get('email'), self.email)
@test_util.patch_get_utility()
def test_bad_status(self, mock_get_utility):
def test_bad_status(self):
self.json['status'] = False
self._call()
actual = self._get_reported_message(mock_get_utility)
actual = self._get_reported_message()
expected_part = 'because your e-mail address appears to be invalid.'
self.assertTrue(expected_part in actual)
@test_util.patch_get_utility()
def test_not_ok(self, mock_get_utility):
def test_not_ok(self):
self.response.ok = False
self.response.raise_for_status.side_effect = requests.exceptions.HTTPError
self._call()
actual = self._get_reported_message(mock_get_utility)
actual = self._get_reported_message()
unexpected_part = 'because'
self.assertFalse(unexpected_part in actual)
@test_util.patch_get_utility()
def test_response_not_json(self, mock_get_utility):
def test_response_not_json(self):
self.response.json.side_effect = ValueError()
self._call()
actual = self._get_reported_message(mock_get_utility)
actual = self._get_reported_message()
expected_part = 'problem'
self.assertTrue(expected_part in actual)
@test_util.patch_get_utility()
def test_response_json_missing_status_element(self, mock_get_utility):
def test_response_json_missing_status_element(self):
self.json.clear()
self._call()
actual = self._get_reported_message(mock_get_utility)
actual = self._get_reported_message()
expected_part = 'problem'
self.assertTrue(expected_part in actual)
def _get_reported_message(self, mock_get_utility):
self.assertTrue(mock_get_utility().add_message.called)
return mock_get_utility().add_message.call_args[0][0]
def _get_reported_message(self):
self.assertTrue(self.mock_notify.called)
return self.mock_notify.call_args[0][0]
@test_util.patch_get_utility()
def test_subscribe(self, mock_get_utility):

25
certbot/tests/main_test.py
View File

@@ -339,23 +339,23 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase):
from certbot._internal.main import _delete_if_appropriate
_delete_if_appropriate(mock_config)
def _test_delete_opt_out_common(self, mock_get_utility):
def _test_delete_opt_out_common(self):
with mock.patch('certbot._internal.cert_manager.delete') as mock_delete:
self._call(self.config)
mock_delete.assert_not_called()
self.assertTrue(mock_get_utility().add_message.called)
@test_util.patch_get_utility()
def test_delete_flag_opt_out(self, mock_get_utility):
def test_delete_flag_opt_out(self, unused_mock_get_utility):
self.config.delete_after_revoke = False
self._test_delete_opt_out_common(mock_get_utility)
self._test_delete_opt_out_common()
@test_util.patch_get_utility()
def test_delete_prompt_opt_out(self, mock_get_utility):
util_mock = mock_get_utility()
util_mock.yesno.return_value = False
self._test_delete_opt_out_common(mock_get_utility)
self._test_delete_opt_out_common()
@mock.patch("certbot._internal.main.logger.warning")
@mock.patch('certbot._internal.storage.renewal_file_for_certname')
@mock.patch('certbot._internal.cert_manager.delete')
@mock.patch('certbot._internal.cert_manager.match_and_check_overlaps')
@@ -365,7 +365,7 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase):
def test_overlapping_archive_dirs(self, mock_get_utility,
mock_cert_path_to_lineage, mock_archive,
mock_match_and_check_overlaps, mock_delete,
mock_renewal_file_for_certname):
mock_renewal_file_for_certname, mock_warning):
# pylint: disable = unused-argument
config = self.config
config.cert_path = "/some/reasonable/path"
@@ -374,6 +374,7 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase):
mock_match_and_check_overlaps.side_effect = errors.OverlappingMatchFound()
self._call(config)
mock_delete.assert_not_called()
self.assertEqual(mock_warning.call_count, 1)
@mock.patch('certbot._internal.storage.renewal_file_for_certname')
@mock.patch('certbot._internal.cert_manager.match_and_check_overlaps')
@@ -1448,9 +1449,10 @@ class MainTest(test_util.ConfigTestCase):
# ensure we didn't try to subscribe (no email to subscribe with)
self.assertFalse(mock_prepare.called)
@mock.patch("certbot._internal.main.display_util.notify")
@mock.patch('certbot._internal.main.display_ops.get_email')
@test_util.patch_get_utility()
def test_update_account_with_email(self, mock_utility, mock_email):
def test_update_account_with_email(self, mock_utility, mock_email, mock_notify):
email = "user@example.com"
mock_email.return_value = email
with mock.patch('certbot._internal.eff.prepare_subscription') as mock_prepare:
@@ -1475,7 +1477,7 @@ class MainTest(test_util.ConfigTestCase):
# and we saved the updated registration on disk
self.assertTrue(mocked_storage.update_regr.called)
self.assertTrue(
email in mock_utility().add_message.call_args[0][0])
email in mock_notify.call_args[0][0])
self.assertTrue(mock_prepare.called)
@mock.patch('certbot._internal.plugins.selection.choose_configurator_plugins')
@@ -1517,7 +1519,8 @@ class UnregisterTest(unittest.TestCase):
res = main.unregister(config, unused_plugins)
self.assertEqual(res, "Deactivation aborted.")
def test_unregister(self):
@mock.patch("certbot._internal.main.display_util.notify")
def test_unregister(self, mock_notify):
mocked_storage = mock.MagicMock()
mocked_storage.find_all.return_value = ["an account"]
@@ -1533,9 +1536,7 @@ class UnregisterTest(unittest.TestCase):
res = main.unregister(config, unused_plugins)
self.assertTrue(res is None)
self.assertTrue(cb_client.acme.deactivate_registration.called)
m = "Account deactivated."
self.assertTrue(m in self.mocks['get_utility']().add_message.call_args[0][0])
mock_notify.assert_called_once_with("Account deactivated.")
def test_unregister_no_account(self):
mocked_storage = mock.MagicMock()