chore: move port forwarding out of cli package #19092
Conversation
fb745d4 to
7512819
Compare
|
Definitely, I've removed the protobuf changes from this PR. |
d94b25e to
86d37e2
Compare
|
|
||
| // Test if we can actually bind to the port before adding the forwarder | ||
| listenAddress := netip.AddrPortFrom(spec.ListenHost, spec.ListenPort) | ||
| testListener, err := m.opts.Listener.Listen(spec.Network, listenAddress.String()) |
There was a problem hiding this comment.
Add() listens but doesn't start forwarding? IMO the Start() API should just be removed in favor of Add() always creating the listener and starting the forwarding goroutine
| return nil | ||
| } | ||
|
|
||
| func (m *manager) Stop() error { |
There was a problem hiding this comment.
| func (m *manager) Stop() error { | |
| func (m *manager) Close() error { |
so it implements io.Closer
| } | ||
|
|
||
| // Forwarder handles a single port forward. | ||
| type Forwarder interface { |
There was a problem hiding this comment.
The forwarder interface doesn't seem to do much except track a goroutine. All of it's methods could probably just be added as private methods onto the manager implementation IMO.
| "golang.org/x/xerrors" | ||
|
|
||
| "cdr.dev/slog" | ||
|
|
| m.mu.Lock() | ||
| defer m.mu.Unlock() | ||
|
|
||
| key := fmt.Sprintf("%s:%s:%d", spec.Network, spec.ListenHost, spec.ListenPort) |
There was a problem hiding this comment.
This key generation should be added as a method on the spec struct. Probably should also use a different character as a separator since : shows up in IPv6 addresses
This PR is the initial work needed for hosting the port forwarding capabilities both from the vpn tunnel package and cli instead of having the logic duplicated.