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

Draupnir proxy #3313

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Chasethechicken
Copy link

Mjolnir / Draupnir can receive abuse reports in the management room.

There are two options to configure this:

  1. Poll reports from the admin-API
  2. Redirect the endpoint to the draupnir container using a reverse proxy

The second option seems to be the preferred one.

This PR configures the MDAD-managed traefik to proxy the relevant requests to Draupnir (the second option). To enable it set matrix_bot_draupnir_abuse_reporting_enabled: true.

If this gets merged I'd be willing to create a PR adding the same options for Mjolnir, which should more or less be a big find and replace.

enabled: true

# The port to expose the webserver on. Defaults to 8080.
port: 80
Copy link
Owner

Choose a reason for hiding this comment

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

If you use port 8080, which seems to have been the default recommendation, then you won't need --cap-add=NET_BIND_SERVICE.

You'd also need to adjust the traefik.http.services.matrix-bot-draupnir.loadbalancer.server.port=80 line in labels.j2 though.

@@ -261,4 +258,4 @@ pollReports: false

# Whether or not new reports, received either by webapi or polling,
# should be printed to our managementRoom.
displayReports: false
displayReports: {{ matrix_bot_draupnir_abuse_reporting_enabled }}
Copy link
Owner

Choose a reason for hiding this comment

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

It's probably better to add a new variable for this. Its default value can be defined in terms of matrix_bot_draupnir_abuse_reporting_enabled.

Based the setting key, the variable should be named matrix_bot_draupnir_display_reports.

Perhaps abuseReporting.enabled should also receive its own variable (or use matrix_bot_draupnir_abuse_reporting_enabled).

Then, web.enabled can be its own variable too, with a default defined in terms of matrix_bot_draupnir_abuse_reporting_enabled:

matrix_bot_draupnir_web_enabled: "{{ matrix_bot_draupnir_abuse_reporting_enabled }}"

matrix_bot_draupnir_abuse_reporting_enabled should probably be renamed to matrix_bot_draupnir_web_abuse_reporting_enabled.

All these variables names are meant to indicate where the variable is being used. We usually try to make variables take their names from the setting path that they control.


All this would let people:

  • enable the web-interface manually (for other purposes), without enabling abuse-reporting
  • control displayReports separately (being able to turn it off, even if abuse reporting is enabled)

Comment on lines +2693 to 2705
matrix_bot_draupnir_container_additional_networks_auto_homeserver: |-
{{
[] if matrix_addons_homeserver_container_network == '' else [matrix_addons_homeserver_container_network]
}}
matrix_bot_draupnir_container_additional_networks_auto_reverse_proxy: |-
{{
[matrix_playbook_reverse_proxyable_services_additional_network] if matrix_playbook_reverse_proxyable_services_additional_network else []
}}

matrix_bot_draupnir_container_additional_networks_auto: |-
{{
([] if matrix_addons_homeserver_container_network == '' else [matrix_addons_homeserver_container_network])
matrix_bot_draupnir_container_additional_networks_auto_homeserver + matrix_bot_draupnir_container_additional_networks_auto_reverse_proxy
}}
Copy link
Owner

Choose a reason for hiding this comment

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

It's more consistent with the rest of the playbook if matrix_bot_draupnir_container_additional_networks_auto contained both network additions, instead of splitting them over into matrix_bot_draupnir_container_additional_networks_auto_homeserver and matrix_bot_draupnir_container_additional_networks_auto_reverse_proxy variables.

There are many examples of this in group_vars/matrix_servers, but here are a few:

  • matrix_appservice_discord_container_additional_networks_auto: |-
    {{
    (
    ([] if matrix_addons_homeserver_container_network == '' else [matrix_addons_homeserver_container_network])
    +
    ([devture_postgres_container_network] if (devture_postgres_enabled and matrix_appservice_discord_database_hostname == devture_postgres_connection_hostname and matrix_appservice_discord_container_network != devture_postgres_container_network) else [])
    ) | unique
    }}

  • matrix_mautrix_facebook_container_additional_networks_auto: |-
    {{
    (
    ([] if matrix_addons_homeserver_container_network == '' else [matrix_addons_homeserver_container_network])
    +
    ([devture_postgres_container_network] if (devture_postgres_enabled and matrix_mautrix_facebook_database_hostname == devture_postgres_connection_hostname and matrix_mautrix_facebook_container_network != devture_postgres_container_network) else [])
    +
    ([matrix_playbook_reverse_proxyable_services_additional_network] if (matrix_playbook_reverse_proxyable_services_additional_network and matrix_mautrix_facebook_container_labels_traefik_enabled) else [])
    ) | unique
    }}

Using | unique is a good idea to avoid network duplication.

@spantaleev
Copy link
Owner

Thank you for working on this feature!

I've posted some feedback to improve consistency and simplify some things.

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