upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-08 16:22:18 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Simplify and deprecate viewing config changes (#7198)

Browse source 
* Remove apache and nginx from config_changes help

* Deprecate certbot_config changes.

* Document config_changes deprecation.

* Remove view_config_changes as IInstaller method.

* Remove view_config_changes from plugins.

* Add view_config_changes warnings.

* simplify test_config_changes_deprecation
This commit is contained in:
Brad Warren 2019-07-02 17:20:12 -07:00 committed by ohemorange
parent 88876b9901
commit 20b595bc9e
12 changed files with 67 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
CHANGELOG.md
View file

@ -15,7 +15,12 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
* Update the 'manage your account' help to be more generic.
* The error message when Certbot's Apache plugin is unable to modify your
Apache configuration has been improved.
* Certbot's config_changes subcommand has been deprecated and will be
removed in a future release.
* `certbot config_changes` no longer accepts a --num parameter.
* The functions `certbot.plugins.common.Installer.view_config_changes` and
`certbot.reverter.Reverter.view_config_changes` have been deprecated and will
be removed in a future release.
### Fixed

8
certbot-apache/certbot_apache/tests/configurator_reverter_test.py
View file

@ -74,14 +74,6 @@ class ConfiguratorReverterTest(util.ApacheTest):
side_effect=errors.ReverterError)
self.assertRaises(errors.PluginError, self.config.rollback_checkpoints)
def test_view_config_changes(self):
self.config.view_config_changes()
def test_view_config_changes_error(self):
self.config.reverter.view_config_changes = mock.Mock(
side_effect=errors.ReverterError)
self.assertRaises(errors.PluginError, self.config.view_config_changes)
def test_recovery_routine_reload(self):
mock_load = mock.Mock()
self.config.parser.aug.load = mock_load

5
certbot-nginx/certbot_nginx/tests/configurator_test.py
View file

@ -427,11 +427,6 @@ class NginxConfiguratorTest(util.NginxTest):
mock_recovery_routine.side_effect = errors.ReverterError("foo")
self.assertRaises(errors.PluginError, self.config.recovery_routine)
@mock.patch("certbot.reverter.Reverter.view_config_changes")
def test_view_config_changes_throws_error_from_reverter(self, mock_view_config_changes):
mock_view_config_changes.side_effect = errors.ReverterError("foo")
self.assertRaises(errors.PluginError, self.config.view_config_changes)
@mock.patch("certbot.reverter.Reverter.rollback_checkpoints")
def test_rollback_checkpoints_throws_error_from_reverter(self, mock_rollback_checkpoints):
mock_rollback_checkpoints.side_effect = errors.ReverterError("foo")

4
certbot/cli.py
View file

@ -1418,10 +1418,10 @@ def _plugins_parsing(helpful, plugins):
help="Authenticator plugin name.")
helpful.add("plugins", "-i", "--installer", default=flag_default("installer"),
help="Installer plugin name (also used to find domains).")
helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
helpful.add(["plugins", "certonly", "run", "install"],
"--apache", action="store_true", default=flag_default("apache"),
help="Obtain and install certificates using Apache")
helpful.add(["plugins", "certonly", "run", "install", "config_changes"],
helpful.add(["plugins", "certonly", "run", "install"],
"--nginx", action="store_true", default=flag_default("nginx"),
help="Obtain and install certificates using Nginx")
helpful.add(["plugins", "certonly"], "--standalone", action="store_true",

7
certbot/interfaces.py
View file

@ -355,13 +355,6 @@ class IInstaller(IPlugin):
"""
def view_config_changes(): # type: ignore
"""Display all of the LE config changes.
:raises .PluginError: when config changes cannot be parsed
"""
def config_test(): # type: ignore
"""Make sure the configuration is valid.

2
certbot/main.py
View file

@ -976,6 +976,8 @@ def config_changes(config, unused_plugins):
:rtype: None
"""
logger.warning("The config_changes subcommand has been deprecated"
" and will be removed in a future release.")
client.view_config_changes(config)
def update_symlinks(config, unused_plugins):

17
certbot/plugins/common.py
View file

@ -3,6 +3,7 @@ import logging
import re
import shutil
import tempfile
import warnings
import OpenSSL
import pkg_resources
@ -197,10 +198,18 @@ class Installer(Plugin):
the checkpoints directories.
"""
try:
self.reverter.view_config_changes()
except errors.ReverterError as err:
raise errors.PluginError(str(err))
warnings.warn(
"The view_config_changes method is no longer part of Certbot's"
" plugin interface, has been deprecated, and will be removed in a"
" future release.", DeprecationWarning, stacklevel=2)
with warnings.catch_warnings():
# Don't let the reverter code warn about this function. Calling
# this function in the first place is the real problem.
warnings.filterwarnings("ignore", ".*view_config_changes", DeprecationWarning)
try:
self.reverter.view_config_changes()
except errors.ReverterError as err:
raise errors.PluginError(str(err))
@property
def ssl_dhparams(self):

47
certbot/plugins/common_test.py
View file

@ -3,6 +3,7 @@ import functools
import shutil
import tempfile
import unittest
import warnings
import OpenSSL
import josepy as jose
@ -97,9 +98,8 @@ class InstallerTest(test_util.ConfigTestCase):
os.mkdir(self.config.config_dir)
from certbot.plugins.common import Installer
with mock.patch("certbot.plugins.common.reverter.Reverter"):
self.installer = Installer(config=self.config,
name="Installer")
self.installer = Installer(config=self.config,
name="Installer")
self.reverter = self.installer.reverter
def test_add_to_real_checkpoint(self):
@ -121,12 +121,11 @@ class InstallerTest(test_util.ConfigTestCase):
temporary=temporary)
if temporary:
reverter_func = self.reverter.add_to_temp_checkpoint
reverter_func_name = "add_to_temp_checkpoint"
else:
reverter_func = self.reverter.add_to_checkpoint
reverter_func_name = "add_to_checkpoint"
self._test_adapted_method(
installer_func, reverter_func, files, save_notes)
self._test_adapted_method(installer_func, reverter_func_name, files, save_notes)
def test_finalize_checkpoint(self):
self._test_wrapped_method("finalize_checkpoint", "foo")
@ -143,6 +142,19 @@ class InstallerTest(test_util.ConfigTestCase):
def test_view_config_changes(self):
self._test_wrapped_method("view_config_changes")
def test_view_config_changes_warning_supression(self):
with warnings.catch_warnings():
# Without the catch_warnings() code in
# common.Installer.view_config_changes, this would raise an
# exception. The module parameter here is ".*common$" because the
# stacklevel=2 parameter of warnings.warn causes the warning to
# refer to the code in the caller rather than the call to
# warnings.warn. This means the warning in common.Installer refers
# to this module and the warning in the reverter refers to the
# plugins.common module.
warnings.filterwarnings("error", ".*view_config_changes", module=".*common$")
self.installer.view_config_changes()
def _test_wrapped_method(self, name, *args, **kwargs):
"""Test a wrapped reverter method.
@ -152,28 +164,27 @@ class InstallerTest(test_util.ConfigTestCase):
"""
installer_func = getattr(self.installer, name)
reverter_func = getattr(self.reverter, name)
self._test_adapted_method(
installer_func, reverter_func, *args, **kwargs)
self._test_adapted_method(installer_func, name, *args, **kwargs)
def _test_adapted_method(self, installer_func,
reverter_func, *passed_args, **passed_kwargs):
reverter_func_name, *passed_args, **passed_kwargs):
"""Test an adapted reverter method
:param callable installer_func: installer method to test
:param mock.MagicMock reverter_func: mocked adapated
reverter method
:param str reverter_func_name: name of the method on the
reverter that should be called
:param tuple passed_args: positional arguments passed from
installer method to the reverter method
:param dict passed_kargs: keyword arguments passed from
installer method to the reverter method
"""
installer_func(*passed_args, **passed_kwargs)
reverter_func.assert_called_once_with(*passed_args, **passed_kwargs)
reverter_func.side_effect = errors.ReverterError
self.assertRaises(
errors.PluginError, installer_func, *passed_args, **passed_kwargs)
with mock.patch.object(self.reverter, reverter_func_name) as reverter_func:
installer_func(*passed_args, **passed_kwargs)
reverter_func.assert_called_once_with(*passed_args, **passed_kwargs)
reverter_func.side_effect = errors.ReverterError
self.assertRaises(
errors.PluginError, installer_func, *passed_args, **passed_kwargs)
def test_install_ssl_dhparams(self):
self.installer.install_ssl_dhparams()

3
certbot/plugins/null.py
View file

@ -49,9 +49,6 @@ class Installer(common.Plugin):
def recovery_routine(self):
pass # pragma: no cover
def view_config_changes(self):
pass # pragma: no cover
def config_test(self):
pass # pragma: no cover

7
certbot/reverter.py
View file

@ -6,6 +6,7 @@ import shutil
import sys
import time
import traceback
import warnings
import six
import zope.component
@ -143,6 +144,12 @@ class Reverter(object):
:raises .errors.ReverterError: If invalid directory structure.
"""
warnings.warn(
"The view_config_changes method has been deprecated and will be"
" removed in a future release. If you were using this method to"
" implement the view_config_changes method of IInstaller, know that"
" that method has been removed from the plugin interface and is no"
" longer used by Certbot.", DeprecationWarning, stacklevel=2)
backups = os.listdir(self.config.backup_dir)
backups.sort(reverse=True)
if not backups:

7
certbot/tests/main_test.py
View file

@ -757,6 +757,13 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met
_, _, _, client = self._call(['config_changes'])
self.assertEqual(1, client.view_config_changes.call_count)
@mock.patch('certbot.main.logger.warning')
def test_config_changes_deprecation(self, mock_warning):
self._call(['config_changes'])
self.assertTrue(mock_warning.called)
msg = mock_warning.call_args[0][0]
self.assertIn("config_changes subcommand has been deprecated", msg)
@mock.patch('certbot.cert_manager.update_live_symlinks')
def test_update_symlinks(self, mock_cert_manager):
self._call_no_clientmock(['update_symlinks'])

2
pytest.ini
View file

@ -10,9 +10,11 @@
# but it should be corrected to allow Certbot compatiblity with Python >= 3.8
# 4- ipdb uses deprecated functionality of IPython. See
# https://github.com/gotcha/ipdb/issues/144.
# 5- ignore our own warnings about deprecated view_config_changes methods
filterwarnings =
error
ignore:decodestring:DeprecationWarning
ignore:(TLSSNI01|TLS-SNI-01):DeprecationWarning
ignore:.*collections\.abc:DeprecationWarning
ignore:The `color_scheme` argument is deprecated:DeprecationWarning:IPython.*
ignore:.*view_config_changes:DeprecationWarning