Skip to content

Commit

Permalink
misc fixups for #629
Browse files Browse the repository at this point in the history
  • Loading branch information
@aaronhurt
aaronhurt committed Apr 19, 2019
1 parent 65b31ca commit bdc0364
Show file tree
Hide file tree
Showing 12 changed files with 35 additions and 27 deletions.
2 changes: 1 addition & 1 deletion main.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ func watchBackend(cfg *config.Config, first chan bool) {
svccfg string
mancfg string
customBE string
next bytes.Buffer
next *bytes.Buffer

once sync.Once
)
Expand Down
14 changes: 7 additions & 7 deletions proxy/http_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ func TestProxyStripsPath(t *testing.T) {
proxy := httptest.NewServer(&HTTPProxy{
Transport: http.DefaultTransport,
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable("route add mock /foo/bar " + server.URL + ` opts "strip=/foo"`)
tbl, _ := route.NewTable(bytes.NewBufferString("route add mock /foo/bar " + server.URL + ` opts "strip=/foo"`))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled)
},
})
Expand All @@ -220,7 +220,7 @@ func TestProxyHost(t *testing.T) {
routes += "route add mock /hostcustom http://a.com/ opts \"host=foo.com\"\n"
routes += "route add mock /hostcustom http://b.com/ opts \"host=bar.com\"\n"
routes += "route add mock / http://a.com/"
tbl, _ := route.NewTable(routes)
tbl, _ := route.NewTable(bytes.NewBufferString(routes))

proxy := httptest.NewServer(&HTTPProxy{
Transport: &http.Transport{
Expand Down Expand Up @@ -271,7 +271,7 @@ func TestProxyHost(t *testing.T) {
func TestHostRedirect(t *testing.T) {
routes := "route add https-redir *:80 https://$host$path opts \"redirect=301\"\n"

tbl, _ := route.NewTable(routes)
tbl, _ := route.NewTable(bytes.NewBufferString(routes))

proxy := httptest.NewServer(&HTTPProxy{
Transport: http.DefaultTransport,
Expand Down Expand Up @@ -311,7 +311,7 @@ func TestPathRedirect(t *testing.T) {
routes := "route add mock / http://a.com/$path opts \"redirect=301\"\n"
routes += "route add mock /foo http://a.com/abc opts \"redirect=301\"\n"
routes += "route add mock /bar http://b.com/$path opts \"redirect=302 strip=/bar\"\n"
tbl, _ := route.NewTable(routes)
tbl, _ := route.NewTable(bytes.NewBufferString(routes))

proxy := httptest.NewServer(&HTTPProxy{
Transport: http.DefaultTransport,
Expand Down Expand Up @@ -483,7 +483,7 @@ func TestProxyHTTPSUpstream(t *testing.T) {
Config: config.Proxy{},
Transport: &http.Transport{TLSClientConfig: tlsClientConfig()},
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable("route add srv / " + server.URL + ` opts "proto=https"`)
tbl, _ := route.NewTable(bytes.NewBufferString("route add srv / " + server.URL + ` opts "proto=https"`))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled)
},
})
Expand All @@ -510,7 +510,7 @@ func TestProxyHTTPSUpstreamSkipVerify(t *testing.T) {
TLSClientConfig: &tls.Config{InsecureSkipVerify: true},
},
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable("route add srv / " + server.URL + ` opts "proto=https tlsskipverify=true"`)
tbl, _ := route.NewTable(bytes.NewBufferString("route add srv / " + server.URL + ` opts "proto=https tlsskipverify=true"`))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled)
},
})
Expand Down Expand Up @@ -711,7 +711,7 @@ func BenchmarkProxyLogger(b *testing.B) {
},
Transport: http.DefaultTransport,
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable("route add mock / " + server.URL)
tbl, _ := route.NewTable(bytes.NewBufferString("route add mock / " + server.URL))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globEnabled)
},
Logger: l,
Expand Down
3 changes: 2 additions & 1 deletion proxy/listen_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package proxy

import (
"bytes"
"net/http"
"net/http/httptest"
"strings"
Expand Down Expand Up @@ -33,7 +34,7 @@ func TestGracefulShutdown(t *testing.T) {
h := &HTTPProxy{
Transport: http.DefaultTransport,
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable("route add svc / " + srv.URL)
tbl, _ := route.NewTable(bytes.NewBufferString("route add svc / " + srv.URL))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
}
Expand Down
4 changes: 2 additions & 2 deletions proxy/tcp_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestTCPProxy(t *testing.T) {
go func() {
h := &tcp.Proxy{
Lookup: func(h string) *route.Target {
tbl, _ := route.NewTable("route add srv :57778 tcp://" + srv.Addr)
tbl, _ := route.NewTable(bytes.NewBufferString("route add srv :57778 tcp://" + srv.Addr))
return tbl.LookupHost(h, route.Picker["rr"])
},
}
Expand Down Expand Up @@ -224,7 +224,7 @@ func TestTCPProxyWithProxyProto(t *testing.T) {
go func() {
h := &tcp.Proxy{
Lookup: func(h string) *route.Target {
tbl, _ := route.NewTable("route add srv :57778 tcp://" + srv.Addr + " opts \"pxyproto=true\"")
tbl, _ := route.NewTable(bytes.NewBufferString("route add srv :57778 tcp://" + srv.Addr + " opts \"pxyproto=true\""))
tgt := tbl.LookupHost(h, route.Picker["rr"])
return tgt
},
Expand Down
4 changes: 2 additions & 2 deletions proxy/ws_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ func TestProxyWSUpstream(t *testing.T) {
Transport: &http.Transport{TLSClientConfig: tlsClientConfig()},
InsecureTransport: &http.Transport{TLSClientConfig: tlsInsecureConfig()},
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable(routes)
tbl, _ := route.NewTable(bytes.NewBufferString(routes))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
})
Expand All @@ -57,7 +57,7 @@ func TestProxyWSUpstream(t *testing.T) {
Transport: &http.Transport{TLSClientConfig: tlsClientConfig()},
InsecureTransport: &http.Transport{TLSClientConfig: tlsInsecureConfig()},
Lookup: func(r *http.Request) *route.Target {
tbl, _ := route.NewTable(routes)
tbl, _ := route.NewTable(bytes.NewBufferString(routes))
return tbl.Lookup(r, "", route.Picker["rr"], route.Matcher["prefix"], globDisabled)
},
})
Expand Down
3 changes: 2 additions & 1 deletion route/issue57_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"bytes"
"net/http"
"testing"
)
Expand Down Expand Up @@ -28,7 +29,7 @@ func TestIssue57(t *testing.T) {
want := "http://foo.com:800"

for i, tt := range tests {
tbl, err := NewTable(tt)
tbl, err := NewTable(bytes.NewBufferString(tt))
if err != nil {
t.Fatalf("%d: got %v want nil", i, err)
}
Expand Down
8 changes: 5 additions & 3 deletions route/parse_new.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,14 @@ route weight service host/path weight w tags "tag1,tag2"
// The commands are parsed in order and order matters.
// Deleting a route that has not been created yet yields
// a different result than the other way around.
func Parse(in bytes.Buffer) (defs []*RouteDef, err error) {
func Parse(in *bytes.Buffer) (defs []*RouteDef, err error) {
var def *RouteDef
scanner := bufio.NewScanner(&in)
var i int
scanner := bufio.NewScanner(in)
for scanner.Scan() {
def, err = nil, nil
result := strings.TrimSpace(scanner.Text())
i++
switch {
case reComment.MatchString(result) || reBlankLine.MatchString(result):
continue
Expand All @@ -86,7 +88,7 @@ func Parse(in bytes.Buffer) (defs []*RouteDef, err error) {
err = errors.New("syntax error: 'route' expected")
}
if err != nil {
return nil, fmt.Errorf("line %d: %s", err)
return nil, fmt.Errorf("line %d: %s", i, err)
}
defs = append(defs, def)
}
Expand Down
5 changes: 3 additions & 2 deletions route/parse_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"bytes"
"reflect"
"regexp"
"testing"
Expand Down Expand Up @@ -162,8 +163,8 @@ func TestParse(t *testing.T) {
return
}

run := func(in string, def []*RouteDef, fail bool, parseFn func(string) ([]*RouteDef, error)) {
out, err := parseFn(in)
run := func(in string, def []*RouteDef, fail bool, parseFn func(*bytes.Buffer) ([]*RouteDef, error)) {
out, err := parseFn(bytes.NewBufferString(in))
switch {
case err == nil && fail:
t.Errorf("got error nil want fail")
Expand Down
3 changes: 2 additions & 1 deletion route/route_bench_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"bytes"
"fmt"
"net/http"
"sync"
Expand Down Expand Up @@ -98,7 +99,7 @@ func makeRoutes(domains, paths, depth, urls int) Table {
}
}

t, err := NewTable(s)
t, err := NewTable(bytes.NewBufferString(s))
if err != nil {
panic(err)
}
Expand Down
6 changes: 3 additions & 3 deletions route/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ func hostpath(prefix string) (host string, path string) {
return p[0], "/" + p[1]
}

func NewTable(s bytes.Buffer) (t Table, err error) {
defs, err := Parse(s)
func NewTable(b *bytes.Buffer) (t Table, err error) {
defs, err := Parse(b)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -490,7 +490,7 @@ func (t Table) String() string {

// Dump returns the routing table as a detailed
func (t Table) Dump() string {
w := new(bytes.Buffer)
var w *bytes.Buffer

This comment has been minimized.

Sign in to view

Copy link
@dcarbone

dcarbone Nov 18, 2019

Contributor

Is there a reason this was changed? This is causing NPE's. What was the intended mechanism for this to be initialized?

This comment has been minimized.

Sign in to view

Copy link
@pschultz

pschultz Nov 18, 2019

Member

Must have been an oversight. Addressed in #720.

hosts := []string{}
for k := range t {
Expand Down
7 changes: 4 additions & 3 deletions route/table_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"bytes"
"crypto/tls"
"fmt"
"math"
Expand Down Expand Up @@ -380,7 +381,7 @@ func TestTableParse(t *testing.T) {
// actual lookup which it probably should.
t.Run(tt.desc, func(t *testing.T) {
// parse the routes
tbl, err := NewTable(strings.Join(tt.in, "\n"))
tbl, err := NewTable(bytes.NewBufferString(strings.Join(tt.in, "\n")))
if err != nil {
t.Fatalf("got %v want nil", err)
}
Expand Down Expand Up @@ -499,7 +500,7 @@ func TestTableLookupIssue448(t *testing.T) {
route add mock / http://foo.com/
`

tbl, err := NewTable(s)
tbl, err := NewTable(bytes.NewBufferString(s))
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -581,7 +582,7 @@ func TestTableLookup(t *testing.T) {
route add svc xyz.com:80/ https://xyz.com
`

tbl, err := NewTable(s)
tbl, err := NewTable(bytes.NewBufferString(s))
if err != nil {
t.Fatal(err)
}
Expand Down
3 changes: 2 additions & 1 deletion route/target_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package route

import (
"bytes"
"net/url"
"testing"
)
Expand Down Expand Up @@ -140,7 +141,7 @@ func TestTarget_BuildRedirectURL(t *testing.T) {
return nil
}
for _, tt := range tests {
tbl, _ := NewTable(tt.route)
tbl, _ := NewTable(bytes.NewBufferString(tt.route))
route := firstRoute(tbl)
target := route.Targets[0]
for _, rt := range tt.tests {
Expand Down

2 comments on commit bdc0364

@aaronhurt
Copy link
Member Author

Choose a reason for hiding this comment

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

@murphymj25 Just a few small changes, mainly around fixing test cases. Would you mind running your test against this version again to ensure it still fixes your issue? I can't see how it would break anything but would be nice to confirm. Thanks again!

@murphymj25
Copy link
Contributor

Choose a reason for hiding this comment

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

@leprechau Sure thing! Should be able to benchmark it next week.

Please sign in to comment.