upstream/certbot
1
0
Fork 0
mirror of https://github.com/certbot/certbot.git synced 2026-06-04 14:26:10 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Merge pull request #2388 from letsencrypt/issue_2347

Browse source 
Ensure that command line arguments still work with `renew`
This commit is contained in:
bmw 2016-02-10 12:42:12 -08:00
commit 636daa648a
5 changed files with 174 additions and 86 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

22
CHANGES.rst
View file

@ -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

3
docs/contributing.rst
View file

@ -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

145
letsencrypt/cli.py
View file

@ -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
@ -421,7 +420,6 @@ def _suggest_donation_if_appropriate(config, action):
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":
@ -745,6 +743,49 @@ def install(config, plugins):
le_client.enhance_config(domains, config)
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
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(
plugins, reconstructed_args, detect_defaults=True)
# propagate plugin requests: eg --standalone modifies config.authenticator
auth, inst = cli_plugin_requests(detector)
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?
change_detected = detector.__getattr__(var)
except AttributeError:
logger.warning("Missing default analysis for %r", var)
return False
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:
return True
else:
return False
# static housekeeping var
_set_by_cli.detector = None
def _restore_required_config_elements(config, renewalparams):
"""Sets non-plugin specific values in config from renewalparams
@ -756,7 +797,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:
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!
@ -765,7 +806,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:
if config_item in renewalparams and not _set_by_cli(config_item):
try:
value = int(renewalparams[config_item])
setattr(config.namespace, config_item, value)
@ -798,7 +839,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 renewalparams.iteritems():
if config_item.startswith(plugin_prefix + "_"):
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
@ -863,7 +904,7 @@ def _reconstitute(config, full_path):
# 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:
if "webroot_map" in renewalparams and not _set_by_cli("webroot_map"):
setattr(config.namespace, "webroot_map", renewalparams["webroot_map"])
try:
@ -875,9 +916,10 @@ def _reconstitute(config, full_path):
"was: %s. Skipping.", full_path, error)
return None
setattr(config.namespace, "domains", domains)
return renewal_candidate
if not _set_by_cli("domains"):
config.namespace.domains = domains
return renewal_candidate
def _renewal_conf_files(config):
"""Return /path/to/*.conf in the renewal conf directory"""
@ -922,6 +964,7 @@ def _renew_describe_results(config, renew_successes, renew_failures,
def renew(config, unused_plugins):
"""Renew previously-obtained certificates."""
if config.domains != []:
raise errors.Error("Currently, the renew verb is only capable of "
"renewing all installed certificates that are due "
@ -1103,7 +1146,7 @@ class HelpfulArgumentParser(object):
HELP_TOPICS = ["all", "security",
"paths", "automation", "testing"] + VERBS.keys()
def __init__(self, args, plugins):
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)
@ -1117,6 +1160,14 @@ 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 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"
self.args = args
self.determine_verb()
help1 = self.prescan_for_flag("-h", self.help_topics)
@ -1128,7 +1179,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.
@ -1150,24 +1201,32 @@ 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.detect_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
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)
if self.detect_defaults: # plumbing
parsed_args.store_false_vars = self.store_false_vars
return parsed_args
def handle_csr(self, parsed_args):
@ -1260,10 +1319,16 @@ 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.detect_defaults:
kwargs = self.modify_arg_for_default_detection(self, *args, **kwargs)
if self.visible_topics[topic]:
if topic in self.groups:
group = self.groups[topic]
@ -1274,6 +1339,39 @@ 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
:returns: a modified versions of kwargs
"""
# 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
if kwargs.get("action", "") == "store_false":
kwargs["default"] = None
for var in args:
self.store_false_vars[var] = True
return kwargs
def add_deprecated_argument(self, argument_name, num_args):
"""Adds a deprecated argument with the name argument_name.
@ -1341,7 +1439,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, detect_defaults=False):
"""Returns parsed command line arguments.
:param .PluginsRegistry plugins: available plugins
@ -1351,7 +1449,7 @@ def prepare_and_parse_args(plugins, args):
:rtype: argparse.Namespace
"""
helpful = HelpfulArgumentParser(args, plugins)
helpful = HelpfulArgumentParser(args, plugins, detect_defaults)
# --help is automatically provided by argparse
helpful.add(
@ -1513,8 +1611,9 @@ def prepare_and_parse_args(plugins, args):
# 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()
@ -1821,9 +1920,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)

56
letsencrypt/plugins/common.py
View file

@ -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

34
letsencrypt/tests/cli_test.py
View file

@ -546,24 +546,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())