Replace (most) global state in cli/__init__.py (#9678)

* Rewrite helpful_test to appease the linter

* Use public interface to access argparse sources dict

* HelpfulParser builds ArgumentSources dict, stores it in NamespaceConfig

After arguments/config files/user prompted input have been parsed, we
build a mapping of Namespace options to an ArgumentSource value. These
generally come from argparse's builtin "source_to_settings" dict, but
we also add a source value representing dynamic values set at runtime.

This dict is then passed to NamespaceConfig, which can then be queried
directly or via the "set_by_user" method, which replaces the global
"set_by_cli" and "option_was_set" functions.

* Use NamespaceConfig.set_by_user instead of set_by_cli/option_was_set

This involves passing the NamespaceConfig around to more functions
than before, removes the need for most of the global state shenanigans
needed by set_by_cli and friends.

* Set runtime config values on the NamespaceConfig object

This'll correctly mark them as being "runtime" values in the
ArgumentSources dict

* Bump oldest configargparse version

We need a version that has get_source_to_settings_dict()

* Add more cli unit tests, use ArgumentSource.DEFAULT by default

One of the tests revealed that ConfigArgParse's source dict excludes
arguments it considers unimportant/irrelevant. We now mark all arguments
as having a DEFAULT source by default, and update them otherwise.

* Mark more argument sources as RUNTIME

* Removes some redundant helpful_test.py, moves one to cli_test.py

We were already testing most of these cases in cli_test.py, only
with a more complete HelpfulArgumentParser setup. And since the hsts/no-hsts
test was manually performing the kind of argument adding that cli
already does out of the box, I figured the cli tests were a more natural
place for it.

* appease the linter

* Various fixups from review

* Add windows compatability fix

* Add test ensuring relevant_values behaves properly

* Build sources dict in a more predictable manner

The dict is now built in a defined order: first defaults, then config
files, then env vars, then command line args. This way we eliminate the
possibility of undefined behavior if configargparse puts an arg's entry
in multiple source dicts.

* remove superfluous update to sources dict

* remove duplicate constant defines, resolve circular import situation

certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py

@@ -58,7 +58,7 @@ def _prepare_configurator(self) -> None:
                     getattr(entrypoint.ENTRYPOINT.OS_DEFAULTS, k))
 
         self._configurator = entrypoint.ENTRYPOINT(
-            config=configuration.NamespaceConfig(self.le_config),
+            config=configuration.NamespaceConfig(self.le_config, {}),
             name="apache")
         self._configurator.prepare()
 

certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py

@@ -44,7 +44,7 @@ def _prepare_configurator(self) -> None:
         for k in constants.CLI_DEFAULTS:
             setattr(self.le_config, "nginx_" + k, constants.os_constant(k))
 
-        conf = configuration.NamespaceConfig(self.le_config)
+        conf = configuration.NamespaceConfig(self.le_config, {})
         self._configurator = configurator.NginxConfigurator(config=conf, name="nginx")
         self._configurator.prepare()
 

certbot/certbot/_internal/cli/__init__.py

@@ -10,6 +10,7 @@
 from typing import Type
 
 import certbot
+from certbot.configuration import NamespaceConfig
 from certbot._internal import constants
 from certbot._internal.cli.cli_constants import ARGPARSE_PARAMS_TO_REMOVE
 from certbot._internal.cli.cli_constants import cli_command
@@ -20,7 +21,6 @@
 from certbot._internal.cli.cli_constants import SHORT_USAGE
 from certbot._internal.cli.cli_constants import VAR_MODIFIERS
 from certbot._internal.cli.cli_constants import ZERO_ARG_ACTIONS
-from certbot._internal.cli.cli_utils import _Default
 from certbot._internal.cli.cli_utils import _DeployHookAction
 from certbot._internal.cli.cli_utils import _DomainsAction
 from certbot._internal.cli.cli_utils import _EncodeReasonAction
@@ -54,19 +54,19 @@
 helpful_parser: Optional[HelpfulArgumentParser] = None
 
 
