Skip to content
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

Fixed Traefik routing when using Synapse workers #3102

Conversation

Michael-Hollister
Copy link
Contributor

Traefik seems to have routing priority conflicts when using the reverse proxy companion (and likely MMR as well, though I did not have an issue on my server). With the default priority rules being used, I have seen requests being routed directly to the Synapse container instead of the reverse proxy companion container.

I went with disabling registration of Synapse routes instead of adjusting priority values if the reverse proxy companion is enabled. Tested the changes for correct behavior of the reverse proxy companion and MMR routing, but I don't use any of the bridges/bots, so those roles have not been tested.

@@ -82,7 +82,7 @@ traefik.http.routers.matrix-synapse-public-client-api.tls.certResolver={{ matrix



{% if matrix_synapse_container_labels_internal_client_api_enabled %}
{% if matrix_synapse_container_labels_internal_client_api_enabled and not matrix_synapse_reverse_proxy_companion_enabled %}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The check was added here instead of roles/custom/matrix-synapse/defaults/main.yml since matrix_synapse_container_labels_internal_client_api_enabled is configured in group_vars/matrix_servers depending on the proxy setup being used.

@cvwright cvwright mentioned this pull request Jan 16, 2024
@spantaleev
Copy link
Owner

There's no need for disabling labels for matrix-synapse individually based on whether matrix-synapse-reverse-proxy-companion is enabled or not.

The companion is only enabled (at least automatically) when workers are enabled:

matrix_synapse_reverse_proxy_companion_enabled: "{{ matrix_synapse_enabled and matrix_synapse_workers_enabled }}"

When workers are enabled, matrix-synapse knows it cannot route to them appropriately so it automatically disables all Matrix-related routers (/_matrix and also /_synapse/*), courtesy of matrix_synapse_container_labels_matrix_related_labels_enabled`:

# Controls whether Matrix-related labels will be added.
#
# When set to false, variables like the following take no effect:
# - `matrix_synapse_container_labels_public_client_api_enabled`
# - `matrix_synapse_container_labels_public_client_synapse_client_api_enabled`
# - `matrix_synapse_container_labels_public_client_synapse_oidc_api_enabled`
# - `matrix_synapse_container_labels_public_client_synapse_admin_api_enabled`
# - `matrix_synapse_container_labels_public_federation_api_enabled`
#
# When workers are enabled, we do not capture these requests, because we can't route them appropriately.
matrix_synapse_container_labels_matrix_related_labels_enabled: "{{ not matrix_synapse_workers_enabled }}"

Some of these routes (e.g. /_synapse/client) may or may not be load-balanced to workers, so Synapse could potentially keep them enabled, but.. we disable them all just in case.

I can see a situation where someone keeps workers disabled, but force-enables the companion explicitly (for whatever reason). In such a case, both components would end up registering the same labels. It shouldn't hurt though, because whichever services ends up receiving these routes, can handle them correctly. This is because:

  • a non-workers Synapse can handle routing on its own (everything goes to matrix-synapse:8008)
  • matrix-synapse-reverse-proxy-companion, when enabled, in combination with a non-workers Synapse sends everything to matrix-synapse:8008 anyway. It's one extra (unnecessary) hop, but it should work.

So.. the current configuraton should be fine. However, I had forgotten to make use of matrix_synapse_container_labels_matrix_related_labels_enabled in the labels.j2 template for matrix-synapse, so it wasn't fine. This has been remedied in 94378a7. Thank you for catching the problem!

KarolosLykos pushed a commit to KarolosLykos/matrix-docker-ansible-deploy that referenced this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants