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

Neilj/improve federation docs #5419

Merged
merged 10 commits into from
Jun 11, 2019

Conversation

neilisfragile
Copy link
Contributor

No description provided.

@neilisfragile neilisfragile marked this pull request as ready for review June 10, 2019 16:46
@neilisfragile neilisfragile requested a review from a team June 10, 2019 16:46
Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

Otherwise, some inconsistencies between using `'s and nothing for ".well-known".

I'm also concerned about the fact that we're now lacking a doc that says "if you do delegation using X and your server name is Y then you need a cert for Z" since we're obsoleting the MSC1711 FAQ, which could be confusing for new admins.

docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Outdated Show resolved Hide resolved
docs/federate.md Outdated

For instance, if you registered `example.com` and pointed its DNS A record at a
fresh Upcloud VPS or similar, you could install Synapse on that host,
giving it a server_name of `example.com`, and it would automatically generate a
Copy link
Contributor

Choose a reason for hiding this comment

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

it would automatically generate

IIRC you need to explicitly enable the ACME support and I read that as implying it was enabled by default.

docs/federate.md Outdated
Practically speaking, this is no longer necessary.

If you are using a reverse proxy for all of your TLS traffic, then you can set
`no_tls: True`. In that case, the only reason Synapse needs the certificate is
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe explicitly mention that no_tls happens in Synapse's config?

docs/federate.md Outdated Show resolved Hide resolved
Co-Authored-By: Brendan Abolivier <[email protected]>
@codecov
Copy link

codecov bot commented Jun 11, 2019

Codecov Report

Merging #5419 into release-v1.0.0 will decrease coverage by 0.03%.
The diff coverage is n/a.

@@                Coverage Diff                 @@
##           release-v1.0.0    #5419      +/-   ##
==================================================
- Coverage           62.56%   62.53%   -0.04%     
==================================================
  Files                 326      326              
  Lines               35655    35649       -6     
  Branches             5850     5848       -2     
==================================================
- Hits                22308    22293      -15     
- Misses              11794    11804      +10     
+ Partials             1553     1552       -1

Co-Authored-By: Brendan Abolivier <[email protected]>
@codecov
Copy link

codecov bot commented Jun 11, 2019

Codecov Report

Merging #5419 into release-v1.0.0 will decrease coverage by <.01%.
The diff coverage is n/a.

@@                Coverage Diff                 @@
##           release-v1.0.0    #5419      +/-   ##
==================================================
- Coverage           62.56%   62.56%   -0.01%     
==================================================
  Files                 326      326              
  Lines               35655    35649       -6     
  Branches             5850     5848       -2     
==================================================
- Hits                22308    22304       -4     
+ Misses              11794    11793       -1     
+ Partials             1553     1552       -1

neilisfragile and others added 2 commits June 11, 2019 11:12
@neilisfragile neilisfragile requested a review from babolivier June 11, 2019 10:36
@babolivier
Copy link
Contributor

I'm also concerned about the fact that we're now lacking a doc that says "if you do delegation using X and your server name is Y then you need a cert for Z" since we're obsoleting the MSC1711 FAQ, which could be confusing for new admins.

ftr this was already in the doc, and I've missed it by looking only at the diff. mb, forget that part.

Copy link
Contributor

@babolivier babolivier left a comment

Choose a reason for hiding this comment

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

lgtm otherwise


Yes, you are welcome to manage your certificates yourself. Synapse will only
attempt to obtain certificates from Let's Encrypt if you configure it to do
so.The only requirement is that there is a valid TLS cert present for
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
so.The only requirement is that there is a valid TLS cert present for
so. The only requirement is that there is a valid TLS certificate present for

@neilisfragile neilisfragile merged commit 4262183 into release-v1.0.0 Jun 11, 2019
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.

2 participants