Use setattr in NamespaceConfig (#4362)

* set setattr in NamespaceConfig

* remove unnecessary uses of .namespace

* add simple test to ensure it works

certbot/client.py

@@ -166,7 +166,7 @@ def perform_registration(acme, config):
                        "registration again." % config.email)
                 raise errors.Error(msg)
             else:
-                config.namespace.email = display_ops.get_email(invalid=True)
+                config.email = display_ops.get_email(invalid=True)
                 return perform_registration(acme, config)
         else:
             raise

certbot/configuration.py

@@ -42,7 +42,7 @@ class NamespaceConfig(object):
     """
 
     def __init__(self, namespace):
-        self.namespace = namespace
+        object.__setattr__(self, 'namespace', namespace)
 
         self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
         self.namespace.work_dir = os.path.abspath(self.namespace.work_dir)
@@ -54,6 +54,9 @@ class NamespaceConfig(object):
     def __getattr__(self, name):
         return getattr(self.namespace, name)
 
+    def __setattr__(self, name, value):
+        setattr(self.namespace, name, value)
+
     @property
     def server_path(self):
         """File path based on ``server``."""

certbot/main.py

@@ -359,7 +359,7 @@ def _determine_account(config):
             acc = accounts[0]
         else:  # no account registered yet
             if config.email is None and not config.register_unsafely_without_email:
-                config.namespace.email = display_ops.get_email()
+                config.email = display_ops.get_email()
 
             def _tos_cb(regr):
                 if config.tos:
@@ -382,7 +382,7 @@ def _determine_account(config):
                 raise errors.Error(
                     "Unable to register an account with ACME server")
 
-    config.namespace.account = acc.id
+    config.account = acc.id
     return acc, acme
 
 
@@ -459,7 +459,7 @@ def register(config, unused_plugins):
             return ("--register-unsafely-without-email provided, however, a "
                     "new e-mail address must\ncurrently be provided when "
                     "updating a registration.")
-        config.namespace.email = display_ops.get_email(optional=False)
+        config.email = display_ops.get_email(optional=False)
 
     acc, acme = _determine_account(config)
     acme_client = client.Client(config, acc, None, None, acme=acme)
@@ -565,7 +565,7 @@ def certificates(config, unused_plugins):
 def revoke(config, unused_plugins):  # TODO: coop with renewal config
     """Revoke a previously obtained certificate."""
     # For user-agent construction
-    config.namespace.installer = config.namespace.authenticator = "None"
+    config.installer = config.authenticator = "None"
     if config.key_path is not None:  # revocation by cert key
         logger.debug("Revoking %s using cert key %s",
                      config.cert_path[0], config.key_path[0])

certbot/plugins/selection.py

@@ -137,9 +137,8 @@ noninstaller_plugins = ["webroot", "manual", "standalone"]
 
 def record_chosen_plugins(config, plugins, auth, inst):
     "Update the config entries to reflect the plugins we actually selected."
-    cn = config.namespace
+    config.authenticator = plugins.find_init(auth).name if auth else "None"
-    cn.authenticator = plugins.find_init(auth).name if auth else "None"
+    config.installer = plugins.find_init(inst).name if inst else "None"
-    cn.installer = plugins.find_init(inst).name if inst else "None"
 
 
 def choose_configurator_plugins(config, plugins, verb):

certbot/renewal.py

@@ -103,13 +103,13 @@ def _restore_webroot_config(config, renewalparams):
     """
     if "webroot_map" in renewalparams:
         if not cli.set_by_cli("webroot_map"):
-            config.namespace.webroot_map = renewalparams["webroot_map"]
+            config.webroot_map = renewalparams["webroot_map"]
     elif "webroot_path" in renewalparams:
         logger.debug("Ancient renewal conf file without webroot-map, restoring webroot-path")
         wp = renewalparams["webroot_path"]
         if isinstance(wp, str):  # prior to 0.1.0, webroot_path was a string
             wp = [wp]
-        config.namespace.webroot_path = wp
+        config.webroot_path = wp
 
 
 def _restore_plugin_configs(config, renewalparams):
@@ -148,10 +148,10 @@ def _restore_plugin_configs(config, renewalparams):
                 if config_value in ("None", "True", "False"):
                     # bool("False") == True
                     # pylint: disable=eval-used
-                    setattr(config.namespace, config_item, eval(config_value))
+                    setattr(config, config_item, eval(config_value))
                 else:
                     cast = cli.argparse_type(config_item)
-                    setattr(config.namespace, config_item, cast(config_value))
+                    setattr(config, config_item, cast(config_value))
 
 
 def restore_required_config_elements(config, renewalparams):
@@ -172,7 +172,7 @@ def restore_required_config_elements(config, renewalparams):
     for item_name, restore_func in required_items:
         if item_name in renewalparams and not cli.set_by_cli(item_name):
             value = restore_func(item_name, renewalparams[item_name])
-            setattr(config.namespace, item_name, value)
+            setattr(config, item_name, value)
 
 
 def _restore_pref_challs(unused_name, value):

certbot/tests/configuration_test.py

@@ -120,6 +120,12 @@ class NamespaceConfigTest(unittest.TestCase):
         self.assertTrue(os.path.isabs(config.live_dir))
         self.assertTrue(os.path.isabs(config.renewal_configs_dir))
 
+    def test_get_and_set_attr(self):
+        self.config.foo = 42
+        self.assertEqual(self.config.namespace.foo, 42)
+        self.config.namespace.bar = 1337
+        self.assertEqual(self.config.bar, 1337)
+
 
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover

certbot/tests/renewal_test.py

@@ -52,7 +52,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase):
     def test_allow_subset_of_names_success(self, mock_set_by_cli):
         mock_set_by_cli.return_value = False
         self._call(self.config, {'allow_subset_of_names': 'True'})
-        self.assertTrue(self.config.namespace.allow_subset_of_names is True)
+        self.assertTrue(self.config.allow_subset_of_names is True)
 
     @mock.patch('certbot.renewal.cli.set_by_cli')
     def test_allow_subset_of_names_failure(self, mock_set_by_cli):
@@ -68,7 +68,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase):
         self._call(self.config, renewalparams)
         expected = [challenges.TLSSNI01.typ,
                     challenges.HTTP01.typ, challenges.DNS01.typ]
-        self.assertEqual(self.config.namespace.pref_challs, expected)
+        self.assertEqual(self.config.pref_challs, expected)
 
     @mock.patch('certbot.renewal.cli.set_by_cli')
     def test_pref_challs_str(self, mock_set_by_cli):
@@ -76,7 +76,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase):
         renewalparams = {'pref_challs': 'dns'}
         self._call(self.config, renewalparams)
         expected = [challenges.DNS01.typ]
-        self.assertEqual(self.config.namespace.pref_challs, expected)
+        self.assertEqual(self.config.pref_challs, expected)
 
     @mock.patch('certbot.renewal.cli.set_by_cli')
     def test_pref_challs_failure(self, mock_set_by_cli):
@@ -88,7 +88,7 @@ class RestoreRequiredConfigElementsTest(unittest.TestCase):
     def test_must_staple_success(self, mock_set_by_cli):
         mock_set_by_cli.return_value = False
         self._call(self.config, {'must_staple': 'True'})
-        self.assertTrue(self.config.namespace.must_staple is True)
+        self.assertTrue(self.config.must_staple is True)
 
     @mock.patch('certbot.renewal.cli.set_by_cli')
     def test_must_staple_failure(self, mock_set_by_cli):