Neilj/improved delegation doc 2 #4832

neilisfragile · 2019-03-07T23:08:18Z

Based from #4781

neilisfragile · 2019-03-07T23:10:08Z

Especially interested in technical review on https://github.com/matrix-org/synapse/blob/neilj/improved-delegation-doc-2/docs/federate.md#dns-srv-delegation

codecov · 2019-03-07T23:13:37Z

Codecov Report

Merging #4832 into master will decrease coverage by 0.51%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4832      +/-   ##
==========================================
- Coverage   75.07%   74.56%   -0.52%     
==========================================
  Files         340      340              
  Lines       34882    34882              
  Branches     5714     5714              
==========================================
- Hits        26188    26009     -179     
- Misses       7082     7263     +181     
+ Partials     1612     1610       -2

babolivier

Mostly me being picky, lgtm otherwise

README.rst

babolivier · 2019-03-08T10:32:20Z

docs/federate.md

+
+For a more flexible configuration, you can have ``server_name``
+resources (ie: ``@user:example.com``) served by a different host and
+port (ie: ``synapse.example.com:443``). There are 2 ways to do this:


Maybe mention somewhere around here that both ways are described more in-depth in dedicated sections below? This section ending with "If all goes well, you should be able to connect to your server with a client and then join a room via federation" would make me think that I should have delegation working by reaching this point of the documentation.

docs/federate.md

richvdh

should we mention somewhere that .well-known doesn't work for synapse 0.34 / current sydent / current scalar / current dendrite so you might want to use a SRV record on the origin domain as well as a .well-known?

INSTALL.md

richvdh · 2019-03-08T14:02:49Z

README.rst

@@ -211,100 +162,19 @@ versions of synapse.

 .. _UPGRADE.rst: UPGRADE.rst

-.. _federation:


I've a suspicion this anchor will be used somewhere. It would probably be a good idea to keep it (maybe at the "installation" section).

I can't see where you've moved this to?

oversight, added back in.

docs/federate.md

richvdh

sorry, pressed go too soon before.

I have lots of comments here, but I am conscious that we must not let "perfect" become the enemy of "good" and it might be better to get something out sooner rather than later.

(On a related note: it might be best to PR this against master so that we don't have to wait for a release. That probably means doing a git rebase.)

docs/federate.md

richvdh · 2019-03-08T15:15:07Z

docs/federate.md

+	    "m.server": "synapse.example.com:443"
+    }
+
+This delegation method allows a full delegation contrary to the DNS SRV


I think this paragraph needs a rethink, but I'm not quite sure what it should look like.

Can you explain your concerns?

Addressed our of band and changes committed - wanted to ensure that the methods were considered 'different' rather than one being preferable to another.

docs/federate.md

README.rst

Signed-off-by: Valentin Lab <[email protected]>

Co-Authored-By: neilisfragile <[email protected]>

fixed

richvdh · 2019-03-11T17:15:46Z

INSTALL.md

@@ -348,6 +349,26 @@ https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/misc/matrix-

 Once you have installed synapse as above, you will need to configure it.

+
+## Setting your server_name


The instructions above talk about generating the config and setting the server_name, so this is too late.

basically it feels like this section isn't adding anything here. One idea would be to have a section which explains what the server_name does, but instructions here on how you change this one configuration option don't seem to be very helpful.

Agreed, it shouldn't be here - have deleted since it doesn't add much over the config at the beginning of the file

richvdh · 2019-03-11T17:15:52Z

INSTALL.md

+You can configure your homeserver to use ``<yourdomain.com>`` as the domain in
+its user-ids, by setting ``server_name`` on the command line:
+
+    python -m synapse.app.homeserver \


this is only valid when installing from source - the packaged installations generally have different ways of doing things.

Agreed, deleted

richvdh · 2019-03-11T17:18:02Z

INSTALL.md

+        --server-name <yourdomain.com> \
+        --config-path homeserver.yaml \
+        --generate-config
+    python -m synapse.app.homeserver --config-path homeserver.yaml


you almost certainly don't want to do this here.

richvdh · 2019-03-11T17:20:41Z

README.rst

@@ -211,100 +162,19 @@ versions of synapse.

 .. _UPGRADE.rst: UPGRADE.rst

-.. _federation:


I can't see where you've moved this to?

docs/federate.md

richvdh · 2019-03-11T17:47:20Z

docs/federate.md

+Note that you'll have to modify this URL to replace ``DOMAIN`` with your
+``server_name``. Hitting the URL directly provides extra detail.
+
+For more details the see the [Server to Server Spec](https://matrix.org/docs/spec/server_server/r0.1.1.html#resolving-server-names).


this is a bit of a funny place for this.

docs/federate.md

Co-Authored-By: neilisfragile <[email protected]>

richvdh

sorry just a few nits left

README.rst

docs/federate.md

Co-Authored-By: neilisfragile <[email protected]>

richvdh

lgtm otherwise!

richvdh · 2019-03-12T14:07:07Z

docs/federate.md

-following methods allow you retain a ```server_name``` of ```example.com`` 
-while providing a different server and port for ``*:example.com`` resources 
+following methods allow you retain a `server_name` of `example.com`
+so that your user IDs, room aliases, etc continue to look like `*:example.com`, whilst having your federation traffic routed to a different server. 
 such user ids and room aliases.


orphaned sentence

richvdh · 2019-03-12T14:07:10Z

docs/federate.md

-following methods allow you retain a ```server_name``` of ```example.com`` 
-while providing a different server and port for ``*:example.com`` resources 
+following methods allow you retain a `server_name` of `example.com`
+so that your user IDs, room aliases, etc continue to look like `*:example.com`, whilst having your federation traffic routed to a different server. 


please wrap

richvdh · 2019-03-12T14:07:42Z

docs/federate.md

@@ -77,7 +77,7 @@ To use this delegation method, you need to have write access to your
 ``example.com`` DNS zone).

 This method requires the target server to provide a
-valid SSL certificate identifying it as the original ``server_name``
+valid TLS certificate for the original ``server_name``.
 domain zone. 


Improved federation configuration docs. Specifically detailing .well-known and SRV based delegation methods. Inspiration Valentin Lab <[email protected]> for #4781

…s_use_fullchain * 'develop' of github.com:matrix-org/synapse: fix orphaned sentence Neilj/improved delegation doc 2 (#4832) Add zwsp in bug report template (#4811)

neilisfragile changed the base branch from master to develop March 7, 2019 23:08

neilisfragile requested a review from a team March 7, 2019 23:08

neilisfragile mentioned this pull request Mar 7, 2019

Improved delegation doc (adding .well-known info) #4781

Closed

2 tasks

babolivier suggested changes Mar 8, 2019

View reviewed changes

richvdh mentioned this pull request Mar 8, 2019

Neilj/readme wellknown #4766

Closed

richvdh reviewed Mar 8, 2019

View reviewed changes

richvdh previously requested changes Mar 8, 2019

View reviewed changes

neilisfragile force-pushed the neilj/improved-delegation-doc-2 branch from a6506e9 to 3558ea1 Compare March 11, 2019 16:08

neilisfragile changed the base branch from develop to master March 11, 2019 16:10

vaab and others added 13 commits March 11, 2019 16:15

improved delegation documentation

974602a

Signed-off-by: Valentin Lab <[email protected]>

wip

5913669

links to federate and SQL guidance

aeda2d9

language nits and linking

2492c1f

improve sql wording

0f22464

move trouble shooting to the bottom of the README

1711246

style

062d303

convert to md

19bf247

towncrier

95a9dc6

town crier needs sentences to finish with a '.'

d040ca1

Nits

8497958

Co-Authored-By: neilisfragile <[email protected]>

review comments

0f2af37

be clearer on when to use .well-known

400fc11

neilisfragile force-pushed the neilj/improved-delegation-doc-2 branch from 3558ea1 to 400fc11 Compare March 11, 2019 16:15

richvdh self-requested a review March 11, 2019 17:01

richvdh suggested changes Mar 11, 2019

View reviewed changes

richvdh and others added 2 commits March 12, 2019 09:27

Apply suggestions from code review - style

ac94d9e

Co-Authored-By: neilisfragile <[email protected]>

respond to review comments

94615c5

richvdh self-requested a review March 12, 2019 10:24

richvdh suggested changes Mar 12, 2019

View reviewed changes

richvdh and others added 2 commits March 12, 2019 13:51

Apply suggestions from code review

7dc5d82

Co-Authored-By: neilisfragile <[email protected]>

fix anchor

e5b5ab7

richvdh approved these changes Mar 12, 2019

View reviewed changes

review comments

c622e47

neilisfragile merged commit 8b692bf into master Mar 12, 2019

anoadragon453 pushed a commit that referenced this pull request Mar 13, 2019

Neilj/improved delegation doc 2 (#4832)

430135a

Improved federation configuration docs. Specifically detailing .well-known and SRV based delegation methods. Inspiration Valentin Lab <[email protected]> for #4781

neilisfragile deleted the neilj/improved-delegation-doc-2 branch March 30, 2019 20:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Neilj/improved delegation doc 2 #4832

Neilj/improved delegation doc 2 #4832

neilisfragile commented Mar 7, 2019

neilisfragile commented Mar 7, 2019

codecov bot commented Mar 7, 2019 •

edited

Loading

babolivier left a comment •

edited

Loading

babolivier Mar 8, 2019

neilisfragile Mar 8, 2019

richvdh left a comment

richvdh Mar 8, 2019

neilisfragile Mar 8, 2019

richvdh Mar 11, 2019

neilisfragile Mar 12, 2019

richvdh left a comment

richvdh Mar 8, 2019

neilisfragile Mar 8, 2019

neilisfragile Mar 11, 2019

richvdh Mar 11, 2019

richvdh Mar 11, 2019

neilisfragile Mar 12, 2019

richvdh Mar 11, 2019

neilisfragile Mar 12, 2019

richvdh Mar 11, 2019

neilisfragile Mar 12, 2019

richvdh Mar 11, 2019

richvdh Mar 11, 2019

neilisfragile Mar 12, 2019

richvdh left a comment

richvdh left a comment

richvdh Mar 12, 2019

richvdh Mar 12, 2019

richvdh Mar 12, 2019

		@@ -211,100 +162,19 @@ versions of synapse.

		.. _UPGRADE.rst: UPGRADE.rst

		.. _federation:

		@@ -348,6 +349,26 @@ https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/services/misc/matrix-

		Once you have installed synapse as above, you will need to configure it.


		## Setting your server_name

Neilj/improved delegation doc 2 #4832

Neilj/improved delegation doc 2 #4832

Conversation

neilisfragile commented Mar 7, 2019

neilisfragile commented Mar 7, 2019

codecov bot commented Mar 7, 2019 • edited Loading

Codecov Report

babolivier left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

richvdh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Mar 7, 2019 •

edited

Loading

babolivier left a comment •

edited

Loading