From e9f59f6fe2b4352b3ff72b8b552962ab4277ef74 Mon Sep 17 00:00:00 2001
From: Noah Swartz <noah@eff.org>
Date: Wed, 3 Feb 2016 13:51:41 -0800
Subject: [PATCH 01/36] add diff checks before each setattr

---
 letsencrypt/cli.py | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 1d4764835..fa9e77d94 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -53,7 +53,7 @@ _parser = None
 # file's renewalparams and actually used in the client configuration
 # during the renewal process. We have to record their types here because
 # the renewal configuration process loses this information.
-STR_CONFIG_ITEMS = ["config_dir", "log_dir", "work_dir", "user_agent",
+STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "server", "account", "authenticator", "installer",
                     "standalone_supported_challenges"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "tls_sni_01_port", "http01_port"]
@@ -730,6 +730,17 @@ def install(config, plugins):
     le_client.enhance_config(domains, config)
 
 
+def _diff_from_default(default_conf, cli_conf, value):
+    try:
+        default = default_conf.__getattr__(value)
+        cli = cli_conf.__getattr__(value)
+    except AttributeError:
+        return False
+    if cli != default:
+        return True
+    else:
+        return False
+
 def renew(cli_config, plugins):
     """Renew previously-obtained certificates."""
     cli_config = configuration.RenewerConfiguration(cli_config)
@@ -750,6 +761,8 @@ def renew(cli_config, plugins):
         #      each time?
         config = configuration.RenewerConfiguration(copy.deepcopy(cli_config))
         config.noninteractive_mode = True
+        default_args = prepare_and_parse_args(plugins, [])
+        default_conf = configuration.NamespaceConfig(default_args)
         full_path = os.path.join(configs_dir, renewal_file)
 
 
@@ -770,12 +783,13 @@ def renew(cli_config, plugins):
             continue
     # ?? config = configuration.NamespaceConfig(_AttrDict(renewalparams))
         # webroot_map is, uniquely, a dict
-        if "webroot_map" in renewalparams:
+        if "webroot_map" in renewalparams and not _diff_from_default(default_conf, cli_config, "webroot_map"):
             config.__setattr__("webroot_map", renewalparams["webroot_map"])
         # XXX: also need: nginx_, apache_, and plesk_ items
         # string-valued items to add if they're present
         for config_item in STR_CONFIG_ITEMS:
-            if config_item in renewalparams:
+            #TODO make sure that we don't lose passed command line args if they aren't in renewal params?
+            if config_item in renewalparams and not _diff_from_default(default_conf, cli_config, config_item):
                 value = renewalparams[config_item]
                 # Unfortunately, we've lost type information from ConfigObj,
                 # so we don't know if the original was NoneType or str!
@@ -784,7 +798,7 @@ def renew(cli_config, plugins):
                 config.__setattr__(config_item, value)
         # int-valued items to add if they're present
         for config_item in INT_CONFIG_ITEMS:
-            if config_item in renewalparams:
+            if config_item in renewalparams and not _diff_from_default(default_conf, cli_config, config_item):
                 try:
                     value = int(renewalparams[config_item])
                     config.__setattr__(config_item, value)
@@ -797,7 +811,7 @@ def renew(cli_config, plugins):
         # XXX: is it true that an item will end up in _parser._actions even
         #      when no action was explicitly specified?
         for plugin_prefix in EXTRACT_PLUGIN_PREFIXES:
-            for config_item in renewalparams.keys():
+            for config_item in renewalparams.keys() and not _diff_from_default(default_conf, cli_config, config_item):
                 if config_item.startswith(plugin_prefix):
                     for action in _parser.parser._actions:
                        if action.dest == config_item:

From 72aa72ec748f637ba61aeecc3e241bb8335896b9 Mon Sep 17 00:00:00 2001
From: Noah Swartz <noah@eff.org>
Date: Wed, 3 Feb 2016 14:07:25 -0800
Subject: [PATCH 02/36] move inside for loop and check domains as well

---
 letsencrypt/cli.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index fa9e77d94..2812b1592 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -811,8 +811,8 @@ def renew(cli_config, plugins):
         # XXX: is it true that an item will end up in _parser._actions even
         #      when no action was explicitly specified?
         for plugin_prefix in EXTRACT_PLUGIN_PREFIXES:
-            for config_item in renewalparams.keys() and not _diff_from_default(default_conf, cli_config, config_item):
-                if config_item.startswith(plugin_prefix):
+            for config_item in renewalparams.keys():
+                if config_item.startswith(plugin_prefix) and not _diff_from_default(default_conf, cli_config, config_item):
                     for action in _parser.parser._actions:
                        if action.dest == config_item:
                            if action.type is not None:
@@ -848,7 +848,8 @@ def renew(cli_config, plugins):
                            "invalid. Skipping.", full_path)
             continue
 
-        config.__setattr__("domains", domains)
+        if not _diff_from_default(default_conf, cli_config, "domains"):
+            config.__setattr__("domains", domains)
 
         print("Trying...")
         print(obtain_cert(config, plugins, renewal_candidate))

From 31ddf0a3d87954573df15e78725e2c2ae61e32d1 Mon Sep 17 00:00:00 2001
From: Noah Swartz <noah@eff.org>
Date: Wed, 3 Feb 2016 14:48:54 -0800
Subject: [PATCH 03/36] include alt confs

---
 letsencrypt/cli.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 5ec9b08f6..6833349fb 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -738,7 +738,7 @@ def _diff_from_default(default_conf, cli_conf, value):
     else:
         return False
 
-def _restore_required_config_elements(full_path, config, renewalparams):
+def _restore_required_config_elements(full_path, config, renewalparams, cli_config, default_conf):
     # string-valued items to add if they're present
     for config_item in STR_CONFIG_ITEMS:
         if config_item in renewalparams and not _diff_from_default(default_conf, cli_config, config_item):
@@ -790,7 +790,7 @@ def _restore_required_config_elements(full_path, config, renewalparams):
     return True
 
 
-def _reconstitute(full_path, config):
+def _reconstitute(full_path, config, cli_config):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
     This is specifically for use in renewal and enforces several checks
@@ -802,6 +802,8 @@ def _reconstitute(full_path, config):
     :rtype: `storage.RenewableCert` or NoneType
     """
 
+    default_args = prepare_and_parse_args(plugins, [])
+    default_conf = configuration.NamespaceConfig(default_args)
     try:
         renewal_candidate = storage.RenewableCert(full_path, config)
     except (errors.CertStorageError, IOError):
@@ -820,7 +822,7 @@ def _reconstitute(full_path, config):
     # Now restore specific values along with their data types, if
     # those elements are present.
     try:
-        _restore_required_config_elements(full_path, config, renewalparams)
+        _restore_required_config_elements(full_path, config, renewalparams, cli_config, default_conf)
     except ValueError:
         # There was a data type error which has already been
         # logged.
@@ -866,14 +868,12 @@ def renew(cli_config, plugins):
         #      each time?
         config = configuration.RenewerConfiguration(copy.deepcopy(cli_config))
         config.noninteractive_mode = True
-        default_args = prepare_and_parse_args(plugins, [])
-        default_conf = configuration.NamespaceConfig(default_args)
         full_path = os.path.join(configs_dir, renewal_file)
 
         # Note that this modifies config (to add back the configuration
         # elements from within the renewal configuration file).
         try:
-            renewal_candidate = _reconstitute(full_path, config)
+            renewal_candidate = _reconstitute(full_path, config, cli_config)
         except Exception as e:
             # reconstitute encountered an unanticipated problem.
             logger.warning("Renewal configuration file %s produced an "

From 50470c91ffeb0ce26965aaf08218fd8e42e8cefd Mon Sep 17 00:00:00 2001
From: Noah Swartz <noah@eff.org>
Date: Wed, 3 Feb 2016 15:04:31 -0800
Subject: [PATCH 04/36] make default_conf in renew

---
 letsencrypt/cli.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6833349fb..ffea8faa5 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -790,7 +790,7 @@ def _restore_required_config_elements(full_path, config, renewalparams, cli_conf
     return True
 
 
-def _reconstitute(full_path, config, cli_config):
+def _reconstitute(full_path, config, cli_config, default_conf):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
     This is specifically for use in renewal and enforces several checks
@@ -802,8 +802,6 @@ def _reconstitute(full_path, config, cli_config):
     :rtype: `storage.RenewableCert` or NoneType
     """
 
-    default_args = prepare_and_parse_args(plugins, [])
-    default_conf = configuration.NamespaceConfig(default_args)
     try:
         renewal_candidate = storage.RenewableCert(full_path, config)
     except (errors.CertStorageError, IOError):
@@ -868,6 +866,8 @@ def renew(cli_config, plugins):
         #      each time?
         config = configuration.RenewerConfiguration(copy.deepcopy(cli_config))
         config.noninteractive_mode = True
+        default_args = prepare_and_parse_args(plugins, [])
+        default_conf = configuration.NamespaceConfig(default_args)
         full_path = os.path.join(configs_dir, renewal_file)
 
         # Note that this modifies config (to add back the configuration

From cd1c04a4efb4b0cd0ea934da05645a79b96843e7 Mon Sep 17 00:00:00 2001
From: Noah Swartz <noah@eff.org>
Date: Wed, 3 Feb 2016 15:06:04 -0800
Subject: [PATCH 05/36] pass default_conf

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ffea8faa5..25b687886 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -873,7 +873,7 @@ def renew(cli_config, plugins):
         # Note that this modifies config (to add back the configuration
         # elements from within the renewal configuration file).
         try:
