Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Clean up default listener configuration #4586

Merged
merged 5 commits into from
Feb 11, 2019
Merged

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Feb 7, 2019

Rearrange the comments to try to clarify them, and expand on what some of it
means.

Use a sensible default 'bind_addresses' setting.

For the insecure port, only bind to localhost, and enable x_forwarded, since
apparently it's for use behind a load-balancer.

Rearrange the comments to try to clarify them, and expand on what some of it
means.

Use a sensible default 'bind_addresses' setting.

For the insecure port, only bind to localhost, and enable x_forwarded, since
apparently it's for use behind a load-balancer.
@richvdh richvdh requested a review from a team February 7, 2019 19:12
@@ -24,6 +24,14 @@

logger = logging.Logger(__name__)

# by default, we attempt to listen on both '::' *and* '0.0.0.0' because some OSes
# (windows? old linux? those where net.ipv6.bindv6only is set?) will only listen
# on IPv6 when ':' is set.
Copy link
Contributor

Choose a reason for hiding this comment

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

::. This applies to Windows and BSD-variants where net.ipv6.bindv6only is set (ie. operating systems that do not follow RFC3493). See #2232.

Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting. https://github.com/matrix-org/synapse/pull/4586/files#diff-7604431227e17cc581f690570dfacca6L306 appears to say the opposite for Mac OS (by which I assume it means macOS, or however Apple are spelling it this month) ?

- port: %(unsecure_port)s
tls: false
bind_addresses: ['::', '0.0.0.0']
bind_addresses: ['127.0.0.1']
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have ::1 here by default as well? In some Linux distributions localhost is set to ::1 and 127.0.0.1 in /etc/hosts.

@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #4586 into develop will decrease coverage by 16.69%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #4586      +/-   ##
===========================================
- Coverage    75.36%   58.66%   -16.7%     
===========================================
  Files          338      338              
  Lines        34479    34501      +22     
  Branches      5628     5635       +7     
===========================================
- Hits         25984    20241    -5743     
- Misses        6913    12898    +5985     
+ Partials      1582     1362     -220

If we have a bind_address, don't use the default bind address list as well
@richvdh richvdh merged commit 24b7f39 into develop Feb 11, 2019
@richvdh richvdh deleted the rav/clean_up_listeners_config branch February 11, 2019 12:50
richvdh added a commit that referenced this pull request Feb 14, 2019
Synapse 0.99.1 (2019-02-14)
===========================

Features
--------

- Include m.room.encryption on invites by default ([\#3902](#3902))
- Federation OpenID listener resource can now be activated even if federation is disabled ([\#4420](#4420))
- Synapse's ACME support will now correctly reprovision a certificate that approaches its expiry while Synapse is running. ([\#4522](#4522))
- Add ability to update backup versions ([\#4580](#4580))
- Allow the "unavailable" presence status for /sync.
  This change makes Synapse compliant with r0.4.0 of the Client-Server specification. ([\#4592](#4592))
- There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners ([\#4613](#4613), [\#4615](#4615), [\#4617](#4617), [\#4636](#4636))
- The default configuration no longer requires TLS certificates. ([\#4614](#4614))

Bugfixes
--------

- Copy over room federation ability on room upgrade. ([\#4530](#4530))
- Fix noisy "twisted.internet.task.TaskStopped" errors in logs ([\#4546](#4546))
- Synapse is now tolerant of the `tls_fingerprints` option being None or not specified. ([\#4589](#4589))
- Fix 'no unique or exclusion constraint' error ([\#4591](#4591))
- Transfer Server ACLs on room upgrade. ([\#4608](#4608))
- Fix failure to start when not TLS certificate was given even if TLS was disabled. ([\#4618](#4618))
- Fix self-signed cert notice from generate-config. ([\#4625](#4625))
- Fix performance of `user_ips` table deduplication background update ([\#4626](#4626), [\#4627](#4627))

Internal Changes
----------------

- Change the user directory state query to use a filtered call to the db instead of a generic one. ([\#4462](#4462))
- Reject federation transactions if they include more than 50 PDUs or 100 EDUs. ([\#4513](#4513))
- Reduce duplication of ``synapse.app`` code. ([\#4567](#4567))
- Fix docker upload job to push -py2 images. ([\#4576](#4576))
- Add port configuration information to ACME instructions. ([\#4578](#4578))
- Update MSC1711 FAQ to calrify .well-known usage ([\#4584](#4584))
- Clean up default listener configuration ([\#4586](#4586))
- Clarifications for reverse proxy docs ([\#4607](#4607))
- Move ClientTLSOptionsFactory init out of `refresh_certificates` ([\#4611](#4611))
- Fail cleanly if listener config lacks a 'port' ([\#4616](#4616))
- Remove redundant entries from docker config ([\#4619](#4619))
- README updates ([\#4621](#4621))
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants