Merge pull request #4895 from certbot/deploy-hook

Deploy hook

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 "
@@ -1085,24 +1085,28 @@ 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",
+                action=_RenewHookAction, help=argparse.SUPPRESS)
     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\"")
+        "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"
+        ' (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,
         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)
@@ -1349,3 +1353,25 @@ 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
+
+
+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

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):

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)
 

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
@@ -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:
@@ -323,6 +324,46 @@ 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_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(
+            ["--deploy-hook", value, "--disable-hook-validation"])
+        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_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(
+            ["--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."""

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')
@@ -25,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):
@@ -35,6 +49,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()

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
+root user, a deploy 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
 
@@ -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
@@ -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:
 

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,49 @@ CheckHooks() {
     rm "$HOOK_TEST"
 }
 
+# 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 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 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
 coverage erase
 
@@ -101,27 +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"'\
-       --renew-hook 'echo renew >> "$HOOK_TEST"'
+       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
 kill $python_server_pid
+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"'\
-       --renew-hook 'echo renew >> "$HOOK_TEST"'
+       --deploy-hook 'echo deploy >> "$HOOK_TEST"'
 kill $python_server_pid
+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"'
+    --post-hook 'echo wtf2.post >> "$HOOK_TEST"' \
+    --renew-hook 'echo deploy >> "$HOOK_TEST"'
+CheckRenewHook $certname
 
-common certonly -a manual -d dns.le.wtf --preferred-challenges dns,tls-sni \
-    --manual-auth-hook ./tests/manual-dns-auth.sh
+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"'
+CheckRenewHook $certname
 
 common certonly --cert-name newname -d newname.le.wtf
 

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 \