From 5828bf7eda5f3ff9ad691814740ffa1170f87144 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 25 Feb 2016 11:58:18 -0800 Subject: [PATCH 1/5] Cast webroot-path from str to [str] if needed - for compatibility with pre-public-beta renewal conf files - fixes #2542 --- letsencrypt/cli.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e8675a169..2258e29c3 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -871,8 +871,11 @@ def _restore_webroot_config(config, renewalparams): if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")): setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) elif "webroot_path" in renewalparams: + wp = renewalparams["webroot_path"] + if isinstance(wp, str): + wp = [wp] logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path") - setattr(config.namespace, "webroot_path", renewalparams["webroot_path"]) + setattr(config.namespace, "webroot_path", wp) def _reconstitute(config, full_path): From 152bfce313c75b0b5cac1d64b553465eea1fc797 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 25 Feb 2016 16:21:13 -0800 Subject: [PATCH 2/5] After much madness, a test case --- letsencrypt/cli.py | 6 +++--- letsencrypt/tests/cli_test.py | 22 ++++++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 2258e29c3..3551d5a10 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -871,10 +871,10 @@ def _restore_webroot_config(config, renewalparams): if not (_set_by_cli("webroot_map") or _set_by_cli("webroot_path")): setattr(config.namespace, "webroot_map", renewalparams["webroot_map"]) elif "webroot_path" in renewalparams: - wp = renewalparams["webroot_path"] - if isinstance(wp, str): - wp = [wp] logger.info("Ancient renewal conf file without webroot-map, restoring webroot-path") + wp = renewalparams["webroot_path"] + if isinstance(wp, str): # prior to 0.1.0, webroot_path was a string + wp = [wp] setattr(config.namespace, "webroot_path", wp) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2b4e443a8..ffef21c63 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -20,6 +20,7 @@ from letsencrypt import constants from letsencrypt import crypto_util from letsencrypt import errors from letsencrypt import le_util +from letsencrypt import storage from letsencrypt.plugins import disco from letsencrypt.plugins import manual @@ -630,8 +631,9 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print "Logs:" print lf.read() - def test_renew_verb(self): - with open(test_util.vector_path('sample-renewal.conf')) as src: + + def _make_test_renewal_conf(self, testfile): + with open(test_util.vector_path(testfile)) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) rd = os.path.join(self.config_dir, "renewal") @@ -640,9 +642,25 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) + return rc + + def test_renew_verb(self): + self._make_test_renewal_conf('sample-renewal.conf') args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) + @mock.patch("letsencrypt.cli._set_by_cli") + def test_ancient_webroot(self, mock_set_by_cli): + mock_set_by_cli.return_value = False + rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf') + args = mock.MagicMock(account=None, email=None, webroot_path=None) + config = configuration.NamespaceConfig(args) + lineage = storage.RenewableCert(rc_path, + configuration.RenewerConfiguration(config)) + renewalparams = lineage.configuration["renewalparams"] + cli._restore_webroot_config(config, renewalparams) + self.assertEqual(config.webroot_path, ["/var/www/"]) + def test_renew_verb_empty_config(self): rd = os.path.join(self.config_dir, 'renewal') if not os.path.exists(rd): From 087271204d6657e77a8a73d1e342669802d9012e Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 25 Feb 2016 16:22:02 -0800 Subject: [PATCH 3/5] And the renewal conf file for the test case... --- .../testdata/sample-renewal-ancient.conf | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100755 letsencrypt/tests/testdata/sample-renewal-ancient.conf diff --git a/letsencrypt/tests/testdata/sample-renewal-ancient.conf b/letsencrypt/tests/testdata/sample-renewal-ancient.conf new file mode 100755 index 000000000..ff246ba7c --- /dev/null +++ b/letsencrypt/tests/testdata/sample-renewal-ancient.conf @@ -0,0 +1,75 @@ +cert = MAGICDIR/live/sample-renewal/cert.pem +privkey = MAGICDIR/live/sample-renewal/privkey.pem +chain = MAGICDIR/live/sample-renewal/chain.pem +fullchain = MAGICDIR/live/sample-renewal/fullchain.pem +renew_before_expiry = 1 year + +# Options and defaults used in the renewal process +[renewalparams] +no_self_upgrade = False +apache_enmod = a2enmod +no_verify_ssl = False +ifaces = None +apache_dismod = a2dismod +register_unsafely_without_email = False +apache_handle_modules = True +uir = None +installer = none +nginx_ctl = nginx +config_dir = MAGICDIR +text_mode = False +func = +staging = True +prepare = False +work_dir = /var/lib/letsencrypt +tos = False +init = False +http01_port = 80 +duplicate = False +noninteractive_mode = True +key_path = None +nginx = False +nginx_server_root = /etc/nginx +fullchain_path = /home/ubuntu/letsencrypt/chain.pem +email = None +csr = None +agree_dev_preview = None +redirect = None +verb = certonly +verbose_count = -3 +config_file = None +renew_by_default = False +hsts = False +apache_handle_sites = True +authenticator = webroot +domains = isnot.org, +rsa_key_size = 2048 +apache_challenge_location = /etc/apache2 +checkpoints = 1 +manual_test_mode = False +apache = False +cert_path = /home/ubuntu/letsencrypt/cert.pem +webroot_path = /var/www/ +reinstall = False +expand = False +strict_permissions = False +apache_server_root = /etc/apache2 +account = None +dry_run = False +manual_public_ip_logging_ok = False +chain_path = /home/ubuntu/letsencrypt/chain.pem +break_my_certs = False +standalone = True +manual = False +server = https://acme-staging.api.letsencrypt.org/directory +standalone_supported_challenges = "tls-sni-01,http-01" +webroot = True +os_packages_only = False +apache_init_script = None +user_agent = None +apache_le_vhost_ext = -le-ssl.conf +debug = False +tls_sni_01_port = 443 +logs_dir = /var/log/letsencrypt +apache_vhost_root = /etc/apache2/sites-available +configurator = None From 6e0457841cafc38da6c3d0354f42a1df14c3a4a2 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 25 Feb 2016 16:41:34 -0800 Subject: [PATCH 4/5] More accurate function name --- letsencrypt/tests/cli_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index ffef21c63..e7f4ca2d4 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -650,7 +650,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self._test_renewal_common(True, [], args=args, renew=True) @mock.patch("letsencrypt.cli._set_by_cli") - def test_ancient_webroot(self, mock_set_by_cli): + def test_ancient_webroot_renewal_conf(self, mock_set_by_cli): mock_set_by_cli.return_value = False rc_path = self._make_test_renewal_conf('sample-renewal-ancient.conf') args = mock.MagicMock(account=None, email=None, webroot_path=None) From 5c6638f60a99f358b118deab48574502c60834e6 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Thu, 25 Feb 2016 16:43:05 -0800 Subject: [PATCH 5/5] lint --- letsencrypt/tests/cli_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index e7f4ca2d4..aef3447c3 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -658,6 +658,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods lineage = storage.RenewableCert(rc_path, configuration.RenewerConfiguration(config)) renewalparams = lineage.configuration["renewalparams"] + # pylint: disable=protected-access cli._restore_webroot_config(config, renewalparams) self.assertEqual(config.webroot_path, ["/var/www/"])