Skip to content

Conversation

@invario
Copy link

@invario invario commented Jul 7, 2025

Deploy-hook to very simply copy files to set directories and then execute whatever reloadcmd the admin needs afterwards. This can be useful for configurations where the "multideploy" hook (in development) is used or when an admin wants ACME.SH to renew certs but needs to manually configure deployment via an external script (e.g. The deploy-freenas script for TrueNAS Core/Scale

https://github.com/danb35/deploy-freenas/

Note: replaces my earlier PR #6379 which I closed.

Deploy-hook to very simply copy files to set directories and then execute whatever reloadcmd
the admin needs afterwards. This can be useful for configurations where the "multideploy"
hook (in development) is used or when an admin wants ACME.SH to renew certs but needs to
manually configure deployment via an external script (e.g. The deploy-freenas script for TrueNAS Core/Scale

https://github.com/danb35/deploy-freenas/

Signed-off-by: invario <67800603+invario@users.noreply.github.com>
@rbicker
Copy link

rbicker commented Jul 14, 2025

Hi,

We have a use case where we need to deploy a combined PEM file (containing both the full certificate chain and the private key) to multiple web services. While acme.sh itself supports specifying the same path for both --key-file and --fullchain-file (or --cert-file), the current localcopy deploy hook does not account for this scenario—it performs separate copy operations, resulting in one file overwriting the other.

Would it be possible to enhance the hook by adding logic similar to the snippet below (around line 41), to detect when the cert and key targets are the same and generate a proper combined PEM?

  _combined_target=""
  _combined_srccert=""

  if [ "$DEPLOY_LOCALCOPY_CERTKEY" ] && \
     { [ "$DEPLOY_LOCALCOPY_CERTKEY" = "$DEPLOY_LOCALCOPY_FULLCHAIN" ] || \
       [ "$DEPLOY_LOCALCOPY_CERTKEY" = "$DEPLOY_LOCALCOPY_CERTIFICATE" ]; }; then

    _combined_target="$DEPLOY_LOCALCOPY_CERTKEY"

    if [ "$DEPLOY_LOCALCOPY_CERTKEY" = "$DEPLOY_LOCALCOPY_FULLCHAIN" ]; then
      _combined_srccert="$_cfullchain"
      DEPLOY_LOCALCOPY_FULLCHAIN=""
    else
      _combined_srccert="$_ccert"
      DEPLOY_LOCALCOPY_CERTIFICATE=""
    fi

    _info "Creating combined PEM at $_combined_target"
    _tmpfile="$(mktemp)"
    if ! cat "$_combined_srccert" "$_ckey" > "$_tmpfile"; then
      _err "Failed to build combined PEM file"
      return 1
    fi
    if ! mv "$_tmpfile" "$_combined_target"; then
      _err "Failed to move combined PEM into place"
      return 1
    fi

    DEPLOY_LOCALCOPY_CERTKEY=""
    _savedeployconf DEPLOY_LOCALCOPY_CERTKEY "$_combined_target"
  fi

This change would make the deploy hook more robust in real-world deployment scenarios (e.g. HAProxy), without affecting current behavior for users who specify separate files.

Thanks!

@invario
Copy link
Author

invario commented Jul 14, 2025

    if ! cat "$_combined_srccert" "$_ckey" > "$_tmpfile"; then
      _err "Failed to build combined PEM file"
      return 1
    fi
    if ! mv "$_tmpfile" "$_combined_target"; then
      _err "Failed to move combined PEM into place"
      return 1
    fi

Thanks for the suggestion! Is there any reason a temp file has to be used as opposed to just concatenating directly to the target?

@rbicker
Copy link

rbicker commented Jul 28, 2025

Hi
Please excuse my late response, unfortunately I have missed your reply.
I opted for using a temporary file mainly as a safety measure, to avoid partially written or corrupted files in case something goes wrong during the concatenation (e.g. disk full, permissions, interruption, etc.).

Signed-off-by: invario <67800603+invario@users.noreply.github.com>
@invario
Copy link
Author

invario commented Jul 28, 2025

Hi Please excuse my late response, unfortunately I have missed your reply. I opted for using a temporary file mainly as a safety measure, to avoid partially written or corrupted files in case something goes wrong during the concatenation (e.g. disk full, permissions, interruption, etc.).

I used your code and made some changes and incorporated it. Let me know if it works for you, thanks!

@rbicker
Copy link

rbicker commented Oct 14, 2025

Thanks! I’ve had a chance to test the hook a few times over the last couple of weeks, and it works really well.

One thing I noticed that could be optimized: currently, the hook does not preserve permissions for existing files. It would be great if the temporary file used for creating the combined PEM could inherit the target file’s attributes. For example:

if ! cp --attributes-only "$_combined_target" "$_tmpfile"; then
    _err "Failed to copy file attributes"
    return 1
fi

Perhaps there’s a better approach, but this would ensure ownership and permissions remain consistent.

Also, could you clarify how pull requests like this get released? At the moment, I’m manually copying the hooks I need (this one + multideploy) from GitHub, which obviously isn’t ideal for updates.

Thanks again for your work on this!

@invario
Copy link
Author

invario commented Oct 18, 2025

Thanks! I’ve had a chance to test the hook a few times over the last couple of weeks, and it works really well.

One thing I noticed that could be optimized: currently, the hook does not preserve permissions for existing files. It would be great if the temporary file used for creating the combined PEM could inherit the target file’s attributes. For example:

if ! cp --attributes-only "$_combined_target" "$_tmpfile"; then
    _err "Failed to copy file attributes"
    return 1
fi

Perhaps there’s a better approach, but this would ensure ownership and permissions remain consistent.

Also, could you clarify how pull requests like this get released? At the moment, I’m manually copying the hooks I need (this one + multideploy) from GitHub, which obviously isn’t ideal for updates.

Thanks again for your work on this!

You're very welcome.

The permissions issue I'll work on, but your code would only work if there were an existing target file already. Might I suggest putting a chown/chmod in the reloadcmd as necessary?

I only wrote this snippet of code and created the PR and am awaiting approval. I'm not the acm.sh repo maintainer; that would be @Neilpang, so the only way is for the maintainer to review and approve the PR.

@rbicker
Copy link

rbicker commented Oct 21, 2025

Thanks for the quick reply!
Good point about the existing file. It should be easy to handle by checking if the target file already exists before copying attributes, for example:

if [ -e "$_combined_target" ]; then
  cp --attributes-only "$_combined_target" "$_tmpfile" || _err "Failed to copy file attributes"
fi

That way, ownership and permissions are preserved when a file already exists (e.g., during renewal), without needing to rely on reloadcmd. Also, other deploy hooks like nginx or apache already handle permissions internally, so keeping that behavior consistent here would avoid confusion for users who expect similar handling.

And got it about the PR process — I’ll keep an eye out for @Neilpang's review. Thanks again for your work on this hook; it’s been really helpful in my setup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants