Cleanup responding to protected-access problems

This commit is contained in:
Seth Schoen 2016-03-23 14:57:01 -07:00
parent 4bc8e9a44a
commit 4f7c5b32e8
5 changed files with 18 additions and 15 deletions

View file

@ -271,20 +271,20 @@ def record_chosen_plugins(config, plugins, auth, inst):
cn.installer = plugins.find_init(inst).name if inst else "none"
def _set_by_cli(var):
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 value.
"""
detector = _set_by_cli.detector
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]
detector = _set_by_cli.detector = prepare_and_parse_args(
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)
@ -312,7 +312,7 @@ def _set_by_cli(var):
else:
return False
# static housekeeping var
_set_by_cli.detector = None
set_by_cli.detector = None
def _restore_required_config_elements(config, renewalparams):
"""Sets non-plugin specific values in config from renewalparams
@ -325,7 +325,7 @@ def _restore_required_config_elements(config, renewalparams):
"""
# string-valued items to add if they're present
for config_item in STR_CONFIG_ITEMS:
if config_item in renewalparams and not _set_by_cli(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!
@ -334,7 +334,7 @@ def _restore_required_config_elements(config, renewalparams):
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 _set_by_cli(config_item):
if config_item in renewalparams and not set_by_cli(config_item):
config_value = renewalparams[config_item]
# the default value for http01_port was None during private beta
if config_item == "http01_port" and config_value == "None":
@ -378,7 +378,7 @@ def _restore_plugin_configs(config, renewalparams):
plugin_prefixes.append(renewalparams["installer"])
for plugin_prefix in set(plugin_prefixes):
for config_item, config_value in six.iteritems(renewalparams):
if config_item.startswith(plugin_prefix + "_") and not _set_by_cli(config_item):
if config_item.startswith(plugin_prefix + "_") and not set_by_cli(config_item):
# Values None, True, and False need to be treated specially,
# As they don't get parsed correctly based on type
if config_value in ("None", "True", "False"):
@ -403,7 +403,7 @@ def _restore_webroot_config(config, renewalparams):
if "webroot_map" in renewalparams:
# if the user does anything that would create a new webroot map on the
# CLI, don't use the old one
if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")):
if not (set_by_cli("webroot_map") or set_by_cli("webroot_path")):
setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
elif "webroot_path" in renewalparams:
logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path")
@ -844,7 +844,7 @@ class HelpfulArgumentParser(object):
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.
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

View file

@ -52,7 +52,7 @@ class Plugin(object):
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.
cli.set_by_cli() works for your variable.
"""

View file

@ -152,7 +152,7 @@ def relevant_values(all_values):
# Look through the CLI parser defaults and see if this option is
# both present and equal to the specified value. If not, return
# False.
for x in cli._parser.parser._actions:
for x in cli._parser.parser._actions: # pylint: disable=protected-access
if x.dest == option:
if x.default == value:
return True
@ -164,10 +164,10 @@ def relevant_values(all_values):
for option, value in all_values.iteritems():
# Try to find reasons to store this item in the
# renewal config. It can be stored if it is relevant and
# (it is _set_by_cli() or flag_default() is different
# (it is set_by_cli() or flag_default() is different
# from the value or flag_default() doesn't exist).
if _relevant(option):
if (cli._set_by_cli(option)
if (cli.set_by_cli(option)
or not _is_cli_default(option, value)):
# or option not in constants.CLI_DEFAULTS
# or constants.CLI_DEFAULTS[option] != value):

View file

@ -54,7 +54,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods
shutil.rmtree(self.tmp_dir)
# Reset globals in cli
# pylint: disable=protected-access
cli._parser = cli._set_by_cli.detector = None
cli._parser = cli.set_by_cli.detector = None
def _call(self, args):
"Run the cli with output streams and actual client mocked out"
@ -660,7 +660,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods
args = ["renew", "--dry-run", "-tvv"]
self._test_renewal_common(True, [], args=args, renew=True)
@mock.patch("letsencrypt.cli._set_by_cli")
@mock.patch("letsencrypt.cli.set_by_cli")
def test_ancient_webroot_renewal_conf(self, mock_set_by_cli):
mock_set_by_cli.return_value = False
rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf')

View file

@ -565,6 +565,7 @@ class RenewableCertTests(BaseRenewableCertTest):
@mock.patch("letsencrypt.cli._parser")
def test_relevant_values(self, mock_parser):
"""Test that relevant_values() can reject an irrelevant value."""
# pylint: disable=protected-access
from letsencrypt import storage
mock_parser.verb = "certonly"
mock_parser.args = ["--standalone"]
@ -575,6 +576,7 @@ class RenewableCertTests(BaseRenewableCertTest):
@mock.patch("letsencrypt.cli._parser")
def test_relevant_values_default(self, mock_parser):
"""Test that relevant_values() can reject a default value."""
# pylint: disable=protected-access
from letsencrypt import storage
mock_parser.verb = "certonly"
mock_parser.args = ["--standalone"]
@ -585,6 +587,7 @@ class RenewableCertTests(BaseRenewableCertTest):
@mock.patch("letsencrypt.cli._parser")
def test_relevant_values_nondefault(self, mock_parser):
"""Test that relevant_values() can retain a non-default value."""
# pylint: disable=protected-access
from letsencrypt import storage
mock_parser.verb = "certonly"
mock_parser.args = ["--standalone"]