From 211b0ec5388803d07d0836a26077651b36910d5b Mon Sep 17 00:00:00 2001 From: Alexander Yastrebov Date: Mon, 19 Dec 2022 23:14:48 +0100 Subject: [PATCH] Fixes goroutine leaks in tests (#2169) Adds main_test.go with noleak.CheckMain(m) into more packages and fixes leaks in tests. Signed-off-by: Alexander Yastrebov Signed-off-by: Alexander Yastrebov --- eskipfile/main_test.go | 12 ++ eskipfile/remote.go | 2 + eskipfile/remote_test.go | 10 +- eskipfile/watch.go | 12 +- filters/builtin/compress_test.go | 23 ++- filters/builtin/creationmetrics_test.go | 2 + filters/builtin/header_test.go | 10 +- filters/builtin/main_test.go | 12 ++ filters/builtin/redirect_test.go | 179 ++++++++++++------------ filters/diag/main_test.go | 12 ++ filters/ratelimit/fail_closed_test.go | 5 +- filters/ratelimit/main_test.go | 12 ++ filters/scheduler/fifo_test.go | 10 ++ filters/scheduler/lifo_test.go | 3 + filters/scheduler/loopback_test.go | 5 + filters/scheduler/main_test.go | 12 ++ filters/tee/main_test.go | 12 ++ filters/tee/teeloopback_test.go | 31 +++- proxy/proxytest/proxytest.go | 17 +-- routing/datasource_test.go | 4 + routing/example_test.go | 2 + routing/main_test.go | 12 ++ routing/predicates_test.go | 1 + routing/routing_test.go | 53 ++++++- routing/subtreeconflict_test.go | 2 + routing/testdataclient/dataclient.go | 14 +- routing/tree_test.go | 1 + secrets/main_test.go | 12 ++ 28 files changed, 365 insertions(+), 117 deletions(-) create mode 100644 eskipfile/main_test.go create mode 100644 filters/builtin/main_test.go create mode 100644 filters/diag/main_test.go create mode 100644 filters/ratelimit/main_test.go create mode 100644 filters/scheduler/main_test.go create mode 100644 filters/tee/main_test.go create mode 100644 routing/main_test.go create mode 100644 secrets/main_test.go diff --git a/eskipfile/main_test.go b/eskipfile/main_test.go new file mode 100644 index 0000000000..3a8f2bd0c5 --- /dev/null +++ b/eskipfile/main_test.go @@ -0,0 +1,12 @@ +package eskipfile + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/eskipfile/remote.go b/eskipfile/remote.go index 216f5d83a0..fe8041fd05 100644 --- a/eskipfile/remote.go +++ b/eskipfile/remote.go @@ -69,6 +69,7 @@ func RemoteWatch(o *RemoteWatchOptions) (routing.DataClient, error) { err = dataClient.DownloadRemoteFile() if err != nil { + dataClient.http.Close() return nil, err } } else { @@ -78,6 +79,7 @@ func RemoteWatch(o *RemoteWatchOptions) (routing.DataClient, error) { } if err != nil { + dataClient.http.Close() return nil, err } dataClient.preloaded = true diff --git a/eskipfile/remote_test.go b/eskipfile/remote_test.go index 3cf45257bc..92aa5495de 100644 --- a/eskipfile/remote_test.go +++ b/eskipfile/remote_test.go @@ -141,6 +141,10 @@ func TestLoadAllAndUpdate(t *testing.T) { options := &RemoteWatchOptions{RemoteFile: testValidServer.URL, Threshold: 10, Verbose: true, FailOnStartup: true} client, err := RemoteWatch(options) + if err == nil { + defer client.(*remoteEskipFile).Close() + } + if err == nil && test.fail { t.Error("failed to fail") return @@ -175,7 +179,11 @@ func TestHTTPTimeout(t *testing.T) { time.Sleep(2 * time.Second) })) defer server.Close() - _, err := RemoteWatch(&RemoteWatchOptions{RemoteFile: server.URL, HTTPTimeout: 1 * time.Second, FailOnStartup: true}) + client, err := RemoteWatch(&RemoteWatchOptions{RemoteFile: server.URL, HTTPTimeout: 1 * time.Second, FailOnStartup: true}) + if err == nil { + defer client.(*remoteEskipFile).Close() + } + if err, ok := err.(net.Error); !ok || !err.Timeout() { t.Errorf("got %v, expected net.Error with timeout", err) } diff --git a/eskipfile/watch.go b/eskipfile/watch.go index a86cf55924..42a3a5fb45 100644 --- a/eskipfile/watch.go +++ b/eskipfile/watch.go @@ -145,7 +145,11 @@ func (c *WatchClient) watch() { // LoadAll returns the parsed route definitions found in the file. func (c *WatchClient) LoadAll() ([]*eskip.Route, error) { req := make(chan watchResponse) - c.getAll <- req + select { + case c.getAll <- req: + case <-c.quit: + return nil, nil + } rsp := <-req return rsp.routes, rsp.err } @@ -153,7 +157,11 @@ func (c *WatchClient) LoadAll() ([]*eskip.Route, error) { // LoadUpdate returns differential updates when a watched file has changed. func (c *WatchClient) LoadUpdate() ([]*eskip.Route, []string, error) { req := make(chan watchResponse) - c.getUpdates <- req + select { + case c.getUpdates <- req: + case <-c.quit: + return nil, nil, nil + } rsp := <-req return rsp.routes, rsp.deletedIDs, rsp.err } diff --git a/filters/builtin/compress_test.go b/filters/builtin/compress_test.go index acb213653b..c80a8134c7 100644 --- a/filters/builtin/compress_test.go +++ b/filters/builtin/compress_test.go @@ -5,7 +5,6 @@ import ( "compress/flate" "compress/gzip" "errors" - "github.com/andybalholm/brotli" "io" "math/rand" "net/http" @@ -19,6 +18,8 @@ import ( "github.com/zalando/skipper/filters" "github.com/zalando/skipper/filters/filtertest" "github.com/zalando/skipper/proxy/proxytest" + + "github.com/andybalholm/brotli" ) const ( @@ -566,6 +567,16 @@ func TestForwardError(t *testing.T) { } } +type readCloser struct { + io.Reader + done chan struct{} +} + +func (h *readCloser) Close() error { + close(h.done) + return nil +} + func TestCompressWithEncodings(t *testing.T) { spec, err := NewCompressWithOptions(CompressOptions{Encodings: []string{"br", "gzip"}}) if err != nil { @@ -576,15 +587,21 @@ func TestCompressWithEncodings(t *testing.T) { t.Fatal(err) } + done := make(chan struct{}) + req := &http.Request{Header: http.Header{"Accept-Encoding": []string{"gzip,br,deflate"}}} - body := io.NopCloser(&io.LimitedReader{R: rand.New(rand.NewSource(0)), N: 100}) + body := &readCloser{Reader: &io.LimitedReader{R: rand.New(rand.NewSource(0)), N: 100}, done: done} rsp := &http.Response{ Header: http.Header{"Content-Type": []string{"application/octet-stream"}}, - Body: body} + Body: body, + } ctx := &filtertest.Context{FRequest: req, FResponse: rsp} f.Response(ctx) + io.Copy(io.Discard, rsp.Body) + <-done + enc := rsp.Header.Get("Content-Encoding") if enc != "br" { t.Error("unexpected value", enc) diff --git a/filters/builtin/creationmetrics_test.go b/filters/builtin/creationmetrics_test.go index c2e4517624..d481e3fb92 100644 --- a/filters/builtin/creationmetrics_test.go +++ b/filters/builtin/creationmetrics_test.go @@ -265,6 +265,7 @@ func TestRouteCreationMetricsMarkerDropped(t *testing.T) { if err != nil { t.Fatal(err) } + defer dc.Close() filtersBefore := make(map[string]bool) er, err := dc.LoadAll() @@ -284,6 +285,7 @@ func TestRouteCreationMetricsMarkerDropped(t *testing.T) { PostProcessors: []routing.PostProcessor{NewRouteCreationMetrics(&metricstest.MockMetrics{})}, SignalFirstLoad: true, }) + defer rt.Close() <-rt.FirstLoad() r, _ := rt.Route(&http.Request{URL: &url.URL{Path: "/"}}) diff --git a/filters/builtin/header_test.go b/filters/builtin/header_test.go index 8835323612..6ae6db2bc2 100644 --- a/filters/builtin/header_test.go +++ b/filters/builtin/header_test.go @@ -1,14 +1,15 @@ package builtin import ( - "github.com/zalando/skipper/eskip" - "github.com/zalando/skipper/filters" - "github.com/zalando/skipper/filters/filtertest" - "github.com/zalando/skipper/proxy/proxytest" "net/http" "net/http/httptest" "strings" "testing" + + "github.com/zalando/skipper/eskip" + "github.com/zalando/skipper/filters" + "github.com/zalando/skipper/filters/filtertest" + "github.com/zalando/skipper/proxy/proxytest" ) type testContext struct { @@ -522,6 +523,7 @@ func TestHeader(t *testing.T) { t.Error(err) return } + defer rsp.Body.Close() if ti.valid && rsp.StatusCode != http.StatusOK || !ti.valid && rsp.StatusCode != http.StatusNotFound { diff --git a/filters/builtin/main_test.go b/filters/builtin/main_test.go new file mode 100644 index 0000000000..74e53cf2be --- /dev/null +++ b/filters/builtin/main_test.go @@ -0,0 +1,12 @@ +package builtin + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/filters/builtin/redirect_test.go b/filters/builtin/redirect_test.go index 602aaf1d31..a0b348f766 100644 --- a/filters/builtin/redirect_test.go +++ b/filters/builtin/redirect_test.go @@ -1,6 +1,7 @@ package builtin import ( + "fmt" "net/http" "net/http/httptest" "net/url" @@ -123,54 +124,56 @@ func TestRedirect(t *testing.T) { "not deprecated", filters.RedirectToName, }} { - var args []interface{} - if ti.skipLocationArg { - args = []interface{}{float64(ti.code)} - } else { - args = []interface{}{float64(ti.code), ti.filterLocation} - } - dc := testdataclient.New([]*eskip.Route{{ - Shunt: true, - Filters: []*eskip.Filter{{ - Name: tii.name, - Args: args}}}}) - tl := loggingtest.New() - rt := routing.New(routing.Options{ - FilterRegistry: MakeRegistry(), - DataClients: []routing.DataClient{dc}, - Log: tl}) - p := proxy.WithParams(proxy.Params{ - Routing: rt, - }) + t.Run(fmt.Sprintf("%s/%s", ti.msg, tii.name), func(t *testing.T) { + var args []interface{} + if ti.skipLocationArg { + args = []interface{}{float64(ti.code)} + } else { + args = []interface{}{float64(ti.code), ti.filterLocation} + } + dc := testdataclient.New([]*eskip.Route{{ + Shunt: true, + Filters: []*eskip.Filter{{ + Name: tii.name, + Args: args}}}, + }) + defer dc.Close() + + tl := loggingtest.New() + defer tl.Close() + + rt := routing.New(routing.Options{ + FilterRegistry: MakeRegistry(), + DataClients: []routing.DataClient{dc}, + Log: tl, + }) + defer rt.Close() + + p := proxy.WithParams(proxy.Params{ + Routing: rt, + }) + defer p.Close() - closeAll := func() { - p.Close() - rt.Close() - tl.Close() - } - - // pick up routing - if err := tl.WaitFor("route settings applied", time.Second); err != nil { - t.Error(err) - closeAll() - continue - } - - req := &http.Request{ - URL: &url.URL{Path: "/some/path", RawQuery: "foo=1&bar=2"}, - Host: "incoming.example.org"} - w := httptest.NewRecorder() - p.ServeHTTP(w, req) - - if w.Code != ti.code { - t.Error(ti.msg, tii.msg, "invalid status code", w.Code) - } - - if w.Header().Get("Location") != ti.checkLocation { - t.Error(ti.msg, tii.msg, "invalid location", w.Header().Get("Location")) - } - - closeAll() + // pick up routing + if err := tl.WaitFor("route settings applied", time.Second); err != nil { + t.Error(err) + return + } + + req := &http.Request{ + URL: &url.URL{Path: "/some/path", RawQuery: "foo=1&bar=2"}, + Host: "incoming.example.org"} + w := httptest.NewRecorder() + p.ServeHTTP(w, req) + + if w.Code != ti.code { + t.Error("invalid status code", w.Code) + } + + if w.Header().Get("Location") != ti.checkLocation { + t.Error("invalid location", w.Header().Get("Location")) + } + }) } } } @@ -209,47 +212,49 @@ func TestRedirectLower(t *testing.T) { "lowercase", filters.RedirectToLowerName, }} { - dc := testdataclient.New([]*eskip.Route{{ - Shunt: true, - Filters: []*eskip.Filter{{ - Name: tii.name, - Args: []interface{}{float64(ti.code), ti.filterLocation}}}}}) - tl := loggingtest.New() - rt := routing.New(routing.Options{ - FilterRegistry: MakeRegistry(), - DataClients: []routing.DataClient{dc}, - Log: tl}) - p := proxy.WithParams(proxy.Params{ - Routing: rt, - }) + t.Run(fmt.Sprintf("%s/%s", ti.msg, tii.name), func(t *testing.T) { + dc := testdataclient.New([]*eskip.Route{{ + Shunt: true, + Filters: []*eskip.Filter{{ + Name: tii.name, + Args: []interface{}{float64(ti.code), ti.filterLocation}}}}, + }) + defer dc.Close() + + tl := loggingtest.New() + defer tl.Close() + + rt := routing.New(routing.Options{ + FilterRegistry: MakeRegistry(), + DataClients: []routing.DataClient{dc}, + Log: tl, + }) + defer rt.Close() + + p := proxy.WithParams(proxy.Params{ + Routing: rt, + }) + defer p.Close() - closeAll := func() { - p.Close() - rt.Close() - tl.Close() - } - - if err := tl.WaitFor("route settings applied", time.Second); err != nil { - t.Error(err) - closeAll() - continue - } - - req := &http.Request{ - URL: &url.URL{Path: "/some/path", RawQuery: "foo=1&bar=2"}, - Host: "incoming.example.org"} - w := httptest.NewRecorder() - p.ServeHTTP(w, req) - - if w.Code != ti.code { - t.Error(ti.msg, tii.msg, "invalid status code", w.Code) - } - - if w.Header().Get("Location") != ti.checkLocation { - t.Error(ti.msg, tii.msg, "invalid location", w.Header().Get("Location")) - } - - closeAll() + if err := tl.WaitFor("route settings applied", time.Second); err != nil { + t.Error(err) + return + } + + req := &http.Request{ + URL: &url.URL{Path: "/some/path", RawQuery: "foo=1&bar=2"}, + Host: "incoming.example.org"} + w := httptest.NewRecorder() + p.ServeHTTP(w, req) + + if w.Code != ti.code { + t.Error("invalid status code", w.Code) + } + + if w.Header().Get("Location") != ti.checkLocation { + t.Error("invalid location", w.Header().Get("Location")) + } + }) } } } diff --git a/filters/diag/main_test.go b/filters/diag/main_test.go new file mode 100644 index 0000000000..3d96b8f279 --- /dev/null +++ b/filters/diag/main_test.go @@ -0,0 +1,12 @@ +package diag + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/filters/ratelimit/fail_closed_test.go b/filters/ratelimit/fail_closed_test.go index f4c7c6cc2f..3c64684184 100644 --- a/filters/ratelimit/fail_closed_test.go +++ b/filters/ratelimit/fail_closed_test.go @@ -120,8 +120,9 @@ func TestFailureMode(t *testing.T) { PostProcessors: []routing.PostProcessor{ fratelimit.NewFailClosedPostProcessor(), }, - }, - r) + }, r) + defer proxy.Close() + reqURL, err := url.Parse(proxy.URL) if err != nil { t.Fatalf("Failed to parse url %s: %v", proxy.URL, err) diff --git a/filters/ratelimit/main_test.go b/filters/ratelimit/main_test.go new file mode 100644 index 0000000000..747dc9c18b --- /dev/null +++ b/filters/ratelimit/main_test.go @@ -0,0 +1,12 @@ +package ratelimit + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/filters/scheduler/fifo_test.go b/filters/scheduler/fifo_test.go index 3aa3142261..ba6c3a175a 100644 --- a/filters/scheduler/fifo_test.go +++ b/filters/scheduler/fifo_test.go @@ -239,6 +239,7 @@ func TestFifo(t *testing.T) { w.WriteHeader(http.StatusOK) w.Write([]byte("OK")) })) + defer backend.Close() var fmtStr string switch len(tt.args) { @@ -262,6 +263,8 @@ func TestFifo(t *testing.T) { if err != nil { t.Fatalf("Failed to create testdataclient: %v", err) } + defer dc.Close() + ro := routing.Options{ SignalFirstLoad: true, FilterRegistry: fr, @@ -291,6 +294,8 @@ func TestFifo(t *testing.T) { if err != nil { t.Fatalf("Failed to get response from %s: %v", reqURL.String(), err) } + defer rsp.Body.Close() + if rsp.StatusCode != http.StatusOK { t.Fatalf("Failed to get valid response from endpoint: %d", rsp.StatusCode) } @@ -402,6 +407,7 @@ func TestConstantRouteUpdatesFifo(t *testing.T) { w.WriteHeader(http.StatusOK) w.Write([]byte("OK")) })) + defer backend.Close() args := append(tt.args, backend.URL) doc := fmt.Sprintf(`aroute: * -> fifo(%v, %v, "%v") -> "%s"`, args...) @@ -410,6 +416,8 @@ func TestConstantRouteUpdatesFifo(t *testing.T) { if err != nil { t.Fatalf("Failed to create testdataclient: %v", err) } + defer dc.Close() + ro := routing.Options{ SignalFirstLoad: true, FilterRegistry: fr, @@ -439,6 +447,8 @@ func TestConstantRouteUpdatesFifo(t *testing.T) { if err != nil { t.Fatalf("Failed to get response from %s: %v", reqURL.String(), err) } + defer rsp.Body.Close() + if rsp.StatusCode != http.StatusOK { t.Fatalf("Failed to get valid response from endpoint: %d", rsp.StatusCode) } diff --git a/filters/scheduler/lifo_test.go b/filters/scheduler/lifo_test.go index d2fdb7fb9e..1d61e89d2b 100644 --- a/filters/scheduler/lifo_test.go +++ b/filters/scheduler/lifo_test.go @@ -344,6 +344,7 @@ func TestNewLIFO(t *testing.T) { w.WriteHeader(tt.wantCode) w.Write([]byte("Hello")) })) + defer backend.Close() args := append(tt.args, backend.URL) args = append([]interface{}{l.Name()}, args...) @@ -370,6 +371,7 @@ func TestNewLIFO(t *testing.T) { if err != nil { t.Fatalf("Failed to create testdataclient: %v", err) } + defer dc.Close() metrics := &metricstest.MockMetrics{} reg := scheduler.RegistryWith(scheduler.Options{ @@ -482,6 +484,7 @@ func TestLifoErrors(t *testing.T) { dc, err := testdataclient.NewDoc(doc) require.NoError(t, err) + defer dc.Close() metrics := &metricstest.MockMetrics{} reg := scheduler.RegistryWith(scheduler.Options{ diff --git a/filters/scheduler/loopback_test.go b/filters/scheduler/loopback_test.go index 56780ca88a..d02039408c 100644 --- a/filters/scheduler/loopback_test.go +++ b/filters/scheduler/loopback_test.go @@ -24,6 +24,7 @@ func TestFifoLoopback(t *testing.T) { loop: Path("/loop") -> fifo(100, 100, "1s") -> inlineContent("ok") -> ; `) require.NoError(t, err) + defer dc.Close() filterRegistry := make(filters.Registry) filterRegistry.Register(fifo.NewFifo()) @@ -40,9 +41,13 @@ func TestFifoLoopback(t *testing.T) { PostProcessors: []routing.PostProcessor{schedulerRegistry}, SignalFirstLoad: true, }) + defer rt.Close() + <-rt.FirstLoad() pr := proxy.WithParams(proxy.Params{Routing: rt}) + defer pr.Close() + tsp := httptest.NewServer(pr) defer tsp.Close() diff --git a/filters/scheduler/main_test.go b/filters/scheduler/main_test.go new file mode 100644 index 0000000000..ee147de88f --- /dev/null +++ b/filters/scheduler/main_test.go @@ -0,0 +1,12 @@ +package scheduler + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/filters/tee/main_test.go b/filters/tee/main_test.go new file mode 100644 index 0000000000..d281e2822c --- /dev/null +++ b/filters/tee/main_test.go @@ -0,0 +1,12 @@ +package tee + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/filters/tee/teeloopback_test.go b/filters/tee/teeloopback_test.go index 0332b82dd5..aa1a58cdb0 100644 --- a/filters/tee/teeloopback_test.go +++ b/filters/tee/teeloopback_test.go @@ -12,6 +12,7 @@ import ( "github.com/zalando/skipper/predicates/source" teePredicate "github.com/zalando/skipper/predicates/tee" "github.com/zalando/skipper/predicates/traffic" + "github.com/zalando/skipper/proxy" "github.com/zalando/skipper/proxy/backendtest" "github.com/zalando/skipper/proxy/proxytest" "github.com/zalando/skipper/routing" @@ -52,6 +53,8 @@ func TestLoopbackAndMatchPredicate(t *testing.T) { traffic.New(), }, }, routes...) + defer p.Close() + _, err := http.Get(p.URL + "/foo") if err != nil { t.Error("teeloopback: failed to execute the request.", err) @@ -73,18 +76,29 @@ func TestOriginalBackendServeEvenWhenShadowDoesNotReply(t *testing.T) { ` original := backendtest.NewBackendRecorder(listenFor) split := backendtest.NewBackendRecorder(listenFor) + + const responseTimeout = 2 * time.Second shadow := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - time.Sleep(time.Second * 120) + time.Sleep(2 * responseTimeout) })) + defer shadow.Close() + routes, _ := eskip.Parse(fmt.Sprintf(routeDoc, split.GetURL(), split.GetURL(), shadow.URL)) registry := make(filters.Registry) registry.Register(NewTeeLoopback()) - p := proxytest.WithRoutingOptions(registry, routing.Options{ - Predicates: []routing.PredicateSpec{ - teePredicate.New(), - traffic.New(), + p := proxytest.WithParamsAndRoutingOptions(registry, + proxy.Params{ + ResponseHeaderTimeout: responseTimeout, + CloseIdleConnsPeriod: -time.Second, }, - }, routes...) + routing.Options{ + Predicates: []routing.PredicateSpec{ + teePredicate.New(), + traffic.New(), + }, + }, routes...) + defer p.Close() + _, err := http.Get(p.URL + "/foo") if err != nil { t.Error("teeloopback: failed to execute the request.", err) @@ -116,6 +130,8 @@ func TestOriginalBackendServeEvenWhenShadowIsDown(t *testing.T) { traffic.New(), }, }, routes...) + defer p.Close() + _, err := http.Get(p.URL + "/foo") if err != nil { t.Error("teeloopback: failed to execute the request.", err) @@ -143,6 +159,8 @@ func TestInfiniteLoopback(t *testing.T) { teePredicate.New(), }, }, routes...) + defer p.Close() + _, err := http.Get(p.URL + "/foo") if err != nil { t.Error("teeloopback: failed to execute the request.", err) @@ -177,6 +195,7 @@ func TestLoopbackWithClientIP(t *testing.T) { source.NewClientIP(), }, }, routes...) + defer p.Close() rsp, err := http.Get(p.URL + "/foo") if err != nil { diff --git a/proxy/proxytest/proxytest.go b/proxy/proxytest/proxytest.go index 9991e80dff..583240f0f3 100644 --- a/proxy/proxytest/proxytest.go +++ b/proxy/proxytest/proxytest.go @@ -16,6 +16,7 @@ import ( type TestProxy struct { URL string Log *loggingtest.Logger + dc *testdataclient.Client routing *routing.Routing proxy *proxy.Proxy server *httptest.Server @@ -35,9 +36,10 @@ func WithParams(fr filters.Registry, proxyParams proxy.Params, routes ...*eskip. func newTestProxy(fr filters.Registry, routingOptions routing.Options, proxyParams proxy.Params, routes ...*eskip.Route) *TestProxy { tl := loggingtest.New() + var dc *testdataclient.Client if len(routingOptions.DataClients) == 0 { - dc := testdataclient.New(routes) + dc = testdataclient.New(routes) routingOptions.DataClients = []routing.DataClient{dc} } @@ -58,6 +60,7 @@ func newTestProxy(fr filters.Registry, routingOptions routing.Options, proxyPara return &TestProxy{ URL: tsp.URL, Log: tl, + dc: dc, routing: rt, proxy: pr, server: tsp, @@ -70,13 +73,11 @@ func New(fr filters.Registry, routes ...*eskip.Route) *TestProxy { func (p *TestProxy) Close() error { p.Log.Close() - p.routing.Close() - - err := p.proxy.Close() - if err != nil { - return err + if p.dc != nil { + p.dc.Close() } - + p.routing.Close() p.server.Close() - return nil + + return p.proxy.Close() } diff --git a/routing/datasource_test.go b/routing/datasource_test.go index 38936fe8e5..8570406dc9 100644 --- a/routing/datasource_test.go +++ b/routing/datasource_test.go @@ -44,6 +44,7 @@ func TestNoMultipleTreePredicates(t *testing.T) { return } + defer dc.Close() defs, err := dc.LoadAll() if err != nil { @@ -103,6 +104,7 @@ func TestProcessRouteDefErrors(t *testing.T) { return } + defer dc.Close() defs, err := dc.LoadAll() if err != nil { @@ -163,6 +165,7 @@ func TestProcessRouteDefWeight(t *testing.T) { return } + defer dc.Close() defs, err := dc.LoadAll() if err != nil { @@ -221,6 +224,7 @@ func TestLogging(t *testing.T) { t.Error(err) return } + defer client.Close() testLog := loggingtest.New() defer testLog.Close() diff --git a/routing/example_test.go b/routing/example_test.go index 124a90edeb..a539c83add 100644 --- a/routing/example_test.go +++ b/routing/example_test.go @@ -32,12 +32,14 @@ func Example() { if err != nil { log.Fatal(err) } + defer dataClient.Close() // create a router: r := routing.New(routing.Options{ FilterRegistry: builtin.MakeRegistry(), MatchingOptions: routing.IgnoreTrailingSlash, DataClients: []routing.DataClient{dataClient}}) + defer r.Close() // let the route data get propagated in the background: time.Sleep(36 * time.Millisecond) diff --git a/routing/main_test.go b/routing/main_test.go new file mode 100644 index 0000000000..6c32191fbf --- /dev/null +++ b/routing/main_test.go @@ -0,0 +1,12 @@ +package routing + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +} diff --git a/routing/predicates_test.go b/routing/predicates_test.go index c21ce144d3..6e991402c6 100644 --- a/routing/predicates_test.go +++ b/routing/predicates_test.go @@ -1110,6 +1110,7 @@ func TestPredicateList(t *testing.T) { }} { t.Run(test.title, func(t *testing.T) { dc := testdataclient.New(test.routes) + defer dc.Close() l := loggingtest.New() l.Unmute() diff --git a/routing/routing_test.go b/routing/routing_test.go index 868258672d..ea6f21b0f0 100644 --- a/routing/routing_test.go +++ b/routing/routing_test.go @@ -135,6 +135,8 @@ func stringsAreSame(xs, ys []string) bool { func TestKeepsReceivingInitialRouteDataUntilSucceeds(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc.Close() + dc.FailNext() dc.FailNext() dc.FailNext() @@ -154,6 +156,8 @@ func TestKeepsReceivingInitialRouteDataUntilSucceeds(t *testing.T) { func TestReceivesInitial(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) @@ -168,6 +172,8 @@ func TestReceivesInitial(t *testing.T) { func TestReceivesFullOnFailedUpdate(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) @@ -192,6 +198,8 @@ func TestReceivesFullOnFailedUpdate(t *testing.T) { func TestReceivesUpdate(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) @@ -217,6 +225,8 @@ func TestReceivesDelete(t *testing.T) { dc := testdataclient.New([]*eskip.Route{ {Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}, {Id: "route2", Path: "/some-other", Backend: "https://other.example.org"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) @@ -240,12 +250,13 @@ func TestReceivesDelete(t *testing.T) { func TestUpdateDoesNotChangeRouting(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) return } - defer tr.close() tr.log.Reset() @@ -263,8 +274,14 @@ func TestUpdateDoesNotChangeRouting(t *testing.T) { func TestMergesMultipleSources(t *testing.T) { dc1 := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc1.Close() + dc2 := testdataclient.New([]*eskip.Route{{Id: "route2", Path: "/some-other", Backend: "https://other.example.org"}}) + defer dc2.Close() + dc3 := testdataclient.New([]*eskip.Route{{Id: "route3", Path: "/another", Backend: "https://another.example.org"}}) + defer dc3.Close() + tr, err := newTestRouting(dc1, dc2, dc3) if err != nil { t.Error(err) @@ -288,8 +305,14 @@ func TestMergesMultipleSources(t *testing.T) { func TestMergesUpdatesFromMultipleSources(t *testing.T) { dc1 := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "https://www.example.org"}}) + defer dc1.Close() + dc2 := testdataclient.New([]*eskip.Route{{Id: "route2", Path: "/some-other", Backend: "https://other.example.org"}}) + defer dc2.Close() + dc3 := testdataclient.New([]*eskip.Route{{Id: "route3", Path: "/another", Backend: "https://another.example.org"}}) + defer dc3.Close() + tr, err := newTestRouting(dc1, dc2, dc3) if err != nil { t.Error(err) @@ -336,6 +359,8 @@ func TestMergesUpdatesFromMultipleSources(t *testing.T) { func TestIgnoresInvalidBackend(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{Id: "route1", Path: "/some-path", Backend: "invalid backend"}}) + defer dc.Close() + tr, err := newTestRouting(dc) if err != nil { t.Error(err) @@ -358,6 +383,7 @@ func TestProcessesFilterDefinitions(t *testing.T) { Path: "/some-path", Filters: []*eskip.Filter{{Name: "filter1", Args: []interface{}{3.14, "Hello, world!"}}}, Backend: "https://www.example.org"}}) + defer dc.Close() tr, err := newTestRoutingWithFilters(fr, dc) if err != nil { @@ -392,6 +418,7 @@ func TestProcessesPredicates(t *testing.T) { t.Error(err) return } + defer dc.Close() cps := []routing.PredicateSpec{&predicate{}, &predicate{}} @@ -467,8 +494,12 @@ func TestNonMatchedStaticRoute(t *testing.T) { } func TestRoutingHandlerParameterChecking(t *testing.T) { + rt := routing.New(routing.Options{}) + defer rt.Close() + mux := http.NewServeMux() - mux.Handle("/", routing.New(routing.Options{})) + mux.Handle("/", rt) + server := httptest.NewServer(mux) defer server.Close() @@ -522,6 +553,7 @@ func TestRoutingHandlerEskipResponse(t *testing.T) { t.Error(err) return } + defer dc.Close() cps := []routing.PredicateSpec{&predicate{}, &predicate{}} @@ -583,6 +615,8 @@ func TestRoutingHandlerJsonResponse(t *testing.T) { route1: CustomPredicate("custom1") -> "https://route1.example.org"; route2: CustomPredicate("custom2") -> "https://route2.example.org"; catchAll: * -> "https://route.example.org"`) + defer dc.Close() + cps := []routing.PredicateSpec{&predicate{}, &predicate{}} tr, _ := newTestRoutingWithPredicates(cps, dc) defer tr.close() @@ -623,6 +657,8 @@ func TestRoutingHandlerFilterInvalidRoutes(t *testing.T) { route1: CustomPredicate("custom1") -> "https://route1.example.org"; route2: FooBar("custom2") -> "https://route2.example.org"; catchAll: * -> "https://route.example.org"`) + defer dc.Close() + cps := []routing.PredicateSpec{&predicate{}, &predicate{}} tr, _ := newTestRoutingWithPredicates(cps, dc) defer tr.close() @@ -661,6 +697,7 @@ func TestRoutingHandlerPagination(t *testing.T) { route2: CustomPredicate("custom2") -> "https://route2.example.org"; catchAll: * -> "https://route.example.org" `) + defer dc.Close() cps := []routing.PredicateSpec{&predicate{}, &predicate{}} tr, _ := newTestRoutingWithPredicates(cps, dc) @@ -711,6 +748,7 @@ func TestRoutingHandlerHEAD(t *testing.T) { route2: CustomPredicate("custom2") -> "https://route2.example.org"; catchAll: * -> "https://route.example.org" `) + defer dc.Close() cps := []routing.PredicateSpec{&predicate{}, &predicate{}} tr, err := newTestRoutingWithPredicates(cps, dc) @@ -768,6 +806,7 @@ func TestUpdateFailsRecovers(t *testing.T) { if err != nil { t.Fatal(err) } + defer dc.Close() rt := routing.New(routing.Options{ FilterRegistry: builtin.MakeRegistry(), @@ -825,6 +864,7 @@ func TestUpdateFailsRecovers(t *testing.T) { func TestSignalFirstLoad(t *testing.T) { t.Run("disabled", func(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{}}) + defer dc.Close() l := loggingtest.New() defer l.Close() @@ -835,6 +875,7 @@ func TestSignalFirstLoad(t *testing.T) { PollTimeout: 12 * time.Millisecond, Log: l, }) + defer rt.Close() select { case <-rt.FirstLoad(): @@ -849,6 +890,7 @@ func TestSignalFirstLoad(t *testing.T) { t.Run("enabled", func(t *testing.T) { dc := testdataclient.New([]*eskip.Route{{}}) + defer dc.Close() l := loggingtest.New() defer l.Close() @@ -860,6 +902,7 @@ func TestSignalFirstLoad(t *testing.T) { PollTimeout: 12 * time.Millisecond, Log: l, }) + defer rt.Close() select { case <-rt.FirstLoad(): @@ -880,6 +923,7 @@ func TestSignalFirstLoad(t *testing.T) { t.Run("enabled, empty", func(t *testing.T) { dc := testdataclient.New(nil) + defer dc.Close() l := loggingtest.New() defer l.Close() @@ -891,6 +935,7 @@ func TestSignalFirstLoad(t *testing.T) { PollTimeout: 12 * time.Millisecond, Log: l, }) + defer rt.Close() select { case <-rt.FirstLoad(): @@ -911,7 +956,10 @@ func TestSignalFirstLoad(t *testing.T) { t.Run("multiple data clients", func(t *testing.T) { dc1 := testdataclient.New([]*eskip.Route{{}}) + defer dc1.Close() + dc2 := testdataclient.New([]*eskip.Route{{}}) + defer dc2.Close() l := loggingtest.New() defer l.Close() @@ -923,6 +971,7 @@ func TestSignalFirstLoad(t *testing.T) { PollTimeout: 12 * time.Millisecond, Log: l, }) + defer rt.Close() select { case <-rt.FirstLoad(): diff --git a/routing/subtreeconflict_test.go b/routing/subtreeconflict_test.go index 28730adf25..9f7461ee6e 100644 --- a/routing/subtreeconflict_test.go +++ b/routing/subtreeconflict_test.go @@ -196,6 +196,7 @@ func TestSubtreeConflict(t *testing.T) { t.Error(err) return } + defer dc.Close() r := routing.New(routing.Options{ FilterRegistry: builtin.MakeRegistry(), @@ -203,6 +204,7 @@ func TestSubtreeConflict(t *testing.T) { Log: log, MatchingOptions: def.options, }) + defer r.Close() if err := log.WaitFor("applied", 12*time.Millisecond); err != nil { t.Error(err) diff --git a/routing/testdataclient/dataclient.go b/routing/testdataclient/dataclient.go index d3fee74fdb..3f80310d4b 100644 --- a/routing/testdataclient/dataclient.go +++ b/routing/testdataclient/dataclient.go @@ -25,6 +25,7 @@ type Client struct { deletedIDs []string failNext int signalUpdate chan incomingUpdate + quit chan struct{} } // Creates a Client with an initial set of route definitions. @@ -37,6 +38,7 @@ func New(initial []*eskip.Route) *Client { return &Client{ routes: routes, signalUpdate: make(chan incomingUpdate), + quit: make(chan struct{}), } } @@ -70,8 +72,12 @@ func (c *Client) LoadAll() ([]*eskip.Route, error) { // Returns the route definitions upserted/deleted since the last call to // LoadAll. func (c *Client) LoadUpdate() ([]*eskip.Route, []string, error) { - update := <-c.signalUpdate - c.upsert, c.deletedIDs = update.upsert, update.deletedIDs + select { + case update := <-c.signalUpdate: + c.upsert, c.deletedIDs = update.upsert, update.deletedIDs + case <-c.quit: + return nil, nil, nil + } for _, id := range c.deletedIDs { delete(c.routes, id) @@ -121,3 +127,7 @@ func (c *Client) UpdateDoc(upsertDoc string, deletedIDs []string) error { func (c *Client) FailNext() { c.failNext++ } + +func (c *Client) Close() { + close(c.quit) +} diff --git a/routing/tree_test.go b/routing/tree_test.go index 2ae05ff918..8577ae1bfa 100644 --- a/routing/tree_test.go +++ b/routing/tree_test.go @@ -45,6 +45,7 @@ func createPathMatchRouting(routes string, opts MatchingOptions) (*pathTestRouti if err != nil { return nil, err } + defer dc.Close() logger := loggingtest.New() rt := New(Options{MatchingOptions: opts, DataClients: []DataClient{dc}, Log: logger}) diff --git a/secrets/main_test.go b/secrets/main_test.go new file mode 100644 index 0000000000..3c4a337a3d --- /dev/null +++ b/secrets/main_test.go @@ -0,0 +1,12 @@ +package secrets + +import ( + "os" + "testing" + + "github.com/AlexanderYastrebov/noleak" +) + +func TestMain(m *testing.M) { + os.Exit(noleak.CheckMain(m)) +}