From aa80c09c803e132c3a05a642e9ada82ccb0e7dd8 Mon Sep 17 00:00:00 2001
From: Evilham <cvs@evilham.com>
Date: Wed, 10 Feb 2021 10:10:21 +0100
Subject: [PATCH] [__letsencrypt_cert] Move hook contents generation out of
 manifest

While there address some minor issues in the comments in the hook contents.
---
 .../type/__letsencrypt_cert/files/gen_hook.sh | 84 +++++++++++++++++++
 cdist/conf/type/__letsencrypt_cert/manifest   | 78 +----------------
 2 files changed, 88 insertions(+), 74 deletions(-)
 create mode 100644 cdist/conf/type/__letsencrypt_cert/files/gen_hook.sh

diff --git a/cdist/conf/type/__letsencrypt_cert/files/gen_hook.sh b/cdist/conf/type/__letsencrypt_cert/files/gen_hook.sh
new file mode 100644
index 00000000..81ea4856
--- /dev/null
+++ b/cdist/conf/type/__letsencrypt_cert/files/gen_hook.sh
@@ -0,0 +1,84 @@
+#!/bin/sh -e
+
+# It is expected that this defines hook_contents
+
+# Reasonable defaults
+hook_source="${__object}/parameter/${hook}-hook"
+hook_state="absent"
+hook_contents_head="#!/bin/sh -e"
+hook_contents_logic=""
+hook_contents_tail=""
+
+# Backwards compatibility
+# Remove this when renew-hook is removed
+# Falling back to renew-hook if deploy-hook is not passed
+if [ "${hook}" = "deploy" ] && [ ! -f "${hook_source}" ]; then
+	hook_source="${__object}/parameter/renew-hook"
+fi
+if [ "${state}" = "present" ] && \
+	[ -f "${hook_source}" ]; then
+	# This hook is to be installed, let's generate it with some
+	# safety boilerplate
+	# Since certbot runs all hooks for all renewal processes
+	# (at each state for deploy, pre, post), it is up to us to
+	# differentiate whether or not the hook must run
+	hook_state="present"
+	hook_contents_head="$(cat <<EOF
+#!/bin/sh -e
+#
+# Managed remotely with https://cdi.st
+#
+# Domains for which this hook is supposed to apply
+lineage="${LE_DIR}/live/${__object_id}"
+domains="\$(cat <<eof
+${domains}
+eof
+)"
+EOF
+)"
+	case "${hook}" in
+		pre|post)
+			# Certbot is kind of terrible, we have
+			# no way of knowing what domain/lineage the
+			# hook is running for
+			hook_contents_logic="$(cat <<EOF
+# pre/post-hooks apply always due to a certbot limitation
+APPLY_HOOK="YES"
+EOF
+)"
+		;;
+		deploy)
+			hook_contents_logic="$(cat <<EOF
+# certbot defines these environment variables:
+# RENEWED_DOMAINS="DOMAIN1 DOMAIN2"
+# RENEWED_LINEAGE="/etc/letsencrypt/live/__object_id"
+# It feels more stable to use RENEWED_LINEAGE
+if [ "\${lineage}" = "\${RENEWED_LINEAGE}" ]; then
+	APPLY_HOOK="YES"
+fi
+EOF
+)"
+		;;
+		*)
+			echo "Unknown hook '${hook}'" >> /dev/stderr
+			exit 1
+		;;
+	esac
+
+	hook_contents_tail="$(cat <<EOF
+if [ -n "\${APPLY_HOOK}" ]; then
+	# Messing with indentation can eff up the users' scripts, let's not
+$(cat "${hook_source}")
+fi
+EOF
+)"
+fi
+
+hook_contents="$(cat <<EOF
+${hook_contents_head}
+
+${hook_contents_logic}
+
+${hook_contents_tail}
+EOF
+)"
diff --git a/cdist/conf/type/__letsencrypt_cert/manifest b/cdist/conf/type/__letsencrypt_cert/manifest
index 84221a80..1df3574a 100644
--- a/cdist/conf/type/__letsencrypt_cert/manifest
+++ b/cdist/conf/type/__letsencrypt_cert/manifest
@@ -141,77 +141,11 @@ fi
 for hook in deploy pre post; do
 	# Using something unique and specific to this object
 	hook_file="${HOOKS_DIR}/${hook}/${__object_id}.cdist.sh"
-	# Reasonable defaults
-	hook_source="${__object}/parameter/${hook}-hook"
-	hook_state="absent"
-	hook_contents_head="#!/bin/sh -e"
-	hook_contents_logic=""
-	hook_contents_tail=""
 
-	# Backwards compatibility
-	# Remove this when renew-hook is removed
-	# Falling back to renew-hook if deploy-hook is not passed
-	if [ "${hook}" = "deploy" ] && [ ! -f "${hook_source}" ]; then
-		hook_source="${__object}/parameter/renew-hook"
-	fi
-	if [ "${state}" = "present" ] && \
-		[ -f "${hook_source}" ]; then
-		# This hook is to be installed, let's generate it with some
-		# safety boilerplate
-		# Since certbot runs all hooks for all renewal processes
-		# (at each state for deploy, pre, post), it is up to us to
-		# differentiate whether or not the hook must run
-		hook_state="present"
-		hook_contents_head="$(cat <<EOF
-#!/bin/sh -e
-#
-# Managed remotely with https://cdi.st
-#
-# Domains for which this hook is supposed to apply
-lineage="${LE_DIR}/live/${__object_id}"
-domains="\$(cat <<eof
-${domains}
-eof
-)"
-EOF
-)"
-		case "${hook}" in
-			pre|post)
-				# Certbot is kind of terrible, we have
-				# no way of knowing what domain/lineage the
-				# hook is running for
-				hook_contents_logic="$(cat <<EOF
-# pre/post-hooks apply always due to a certbot limitation
-APPLY_HOOK="YES"
-EOF
-)"
-			;;
-			deploy)
-				hook_contents_logic="$(cat <<EOF
-# certbot defines these:
-# RENEWED_DOMAINS: DOMAIN1,DOMAIN2
-# RENEWED_LINEAGE: /etc/letsencrypt/live/__object_id
-# It feels more stable to use RENEWED_LINEAGE
-if [ "\${lineage}" = "\${RENEWED_LINEAGE}" ]; then
-	APPLY_HOOK="YES"
-fi
-EOF
-)"
-			;;
-			*)
-				echo "Unknown hook '${hook}'" >> /dev/stderr
-				exit 1
-			;;
-		esac
+	# This defines hook_contents
+	# shellcheck source=cdist/conf/type/__letsencrypt_cert/files/gen_hook.sh
+	. "${__type}/files/gen_hook.sh"
 
-		hook_contents_tail="$(cat <<EOF
-if [ -n "\${APPLY_HOOK}" ]; then
-# Messing with indentation can eff up the users' scripts, let's not
-$(cat "${hook_source}")
-fi
-EOF
-)"
-	fi
 	# Ensure hook directory exists
 	require="__directory/${HOOKS_DIR}" __directory "${HOOKS_DIR}/${hook}" \
 		--mode 0755
@@ -219,10 +153,6 @@ EOF
 		--mode 0555 \
 		--source '-' \
 		--state "${hook_state}" <<EOF
-${hook_contents_head}
-
-${hook_contents_logic}
-
-${hook_contents_tail}
+${hook_contents}
 EOF
 done