-            renewal_candidate = _reconstitute(full_path, config, cli_config)
+            renewal_candidate = _reconstitute(full_path, config, cli_config, default_conf)
         except Exception as e:
             # reconstitute encountered an unanticipated problem.
             logger.warning("Renewal configuration file %s produced an "

From 32e0faa7b6585303d8ce48476826f75dc0d6eee1 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 5 Feb 2016 16:35:55 -0800
Subject: [PATCH 06/36] Detect any setting of arguments as non-default

Even if the user set the argument to the default value.

This involves a hack (empty_defaults=True) where we shim all the arguments
so that their default values evaluate to false.  This hack may be buggy...
---
 letsencrypt/cli.py           | 88 ++++++++++++++++++++++++------------
 letsencrypt/configuration.py |  5 +-
 2 files changed, 62 insertions(+), 31 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ae09b45e0..cc3d110b3 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -727,18 +727,17 @@ def install(config, plugins):
     le_client.enhance_config(domains, config)
 
 
-def _diff_from_default(default_conf, cli_conf, value):
+def _diff_from_default(default_detector_conf, value):
     try:
-        default = default_conf.__getattr__(value)
-        cli = cli_conf.__getattr__(value)
+        if default_detector_conf.__getattr__(value):
+            return True
+        else:
+            return False
     except AttributeError:
-        return False
-    if cli != default:
-        return True
-    else:
+        print("Missing default", value)
         return False
 
-def _restore_required_config_elements(config, renewalparams, cli_config, default_conf):
+def _restore_required_config_elements(config, renewalparams, default_detector_conf):
     """Sets non-plugin specific values in config from renewalparams
 
     :param configuration.NamespaceConfig config: configuration for the
@@ -749,7 +748,8 @@ def _restore_required_config_elements(config, renewalparams, cli_config, default
     """
     # string-valued items to add if they're present
     for config_item in STR_CONFIG_ITEMS:
-        if config_item in renewalparams and not _diff_from_default(default_conf, cli_config, config_item):
+        if config_item in renewalparams and not _diff_from_default(default_detector_conf,
+                                                                   config_item):
             value = renewalparams[config_item]
             # Unfortunately, we've lost type information from ConfigObj,
             # so we don't know if the original was NoneType or str!
@@ -758,7 +758,8 @@ def _restore_required_config_elements(config, renewalparams, cli_config, default
             setattr(config.namespace, config_item, value)
     # int-valued items to add if they're present
     for config_item in INT_CONFIG_ITEMS:
-        if config_item in renewalparams and not _diff_from_default(default_conf, cli_config, config_item):
+        if config_item in renewalparams and not _diff_from_default(default_detector_conf,
+                                                                   config_item):
             try:
                 value = int(renewalparams[config_item])
                 setattr(config.namespace, config_item, value)
@@ -766,7 +767,7 @@ def _restore_required_config_elements(config, renewalparams, cli_config, default
                 raise errors.Error(
                     "Expected a numeric value for {0}".format(config_item))
 
-def _restore_plugin_configs(config, renewalparams):
+def _restore_plugin_configs(config, renewalparams, default_detector_conf):
     """Sets plugin specific values in config from renewalparams
 
     :param configuration.NamespaceConfig config: configuration for the
@@ -795,7 +796,8 @@ def _restore_plugin_configs(config, renewalparams):
                 # its value to None?
                 setattr(config.namespace, config_item, None)
                 continue
-            if config_item.startswith(plugin_prefix + "_") and not _diff_from_default(default_conf, cli_config, config_item):
+            if config_item.startswith(plugin_prefix + "_") and not _diff_from_default(
+                default_detector_conf, config_item):
                 for action in _parser.parser._actions: # pylint: disable=protected-access
                     if action.type is not None and action.dest == config_item:
                         setattr(config.namespace, config_item,
@@ -807,7 +809,7 @@ def _restore_plugin_configs(config, renewalparams):
     return True
 
 
-def _reconstitute(config, full_path, cli_config, default_conf):
+def _reconstitute(config, full_path, default_detector_conf):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
     This is specifically for use in renewal and enforces several checks
@@ -843,8 +845,8 @@ def _reconstitute(config, full_path, cli_config, default_conf):
     # Now restore specific values along with their data types, if
     # those elements are present.
     try:
-        _restore_required_config_elements(config, renewalparams, cli_config, default_conf)
-        _restore_plugin_configs(config, renewalparams)
+        _restore_required_config_elements(config, renewalparams, default_detector_conf)
+        _restore_plugin_configs(config, renewalparams, default_detector_conf)
     except (ValueError, errors.Error) as error:
         logger.warning(
             "An error occured while parsing %s. The error was %s. "
@@ -855,7 +857,8 @@ def _reconstitute(config, full_path, cli_config, default_conf):
     # webroot_map is, uniquely, a dict, and the general-purpose
     # configuration restoring logic is not able to correctly parse it
     # from the serialized form.
-    if "webroot_map" in renewalparams and not _diff_from_default(default_conf, cli_config, "webroot_map"):
+    if "webroot_map" in renewalparams and not _diff_from_default(default_detector_conf,
+                                                                 "webroot_map"):
         setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
 
     try:
@@ -867,7 +870,7 @@ def _reconstitute(config, full_path, cli_config, default_conf):
                        "invalid. Skipping.", full_path)
         return None
 
-    if not _diff_from_default(default_conf, cli_config, "domains"):
+    if not _diff_from_default(default_detector_conf, "domains"):
         setattr(config.namespace, "domains", domains)
 
     return renewal_candidate
@@ -877,8 +880,12 @@ def _renewal_conf_files(config):
     return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
 
 
-def renew(config, unused_plugins):
+def renew(config, plugins):
     """Renew previously-obtained certificates."""
+
+    default_args = prepare_and_parse_args(plugins, sys.argv[1:], empty_defaults=True)
+    default_detector_conf = configuration.NamespaceConfig(default_args, fake=True)
+
     if config.domains != []:
         raise errors.Error("Currently, the renew verb is only capable of "
                            "renewing all installed certificates that are due "
@@ -892,20 +899,20 @@ def renew(config, unused_plugins):
                            "specifying a CSR file. Please try the certonly "
                            "command instead.")
     renewer_config = configuration.RenewerConfiguration(config)
+
     for renewal_file in _renewal_conf_files(renewer_config):
         if not renewal_file.endswith(".conf"):
             continue
         print("Processing " + renewal_file)
         # XXX: does this succeed in making a fully independent config object
         #      each time?
-        default_args = prepare_and_parse_args(plugins, [])
-        default_conf = configuration.NamespaceConfig(default_args)
         lineage_config = copy.deepcopy(config)
 
         # Note that this modifies config (to add back the configuration
         # elements from within the renewal configuration file).
         try:
-            renewal_candidate = _reconstitute(lineage_config, renewal_file, config, default_conf)
+            renewal_candidate = _reconstitute(lineage_config, renewal_file,
+                                              default_detector_conf)
         except Exception as e: # pylint: disable=broad-except
             # reconstitute encountered an unanticipated problem.
             logger.warning("Renewal configuration file %s produced an "
@@ -1053,7 +1060,7 @@ class HelpfulArgumentParser(object):
     HELP_TOPICS = ["all", "security",
                    "paths", "automation", "testing"] + VERBS.keys()
 
-    def __init__(self, args, plugins):
+    def __init__(self, args, plugins, empty_defaults=False):
         plugin_names = [name for name, _p in plugins.iteritems()]
         self.help_topics = self.HELP_TOPICS + plugin_names + [None]
         usage, short_usage = usage_strings(plugins)
@@ -1067,6 +1074,11 @@ class HelpfulArgumentParser(object):
         self.parser._add_config_file_help = False  # pylint: disable=protected-access
         self.silent_parser = SilentParser(self.parser)
 
+        # This setting attempts to force all default values to None; it
+        # is used to detect when values have been explicitly set by the user,
+        # including when they are set to their normal default value
+        self.empty_defaults = empty_defaults
+
         self.args = args
         self.determine_verb()
         help1 = self.prescan_for_flag("-h", self.help_topics)
@@ -1100,19 +1112,19 @@ class HelpfulArgumentParser(object):
                     parsed_args.domains.append(domain)
 
         if parsed_args.staging or parsed_args.dry_run:
-            if (parsed_args.server not in
-                    (flag_default("server"), constants.STAGING_URI)):
+            if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
                 conflicts = ["--staging"] if parsed_args.staging else []
                 conflicts += ["--dry-run"] if parsed_args.dry_run else []
-                raise errors.Error("--server value conflicts with {0}".format(
-                    " and ".join(conflicts)))
+                if not self.empty_defaults:
+                    raise errors.Error("--server value conflicts with {0}".format(
+                        " and ".join(conflicts)))
 
             parsed_args.server = constants.STAGING_URI
 
             if parsed_args.dry_run:
                 if self.verb not in ["certonly", "renew"]:
                     raise errors.Error("--dry-run currently only works with the "
-                                       "'certonly' or 'renew' subcommands")
+                                       "'certonly' or 'renew' subcommands (%r)" % self.verb)
                 parsed_args.break_my_certs = parsed_args.staging = True
 
         return parsed_args
@@ -1169,6 +1181,24 @@ class HelpfulArgumentParser(object):
         it, but can be None for `always documented'.
 
         """
+
+        if self.empty_defaults:
+            # These are config elements which cannot tolerate being set to ""
+            # during parsing; that's fine as long as their defaults evalute to
+            # boolean false.
+            if not any(exception in args for exception in ["--webroot-map", "-d", "-w", "-v"]):
+                if kwargs.get("type", None) == int:
+                    kwargs["default"] = 0
+                elif "--csr" in args:
+                    kwargs["default"] = ""
+                    kwargs["type"] = str
+                else:
+                    kwargs["default"] = ""
+                #logger.info("Munging %r %r", args, "-v" in args)
+            else:
+                #logger.info("Not munging default for %r", args)
+                pass
+
         if self.visible_topics[topic]:
             if topic in self.groups:
                 group = self.groups[topic]
@@ -1246,7 +1276,7 @@ class HelpfulArgumentParser(object):
             return dict([(t, t == chosen_topic) for t in self.help_topics])
 
 
-def prepare_and_parse_args(plugins, args):
+def prepare_and_parse_args(plugins, args, empty_defaults=False):
     """Returns parsed command line arguments.
 
     :param .PluginsRegistry plugins: available plugins
@@ -1256,7 +1286,7 @@ def prepare_and_parse_args(plugins, args):
     :rtype: argparse.Namespace
 
     """
-    helpful = HelpfulArgumentParser(args, plugins)
+    helpful = HelpfulArgumentParser(args, plugins, empty_defaults)
 
     # --help is automatically provided by argparse
     helpful.add(
diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py
index 2bbf1b019..979d5e985 100644
--- a/letsencrypt/configuration.py
+++ b/letsencrypt/configuration.py
@@ -34,7 +34,7 @@ class NamespaceConfig(object):
     """
     zope.interface.implements(interfaces.IConfig)
 
-    def __init__(self, namespace):
+    def __init__(self, namespace, fake=False):
         self.namespace = namespace
 
         self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
@@ -42,7 +42,8 @@ class NamespaceConfig(object):
         self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir)
 
         # Check command line parameters sanity, and error out in case of problem.
-        check_config_sanity(self)
+        if not fake:
+            check_config_sanity(self)
 
     def __getattr__(self, name):
         return getattr(self.namespace, name)

From fd76b32aed364b365f851877a0071b8bda0262c3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 5 Feb 2016 16:42:41 -0800
Subject: [PATCH 07/36] Slightly better renewal debuggery

---
 letsencrypt/tests/cli_test.py | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py
index a5757399e..b893bc85a 100644
--- a/letsencrypt/tests/cli_test.py
+++ b/letsencrypt/tests/cli_test.py
@@ -542,24 +542,24 @@ class CLITest(unittest.TestCase):  # pylint: disable=too-many-public-methods
         mock_client = mock.MagicMock()
         mock_client.obtain_certificate.return_value = (mock_certr, 'chain',
                                                        mock_key, 'csr')
-        with mock.patch('letsencrypt.cli._find_duplicative_certs') as mock_fdc:
-            mock_fdc.return_value = (mock_lineage, None)
-            with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
-                mock_init.return_value = mock_client
-                get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
-                with mock.patch(get_utility_path) as mock_get_utility:
-                    with mock.patch('letsencrypt.cli.OpenSSL') as mock_ssl:
-                        mock_latest = mock.MagicMock()
-                        mock_latest.get_issuer.return_value = "Fake fake"
-                        mock_ssl.crypto.load_certificate.return_value = mock_latest
-                        with mock.patch('letsencrypt.cli.crypto_util'):
-                            if not args:
-                                args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly']
-                            if extra_args:
-                                args += extra_args
-                            self._call(args)
-
         try:
+            with mock.patch('letsencrypt.cli._find_duplicative_certs') as mock_fdc:
+                mock_fdc.return_value = (mock_lineage, None)
+                with mock.patch('letsencrypt.cli._init_le_client') as mock_init:
+                    mock_init.return_value = mock_client
+                    get_utility_path = 'letsencrypt.cli.zope.component.getUtility'
+                    with mock.patch(get_utility_path) as mock_get_utility:
+                        with mock.patch('letsencrypt.cli.OpenSSL') as mock_ssl:
+                            mock_latest = mock.MagicMock()
+                            mock_latest.get_issuer.return_value = "Fake fake"
+                            mock_ssl.crypto.load_certificate.return_value = mock_latest
+                            with mock.patch('letsencrypt.cli.crypto_util'):
+                                if not args:
+                                    args = ['-d', 'isnot.org', '-a', 'standalone', 'certonly']
+                                if extra_args:
+                                    args += extra_args
+                                self._call(args)
+
             if log_out:
                 with open(os.path.join(self.logs_dir, "letsencrypt.log")) as lf:
                     self.assertTrue(log_out in lf.read())

From 7906b31f55d91bde8f8f2a665f8491652883c5ba Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Fri, 5 Feb 2016 18:25:38 -0800
Subject: [PATCH 08/36] Cleanup and refactor a little

---
 letsencrypt/cli.py | 57 +++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6bec3808d..ee2001707 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -46,8 +46,7 @@ from letsencrypt.plugins import disco as plugins_disco
 
 logger = logging.getLogger(__name__)
 
-# This is global scope in order to be able to extract type information from
-# it later
+# Global, to save us from a lot of argument passing within the scope of this module
 _parser = None
 
 # These are the items which get pulled out of a renewal configuration
@@ -733,17 +732,31 @@ def install(config, plugins):
     le_client.enhance_config(domains, config)
 
 
-def _diff_from_default(default_detector_conf, value):
+def _set_by_cli(variable):
+    """
+    Return True if a particular config variable has been set by the user
+    (CLI or config file) including if the user explicitly set it to the
+    default.  Returns False if the variable was assigned a default variable.
+    """
+    if _set_by_cli.detector is None:
+        # Setup on first run: `detector` is a weird version of config in which
+        # the default value of every attribute is wrangled to be boolean-false
+        plugins = plugins_disco.PluginsRegistry.find_all()
+        default_args = prepare_and_parse_args(plugins, sys.argv[1:], empty_defaults=True)
+        _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
     try:
-        if default_detector_conf.__getattr__(value):
+        # Is detector.variable something that isn't false?
+        if _set_by_cli.detector.__getattr__(variable):
             return True
         else:
             return False
     except AttributeError:
-        print("Missing default", value)
+        logger.warning("Missing default analysis for %r", variable)
         return False
+# static housekeeping variable
+_set_by_cli.detector = None
 
-def _restore_required_config_elements(config, renewalparams, default_detector_conf):
+def _restore_required_config_elements(config, renewalparams):
     """Sets non-plugin specific values in config from renewalparams
 
     :param configuration.NamespaceConfig config: configuration for the
@@ -754,8 +767,7 @@ def _restore_required_config_elements(config, renewalparams, default_detector_co
     """
     # string-valued items to add if they're present
     for config_item in STR_CONFIG_ITEMS:
-        if config_item in renewalparams and not _diff_from_default(default_detector_conf,
-                                                                   config_item):
+        if config_item in renewalparams and not _set_by_cli(config_item):
             value = renewalparams[config_item]
             # Unfortunately, we've lost type information from ConfigObj,
             # so we don't know if the original was NoneType or str!
@@ -764,8 +776,7 @@ def _restore_required_config_elements(config, renewalparams, default_detector_co
             setattr(config.namespace, config_item, value)
     # int-valued items to add if they're present
     for config_item in INT_CONFIG_ITEMS:
-        if config_item in renewalparams and not _diff_from_default(default_detector_conf,
-                                                                   config_item):
+        if config_item in renewalparams and not _set_by_cli(config_item):
             try:
                 value = int(renewalparams[config_item])
                 setattr(config.namespace, config_item, value)
@@ -773,7 +784,7 @@ def _restore_required_config_elements(config, renewalparams, default_detector_co
                 raise errors.Error(
                     "Expected a numeric value for {0}".format(config_item))
 
-def _restore_plugin_configs(config, renewalparams, default_detector_conf):
+def _restore_plugin_configs(config, renewalparams):
     """Sets plugin specific values in config from renewalparams
 
     :param configuration.NamespaceConfig config: configuration for the
@@ -797,8 +808,7 @@ def _restore_plugin_configs(config, renewalparams, default_detector_conf):
         plugin_prefixes.append(renewalparams["installer"])
     for plugin_prefix in set(plugin_prefixes):
         for config_item, config_value in renewalparams.iteritems():
-            if config_item.startswith(plugin_prefix + "_") and not _diff_from_default(
-                    default_detector_conf, config_item):
+            if config_item.startswith(plugin_prefix + "_") and not _set_by_cli(config_item):
                 # Avoid confusion when, for example, "csr = None" (avoid
                 # trying to read the file called "None")
                 # Should we omit the item entirely rather than setting
@@ -816,7 +826,7 @@ def _restore_plugin_configs(config, renewalparams, default_detector_conf):
                     setattr(config.namespace, config_item, str(config_value))
 
 
-def _reconstitute(config, full_path, default_detector_conf):
+def _reconstitute(config, full_path):
     """Try to instantiate a RenewableCert, updating config with relevant items.
 
     This is specifically for use in renewal and enforces several checks
@@ -852,8 +862,8 @@ def _reconstitute(config, full_path, default_detector_conf):
     # Now restore specific values along with their data types, if
     # those elements are present.
     try:
-        _restore_required_config_elements(config, renewalparams, default_detector_conf)
-        _restore_plugin_configs(config, renewalparams, default_detector_conf)
+        _restore_required_config_elements(config, renewalparams)
+        _restore_plugin_configs(config, renewalparams)
     except (ValueError, errors.Error) as error:
         logger.warning(
             "An error occured while parsing %s. The error was %s. "
@@ -864,8 +874,7 @@ def _reconstitute(config, full_path, default_detector_conf):
     # webroot_map is, uniquely, a dict, and the general-purpose
     # configuration restoring logic is not able to correctly parse it
     # from the serialized form.
-    if "webroot_map" in renewalparams and not _diff_from_default(default_detector_conf,
-                                                                 "webroot_map"):
+    if "webroot_map" in renewalparams and not _set_by_cli("webroot_map"):
         setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
 
     try:
@@ -877,7 +886,7 @@ def _reconstitute(config, full_path, default_detector_conf):
                        "invalid. Skipping.", full_path)
         return None
 
-    if not _diff_from_default(default_detector_conf, "domains"):
+    if not _set_by_cli("domains"):
         setattr(config.namespace, "domains", domains)
 
     return renewal_candidate
@@ -887,12 +896,9 @@ def _renewal_conf_files(config):
     return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
 
 
-def renew(config, plugins):
+def renew(config, unused_plugins):
     """Renew previously-obtained certificates."""
 
-    default_args = prepare_and_parse_args(plugins, sys.argv[1:], empty_defaults=True)
-    default_detector_conf = configuration.NamespaceConfig(default_args, fake=True)
-
     if config.domains != []:
         raise errors.Error("Currently, the renew verb is only capable of "
                            "renewing all installed certificates that are due "
@@ -916,8 +922,7 @@ def renew(config, plugins):
         # Note that this modifies config (to add back the configuration
         # elements from within the renewal configuration file).
         try:
-            renewal_candidate = _reconstitute(lineage_config, renewal_file,
-                                              default_detector_conf)
+            renewal_candidate = _reconstitute(lineage_config, renewal_file)
         except Exception as e: # pylint: disable=broad-except
             # reconstitute encountered an unanticipated problem.
             logger.warning("Renewal configuration file %s produced an "
@@ -1752,9 +1757,9 @@ def _handle_exception(exc_type, exc_value, trace, config):
 def main(cli_args=sys.argv[1:]):
     """Command line argument parsing and main script execution."""
     sys.excepthook = functools.partial(_handle_exception, config=None)
+    plugins = plugins_disco.PluginsRegistry.find_all()
 
     # note: arg parser internally handles --help (and exits afterwards)
-    plugins = plugins_disco.PluginsRegistry.find_all()
     args = prepare_and_parse_args(plugins, cli_args)
     config = configuration.NamespaceConfig(args)
     zope.component.provideUtility(config)

From c9df10a87e0c85a3c2cc4f8e63266afa66679351 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 7 Feb 2016 19:04:43 -0800
Subject: [PATCH 09/36] Debugging / work in progress

---
 letsencrypt/cli.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index ee2001707..c8a458c7f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1204,9 +1204,12 @@ class HelpfulArgumentParser(object):
             # during parsing; that's fine as long as their defaults evalute to
             # boolean false.
             if not any(exception in args for exception in ["--webroot-map", "-d", "-w", "-v"]):
-                if kwargs.get("type", None) == int:
+                arg_type = kwargs.get("type", None)
+                if arg_type == int:
                     kwargs["default"] = 0
-                elif "--csr" in args:
+                elif arg_type == read_file or "-c" in args:
+                    #if "-c" in args:
+                    #    raise TypeError("Skipping %r " % args)
                     kwargs["default"] = ""
                     kwargs["type"] = str
                 else:

From 317557086c19b376289b97723e52ba743dc802c2 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 7 Feb 2016 20:08:32 -0800
Subject: [PATCH 10/36] sys.argv[1:] does not work in tests

---
 letsencrypt/cli.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index c8a458c7f..32dd3ceca 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -742,7 +742,9 @@ def _set_by_cli(variable):
         # Setup on first run: `detector` is a weird version of config in which
         # the default value of every attribute is wrangled to be boolean-false
         plugins = plugins_disco.PluginsRegistry.find_all()
-        default_args = prepare_and_parse_args(plugins, sys.argv[1:], empty_defaults=True)
+        # reconstructed_args == sys.argv[1:], or whatever was passed to main()
+        reconstructed_args = _parser.args + [_parser.verb]
+        default_args = prepare_and_parse_args(plugins, reconstructed_args, empty_defaults=True)
         _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
     try:
         # Is detector.variable something that isn't false?

From d2ea078422fe25dece972415df9382311469ba20 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Sun, 7 Feb 2016 20:12:01 -0800
Subject: [PATCH 11/36] Remove some commented debugging statements

---
 letsencrypt/cli.py | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 32dd3ceca..47a8f7dc0 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1210,15 +1210,11 @@ class HelpfulArgumentParser(object):
                 if arg_type == int:
                     kwargs["default"] = 0
                 elif arg_type == read_file or "-c" in args:
-                    #if "-c" in args:
-                    #    raise TypeError("Skipping %r " % args)
                     kwargs["default"] = ""
                     kwargs["type"] = str
                 else:
                     kwargs["default"] = ""
-                #logger.info("Munging %r %r", args, "-v" in args)
             else:
-                #logger.info("Not munging default for %r", args)
                 pass
 
         if self.visible_topics[topic]:

From edb07aeecb51b23ba45a6cf3f92c38a4f468667c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 10:24:09 -0800
Subject: [PATCH 12/36] Tweak changelog

(but really, provoke a re-run of travis)
---
 CHANGES.rst | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 3ed13041b..55e4bd796 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,23 +5,7 @@ Please note:
 the change log will only get updated after first release - for now please use the
 `commit log <https://github.com/letsencrypt/letsencrypt/commits/master>`_.
 
+To see the changes in a given release, inspect the github milestone for the
+release.  For instance:
 
-Release 0.1.0 (not released yet)
---------------------------------
-
-New Features:
-
-* ...
-
-Fixes:
-
-* ...
-
-Other changes:
-
-* ...
-
-Release 0.0.0 (not released yet)
---------------------------------
-
-Initial release.
+https://github.com/letsencrypt/letsencrypt/issues?utf8=%E2%9C%93&q=milestone%3A0.3.0

From c5c72de95932aeae2b6517314875570b7ab881b3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 15:09:47 -0800
Subject: [PATCH 13/36] Cleanup default detection a little, and handle an extra
 weird case

  - Noah spotted a theoretical issue with store_false args, so handle that
    prospectively
  - overall probably not really making anything neater
---
 letsencrypt/cli.py | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 47a8f7dc0..6390a3b1c 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -732,11 +732,11 @@ def install(config, plugins):
     le_client.enhance_config(domains, config)
 
 
-def _set_by_cli(variable):
+def _set_by_cli(var):
     """
     Return True if a particular config variable has been set by the user
     (CLI or config file) including if the user explicitly set it to the
-    default.  Returns False if the variable was assigned a default variable.
+    default.  Returns False if the variable was assigned a default value.
     """
     if _set_by_cli.detector is None:
         # Setup on first run: `detector` is a weird version of config in which
@@ -747,15 +747,20 @@ def _set_by_cli(variable):
         default_args = prepare_and_parse_args(plugins, reconstructed_args, empty_defaults=True)
         _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
     try:
-        # Is detector.variable something that isn't false?
-        if _set_by_cli.detector.__getattr__(variable):
+        # Is detector.var something that isn't false?
+        change_detected = _set_by_cli.detector.__getattr__(var)
+        if change_detected:
+            return True
+        # Special case: like --no-redirect vars that get set True -> False are
+        # will be None here
+        elif var in _set_by_cli.detector.store_false_vars and change_detected == False:
             return True
         else:
             return False
     except AttributeError:
-        logger.warning("Missing default analysis for %r", variable)
+        logger.warning("Missing default analysis for %r", var)
         return False
-# static housekeeping variable
+# static housekeeping var
 _set_by_cli.detector = None
 
 def _restore_required_config_elements(config, renewalparams):
@@ -1097,6 +1102,8 @@ class HelpfulArgumentParser(object):
         # is used to detect when values have been explicitly set by the user,
         # including when they are set to their normal default value
         self.empty_defaults = empty_defaults
+        if empty_defaults:
+            self.store_false_vars = {}  # vars that use "store_false"
 
         self.args = args
         self.determine_verb()
@@ -1109,7 +1116,7 @@ class HelpfulArgumentParser(object):
             print(usage)
             sys.exit(0)
         self.visible_topics = self.determine_help_topics(self.help_arg)
-        self.groups = {}  # elements are added by .add_group()
+        self.groups = {}       # elements are added by .add_group()
 
     def parse_args(self):
         """Parses command line arguments and returns the result.
@@ -1205,17 +1212,25 @@ class HelpfulArgumentParser(object):
             # These are config elements which cannot tolerate being set to ""
             # during parsing; that's fine as long as their defaults evalute to
             # boolean false.
-            if not any(exception in args for exception in ["--webroot-map", "-d", "-w", "-v"]):
+
+            # argument either doesn't have a default, or the default doesn't
+            # isn't Pythonically false
+            if kwargs.get("default", True):
                 arg_type = kwargs.get("type", None)
-                if arg_type == int:
+                if arg_type == int or kwargs.get("action", "") == "count":
                     kwargs["default"] = 0
                 elif arg_type == read_file or "-c" in args:
                     kwargs["default"] = ""
                     kwargs["type"] = str
                 else:
                     kwargs["default"] = ""
-            else:
-                pass
+                # This doesn't matter at present (none of the store_false args
+                # are renewal-relevant), but implement it for future sanity:
+                # detect the setting of args whose presence causes True -> False
+            elif kwargs.get("action", "") == "store_false":
+                kwargs["default"] = None
+                for var in args:
+                    self.store_false_vars[var] = True
 
         if self.visible_topics[topic]:
             if topic in self.groups:

From de43a4b0520c61b2c8cf167a91e91da846517590 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 15:20:04 -0800
Subject: [PATCH 14/36] Split default detection into its own method

---
 letsencrypt/cli.py | 80 ++++++++++++++++++++++++++--------------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6390a3b1c..9e9b8fa52 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -744,16 +744,16 @@ def _set_by_cli(var):
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = _parser.args + [_parser.verb]
-        default_args = prepare_and_parse_args(plugins, reconstructed_args, empty_defaults=True)
+        default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
         _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
     try:
         # Is detector.var something that isn't false?
         change_detected = _set_by_cli.detector.__getattr__(var)
         if change_detected:
             return True
-        # Special case: like --no-redirect vars that get set True -> False are
-        # will be None here
-        elif var in _set_by_cli.detector.store_false_vars and change_detected == False:
+        # Special case: vars like --no-redirect that get set True -> False
+        # default to None; False means they were set
+        elif var in _set_by_cli.detector.store_false_vars and change_detected is not None:
             return True
         else:
             return False
@@ -1084,7 +1084,7 @@ class HelpfulArgumentParser(object):
     HELP_TOPICS = ["all", "security",
                    "paths", "automation", "testing"] + VERBS.keys()
 
-    def __init__(self, args, plugins, empty_defaults=False):
+    def __init__(self, args, plugins, detect_defaults=False):
         plugin_names = [name for name, _p in plugins.iteritems()]
         self.help_topics = self.HELP_TOPICS + plugin_names + [None]
         usage, short_usage = usage_strings(plugins)
@@ -1101,8 +1101,8 @@ class HelpfulArgumentParser(object):
         # This setting attempts to force all default values to None; it
         # is used to detect when values have been explicitly set by the user,
         # including when they are set to their normal default value
-        self.empty_defaults = empty_defaults
-        if empty_defaults:
+        self.detect_defaults = detect_defaults
+        if detect_defaults:
             self.store_false_vars = {}  # vars that use "store_false"
 
         self.args = args
@@ -1141,7 +1141,7 @@ class HelpfulArgumentParser(object):
             if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
                 conflicts = ["--staging"] if parsed_args.staging else []
                 conflicts += ["--dry-run"] if parsed_args.dry_run else []
-                if not self.empty_defaults:
+                if not self.detect_defaults:
                     raise errors.Error("--server value conflicts with {0}".format(
                         " and ".join(conflicts)))
 
@@ -1203,34 +1203,15 @@ class HelpfulArgumentParser(object):
     def add(self, topic, *args, **kwargs):
         """Add a new command line argument.
 
-        @topic is required, to indicate which part of the help will document
-        it, but can be None for `always documented'.
+        :param str: help topic this should be listed under, can be None for
+                    "always documented"
+        :param list *args: the names of this argument flag
+        :param dict **kwargs: various argparse settings for this argument
 
         """
 
-        if self.empty_defaults:
-            # These are config elements which cannot tolerate being set to ""
-            # during parsing; that's fine as long as their defaults evalute to
-            # boolean false.
-
-            # argument either doesn't have a default, or the default doesn't
-            # isn't Pythonically false
-            if kwargs.get("default", True):
-                arg_type = kwargs.get("type", None)
-                if arg_type == int or kwargs.get("action", "") == "count":
-                    kwargs["default"] = 0
-                elif arg_type == read_file or "-c" in args:
-                    kwargs["default"] = ""
-                    kwargs["type"] = str
-                else:
-                    kwargs["default"] = ""
-                # This doesn't matter at present (none of the store_false args
-                # are renewal-relevant), but implement it for future sanity:
-                # detect the setting of args whose presence causes True -> False
-            elif kwargs.get("action", "") == "store_false":
-                kwargs["default"] = None
-                for var in args:
-                    self.store_false_vars[var] = True
+        if self.detect_defaults:
+            self.modify_arg_for_default_detection(self, *args, **kwargs)
 
         if self.visible_topics[topic]:
             if topic in self.groups:
@@ -1242,6 +1223,35 @@ class HelpfulArgumentParser(object):
             kwargs["help"] = argparse.SUPPRESS
             self.parser.add_argument(*args, **kwargs)
 
+
+    def modify_arg_for_default_detection(self, *args, **kwargs):
+        """
+        Adding an arg, but ensure that it has a default that evaluates to false,
+        so that _set_by_cli can tell if it was set.  Only called if detect_defaults==True.
+
+        :param list *args: the names of this argument flag
+        :param dict **kwargs: various argparse settings for this argument
+        """
+        # argument either doesn't have a default, or the default doesn't
+        # isn't Pythonically false
+        if kwargs.get("default", True):
+            arg_type = kwargs.get("type", None)
+            if arg_type == int or kwargs.get("action", "") == "count":
+                kwargs["default"] = 0
+            elif arg_type == read_file or "-c" in args:
+                kwargs["default"] = ""
+                kwargs["type"] = str
+            else:
+                kwargs["default"] = ""
+            # This doesn't matter at present (none of the store_false args
+            # are renewal-relevant), but implement it for future sanity:
+            # detect the setting of args whose presence causes True -> False
+        elif kwargs.get("action", "") == "store_false":
+            kwargs["default"] = None
+            for var in args:
+                self.store_false_vars[var] = True
+
+
     def add_deprecated_argument(self, argument_name, num_args):
         """Adds a deprecated argument with the name argument_name.
 
@@ -1309,7 +1319,7 @@ class HelpfulArgumentParser(object):
             return dict([(t, t == chosen_topic) for t in self.help_topics])
 
 
-def prepare_and_parse_args(plugins, args, empty_defaults=False):
+def prepare_and_parse_args(plugins, args, detect_defaults=False):
     """Returns parsed command line arguments.
 
     :param .PluginsRegistry plugins: available plugins
@@ -1319,7 +1329,7 @@ def prepare_and_parse_args(plugins, args, empty_defaults=False):
     :rtype: argparse.Namespace
 
     """
-    helpful = HelpfulArgumentParser(args, plugins, empty_defaults)
+    helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
 
     # --help is automatically provided by argparse
     helpful.add(

From b61bfdc468d5cc31583241b8aa72bf9ee9d83c9b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 16:18:30 -0800
Subject: [PATCH 15/36] Don't set default="" for store_false args...

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9e9b8fa52..43a6ed1ab 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1246,7 +1246,7 @@ class HelpfulArgumentParser(object):
             # This doesn't matter at present (none of the store_false args
             # are renewal-relevant), but implement it for future sanity:
             # detect the setting of args whose presence causes True -> False
-        elif kwargs.get("action", "") == "store_false":
+        if kwargs.get("action", "") == "store_false":
             kwargs["default"] = None
             for var in args:
                 self.store_false_vars[var] = True

From 0946ea8e046133abcc602bb35f818b76636054f7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 16:42:13 -0800
Subject: [PATCH 16/36] Bleh.

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 43a6ed1ab..0b5c2bdfd 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -753,7 +753,7 @@ def _set_by_cli(var):
             return True
         # Special case: vars like --no-redirect that get set True -> False
         # default to None; False means they were set
-        elif var in _set_by_cli.detector.store_false_vars and change_detected is not None:
+        elif var in _set_by_cli.detector.namespace.store_false_vars and change_detected is not None:
             return True
         else:
             return False

From 994c96180a0984133834aa083666332e0e236011 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 16:48:21 -0800
Subject: [PATCH 17/36] Don't catch the wrong exception by accident

---
 letsencrypt/cli.py | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 0b5c2bdfd..12aa69b46 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -746,20 +746,22 @@ def _set_by_cli(var):
         reconstructed_args = _parser.args + [_parser.verb]
         default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
         _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
+
     try:
         # Is detector.var something that isn't false?
         change_detected = _set_by_cli.detector.__getattr__(var)
-        if change_detected:
-            return True
-        # Special case: vars like --no-redirect that get set True -> False
-        # default to None; False means they were set
-        elif var in _set_by_cli.detector.namespace.store_false_vars and change_detected is not None:
-            return True
-        else:
-            return False
     except AttributeError:
         logger.warning("Missing default analysis for %r", var)
         return False
+
+    if change_detected:
+        return True
+    # Special case: vars like --no-redirect that get set True -> False
+    # default to None; False means they were set
+    elif var in _set_by_cli.detector.namespace.store_false_vars and change_detected is not None:
+        return True
+    else:
+        return False
 # static housekeeping var
 _set_by_cli.detector = None
 

From bcf38476faa9bf3c35fa24df81d2f9f732a2ee79 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 16:59:11 -0800
Subject: [PATCH 18/36] Fix bugs, clean up plumbing.

---
 letsencrypt/cli.py           | 14 +++++++++-----
 letsencrypt/configuration.py |  5 ++---
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 12aa69b46..79a27a200 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -744,8 +744,8 @@ def _set_by_cli(var):
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = _parser.args + [_parser.verb]
-        default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
-        _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
+        _set_by_cli.detector = prepare_and_parse_args(plugins, reconstructed_args,
+                                                      detect_defaults=True)
 
     try:
         # Is detector.var something that isn't false?
@@ -758,7 +758,7 @@ def _set_by_cli(var):
         return True
     # Special case: vars like --no-redirect that get set True -> False
     # default to None; False means they were set
-    elif var in _set_by_cli.detector.namespace.store_false_vars and change_detected is not None:
+    elif var in _set_by_cli.detector.store_false_vars and change_detected is not None:
         return True
     else:
         return False
@@ -1154,6 +1154,9 @@ class HelpfulArgumentParser(object):
                     raise errors.Error("--dry-run currently only works with the "
                                        "'certonly' or 'renew' subcommands (%r)" % self.verb)
                 parsed_args.break_my_certs = parsed_args.staging = True
+        
+        if self.detect_defaults:  # plumbing
+            parsed_args.store_false_vars = self.store_false_vars
 
         return parsed_args
 
@@ -1476,8 +1479,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
     # parser (--help should display plugin-specific options last)
     _plugins_parsing(helpful, plugins)
 
-    global _parser # pylint: disable=global-statement
-    _parser = helpful
+    if not detect_defaults:
+        global _parser # pylint: disable=global-statement
+        _parser = helpful
     return helpful.parse_args()
 
 
diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py
index 979d5e985..2bbf1b019 100644
--- a/letsencrypt/configuration.py
+++ b/letsencrypt/configuration.py
@@ -34,7 +34,7 @@ class NamespaceConfig(object):
     """
     zope.interface.implements(interfaces.IConfig)
 
-    def __init__(self, namespace, fake=False):
+    def __init__(self, namespace):
         self.namespace = namespace
 
         self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
@@ -42,8 +42,7 @@ class NamespaceConfig(object):
         self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir)
 
         # Check command line parameters sanity, and error out in case of problem.
-        if not fake:
-            check_config_sanity(self)
+        check_config_sanity(self)
 
     def __getattr__(self, name):
         return getattr(self.namespace, name)

From 0af5b4b0a9f64f1a8190e4a82046a6ff990bee11 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Mon, 8 Feb 2016 17:07:39 -0800
Subject: [PATCH 19/36] arghlint

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9c9bf912a..35211c0bd 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1154,7 +1154,7 @@ class HelpfulArgumentParser(object):
                     raise errors.Error("--dry-run currently only works with the "
                                        "'certonly' or 'renew' subcommands (%r)" % self.verb)
                 parsed_args.break_my_certs = parsed_args.staging = True
-        
+
         if self.detect_defaults:  # plumbing
             parsed_args.store_false_vars = self.store_false_vars
 

From 3603f482e5fe81cb5948699a77a2abaa16a8839d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:06:59 -0800
Subject: [PATCH 20/36] Resume using a fully-constructed config namespace

---
 letsencrypt/cli.py           | 4 ++--
 letsencrypt/configuration.py | 5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 35211c0bd..a385f5e05 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -744,8 +744,8 @@ def _set_by_cli(var):
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = _parser.args + [_parser.verb]
-        _set_by_cli.detector = prepare_and_parse_args(plugins, reconstructed_args,
-                                                      detect_defaults=True)
+        default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
+        _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
 
     try:
         # Is detector.var something that isn't false?
diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py
index 2bbf1b019..979d5e985 100644
--- a/letsencrypt/configuration.py
+++ b/letsencrypt/configuration.py
@@ -34,7 +34,7 @@ class NamespaceConfig(object):
     """
     zope.interface.implements(interfaces.IConfig)
 
-    def __init__(self, namespace):
+    def __init__(self, namespace, fake=False):
         self.namespace = namespace
 
         self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
@@ -42,7 +42,8 @@ class NamespaceConfig(object):
         self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir)
 
         # Check command line parameters sanity, and error out in case of problem.
-        check_config_sanity(self)
+        if not fake:
+            check_config_sanity(self)
 
     def __getattr__(self, name):
         return getattr(self.namespace, name)

From 60392cce04cf25989e4dd5acb8633278650e6e5b Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:40:09 -0800
Subject: [PATCH 21/36] Try to reconstruct all the plugin cli vars

---
 letsencrypt/cli.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 83dec0c32..395ad2dd7 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -753,6 +753,9 @@ def _set_by_cli(var):
         reconstructed_args = _parser.args + [_parser.verb]
         default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
         _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
+        # propagate plugin requests: eg --standalone modifies config.authenticator
+        plugin_reqs = cli_plugin_requests(_set_by_cli.detector)
+        _set_by_cli.detector.authenticator, _set_by_cli.detector.installer = plugin_reqs
 
     try:
         # Is detector.var something that isn't false?

From 55f1840d834153019cb7df722439e1c8481cf1c3 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:41:52 -0800
Subject: [PATCH 22/36] Make static var less verbose

---
 letsencrypt/cli.py | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 395ad2dd7..27c9069c2 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -745,21 +745,22 @@ def _set_by_cli(var):
     (CLI or config file) including if the user explicitly set it to the
     default.  Returns False if the variable was assigned a default value.
     """
-    if _set_by_cli.detector is None:
+    detector = _set_by_cli.detector
+    if detector is None:
         # Setup on first run: `detector` is a weird version of config in which
         # the default value of every attribute is wrangled to be boolean-false
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = _parser.args + [_parser.verb]
         default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
-        _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
+        detector = _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
-        plugin_reqs = cli_plugin_requests(_set_by_cli.detector)
-        _set_by_cli.detector.authenticator, _set_by_cli.detector.installer = plugin_reqs
+        plugin_reqs = cli_plugin_requests(detector)
+        detector.authenticator, detector.installer = plugin_reqs
 
     try:
         # Is detector.var something that isn't false?
-        change_detected = _set_by_cli.detector.__getattr__(var)
+        change_detected = detector.__getattr__(var)
     except AttributeError:
         logger.warning("Missing default analysis for %r", var)
         return False
@@ -768,7 +769,7 @@ def _set_by_cli(var):
         return True
     # Special case: vars like --no-redirect that get set True -> False
     # default to None; False means they were set
-    elif var in _set_by_cli.detector.store_false_vars and change_detected is not None:
+    elif var in detector.store_false_vars and change_detected is not None:
         return True
     else:
         return False

From 4a7a0bd47ad5bb110b3416eca850eee38e01b2c4 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:52:10 -0800
Subject: [PATCH 23/36] Update detector's namespace correctly

---
 letsencrypt/cli.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 27c9069c2..f4524d6b7 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -755,8 +755,13 @@ def _set_by_cli(var):
         default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
         detector = _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
-        plugin_reqs = cli_plugin_requests(detector)
-        detector.authenticator, detector.installer = plugin_reqs
+        auth, inst = cli_plugin_requests(detector)
+        if auth:
+            detector.namespace.__setattr__("authenticator", auth)
+        if inst:
+            detector.namespace.__setattr__("installer", inst)
+        # more spammy than just debug
+        logger.log(-10, "Default Detector is %r", auth, inst, detector.namespace)
 
     try:
         # Is detector.var something that isn't false?

From 5514776a7e42d246103c5ba029803e0f9791beb9 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:53:48 -0800
Subject: [PATCH 24/36] lint / fix

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index f4524d6b7..589a403e0 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -761,7 +761,7 @@ def _set_by_cli(var):
         if inst:
             detector.namespace.__setattr__("installer", inst)
         # more spammy than just debug
-        logger.log(-10, "Default Detector is %r", auth, inst, detector.namespace)
+        logger.log(-10, "Default Detector is %r",detector.namespace)
 
     try:
         # Is detector.var something that isn't false?

From d0d63b65b85202cd90a8984f4b317683ebbb6d3c Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 10:54:24 -0800
Subject: [PATCH 25/36] lintlint

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 589a403e0..302df26cc 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -761,7 +761,7 @@ def _set_by_cli(var):
         if inst:
             detector.namespace.__setattr__("installer", inst)
         # more spammy than just debug
-        logger.log(-10, "Default Detector is %r",detector.namespace)
+        logger.log(-10, "Default Detector is %r", detector.namespace)
 
     try:
         # Is detector.var something that isn't false?

From 0fb3bf689db5bd29b6765985df9861350736f3a9 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 11:46:43 -0800
Subject: [PATCH 26/36] More debugging.

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 302df26cc..f5a00ba8f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -761,7 +761,7 @@ def _set_by_cli(var):
         if inst:
             detector.namespace.__setattr__("installer", inst)
         # more spammy than just debug
-        logger.log(-10, "Default Detector is %r", detector.namespace)
+        logger.debug("Default Detector is %r", detector.namespace)
 
     try:
         # Is detector.var something that isn't false?

From 13aed36cd5ce6e3e4e974a6fbe0065d51c65181d Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 12:22:36 -0800
Subject: [PATCH 27/36] "" is a reasonable server value in
 default_detection=True

---
 letsencrypt/cli.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index f5a00ba8f..25eb821aa 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1195,7 +1195,7 @@ class HelpfulArgumentParser(object):
                     parsed_args.domains.append(domain)
 
         if parsed_args.staging or parsed_args.dry_run:
-            if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
+            if parsed_args.server not in ("", flag_default("server"), constants.STAGING_URI):
                 conflicts = ["--staging"] if parsed_args.staging else []
                 conflicts += ["--dry-run"] if parsed_args.dry_run else []
                 if not self.detect_defaults:

From 56be2e054cacb464df970235e918c55ffd6f5010 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 12:24:25 -0800
Subject: [PATCH 28/36] For testing purposes, implement a way to specify which
 renewal files are run

---
 letsencrypt/cli.py       | 8 +++++++-
 letsencrypt/constants.py | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 25eb821aa..cce69ca47 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -918,7 +918,7 @@ def _reconstitute(config, full_path):
 
 def _renewal_conf_files(config):
     """Return /path/to/*.conf in the renewal conf directory"""
-    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
+    return glob.glob(os.path.join(config.renewal_configs_dir, config.renewal_glob))
 
 
 def _renew_describe_results(config, renew_successes, renew_failures,
@@ -1356,6 +1356,9 @@ class HelpfulArgumentParser(object):
             for var in args:
                 self.store_false_vars[var] = True
 
+        if "--server" in args:
+            print("Munged? server to", kwargs)
+
 
     def add_deprecated_argument(self, argument_name, num_args):
         """Adds a deprecated argument with the name argument_name.
@@ -1484,6 +1487,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         "automation", "--expand", action="store_true",
         help="If an existing cert covers some subset of the requested names, "
              "always expand and replace it with the additional names.")
+    helpful.add(
+        "automation", "--renewal-glob", default=flag_default("renewal_glob"),
+        help="A pattern for which renewal files in /etc/letsencrypt/renewal/ to process")
     helpful.add(
         "automation", "--version", action="version",
         version="%(prog)s {0}".format(letsencrypt.__version__),
diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py
index 402f5e9a1..dc543980e 100644
--- a/letsencrypt/constants.py
+++ b/letsencrypt/constants.py
@@ -22,6 +22,7 @@ CLI_DEFAULTS = dict(
     config_dir="/etc/letsencrypt",
     work_dir="/var/lib/letsencrypt",
     logs_dir="/var/log/letsencrypt",
+    renewal_glob="*.conf",
     no_verify_ssl=False,
     http01_port=challenges.HTTP01Response.PORT,
     tls_sni_01_port=challenges.TLSSNI01Response.PORT,

From 04926a4662fa74566c12fd136425b1592aeb236e Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 12:44:05 -0800
Subject: [PATCH 29/36] Fix some bugs:

 - modify_arg_for_default_detection was not actually succeeding in modifying
   the kwargs it was called with
 - it should be good enough for the default detector to notice --dry-run and
   pass that to the real conf, we don't need to play with .server as well...
---
 letsencrypt/cli.py | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index cce69ca47..24cea0e30 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1194,13 +1194,14 @@ class HelpfulArgumentParser(object):
                 if domain not in parsed_args.domains:
                     parsed_args.domains.append(domain)
 
-        if parsed_args.staging or parsed_args.dry_run:
-            if parsed_args.server not in ("", flag_default("server"), constants.STAGING_URI):
-                conflicts = ["--staging"] if parsed_args.staging else []
-                conflicts += ["--dry-run"] if parsed_args.dry_run else []
-                if not self.detect_defaults:
-                    raise errors.Error("--server value conflicts with {0}".format(
-                        " and ".join(conflicts)))
+        if not self.detect_defaults:
+            if parsed_args.staging or parsed_args.dry_run:
+                if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
+                    conflicts = ["--staging"] if parsed_args.staging else []
+                    conflicts += ["--dry-run"] if parsed_args.dry_run else []
+                    if not self.detect_defaults:
+                        raise errors.Error("--server value conflicts with {0}".format(
+                            " and ".join(conflicts)))
 
             parsed_args.server = constants.STAGING_URI
 
@@ -1316,7 +1317,7 @@ class HelpfulArgumentParser(object):
         """
 
         if self.detect_defaults:
-            self.modify_arg_for_default_detection(self, *args, **kwargs)
+            kwargs = self.modify_arg_for_default_detection(self, *args, **kwargs)
 
         if self.visible_topics[topic]:
             if topic in self.groups:
@@ -1336,6 +1337,8 @@ class HelpfulArgumentParser(object):
 
         :param list *args: the names of this argument flag
         :param dict **kwargs: various argparse settings for this argument
+
+        :returns: a modified versions of kwargs
         """
         # argument either doesn't have a default, or the default doesn't
         # isn't Pythonically false
@@ -1356,8 +1359,7 @@ class HelpfulArgumentParser(object):
             for var in args:
                 self.store_false_vars[var] = True
 
-        if "--server" in args:
-            print("Munged? server to", kwargs)
+        return kwargs
 
 
     def add_deprecated_argument(self, argument_name, num_args):

From ed348f5528c3496203a1dd58317db2c711a9d4a7 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 12:50:49 -0800
Subject: [PATCH 30/36] Actually all of this logic was fine, and we do need to
 bring in the staging server value

---
 letsencrypt/cli.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 24cea0e30..0111b6f10 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -1194,14 +1194,13 @@ class HelpfulArgumentParser(object):
                 if domain not in parsed_args.domains:
                     parsed_args.domains.append(domain)
 
-        if not self.detect_defaults:
-            if parsed_args.staging or parsed_args.dry_run:
-                if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
-                    conflicts = ["--staging"] if parsed_args.staging else []
-                    conflicts += ["--dry-run"] if parsed_args.dry_run else []
-                    if not self.detect_defaults:
-                        raise errors.Error("--server value conflicts with {0}".format(
-                            " and ".join(conflicts)))
+        if parsed_args.staging or parsed_args.dry_run:
+            if parsed_args.server not in (flag_default("server"), constants.STAGING_URI):
+                conflicts = ["--staging"] if parsed_args.staging else []
+                conflicts += ["--dry-run"] if parsed_args.dry_run else []
+                if not self.detect_defaults:
+                    raise errors.Error("--server value conflicts with {0}".format(
+                        " and ".join(conflicts)))
 
             parsed_args.server = constants.STAGING_URI
 

From 6ca84acc1cb9f76bb998f51165127f2887af9503 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 14:19:53 -0800
Subject: [PATCH 31/36] Fix more bugs

  * setting --dry-run or --staging implies changing account
  * --dry-run: be willing to make a staging account if the user only
    has a prod one
---
 letsencrypt/cli.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 0111b6f10..1465aa789 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -416,7 +416,6 @@ def _suggest_donation_if_appropriate(config):
         reporter_util.add_message(msg, reporter_util.LOW_PRIORITY)
 
 
-
 def _report_successful_dry_run(config):
     reporter_util = zope.component.getUtility(interfaces.IReporter)
     if config.verb != "renew":
@@ -772,6 +771,10 @@ def _set_by_cli(var):
 
     if change_detected:
         return True
+    # Special case: we actually want account to be set to "" if the server
+    # the account was on has changed
+    elif var == "account" and (detector.server or detector.dry_run or detector.staging):
+        return True
     # Special case: vars like --no-redirect that get set True -> False
     # default to None; False means they were set
     elif var in detector.store_false_vars and change_detected is not None:
@@ -1209,6 +1212,11 @@ class HelpfulArgumentParser(object):
                     raise errors.Error("--dry-run currently only works with the "
                                        "'certonly' or 'renew' subcommands (%r)" % self.verb)
                 parsed_args.break_my_certs = parsed_args.staging = True
+                if glob.glob(os.path.join(parsed_args.config_dir, constants.ACCOUNTS_DIR, "*")):
+                    # The user has a prod account, but might not have a staging
+                    # one; we don't want to start trying to perform interactive registration
+                    parsed_args.agree_tos = True
+                    parsed_args.register_unsafely_without_email = True
 
         if parsed_args.csr:
             self.handle_csr(parsed_args)

From c8b89a9f5adda40c539033d555c174fb5396e9ce Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 14:50:32 -0800
Subject: [PATCH 32/36] Revert "For testing purposes, implement a way to
 specify which renewal files are run"

This reverts commit 56be2e054cacb464df970235e918c55ffd6f5010.
---
 letsencrypt/cli.py | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 1465aa789..9ed6cdd8f 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -921,7 +921,7 @@ def _reconstitute(config, full_path):
 
 def _renewal_conf_files(config):
     """Return /path/to/*.conf in the renewal conf directory"""
-    return glob.glob(os.path.join(config.renewal_configs_dir, config.renewal_glob))
+    return glob.glob(os.path.join(config.renewal_configs_dir, "*.conf"))
 
 
 def _renew_describe_results(config, renew_successes, renew_failures,
@@ -1496,9 +1496,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         "automation", "--expand", action="store_true",
         help="If an existing cert covers some subset of the requested names, "
              "always expand and replace it with the additional names.")
-    helpful.add(
-        "automation", "--renewal-glob", default=flag_default("renewal_glob"),
-        help="A pattern for which renewal files in /etc/letsencrypt/renewal/ to process")
     helpful.add(
         "automation", "--version", action="version",
         version="%(prog)s {0}".format(letsencrypt.__version__),

From b4b584155e99a0c823630fd58ec0797263737e30 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 17:40:01 -0800
Subject: [PATCH 33/36] Address stylistic review comments

---
 letsencrypt/cli.py       | 16 +++++++---------
 letsencrypt/constants.py |  1 -
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 9ed6cdd8f..aa69df3c6 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -755,11 +755,8 @@ def _set_by_cli(var):
         detector = _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
         auth, inst = cli_plugin_requests(detector)
-        if auth:
-            detector.namespace.__setattr__("authenticator", auth)
-        if inst:
-            detector.namespace.__setattr__("installer", inst)
-        # more spammy than just debug
+        detector.namespace.authenticator = auth if auth else ""
+        detector.namespace.installer = inst if inst else ""
         logger.debug("Default Detector is %r", detector.namespace)
 
     try:
@@ -915,7 +912,7 @@ def _reconstitute(config, full_path):
         return None
 
     if not _set_by_cli("domains"):
-        setattr(config.namespace, "domains", domains)
+        config.namespace.domains = domains
 
     return renewal_candidate
 
@@ -1158,9 +1155,10 @@ class HelpfulArgumentParser(object):
         self.parser._add_config_file_help = False  # pylint: disable=protected-access
         self.silent_parser = SilentParser(self.parser)
 
-        # This setting attempts to force all default values to None; it
-        # is used to detect when values have been explicitly set by the user,
-        # including when they are set to their normal default value
+        # This setting attempts to force all default values to things that are
+        # pythonically false; it is used to detect when values have been
+        # explicitly set by the user, including when they are set to their
+        # normal default value
         self.detect_defaults = detect_defaults
         if detect_defaults:
             self.store_false_vars = {}  # vars that use "store_false"
diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py
index dc543980e..402f5e9a1 100644
--- a/letsencrypt/constants.py
+++ b/letsencrypt/constants.py
@@ -22,7 +22,6 @@ CLI_DEFAULTS = dict(
     config_dir="/etc/letsencrypt",
     work_dir="/var/lib/letsencrypt",
     logs_dir="/var/log/letsencrypt",
-    renewal_glob="*.conf",
     no_verify_ssl=False,
     http01_port=challenges.HTTP01Response.PORT,
     tls_sni_01_port=challenges.TLSSNI01Response.PORT,

From 64600ff61c1bb1b28881e8a372078dd5fc883ed8 Mon Sep 17 00:00:00 2001
From: Peter Eckersley <pde@eff.org>
Date: Tue, 9 Feb 2016 17:52:30 -0800
Subject: [PATCH 34/36] Place a signpost about default detection for plugin
 args

---
 docs/contributing.rst         |  3 +-
 letsencrypt/plugins/common.py | 56 +++++++++++++++++++----------------
 2 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/docs/contributing.rst b/docs/contributing.rst
index 8a27d565e..36dff01b9 100644
--- a/docs/contributing.rst
+++ b/docs/contributing.rst
@@ -157,7 +157,7 @@ Plugin-architecture
 Let's Encrypt has a plugin architecture to facilitate support for
 different webservers, other TLS servers, and operating systems.
 The interfaces available for plugins to implement are defined in
-`interfaces.py`_.
+`interfaces.py`_ and `plugins/common.py`_.
 
 The most common kind of plugin is a "Configurator", which is likely to
 implement the `~letsencrypt.interfaces.IAuthenticator` and
@@ -168,6 +168,7 @@ There are also `~letsencrypt.interfaces.IDisplay` plugins,
 which implement bindings to alternative UI libraries.
 
 .. _interfaces.py: https://github.com/letsencrypt/letsencrypt/blob/master/letsencrypt/interfaces.py
+.. _plugins/common.py: https://github.com/letsencrypt/letsencrypt/blob/master/letsencrypt/plugins/common.py#L34
 
 
 Authenticators
diff --git a/letsencrypt/plugins/common.py b/letsencrypt/plugins/common.py
index f18b1fb3b..bf996ba5e 100644
--- a/letsencrypt/plugins/common.py
+++ b/letsencrypt/plugins/common.py
@@ -41,6 +41,36 @@ class Plugin(object):
         self.config = config
         self.name = name
 
+    @jose_util.abstractclassmethod
+    def add_parser_arguments(cls, add):
+        """Add plugin arguments to the CLI argument parser.
+
+        :param callable add: Function that proxies calls to
+            `argparse.ArgumentParser.add_argument` prepending options
+            with unique plugin name prefix.
+
+        NOTE: if you add argpase arguments such that users setting them can
+        create a config entry that python's bool() would consider false (ie,
+        the use might set the variable to "", [], 0, etc), please ensure that
+        cli._set_by_cli() works for your variable.
+
+        """
+
+    @classmethod
+    def inject_parser_options(cls, parser, name):
+        """Inject parser options.
+
+        See `~.IPlugin.inject_parser_options` for docs.
+
+        """
+        # dummy function, doesn't check if dest.startswith(self.dest_namespace)
+        def add(arg_name_no_prefix, *args, **kwargs):
+            # pylint: disable=missing-docstring
+            return parser.add_argument(
+                "--{0}{1}".format(option_namespace(name), arg_name_no_prefix),
+                *args, **kwargs)
+        return cls.add_parser_arguments(add)
+
     @property
     def option_namespace(self):
         """ArgumentParser options namespace (prefix of all options)."""
@@ -64,32 +94,6 @@ class Plugin(object):
     def conf(self, var):
         """Find a configuration value for variable ``var``."""
         return getattr(self.config, self.dest(var))
-
-    @classmethod
-    def inject_parser_options(cls, parser, name):
-        """Inject parser options.
-
-        See `~.IPlugin.inject_parser_options` for docs.
-
-        """
-        # dummy function, doesn't check if dest.startswith(self.dest_namespace)
-        def add(arg_name_no_prefix, *args, **kwargs):
-            # pylint: disable=missing-docstring
-            return parser.add_argument(
-                "--{0}{1}".format(option_namespace(name), arg_name_no_prefix),
-                *args, **kwargs)
-        return cls.add_parser_arguments(add)
-
-    @jose_util.abstractclassmethod
-    def add_parser_arguments(cls, add):
-        """Add plugin arguments to the CLI argument parser.
-
-        :param callable add: Function that proxies calls to
-            `argparse.ArgumentParser.add_argument` prepending options
-            with unique plugin name prefix.
-
-        """
-
 # other
 
 

From 8c970a890da5a56f075c641a7497645c2eb9c3c5 Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Tue, 9 Feb 2016 18:28:56 -0800
Subject: [PATCH 35/36] Revert "Resume using a fully-constructed config
 namespace"

This reverts commit 3603f482e5fe81cb5948699a77a2abaa16a8839d.
---
 letsencrypt/cli.py           | 4 ++--
 letsencrypt/configuration.py | 5 ++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index aa69df3c6..6655c33dd 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -751,8 +751,8 @@ def _set_by_cli(var):
         plugins = plugins_disco.PluginsRegistry.find_all()
         # reconstructed_args == sys.argv[1:], or whatever was passed to main()
         reconstructed_args = _parser.args + [_parser.verb]
-        default_args = prepare_and_parse_args(plugins, reconstructed_args, detect_defaults=True)
-        detector = _set_by_cli.detector = configuration.NamespaceConfig(default_args, fake=True)
+        detector = _set_by_cli.detector = prepare_and_parse_args(
+            plugins, reconstructed_args, detect_defaults=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
         auth, inst = cli_plugin_requests(detector)
         detector.namespace.authenticator = auth if auth else ""
diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py
index 979d5e985..2bbf1b019 100644
--- a/letsencrypt/configuration.py
+++ b/letsencrypt/configuration.py
@@ -34,7 +34,7 @@ class NamespaceConfig(object):
     """
     zope.interface.implements(interfaces.IConfig)
 
-    def __init__(self, namespace, fake=False):
+    def __init__(self, namespace):
         self.namespace = namespace
 
         self.namespace.config_dir = os.path.abspath(self.namespace.config_dir)
@@ -42,8 +42,7 @@ class NamespaceConfig(object):
         self.namespace.logs_dir = os.path.abspath(self.namespace.logs_dir)
 
         # Check command line parameters sanity, and error out in case of problem.
-        if not fake:
-            check_config_sanity(self)
+        check_config_sanity(self)
 
     def __getattr__(self, name):
         return getattr(self.namespace, name)

From 7aa2f4b5f6c56ae494a97b7fb48906c8190c291d Mon Sep 17 00:00:00 2001
From: Brad Warren <bmw@eff.org>
Date: Tue, 9 Feb 2016 18:34:44 -0800
Subject: [PATCH 36/36] Remove stay .namespace

---
 letsencrypt/cli.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py
index 6655c33dd..6f0b45888 100644
--- a/letsencrypt/cli.py
+++ b/letsencrypt/cli.py
@@ -755,9 +755,9 @@ def _set_by_cli(var):
             plugins, reconstructed_args, detect_defaults=True)
         # propagate plugin requests: eg --standalone modifies config.authenticator
         auth, inst = cli_plugin_requests(detector)
-        detector.namespace.authenticator = auth if auth else ""
-        detector.namespace.installer = inst if inst else ""
-        logger.debug("Default Detector is %r", detector.namespace)
+        detector.authenticator = auth if auth else ""
+        detector.installer = inst if inst else ""
+        logger.debug("Default Detector is %r", detector)
 
     try:
         # Is detector.var something that isn't false?