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

Support for Unix Domain Sockets #2471

Merged
merged 22 commits into from Aug 20, 2020
Merged

Conversation

dimitribouniol
Copy link
Contributor

@dimitribouniol dimitribouniol commented Aug 12, 2020

Adds support for Unix Domain Sockets (#2471, #2226, #2371).

A server can be started using the serve command:

vapor run serve --unix-socket /tmp/vapor.socket

Alternatively, the app can be configured in code to connect to a socket:

app.http.server.configuration.unixDomainSocketPath = "/tmp/vapor.socket"

In either scenario, if a socket path is specified, it takes precedence over binding the server to a hostname/port. Note that TLS may be combined with this options if you wish the server to only listen to encrypted connections made to the socket file.

The socket file must not exist prior to starting the server. If one is left behind during an incomplete shutdown, however, and the app has permission to delete it, it will be overidden.

To assist with making connections to socket paths, URI.Scheme has a .httpUnixDomainSocket property.

try app.client.get(.init(scheme: .httpUnixDomainSocket, host: "/tmp/vapor.socket", path: "/foo"))

@tanner0101 tanner0101 added enhancement New feature or request semver-minor Contains new API labels Aug 12, 2020
@tanner0101 tanner0101 added this to Awaiting Review in Vapor 4 via automation Aug 12, 2020
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

This looks great, thanks! LMK what you think of my comments inline.

Sources/Vapor/Commands/ServeCommand.swift Outdated Show resolved Hide resolved
Sources/Vapor/HTTP/Server/HTTPServer.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

Great changes, thank you!

I've prefixed non-critical things with nit:. Feel free to ignore those. The other things I think must be fixed before merging.

Sources/Vapor/Commands/ServeCommand.swift Outdated Show resolved Hide resolved
Sources/Vapor/HTTP/Server/HTTPServer.swift Outdated Show resolved Hide resolved
Sources/Vapor/HTTP/Server/HTTPServer.swift Outdated Show resolved Hide resolved
Sources/Vapor/HTTP/Server/HTTPServer.swift Outdated Show resolved Hide resolved
Sources/Vapor/Server/Server.swift Outdated Show resolved Hide resolved
Sources/Vapor/Server/Server.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Tests/VaporTests/ServerTests.swift Show resolved Hide resolved
@dimitribouniol
Copy link
Contributor Author

Ok, I think I got everything. Let me know if I should make any more changes!

dimitribouniol and others added 18 commits August 17, 2020 09:28
… a socket path as opposed to a hostname and port
Co-Authored-By: Gwynne Raskind <gwynne@users.noreply.github.com>
@dimitribouniol
Copy link
Contributor Author

Rebased off of the latest master.

Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

Thanks! The updated Server protocol is much clearer and URI.Host is a great addition. Just a couple minor comments left. :)

Sources/Vapor/Server/Server.swift Outdated Show resolved Hide resolved
Sources/Vapor/Commands/ServeCommand.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Outdated Show resolved Hide resolved
Sources/Vapor/Utilities/URI.swift Show resolved Hide resolved
Copy link
Member

@tanner0101 tanner0101 left a comment

Choose a reason for hiding this comment

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

These changes LGTM. :) I've updated the PR body slightly to reflect the latest API. Ready to merge this?

@dimitribouniol
Copy link
Contributor Author

Ready when you are! 🎉

@dimitribouniol
Copy link
Contributor Author

Removed Note that making requests to other socket-path-based services requires AsyncHTTPClient 1.2.0 or newer. from the description, since Vapor now requires it anyways, so it should update on user's repos automatically.

@MrLotU MrLotU merged commit d910be4 into vapor:master Aug 20, 2020
Vapor 4 automation moved this from Awaiting Review to Done Aug 20, 2020
@tanner0101
Copy link
Member

These changes are now available in 4.29.0

@MrLotU
Copy link
Sponsor Member

MrLotU commented Aug 20, 2020

Thanks @dimitribouniol :D

@tanner0101
Copy link
Member

tanner0101 commented Aug 20, 2020

Thanks for the great additions @dimitribouniol !

@dimitribouniol
Copy link
Contributor Author

@tanner0101 @MrLotU Thank you both for helping make this the best it could be!

@dimitribouniol dimitribouniol deleted the unix-domain-sockets branch August 22, 2020 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semver-minor Contains new API
Projects
Vapor 4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants