New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hookshot: Add support for encryption #3042
Hookshot: Add support for encryption #3042
Conversation
… support into seperate statements
Thank you for picking this up! |
roles/custom/matrix-bridge-hookshot/templates/systemd/matrix-hookshot.service.j2
Outdated
Show resolved
Hide resolved
roles/custom/matrix-bridge-hookshot/tasks/reset_encryptioon.yml
Outdated
Show resolved
Hide resolved
Thank you for picking this up! I've posted a detailed review which should help make it ready for merge |
fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com>
…er-case first letter for consistency
change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com>
…real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption
Thanks, @spantaleev! I think I've applied everything, let me know if there is still anything amiss. :) |
If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled.
This seems to have been removed by accident.
Squashed based on the work done in #3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
… to matrix_hookshot_ident) Related to #3042
It seems like connectivity is problematic, even though the networks appear to be configured correctly: > [ioredis] Unhandled error event: Error: connect ECONNREFUSED 172.22.0.2:6739 > at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) For now, I disable pointing the queue host to Redis to avoid it. It should be investigated. People who enable Hookshot's new experimental encryption may encounter this also. Related to #3042
There were a lot of things missing and various things to fix - YAML syntax problems, etc. It makes me wonder if the end result was tested at all or done blindly? The Hookshot container was not being connected to any network.. The syntax for connecting it to additional networks was broken, etc. The reset-encryption command had a broken YAML syntax.
Network connectivity seems problematic for some reasons abnd Hookshot cannot connect to Redis (at least one my machine), hence me disabling it (dbf1a68) for people who haven't explicitly enabled encryption. This PR couldn't have possibly worked without all these fixups.. And even now, it still likely has some issues (network connectivity between Hookshot and Redis). |
As you can see, this is still a draft. I am working on implementing your feedback. My next step would be to get a VPS and DNS set up so I can test this without doing that on my production setup. Once I am satisfied I will remove the "draft" marking and proceed. |
Squashed based on the work done in spantaleev#3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (spantaleev#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
… to matrix_hookshot_ident) Related to spantaleev#3042
It seems like connectivity is problematic, even though the networks appear to be configured correctly: > [ioredis] Unhandled error event: Error: connect ECONNREFUSED 172.22.0.2:6739 > at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) For now, I disable pointing the queue host to Redis to avoid it. It should be investigated. People who enable Hookshot's new experimental encryption may encounter this also. Related to spantaleev#3042
Squashed based on the work done in spantaleev#3042 commit 49932b8 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:21:31 2023 +0200 Fix syntax in matrix-bridge-hookshot/tasks/reset_encryption.yml Also, this task always does work and side-effects, so it should always report changes (`changed_when: true`). commit 6bdf7a9 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:12:41 2023 +0200 Add Hookshot validation task to ensure queue settings are set when encryption is enabled commit 8c531b7 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:10:17 2023 +0200 Add missing variables rewiring in group_vars/matrix_servers for Hookshot commit 7d26dab Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:08:19 2023 +0200 Add defaults for matrix_hookshot_queue_host and matrix_hookshot_queue_port commit 74f9113 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:06:17 2023 +0200 Fix syntax for connecting to additional networks for Hookshot commit ca7b41f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:05:28 2023 +0200 Fix indentation and remove unnecessary if-statements commit ac4a918 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:04:44 2023 +0200 Add missing --network for Hookshot This seems to have been removed by accident. commit 6a81fa2 Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:02:47 2023 +0200 Make automatic Redis enabling safer, when Hookshot encryption enabled If we ever default encryption to enabled for Hookshot, we only wish to force-enable Redis if Hookshot is actually enabled. commit 75a8e0f Author: Slavi Pantaleev <slavi@devture.com> Date: Sat Dec 16 09:01:10 2023 +0200 Fix typo commit 98ad182 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:37:40 2023 +0100 Add defaults for Hookshot's encryption commit 29fa9fa Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:35:11 2023 +0100 Improve wording of Hookshot's encryption section commit 4f835e0 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:28:52 2023 +0100 use safer mount options for the container's files commit 8c93327 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:26:01 2023 +0100 fix filename commit 03a7bb6 Merge: e55d769 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:23:44 2023 +0100 Merge branch 'HarHarLinks/hookshot-encryption' of https://github.com/real-joshua/matrix-docker-ansible-deploy into HarHarLinks/hookshot-encryption commit 0604776 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:15:54 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 change the if statement to not require a variable with a length > 0 and add a filter to json for the redis host Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit e55d769 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:13:50 2023 +0100 clarify that Redis is required, standardadise on Hookshot with an upper-case first letter for consistency commit 66706e4 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 22:08:20 2023 +0100 Update roles/custom/matrix-bridge-hookshot/templates/config.yml.j2 fix for a typo Co-authored-by: Slavi Pantaleev <slavi@devture.com> commit f6aaeb9 Merge: e5d3400 869dd33 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:22:34 2023 +0100 Merge branch 'master' into HarHarLinks/hookshot-encryption commit e5d3400 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Fri Dec 15 00:09:27 2023 +0100 Add Jinja loop to allow adding multiple networks commit 69f9477 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:52:41 2023 +0100 split if statements for the message queue and experimental encryption support into seperate statements commit 4c13be1 Author: Joshua Hoffmann <joshua.hoffmann@b1-systems.de> Date: Thu Dec 14 23:31:19 2023 +0100 change variable name per spantaleev's suggestion (spantaleev#2979 (comment)) commit 9905309 Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:14:04 2023 +0100 amend docs commit 94abf2d Author: HarHarLinks <kim.brose@rwth-aachen.de> Date: Wed Nov 1 16:05:22 2023 +0100 draft encryption support for hookshot
… to matrix_hookshot_ident) Related to spantaleev#3042
It seems like connectivity is problematic, even though the networks appear to be configured correctly: > [ioredis] Unhandled error event: Error: connect ECONNREFUSED 172.22.0.2:6739 > at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1595:16) For now, I disable pointing the queue host to Redis to avoid it. It should be investigated. People who enable Hookshot's new experimental encryption may encounter this also. Related to spantaleev#3042
This is a completion/improvement of #2979 by @HarHarLinks.