nickjj / ansible-acme-sh Goto Github PK

I would recommend replacing the call to the apt module with the more modern package module Link.

Ratio:
Right now, only Debian based distributions are supported. This is mainly caused by the use of the apt task installing the required dependencies. Replacing this very first task would open up this role for other distributions as well.

In fact, I tested the role successfully with CentOS Stream by simply deleteing the apt task in main.yml

This role is so much better than the one I'd started to write on my own! One issue though: There's only one variable {{ acme_sh_copy_certs_to_path }} which is used for both the certificate and the key. They are both installed to a folder with 755 permissions. For security reasons, the key file should have a separate {{ acme_sh_copy_keys_to_path }} variable corresponding to a folder with 750 permissions.

Thanks!

The optional Step Upgrade acme.sh which runs acme.sh --upgrade in the background, produces the output

[Thu Jun 13 17:31:04 CEST 2024] Already uptodate!
[Thu Jun 13 17:31:04 CEST 2024] Upgrade success!

when acme.sh already was up to date when triggering the upgrade.

Yet line 54 in tasks/main.yml only queries if "Upgrade success!" was in stdout. If said string was in stdout, it sets changed to true.

My recommendation is, adding a condition that sets changed to false when Already uptodate! is in stdout.

For example this seems to work:

  changed_when:
    - upgrade_result.rc == 0
    - "Upgrade success" in upgrade_result.stdout
    - not("Already uptodate" in upgrade_result.stdout)

Link to the mentioned code: https://github.com/nickjj/ansible-acme-sh/blob/76c835e9f7dd387d1612ff8b4d58c4aaf2d812e1/tasks/main.yml#L54C3-L54C86

Hi,

first, thanks for the great ansible role.
It works nearly perfectly for me, there is just one issue: It does not support ansible's check mode, but breaks at listing the domains:

TASK [nickjj.acme-sh : List acme.sh certificate information] *******************
Sunday 16 August 2020  08:50:32 +0000 (0:00:00.664)       0:00:20.393 ********* 
fatal: [host.mydomain.com]: FAILED! => {"msg": "The task includes an option with an undefined variable. The error was: 'dict object' has no attribute 'stdout_lines'\n\nThe error appears to be in '/root/.ansible/roles/nickjj.acme-sh/tasks/main.yml': line 229, column 3, but may\nbe elsewhere in the file depending on the exact syntax problem.\n\nThe offending line appears to be:\n\n\n- name: List acme.sh certificate information\n  ^ here\n"}

In check mode, the task to generate the domain list is never being run, so the variable list_domains is never set.
So, how about adding another check to the debug output task:

- name: List acme.sh certificate information
  debug:
    msg: "{{ list_domains.stdout_lines }}"
  when: acme_sh_list_domains and not ansible_check_mode and not acme_sh_uninstall

@nickjj I hope you don't mind, I'm making an issue so that if someone wants to take it up.

Hi,

it would be great to dissociate setup tasks with usage ones to be able to execute certificate creations on multiple lays without having to replay the setup.

Hi,
Ansible version : 2.9.9

It looks like there is an issue to delete installed certificates when setting remove option to True in domain definition.

acme_sh_domains:
  - domains: ["sub.domain.tld"]
    remove: True

The following tasks should do the cleanup but says only [OK] and nothing is cleaning the files are still here on the file system. It looks like using a wildcard is not working.

- name: Remove acme.sh installed certificate files
  file:
    path: "{{ acme_sh_copy_certs_to_path }}/{{ item.domains | first }}*"
    state: "absent"
  loop: "{{ acme_sh_domains }}"
  when:
    - acme_sh_domains and item.domains is defined and item.domains
    - item.remove is defined and item.remove
    - not acme_sh_uninstall

I report the issue, may be going to try to fix it later.