Refactor the cli params

certbot-apache/certbot_apache/configurator.py

@@ -1990,4 +1990,3 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         return common.install_version_controlled_file(options_ssl, options_ssl_digest,
             self.constant("MOD_SSL_CONF_SRC"), constants.ALL_SSL_OPTIONS_HASHES)
 
-

certbot/cli.py

@@ -1073,12 +1073,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
     helpful.add(
         "security", "--rsa-key-size", type=int, metavar="N",
         default=flag_default("rsa_key_size"), help=config_help("rsa_key_size"))
-    helpful.add(
-        "security", "--disable-installer-updates", action="store_false",
-        default=flag_default("installer_updates"), dest="installer_updates",
-        help="Disable any automatic updates to your server configuration that"
-        " would otherwise be done by the selected installer plugin, and"
-        " triggered by the \"renew\" verb.")
     helpful.add(
         "security", "--must-staple", action="store_true",
         dest="must_staple", default=flag_default("must_staple"),
@@ -1123,8 +1117,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         help="Require that all configuration files are owned by the current "
              "user; only needed if your config is somewhere unsafe like /tmp/")
     helpful.add(
-        "security", "--dangerously-disable-server-tls-updates", action="store_false",
-        default=flag_default("server_tls_updates"), dest="server_tls_updates",
+        "security", "--dangerously-disable-server-tls-updates", action="store_true",
+        default=flag_default("disable_server_tls_updates"),
+        dest="disable_server_tls_updates",
         help="Disable any updates to your server's TLS configuration"
         " other than setting the certificate and key to be used when"
         " Certbot installs a new certificate. Using this flag is"
@@ -1188,6 +1183,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):  # pylint: dis
         default=flag_default("directory_hooks"), dest="directory_hooks",
         help="Disable running executables found in Certbot's hook directories"
         " during renewal. (default: False)")
+    helpful.add(
+        "renew", "--disable-renew-updates", action="store_true",
+        default=flag_default("disable_renew_updates"), dest="disable_renew_updates",
+        help="Disable automatic updates to your server configuration that"
+        " would otherwise be done by the selected installer plugin, and"
+        " triggered by the \"renew\" verb. This setting does not apply to"
+        " important TLS configuration updates.")
 
     helpful.add_deprecated_argument("--agree-dev-preview", 0)
     helpful.add_deprecated_argument("--dialog", 0)

certbot/constants.py

@@ -64,8 +64,8 @@ CLI_DEFAULTS = dict(
     pref_challs=[],
     validate_hooks=True,
     directory_hooks=True,
-    server_tls_updates=True,
-    installer_updates=False,
+    disable_server_tls_updates=False,
+    disable_renew_updates=False,
 
     # Subparsers
     num=None,

certbot/interfaces.py

@@ -256,13 +256,13 @@ class IConfig(zope.interface.Interface):
         "user; only needed if your config is somewhere unsafe like /tmp/."
         "This is a boolean")
 
-    server_tls_updates = zope.interface.Attribute(
-        "If updates to the server's TLS configuration should be"
-        " performed by the installer.")
+    disable_server_tls_updates = zope.interface.Attribute(
+        "If updates to the server's TLS configuration performed by the installer"
+        " should be disabled.")
 
-    installer_updates = zope.interface.Attribute(
-        "If updates provided by installer enhancements should be performed"
-        " when Certbot is being run with \"renew\" verb.")
+    disable_renew_updates = zope.interface.Attribute(
+        "If updates provided by installer enhancements when Certbot is being run"
+        " with \"renew\" verb should be disabled.")
 
 class IInstaller(IPlugin):
     """Generic Certbot Installer Interface.
@@ -679,14 +679,14 @@ class ServerTLSUpdater(object):
     and keys used by the server.
 
     An installer can determine if TLS server updates are enabled by checking
-    :attr:`IConfig.server_tls_updates`.
+    :attr:`IConfig.disable_server_tls_updates`.
 
     """
 
     __metaclass__ = abc.ABCMeta
 
     @abc.abstractmethod
-    def server_tls_updates(self, domain, lineage=None, *args, **kwargs):
+    def server_tls_updates(self, domain, *args, **kwargs):
         """Set the server's TLS config to latest recommended version.
 
         This function will only be called if the user hasn't disabled TLS

certbot/plugins/selection.py

@@ -192,7 +192,7 @@ def choose_configurator_plugins(config, plugins, verb):  # pylint: disable=too-m
             installer = pick_installer(config, req_inst, plugins)
         if need_auth:
             authenticator = pick_authenticator(config, req_auth, plugins)
-    if installer is not None:
+    if installer is not None and verb != "renew":
         verify_enhancements_supported(config, installer)
 
     logger.debug("Selected authenticator %s and installer %s", authenticator, installer)
@@ -226,7 +226,7 @@ def verify_enhancements_supported(config, installer):
     :raises errors.MisconfigurationError: configuration conflict
 
     """
-    if not config.server_tls_updates:
+    if config.disable_server_tls_updates:
         flag = "--dangerously-disable-server-tls-updates"
         if isinstance(installer, interfaces.ServerTLSUpdater):
             verified = z_util(interfaces.IDisplay).yesno(

certbot/tests/renewupdater_test.py

@@ -75,8 +75,8 @@ class RenewUpdaterTest(unittest.TestCase):
     @mock.patch('certbot.plugins.selection.choose_configurator_plugins')
     @test_util.patch_get_utility()
     def test_server_updates(self, _, mock_select, mock_getsave):
-        config = self.get_config({"server_tls_updates": True,
-                                  "installer_updates": True})
+        config = self.get_config({"disable_server_tls_updates": False,
+                                  "disable_renew_updates": False})
 
         lineage = mock.MagicMock()
         lineage.names.return_value = ['firstdomain', 'seconddomain']
@@ -104,7 +104,6 @@ class RenewUpdaterTest(unittest.TestCase):
 
         mock_generic_updater.restart.reset_mock()
         mock_generic_updater.callcounter.reset_mock()
-
         updater.run_renewal_updaters(config, None, lineage)
         self.assertEqual(mock_generic_updater.callcounter.call_count, 2)
         self.assertFalse(mock_generic_updater.restart.called)

certbot/updater.py

@@ -44,7 +44,8 @@ def run_renewal_deployer(lineage, installer, config):
     :returns: `None`
     :rtype: None
     """
-    if config.installer_updates and isinstance(installer, interfaces.RenewDeployer):
+    if not config.disable_renew_updates and isinstance(installer,
+                                                       interfaces.RenewDeployer):
         installer.renew_deploy(lineage)
 
 def _run_updaters(lineage, installer, config):
@@ -61,9 +62,9 @@ def _run_updaters(lineage, installer, config):
     :rtype: None
     """
     for domain in lineage.names():
-        if config.server_tls_updates:
+        if not config.disable_server_tls_updates:
             if isinstance(installer, interfaces.ServerTLSUpdater):
                 installer.server_tls_updates(domain)
-        if config.installer_updates:
+        if not config.disable_renew_updates:
             if isinstance(installer, interfaces.GenericUpdater):
                 installer.generic_updates(domain)