From 4a9846b91184fcd95947c12e95a6d1d8ceb6a928 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Jul 2016 20:02:40 -0700 Subject: [PATCH 01/47] Begin work on multi-topic help listings --- certbot/cli.py | 84 +++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 35b3b74ae..fe3a26c87 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -497,16 +497,23 @@ class HelpfulArgumentParser(object): pass return True - def add(self, topic, *args, **kwargs): + def add(self, topics, *args, **kwargs): """Add a new command line argument. - :param str: help topic this should be listed under, can be None for - "always documented" + :param topics: str or [str] help topic(s) this should be listed under, + or None for "always documented" :param list *args: the names of this argument flag :param dict **kwargs: various argparse settings for this argument """ + if isinstance(topics, list): + # if this flag can be listed in multiple sections, try to pick the one + # that the user has asked for help about + topic = self.help_arg if self.help_arg in topics else topics[0] + else: + topic = topics # there's only one + if self.detect_defaults: kwargs = self.modify_kwargs_for_default_detection(**kwargs) @@ -607,6 +614,34 @@ class HelpfulArgumentParser(object): else: return dict([(t, t == chosen_topic) for t in self.help_topics]) +def _add_all_groups(helpful): + helpful.add_group("automation", description="Arguments for automating execution & other tweaks") + helpful.add_group("security", description="Security parameters & server settings") + helpful.add_group( + "testing", description="The following flags are meant for " + "testing purposes only! Do NOT change them, unless you " + "really know what you're doing!") + # VERBS + helpful.add_group( + "renew", description="The 'renew' subcommand will attempt to renew all" + " certificates (or more precisely, certificate lineages) you have" + " previously obtained if they are close to expiry, and print a" + " summary of the results. By default, 'renew' will reuse the options" + " used to create obtain or most recently successfully renew each" + " certificate lineage. You can try it with `--dry-run` first. For" + " more fine-grained control, you can renew individual lineages with" + " the `certonly` subcommand. Hooks are available to run commands " + " before and after renewal; see" + " https://certbot.eff.org/docs/using.html#renewal for more information on these.") + helpful.add_group("certonly", description="Options for modifying how a cert is obtained") + helpful.add_group("install", description="Options for modifying how a cert is deployed") + helpful.add_group("revoke", description="Options for revocation of certs") + helpful.add_group("rollback", description="Options for reverting config changes") + helpful.add_group("plugins", description='Options for the "plugins" subcommand') + helpful.add_group("config_changes", + description="Options for showing a history of config changes") + helpful.add_group("paths", description="Arguments changing execution paths & servers") + def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: disable=too-many-statements """Returns parsed command line arguments. @@ -622,6 +657,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis # pylint: disable=too-many-statements helpful = HelpfulArgumentParser(args, plugins, detect_defaults) + _add_all_groups(helpful) # --help is automatically provided by argparse helpful.add( @@ -678,11 +714,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="Domain names to apply. For multiple domains you can use " "multiple -d flags or enter a comma separated list of domains " "as a parameter.") - helpful.add_group( - "automation", - description="Arguments for automating execution & other tweaks") helpful.add( - "automation", "--keep-until-expiring", "--keep", "--reinstall", + ["automation", "renew"], "--keep-until-expiring", "--keep", "--reinstall", dest="reinstall", action="store_true", help="If the requested cert matches an existing cert, always keep the " "existing one until it is due for renewal (for the " @@ -721,23 +754,19 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "(both can be renewed in parallel)") helpful.add( "automation", "--os-packages-only", action="store_true", - help="(letsencrypt-auto only) install OS package dependencies and then stop") + help="(certbot-auto only) install OS package dependencies and then stop") helpful.add( "automation", "--no-self-upgrade", action="store_true", - help="(letsencrypt-auto only) prevent the letsencrypt-auto script from" + help="(certbot-auto only) prevent the certbot-auto script from" " upgrading itself to newer released versions") helpful.add( "automation", "-q", "--quiet", dest="quiet", action="store_true", help="Silence all output except errors. Useful for automation via cron." " Implies --non-interactive.") - helpful.add_group( - "testing", description="The following flags are meant for " - "testing purposes only! Do NOT change them, unless you " - "really know what you're doing!") helpful.add( "testing", "--debug", action="store_true", - help="Show tracebacks in case of errors, and allow letsencrypt-auto " + help="Show tracebacks in case of errors, and allow certbot-auto " "execution on experimental platforms") helpful.add( "testing", "--no-verify-ssl", action="store_true", @@ -754,8 +783,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "testing", "--break-my-certs", action="store_true", help="Be willing to replace or renew valid certs with invalid " "(testing/staging) certs") - helpful.add_group( - "security", description="Security parameters & server settings") helpful.add( "security", "--rsa-key-size", type=int, metavar="N", default=flag_default("rsa_key_size"), help=config_help("rsa_key_size")) @@ -805,18 +832,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") - helpful.add_group( - "renew", description="The 'renew' subcommand will attempt to renew all" - " certificates (or more precisely, certificate lineages) you have" - " previously obtained if they are close to expiry, and print a" - " summary of the results. By default, 'renew' will reuse the options" - " used to create obtain or most recently successfully renew each" - " certificate lineage. You can try it with `--dry-run` first. For" - " more fine-grained control, you can renew individual lineages with" - " the `certonly` subcommand. Hooks are available to run commands " - " before and after renewal; see" - " https://certbot.eff.org/docs/using.html#renewal for more information on these.") - helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates. Intended" @@ -859,13 +874,6 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis def _create_subparsers(helpful): - helpful.add_group("certonly", description="Options for modifying how a cert is obtained") - helpful.add_group("install", description="Options for modifying how a cert is deployed") - helpful.add_group("revoke", description="Options for revocation of certs") - helpful.add_group("rollback", description="Options for reverting config changes") - helpful.add_group("plugins", description="Plugin options") - helpful.add_group("config_changes", - description="Options for showing a history of config changes") helpful.add("config_changes", "--num", type=int, help="How many past revisions you want to be displayed") helpful.add( @@ -901,8 +909,6 @@ def _paths_parser(helpful): verb = helpful.verb if verb == "help": verb = helpful.help_arg - helpful.add_group( - "paths", description="Arguments changing execution paths & servers") cph = "Path to where cert is saved (with auth --csr), installed from or revoked." section = "paths" @@ -948,12 +954,14 @@ def _paths_parser(helpful): def _plugins_parsing(helpful, plugins): + # It's nuts, but there are two "plugins" topics. Somehow this works helpful.add_group( - "plugins", description="Certbot client supports an " + "plugins", description="Plugin Selection: Certbot client supports an " "extensible plugins architecture. See '%(prog)s plugins' for a " "list of all installed plugins and their names. You can force " "a particular plugin by setting options provided below. Running " "--help will list flags specific to that plugin.") + helpful.add( "plugins", "-a", "--authenticator", help="Authenticator plugin name.") helpful.add( From d96278505e6b01f115c597aaf035452924714425 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 6 Jul 2016 20:07:33 -0700 Subject: [PATCH 02/47] Fix test --- certbot/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 896550837..388c38152 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -113,7 +113,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods out = self._help_output(['--help', 'plugins']) self.assertTrue("--manual-test-mode" not in out) self.assertTrue("--prepare" in out) - self.assertTrue("Plugin options" in out) + self.assertTrue('"plugins" subcommand' in out) out = self._help_output(['--help', 'install']) self.assertTrue("--cert-path" in out) From 204c3f0dfbd242751efaf7a36ced35f7e5d16ca5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 7 Jul 2016 15:11:23 -0700 Subject: [PATCH 03/47] Start using multi-topic CLI flag documentation --- certbot/cli.py | 53 +++++++++++++++++++++------------------ certbot/tests/cli_test.py | 8 ++++++ 2 files changed, 37 insertions(+), 24 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index fe3a26c87..5b7f6ca8a 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -88,8 +88,8 @@ More detailed help: the available topics are: all, automation, paths, security, testing, or any of the subcommands or - plugins (certonly, install, register, nginx, apache, standalone, webroot, - etc.) + plugins (certonly, renew, install, register, nginx, apache, standalone, + webroot, etc.) """ @@ -390,8 +390,7 @@ class HelpfulArgumentParser(object): if getattr(parsed_args, arg): raise errors.Error( ("Conflicting values for displayer." - " {0} conflicts with dialog_mode").format(arg) - ) + " {0} conflicts with dialog_mode").format(arg)) if parsed_args.validate_hooks: hooks.validate_hooks(parsed_args) @@ -676,9 +675,18 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "which ones are required if it finds one missing") helpful.add( None, "--dialog", dest="dialog_mode", action="store_true", - help="Run using dialog") + help="Run using interactive dialog menus") helpful.add( - None, "--dry-run", action="store_true", dest="dry_run", + [None, "run", "certonly"], + "-d", "--domains", "--domain", dest="domains", + metavar="DOMAIN", action=_DomainsAction, default=[], + help="Domain names to apply. For multiple domains you can use " + "multiple -d flags or enter a comma separated list of domains " + "as a parameter.") + + helpful.add( + [None, "testing", "renew", "certonly"], + "--dry-run", action="store_true", dest="dry_run", help="Perform a test run of the client, obtaining test (invalid) certs" " but not saving them to disk. This can currently only be used" " with the 'certonly' and 'renew' subcommands. \nNote: Although --dry-run" @@ -705,17 +713,9 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "with an existing registration, such as the e-mail address, " "should be updated, rather than registering a new account.") helpful.add(None, "-m", "--email", help=config_help("email")) - # positional arg shadows --domains, instead of appending, and - # --domains is useful, because it can be stored in config - #for subparser in parser_run, parser_auth, parser_install: - # subparser.add_argument("domains", nargs="*", metavar="domain") - helpful.add(None, "-d", "--domains", "--domain", dest="domains", - metavar="DOMAIN", action=_DomainsAction, default=[], - help="Domain names to apply. For multiple domains you can use " - "multiple -d flags or enter a comma separated list of domains " - "as a parameter.") helpful.add( - ["automation", "renew"], "--keep-until-expiring", "--keep", "--reinstall", + ["automation", "renew", "certonly", "run"], + "--keep-until-expiring", "--keep", "--reinstall", dest="reinstall", action="store_true", help="If the requested cert matches an existing cert, always keep the " "existing one until it is due for renewal (for the " @@ -729,14 +729,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis version="%(prog)s {0}".format(certbot.__version__), help="show program's version number and exit") helpful.add( - "automation", "--force-renewal", "--renew-by-default", + ["automation", "renew"], + "--force-renewal", "--renew-by-default", action="store_true", dest="renew_by_default", help="If a certificate " "already exists for the requested domains, renew it now, " "regardless of whether it is near expiry. (Often " "--keep-until-expiring is more appropriate). Also implies " "--expand.") helpful.add( - "automation", "--allow-subset-of-names", action="store_true", + ["automation", "renew"], + "--allow-subset-of-names", action="store_true", help="When performing domain validation, do not consider it a failure " "if authorizations can not be obtained for a strict subset of " "the requested domains. This may be useful for allowing renewals for " @@ -760,7 +762,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="(certbot-auto only) prevent the certbot-auto script from" " upgrading itself to newer released versions") helpful.add( - "automation", "-q", "--quiet", dest="quiet", action="store_true", + ["automation", "renew"], + "-q", "--quiet", dest="quiet", action="store_true", help="Silence all output except errors. Useful for automation via cron." " Implies --non-interactive.") @@ -970,15 +973,17 @@ def _plugins_parsing(helpful, plugins): "plugins", "--configurator", help="Name of the plugin that is " "both an authenticator and an installer. Should not be used " "together with --authenticator or --installer.") - helpful.add("plugins", "--apache", action="store_true", + helpful.add(["plugins", "certonly", "run", "install"], + "--apache", action="store_true", help="Obtain and install certs using Apache") - helpful.add("plugins", "--nginx", action="store_true", + helpful.add(["plugins", "certonly", "run", "install"], + "--nginx", action="store_true", help="Obtain and install certs using Nginx") - helpful.add("plugins", "--standalone", action="store_true", + helpful.add(["plugins", "certonly"], "--standalone", action="store_true", help='Obtain certs using a "standalone" webserver.') - helpful.add("plugins", "--manual", action="store_true", + helpful.add(["plugins", "certonly"], "--manual", action="store_true", help='Provide laborious manual instructions for obtaining a cert') - helpful.add("plugins", "--webroot", action="store_true", + helpful.add(["plugins", "certonly"], "--webroot", action="store_true", help='Obtain certs by placing files in a webroot directory.') # things should not be reorder past/pre this comment: diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 388c38152..63c682b20 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -115,6 +115,14 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue("--prepare" in out) self.assertTrue('"plugins" subcommand' in out) + # test multiple topics + out = self._help_output(['-h', 'renew']) + self.assertTrue("--keep" in out) + out = self._help_output(['-h', 'automation']) + self.assertTrue("--keep" in out) + out = self._help_output(['-h', 'revoke']) + self.assertTrue("--keep" not in out) + out = self._help_output(['--help', 'install']) self.assertTrue("--cert-path" in out) self.assertTrue("--key-path" in out) From ecd1ca4645fe594df805d1e4b6afa2c6b4441245 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 8 Jul 2016 10:05:56 -0700 Subject: [PATCH 04/47] grammatical improvement --- certbot/cli.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 5b7f6ca8a..89dacac0d 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -828,13 +828,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "security", "--no-staple-ocsp", action="store_false", help="Do not automatically enable OCSP Stapling.", dest="staple", default=None) - - helpful.add( "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") - helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates. Intended" From c3244df951fefdbc5158f59383f6695978dee1a6 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 8 Jul 2016 10:12:57 -0700 Subject: [PATCH 05/47] more doc improvements --- certbot-apache/certbot_apache/configurator.py | 2 +- certbot/cli.py | 9 ++++----- certbot/interfaces.py | 2 +- certbot/plugins/standalone.py | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index d1c2b7165..50fd10895 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -83,7 +83,7 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): """ - description = "Apache Web Server - Alpha" + description = "Apache Web Server plugin - Beta" @classmethod def add_parser_arguments(cls, add): diff --git a/certbot/cli.py b/certbot/cli.py index 89dacac0d..97dfc6d7e 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -766,7 +766,10 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "-q", "--quiet", dest="quiet", action="store_true", help="Silence all output except errors. Useful for automation via cron." " Implies --non-interactive.") - + # overwrites server, handled in HelpfulArgumentParser.parse_args() + helpful.add("testing", "--test-cert", "--staging", action='store_true', dest='staging', + help='Use the staging server to obtain test (invalid) certs; equivalent' + ' to --server ' + constants.STAGING_URI) helpful.add( "testing", "--debug", action="store_true", help="Show tracebacks in case of errors, and allow certbot-auto " @@ -947,10 +950,6 @@ def _paths_parser(helpful): help="Logs directory.") add("paths", "--server", default=flag_default("server"), help=config_help("server")) - # overwrites server, handled in HelpfulArgumentParser.parse_args() - add("testing", "--test-cert", "--staging", action='store_true', dest='staging', - help='Use the staging server to obtain test (invalid) certs; equivalent' - ' to --server ' + constants.STAGING_URI) def _plugins_parsing(helpful, plugins): diff --git a/certbot/interfaces.py b/certbot/interfaces.py index e4e62e0a2..d4b391378 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -227,7 +227,7 @@ class IConfig(zope.interface.Interface): "Location of renewal configuration file.") no_verify_ssl = zope.interface.Attribute( - "Disable SSL certificate verification.") + "Disable verification of the ACME server's certificate.") tls_sni_01_port = zope.interface.Attribute( "Port number to perform tls-sni-01 challenge. " "Boulder in testing mode defaults to 5001.") diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 8e1cb72a4..97aca351a 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -154,7 +154,7 @@ class Authenticator(common.Plugin): rely on any existing server program. """ - description = "Automatically use a temporary webserver" + description = "Spin up a temporary webserver" def __init__(self, *args, **kwargs): super(Authenticator, self).__init__(*args, **kwargs) From 894af917778761c2cd3e9143402f71dc1070fe0f Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Fri, 8 Jul 2016 14:09:26 -0700 Subject: [PATCH 06/47] Fix test --- certbot/plugins/disco_test.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/certbot/plugins/disco_test.py b/certbot/plugins/disco_test.py index cef6ede8f..75544ee9b 100644 --- a/certbot/plugins/disco_test.py +++ b/certbot/plugins/disco_test.py @@ -55,9 +55,7 @@ class PluginEntryPointTest(unittest.TestCase): name, PluginEntryPoint.entry_point_to_plugin_name(entry_point)) def test_description(self): - self.assertEqual( - "Automatically use a temporary webserver", - self.plugin_ep.description) + self.assertTrue("temporary webserver" in self.plugin_ep.description) def test_description_with_name(self): self.plugin_ep.plugin_cls = mock.MagicMock(description="Desc") From 64012a6053dd97b1832ed241929df36d8963e8db Mon Sep 17 00:00:00 2001 From: Jacob Sachs Date: Wed, 15 Jun 2016 13:23:52 -0400 Subject: [PATCH 07/47] set dialog widgets to use autowidgetsize --- certbot/display/util.py | 21 +++++++-------------- certbot/tests/display/util_test.py | 7 ++----- 2 files changed, 9 insertions(+), 19 deletions(-) diff --git a/certbot/display/util.py b/certbot/display/util.py index 39486b2bd..258afd948 100644 --- a/certbot/display/util.py +++ b/certbot/display/util.py @@ -82,7 +82,7 @@ class NcursesDisplay(object): def __init__(self, width=WIDTH, height=HEIGHT): super(NcursesDisplay, self).__init__() - self.dialog = dialog.Dialog() + self.dialog = dialog.Dialog(autowidgetsize=True) assert OK == self.dialog.DIALOG_OK, "What kind of absurdity is this?" self.width = width self.height = height @@ -101,7 +101,7 @@ class NcursesDisplay(object): :param bool pause: Not applicable to NcursesDisplay """ - self.dialog.msgbox(message, height, width=self.width) + self.dialog.msgbox(message) def menu(self, message, choices, ok_label="OK", cancel_label="Cancel", help_label="", **unused_kwargs): @@ -170,11 +170,7 @@ class NcursesDisplay(object): `string` - input entered by the user """ - sections = message.split("\n") - # each section takes at least one line, plus extras if it's longer than self.width - wordlines = [1 + (len(section) / self.width) for section in sections] - height = 6 + sum(wordlines) + len(sections) - return _clean(self.dialog.inputbox(message, width=self.width, height=height)) + return self.dialog.inputbox(message) def yesno(self, message, yes_label="Yes", no_label="No", **unused_kwargs): """Display a Yes/No dialog box. @@ -191,8 +187,7 @@ class NcursesDisplay(object): """ return self.dialog.DIALOG_OK == self.dialog.yesno( - message, self.height, self.width, - yes_label=yes_label, no_label=no_label) + message, yes_label=yes_label, no_label=no_label) def checklist(self, message, tags, default_status=True, **unused_kwargs): """Displays a checklist. @@ -210,8 +205,7 @@ class NcursesDisplay(object): """ choices = [(tag, "", default_status) for tag in tags] - return _clean(self.dialog.checklist( - message, width=self.width, height=self.height, choices=choices)) + return self.dialog.checklist(message, choices=choices) def directory_select(self, message, **unused_kwargs): """Display a directory selection screen. @@ -224,9 +218,8 @@ class NcursesDisplay(object): """ root_directory = os.path.abspath(os.sep) - return _clean(self.dialog.dselect( - filepath=root_directory, width=self.width, - height=self.height, help_button=True, title=message)) + return self.dialog.dselect( + filepath=root_directory, help_button=True, title=message) @zope.interface.implementer(interfaces.IDisplay) diff --git a/certbot/tests/display/util_test.py b/certbot/tests/display/util_test.py index a6ced90ab..00ee6be36 100644 --- a/certbot/tests/display/util_test.py +++ b/certbot/tests/display/util_test.py @@ -107,8 +107,7 @@ class NcursesDisplayTest(unittest.TestCase): self.assertTrue(self.displayer.yesno("message")) mock_yesno.assert_called_with( - "message", display_util.HEIGHT, display_util.WIDTH, - yes_label="Yes", no_label="No") + "message", yes_label="Yes", no_label="No") @mock.patch("certbot.display.util." "dialog.Dialog.checklist") @@ -121,9 +120,7 @@ class NcursesDisplayTest(unittest.TestCase): (TAGS[1], "", True), (TAGS[2], "", True), ] - mock_checklist.assert_called_with( - "message", width=display_util.WIDTH, height=display_util.HEIGHT, - choices=choices) + mock_checklist.assert_called_with("message", choices=choices) @mock.patch("certbot.display.util.dialog.Dialog.dselect") def test_directory_select(self, mock_dselect): From df42f69d8ce646367aa382f982a2124031567b29 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 26 Jul 2016 18:08:21 -0700 Subject: [PATCH 08/47] Address review comments --- certbot/cli.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 9537af5f3..a93e072e3 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -503,7 +503,9 @@ class HelpfulArgumentParser(object): """Add a new command line argument. :param topics: str or [str] help topic(s) this should be listed under, - or None for "always documented" + or None for "always documented". The first entry + determines where the flag lives in the "--help all" + output (None -> "optional arguments"). :param list *args: the names of this argument flag :param dict **kwargs: various argparse settings for this argument @@ -671,7 +673,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis None, "-t", "--text", dest="text_mode", action="store_true", help="Use the text output instead of the curses UI.") helpful.add( - None, "-n", "--non-interactive", "--noninteractive", + [None, "automation"], "-n", "--non-interactive", "--noninteractive", dest="noninteractive_mode", action="store_true", help="Run without ever asking for user input. This may require " "additional command line flags; the client will try to explain " @@ -701,7 +703,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " if they are defined because they may be necessary to accurately simulate" " renewal. --renew-hook commands are not called.") helpful.add( - None, "--register-unsafely-without-email", action="store_true", + ["register", "automation"], "--register-unsafely-without-email", action="store_true", help="Specifying this flag enables registering an account with no " "email address. This is strongly discouraged, because in the " "event of key loss or account compromise you will irrevocably " @@ -740,7 +742,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "--keep-until-expiring is more appropriate). Also implies " "--expand.") helpful.add( - ["automation", "renew"], + ["automation", "renew", "certonly"], "--allow-subset-of-names", action="store_true", help="When performing domain validation, do not consider it a failure " "if authorizations can not be obtained for a strict subset of " From fbadf7550c81d317aa70a8238389709884049e61 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 26 Jul 2016 19:20:02 -0700 Subject: [PATCH 09/47] Also put -q in certonly --- certbot/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index a93e072e3..22787392d 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -767,7 +767,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="(certbot-auto only) prevent the certbot-auto script from" " upgrading itself to newer released versions") helpful.add( - ["automation", "renew"], + ["automation", "renew", "certonly"], "-q", "--quiet", dest="quiet", action="store_true", help="Silence all output except errors. Useful for automation via cron." " Implies --non-interactive.") From 9ebda1879cbcdf400718ac033113dd10a53c958d Mon Sep 17 00:00:00 2001 From: Peter Date: Thu, 28 Jul 2016 15:43:57 -0700 Subject: [PATCH 10/47] Restructured installation docs. Mainly put everything together in a sensible order in using.rst and pointed to it from README.rst. --- README.rst | 35 +---- docs/using.rst | 405 +++++++++++++++++++++++++------------------------ 2 files changed, 213 insertions(+), 227 deletions(-) diff --git a/README.rst b/README.rst index c71079f9a..007b9b469 100644 --- a/README.rst +++ b/README.rst @@ -36,33 +36,9 @@ If you'd like to contribute to this project please read `Developer Guide Installation ------------ -If ``certbot`` (or ``letsencrypt``) is packaged for your Unix OS (visit -certbot.eff.org_ to find out), you can install it -from there, and run it by typing ``certbot`` (or ``letsencrypt``). Because -not all operating systems have packages yet, we provide a temporary solution -via the ``certbot-auto`` wrapper script, which obtains some dependencies from -your OS and puts others in a python virtual environment:: - - user@webserver:~$ wget https://dl.eff.org/certbot-auto - user@webserver:~$ chmod a+x ./certbot-auto - user@webserver:~$ ./certbot-auto --help - -.. hint:: The certbot-auto download is protected by HTTPS, which is pretty good, but if you'd like to - double check the integrity of the ``certbot-auto`` script, you can use these steps for verification before running it:: - - user@server:~$ wget -N https://dl.eff.org/certbot-auto.asc - user@server:~$ gpg2 --recv-key A2CFB51FA275A7286234E7B24D17C995CD9775F2 - user@server:~$ gpg2 --trusted-key 4D17C995CD9775F2 --verify certbot-auto.asc certbot-auto - -And for full command line help, you can type:: - - ./certbot-auto --help all - -``certbot-auto`` updates to the latest client release automatically. And -since ``certbot-auto`` is a wrapper to ``certbot``, it accepts exactly -the same command line flags and arguments. More details about this script and -other installation methods can be found `in the User Guide -`_. +The easiest way to install Certbot is by visiting certbot.eff.org_, where you can +find the correct installation instructions for many web server and OS combinations. +For more information, see the `User Guide `_. How to run the client --------------------- @@ -71,6 +47,11 @@ In many cases, you can just run ``certbot-auto`` or ``certbot``, and the client will guide you through the process of obtaining and installing certs interactively. +For full command line help, you can type:: + + ./certbot-auto --help all + + You can also tell it exactly what you want it to do from the command line. For instance, if you want to obtain a cert for ``example.com``, ``www.example.com``, and ``other.example.net``, using the Apache plugin to both diff --git a/docs/using.rst b/docs/using.rst index 0945f4faf..fdd235ce0 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -5,35 +5,215 @@ User Guide .. contents:: Table of Contents :local: +.. _installation: + Getting Certbot =============== -To get specific instructions for installing Certbot on your OS, -visit certbot.eff.org_. This is the easiest way to learn how to get -Certbot up and running on your system. - -If you're offline, you can find some general -instructions `in the README / Introduction `__ - -__ installation_ .. _certbot.eff.org: https://certbot.eff.org .. _certbot-auto: https://certbot.eff.org/docs/using.html#certbot-auto -The name of the certbot command -------------------------------- +Certbot is packaged for many common operating systems and web servers. Check whether +``certbot`` (or ``letsencrypt``) is packaged for your web server's OS by visiting +certbot.eff.org_, where you will also find the correct installation instructions for +your system. -Many platforms now have native packages that give you a ``certbot`` or (for -older packages) ``letsencrypt`` command you can run. On others, the -``certbot-auto`` / ``letsencrypt-auto`` installer and wrapper script is a -stand-in. Throughout the documentation, whenever you see references to -``certbot`` script/binary, you should substitute in the name of the command -that certbot.eff.org_ told you to use on your system (``certbot``, -``letsencrypt``, or ``certbot-auto``). +.. Note:: Unless you have very specific requirements, we kindly suggest that you use the Certbot packages provided by your package manager (see certbot.eff.org_). If such packages are not available, we recommend using ``certbot-auto``, which automates the process of installing Certbot on your system. + +The ``certbot`` script on your web server might be named ``letsencrypt`` if your system uses an older package, or ``certbot-auto`` if you used an alternate installation method. Throughout the docs, whenever you see ``certbot``, swap in the correct name as needed. -Plugins -======= +Other installation methods +-------------------------- +If you are offline or your operating system doesn't provide a package, you can use +an alternate method fo install ``certbot``. + +Certbot-Auto +^^^^^^^^^^^^ +The ``certbot-auto`` wrapper script installs Certbot, obtaining some dependencies +from your web server OS and putting others in a python virtual environment. You can +download and run it as follows:: + + user@webserver:~$ wget https://dl.eff.org/certbot-auto + user@webserver:~$ chmod a+x ./certbot-auto + user@webserver:~$ ./certbot-auto --help + +.. hint:: The certbot-auto download is protected by HTTPS, which is pretty good, but if you'd like to + double check the integrity of the ``certbot-auto`` script, you can use these steps for verification before running it:: + + user@server:~$ wget -N https://dl.eff.org/certbot-auto.asc + user@server:~$ gpg2 --recv-key A2CFB51FA275A7286234E7B24D17C995CD9775F2 + user@server:~$ gpg2 --trusted-key 4D17C995CD9775F2 --verify certbot-auto.asc certbot-auto + +The ``certbot-auto`` command updates to the latest client release automatically. +Since ``certbot-auto`` is a wrapper to ``certbot``, it accepts exactly +the same command line flags and arguments. For more information, see +`Certbot command-line options `_. + +Running with Docker +^^^^^^^^^^^^^^^^^^^ + +Docker_ is an amazingly simple and quick way to obtain a +certificate. However, this mode of operation is unable to install +certificates or configure your webserver, because our installer +plugins cannot reach your webserver from inside the Docker container. + +Most users should use the operating system packages (see instructions at +certbot.eff.org_) or, as a fallback, ``certbot-auto``. You should only +use Docker if you are sure you know what you are doing and have a +good reason to do so. + +You should definitely read the :ref:`where-certs` section, in order to +know how to manage the certs +manually. `Our ciphersuites page `__ +provides some information about recommended ciphersuites. If none of +these make much sense to you, you should definitely use the +certbot-auto_ method, which enables you to use installer plugins +that cover both of those hard topics. + +If you're still not convinced and have decided to use this method, +from the server that the domain you're requesting a cert for resolves +to, `install Docker`_, then issue the following command: + +.. code-block:: shell + + sudo docker run -it --rm -p 443:443 -p 80:80 --name certbot \ + -v "/etc/letsencrypt:/etc/letsencrypt" \ + -v "/var/lib/letsencrypt:/var/lib/letsencrypt" \ + quay.io/letsencrypt/letsencrypt:latest certonly + +Running Certbot with the ``certonly`` command will obtain a certificate and place it in the directory +``/etc/letsencrypt/live`` on your system. Because Certonly cannot install the certificate from +within Docker, you must install the certificate manually according to the procedure +recommended by the provider of your webserver. + +For more information about the layout +of the ``/etc/letsencrypt`` directory, see :ref:`where-certs`. + +.. _Docker: https://docker.com +.. _`install Docker`: https://docs.docker.com/userguide/ + + +Operating System Packages +^^^^^^^^^^^^^^^^^^^^^^^^^ + +**FreeBSD** + + * Port: ``cd /usr/ports/security/py-certbot && make install clean`` + * Package: ``pkg install py27-certbot`` + +**OpenBSD** + + * Port: ``cd /usr/ports/security/letsencrypt/client && make install clean`` + * Package: ``pkg_add letsencrypt`` + +**Arch Linux** + +.. code-block:: shell + + sudo pacman -S certbot + +**Debian** + +If you run Debian Stretch or Debian Sid, you can install certbot packages. + +.. code-block:: shell + + sudo apt-get update + sudo apt-get install certbot python-certbot-apache + +If you don't want to use the Apache plugin, you can omit the +``python-certbot-apache`` package. + +Packages exist for Debian Jessie via backports. First you'll have to follow the +instructions at http://backports.debian.org/Instructions/ to enable the Jessie backports +repo, if you have not already done so. Then run: + +.. code-block:: shell + + sudo apt-get install letsencrypt python-letsencrypt-apache -t jessie-backports + +**Fedora** + +.. code-block:: shell + + sudo dnf install letsencrypt + +**Gentoo** + +The official Certbot client is available in Gentoo Portage. If you +want to use the Apache plugin, it has to be installed separately: + +.. code-block:: shell + + emerge -av app-crypt/letsencrypt + emerge -av app-crypt/letsencrypt-apache + +Currently, only the Apache plugin is included in Portage. However, if you +Warning! +You can use Layman to add the mrueg overlay which does include a package for the +Certbot Nginx plugin, however, this plugin is known to be buggy and should only +be used with caution after creating a backup up your Nginx configuration. +We strongly recommend you use the app-crypt/letsencrypt package instead until +the Nginx plugin is ready. + +.. code-block:: shell + + emerge -av app-portage/layman + layman -S + layman -a mrueg + emerge -av app-crypt/letsencrypt-nginx + +When using the Apache plugin, you will run into a "cannot find a cert or key +directive" error if you're sporting the default Gentoo ``httpd.conf``. +You can fix this by commenting out two lines in ``/etc/apache2/httpd.conf`` +as follows: + +Change + +.. code-block:: shell + + + LoadModule ssl_module modules/mod_ssl.so + + +to + +.. code-block:: shell + + # + LoadModule ssl_module modules/mod_ssl.so + # + +For the time being, this is the only way for the Apache plugin to recognise +the appropriate directives when installing the certificate. +Note: this change is not required for the other plugins. + +**Other Operating Systems** + +OS packaging is an ongoing effort. If you'd like to package +Certbot for your distribution of choice please have a +look at the :doc:`packaging`. + + +Installing from source +^^^^^^^^^^^^^^^^^^^^^^ + +Installation from source is only supported for developers and the +whole process is described in the :doc:`contributing`. + +.. warning:: Please do **not** use ``python setup.py install`` or + ``python pip install .``. Please do **not** attempt the + installation commands as superuser/root and/or without virtual + environment, e.g. ``sudo python setup.py install``, ``sudo pip + install``, ``sudo ./venv/bin/...``. These modes of operation might + corrupt your operating system and are **not supported** by the + Certbot team! + + +Getting certificates +==================== The Certbot client supports a number of different "plugins" that can be used to obtain and/or install certificates. Plugins that can obtain a cert @@ -57,7 +237,7 @@ manual_ Y N Helps you obtain a cert by giving you instructions to perf nginx_ Y Y Very experimental and not included in certbot-auto_. =========== ==== ==== =============================================================== -There are many third-party-plugins_ available. +There are also many third-party-plugins_ available. Apache ------ @@ -186,10 +366,10 @@ postfix_ N Y STARTTLS Everywhere is becoming a Certbot Postfix/Exim plu If you're interested, you can also :ref:`write your own plugin `. +.. _renewal: - -Renewal -======= +Renewing certificates +===================== .. note:: Let's Encrypt CA issues short-lived certificates (90 days). Make sure you renew the certificates at least once in 3 @@ -268,8 +448,8 @@ commands into your individual environment. .. _command-line: -Command line options -==================== +Certbot command-line options +============================ Certbot supports a lot of command line options. Here's the full list, from ``certbot --help all``: @@ -388,179 +568,4 @@ give us as much information as possible: - your operating system, including specific version - specify which installation method you've chosen -Other methods of installation -============================= -Running with Docker -------------------- - -Docker_ is an amazingly simple and quick way to obtain a -certificate. However, this mode of operation is unable to install -certificates or configure your webserver, because our installer -plugins cannot reach your webserver from inside the Docker container. - -Most users should use the operating system packages (see instructions at -certbot.eff.org_) or, as a fallback, ``certbot-auto``. You should only -use Docker if you are sure you know what you are doing and have a -good reason to do so. - -You should definitely read the :ref:`where-certs` section, in order to -know how to manage the certs -manually. `Our ciphersuites page `__ -provides some information about recommended ciphersuites. If none of -these make much sense to you, you should definitely use the -certbot-auto_ method, which enables you to use installer plugins -that cover both of those hard topics. - -If you're still not convinced and have decided to use this method, -from the server that the domain you're requesting a cert for resolves -to, `install Docker`_, then issue the following command: - -.. code-block:: shell - - sudo docker run -it --rm -p 443:443 -p 80:80 --name certbot \ - -v "/etc/letsencrypt:/etc/letsencrypt" \ - -v "/var/lib/letsencrypt:/var/lib/letsencrypt" \ - quay.io/letsencrypt/letsencrypt:latest certonly - -Running Certbot with the ``certonly`` command will obtain a certificate and place it in the directory -``/etc/letsencrypt/live`` on your system. Because Certonly cannot install the certificate from -within Docker, you must install the certificate manually according to the procedure -recommended by the provider of your webserver. - -For more information about the layout -of the ``/etc/letsencrypt`` directory, see :ref:`where-certs`. - -.. _Docker: https://docker.com -.. _`install Docker`: https://docs.docker.com/userguide/ - - -Operating System Packages --------------------------- - -**FreeBSD** - - * Port: ``cd /usr/ports/security/py-certbot && make install clean`` - * Package: ``pkg install py27-certbot`` - -**OpenBSD** - - * Port: ``cd /usr/ports/security/letsencrypt/client && make install clean`` - * Package: ``pkg_add letsencrypt`` - -**Arch Linux** - -.. code-block:: shell - - sudo pacman -S certbot - -**Debian** - -If you run Debian Stretch or Debian Sid, you can install certbot packages. - -.. code-block:: shell - - sudo apt-get update - sudo apt-get install certbot python-certbot-apache - -If you don't want to use the Apache plugin, you can omit the -``python-certbot-apache`` package. - -Packages exist for Debian Jessie via backports. First you'll have to follow the -instructions at http://backports.debian.org/Instructions/ to enable the Jessie backports -repo, if you have not already done so. Then run: - -.. code-block:: shell - - sudo apt-get install letsencrypt python-letsencrypt-apache -t jessie-backports - -**Fedora** - -.. code-block:: shell - - sudo dnf install letsencrypt - -**Gentoo** - -The official Certbot client is available in Gentoo Portage. If you -want to use the Apache plugin, it has to be installed separately: - -.. code-block:: shell - - emerge -av app-crypt/letsencrypt - emerge -av app-crypt/letsencrypt-apache - -Currently, only the Apache plugin is included in Portage. However, if you -Warning! -You can use Layman to add the mrueg overlay which does include a package for the -Certbot Nginx plugin, however, this plugin is known to be buggy and should only -be used with caution after creating a backup up your Nginx configuration. -We strongly recommend you use the app-crypt/letsencrypt package instead until -the Nginx plugin is ready. - -.. code-block:: shell - - emerge -av app-portage/layman - layman -S - layman -a mrueg - emerge -av app-crypt/letsencrypt-nginx - -When using the Apache plugin, you will run into a "cannot find a cert or key -directive" error if you're sporting the default Gentoo ``httpd.conf``. -You can fix this by commenting out two lines in ``/etc/apache2/httpd.conf`` -as follows: - -Change - -.. code-block:: shell - - - LoadModule ssl_module modules/mod_ssl.so - - -to - -.. code-block:: shell - - # - LoadModule ssl_module modules/mod_ssl.so - # - -For the time being, this is the only way for the Apache plugin to recognise -the appropriate directives when installing the certificate. -Note: this change is not required for the other plugins. - -**Other Operating Systems** - -OS packaging is an ongoing effort. If you'd like to package -Certbot for your distribution of choice please have a -look at the :doc:`packaging`. - - -From source ------------ - -Installation from source is only supported for developers and the -whole process is described in the :doc:`contributing`. - -.. warning:: Please do **not** use ``python setup.py install`` or - ``python pip install .``. Please do **not** attempt the - installation commands as superuser/root and/or without virtual - environment, e.g. ``sudo python setup.py install``, ``sudo pip - install``, ``sudo ./venv/bin/...``. These modes of operation might - corrupt your operating system and are **not supported** by the - Certbot team! - - -Comparison of different methods -------------------------------- - -Unless you have very specific requirements, we kindly suggest that you use -the Certbot packages provided by your package manager (see certbot.eff.org_). -If such packages are not available, we recommend using ``certbot-auto``, which -automates the process of installing Certbot on your system. - -Beyond the methods discussed here, other methods may be possible, such as -installing Certbot directly with pip from PyPI or downloading a ZIP -archive from GitHub may be technically possible but are not presently -recommended or supported. From 89f576babb88722f3273fe770971a4942b23ac96 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 29 Jul 2016 16:51:33 -0700 Subject: [PATCH 11/47] Primarily simple s/apache/nginx/ and the like --- .../configurators/nginx/Dockerfile | 20 +++ .../configurators/nginx/__init__.py | 1 + .../configurators/nginx/common.py | 153 ++++++++++++++++++ .../certbot_compatibility_test/test_driver.py | 5 +- 4 files changed, 177 insertions(+), 2 deletions(-) create mode 100644 certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile create mode 100644 certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py create mode 100644 certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile new file mode 100644 index 000000000..ea9bb857f --- /dev/null +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile @@ -0,0 +1,20 @@ +FROM httpd +MAINTAINER Brad Warren + +RUN mkdir /var/run/apache2 + +ENV APACHE_RUN_USER=daemon \ + APACHE_RUN_GROUP=daemon \ + APACHE_PID_FILE=/usr/local/apache2/logs/httpd.pid \ + APACHE_RUN_DIR=/var/run/apache2 \ + APACHE_LOCK_DIR=/var/lock \ + APACHE_LOG_DIR=/usr/local/apache2/logs + +COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2enmod.sh /usr/local/bin/ +COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2dismod.sh /usr/local/bin/ +COPY certbot-compatibility-test/certbot_compatibility_test/testdata/rsa1024_key2.pem /usr/local/apache2/conf/ +COPY certbot-compatibility-test/certbot_compatibility_test/testdata/empty_cert.pem /usr/local/apache2/conf/ + +# Note: this only exposes the port to other docker containers. You +# still have to bind to 443@host at runtime. +EXPOSE 443 diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py new file mode 100644 index 000000000..d559d0645 --- /dev/null +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py @@ -0,0 +1 @@ +"""Certbot compatibility test Apache configurators""" diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py new file mode 100644 index 000000000..29f46ca4f --- /dev/null +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -0,0 +1,153 @@ +"""Provides a common base for Apache proxies""" +import re +import os +import shutil +import subprocess + +import mock +import zope.interface + +from certbot import configuration +from certbot import errors as le_errors +from certbot_nginx import configurator +from certbot_nginx import constants +from certbot_compatibility_test import errors +from certbot_compatibility_test import interfaces +from certbot_compatibility_test import util +from certbot_compatibility_test.configurators import common as configurators_common + + +# APACHE_VERSION_REGEX = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE) +# XXX APACHE_COMMANDS = ["apachectl", "a2enmod", "a2dismod"] + + +@zope.interface.implementer(interfaces.IConfiguratorProxy) +class Proxy(configurators_common.Proxy): + # pylint: disable=too-many-instance-attributes + """A common base for Nginx test configurators""" + + def __init__(self, args): + # XXX: This is still apache-specific + """Initializes the plugin with the given command line args""" + super(Proxy, self).__init__(args) + self.le_config.apache_le_vhost_ext = "-le-ssl.conf" + + self.modules = self.server_root = self.test_conf = self.version = None + self._nginx_configurator = self._all_names = self._test_names = None + patch = mock.patch( + "certbot_apache.configurator.display_ops.select_vhost") + mock_display = patch.start() + mock_display.side_effect = le_errors.PluginError( + "Unable to determine vhost") + + def __getattr__(self, name): + """Wraps the Nginx Configurator methods""" + method = getattr(self._nginx_configurator, name, None) + if callable(method): + return method + else: + raise AttributeError() + + def load_config(self): + """Loads the next configuration for the plugin to test""" + + config = super(Proxy, self).load_config() + self._all_names, self._test_names = _get_names(config) + + server_root = _get_server_root(config) + # with open(os.path.join(config, "config_file")) as f: + # config_file = os.path.join(server_root, f.readline().rstrip()) + + # XXX: Deleting all of this is kind of scary unless the test + # instances really each have a complete configuration! + shutil.rmtree("/etc/nginx") + shutil.copytree(server_root, "/etc/nginx", symlinks=True) + + self._prepare_configurator() + + try: + subprocess.check_call("nginx".split()) + except errors.Error: + raise errors.Error( + "Nginx failed to load {0} before tests started".format( + config)) + + return config + + def _prepare_configurator(self): + """Prepares the Nginx plugin for testing""" + for k in constants.CLI_DEFAULTS.keys(): + setattr(self.le_config, "nginx_" + k, constants.os_constant(k)) + + # An alias + self.le_config.nginx_handle_modules = self.le_config.nginx_handle_mods + + self._nginx_configurator = configurator.NginxConfigurator( + config=configuration.NamespaceConfig(self.le_config), + name="nginx") + self._nginx_configurator.prepare() + + def cleanup_from_tests(self): + """Performs any necessary cleanup from running plugin tests""" + super(Proxy, self).cleanup_from_tests() + mock.patch.stopall() + + def get_all_names_answer(self): + """Returns the set of domain names that the plugin should find""" + if self._all_names: + return self._all_names + else: + raise errors.Error("No configuration file loaded") + + def get_testable_domain_names(self): + """Returns the set of domain names that can be tested against""" + if self._test_names: + return self._test_names + else: + return {"example.com"} + + def deploy_cert(self, domain, cert_path, key_path, chain_path=None, + fullchain_path=None): + """Installs cert""" + cert_path, key_path, chain_path = self.copy_certs_and_keys( + cert_path, key_path, chain_path) + self._nginx_configurator.deploy_cert( + domain, cert_path, key_path, chain_path, fullchain_path) + + +def _get_server_root(config): + """Returns the server root directory in config""" + subdirs = [ + name for name in os.listdir(config) + if os.path.isdir(os.path.join(config, name))] + + if len(subdirs) != 1: + errors.Error("Malformed configuration directory {0}".format(config)) + + return os.path.join(config, subdirs[0].rstrip()) + + +def _get_names(config): + """Returns all and testable domain names in config""" + # XXX: This is still Apache-specific + all_names = set() + non_ip_names = set() + with open(os.path.join(config, "vhosts")) as f: + for line in f: + # If parsing a specific vhost + if line[0].isspace(): + words = line.split() + if words[0] == "alias": + all_names.add(words[1]) + non_ip_names.add(words[1]) + # If for port 80 and not IP vhost + elif words[1] == "80" and not util.IP_REGEX.match(words[3]): + all_names.add(words[3]) + non_ip_names.add(words[3]) + elif "NameVirtualHost" not in line: + words = line.split() + if (words[0].endswith("*") or words[0].endswith("80") and + not util.IP_REGEX.match(words[1]) and + words[1].find(".") != -1): + all_names.add(words[1]) + return all_names, non_ip_names diff --git a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py index 2c78eea1f..3d03f7771 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py +++ b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py @@ -22,7 +22,8 @@ from certbot_compatibility_test import errors from certbot_compatibility_test import util from certbot_compatibility_test import validator -from certbot_compatibility_test.configurators.apache import common +from certbot_compatibility_test.configurators.apache import common as a_common +from certbot_compatibility_test.configurators.nginx import common as n_common DESCRIPTION = """ @@ -32,7 +33,7 @@ tests that the plugin supports are performed. """ -PLUGINS = {"apache": common.Proxy} +PLUGINS = {"apache": a_common.Proxy, "nginx": n_common.Proxy} logger = logging.getLogger(__name__) From 7b67ba6797ce41aad1ec70e0ad894b90204c51f0 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 29 Jul 2016 17:14:23 -0700 Subject: [PATCH 12/47] Remove unused Apache-related variables --- .../certbot_compatibility_test/configurators/apache/common.py | 4 ---- .../certbot_compatibility_test/configurators/nginx/common.py | 4 ---- 2 files changed, 8 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py index ed3d9d67a..0c53058de 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py @@ -17,10 +17,6 @@ from certbot_compatibility_test import util from certbot_compatibility_test.configurators import common as configurators_common -APACHE_VERSION_REGEX = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE) -APACHE_COMMANDS = ["apachectl", "a2enmod", "a2dismod"] - - @zope.interface.implementer(interfaces.IConfiguratorProxy) class Proxy(configurators_common.Proxy): # pylint: disable=too-many-instance-attributes diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 29f46ca4f..c474d078c 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -17,10 +17,6 @@ from certbot_compatibility_test import util from certbot_compatibility_test.configurators import common as configurators_common -# APACHE_VERSION_REGEX = re.compile(r"Apache/([0-9\.]*)", re.IGNORECASE) -# XXX APACHE_COMMANDS = ["apachectl", "a2enmod", "a2dismod"] - - @zope.interface.implementer(interfaces.IConfiguratorProxy) class Proxy(configurators_common.Proxy): # pylint: disable=too-many-instance-attributes From 353cb6e6c627e680c8e573a101a3548c193fc769 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 3 Aug 2016 17:10:20 -0700 Subject: [PATCH 13/47] New _get_names approach for nginx test --- .../configurators/nginx/common.py | 27 +++++-------------- 1 file changed, 7 insertions(+), 20 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index c474d078c..779ba26a7 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -125,25 +125,12 @@ def _get_server_root(config): def _get_names(config): """Returns all and testable domain names in config""" - # XXX: This is still Apache-specific all_names = set() - non_ip_names = set() - with open(os.path.join(config, "vhosts")) as f: - for line in f: - # If parsing a specific vhost - if line[0].isspace(): - words = line.split() - if words[0] == "alias": - all_names.add(words[1]) - non_ip_names.add(words[1]) - # If for port 80 and not IP vhost - elif words[1] == "80" and not util.IP_REGEX.match(words[3]): - all_names.add(words[3]) - non_ip_names.add(words[3]) - elif "NameVirtualHost" not in line: - words = line.split() - if (words[0].endswith("*") or words[0].endswith("80") and - not util.IP_REGEX.match(words[1]) and - words[1].find(".") != -1): - all_names.add(words[1]) + for root, _dirs, files in os.walk(config): + for this_file in files: + for line in open(os.path.join(root, this_file)): + if line.strip().starts_with("server_name"): + names = line.partition("server_name")[2].rstrip(";") + [all_names.add(n) for n in names.split()] + non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n)) return all_names, non_ip_names From 2cd2228ca6a8e3e2b311d5974afa816084695bb7 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Aug 2016 15:07:35 -0700 Subject: [PATCH 14/47] starts_with is actually called startswith --- .../certbot_compatibility_test/configurators/nginx/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 779ba26a7..9fbb538e8 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -129,7 +129,7 @@ def _get_names(config): for root, _dirs, files in os.walk(config): for this_file in files: for line in open(os.path.join(root, this_file)): - if line.strip().starts_with("server_name"): + if line.strip().startswith("server_name"): names = line.partition("server_name")[2].rstrip(";") [all_names.add(n) for n in names.split()] non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n)) From ae6ca4d4ca932dd364b7c6367c96cff25987fad6 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Fri, 5 Aug 2016 15:13:04 -0700 Subject: [PATCH 15/47] Minimal fake os_constant() for nginx constants.py --- certbot-nginx/certbot_nginx/constants.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/certbot-nginx/certbot_nginx/constants.py b/certbot-nginx/certbot_nginx/constants.py index 5dde30efc..453266878 100644 --- a/certbot-nginx/certbot_nginx/constants.py +++ b/certbot-nginx/certbot_nginx/constants.py @@ -16,3 +16,10 @@ MOD_SSL_CONF_SRC = pkg_resources.resource_filename( "certbot_nginx", "options-ssl-nginx.conf") """Path to the nginx mod_ssl config file found in the Certbot distribution.""" + +def os_constant(key): + # XXX TODO: In the future, this could return different constants + # based on what OS we are running under. To see an + # approach to how to handle different OSes, see the + # apache version of this file. + return CLI_DEFAULTS From fe76d558edcd493cc302060761426dc4b6cc0248 Mon Sep 17 00:00:00 2001 From: Yen Chi Hsuan Date: Sat, 6 Aug 2016 20:31:21 +0800 Subject: [PATCH 16/47] Enable unit tests of certbot core on Python 3 --- certbot/account.py | 3 ++- certbot/client.py | 5 ++--- certbot/crypto_util.py | 25 ++++++++++++++++--------- certbot/display/ops.py | 7 ++++++- certbot/display/util.py | 9 +++++---- certbot/plugins/disco.py | 2 +- certbot/plugins/disco_test.py | 3 ++- certbot/plugins/manual_test.py | 2 +- certbot/plugins/selection.py | 3 ++- certbot/plugins/util_test.py | 4 ++-- certbot/plugins/webroot.py | 2 +- certbot/reverter.py | 12 +++++++----- certbot/storage.py | 2 +- certbot/tests/account_test.py | 8 ++++---- certbot/tests/acme_util.py | 9 +++++---- certbot/tests/auth_handler_test.py | 17 +++++++++-------- certbot/tests/cli_test.py | 23 ++++++++++++----------- certbot/tests/client_test.py | 4 ++-- certbot/tests/configuration_test.py | 2 ++ certbot/tests/crypto_util_test.py | 4 ++-- certbot/tests/display/util_test.py | 24 ++++++++++++------------ certbot/tests/reverter_test.py | 4 ++-- certbot/tests/storage_test.py | 29 +++++++++++++++-------------- certbot/tests/util_test.py | 12 +++++++++--- certbot/util.py | 20 ++++++++++++++------ tox.ini | 6 ++++++ 26 files changed, 142 insertions(+), 99 deletions(-) diff --git a/certbot/account.py b/certbot/account.py index 798f8664e..71951d8e0 100644 --- a/certbot/account.py +++ b/certbot/account.py @@ -8,6 +8,7 @@ import socket from cryptography.hazmat.primitives import serialization import pyrfc3339 import pytz +import six import zope.component from acme import fields as acme_fields @@ -108,7 +109,7 @@ class AccountMemoryStorage(interfaces.AccountStorage): self.accounts = initial_accounts if initial_accounts is not None else {} def find_all(self): - return self.accounts.values() + return list(six.itervalues(self.accounts)) def save(self, account): if account.id in self.accounts: diff --git a/certbot/client.py b/certbot/client.py index 0f414b474..119fb0947 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -246,8 +246,7 @@ class Client(object): domains, self.config.allow_subset_of_names) - auth_domains = set(a.body.identifier.value.encode('ascii') - for a in authzr) + auth_domains = set(a.body.identifier.value for a in authzr) domains = [d for d in domains if d in auth_domains] # Create CSR from names @@ -317,7 +316,7 @@ class Client(object): self.config.strict_permissions) cert_pem = OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) + OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped).decode('ascii') cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path) diff --git a/certbot/crypto_util.py b/certbot/crypto_util.py index 1e831dd8f..7253742b0 100644 --- a/certbot/crypto_util.py +++ b/certbot/crypto_util.py @@ -10,6 +10,7 @@ import traceback import OpenSSL import pyrfc3339 +import six import zope.component from acme import crypto_util as acme_crypto_util @@ -115,16 +116,16 @@ def make_csr(key_str, domains, must_staple=False): # TODO: put SAN if len(domains) > 1 extensions = [ OpenSSL.crypto.X509Extension( - "subjectAltName", + b"subjectAltName", critical=False, - value=", ".join("DNS:%s" % d for d in domains) + value=", ".join("DNS:%s" % d for d in domains).encode('ascii') ) ] if must_staple: extensions.append(OpenSSL.crypto.X509Extension( - "1.3.6.1.5.5.7.1.24", + b"1.3.6.1.5.5.7.1.24", critical=False, - value="DER:30:03:02:01:05")) + value=b"DER:30:03:02:01:05")) req.add_extensions(extensions) req.set_version(2) req.set_pubkey(pkey) @@ -350,7 +351,7 @@ def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM): if isinstance(cert, jose.ComparableX509): # pylint: disable=protected-access cert = cert.wrapped - return OpenSSL.crypto.dump_certificate(filetype, cert) + return OpenSSL.crypto.dump_certificate(filetype, cert).decode('ascii') # assumes that OpenSSL.crypto.dump_certificate includes ending # newline character @@ -395,8 +396,14 @@ def _notAfterBefore(cert_path, method): with open(cert_path) as f: x509 = OpenSSL.crypto.load_certificate(OpenSSL.crypto.FILETYPE_PEM, f.read()) + # pyopenssl always returns bytes timestamp = method(x509) - reformatted_timestamp = [timestamp[0:4], "-", timestamp[4:6], "-", - timestamp[6:8], "T", timestamp[8:10], ":", - timestamp[10:12], ":", timestamp[12:]] - return pyrfc3339.parse("".join(reformatted_timestamp)) + reformatted_timestamp = [timestamp[0:4], b"-", timestamp[4:6], b"-", + timestamp[6:8], b"T", timestamp[8:10], b":", + timestamp[10:12], b":", timestamp[12:]] + timestamp_str = b"".join(reformatted_timestamp) + # pyrfc3339 uses "native" strings. That is, bytes on Python 2 and unicode + # on Python 3 + if six.PY3: + timestamp_str = timestamp_str.decode('ascii') + return pyrfc3339.parse(timestamp_str) diff --git a/certbot/display/ops.py b/certbot/display/ops.py index c7e566256..4db6d71e2 100644 --- a/certbot/display/ops.py +++ b/certbot/display/ops.py @@ -180,7 +180,12 @@ def _choose_names_manually(): try: domain_list[i] = util.enforce_domain_sanity(domain) except errors.ConfigurationError as e: - invalid_domains[domain] = e.message + try: # Python 2 + # pylint: disable=no-member + err_msg = e.message.encode('utf-8') + except AttributeError: + err_msg = str(e) + invalid_domains[domain] = err_msg if len(invalid_domains): retry_message = ( diff --git a/certbot/display/util.py b/certbot/display/util.py index 39486b2bd..e2624395c 100644 --- a/certbot/display/util.py +++ b/certbot/display/util.py @@ -4,6 +4,7 @@ import os import textwrap import dialog +import six import zope.interface from certbot import interfaces @@ -253,7 +254,7 @@ class FileDisplay(object): "{line}{frame}{line}{msg}{line}{frame}{line}".format( line=os.linesep, frame=side_frame, msg=message)) if pause: - raw_input("Press Enter to Continue") + six.moves.input("Press Enter to Continue") def menu(self, message, choices, ok_label="", cancel_label="", help_label="", **unused_kwargs): @@ -295,7 +296,7 @@ class FileDisplay(object): :rtype: tuple """ - ans = raw_input( + ans = six.moves.input( textwrap.fill( "%s (Enter 'c' to cancel): " % message, 80, @@ -330,7 +331,7 @@ class FileDisplay(object): os.linesep, frame=side_frame, msg=message)) while True: - ans = raw_input("{yes}/{no}: ".format( + ans = six.moves.input("{yes}/{no}: ".format( yes=_parens_around_char(yes_label), no=_parens_around_char(no_label))) @@ -468,7 +469,7 @@ class FileDisplay(object): input_msg = ("Press 1 [enter] to confirm the selection " "(press 'c' to cancel): ") while selection < 1: - ans = raw_input(input_msg) + ans = six.moves.input(input_msg) if ans.startswith("c") or ans.startswith("C"): return CANCEL, -1 try: diff --git a/certbot/plugins/disco.py b/certbot/plugins/disco.py index 59410757c..a6e8e7ed7 100644 --- a/certbot/plugins/disco.py +++ b/certbot/plugins/disco.py @@ -255,4 +255,4 @@ class PluginsRegistry(collections.Mapping): def __str__(self): if not self._plugins: return "No plugins" - return "\n\n".join(str(p_ep) for p_ep in self._plugins.itervalues()) + return "\n\n".join(str(p_ep) for p_ep in six.itervalues(self._plugins)) diff --git a/certbot/plugins/disco_test.py b/certbot/plugins/disco_test.py index cef6ede8f..509110d55 100644 --- a/certbot/plugins/disco_test.py +++ b/certbot/plugins/disco_test.py @@ -3,6 +3,7 @@ import unittest import mock import pkg_resources +import six import zope.interface from certbot import errors @@ -50,7 +51,7 @@ class PluginEntryPointTest(unittest.TestCase): EP_SA: "sa", } - for entry_point, name in names.iteritems(): + for entry_point, name in six.iteritems(names): self.assertEqual( name, PluginEntryPoint.entry_point_to_plugin_name(entry_point)) diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index af1dc9909..dd0905049 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -51,7 +51,7 @@ class AuthenticatorTest(unittest.TestCase): @mock.patch("certbot.plugins.manual.zope.component.getUtility") @mock.patch("certbot.plugins.manual.sys.stdout") @mock.patch("acme.challenges.HTTP01Response.simple_verify") - @mock.patch("__builtin__.raw_input") + @mock.patch("six.moves.input") def test_perform(self, mock_raw_input, mock_verify, mock_stdout, mock_interaction): mock_verify.return_value = True mock_interaction().yesno.return_value = True diff --git a/certbot/plugins/selection.py b/certbot/plugins/selection.py index b16515d8f..a7388c4e1 100644 --- a/certbot/plugins/selection.py +++ b/certbot/plugins/selection.py @@ -4,6 +4,7 @@ from __future__ import print_function import os import logging +import six import zope.component from certbot import errors @@ -78,7 +79,7 @@ def pick_plugin(config, default, plugins, question, ifaces): if len(prepared) > 1: logger.debug("Multiple candidate plugins: %s", prepared) - plugin_ep = choose_plugin(prepared.values(), question) + plugin_ep = choose_plugin(list(six.itervalues(prepared)), question) if plugin_ep is None: return None else: diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index a9466ed63..e1a064fb3 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -63,7 +63,7 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): def test_ports_available(self, mock_getutil): import certbot.plugins.util as plugins_util # Ensure we don't get error - with mock.patch("socket._socketobject.bind"): + with mock.patch("socket.socket.bind"): self.assertFalse(plugins_util.already_listening(80)) self.assertFalse(plugins_util.already_listening(80, True)) self.assertEqual(mock_getutil.call_count, 0) @@ -73,7 +73,7 @@ class AlreadyListeningTestNoPsutil(unittest.TestCase): sys.modules["psutil"] = None import certbot.plugins.util as plugins_util import socket - with mock.patch("socket._socketobject.bind", side_effect=socket.error): + with mock.patch("socket.socket.bind", side_effect=socket.error): self.assertTrue(plugins_util.already_listening(80)) self.assertTrue(plugins_util.already_listening(80, True)) with mock.patch("socket.socket", side_effect=socket.error): diff --git a/certbot/plugins/webroot.py b/certbot/plugins/webroot.py index 624ee2ff4..1cd1d879a 100644 --- a/certbot/plugins/webroot.py +++ b/certbot/plugins/webroot.py @@ -206,7 +206,7 @@ to serve all files under specified web root ({0}).""" old_umask = os.umask(0o022) try: - with open(validation_path, "w") as validation_file: + with open(validation_path, "wb") as validation_file: validation_file.write(validation.encode()) finally: os.umask(old_umask) diff --git a/certbot/reverter.py b/certbot/reverter.py index 1c404e29b..098c74911 100644 --- a/certbot/reverter.py +++ b/certbot/reverter.py @@ -7,7 +7,7 @@ import shutil import time import traceback - +import six import zope.component from certbot import constants @@ -310,7 +310,9 @@ class Reverter(object): def _run_undo_commands(self, filepath): # pylint: disable=no-self-use """Run all commands in a file.""" - with open(filepath, 'rb') as csvfile: + # NOTE: csv module uses native strings. That is, bytes on Python 2 and + # unicode on Python 3 + with open(filepath, 'r') as csvfile: csvreader = csv.reader(csvfile) for command in reversed(list(csvreader)): try: @@ -408,9 +410,9 @@ class Reverter(object): command_file = None try: if os.path.isfile(commands_fp): - command_file = open(commands_fp, "ab") + command_file = open(commands_fp, "a") else: - command_file = open(commands_fp, "wb") + command_file = open(commands_fp, "w") csvwriter = csv.writer(command_file) csvwriter.writerow(command) @@ -569,7 +571,7 @@ class Reverter(object): # It is possible save checkpoints faster than 1 per second resulting in # collisions in the naming convention. - for _ in xrange(2): + for _ in six.moves.range(2): timestamp = self._checkpoint_timestamp() final_dir = os.path.join(self.config.backup_dir, timestamp) try: diff --git a/certbot/storage.py b/certbot/storage.py index 82fdbfd54..5ca2ff6a9 100644 --- a/certbot/storage.py +++ b/certbot/storage.py @@ -90,7 +90,7 @@ def write_renewal_config(o_filename, n_filename, target, relevant_data): # TODO: add human-readable comments explaining other available # parameters logger.debug("Writing new config %s.", n_filename) - with open(n_filename, "w") as f: + with open(n_filename, "wb") as f: config.write(outfile=f) return config diff --git a/certbot/tests/account_test.py b/certbot/tests/account_test.py index 4cd2bfebf..41b835838 100644 --- a/certbot/tests/account_test.py +++ b/certbot/tests/account_test.py @@ -131,8 +131,8 @@ class AccountFileStorageTest(unittest.TestCase): for file_name in "regr.json", "meta.json", "private_key.json": self.assertTrue(os.path.exists( os.path.join(account_path, file_name))) - self.assertEqual("0400", oct(os.stat(os.path.join( - account_path, "private_key.json"))[stat.ST_MODE] & 0o777)) + self.assertTrue(oct(os.stat(os.path.join( + account_path, "private_key.json"))[stat.ST_MODE] & 0o777) in ("0400", "0o400")) # restore self.assertEqual(self.acc, self.storage.load(self.acc.id)) @@ -179,14 +179,14 @@ class AccountFileStorageTest(unittest.TestCase): self.storage.save(self.acc) mock_open = mock.mock_open() mock_open.side_effect = IOError - with mock.patch("__builtin__.open", mock_open): + with mock.patch("six.moves.builtins.open", mock_open): self.assertRaises( errors.AccountStorageError, self.storage.load, self.acc.id) def test_save_ioerrors(self): mock_open = mock.mock_open() mock_open.side_effect = IOError # TODO: [None, None, IOError] - with mock.patch("__builtin__.open", mock_open): + with mock.patch("six.moves.builtins.open", mock_open): self.assertRaises( errors.AccountStorageError, self.storage.save, self.acc) diff --git a/certbot/tests/acme_util.py b/certbot/tests/acme_util.py index 3d33c5723..4f6e86cc7 100644 --- a/certbot/tests/acme_util.py +++ b/certbot/tests/acme_util.py @@ -1,6 +1,7 @@ """ACME utilities for testing.""" import datetime -import itertools + +import six from acme import challenges from acme import jose @@ -13,10 +14,10 @@ KEY = test_util.load_rsa_private_key('rsa512_key.pem') # Challenges HTTP01 = challenges.HTTP01( - token="evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+oA") + token=b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+oA") TLSSNI01 = challenges.TLSSNI01( token=jose.b64decode(b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJyPCt92wrDoA")) -DNS = challenges.DNS(token="17817c66b60ce2e4012dfad92657527a") +DNS = challenges.DNS(token=b"17817c66b60ce2e4012dfad92657527a") CHALLENGES = [HTTP01, TLSSNI01, DNS] @@ -62,7 +63,7 @@ def gen_authzr(authz_status, domain, challs, statuses, combos=True): # pylint: disable=redefined-outer-name challbs = tuple( chall_to_challb(chall, status) - for chall, status in itertools.izip(challs, statuses) + for chall, status in six.moves.zip(challs, statuses) ) authz_kwargs = { "identifier": messages.Identifier( diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index eccc36418..fce130f7c 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -4,6 +4,7 @@ import logging import unittest import mock +import six from acme import challenges from acme import client as acme_client @@ -93,7 +94,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertEqual(mock_poll.call_count, 1) chall_update = mock_poll.call_args[0][0] - self.assertEqual(chall_update.keys(), ["0"]) + self.assertEqual(list(six.iterkeys(chall_update)), ["0"]) self.assertEqual(len(chall_update.values()), 1) self.assertEqual(self.mock_auth.cleanup.call_count, 1) @@ -118,7 +119,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertEqual(mock_poll.call_count, 1) chall_update = mock_poll.call_args[0][0] - self.assertEqual(chall_update.keys(), ["0"]) + self.assertEqual(list(six.iterkeys(chall_update)), ["0"]) self.assertEqual(len(chall_update.values()), 1) self.assertEqual(self.mock_auth.cleanup.call_count, 1) @@ -143,12 +144,12 @@ class GetAuthorizationsTest(unittest.TestCase): # Check poll call self.assertEqual(mock_poll.call_count, 1) chall_update = mock_poll.call_args[0][0] - self.assertEqual(len(chall_update.keys()), 3) - self.assertTrue("0" in chall_update.keys()) + self.assertEqual(len(list(six.iterkeys(chall_update))), 3) + self.assertTrue("0" in list(six.iterkeys(chall_update))) self.assertEqual(len(chall_update["0"]), 1) - self.assertTrue("1" in chall_update.keys()) + self.assertTrue("1" in list(six.iterkeys(chall_update))) self.assertEqual(len(chall_update["1"]), 1) - self.assertTrue("2" in chall_update.keys()) + self.assertTrue("2" in list(six.iterkeys(chall_update))) self.assertEqual(len(chall_update["2"]), 1) self.assertEqual(self.mock_auth.cleanup.call_count, 1) @@ -167,7 +168,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertRaises(errors.AuthorizationError, self.handler.get_authorizations, []) def _validate_all(self, unused_1, unused_2): - for dom in self.handler.authzr.keys(): + for dom in six.iterkeys(self.handler.authzr): azr = self.handler.authzr[dom] self.handler.authzr[dom] = acme_util.gen_authzr( messages.STATUS_VALID, @@ -317,7 +318,7 @@ class GenChallengePathTest(unittest.TestCase): """ def setUp(self): - logging.disable(logging.fatal) + logging.disable(logging.FATAL) def tearDown(self): logging.disable(logging.NOTSET) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index cb32975e4..8cd879144 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -136,7 +136,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods try: with mock.patch('certbot.main.sys.stderr'): main.main(self.standard_args + args[:]) # NOTE: parser can alter its args! - except errors.MissingCommandlineFlag as exc: + except errors.MissingCommandlineFlag as exc_: + exc = exc_ self.assertTrue(message in str(exc)) self.assertTrue(exc is not None) @@ -263,7 +264,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods flags = ['--init', '--prepare', '--authenticators', '--installers'] for args in itertools.chain( *(itertools.combinations(flags, r) - for r in xrange(len(flags)))): + for r in six.moves.range(len(flags)))): self._call(['plugins'] + list(args)) @mock.patch('certbot.main.plugins_disco') @@ -332,7 +333,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._call(['-a', 'bad_auth', 'certonly']) assert False, "Exception should have been raised" except errors.PluginSelectionError as e: - self.assertTrue('The requested bad_auth plugin does not appear' in e.message) + self.assertTrue('The requested bad_auth plugin does not appear' in str(e)) def test_check_config_sanity_domain(self): # Punycode @@ -427,9 +428,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods "The following flags didn't conflict with " '--server: {0}'.format(', '.join(conflicting_args))) except errors.Error as error: - self.assertTrue('--server' in error.message) + self.assertTrue('--server' in str(error)) for arg in conflicting_args: - self.assertTrue(arg in error.message) + self.assertTrue(arg in str(error)) def test_must_staple_flag(self): parse = self._get_argument_parser() @@ -855,10 +856,10 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods server = 'foo.bar' self._call_no_clientmock(['--cert-path', CERT, '--key-path', KEY, '--server', server, 'revoke']) - with open(KEY) as f: + with open(KEY, 'rb') as f: mock_acme_client.Client.assert_called_once_with( server, key=jose.JWK.load(f.read()), net=mock.ANY) - with open(CERT) as f: + with open(CERT, 'rb') as f: cert = crypto_util.pyopenssl_load_certificate(f.read())[0] mock_revoke = mock_acme_client.Client().revoke mock_revoke.assert_called_once_with(jose.ComparableX509(cert)) @@ -885,7 +886,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods config.verbose_count = 1 main._handle_exception( Exception, exc_value=exception, trace=None, config=None) - mock_open().write.assert_called_once_with(''.join( + mock_open().write.assert_any_call(''.join( traceback.format_exception_only(Exception, exception))) error_msg = mock_sys.exit.call_args_list[0][0][0] self.assertTrue('unexpected error' in error_msg) @@ -935,8 +936,8 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises( argparse.ArgumentTypeError, cli.read_file, rel_test_path) - test_contents = 'bar\n' - with open(rel_test_path, 'w') as f: + test_contents = b'bar\n' + with open(rel_test_path, 'wb') as f: f.write(test_contents) path, contents = cli.read_file(rel_test_path) @@ -1106,7 +1107,7 @@ class DuplicativeCertsTest(storage_test.BaseRenewableCertTest): def test_find_duplicative_names(self, unused_makedir): from certbot.main import _find_duplicative_certs test_cert = test_util.load_vector('cert-san.pem') - with open(self.test_rc.cert, 'w') as f: + with open(self.test_rc.cert, 'wb') as f: f.write(test_cert) # No overlap at all diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index e7ae6bbd1..4a8a8bdee 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -232,11 +232,11 @@ class ClientTest(unittest.TestCase): self.assertEqual(os.path.dirname(fullchain_path), os.path.dirname(candidate_fullchain_path)) - with open(cert_path, "r") as cert_file: + with open(cert_path, "rb") as cert_file: cert_contents = cert_file.read() self.assertEqual(cert_contents, test_util.load_vector(certs[0])) - with open(chain_path, "r") as chain_file: + with open(chain_path, "rb") as chain_file: chain_contents = chain_file.read() self.assertEqual(chain_contents, test_util.load_vector(certs[1]) + test_util.load_vector(certs[2])) diff --git a/certbot/tests/configuration_test.py b/certbot/tests/configuration_test.py index 13d85bd9f..211a0eae6 100644 --- a/certbot/tests/configuration_test.py +++ b/certbot/tests/configuration_test.py @@ -60,6 +60,7 @@ class NamespaceConfigTest(unittest.TestCase): config_base = "foo" work_base = "bar" logs_base = "baz" + server = "mock.server" mock_namespace = mock.MagicMock(spec=['config_dir', 'work_dir', 'logs_dir', 'http01_port', @@ -68,6 +69,7 @@ class NamespaceConfigTest(unittest.TestCase): mock_namespace.config_dir = config_base mock_namespace.work_dir = work_base mock_namespace.logs_dir = logs_base + mock_namespace.server = server config = NamespaceConfig(mock_namespace) self.assertTrue(os.path.isabs(config.config_dir)) diff --git a/certbot/tests/crypto_util_test.py b/certbot/tests/crypto_util_test.py index 5a592bbb1..c0dc1de3a 100644 --- a/certbot/tests/crypto_util_test.py +++ b/certbot/tests/crypto_util_test.py @@ -111,7 +111,7 @@ class MakeCSRTest(unittest.TestCase): # OpenSSL.crypto.X509Extension doesn't give us the extension's raw OID, # and the shortname field is just "UNDEF" must_staple_exts = [e for e in csr.get_extensions() - if e.get_data() == "0\x03\x02\x01\x05"] + if e.get_data() == b"0\x03\x02\x01\x05"] self.assertEqual(len(must_staple_exts), 1, "Expected exactly one Must Staple extension") @@ -341,7 +341,7 @@ class CertLoaderTest(unittest.TestCase): def test_load_invalid_cert(self): from certbot.crypto_util import pyopenssl_load_certificate - bad_cert_data = CERT.replace("BEGIN CERTIFICATE", "ASDFASDFASDF!!!") + bad_cert_data = CERT.replace(b"BEGIN CERTIFICATE", b"ASDFASDFASDF!!!") self.assertRaises( errors.Error, pyopenssl_load_certificate, bad_cert_data) diff --git a/certbot/tests/display/util_test.py b/certbot/tests/display/util_test.py index a6ced90ab..0462305bd 100644 --- a/certbot/tests/display/util_test.py +++ b/certbot/tests/display/util_test.py @@ -151,7 +151,7 @@ class FileOutputDisplayTest(unittest.TestCase): self.assertTrue("message" in string) def test_notification_pause(self): - with mock.patch("__builtin__.raw_input", return_value="enter"): + with mock.patch("six.moves.input", return_value="enter"): self.displayer.notification("message") self.assertTrue("message" in self.mock_stdout.write.call_args[0][0]) @@ -164,31 +164,31 @@ class FileOutputDisplayTest(unittest.TestCase): self.assertEqual(ret, (display_util.OK, 0)) def test_input_cancel(self): - with mock.patch("__builtin__.raw_input", return_value="c"): + with mock.patch("six.moves.input", return_value="c"): code, _ = self.displayer.input("message") self.assertTrue(code, display_util.CANCEL) def test_input_normal(self): - with mock.patch("__builtin__.raw_input", return_value="domain.com"): + with mock.patch("six.moves.input", return_value="domain.com"): code, input_ = self.displayer.input("message") self.assertEqual(code, display_util.OK) self.assertEqual(input_, "domain.com") def test_yesno(self): - with mock.patch("__builtin__.raw_input", return_value="Yes"): + with mock.patch("six.moves.input", return_value="Yes"): self.assertTrue(self.displayer.yesno("message")) - with mock.patch("__builtin__.raw_input", return_value="y"): + with mock.patch("six.moves.input", return_value="y"): self.assertTrue(self.displayer.yesno("message")) - with mock.patch("__builtin__.raw_input", side_effect=["maybe", "y"]): + with mock.patch("six.moves.input", side_effect=["maybe", "y"]): self.assertTrue(self.displayer.yesno("message")) - with mock.patch("__builtin__.raw_input", return_value="No"): + with mock.patch("six.moves.input", return_value="No"): self.assertFalse(self.displayer.yesno("message")) - with mock.patch("__builtin__.raw_input", side_effect=["cancel", "n"]): + with mock.patch("six.moves.input", side_effect=["cancel", "n"]): self.assertFalse(self.displayer.yesno("message")) - with mock.patch("__builtin__.raw_input", return_value="a"): + with mock.patch("six.moves.input", return_value="a"): self.assertTrue(self.displayer.yesno("msg", yes_label="Agree")) @mock.patch("certbot.display.util.FileDisplay.input") @@ -275,11 +275,11 @@ class FileOutputDisplayTest(unittest.TestCase): def test_get_valid_int_ans_valid(self): # pylint: disable=protected-access - with mock.patch("__builtin__.raw_input", return_value="1"): + with mock.patch("six.moves.input", return_value="1"): self.assertEqual( self.displayer._get_valid_int_ans(1), (display_util.OK, 1)) ans = "2" - with mock.patch("__builtin__.raw_input", return_value=ans): + with mock.patch("six.moves.input", return_value=ans): self.assertEqual( self.displayer._get_valid_int_ans(3), (display_util.OK, int(ans))) @@ -292,7 +292,7 @@ class FileOutputDisplayTest(unittest.TestCase): ["c"], ] for ans in answers: - with mock.patch("__builtin__.raw_input", side_effect=ans): + with mock.patch("six.moves.input", side_effect=ans): self.assertEqual( self.displayer._get_valid_int_ans(3), (display_util.CANCEL, -1)) diff --git a/certbot/tests/reverter_test.py b/certbot/tests/reverter_test.py index 450cecacf..62a43f0fe 100644 --- a/certbot/tests/reverter_test.py +++ b/certbot/tests/reverter_test.py @@ -1,6 +1,5 @@ """Test certbot.reverter.""" import csv -import itertools import logging import os import shutil @@ -8,6 +7,7 @@ import tempfile import unittest import mock +import six from certbot import errors @@ -153,7 +153,7 @@ class ReverterCheckpointLocalTest(unittest.TestCase): act_coms = get_undo_commands(self.config.temp_checkpoint_dir) - for a_com, com in itertools.izip(act_coms, coms): + for a_com, com in six.moves.zip(act_coms, coms): self.assertEqual(a_com, com) def test_bad_register_undo_command(self): diff --git a/certbot/tests/storage_test.py b/certbot/tests/storage_test.py index 261500b98..7ac7771da 100644 --- a/certbot/tests/storage_test.py +++ b/certbot/tests/storage_test.py @@ -9,6 +9,7 @@ import unittest import configobj import mock import pytz +import six import certbot from certbot import cli @@ -92,8 +93,8 @@ class BaseRenewableCertTest(unittest.TestCase): os.symlink(os.path.join(os.path.pardir, os.path.pardir, "archive", "example.org", "{0}{1}.pem".format(kind, ver)), link) - with open(link, "w") as f: - f.write(kind if value is None else value) + with open(link, "wb") as f: + f.write(kind.encode('ascii') if value is None else value) def _write_out_ex_kinds(self): for kind in ALL_FOUR: @@ -235,7 +236,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.current_version("cert"), None) def test_latest_and_next_versions(self): - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.assertEqual(self.test_rc.latest_common_version(), 5) @@ -258,7 +259,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.next_free_version(), 18) def test_update_link_to(self): - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.assertEqual(ver, self.test_rc.current_version(kind)) @@ -285,12 +286,12 @@ class RenewableCertTests(BaseRenewableCertTest): os.path.basename(self.test_rc.version("cert", 8))) def test_update_all_links_to_success(self): - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.assertEqual(ver, self.test_rc.current_version(kind)) self.assertEqual(self.test_rc.latest_common_version(), 5) - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): self.test_rc.update_all_links_to(ver) for kind in ALL_FOUR: self.assertEqual(ver, self.test_rc.current_version(kind)) @@ -330,11 +331,11 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertEqual(self.test_rc.current_version(kind), 11) def test_has_pending_deployment(self): - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.assertEqual(ver, self.test_rc.current_version(kind)) - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): self.test_rc.update_all_links_to(ver) for kind in ALL_FOUR: self.assertEqual(ver, self.test_rc.current_version(kind)) @@ -373,10 +374,10 @@ class RenewableCertTests(BaseRenewableCertTest): self._write_out_ex_kinds() self.test_rc.update_all_links_to(12) - with open(self.test_rc.cert, "w") as f: + with open(self.test_rc.cert, "wb") as f: f.write(test_cert) self.test_rc.update_all_links_to(11) - with open(self.test_rc.cert, "w") as f: + with open(self.test_rc.cert, "wb") as f: f.write(test_cert) mock_datetime.timedelta = datetime.timedelta @@ -426,7 +427,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.assertFalse(self.test_rc.should_autodeploy()) self.test_rc.configuration["autodeploy"] = "1" # No pending deployment - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.assertFalse(self.test_rc.should_autodeploy()) @@ -461,7 +462,7 @@ class RenewableCertTests(BaseRenewableCertTest): # (to avoid instantiating parser) mock_rv.side_effect = lambda x: x - for ver in xrange(1, 6): + for ver in six.moves.range(1, 6): for kind in ALL_FOUR: self._write_out_kind(kind, ver) self.test_rc.update_all_links_to(3) @@ -492,7 +493,7 @@ class RenewableCertTests(BaseRenewableCertTest): self.test_rc.version("privkey", i)))) for kind in ALL_FOUR: - self.assertEqual(self.test_rc.available_versions(kind), range(1, 9)) + self.assertEqual(self.test_rc.available_versions(kind), list(six.moves.range(1, 9))) self.assertEqual(self.test_rc.current_version(kind), 3) # Test updating from latest version rather than old version self.test_rc.update_all_links_to(8) @@ -501,7 +502,7 @@ class RenewableCertTests(BaseRenewableCertTest): "attempt", self.cli_config)) for kind in ALL_FOUR: self.assertEqual(self.test_rc.available_versions(kind), - range(1, 10)) + list(six.moves.range(1, 10))) self.assertEqual(self.test_rc.current_version(kind), 8) with open(self.test_rc.version("fullchain", 9)) as f: self.assertEqual(f.read(), "last" + "attempt") diff --git a/certbot/tests/util_test.py b/certbot/tests/util_test.py index 36676443a..4aa6d3ff3 100644 --- a/certbot/tests/util_test.py +++ b/certbot/tests/util_test.py @@ -189,6 +189,12 @@ class UniqueFileTest(unittest.TestCase): self.assertTrue(basename3.endswith("foo.txt")) +try: + file_type = file +except NameError: + import io + file_type = io.TextIOWrapper + class UniqueLineageNameTest(unittest.TestCase): """Tests for certbot.util.unique_lineage_name.""" @@ -204,13 +210,13 @@ class UniqueLineageNameTest(unittest.TestCase): def test_basic(self): f, path = self._call("wow") - self.assertTrue(isinstance(f, file)) + self.assertTrue(isinstance(f, file_type)) self.assertEqual(os.path.join(self.root_path, "wow.conf"), path) def test_multiple(self): - for _ in xrange(10): + for _ in six.moves.range(10): f, name = self._call("wow") - self.assertTrue(isinstance(f, file)) + self.assertTrue(isinstance(f, file_type)) self.assertTrue(isinstance(name, str)) self.assertTrue("wow-0009.conf" in name) diff --git a/certbot/util.py b/certbot/util.py index 998808be0..e78ae664c 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -402,18 +402,27 @@ def enforce_domain_sanity(domain): :returns: The domain cast to `str`, with ASCII-only contents :rtype: str """ + if isinstance(domain, six.text_type): + wildcard_marker = u"*." + punycode_marker = u"xn--" + else: + wildcard_marker = b"*." + punycode_marker = b"xn--" + # Check if there's a wildcard domain - if domain.startswith("*."): + if domain.startswith(wildcard_marker): raise errors.ConfigurationError( "Wildcard domains are not supported: {0}".format(domain)) # Punycode - if "xn--" in domain: + if punycode_marker in domain: raise errors.ConfigurationError( "Punycode domains are not presently supported: {0}".format(domain)) # Unicode try: - domain = domain.encode('ascii').lower() + if isinstance(domain, six.binary_type): + domain = domain.decode('utf-8') + domain.encode('ascii') except UnicodeError: error_fmt = (u"Internationalized domain names " "are not presently supported: {0}") @@ -422,11 +431,10 @@ def enforce_domain_sanity(domain): else: raise errors.ConfigurationError(str(error_fmt).format(domain)) - if six.PY3: - domain = domain.decode('ascii') + domain = domain.lower() # Remove trailing dot - domain = domain[:-1] if domain.endswith('.') else domain + domain = domain[:-1] if domain.endswith(u'.') else domain # Explain separately that IP addresses aren't allowed (apart from not # being FQDNs) because hope springs eternal concerning this point diff --git a/tox.ini b/tox.ini index 27979d9df..9b0045d54 100644 --- a/tox.ini +++ b/tox.ini @@ -40,16 +40,22 @@ deps = commands = pip install -e acme[dns,dev] nosetests -v acme + pip install -e .[dev] + nosetests -v certbot [testenv:py34] commands = pip install -e acme[dns,dev] nosetests -v acme + pip install -e .[dev] + nosetests -v certbot [testenv:py35] commands = pip install -e acme[dns,dev] nosetests -v acme + pip install -e .[dev] + nosetests -v certbot [testenv:cover] basepython = python2.7 From 262eb778fe54890e4d36c2093a7a836ac3603b8b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 15:29:23 -0700 Subject: [PATCH 17/47] Update Apache plugin supported OSes --- docs/using.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index f43fd15c5..15658b2c5 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -20,8 +20,10 @@ but for most users who want to avoid running an ACME client as root, either `letsencrypt-nosudo `_ or `simp_le `_ are more appropriate choices. -The Apache plugin currently requires a Debian-based OS with augeas version -1.0; this includes Ubuntu 12.04+ and Debian 7+. +The Apache plugin currently requires OS with augeas version 1.0; currently `it +supports +`_ +modern OSes based on Debian, Fedora, SUSE, Gentoo and Darwin. Getting Certbot From e77a3ed7b968470c908960fb45079c9a2e9e8708 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 8 Aug 2016 17:22:53 -0700 Subject: [PATCH 18/47] Return individual key, not entire config dictionary! --- certbot-nginx/certbot_nginx/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/constants.py b/certbot-nginx/certbot_nginx/constants.py index 453266878..98f924b2b 100644 --- a/certbot-nginx/certbot_nginx/constants.py +++ b/certbot-nginx/certbot_nginx/constants.py @@ -22,4 +22,4 @@ def os_constant(key): # based on what OS we are running under. To see an # approach to how to handle different OSes, see the # apache version of this file. - return CLI_DEFAULTS + return CLI_DEFAULTS[key] From d41ceff86d22e8ec774771fdd4ab0ee093cb9b64 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 8 Aug 2016 17:24:54 -0700 Subject: [PATCH 19/47] Various WIP on nginx compatibility test --- .../configurators/nginx/common.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 9fbb538e8..aff9d9467 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -75,12 +75,13 @@ class Proxy(configurators_common.Proxy): for k in constants.CLI_DEFAULTS.keys(): setattr(self.le_config, "nginx_" + k, constants.os_constant(k)) - # An alias - self.le_config.nginx_handle_modules = self.le_config.nginx_handle_mods + # This does not appear to exist in nginx (yet?) + # self.le_config.nginx_handle_modules = self.le_config.nginx_handle_mods + conf=configuration.NamespaceConfig(self.le_config) + zope.component.provideUtility(conf) self._nginx_configurator = configurator.NginxConfigurator( - config=configuration.NamespaceConfig(self.le_config), - name="nginx") + config=conf, name="nginx") self._nginx_configurator.prepare() def cleanup_from_tests(self): @@ -118,7 +119,7 @@ def _get_server_root(config): if os.path.isdir(os.path.join(config, name))] if len(subdirs) != 1: - errors.Error("Malformed configuration directory {0}".format(config)) + raise errors.Error("Malformed configuration directory {0}".format(config)) return os.path.join(config, subdirs[0].rstrip()) From 0504882e083252f2ed820bb96586235fdebd09d0 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Mon, 8 Aug 2016 17:50:20 -0700 Subject: [PATCH 20/47] Always newline config edits Even if they're transient --- certbot-nginx/certbot_nginx/tls_sni_01.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-nginx/certbot_nginx/tls_sni_01.py b/certbot-nginx/certbot_nginx/tls_sni_01.py index ebc92f5e3..c7ec80931 100644 --- a/certbot-nginx/certbot_nginx/tls_sni_01.py +++ b/certbot-nginx/certbot_nginx/tls_sni_01.py @@ -91,10 +91,10 @@ class NginxTlsSni01(common.TLSSNI01): # Add the 'include' statement for the challenges if it doesn't exist # already in the main config included = False - include_directive = ['include', ' ', self.challenge_conf] + include_directive = ['\n', 'include', ' ', self.challenge_conf] root = self.configurator.parser.loc["root"] - bucket_directive = ['server_names_hash_bucket_size', ' ', '128'] + bucket_directive = ['\n', 'server_names_hash_bucket_size', ' ', '128'] main = self.configurator.parser.parsed[root] for key, body in main: From 7d27c1f50020a3594b5bc05c0db023139aec8578 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 8 Aug 2016 17:51:55 -0700 Subject: [PATCH 21/47] More correct parsing of lines containing trailing space --- .../certbot_compatibility_test/configurators/nginx/common.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index aff9d9467..8e2899933 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -131,7 +131,7 @@ def _get_names(config): for this_file in files: for line in open(os.path.join(root, this_file)): if line.strip().startswith("server_name"): - names = line.partition("server_name")[2].rstrip(";") + names = line.partition("server_name")[2].rpartition(";")[0] [all_names.add(n) for n in names.split()] non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n)) return all_names, non_ip_names From 6e86c71259bfd0128f9d002aaec801768637df1f Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Mon, 8 Aug 2016 18:03:07 -0700 Subject: [PATCH 22/47] Provide a copy of the self-signed cert as the fullchain as well --- .../certbot_compatibility_test/test_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py index 3d03f7771..b5e023f36 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/test_driver.py +++ b/certbot-compatibility-test/certbot_compatibility_test/test_driver.py @@ -144,7 +144,7 @@ def test_deploy_cert(plugin, temp_dir, domains): for domain in domains: try: - plugin.deploy_cert(domain, cert_path, util.KEY_PATH, cert_path) + plugin.deploy_cert(domain, cert_path, util.KEY_PATH, cert_path, cert_path) plugin.save() # Needed by the Apache plugin except le_errors.Error as error: logger.error("Plugin failed to deploy ceritificate for %s:", domain) From c29e8c3332640611909c5f7c6913525c86d9d8d5 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Tue, 9 Aug 2016 23:43:11 +0300 Subject: [PATCH 23/47] Refactored get_file_path --- certbot-apache/certbot_apache/configurator.py | 30 +++++++------------ 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 238255133..4a68461ce 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1801,25 +1801,17 @@ def get_file_path(vhost_path): :rtype: str """ - # Strip off /files - avail_fp = vhost_path[6:] - # This can be optimized... - while True: - # Cast all to lowercase to be case insensitive - find_if = avail_fp.lower().find("/ifmodule") - if find_if != -1: - avail_fp = avail_fp[:find_if] - continue - find_vh = avail_fp.lower().find("/virtualhost") - if find_vh != -1: - avail_fp = avail_fp[:find_vh] - continue - find_macro = avail_fp.lower().find("/macro") - if find_macro != -1: - avail_fp = avail_fp[:find_macro] - continue - break - return avail_fp + # Strip off /files/ + avail_fp = vhost_path[7:].split("/") + last_good = "" + # Loop through the path parts and validate after every addition + for p in avail_fp: + cur_path = last_good+"/"+p + if os.path.exists(cur_path): + last_good = cur_path + else: + break + return last_good def install_ssl_options_conf(options_ssl): From f4948855f02b1e1dde4434e7c8e15beb8c06a150 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 00:25:35 +0300 Subject: [PATCH 24/47] Added None check and according test --- certbot-apache/certbot_apache/configurator.py | 8 +++++++- certbot-apache/certbot_apache/tests/configurator_test.py | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index 4a68461ce..abe3edede 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -538,6 +538,9 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator): is_ssl = True filename = get_file_path(self.aug.get("/augeas/files%s/path" % get_file_path(path))) + if filename is None: + return None + if self.conf("handle-sites"): is_enabled = self.is_site_enabled(filename) else: @@ -1802,7 +1805,10 @@ def get_file_path(vhost_path): """ # Strip off /files/ - avail_fp = vhost_path[7:].split("/") + try: + avail_fp = vhost_path[7:].split("/") + except TypeError: + return None last_good = "" # Loop through the path parts and validate after every addition for p in avail_fp: diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index ac692ae54..59692302d 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -125,6 +125,10 @@ class MultipleVhostsTest(util.ApacheTest): self.assertTrue("google.com" in names) self.assertTrue("certbot.demo" in names) + def test_get_bad_path(self): + from certbot_apache.configurator import get_file_path + self.assertEqual(get_file_path(None), None) + def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( "fp1", "ap1", set([obj.Addr(("*", "443"))]), From 6c3ae10f9b1e94d0f38d73c15f593b348556e5d4 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 01:39:53 +0300 Subject: [PATCH 25/47] Added test case --- certbot-apache/certbot_apache/tests/configurator_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 59692302d..2a8960564 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,6 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) + self.assertEqual(self.config._create_vhost("nonexistent"), None) def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( From 51191c2ea54de8fad135c93b13bb6ab0bc22410c Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 01:50:40 +0300 Subject: [PATCH 26/47] Added linter exception --- certbot-apache/certbot_apache/tests/configurator_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index 2a8960564..f8bf7676c 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,7 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) - self.assertEqual(self.config._create_vhost("nonexistent"), None) + self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access def test_bad_servername_alias(self): ssl_vh1 = obj.VirtualHost( From 14f371025049bbc5621da943e0ee04cf4e4add8d Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 10:39:10 +0300 Subject: [PATCH 27/47] Added check for /files/ --- certbot-apache/certbot_apache/configurator.py | 9 +++++++-- certbot-apache/certbot_apache/tests/configurator_test.py | 1 + 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py index abe3edede..02602ace6 100644 --- a/certbot-apache/certbot_apache/configurator.py +++ b/certbot-apache/certbot_apache/configurator.py @@ -1806,9 +1806,14 @@ def get_file_path(vhost_path): """ # Strip off /files/ try: - avail_fp = vhost_path[7:].split("/") - except TypeError: + if vhost_path.startswith("/files/"): + avail_fp = vhost_path[7:].split("/") + else: + return None + except AttributeError: + # If we recieved a None path return None + last_good = "" # Loop through the path parts and validate after every addition for p in avail_fp: diff --git a/certbot-apache/certbot_apache/tests/configurator_test.py b/certbot-apache/certbot_apache/tests/configurator_test.py index f8bf7676c..dc953174e 100644 --- a/certbot-apache/certbot_apache/tests/configurator_test.py +++ b/certbot-apache/certbot_apache/tests/configurator_test.py @@ -128,6 +128,7 @@ class MultipleVhostsTest(util.ApacheTest): def test_get_bad_path(self): from certbot_apache.configurator import get_file_path self.assertEqual(get_file_path(None), None) + self.assertEqual(get_file_path("nonexistent"), None) self.assertEqual(self.config._create_vhost("nonexistent"), None) # pylint: disable=protected-access def test_bad_servername_alias(self): From 3591667d026c32fd92d2aa9440f98b007ee3f578 Mon Sep 17 00:00:00 2001 From: Joona Hoikkala Date: Wed, 10 Aug 2016 10:43:54 +0300 Subject: [PATCH 28/47] Fix tox tests --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 27979d9df..b53e598f8 100644 --- a/tox.ini +++ b/tox.ini @@ -35,6 +35,7 @@ deps = py{26,27}-oldest: psutil==2.1.0 py{26,27}-oldest: PyOpenSSL==0.13 py{26,27}-oldest: python2-pythondialog==3.2.2rc1 + py{26,27}-oldest: dnspython>=1.12 [testenv:py33] commands = From cb9921f4b1c919ba337aca90309fe8a958033f4b Mon Sep 17 00:00:00 2001 From: Jacob Hoffman-Andrews Date: Wed, 10 Aug 2016 11:14:39 -0700 Subject: [PATCH 29/47] Add more ignored files to gitignore. --- .gitignore | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index b653cb06c..48ec7910b 100644 --- a/.gitignore +++ b/.gitignore @@ -17,9 +17,11 @@ letsencrypt-auto-source/letsencrypt-auto.sig.lzma.base64 /.vagrant +tags + # editor temporary files *~ -*.swp +*.sw? \#*# .idea From dd1de2bc9e4d6de7ffb7192dede23fcc68618510 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 11:49:55 -0700 Subject: [PATCH 30/47] Fix travis --- tests/boulder-fetch.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/boulder-fetch.sh b/tests/boulder-fetch.sh index 469c5cd80..0c0a8009b 100755 --- a/tests/boulder-fetch.sh +++ b/tests/boulder-fetch.sh @@ -5,6 +5,6 @@ set -xe # Check out special branch until latest docker changes land in Boulder master. git clone -b docker-integration https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH -sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml +FAKE_DNS=$(ifconfig docker0 | grep "inet addr:" | cut -d: -f2 | awk '{ print $1}') +sed -i "s/FAKE_DNS: .*/FAKE_DNS: $FAKE_DNS/" docker-compose.yml docker-compose up -d - From fd1629e347a3fce7cf2bc63b7e05063e3d294cd8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 11:51:12 -0700 Subject: [PATCH 31/47] Make letstest docker ip more robust --- tests/letstest/scripts/boulder_install.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/letstest/scripts/boulder_install.sh b/tests/letstest/scripts/boulder_install.sh index 7e298783f..f997268bd 100755 --- a/tests/letstest/scripts/boulder_install.sh +++ b/tests/letstest/scripts/boulder_install.sh @@ -5,5 +5,6 @@ # Check out special branch until latest docker changes land in Boulder master. git clone -b docker-integration https://github.com/letsencrypt/boulder $BOULDERPATH cd $BOULDERPATH -sed -i 's/FAKE_DNS: .*/FAKE_DNS: 172.17.42.1/' docker-compose.yml +FAKE_DNS=$(ifconfig docker0 | grep "inet addr:" | cut -d: -f2 | awk '{ print $1}') +sed -i "s/FAKE_DNS: .*/FAKE_DNS: $FAKE_DNS/" docker-compose.yml docker-compose up -d From 595e51551866d39593e0e61f2a0dcbe7fa7ad844 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 10 Aug 2016 14:57:44 -0700 Subject: [PATCH 32/47] Restart web servers before beginning tests --- .../certbot_compatibility_test/configurators/apache/common.py | 2 +- .../certbot_compatibility_test/configurators/nginx/common.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py index 0c53058de..4e612bbd5 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py @@ -58,7 +58,7 @@ class Proxy(configurators_common.Proxy): self._prepare_configurator() try: - subprocess.check_call("apachectl -k start".split()) + subprocess.check_call("apachectl -k restart".split()) except errors.Error: raise errors.Error( "Apache failed to load {0} before tests started".format( diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 8e2899933..0d72605b7 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -62,7 +62,7 @@ class Proxy(configurators_common.Proxy): self._prepare_configurator() try: - subprocess.check_call("nginx".split()) + subprocess.check_call("service nginx reload".split()) except errors.Error: raise errors.Error( "Nginx failed to load {0} before tests started".format( From 4c596311b068974fd090495be8355323d711c440 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 15:39:35 -0700 Subject: [PATCH 33/47] Add nginx compatibility test data tarball --- .../testdata/nginx.tar.gz | Bin 0 -> 6625 bytes 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz diff --git a/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz b/certbot-compatibility-test/certbot_compatibility_test/testdata/nginx.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..4ca5cc9775a35f64534f90b1db4fb7ef272672a8 GIT binary patch literal 6625 zcmV<786M^ziwFQ)A*ojY1MFRGbK5wQp0CueK*dvgd$%K5?-IvXS4C1};@c$l+RDs% zzNlytk{I)C2+ESPwg3IP0m>vG((-O{p4}Ux%CXY@paJxQ7lMG=tBksNJ6;9xuI(l9 zGFY8{_pMVG(quH^zago=`R87LGV1qx{r+(HUAH&r4+r0o(KoM2P1>;B%?SA}OOpI4 z+%f&XXj&ce&+^T3`7L|F-_QSWFzLVY{}QQ@|GOk;Z}R20C<6%|Bd_)RkB0pb^530I zx*c8cx&Qjl-D7Mi)wbj#QBmuBpVV_>hIgY zYm>vjZsuc&KadThyx1(bJ>A5Cm-w{Ld;h+}gCXvVnB~;dO}m8H{b}E@$Qd~ybdx1Qp^vztYxKF|7OF(^LS+zQ^^Gnl2&n!L-?H?qP}!(5 zCwo;C_-5pA@L@nSwdK_iVMB)RJYL( z_tjg0<-VHQ3bw{up9Cd)BXm7%*`tSr&K0UYSIMm23Kj2v zQ@QixOz61pO}YR1LN(&5kz5ffbi6-K0P|di?l2nmU*<3Y~RIyMwTqP4z%o?*U*Ey0d6{;R@ zw5)m%q|N$VC-a#Jl^{*#Q*bupbH=nU^C?s#u9BTE`p%3G9#ifGUxf2#f-~9Ty-*3x zWMA=q&&LUe=o}(@`a!4!b@G(-MW{@kOD3EZ^|824W zPVOsfTxH4;Z*v|Ebrj!x5?Vg|4(0dZC27Fs?Cb#(%q2_+wf z`be#kEwr3fL(S?4C8t#%C`vJ5*;AfVwfpCHLd%DvK2@}0g0(rTx~?eyTPXRE)D6|H zQ1U^kn~L(iQ1W4^TZ;07Q1XGP&lKfHq2xnTS82Ntq&l2b-BGlcLdiKbROO8z)#0QX zD&to|DM(cr7o0krQ=_jkE|i>8LnTJ^GKW*Dt9|7cL5w3fRqg&Hl!8;$gYs~7IJJh# z!HrM~YSp##7op_T8fvGwE^5ZPHB^Q~KXo{_hT3JW-&sXwPOqVsyVr7l4Yf~|gZFQ( z-}<5a1ykexkBd4e`sy-{?Em`X@mSgajR%v_>;CU066}XEvfXamzMH#et!N>eus?&%^ABARS+G9DrBA*F1wcG()XZ@UExzEINd4n@_V<*?^mI_DmGd^sVN^zEj~=?det+N}79vY|;w2#?^Y_>HJ@Id6 z|GSUdkXe?bDelX`#Qoq0lF{XlW?7Qep}0bT_9eaj5~OM|E0l4H@zcT&n0pJo;!VT? zI({P<;~KDD^o2qI-q{8CDjpusfEfag zEj64+lG9y&nuf4{6CXXsBr-oU$R`QhdzX1x07HRi*hQhFulfI4G?!cl^`;;IsN!ErbO*Wo;UxlL5(xYwIL(gAfMn33f$F33HO}ySG6YbEWc}~;CPQ`oA9p9O>;FrnR>?CV zSN2~%+1K;M2YdeRV#Yh&_Zj_lLs`CvXueKh*{{QYx;iICdc<$CKn4T!>Sl30zdHZ; zb`J(<_OO8E4rY8AZC!kv+aKqP`OT%hN8@{2m^k?C3v*xf&OqqLFABer>&dY+sRj3c|kn$-Y`#UR}&Dtc&;DV6mj`2X(#YuB~_W2hqUq z8O(wam2vOp^UG_w=cCSC`HC_UM4Nh@}4;#bm7SAQ|$o+%sQ9p?*=k^_~ z=5!tpaI1$2L#pV(lB@GLsn=GTT_aLgF86yo(@Q)neYsMH&j0Nl#Uya7( z)&=5?2`VNklmnAe4$u#$5@{1js z*+W^;{xsaMb`-=LR#d?3qt$WqO%2T)Ai?9N8!ehSMCy7^-cr%b5msU(KvCfB?{8=( zGHMpq(;x)RcCSKC&53eVM>D5bAE~?t80V(w=K5H@BBGfFW*9c!+|f+r)GRpVqw#s9 zLQV%wPZ~@IfUvq69!0As(VEpmxza05kRu;@Up+O{tO3dg>It!C4N6$;_MyaUAq6-xj51>`t>aF&!0m^DjBxvriqYZeY7Tpr|OWz8~CZmK8Bnq{Hf zQV)|gYliZfdXB7FHp-Q`35K(U<3o4U!{Zi6wOQ9tD7XSSQeY?)T!S1bFcb>rAV&%e zg#wrXXGnpeP~d9HJ<*A?fc1Qf7?m3(dnioUv$!JbhSCFN`U|y z9c@Z0F3f*6df9a3i%l)f>EcQ*xfqs9hp+FFi{Ww0!KHjExfmWc9Gw5Bl8ZMW9imin z@e-sXlu9nX17A906iY77vAhY2B^P^$#tD8Z`4}H>9392FEarHe-=~tFE8W~NQ7ifT z0i+g6C4X1CxHChoVxCCbn7zMP6uI5wn4ro?c#N8dzoY&Z9_5b{l$|* zKg`arYzz3G8}m<`-F~-QbL7%E|CjewzIu*1GXGCTWA*z#<6(dBI{&{!YKf~Xf8|#r z`8|_$9);wo!~yURJlVjfvoa^%e|a(ipaS0=DTnO#)`Q4h(bE--il-9jtMJb$T|EWo z`p@Y~7q#TzPO0=M9O!01%4m< z9kz)(pZhS*p?+O#PlVp)J zURBA>gXNQ*0rdDIb%@6%+(v0_3qW4Q@7e*(h|OhyR)&9mvJIc*7VO#)Yd6#clTsOW zziQ|;B7q)%zEtyU>20HVOsFqNsCLjLJXy83NsWWI$=;*taA~`lw+`;9(qx!fx9~&V ztR@?4axQ8P@saDbZ-aPEcf+oXU;nzRfwOG_`>UD+4_k-u)Nkrfps9f;EU%v;;IWCq zj~mpC{Jg0Q&>X_CzvBOCCxWSJla=I0gUz%a4}FU6JgW4QU05@36>wxbx&4fnSNDk~ zc_Tbcwlw1}9?KS|4e(;V@Z`#%q#nOYk`+ANKyS&{ox3O$9sX`Vu^fSa5$55;A$-@K zeBm=vhPRrly<$>}_TZV?t>MYr4|y409RY2>oSLBj`$651DYATy!?ewBIX@;8)!`%e zjxrh^bf9cgGgNl%lxDSaMH-T{W=(<&L73D{)oJFz2)M1wlDS!}PqVtwzQae=H9Tsj zu6GBM6ErA>U$E9>^%Y{YjsTeGs9OtPC*IxG1^+KYm#s_qw$}ap88@~)kG z^)oNZ^SinnyHQtfdv4pKnN*4oO-^Y-q$mnw6t^dtJuE#7*dq5{Bu^x z6Q%uz2V#+OCZmT>fx$aN8Gg)}ppW)D9+BtQP2k<(cV?RMB==P~JTgWNNS;t%#qVG` zzJCG_S{WVMe=&z>;ucrvFrwG5FQ-s&_ zubLXy|ILOq+A8Lctb=vv=2ft^v5jk59s2dV zfxmTXod5Y3&tGKz@Ak*)_rE7y`1Lyfzd#Z+&T4U5;K#5SxgB3nEI;|lJlp@nQc9romDy`kE$jgYdcCR8X zqtQC!intPq)~*s-MxnJw)k4b%v`!UXMxV8-U8{^d>r|~&qR!gYPFzNuwe^a)j5cdm zyLK6A);25RGRmxNRm5e4S$kFym(gYIs&gxmW$o%vD5J_cRdFSvtW)Kpj3(<;xhNya zI@RG(Mv--@PAntH+EwN%(PQnZl`?XyQ*HlY)L5tLwK8I?J*gHV#3qk!Sus{digl{q zEThCax&*I8h;^#=E74({Dz|l!VQqOLD5Jt!T}e|$gte3bClmwj%&MXi3Dz1)nlcKk zB@gmy1Xwvs8U587NQz42SJ_r2>Z>Ksc4fp@OP3U7v{y@>1qPofoDWa>SsYG*?*{VcxmFMtE6j#~7Wdv7C=C=~P)sk&h zBDcz3sfpSu`?HMLYE9(Agu>)wP-;*mQmgFgN|aVz$x@BbDwk7@&MNnT-)Uslmq|zB zzm$PjzM>3q$^SX=fBM}~kMIA7gW+rZpO;9_=Km`K?;P9zb;kpl|9-EF@tlRae;z#({Z&>>6xp^bRs@ATBo_I@R{4+FojEd1}wgE=MapN588YTBs{OuZkWf%X- z&2~*Q`osRu%w)w0t4lm+5B_Bw4!qsu1mhEnyl$3&x4^d<;h}eF?9mRn$jE2tiDVz& zHo(9jl>l5FUb{FTIo+`Al)=&nBQq(Yx3=*U-SOzG?35P~B^f0?&0!Q%|9{)N zmgcl!Abf^jp$^O>nIsr6kJL;LeN2u$w8y5+IA9VF#P(oO!lC``-ESqofE3zh=w-jm zWQ?ViWNEeb+N)10OQX^n_-6dU%xTMJTpcW`Fefg}XiKl!Dq}7-X`?*pi|2jycBE(Y z#nrNF%wB6ndp%dB>3qBBJO>y4d^Y6#80)|*D{|J8OdyP|Kr8gZN0ei$ZkTL6){B%B z<8FDMwk81>j94>@c$L-i z8PVN%%%Ooo)NrcqSs69`zFYI$pxRA9u??bz*oQbe(afPlIUb=nzDgGHWNGPTxR+kp zIx@!JGzya>(5aI~{w#>YxupcI*I?!hRr6}Ka!eR8B;QUZBs~e9K);JiM|&_lh%&%b z!In%*1hpdO0Nlcq*Glk$vHZ{;S&cQFk=0ZB1f={`fGKuq&b6%WQbU;-8|=A!fe!SO z8h9O&0V2z#@#PB*WI1I&{wd$g+6%NhVgWj1q8{SK+MHHXezQP*3wctYWbUtdQ!CYL z_h>k4da=GWMy$D9g#6D@Zzz$}@CWx(;)Ac5`Ua5~CPIic<=Rg*XhiLtGK!6|K`q@K zw%YX9@m|yI&}&_--rQkIO?8RFCuP%A4sO=Tm{Ir>?MZi+|rJU zAJ(>xkhL;JDsuvf)!AE3-kOp|q%x5jzg6)UC!ZGlcRc(2Z^`0cqK*Uozb+}+pgnZ) z{iomSi~n~FJn#QycbI>##4e#lo0urQyx4WntSzMZzt*>OEPbz0|VO z!4xm7;9|lz4t2fcj_hiXaZIS_ckA5qp88{LE6IhidR3>@wsS*y>uLLn#&aCK@W-VW zgl2h(4zd52<90iBDn&JmtK2ScbL~bEV%#j{$upIGGTzrHbMVW{@5Go11OkCTAP@)y f0)apv5C{YUfj}S-2m}IwU>|+~rQ-ge0H6Q>$7DuA literal 0 HcmV?d00001 From 2d099680d01c117fa3d2177e95bb297ff908c1b4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 15:39:59 -0700 Subject: [PATCH 34/47] Rename apache compatibility test tarball --- .../testdata/{configs.tar.gz => apache.tar.gz} | Bin 1 file changed, 0 insertions(+), 0 deletions(-) rename certbot-compatibility-test/certbot_compatibility_test/testdata/{configs.tar.gz => apache.tar.gz} (100%) diff --git a/certbot-compatibility-test/certbot_compatibility_test/testdata/configs.tar.gz b/certbot-compatibility-test/certbot_compatibility_test/testdata/apache.tar.gz similarity index 100% rename from certbot-compatibility-test/certbot_compatibility_test/testdata/configs.tar.gz rename to certbot-compatibility-test/certbot_compatibility_test/testdata/apache.tar.gz From a76c36bf1273320da724f727a26ebc73a663c8ff Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 15:52:33 -0700 Subject: [PATCH 35/47] Remove old Dockerfiles --- .../configurators/apache/Dockerfile | 20 ------------------- .../configurators/nginx/Dockerfile | 20 ------------------- 2 files changed, 40 deletions(-) delete mode 100644 certbot-compatibility-test/certbot_compatibility_test/configurators/apache/Dockerfile delete mode 100644 certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/Dockerfile b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/Dockerfile deleted file mode 100644 index ea9bb857f..000000000 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/Dockerfile +++ /dev/null @@ -1,20 +0,0 @@ -FROM httpd -MAINTAINER Brad Warren - -RUN mkdir /var/run/apache2 - -ENV APACHE_RUN_USER=daemon \ - APACHE_RUN_GROUP=daemon \ - APACHE_PID_FILE=/usr/local/apache2/logs/httpd.pid \ - APACHE_RUN_DIR=/var/run/apache2 \ - APACHE_LOCK_DIR=/var/lock \ - APACHE_LOG_DIR=/usr/local/apache2/logs - -COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2enmod.sh /usr/local/bin/ -COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2dismod.sh /usr/local/bin/ -COPY certbot-compatibility-test/certbot_compatibility_test/testdata/rsa1024_key2.pem /usr/local/apache2/conf/ -COPY certbot-compatibility-test/certbot_compatibility_test/testdata/empty_cert.pem /usr/local/apache2/conf/ - -# Note: this only exposes the port to other docker containers. You -# still have to bind to 443@host at runtime. -EXPOSE 443 diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile deleted file mode 100644 index ea9bb857f..000000000 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/Dockerfile +++ /dev/null @@ -1,20 +0,0 @@ -FROM httpd -MAINTAINER Brad Warren - -RUN mkdir /var/run/apache2 - -ENV APACHE_RUN_USER=daemon \ - APACHE_RUN_GROUP=daemon \ - APACHE_PID_FILE=/usr/local/apache2/logs/httpd.pid \ - APACHE_RUN_DIR=/var/run/apache2 \ - APACHE_LOCK_DIR=/var/lock \ - APACHE_LOG_DIR=/usr/local/apache2/logs - -COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2enmod.sh /usr/local/bin/ -COPY certbot-compatibility-test/certbot_compatibility_test/configurators/apache/a2dismod.sh /usr/local/bin/ -COPY certbot-compatibility-test/certbot_compatibility_test/testdata/rsa1024_key2.pem /usr/local/apache2/conf/ -COPY certbot-compatibility-test/certbot_compatibility_test/testdata/empty_cert.pem /usr/local/apache2/conf/ - -# Note: this only exposes the port to other docker containers. You -# still have to bind to 443@host at runtime. -EXPOSE 443 From 0edb1f6792575d303d592acafd9441cf3ed17367 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 16:08:30 -0700 Subject: [PATCH 36/47] Add certbot-compatibility-test Dockerfiles --- certbot-compatibility-test/Dockerfile | 51 ++++++++++++++++++++ certbot-compatibility-test/Dockerfile-apache | 6 +++ certbot-compatibility-test/Dockerfile-nginx | 6 +++ 3 files changed, 63 insertions(+) create mode 100644 certbot-compatibility-test/Dockerfile create mode 100644 certbot-compatibility-test/Dockerfile-apache create mode 100644 certbot-compatibility-test/Dockerfile-nginx diff --git a/certbot-compatibility-test/Dockerfile b/certbot-compatibility-test/Dockerfile new file mode 100644 index 000000000..d5ef9841c --- /dev/null +++ b/certbot-compatibility-test/Dockerfile @@ -0,0 +1,51 @@ +FROM debian:jessie +MAINTAINER Brad Warren + +WORKDIR /opt/certbot + +# no need to mkdir anything: +# https://docs.docker.com/reference/builder/#copy +# If doesn't exist, it is created along with all missing +# directories in its path. + +# TODO: Install non-default Python versions for tox. +# TODO: Install Apache/Nginx for plugin development. +COPY certbot-auto /opt/certbot/src/certbot-auto +RUN /opt/certbot/src/certbot-auto -n --os-packages-only + +# the above is not likely to change, so by putting it further up the +# Dockerfile we make sure we cache as much as possible + +COPY setup.py README.rst CHANGES.rst MANIFEST.in linter_plugin.py tox.cover.sh tox.ini pep8.travis.sh .pep8 .pylintrc /opt/certbot/src/ + +# all above files are necessary for setup.py, however, package source +# code directory has to be copied separately to a subdirectory... +# https://docs.docker.com/reference/builder/#copy: "If is a +# directory, the entire contents of the directory are copied, +# including filesystem metadata. Note: The directory itself is not +# copied, just its contents." Order again matters, three files are far +# more likely to be cached than the whole project directory + +COPY certbot /opt/certbot/src/certbot/ +COPY acme /opt/certbot/src/acme/ +COPY certbot-apache /opt/certbot/src/certbot-apache/ +COPY certbot-nginx /opt/certbot/src/certbot-nginx/ +COPY certbot-compatibility-test /opt/certbot/src/certbot-compatibility-test/ + +RUN virtualenv --no-site-packages -p python2 /opt/certbot/venv && \ + /opt/certbot/venv/bin/pip install -U setuptools && \ + /opt/certbot/venv/bin/pip install -U pip && \ + /opt/certbot/venv/bin/pip install \ + -e /opt/certbot/src/acme \ + -e /opt/certbot/src \ + -e /opt/certbot/src/certbot-apache \ + -e /opt/certbot/src/certbot-nginx \ + -e /opt/certbot/src/certbot-compatibility-test \ + -e /opt/certbot/src[dev,docs] + +# install in editable mode (-e) to save space: it's not possible to +# "rm -rf /opt/certbot/src" (it's stays in the underlaying image); +# this might also help in debugging: you can "docker run --entrypoint +# bash" and investigate, apply patches, etc. + +ENV PATH /opt/certbot/venv/bin:$PATH diff --git a/certbot-compatibility-test/Dockerfile-apache b/certbot-compatibility-test/Dockerfile-apache new file mode 100644 index 000000000..5c0495966 --- /dev/null +++ b/certbot-compatibility-test/Dockerfile-apache @@ -0,0 +1,6 @@ +FROM certbot-compatibility-test +MAINTAINER Brad Warren + +RUN apt-get install apache2 -y + +ENTRYPOINT [ "certbot-compatibility-test", "-p", "apache" ] diff --git a/certbot-compatibility-test/Dockerfile-nginx b/certbot-compatibility-test/Dockerfile-nginx new file mode 100644 index 000000000..4ade03065 --- /dev/null +++ b/certbot-compatibility-test/Dockerfile-nginx @@ -0,0 +1,6 @@ +FROM certbot-compatibility-test +MAINTAINER Brad Warren + +RUN apt-get install nginx -y + +ENTRYPOINT [ "certbot-compatibility-test", "-p", "nginx" ] From 07b85f9f90a12b3b52c19fb0b379955f728095ca Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 16:32:38 -0700 Subject: [PATCH 37/47] Make testdata the CWD of compatibility test dockerfiles --- certbot-compatibility-test/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot-compatibility-test/Dockerfile b/certbot-compatibility-test/Dockerfile index d5ef9841c..e445a3555 100644 --- a/certbot-compatibility-test/Dockerfile +++ b/certbot-compatibility-test/Dockerfile @@ -1,8 +1,6 @@ FROM debian:jessie MAINTAINER Brad Warren -WORKDIR /opt/certbot - # no need to mkdir anything: # https://docs.docker.com/reference/builder/#copy # If doesn't exist, it is created along with all missing @@ -48,4 +46,6 @@ RUN virtualenv --no-site-packages -p python2 /opt/certbot/venv && \ # this might also help in debugging: you can "docker run --entrypoint # bash" and investigate, apply patches, etc. +WORKDIR /opt/certbot/src/certbot-compatibility-test/certbot_compatibility_test/testdata + ENV PATH /opt/certbot/venv/bin:$PATH From fc86f869a71db80c613e60dbd206e2764908c354 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 16:33:56 -0700 Subject: [PATCH 38/47] add compatibility tests to travis --- tox.ini | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 27979d9df..30c895199 100644 --- a/tox.ini +++ b/tox.ini @@ -79,7 +79,6 @@ commands = pip install -e acme -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot {toxinidir}/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test --debian-modules - [testenv:le_auto] # At the moment, this tests under Python 2.7 only, as only that version is # readily available on the Trusty Docker image. @@ -89,3 +88,21 @@ commands = whitelist_externals = docker passenv = DOCKER_* + +[testenv:apache_compat] +commands = + docker build -t certbot-compatibility-test -f certbot-compatibility-test/Dockerfile . + docker build -t apache-compat -f certbot-compatibility-test/Dockerfile-apache . + docker run --rm -it apache-compat -c apache.tar.gz -vvvv +whitelist_externals = + docker +passenv = DOCKER_* + +[testenv:nginx_compat] +commands = + docker build -t certbot-compatibility-test -f certbot-compatibility-test/Dockerfile . + docker build -t nginx-compat -f certbot-compatibility-test/Dockerfile-nginx . + docker run --rm -it nginx-compat -c nginx.tar.gz -vvvv +whitelist_externals = + docker +passenv = DOCKER_* From f864cd0cfe29a1ec8caa82204fd6e71ca0e366dc Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 16:43:15 -0700 Subject: [PATCH 39/47] Add nginxroundtrip to tox --- tox.ini | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tox.ini b/tox.ini index 30c895199..64b5170db 100644 --- a/tox.ini +++ b/tox.ini @@ -79,6 +79,11 @@ commands = pip install -e acme -e .[dev] -e certbot-apache -e certbot-nginx -e certbot-compatibility-test -e letshelp-certbot {toxinidir}/certbot-apache/certbot_apache/tests/apache-conf-files/apache-conf-test --debian-modules +[testenv:nginxroundtrip] +commands = + pip install -e acme[dev] -e .[dev] -e certbot-nginx + python certbot-compatibility-test/nginx/roundtrip.py certbot-compatibility-test/nginx/nginx-roundtrip-testdata + [testenv:le_auto] # At the moment, this tests under Python 2.7 only, as only that version is # readily available on the Trusty Docker image. From 5dda27d757b5200651eaac83bc3e62b99b112880 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 16:46:53 -0700 Subject: [PATCH 40/47] Add nginxroundtrip and compatibility-tests to travis --- .travis.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.travis.yml b/.travis.yml index 6f964dbec..5ccf39811 100644 --- a/.travis.yml +++ b/.travis.yml @@ -34,6 +34,8 @@ matrix: - python: "2.7" env: TOXENV=apacheconftest sudo: required + - python: "2.7" + env: TOXENV=nginxroundtrip - python: "2.7" env: TOXENV=py27 BOULDER_INTEGRATION=1 sudo: true @@ -53,6 +55,16 @@ matrix: services: docker before_install: addons: + - sudo: required + env: TOXENV=apache_compat + services: docker + before_install: + addons: + - sudo: required + env: TOXENV=nginx_compat + services: docker + before_install: + addons: - python: "2.7" env: TOXENV=cover - python: "3.3" From cfc8ce9db436d5a0265a9569bf73e1f887126431 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 10 Aug 2016 17:01:34 -0700 Subject: [PATCH 41/47] Add function docstring --- certbot-nginx/certbot_nginx/constants.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/certbot-nginx/certbot_nginx/constants.py b/certbot-nginx/certbot_nginx/constants.py index 98f924b2b..8cf1f6bc9 100644 --- a/certbot-nginx/certbot_nginx/constants.py +++ b/certbot-nginx/certbot_nginx/constants.py @@ -21,5 +21,12 @@ def os_constant(key): # XXX TODO: In the future, this could return different constants # based on what OS we are running under. To see an # approach to how to handle different OSes, see the - # apache version of this file. + # apache version of this file. Currently, we do not + # actually have any OS-specific constants on Nginx. + """ + Get a constant value for operating system + + :param key: name of cli constant + :return: value of constant for active os + """ return CLI_DEFAULTS[key] From 4bbb12f182341c4d05fe808b03424467949e13b6 Mon Sep 17 00:00:00 2001 From: Seth Schoen Date: Wed, 10 Aug 2016 17:16:54 -0700 Subject: [PATCH 42/47] Satisfying some lint complaints --- .../configurators/apache/common.py | 1 - .../configurators/nginx/__init__.py | 2 +- .../configurators/nginx/common.py | 8 ++++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py index 4e612bbd5..b7b1f52c2 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py @@ -1,5 +1,4 @@ """Provides a common base for Apache proxies""" -import re import os import shutil import subprocess diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py index d559d0645..ed294abe6 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/__init__.py @@ -1 +1 @@ -"""Certbot compatibility test Apache configurators""" +"""Certbot compatibility test Nginx configurators""" diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 0d72605b7..311ae4ba6 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -1,5 +1,4 @@ -"""Provides a common base for Apache proxies""" -import re +"""Provides a common base for Nginx proxies""" import os import shutil import subprocess @@ -78,7 +77,7 @@ class Proxy(configurators_common.Proxy): # This does not appear to exist in nginx (yet?) # self.le_config.nginx_handle_modules = self.le_config.nginx_handle_mods - conf=configuration.NamespaceConfig(self.le_config) + conf = configuration.NamespaceConfig(self.le_config) zope.component.provideUtility(conf) self._nginx_configurator = configurator.NginxConfigurator( config=conf, name="nginx") @@ -132,6 +131,7 @@ def _get_names(config): for line in open(os.path.join(root, this_file)): if line.strip().startswith("server_name"): names = line.partition("server_name")[2].rpartition(";")[0] - [all_names.add(n) for n in names.split()] + for n in names.split(): + all_names.add(n) non_ip_names = set(n for n in all_names if not util.IP_REGEX.match(n)) return all_names, non_ip_names From 1f471da7685a790d9bc4f0d66b5f359df0877025 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 10 Aug 2016 17:39:29 -0700 Subject: [PATCH 43/47] Remove code duplication to make pylint happy --- .../configurators/apache/common.py | 38 +------------ .../configurators/common.py | 35 ++++++++++++ .../configurators/nginx/common.py | 57 +------------------ 3 files changed, 39 insertions(+), 91 deletions(-) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py index b7b1f52c2..64170ca72 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/apache/common.py @@ -27,30 +27,18 @@ class Proxy(configurators_common.Proxy): self.le_config.apache_le_vhost_ext = "-le-ssl.conf" self.modules = self.server_root = self.test_conf = self.version = None - self._apache_configurator = self._all_names = self._test_names = None patch = mock.patch( "certbot_apache.configurator.display_ops.select_vhost") mock_display = patch.start() mock_display.side_effect = le_errors.PluginError( "Unable to determine vhost") - def __getattr__(self, name): - """Wraps the Apache Configurator methods""" - method = getattr(self._apache_configurator, name, None) - if callable(method): - return method - else: - raise AttributeError() - def load_config(self): """Loads the next configuration for the plugin to test""" - config = super(Proxy, self).load_config() self._all_names, self._test_names = _get_names(config) server_root = _get_server_root(config) - # with open(os.path.join(config, "config_file")) as f: - # config_file = os.path.join(server_root, f.readline().rstrip()) shutil.rmtree("/etc/apache2") shutil.copytree(server_root, "/etc/apache2", symlinks=True) @@ -73,38 +61,16 @@ class Proxy(configurators_common.Proxy): # An alias self.le_config.apache_handle_modules = self.le_config.apache_handle_mods - self._apache_configurator = configurator.ApacheConfigurator( + self._configurator = configurator.ApacheConfigurator( config=configuration.NamespaceConfig(self.le_config), name="apache") - self._apache_configurator.prepare() + self._configurator.prepare() def cleanup_from_tests(self): """Performs any necessary cleanup from running plugin tests""" super(Proxy, self).cleanup_from_tests() mock.patch.stopall() - def get_all_names_answer(self): - """Returns the set of domain names that the plugin should find""" - if self._all_names: - return self._all_names - else: - raise errors.Error("No configuration file loaded") - - def get_testable_domain_names(self): - """Returns the set of domain names that can be tested against""" - if self._test_names: - return self._test_names - else: - return {"example.com"} - - def deploy_cert(self, domain, cert_path, key_path, chain_path=None, - fullchain_path=None): - """Installs cert""" - cert_path, key_path, chain_path = self.copy_certs_and_keys( - cert_path, key_path, chain_path) - self._apache_configurator.deploy_cert( - domain, cert_path, key_path, chain_path, fullchain_path) - def _get_server_root(config): """Returns the server root directory in config""" diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/common.py index 03128cc86..2a800c1c2 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/common.py @@ -5,6 +5,7 @@ import shutil import tempfile from certbot import constants +from certbot_compatibility_test import errors from certbot_compatibility_test import util @@ -31,6 +32,18 @@ class Proxy(object): self.args = args self.http_port = 80 self.https_port = 443 + self._configurator = self._all_names = self._test_names = None + + def __getattr__(self, name): + """Wraps the configurator methods""" + if self._configurator is None: + raise AttributeError() + + method = getattr(self._configurator, name, None) + if callable(method): + return method + else: + raise AttributeError() def has_more_configs(self): """Returns true if there are more configs to test""" @@ -63,3 +76,25 @@ class Proxy(object): chain = None return cert, key, chain + + def get_all_names_answer(self): + """Returns the set of domain names that the plugin should find""" + if self._all_names: + return self._all_names + else: + raise errors.Error("No configuration file loaded") + + def get_testable_domain_names(self): + """Returns the set of domain names that can be tested against""" + if self._test_names: + return self._test_names + else: + return {"example.com"} + + def deploy_cert(self, domain, cert_path, key_path, chain_path=None, + fullchain_path=None): + """Installs cert""" + cert_path, key_path, chain_path = self.copy_certs_and_keys( + cert_path, key_path, chain_path) + self._configurator.deploy_cert( + domain, cert_path, key_path, chain_path, fullchain_path) diff --git a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py index 311ae4ba6..3622bee41 100644 --- a/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py +++ b/certbot-compatibility-test/certbot_compatibility_test/configurators/nginx/common.py @@ -3,11 +3,9 @@ import os import shutil import subprocess -import mock import zope.interface from certbot import configuration -from certbot import errors as le_errors from certbot_nginx import configurator from certbot_nginx import constants from certbot_compatibility_test import errors @@ -22,36 +20,15 @@ class Proxy(configurators_common.Proxy): """A common base for Nginx test configurators""" def __init__(self, args): - # XXX: This is still apache-specific """Initializes the plugin with the given command line args""" super(Proxy, self).__init__(args) - self.le_config.apache_le_vhost_ext = "-le-ssl.conf" - - self.modules = self.server_root = self.test_conf = self.version = None - self._nginx_configurator = self._all_names = self._test_names = None - patch = mock.patch( - "certbot_apache.configurator.display_ops.select_vhost") - mock_display = patch.start() - mock_display.side_effect = le_errors.PluginError( - "Unable to determine vhost") - - def __getattr__(self, name): - """Wraps the Nginx Configurator methods""" - method = getattr(self._nginx_configurator, name, None) - if callable(method): - return method - else: - raise AttributeError() def load_config(self): """Loads the next configuration for the plugin to test""" - config = super(Proxy, self).load_config() self._all_names, self._test_names = _get_names(config) server_root = _get_server_root(config) - # with open(os.path.join(config, "config_file")) as f: - # config_file = os.path.join(server_root, f.readline().rstrip()) # XXX: Deleting all of this is kind of scary unless the test # instances really each have a complete configuration! @@ -74,41 +51,11 @@ class Proxy(configurators_common.Proxy): for k in constants.CLI_DEFAULTS.keys(): setattr(self.le_config, "nginx_" + k, constants.os_constant(k)) - # This does not appear to exist in nginx (yet?) - # self.le_config.nginx_handle_modules = self.le_config.nginx_handle_mods - conf = configuration.NamespaceConfig(self.le_config) zope.component.provideUtility(conf) - self._nginx_configurator = configurator.NginxConfigurator( + self._configurator = configurator.NginxConfigurator( config=conf, name="nginx") - self._nginx_configurator.prepare() - - def cleanup_from_tests(self): - """Performs any necessary cleanup from running plugin tests""" - super(Proxy, self).cleanup_from_tests() - mock.patch.stopall() - - def get_all_names_answer(self): - """Returns the set of domain names that the plugin should find""" - if self._all_names: - return self._all_names - else: - raise errors.Error("No configuration file loaded") - - def get_testable_domain_names(self): - """Returns the set of domain names that can be tested against""" - if self._test_names: - return self._test_names - else: - return {"example.com"} - - def deploy_cert(self, domain, cert_path, key_path, chain_path=None, - fullchain_path=None): - """Installs cert""" - cert_path, key_path, chain_path = self.copy_certs_and_keys( - cert_path, key_path, chain_path) - self._nginx_configurator.deploy_cert( - domain, cert_path, key_path, chain_path, fullchain_path) + self._configurator.prepare() def _get_server_root(config): From d541adcfa8d5ee5d867f260f4b465e8753709c8c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 12 Aug 2016 16:20:46 -0700 Subject: [PATCH 44/47] fix extra or lack of spacing between words in help for renew flags --- certbot/cli.py | 40 ++++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index b01b0a7f1..456a8d7f3 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -818,35 +818,39 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " used to create obtain or most recently successfully renew each" " certificate lineage. You can try it with `--dry-run` first. For" " more fine-grained control, you can renew individual lineages with" - " the `certonly` subcommand. Hooks are available to run commands " + " the `certonly` subcommand. Hooks are available to run commands" " before and after renewal; see" - " https://certbot.eff.org/docs/using.html#renewal for more information on these.") + " https://certbot.eff.org/docs/using.html#renewal for more" + " information on these.") helpful.add( "renew", "--pre-hook", - help="Command to be run in a shell before obtaining any certificates. Intended" - " primarily for renewal, where it can be used to temporarily shut down a" - " webserver that might conflict with the standalone plugin. This will " - " only be called if a certificate is actually to be obtained/renewed. ") + help="Command to be run in a shell before obtaining any certificates." + " Intended primarily for renewal, where it can be used to temporarily" + " shut down a webserver that might conflict with the standalone" + " plugin. This will only be called if a certificate is actually to be" + " obtained/renewed.") helpful.add( "renew", "--post-hook", - help="Command to be run in a shell after attempting to obtain/renew " - " certificates. Can be used to deploy renewed certificates, or to restart" - " any servers that were stopped by --pre-hook. This is only run if" - " an attempt was made to obtain/renew a certificate.") + help="Command to be run in a shell after attempting to obtain/renew" + " certificates. Can be used to deploy renewed certificates, or to" + " restart any servers that were stopped by --pre-hook. This is only" + " run if an attempt was made to obtain/renew a certificate.") helpful.add( "renew", "--renew-hook", - help="Command to be run in a shell once for each successfully renewed certificate." - "For this command, the shell variable $RENEWED_LINEAGE will point to the" - "config live subdirectory containing the new certs and keys; the shell variable " - "$RENEWED_DOMAINS will contain a space-delimited list of renewed cert domains") + help="Command to be run in a shell once for each successfully renewed" + " certificate. For this command, the shell variable $RENEWED_LINEAGE" + " will point to the config live subdirectory containing the new certs" + " and keys; the shell variable $RENEWED_DOMAINS will contain a" + " space-delimited list of renewed cert domains") helpful.add( "renew", "--disable-hook-validation", action='store_false', dest='validate_hooks', default=True, - help="Ordinarily the commands specified for --pre-hook/--post-hook/--renew-hook" - " will be checked for validity, to see if the programs being run are in the $PATH," - " so that mistakes can be caught early, even when the hooks aren't being run just yet." - " The validation is rather simplistic and fails if you use more advanced" + help="Ordinarily the commands specified for" + " --pre-hook/--post-hook/--renew-hook will be checked for validity, to" + " see if the programs being run are in the $PATH, so that mistakes can" + " be caught early, even when the hooks aren't being run just yet. The" + " validation is rather simplistic and fails if you use more advanced" " shell constructs, so you can use this switch to disable it.") helpful.add_deprecated_argument("--agree-dev-preview", 0) From 3a9769acbfcce37bbda5fbdf31b89729567ba73a Mon Sep 17 00:00:00 2001 From: Jon Walsh Date: Mon, 8 Aug 2016 15:13:23 +0700 Subject: [PATCH 45/47] Replace '-' with '_' before filtering plugin settings This bug notably occurs when renewing certs for with the plugin `letsencrypt-s3front` --- certbot/renewal.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/renewal.py b/certbot/renewal.py index d0c797a08..339d7b7ff 100644 --- a/certbot/renewal.py +++ b/certbot/renewal.py @@ -143,6 +143,7 @@ def _restore_plugin_configs(config, renewalparams): if renewalparams.get("installer", None) is not None: plugin_prefixes.append(renewalparams["installer"]) for plugin_prefix in set(plugin_prefixes): + plugin_prefix = plugin_prefix.replace('-', '_') for config_item, config_value in six.iteritems(renewalparams): if config_item.startswith(plugin_prefix + "_") and not cli.set_by_cli(config_item): # Values None, True, and False need to be treated specially, From 9ecca9bdf01ad7eccc40c744bc399b63eefc16cb Mon Sep 17 00:00:00 2001 From: Benjamin Piouffle Date: Wed, 17 Aug 2016 01:12:15 +0200 Subject: [PATCH 46/47] Update README.rst Link to https://certbot.eff.org/ was broken --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index e079cb36d..0b8e652c0 100644 --- a/README.rst +++ b/README.rst @@ -30,7 +30,7 @@ If you'd like to contribute to this project please read `Developer Guide Installation ------------ -The easiest way to install Certbot is by visiting certbot.eff.org_, where you can +The easiest way to install Certbot is by visiting https://certbot.eff.org, where you can find the correct installation instructions for many web server and OS combinations. For more information, see the `User Guide `_. From 015d103f8d90bc7a3fa1e94f13013702c1fc9ec5 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Tue, 16 Aug 2016 18:46:19 -0700 Subject: [PATCH 47/47] Unbreak using.html#plugins links Fixes: #3422 --- docs/using.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/using.rst b/docs/using.rst index 03b4cc6f3..92bf59000 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -231,6 +231,7 @@ whole process is described in the :doc:`contributing`. corrupt your operating system and are **not supported** by the Certbot team! +.. _plugins: Getting certificates ====================