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

Idea: pass a socket instead of dqlite_node_set_bind_address #512

Open
cole-miller opened this issue May 5, 2023 · 4 comments
Open

Idea: pass a socket instead of dqlite_node_set_bind_address #512

cole-miller opened this issue May 5, 2023 · 4 comments
Labels
Feature New feature, not a bug

Comments

@cole-miller
Copy link
Contributor

Would it make sense to offer a function like this in the dqlite.h API?

/* Set the socket that the dqlite server will listen on for requests
 * from clients and other servers. This is similar to dqlite_node_set_bind_address,
 * but allows the application to set up its own socket instead of giving
 * that responsibility to dqlite. The socket must already have been prepared
 * with bind(2). If this function is called, any address configured with
 * dqlite_node_set_bind_address is ignored.
 */
int dqlite_node_set_sock(dqlite_node *node, int sock);
/* and/or the same for dqlite_server */

The Juju team is suffering from some papercuts because they can't retrieve the port that the dqlite server is using when they pass 127.0.0.1:0 (etc.) as the bind address; this feature would give them a good solution to that problem.

@freeekanayaka
Copy link
Contributor

Would it make sense to offer a function like this in the dqlite.h API?

/* Set the socket that the dqlite server will listen on for requests
 * from clients and other servers. This is similar to dqlite_node_set_bind_address,
 * but allows the application to set up its own socket instead of giving
 * that responsibility to dqlite. The socket must already have been prepared
 * with bind(2). If this function is called, any address configured with
 * dqlite_node_set_bind_address is ignored.
 */
int dqlite_node_set_sock(dqlite_node *node, int sock);
/* and/or the same for dqlite_server */

The Juju team is suffering from some papercuts because they can't retrieve the port that the dqlite server is using when they pass 127.0.0.1:0 (etc.) as the bind address; this feature would give them a good solution to that problem.

Why are they listening to a local TCP socket in the first place? That seems insecure. If they want to proxy dqlite, the way to do it would be with an abstract unix socket.

@cole-miller
Copy link
Contributor Author

@freeekanayaka it's in the context of writing tests for Juju, so I think security is not a concern. I think lots of tests will be running concurrently, so they need a supply of distinct addresses for dqlite servers to bind to, and a convenient way to do that is to have the OS assign a port to each. With an abstract Unix socket, I guess either you'd specify a distinct path for each test (possibly generating them in some way) or you'd just pass "@" and then retrieve the full path with dqlite_node_get_bind_address -- which is the solution I'll recommend to the Juju team if we decide not to implement this idea.

More broadly, I think that passing a socket directly is in some ways more flexible/"composable" than set_bind_address, so I thought I'd throw the idea out for comment.

@freeekanayaka
Copy link
Contributor

I'd recommend to use "@" since it will be also faster than TCP (which even with the loopback interface is considerably slower than abstract Unix sockets). The dqlite_node_get_bind_address is indeed there to address this use case.

Unless I'm missing something, passing a socket directly seem something we don't absolutely need to support right now, so I'd be keen to not increase the API surface unless there is some real world use case that we can't currently meet.

@cole-miller
Copy link
Contributor Author

Okay, I've passed the suggestion to use dqlite_node_set_bind_address("@") along to the Juju team and will report back whether it's suitable for them.

@MathieuBordere MathieuBordere added the Feature New feature, not a bug label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature, not a bug
Projects
None yet
Development

No branches or pull requests

3 participants