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

Enable connecting to appservices through unix sockets #16399

Open
Sir-Photch opened this issue Sep 29, 2023 · 6 comments · May be fixed by #16425
Open

Enable connecting to appservices through unix sockets #16399

Sir-Photch opened this issue Sep 29, 2023 · 6 comments · May be fixed by #16425
Labels
A-Application-Service Related to AS support O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@Sir-Photch
Copy link
Contributor

Description:

When the URL provided in an appservice-registration is a unix socket of the schema unix:/path/to.sock, the server refuses to connect and reports "Unsupported schema: 'unix'".

For deployments where appservices are on the same machine as the server, it would make sense to enable this functionality.

(Since this seems to be handled specifically by the server, I assume it's not a bug.)

@erikjohnston erikjohnston added T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 29, 2023
@clokep
Copy link
Member

clokep commented Sep 29, 2023

I wonder if this is even allowed by the spec, that defines the url as:

Required: The URL for the application service. May include a path after the domain name. Optionally set to null if no traffic is required.

It doesn't explicitly forbid it, but 🤷 anyway, it is likely fine.

@clokep clokep added the A-Application-Service Related to AS support label Sep 29, 2023
@realtyem
Copy link
Contributor

I am pretty unread into the appservice chapters of this book, but from what I'm understanding from out-of-band conversations with others is that it's probably not a good idea. I can get the SimpleHttpClient to speak to sockets like was done for ReplicationClient, but what happens when Synapse is using workers and if the endpoints it needs to talk to are load-balanced? Or sharded? (Does a bridge ever have to speak directly to a stream writer?) Since the reverse proxy handles which worker gets what work, it may be better to just 'not' do this. And like I said, I'm not fully read in to how they work so I could be completely off-base.

I have to admit though, it does sound attractive.

@clokep
Copy link
Member

clokep commented Sep 29, 2023

but what happens when Synapse is using workers and if the endpoints it needs to talk to are load-balanced? Or sharded?

I'm pretty sure this issue is about the Synapse to appservice communication, not the other way around.

@realtyem
Copy link
Contributor

realtyem commented Sep 30, 2023

I've been giving this a look-over, and there is some information I think I'm missing to make sure I don't break anything.

Does any appservice:

  • Need/use the ip(block|allow)listing functions?
  • Need/use forward proxy support?
  • Need/use HTTP redirects or cookie handling?
  • Buffered responses(I think just for response bodies, which is actually disabled in Synapse)?

EDIT: Also, I think treq which is part of the request machinery was hindering using 'relaxed' ascii for hostnames(in the most general case, for worker names). Would that be something useful/helpful?(Thinking of usage of things like docker compose/kubernetes here, not sure how strict the internal DNS of docker is, etc.)

EDIT 2: (deleted above) No that was ensureValidURI() that was restricting hostnames by IDNA2008. Still a valid question though.

@realtyem
Copy link
Contributor

realtyem commented Oct 4, 2023

After a few days of poking/staring at the appservice code, I think it's fair to mark a few things off the list of requirements:

  • no ip(block|allow)list support
  • no redirects or cookie handling
  • buffered response bodies

None of those seem to make sense when using a appservice. Which means treq doesn't need to be part of the equation. The question of connecting through a proxy remains on the table.

That being said, using the request() machinery of Twisted(as it's the most widely used in Synapse) means there are some restrictions on how to handle passing the filename of the unix socket into that machinery. I think the simplest method will be to append a : to the socket filename internally. It follows along with how Nginx(as a single example) allows for separating the path_to_socket_file(unix:/path/to.socket) and path_to_url_request(/_matrix/app/v1/whatever).

@realtyem
Copy link
Contributor

realtyem commented Oct 4, 2023

And it looks like a proxy isn't going to happen/be needed as even the docs say so:
https://matrix-org.github.io/synapse/latest/setup/forward_proxy.html#connection-types

(Thanks @clokep !!)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support O-Uncommon Most users are unlikely to come across this or unexpected workflow T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants