spantaleev / matrix-docker-ansible-deploy

🐳 Matrix (An open network for secure, decentralized communication) server setup using Ansible and Docker
GNU Affero General Public License v3.0
4.93k stars 1.05k forks source link

Ensure OpenSSL installed and use matrix_host_command_openssl #1510

Open HarHarLinks opened 2 years ago

HarHarLinks commented 2 years ago

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
spantaleev commented 2 years ago

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 commented 2 years ago

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

HarHarLinks commented 2 years ago

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

spantaleev commented 2 years ago

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

HarHarLinks commented 2 years ago

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

spantaleev commented 2 years ago

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