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

Provide easier way to create a TURN client #308

Open
stv0g opened this issue Feb 25, 2023 · 2 comments
Open

Provide easier way to create a TURN client #308

stv0g opened this issue Feb 25, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@stv0g
Copy link
Member

stv0g commented Feb 25, 2023

There has been a discussion initiated on whether we might want to introduce an easier API for creating a TURN client in #276:

There is a slightly related PR in pion/stun#134, which introduces a new stun.DialURI() function.
It can be used in conjunction with pion/turn.NewClient().

I am still pondering whether we also want to have a function in pion/turn to create a client directly by an TURN URI.

I think this would be a nice addition that would make the life of people that just want a quick TURN client connection a lot more easier. Currently it takes some 70 lines of code in the UDP example to create the socket, set up the Client, and then calling client.Listen and client.Allocate, it would ne a big win to cut this down to 1-5 lines for simple use cases that do not require a fully fledged client.

I'm wondering though how to pass the realm and the TURN credentials in to the Dial method, maybe we would need a simple Dialer that can hold these for the Dial?

I've introduced a new type in pion/stun to pass additional settings to the stun.DialURI() function:

// DialConfig is used to pass configuration to DialURI()
type DialConfig struct {
	DTLSConfig dtls.Config
	TLSConfig  tls.Config

	Net transport.Net
}

We could do the same for pion/turn.
And thats where we probably need to discuss whether this addition would be a breaking API change as there exists already a ClientConfig struct.

Maybe it could look something like this:

type ClientConfig struct {
	stun.DialConfig
	
	Username       string
	Password       string
	Realm          string
	Software       string
	RTO            time.Duration
	Conn           net.PacketConn // Listening socket (net.PacketConn)
	LoggerFactory  logging.LoggerFactory
}

// Connect by conn and STUN/TURN server addresses
func NewClientFromConn(conn net.PacketConn, stunAddr net.Addr, turnAddr net.Addr, cfg *ClientConfig) (*Client, error) { ... }

// Connect by URI
func NewClientFromURI(uri stun.URI, *cfg ClientConfig) (*Client, error) { ... }

Ideas for a better API are welcome :)

@stv0g stv0g added the enhancement New feature or request label Feb 25, 2023
@stv0g
Copy link
Member Author

stv0g commented Feb 25, 2023

@rg0now I moved the discussion from #276 to a new issue here.

@rg0now
Copy link
Contributor

rg0now commented Feb 26, 2023

I was under the impression that turn.Dialer would be some sort of a dumbed down version of ClientConfig, just enough to create a basic client without all the bells and whistles. It would not even return a Client object, just the TURN client connection as a net.PacketConn. The idea is to make it simple for people familiar with net.Dialer to create TURN client connections.

type Dialer struct {
	Username   string
	Password   string
	Realm      string
	DTLSConfig dtls.Config
	TLSConfig  tls.Config

	Net transport.Net
}

// Dial connects to the TURN server and returns a client connection. The network specifies
// the TURN transport (`udp`, `tcp`, `tls`, `dtls`) and address is a pair of an address and a port.
func (d *Dialer) Dial(network, address string) (net.PacketConn, error)

// DialURI connects to the TURN server URI and returns a client connection. The URI must be a
// standard TURN server URI as of RFC 7065: `<scheme>:<host>[:port][?transport=<protocol>]`.
func (d *Dialer) DialURI(uri string) (net.PacketConn, error)

Since we do not return the turn.Client to the user, we would need to maintain it for them during the lifetime of the returned net.PacketConn. One way to do that would be to create a separate turn.Client per turn.Dial* call, embed it into the returned net.PacketConn and then closing the underlying client once the PacketConn is closed.

For anything more complex, we would provide the familiar full Client API, like the below.

type ClientConfig struct {
	Dialer

	Software      string
	RTO           time.Duration
	Conn          net.PacketConn // Listening socket (net.PacketConn)
	LoggerFactory logging.LoggerFactory
}

// NewClient returns a new Client instance. listeningAddress is the address and port to listen on,
// default "0.0.0.0:0"
func NewClient(config *ClientConfig) (*Client, error)

I see one problem with embedding the Dialer in the ClientConfig: if the Dialer contains a *tls.Config but the user supplies their own net.PacketConn (say, as a TCP connection) then it's somehow not clear which on takes precedence. Maybe we just want to repeat the Dialer here without the *tls.Config?

Then again, I'm also completely OK with your proposal too. One note: I'm not sure we want a hard dependency on pion/stun, would it be a terrible overkill to just repeat stun.DialConfig as turn.DialConfig here? Plus, don't we want the net API style "call Dial on a Dialer" instead of "pass a DialConfig to Dial as an argument"?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants