From d57e8bfaa39f0b1286f82c8a5d33e2cbf46558fb Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:11:51 -0400 Subject: [PATCH 01/17] add --deploy-hook --- certbot/cli.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/certbot/cli.py b/certbot/cli.py index 14874e63e..42510ad11 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1094,6 +1094,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " and keys; the shell variable $RENEWED_DOMAINS will contain a" " space-delimited list of renewed certificate domains (for example," " \"example.com www.example.com\"") + helpful.add( + "renew", "--deploy-hook", + help="Command to be run in a shell once for each successfully" + " issued certificate. For this command, the shell variable" + " $RENEWED_LINEAGE will point to the config live subdirectory" + ' (for example, "/etc/letsencrypt/live/example.com") containing' + " the new certificates and keys; the shell variable" + " $RENEWED_DOMAINS will contain a space-delimited list of" + ' renewed certificate domains (for example, "example.com' + ' www.example.com"') helpful.add( "renew", "--disable-hook-validation", action='store_false', dest='validate_hooks', default=True, From af354e909916e72068e8824456fb28b0ed6d011c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:18:07 -0400 Subject: [PATCH 02/17] add --deploy-hook parsing --- certbot/cli.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index 42510ad11..5aa267414 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1095,7 +1095,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " space-delimited list of renewed certificate domains (for example," " \"example.com www.example.com\"") helpful.add( - "renew", "--deploy-hook", + "renew", "--deploy-hook", action=_DeployHookAction, help="Command to be run in a shell once for each successfully" " issued certificate. For this command, the shell variable" " $RENEWED_LINEAGE will point to the config live subdirectory" @@ -1359,3 +1359,14 @@ def parse_preferred_challenges(pref_challs): raise errors.Error( "Unrecognized challenges: {0}".format(unrecognized)) return challs + + +class _DeployHookAction(argparse.Action): + """Action class for parsing deploy hooks.""" + + def __call__(self, parser, namespace, values, option_string=None): + renew_hook_set = namespace.deploy_hook != namespace.renew_hook + if renew_hook_set and namespace.renew_hook != values: + raise argparse.ArgumentError( + self, "conflicts with --renew-hook value") + namespace.deploy_hook = namespace.renew_hook = values From 5cf82e4843038ecb86aeb01bf1d298f65ac9d5eb Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:26:18 -0400 Subject: [PATCH 03/17] test --deploy-hook --- certbot/tests/cli_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index f18da240a..84463d2d7 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -323,6 +323,18 @@ class ParseTest(unittest.TestCase): self.assertRaises( errors.Error, self.parse, "-n --force-interactive".split()) + def test_deploy_hook_conflict(self): + with mock.patch("certbot.cli.sys.stderr"): + self.assertRaises(SystemExit, self.parse, + "--renew-hook foo --deploy-hook bar".split()) + + def test_deploy_hook_sets_renew_hook(self): + value = "foo" + namespace = self.parse( + ["--deploy-hook", value, "--disable-hook-validation"]) + self.assertEqual(namespace.deploy_hook, value) + self.assertEqual(namespace.renew_hook, value) + class DefaultTest(unittest.TestCase): """Tests for certbot.cli._Default.""" From ed4be4117cbb9146d9754bc38e8d8eb54a1a0980 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:29:39 -0400 Subject: [PATCH 04/17] hide --renew-hook --- certbot/cli.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 5aa267414..8e0085390 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1085,15 +1085,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " run if an attempt was made to obtain/renew a certificate. If" " multiple renewed certificates have identical post-hooks, only" " one will be run.") - 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 (for example," - " \"/etc/letsencrypt/live/example.com\") containing the new certificates" - " and keys; the shell variable $RENEWED_DOMAINS will contain a" - " space-delimited list of renewed certificate domains (for example," - " \"example.com www.example.com\"") + helpful.add("renew", "--renew-hook", help=argparse.SUPPRESS) helpful.add( "renew", "--deploy-hook", action=_DeployHookAction, help="Command to be run in a shell once for each successfully" From 220d486190f982f60efd90b30ddd492305a18b4f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:36:46 -0400 Subject: [PATCH 05/17] remove --renew-hook from help output --- certbot/cli.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 8e0085390..8e078c058 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -887,7 +887,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " in order to obtain test certificates, and reloads webservers to deploy and then" " roll back those changes. It also calls --pre-hook and --post-hook commands" " if they are defined because they may be necessary to accurately simulate" - " renewal. --renew-hook commands are not called.") + " renewal. --deploy-hook commands are not called.") helpful.add( ["register", "automation"], "--register-unsafely-without-email", action="store_true", help="Specifying this flag enables registering an account with no " @@ -1100,11 +1100,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "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" - " shell constructs, so you can use this switch to disable it." + " --pre-hook/--post-hook/--deploy-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." " (default: False)") helpful.add_deprecated_argument("--agree-dev-preview", 0) From 4243db1525c569f5cf55e372a9714a7e6a0121e8 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:38:39 -0400 Subject: [PATCH 06/17] test --renew-hook is hidden --- certbot/tests/cli_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 84463d2d7..e76db02f7 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -109,6 +109,7 @@ class ParseTest(unittest.TestCase): self.assertTrue("--dialog" not in out) self.assertTrue("%s" not in out) self.assertTrue("{0}" not in out) + self.assertTrue("--renew-hook" not in out) out = self._help_output(['-h', 'nginx']) if "nginx" in PLUGINS: From feffeb275b3123ccba8663b1a37795daf6fccf42 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:46:32 -0400 Subject: [PATCH 07/17] add --renew-hook error handling --- certbot/cli.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index 8e078c058..fd9378079 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1085,7 +1085,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis " run if an attempt was made to obtain/renew a certificate. If" " multiple renewed certificates have identical post-hooks, only" " one will be run.") - helpful.add("renew", "--renew-hook", help=argparse.SUPPRESS) + helpful.add("renew", "--renew-hook", + action=_RenewHookAction, help=argparse.SUPPRESS) helpful.add( "renew", "--deploy-hook", action=_DeployHookAction, help="Command to be run in a shell once for each successfully" @@ -1363,3 +1364,14 @@ class _DeployHookAction(argparse.Action): raise argparse.ArgumentError( self, "conflicts with --renew-hook value") namespace.deploy_hook = namespace.renew_hook = values + + +class _RenewHookAction(argparse.Action): + """Action class for parsing renew hooks.""" + + def __call__(self, parser, namespace, values, option_string=None): + deploy_hook_set = namespace.deploy_hook is not None + if deploy_hook_set and namespace.deploy_hook != values: + raise argparse.ArgumentError( + self, "conflicts with --deploy-hook value") + namespace.renew_hook = values From ad4ed22932740d1c178acae485a9a9f1a9c59cc4 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:55:36 -0400 Subject: [PATCH 08/17] test --renew-hook --- certbot/tests/cli_test.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index e76db02f7..3a943fd1a 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -40,7 +40,7 @@ class TestReadFile(TempDirTestCase): -class ParseTest(unittest.TestCase): +class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods '''Test the cli args entrypoint''' _multiprocess_can_split_ = True @@ -336,6 +336,18 @@ class ParseTest(unittest.TestCase): self.assertEqual(namespace.deploy_hook, value) self.assertEqual(namespace.renew_hook, value) + def test_renew_hook_conflict(self): + with mock.patch("certbot.cli.sys.stderr"): + self.assertRaises(SystemExit, self.parse, + "--deploy-hook foo --renew-hook bar".split()) + + def test_renew_hook_does_not_set_renew_hook(self): + value = "foo" + namespace = self.parse( + ["--renew-hook", value, "--disable-hook-validation"]) + self.assertEqual(namespace.deploy_hook, None) + self.assertEqual(namespace.renew_hook, value) + class DefaultTest(unittest.TestCase): """Tests for certbot.cli._Default.""" From 32fa3b1d04cac71f90b5ac6d9bf92828b37c1306 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 09:59:19 -0400 Subject: [PATCH 09/17] test deploy-hook and renew-hook match --- certbot/tests/cli_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 3a943fd1a..cad04f88e 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -329,6 +329,14 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(SystemExit, self.parse, "--renew-hook foo --deploy-hook bar".split()) + def test_deploy_hook_matches_renew_hook(self): + value = "foo" + namespace = self.parse(["--renew-hook", value, + "--deploy-hook", value, + "--disable-hook-validation"]) + self.assertEqual(namespace.deploy_hook, value) + self.assertEqual(namespace.renew_hook, value) + def test_deploy_hook_sets_renew_hook(self): value = "foo" namespace = self.parse( @@ -341,6 +349,14 @@ class ParseTest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertRaises(SystemExit, self.parse, "--deploy-hook foo --renew-hook bar".split()) + def test_renew_hook_matches_deploy_hook(self): + value = "foo" + namespace = self.parse(["--deploy-hook", value, + "--renew-hook", value, + "--disable-hook-validation"]) + self.assertEqual(namespace.deploy_hook, value) + self.assertEqual(namespace.renew_hook, value) + def test_renew_hook_does_not_set_renew_hook(self): value = "foo" namespace = self.parse( From e94ee31a6f556fc32e424dd7562b6ea363e21bea Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 10:24:00 -0400 Subject: [PATCH 10/17] add hooks.deploy_hook --- certbot/hooks.py | 19 +++++++++++++++++-- certbot/tests/hook_test.py | 3 ++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/certbot/hooks.py b/certbot/hooks.py index b3c1fc3e2..799ef90b7 100644 --- a/certbot/hooks.py +++ b/certbot/hooks.py @@ -18,6 +18,7 @@ def validate_hooks(config): """Check hook commands are executable.""" validate_hook(config.pre_hook, "pre") validate_hook(config.post_hook, "post") + validate_hook(config.deploy_hook, "deploy") validate_hook(config.renew_hook, "renew") @@ -95,16 +96,30 @@ def run_saved_post_hooks(): _run_hook(cmd) +def deploy_hook(config, domains, lineage_path): + """Run post-issuance hook if defined. + + :param configuration.NamespaceConfig config: Certbot settings + :param domains: domains in the obtained certificate + :type domains: `list` of `str` + :param str lineage_path: live directory path for the new cert + + """ + if config.deploy_hook: + renew_hook(config, domains, lineage_path) + + def renew_hook(config, domains, lineage_path): """Run post-renewal hook if defined.""" if config.renew_hook: if not config.dry_run: os.environ["RENEWED_DOMAINS"] = " ".join(domains) os.environ["RENEWED_LINEAGE"] = lineage_path - logger.info("Running renew-hook command: %s", config.renew_hook) + logger.info("Running deploy-hook command: %s", config.renew_hook) _run_hook(config.renew_hook) else: - logger.warning("Dry run: skipping renewal hook command: %s", config.renew_hook) + logger.warning( + "Dry run: skipping deploy hook command: %s", config.renew_hook) def _run_hook(shell_cmd): diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index 0fbb91492..fe2579a89 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -16,7 +16,8 @@ class HookTest(unittest.TestCase): @mock.patch('certbot.hooks._prog') def test_validate_hooks(self, mock_prog): - config = mock.MagicMock(pre_hook="", post_hook="ls -lR", renew_hook="uptime") + config = mock.MagicMock(deploy_hook=None, pre_hook="", + post_hook="ls -lR", renew_hook="uptime") hooks.validate_hooks(config) self.assertEqual(mock_prog.call_count, 2) self.assertEqual(mock_prog.call_args_list[1][0][0], 'uptime') From 1b65ba88d8f86a96b1e231ae657e39e69588c631 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 10:30:33 -0400 Subject: [PATCH 11/17] test hooks.deploy_hook --- certbot/tests/hook_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index fe2579a89..28417ac73 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -36,6 +36,19 @@ class HookTest(unittest.TestCase): self.assertEqual(hooks._prog("funky"), None) self.assertEqual(mock_ps.call_count, 1) + @mock.patch('certbot.hooks.renew_hook') + def test_deploy_hook(self, mock_renew_hook): + args = (mock.Mock(deploy_hook='foo'), ['example.org'], 'path',) + # pylint: disable=star-args + hooks.deploy_hook(*args) + mock_renew_hook.assert_called_once_with(*args) + + @mock.patch('certbot.hooks.renew_hook') + def test_no_deploy_hook(self, mock_renew_hook): + args = (mock.Mock(deploy_hook=None), ['example.org'], 'path',) + hooks.deploy_hook(*args) # pylint: disable=star-args + mock_renew_hook.assert_not_called() + def _test_a_hook(self, config, hook_function, calls_expected, **kwargs): with mock.patch('certbot.hooks.logger') as mock_logger: mock_logger.warning = mock.MagicMock() From 8a664622eac4ca2fa23edf065228ed0ff967f6bd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 10:33:49 -0400 Subject: [PATCH 12/17] Call deploy_hook during certonly and run --- certbot/main.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/certbot/main.py b/certbot/main.py index cd87706b4..f7421d75e 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -82,6 +82,8 @@ def _get_and_save_cert(le_client, config, domains=None, certname=None, lineage=N lineage = le_client.obtain_and_enroll_certificate(domains, certname) if lineage is False: raise errors.Error("Certificate could not be obtained") + elif lineage is not None: + hooks.deploy_hook(config, lineage.names(), lineage.live_dir) finally: hooks.post_hook(config) From 6dedfa62b63d7d20b60461eeea3d32513f49d0f6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 11:06:51 -0400 Subject: [PATCH 13/17] Test renew and deploy hooks are run properly --- tests/boulder-integration.sh | 52 +++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 913880c8b..a356da884 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -52,15 +52,19 @@ CheckHooks() { if [ $(head -n1 $HOOK_TEST) = "wtf.pre" ]; then echo "wtf.pre" > "$EXPECTED" echo "wtf2.pre" >> "$EXPECTED" - echo "renew" >> "$EXPECTED" - echo "renew" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" echo "wtf.post" >> "$EXPECTED" echo "wtf2.post" >> "$EXPECTED" else echo "wtf2.pre" > "$EXPECTED" echo "wtf.pre" >> "$EXPECTED" - echo "renew" >> "$EXPECTED" - echo "renew" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" + echo "deploy" >> "$EXPECTED" echo "wtf2.post" >> "$EXPECTED" echo "wtf.post" >> "$EXPECTED" fi @@ -74,6 +78,27 @@ CheckHooks() { rm "$HOOK_TEST" } +# Checks if deploy was run and deletes the hook file +CheckDeployHook() { + CONTENTS=$(cat "$HOOK_TEST") + rm "$HOOK_TEST" + grep deploy <(echo "$CONTENTS") +} + +# Asserts the deploy hook was run and deletes the hook file +AssertDeployHook() { + if ! CheckDeployHook; then + echo "The deploy hook wasn't run" >&2 + fi +} + +# Asserts the deploy hook wasn't run and deletes the hook file +AssertNoDeployHook() { + if CheckDeployHook; then + echo "The deploy hook was incorrectly run" >&2 + fi +} + # Cleanup coverage data coverage erase @@ -104,24 +129,33 @@ python_server_pid=$! common --domains le1.wtf --preferred-challenges tls-sni-01 auth \ --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf.post >> "$HOOK_TEST"'\ - --renew-hook 'echo renew >> "$HOOK_TEST"' + --deploy-hook 'echo deploy >> "$HOOK_TEST"' kill $python_server_pid +AssertDeployHook + python ./tests/run_http_server.py $tls_sni_01_port & python_server_pid=$! common --domains le2.wtf --preferred-challenges http-01 run \ --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf.post >> "$HOOK_TEST"'\ - --renew-hook 'echo renew >> "$HOOK_TEST"' + --deploy-hook 'echo deploy >> "$HOOK_TEST"' kill $python_server_pid +AssertDeployHook common certonly -a manual -d le.wtf --rsa-key-size 4096 \ --manual-auth-hook ./tests/manual-http-auth.sh \ --manual-cleanup-hook ./tests/manual-http-cleanup.sh \ --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \ - --post-hook 'echo wtf2.post >> "$HOOK_TEST"' + --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \ + --renew-hook 'echo deploy >> "$HOOK_TEST"' +AssertNoDeployHook -common certonly -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni \ - --manual-auth-hook ./tests/manual-dns-auth.sh +common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \ + --manual-auth-hook ./tests/manual-dns-auth.sh \ + --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \ + --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \ + --renew-hook 'echo deploy >> "$HOOK_TEST"' +AssertNoDeployHook common certonly --cert-name newname -d newname.le.wtf From 4c19d19cf5b3fcecd5ca6a0c1b3d68c254efe379 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 11:30:21 -0400 Subject: [PATCH 14/17] Test that deploy and renew hooks are saved right It is important that both renew and deploy hooks are saved as renew_hook in renewal configuration files to preserve forwards compatibility. --- tests/boulder-integration.sh | 59 +++++++++++++++++++++++++++--------- tests/integration/_common.sh | 5 +-- 2 files changed, 47 insertions(+), 17 deletions(-) diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index a356da884..181dcc5fb 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -78,25 +78,47 @@ CheckHooks() { rm "$HOOK_TEST" } -# Checks if deploy was run and deletes the hook file -CheckDeployHook() { +# Checks if deploy is in the hook output and deletes the file +DeployInHookOutput() { CONTENTS=$(cat "$HOOK_TEST") rm "$HOOK_TEST" grep deploy <(echo "$CONTENTS") } -# Asserts the deploy hook was run and deletes the hook file -AssertDeployHook() { - if ! CheckDeployHook; then - echo "The deploy hook wasn't run" >&2 +# Asserts that there is a saved renew_hook for a lineage. +# +# Arguments: +# Name of lineage to check +CheckSavedRenewHook() { + if ! grep renew_hook "$config_dir/renewal/$1.conf"; then + echo "Hook wasn't saved as renew_hook" >&2 + exit 1 fi } -# Asserts the deploy hook wasn't run and deletes the hook file -AssertNoDeployHook() { - if CheckDeployHook; then - echo "The deploy hook was incorrectly run" >&2 +# Asserts the deploy hook was properly run and saved and deletes the hook file +# +# Arguments: +# Lineage name of the issued cert +CheckDeployHook() { + if ! DeployInHookOutput; then + echo "The deploy hook wasn't run" >&2 + exit 1 fi + CheckSavedRenewHook $1 +} + +# Asserts the renew hook wasn't run but was saved and deletes the hook file +# +# Arguments: +# Lineage name of the issued cert +# Asserts the deploy hook wasn't run and deletes the hook file +CheckRenewHook() { + if DeployInHookOutput; then + echo "The renew hook was incorrectly run" >&2 + exit 1 + fi + CheckSavedRenewHook $1 } # Cleanup coverage data @@ -126,36 +148,43 @@ common plugins --init --prepare | grep webroot python ./tests/run_http_server.py $http_01_port & python_server_pid=$! +certname="le1.wtf" common --domains le1.wtf --preferred-challenges tls-sni-01 auth \ + --cert-name $certname \ --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf.post >> "$HOOK_TEST"'\ --deploy-hook 'echo deploy >> "$HOOK_TEST"' kill $python_server_pid -AssertDeployHook +CheckDeployHook $certname python ./tests/run_http_server.py $tls_sni_01_port & python_server_pid=$! +certname="le2.wtf" common --domains le2.wtf --preferred-challenges http-01 run \ + --cert-name $certname \ --pre-hook 'echo wtf.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf.post >> "$HOOK_TEST"'\ --deploy-hook 'echo deploy >> "$HOOK_TEST"' kill $python_server_pid -AssertDeployHook +CheckDeployHook $certname -common certonly -a manual -d le.wtf --rsa-key-size 4096 \ +certname="le.wtf" +common certonly -a manual -d le.wtf --rsa-key-size 4096 --cert-name $certname \ --manual-auth-hook ./tests/manual-http-auth.sh \ --manual-cleanup-hook ./tests/manual-http-cleanup.sh \ --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \ --renew-hook 'echo deploy >> "$HOOK_TEST"' -AssertNoDeployHook +CheckRenewHook $certname +certname="dns.le.wtf" common -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni run \ + --cert-name $certname \ --manual-auth-hook ./tests/manual-dns-auth.sh \ --pre-hook 'echo wtf2.pre >> "$HOOK_TEST"' \ --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \ --renew-hook 'echo deploy >> "$HOOK_TEST"' -AssertNoDeployHook +CheckRenewHook $certname common certonly --cert-name newname -d newname.le.wtf diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 48d20eb3b..d151bdc3f 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -2,12 +2,13 @@ # the kernel to use. root=${root:-$(mktemp -d -t leitXXXX)} echo "Root integration tests directory: $root" -store_flags="--config-dir $root/conf --work-dir $root/work" +config_dir="$root/conf" +store_flags="--config-dir $config_dir --work-dir $root/work" store_flags="$store_flags --logs-dir $root/logs" tls_sni_01_port=5001 http_01_port=5002 sources="acme/,$(ls -dm certbot*/ | tr -d ' \n')" -export root store_flags tls_sni_01_port http_01_port sources +export root config_dir store_flags tls_sni_01_port http_01_port sources certbot_test () { certbot_test_no_force_renew \ From 62327b49c37b9c9114ae2daac14f9b41513fa590 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 30 Jun 2017 11:40:34 -0400 Subject: [PATCH 15/17] Test hook validation order --- certbot/tests/hook_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/certbot/tests/hook_test.py b/certbot/tests/hook_test.py index 28417ac73..964d69866 100644 --- a/certbot/tests/hook_test.py +++ b/certbot/tests/hook_test.py @@ -26,6 +26,19 @@ class HookTest(unittest.TestCase): config = mock.MagicMock(pre_hook="explodinator", post_hook="", renew_hook="") self.assertRaises(errors.HookCommandNotFound, hooks.validate_hooks, config) + @mock.patch('certbot.hooks.validate_hook') + def test_validation_order(self, mock_validate_hook): + # This ensures error messages are about deploy hook when appropriate + config = mock.Mock(deploy_hook=None, pre_hook=None, + post_hook=None, renew_hook=None) + hooks.validate_hooks(config) + + order = [call[0][1] for call in mock_validate_hook.call_args_list] + self.assertTrue('pre' in order) + self.assertTrue('post' in order) + self.assertTrue('deploy' in order) + self.assertEqual(order[-1], 'renew') + @mock.patch('certbot.hooks.util.exe_exists') @mock.patch('certbot.hooks.plug_util.path_surgery') def test_prog(self, mock_ps, mock_exe_exists): From f314ea1d33b7d3e9e054b5e36da2a0f17984d9a1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 6 Jul 2017 10:30:29 -0400 Subject: [PATCH 16/17] s/renew-hook/deploy-hook docs/using.rst --- docs/using.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 0b2206db2..abdf5c619 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -397,15 +397,15 @@ unnecessarily stopping your webserver. ``--pre-hook`` and ``--post-hook`` hooks run before and after every renewal attempt. If you want your hook to run only after a successful renewal, use -``--renew-hook`` in a command like this. +``--deploy-hook`` in a command like this. -``certbot renew --renew-hook /path/to/renew-hook-script`` +``certbot renew --deploy-hook /path/to/deploy-hook-script`` For example, if you have a daemon that does not read its certificates as the root user, a renew hook like this can copy them to the correct location and apply appropriate file permissions. -/path/to/renew-hook-script +/path/to/deploy-hook-script .. code-block:: none @@ -504,7 +504,7 @@ renewal configuration file, located at ``/etc/letsencrypt/renewal/CERTNAME``. .. warning:: Modifying any files in ``/etc/letsencrypt`` can damage them so Certbot can no longer properly manage its certificates, and we do not recommend doing so. For most tasks, it is safest to limit yourself to pointing symlinks at the files there, or using -``--renew-hook`` to copy / make new files based upon those files, if your operational situation requires it +``--deploy-hook`` to copy / make new files based upon those files, if your operational situation requires it (for instance, combining certificates and keys in different way, or having copies of things with different specific permissions that are demanded by other programs). @@ -598,7 +598,7 @@ The following files are available: .. note:: All files are PEM-encoded. If you need other format, such as DER or PFX, then you could convert using ``openssl``. You can automate that with - ``--renew-hook`` if you're using automatic renewal_. + ``--deploy-hook`` if you're using automatic renewal_. .. _hooks: From c3c1609fa025b3628c9908fd8ee3f1fa67adcdba Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 6 Jul 2017 14:59:28 -0400 Subject: [PATCH 17/17] no more renew(al) hook(s) --- docs/using.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index abdf5c619..ef87157aa 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -402,7 +402,7 @@ attempt. If you want your hook to run only after a successful renewal, use ``certbot renew --deploy-hook /path/to/deploy-hook-script`` For example, if you have a daemon that does not read its certificates as the -root user, a renew hook like this can copy them to the correct location and +root user, a deploy hook like this can copy them to the correct location and apply appropriate file permissions. /path/to/deploy-hook-script @@ -438,7 +438,7 @@ apply appropriate file permissions. esac done -More information about renewal hooks can be found by running +More information about hooks can be found by running ``certbot --help renew``. If you're sure that this command executes successfully without human