Skip to content

Enable add/remove routes after server starts #15

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
53 changes: 41 additions & 12 deletions tcpproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import (
"io"
"log"
"net"
"sync"
"time"
)

Expand Down Expand Up @@ -93,11 +94,19 @@ func equals(want string) Matcher {

// config contains the proxying state for one listener.
type config struct {
routes []route
sync.Mutex // protect r/w of routes
Copy link
Collaborator

Choose a reason for hiding this comment

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

comment should be like:

mu sync.Mutex // guards following
x int
y int

and blank line before stuff that is no longer guarded by it.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

nextRouteId int
routes map[int]route
acmeTargets []Target // accumulates targets that should be probed for acme.
stopACME bool // if true, AddSNIRoute doesn't add targets to acmeTargets.
}

func NewConfig() (cfg *config) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

needs docs.

and unname result parameter per https://github.com/golang/go/wiki/CodeReviewComments#named-result-parameters

but actually I'd just delete this and let the zero value of Config be useful. You can initialize the routes map as needed.

Copy link
Author

Choose a reason for hiding this comment

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

Removed the constructor.

cfg = &config{}
cfg.routes = make(map[int]route)
return
}

// A route matches a connection to a target.
type route interface {
// match examines the initial bytes of a connection, looking for a
Expand All @@ -122,25 +131,43 @@ func (p *Proxy) configFor(ipPort string) *config {
p.configs = make(map[string]*config)
}
if p.configs[ipPort] == nil {
p.configs[ipPort] = &config{}
p.configs[ipPort] = NewConfig()
}
return p.configs[ipPort]
}

func (p *Proxy) addRoute(ipPort string, r route) {
func (p *Proxy) addRoute(ipPort string, r route) (routeId int) {
cfg := p.configFor(ipPort)
cfg.routes = append(cfg.routes, r)
cfg.Lock()
defer cfg.Unlock()
routeId = cfg.nextRouteId
cfg.nextRouteId++
cfg.routes[routeId] = r
return
}

// AddRoute appends an always-matching route to the ipPort listener,
// directing any connection to dest.
// directing any connection to dest. The added route's id is returned
// for future removal.
//
// This is generally used as either the only rule (for simple TCP
// proxies), or as the final fallback rule for an ipPort.
//
// The ipPort is any valid net.Listen TCP address.
func (p *Proxy) AddRoute(ipPort string, dest Target) {
p.addRoute(ipPort, fixedTarget{dest})
func (p *Proxy) AddRoute(ipPort string, dest Target) (routeId int) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rebase to fix the case of these originally, rather than fixing it in a later commit in the series.

return p.addRoute(ipPort, fixedTarget{dest})
}

// RemoveRoute removes an existing route for ipPort. If the route is
// not found, this is an no-op.
//
// Both AddRoute and RemoveRoute is go-routine safe.
func (p *Proxy) RemoveRoute(ipPort string, routeId int) (err error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

Done. Thanks.

cfg := p.configFor(ipPort)
cfg.Lock()
defer cfg.Unlock()
delete(cfg.routes, routeId)
return
}

type fixedTarget struct {
Expand Down Expand Up @@ -197,7 +224,7 @@ func (p *Proxy) Start() error {
return err
}
p.lns = append(p.lns, ln)
go p.serveListener(errc, ln, config.routes)
go p.serveListener(errc, ln, config)
}
go p.awaitFirstError(errc)
return nil
Expand All @@ -208,22 +235,24 @@ func (p *Proxy) awaitFirstError(errc <-chan error) {
close(p.donec)
}

func (p *Proxy) serveListener(ret chan<- error, ln net.Listener, routes []route) {
func (p *Proxy) serveListener(ret chan<- error, ln net.Listener, cfg *config) {
for {
c, err := ln.Accept()
if err != nil {
ret <- err
return
}
go p.serveConn(c, routes)
go p.serveConn(c, cfg)
}
}

// serveConn runs in its own goroutine and matches c against routes.
// It returns whether it matched purely for testing.
func (p *Proxy) serveConn(c net.Conn, routes []route) bool {
func (p *Proxy) serveConn(c net.Conn, cfg *config) bool {
br := bufio.NewReader(c)
for _, route := range routes {
cfg.Lock()
defer cfg.Unlock()
for _, route := range cfg.routes {
if target := route.match(br); target != nil {
if n := br.Buffered(); n > 0 {
peeked, _ := br.Peek(br.Buffered())
Expand Down