Adding support for custom SSL by path

[mamercad]

Fixes #309.

[mamercad]

It doesn't seem like the smoke tests are failing because of my changes; can a maintainer help me out?

[mamercad]

Do we introduce 2 new variables just to do the same as the original logic has, but with different input (via remote file vs via text)?

In #309 you mentioned about managing the certificates in another role.

Could you write the needed certificate contents directly to the /etc/ssl/st2/st2.crt and /etc/ssl/st2/st2.key from that role? This way you won't need to do anything else in the stackstorm role, it'll just pick it up.

Ah, that's interesting, I'll look into this.

[mamercad]

This way you won't need to do anything else in the stackstorm role, it'll just pick it up.

I don't think it'll just work ... based on this, both st2web_ssl_certificate and st2web_ssl_certificate_key need to have the "value" of the certificate and the key (notice the combination of the copy module and the content parameter).

The external role that I'm using is doing LetsEncrypt ... I generate a private key and a CSR, submit it, and then fetch back the certificate and chain (storing all of these files on disk) after the LE stuff happens.

Am I missing something -- thoughts @armab?

[mamercad]

This way you won't need to do anything else in the stackstorm role, it'll just pick it up.

I don't think it'll just work ... based on this, both st2web_ssl_certificate and st2web_ssl_certificate_key need to have the "value" of the certificate and the key (notice the combination of the copy module and the content parameter).

The external role that I'm using is doing LetsEncrypt ... I generate a private key and a CSR, submit it, and then fetch back the certificate and chain (storing all of these files on disk) after the LE stuff happens.

Am I missing something -- thoughts @armab?

Oh, you mean, use my role after the StackStorm roles/plays and just clobber /etc/ssl/st2/st2.crt and /etc/ssl/st2/st2.key (followed by restarting nginx)? I suppose that'd work ... that's not very nice though is it?

[arm4b]

Perhaps cp them on a remote host or similar as a prep task before running the st2 role would work. Or reading them to a var before running the st2 role. I see the logic introduced here does somewhat the same.

If we allow adding new vars for SSL cert file, we may allow the same for other vars https://github.com/StackStorm/ansible-st2#variables:

• st2_config -> st2_config_file
• st2_rbac -> st2_rbac_file
• st2_ldap -> st2_ldap_file
• st2web_nginx_config -> st2web_nginx_config_file
• st2chatops_config -> st2chatops_config_file

which looks like unnecessary complexity. With that, I think this kind of work could be done on the user's side.

Another problem with adding new vars named st2web_ssl_certificate_path and st2web_ssl_certificate_key_path, is that it might be confused with the nginx settings we already have here: https://github.com/StackStorm/st2/blob/abb694b85f75dc543ecea7df6b87def4bce53309/conf/nginx/st2.conf#L35-L36

  ssl_certificate           /etc/ssl/st2/st2.crt;
  ssl_certificate_key       /etc/ssl/st2/st2.key;

Eg. we're not changing the /etc/ssl/st2/st2.crt, but providing a path to read the certificate content on a remote server to later write it to the /etc/ssl/st2/st2.crt, which is a bit non-usual and might just bring more confusion from the user's side when using the role.

After all, it's something that's left for maintainers to support. Any other ideas about the better solutions in terms of UX and role vars?

[mamercad]

Perhaps cp them on a remote host or similar as a prep task before running the st2 role would work. Or reading them to a var before running the st2 role. I see the logic introduced here does somewhat the same.

If we allow adding new vars for SSL cert file, we may allow the same for other vars https://github.com/StackStorm/ansible-st2#variables:

• st2_config -> st2_config_file
• st2_rbac -> st2_rbac_file
• st2_ldap -> st2_ldap_file
• st2web_nginx_config -> st2web_nginx_config_file
• st2chatops_config -> st2chatops_config_file

which looks like unnecessary complexity. With that, I think this kind of work could be done on the user's side.

Another problem with adding new vars named st2web_ssl_certificate_path and st2web_ssl_certificate_key_path, is that it might be confused with the nginx settings we already have here: https://github.com/StackStorm/st2/blob/abb694b85f75dc543ecea7df6b87def4bce53309/conf/nginx/st2.conf#L35-L36

  ssl_certificate           /etc/ssl/st2/st2.crt;
  ssl_certificate_key       /etc/ssl/st2/st2.key;

Eg. we're not changing the /etc/ssl/st2/st2.crt, but providing a path to read the certificate content on a remote server to later write it to the /etc/ssl/st2/st2.crt, which is a bit non-usual and might just bring more confusion from the user's side when using the role.

After all, it's something that's left for maintainers to support. Any other ideas about the better solutions in terms of UX and role vars?

Yes; thanks for helping me through this -- I'm going to close this PR, I agree that adding the "alternate path way" isn't the very nice and likely would end up being more confusing than helpful.

Here's what I whipped up:

---
- name: stackstorm
  hosts: stackstorm

  vars:

    st2web_ssl_certificate_path: /etc/ssl/certs/redacted-fullchain.crt
    st2web_ssl_certificate_key_path: /etc/ssl/private/redacted.pem

  tasks:

    - name: fetch our certificate key
      ansible.builtin.slurp:
        src: "{{ st2web_ssl_certificate_key_path }}"
      register: certificate_key
      become: true
      no_log: true

    - name: fetch our certificate
      ansible.builtin.slurp:
        src: "{{ st2web_ssl_certificate_path }}"
      register: certificate

    - name: store them in facts (named as stackstorm expect them to be named)
      ansible.builtin.set_fact:
        st2web_ssl_certificate_key: "{{ certificate_key.content | b64decode }}"
        st2web_ssl_certificate: "{{ certificate.content | b64decode }}"
      no_log: true

    - name: include stackstorm roles
      ansible.builtin.include_role:
        name: "{{ role }}"
      loop:
        - StackStorm.mongodb
        - StackStorm.rabbitmq
        - StackStorm.redis
        - StackStorm.st2repo
        - StackStorm.st2
        - StackStorm.nginx
        - StackStorm.st2web
        - StackStorm.nodejs
        - StackStorm.st2chatops
        - StackStorm.st2smoketests
      loop_control:
        loop_var: role

[arm4b]

So the logic is getting a cert content into a var and passing it to the role. Nice, thanks for sharing! :+1:

[mamercad]

So the logic is getting a cert content into a var and passing it to the role. Nice, thanks for sharing! 👍

Exactly; thanks again for the thoughtful feedback (this would have been a bad change to merge for sure).