matrix-docker-ansible-deploy icon indicating copy to clipboard operation
matrix-docker-ansible-deploy copied to clipboard

Ensure OpenSSL installed and use matrix_host_command_openssl

Open HarHarLinks opened this issue 2 years ago • 6 comments

I need openssl for #1505 and searched the repo for it and found these:

  1. matrix-nginx-proxy has a task to install it if using self-signed certs
  2. some jitsi script wants to use it https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/30339cd313717bf18471f39ee27db76ec3ba2ddf/inventory/scripts/jitsi-generate-passwords.sh#L6
  3. there is a global variable with the path to it https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/61391647e9681968f65096223507c6645ad03acb/roles/matrix-base/defaults/main.yml#L82
  4. which is used by matrix-bridge-appservice-irc https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/1ab507349c752042d26def3e95884f6df8886b74/roles/matrix-bridge-appservice-irc/tasks/setup_install.yml#L100

What needs to be done:

  1. [x] extract the installation from (1) so that all roles depending on it can install it
  2. [ ] jitsi (2) and others should use the variable (3)
  3. [x] use both of these features for #1505

HarHarLinks avatar Jan 05 '22 20:01 HarHarLinks

Indeed sounds like something we need to make better!

On the one hand, it's used often and could be installed by default (by the matrix-base role). On the other hand, it's only used with a non-default configuration and components that are not enabled by default. Installing yet another hard dependency when it might not be needed doesn't sound ideal. We should strive to be more minimalist.


One solution is to extract the openssl installation into another task file in the matrix-base role (e.g. roles/matrix-base/tasks/util/ensure_openssl_installed.yml), which would get included where necessary.

I suppose it would only handle installation and not uninstallation. If openssl is no longer necessary, we just leave it behind.

A downside of including this task like that is that if multiple roles which need openssl are enabled, each one would include these same tasks. Not the end of the world, but some duplicated work is done.


Another solution is to leave installation of dependencies to the matrix-base role.

We could introduce a variable (e.g. matrix_openssl_installation_enabled), which is false by default (in roles/matrix-base/defaults/main.yml), but then gets potentially set to true via group_vars/matrix_servers, using some big if statement.

matrix_openssl_installation_enabled: |
  {{
    (matrix_nginx_proxy_enabled and matrix_ssl_retrieval_method == 'self-signed')
    or
    (matrix_jitsi_enabled)
    or
    (matrix_appservice_irc_enabled)
    or
    (matrix_appservice_irc_enabled)
    or
    (matrix_hookshot_enabled)
  }}

This is a little ugly though. Another downside is that doing --tags=setup-jitsi might not execute the OpenSSL installation tasks that are part of matrix-base (unless we mark them as tags: always.. but that has its own, worse, downsides).


Based on the above, I think that the "include the installation tasks in every role" solution is probably the best one.


On a related note, we similarly install fuse on all operating systems, regardless of whether it's needed or not. It's only needed when S3 (Goofys) is enabled though. We should probably handle fuse installation the same way we solve this openssl thing.

spantaleev avatar Jan 06 '22 06:01 spantaleev

I've done it for fuse in 4e4fb98. If you'd like to make a similar PR for openssl, please go ahead.

spantaleev avatar Jan 08 '22 12:01 spantaleev

@spantaleev please re-open, matrix_host_command_openssl was not addressed in the linked PR

HarHarLinks avatar Jan 30 '22 17:01 HarHarLinks

How would you like to address matrix_host_command_openssl? Seems like the matrix-base role already defines it in a way that should work regardless of where the actual binary is: https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/6e13ed8b7ee8041ba6ba0032af0abf050df35630/roles/matrix-base/defaults/main.yml#L97

spantaleev avatar Jan 30 '22 17:01 spantaleev

correct, and now the roles should actually use that, e.g. https://github.com/spantaleev/matrix-docker-ansible-deploy/blob/7d96526b531a6ffdda79c081663a81e8ffc234ab/roles/matrix-awx/tasks/update_variables.yml#L13

HarHarLinks avatar Jan 30 '22 18:01 HarHarLinks

I see. I've re-opened this issue until it's all updated.

spantaleev avatar Jan 31 '22 06:01 spantaleev