-def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[str],
-                           detect_defaults: bool = False) -> argparse.Namespace:
+def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[str]
+                           ) -> NamespaceConfig:
     """Returns parsed command line arguments.
 
     :param .PluginsRegistry plugins: available plugins
     :param list args: command line arguments with the program name removed
 
     :returns: parsed command line arguments
-    :rtype: argparse.Namespace
+    :rtype: configuration.NamespaceConfig
 
     """
 
-    helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
+    helpful = HelpfulArgumentParser(args, plugins)
     _add_all_groups(helpful)
 
     # --help is automatically provided by argparse
@@ -471,95 +471,16 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[st
     # parser (--help should display plugin-specific options last)
     _plugins_parsing(helpful, plugins)
 
-    if not detect_defaults:
-        global helpful_parser # pylint: disable=global-statement
-        helpful_parser = helpful
+    global helpful_parser # pylint: disable=global-statement
+    helpful_parser = helpful
     return helpful.parse_args()
 
 
-def set_by_cli(var: str) -> bool:
-    """
-    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 value.
-    """
-    # We should probably never actually hit this code. But if we do,
-    # a deprecated option has logically never been set by the CLI.
-    if var in DEPRECATED_OPTIONS:
-        return False
-
-    detector = set_by_cli.detector  # type: ignore
-    if detector is None and helpful_parser is not 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 = helpful_parser.args + [helpful_parser.verb]
-
-        detector = set_by_cli.detector = prepare_and_parse_args(  # type: ignore
-            plugins, reconstructed_args, detect_defaults=True)
-        # propagate plugin requests: eg --standalone modifies config.authenticator
-        detector.authenticator, detector.installer = (
-            plugin_selection.cli_plugin_requests(detector))
-
-    if not isinstance(getattr(detector, var), _Default):
-        logger.debug("Var %s=%s (set by user).", var, getattr(detector, var))
-        return True
-
-    for modifier in VAR_MODIFIERS.get(var, []):
-        if set_by_cli(modifier):
-            logger.debug("Var %s=%s (set by user).",
-                var, VAR_MODIFIERS.get(var, []))
-            return True
-
-    return False
-
-
-# static housekeeping var
-# functions attributed are not supported by mypy
-# https://github.com/python/mypy/issues/2087
-set_by_cli.detector = None  # type: ignore
-
-
-def has_default_value(option: str, value: Any) -> bool:
-    """Does option have the default value?
-
-    If the default value of option is not known, False is returned.
-
-    :param str option: configuration variable being considered
-    :param value: value of the configuration variable named option
-
-    :returns: True if option has the default value, otherwise, False
-    :rtype: bool
-
-    """
-    if helpful_parser is not None:
-        return (option in helpful_parser.defaults and
-                helpful_parser.defaults[option] == value)
-    return False
-
-
-def option_was_set(option: str, value: Any) -> bool:
-    """Was option set by the user or does it differ from the default?
-
-    :param str option: configuration variable being considered
-    :param value: value of the configuration variable named option
-
-    :returns: True if the option was set, otherwise, False
-    :rtype: bool
-
-    """
-    # If an option is deprecated, it was effectively not set by the user.
-    if option in DEPRECATED_OPTIONS:
-        return False
-    return set_by_cli(option) or not has_default_value(option, value)
-
-
 def argparse_type(variable: Any) -> Type:
     """Return our argparse type function for a config variable (default: str)"""
     # pylint: disable=protected-access
     if helpful_parser is not None:
-        for action in helpful_parser.parser._actions:
+        for action in helpful_parser.actions:
             if action.type is not None and action.dest == variable:
                 return action.type
     return str

certbot/certbot/_internal/cli/cli_utils.py

@@ -22,22 +22,6 @@
     from certbot._internal.cli import helpful
 
 
-class _Default:
-    """A class to use as a default to detect if a value is set by a user"""
-
-    def __bool__(self) -> bool:
-        return False
-
-    def __eq__(self, other: Any) -> bool:
-        return isinstance(other, _Default)
-
-    def __hash__(self) -> int:
-        return id(_Default)
-
-    def __nonzero__(self) -> bool:
-        return self.__bool__()
-
-
 def read_file(filename: str, mode: str = "rb") -> Tuple[str, Any]:
     """Returns the given file's contents.