testing: remove unnecessary calls to freeport
Previously we believe it was necessary for all code that required ports to use freeport to prevent conflicts. https://github.com/dnephin/freeport-test shows that it is actually save to use port 0 (`127.0.0.1:0`) as long as it is passed directly to `net.Listen`, and the listener holds the port for as long as it is needed. This works because freeport explicitly avoids the ephemeral port range, and port 0 always uses that range. As you can see from the test output of https://github.com/dnephin/freeport-test, the two systems never use overlapping ports. This commit converts all uses of freeport that were being passed directly to a net.Listen to use port 0 instead. This allows us to remove a bit of wrapping we had around httptest, in a couple places.
This commit is contained in:
parent
59a7fb5ca9
commit
4f0d092c95
|
@ -18,7 +18,6 @@ import (
|
|||
"github.com/hashicorp/consul/agent/consul/authmethod/testauth"
|
||||
"github.com/hashicorp/consul/agent/structs"
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/consul/testrpc"
|
||||
)
|
||||
|
@ -1658,7 +1657,7 @@ func TestACLEndpoint_LoginLogout_jwt(t *testing.T) {
|
|||
testrpc.WaitForLeader(t, a.RPC, "dc1")
|
||||
|
||||
// spin up a fake oidc server
|
||||
oidcServer := oidcauthtest.Start(t, oidcauthtest.WithPort(freeport.Port(t)))
|
||||
oidcServer := oidcauthtest.Start(t)
|
||||
pubKey, privKey := oidcServer.SigningKeys()
|
||||
|
||||
type mConfig = map[string]interface{}
|
||||
|
|
|
@ -1734,10 +1734,12 @@ func TestAgent_RestoreServiceWithAliasCheck(t *testing.T) {
|
|||
a := StartTestAgent(t, TestAgent{HCL: cfg})
|
||||
defer a.Shutdown()
|
||||
|
||||
testCtx, testCancel := context.WithCancel(context.Background())
|
||||
defer testCancel()
|
||||
|
||||
testHTTPServer := launchHTTPCheckServer(t, testCtx)
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte("OK\n"))
|
||||
})
|
||||
testHTTPServer := httptest.NewServer(handler)
|
||||
t.Cleanup(testHTTPServer.Close)
|
||||
|
||||
registerServicesAndChecks := func(t *testing.T, a *TestAgent) {
|
||||
// add one persistent service with a simple check
|
||||
|
@ -1842,27 +1844,6 @@ node_name = "` + a.Config.NodeName + `"
|
|||
}
|
||||
}
|
||||
|
||||
func launchHTTPCheckServer(t *testing.T, ctx context.Context) *httptest.Server {
|
||||
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t)))
|
||||
|
||||
var lc net.ListenConfig
|
||||
listener, err := lc.Listen(ctx, "tcp", addr)
|
||||
require.NoError(t, err)
|
||||
|
||||
handler := http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
w.WriteHeader(http.StatusOK)
|
||||
_, _ = w.Write([]byte("OK\n"))
|
||||
})
|
||||
|
||||
srv := &httptest.Server{
|
||||
Listener: listener,
|
||||
Config: &http.Server{Handler: handler},
|
||||
}
|
||||
srv.Start()
|
||||
t.Cleanup(srv.Close)
|
||||
return srv
|
||||
}
|
||||
|
||||
func TestAgent_AddCheck_Alias(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("too slow for testing.Short")
|
||||
|
|
|
@ -20,7 +20,6 @@ import (
|
|||
"github.com/hashicorp/consul/agent/consul/authmethod/testauth"
|
||||
"github.com/hashicorp/consul/agent/structs"
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/consul/sdk/testutil/retry"
|
||||
)
|
||||
|
@ -4868,7 +4867,7 @@ func TestACLEndpoint_Login_jwt(t *testing.T) {
|
|||
acl := ACL{srv: srv}
|
||||
|
||||
// spin up a fake oidc server
|
||||
oidcServer := oidcauthtest.Start(t, oidcauthtest.WithPort(freeport.Port(t)))
|
||||
oidcServer := oidcauthtest.Start(t)
|
||||
pubKey, privKey := oidcServer.SigningKeys()
|
||||
|
||||
type mConfig = map[string]interface{}
|
||||
|
|
|
@ -4,20 +4,16 @@ import (
|
|||
"bytes"
|
||||
"encoding/json"
|
||||
"encoding/pem"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"log"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/mitchellh/go-testing-interface"
|
||||
"github.com/stretchr/testify/require"
|
||||
authv1 "k8s.io/api/authentication/v1"
|
||||
|
@ -47,7 +43,7 @@ type TestAPIServer struct {
|
|||
// random free port.
|
||||
func StartTestAPIServer(t testing.T) *TestAPIServer {
|
||||
s := &TestAPIServer{}
|
||||
s.srv = httptestNewUnstartedServerWithPort(s, freeport.Port(t))
|
||||
s.srv = httptest.NewUnstartedServer(s)
|
||||
s.srv.Config.ErrorLog = log.New(ioutil.Discard, "", 0)
|
||||
s.srv.StartTLS()
|
||||
|
||||
|
@ -535,19 +531,3 @@ func createStatus(status, message string, reason metav1.StatusReason, details *m
|
|||
Code: code,
|
||||
}
|
||||
}
|
||||
|
||||
func httptestNewUnstartedServerWithPort(handler http.Handler, port int) *httptest.Server {
|
||||
if port == 0 {
|
||||
return httptest.NewUnstartedServer(handler)
|
||||
}
|
||||
addr := net.JoinHostPort("127.0.0.1", strconv.Itoa(port))
|
||||
l, err := net.Listen("tcp", addr)
|
||||
if err != nil {
|
||||
panic(fmt.Sprintf("httptest: failed to listen on a port: %v", err))
|
||||
}
|
||||
|
||||
return &httptest.Server{
|
||||
Listener: l,
|
||||
Config: &http.Server{Handler: handler},
|
||||
}
|
||||
}
|
||||
|
|
|
@ -5,14 +5,14 @@ import (
|
|||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/consul/agent/consul/authmethod"
|
||||
"github.com/hashicorp/consul/agent/structs"
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/go-hclog"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/square/go-jose.v2/jwt"
|
||||
|
||||
"github.com/hashicorp/consul/agent/consul/authmethod"
|
||||
"github.com/hashicorp/consul/agent/structs"
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
)
|
||||
|
||||
func TestJWT_NewValidator(t *testing.T) {
|
||||
|
@ -32,7 +32,7 @@ func TestJWT_NewValidator(t *testing.T) {
|
|||
return method
|
||||
}
|
||||
|
||||
oidcServer := oidcauthtest.Start(t, oidcauthtest.WithPort(freeport.Port(t)))
|
||||
oidcServer := oidcauthtest.Start(t)
|
||||
|
||||
// Note that we won't test ALL of the available config variations here.
|
||||
// The go-sso library has exhaustive tests.
|
||||
|
@ -110,7 +110,7 @@ func TestJWT_ValidateLogin(t *testing.T) {
|
|||
return v
|
||||
}
|
||||
|
||||
oidcServer := oidcauthtest.Start(t, oidcauthtest.WithPort(freeport.Port(t)))
|
||||
oidcServer := oidcauthtest.Start(t)
|
||||
pubKey, privKey := oidcServer.SigningKeys()
|
||||
|
||||
cases := map[string]struct {
|
||||
|
|
|
@ -512,12 +512,11 @@ func TestServer_JoinWAN_SerfAllowedCIDRs(t *testing.T) {
|
|||
}
|
||||
|
||||
func skipIfCannotBindToIP(t *testing.T, ip string) {
|
||||
addr := ipaddr.FormatAddressPort(ip, freeport.Port(t))
|
||||
l, err := net.Listen("tcp", addr)
|
||||
l.Close()
|
||||
l, err := net.Listen("tcp", net.JoinHostPort(ip, "0"))
|
||||
if err != nil {
|
||||
t.Skipf("Cannot bind on %s, to run on Mac OS: `sudo ifconfig lo0 alias %s up`", ip, ip)
|
||||
}
|
||||
l.Close()
|
||||
}
|
||||
|
||||
func TestServer_LANReap(t *testing.T) {
|
||||
|
|
|
@ -4,7 +4,6 @@ import (
|
|||
"context"
|
||||
"fmt"
|
||||
"net"
|
||||
"strconv"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
@ -29,7 +28,7 @@ func useTLSForDcAlwaysTrue(_ string) bool {
|
|||
}
|
||||
|
||||
func TestNewDialer_WithTLSWrapper(t *testing.T) {
|
||||
lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t))))
|
||||
lis, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(logError(t, lis.Close))
|
||||
|
||||
|
@ -65,25 +64,18 @@ func TestNewDialer_WithTLSWrapper(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestNewDialer_WithALPNWrapper(t *testing.T) {
|
||||
ports := freeport.GetN(t, 3)
|
||||
|
||||
var (
|
||||
s1addr = ipaddr.FormatAddressPort("127.0.0.1", ports[0])
|
||||
s2addr = ipaddr.FormatAddressPort("127.0.0.1", ports[1])
|
||||
gwAddr = ipaddr.FormatAddressPort("127.0.0.1", ports[2])
|
||||
)
|
||||
|
||||
lis1, err := net.Listen("tcp", s1addr)
|
||||
lis1, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(logError(t, lis1.Close))
|
||||
|
||||
lis2, err := net.Listen("tcp", s2addr)
|
||||
lis2, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
t.Cleanup(logError(t, lis2.Close))
|
||||
|
||||
// Send all of the traffic to dc2's server
|
||||
var p tcpproxy.Proxy
|
||||
p.AddRoute(gwAddr, tcpproxy.To(s2addr))
|
||||
gwAddr := ipaddr.FormatAddressPort("127.0.0.1", freeport.Port(t))
|
||||
p.AddRoute(gwAddr, tcpproxy.To(lis2.Addr().String()))
|
||||
p.AddStopACMESearch(gwAddr)
|
||||
require.NoError(t, p.Start())
|
||||
defer func() {
|
||||
|
|
|
@ -6,7 +6,6 @@ import (
|
|||
"fmt"
|
||||
"io"
|
||||
"net"
|
||||
"strconv"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
@ -18,7 +17,6 @@ import (
|
|||
"github.com/hashicorp/consul/agent/grpc/internal/testservice"
|
||||
"github.com/hashicorp/consul/agent/metadata"
|
||||
"github.com/hashicorp/consul/agent/pool"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/hashicorp/consul/tlsutil"
|
||||
)
|
||||
|
||||
|
@ -47,7 +45,7 @@ func newTestServer(t *testing.T, name string, dc string, tlsConf *tlsutil.Config
|
|||
testservice.RegisterSimpleServer(server, &simple{name: name, dc: dc})
|
||||
})
|
||||
|
||||
lis, err := net.Listen("tcp", net.JoinHostPort("127.0.0.1", strconv.Itoa(freeport.Port(t))))
|
||||
lis, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
|
||||
rpc := &fakeRPCListener{t: t, handler: handler, tlsConf: tlsConf}
|
||||
|
|
|
@ -8,18 +8,18 @@ import (
|
|||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/mitchellh/cli"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/square/go-jose.v2/jwt"
|
||||
|
||||
"github.com/hashicorp/consul/agent"
|
||||
"github.com/hashicorp/consul/agent/consul/authmethod/kubeauth"
|
||||
"github.com/hashicorp/consul/agent/consul/authmethod/testauth"
|
||||
"github.com/hashicorp/consul/api"
|
||||
"github.com/hashicorp/consul/command/acl"
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/oidcauthtest"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/consul/testrpc"
|
||||
"github.com/mitchellh/cli"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/square/go-jose.v2/jwt"
|
||||
)
|
||||
|
||||
func TestLoginCommand_noTabs(t *testing.T) {
|
||||
|
@ -352,7 +352,7 @@ func TestLoginCommand_jwt(t *testing.T) {
|
|||
bearerTokenFile := filepath.Join(testDir, "bearer.token")
|
||||
|
||||
// spin up a fake oidc server
|
||||
oidcServer := oidcauthtest.Start(t, oidcauthtest.WithPort(freeport.Port(t)))
|
||||
oidcServer := oidcauthtest.Start(t)
|
||||
pubKey, privKey := oidcServer.SigningKeys()
|
||||
|
||||
type mConfig = map[string]interface{}
|
||||
|
|
|
@ -5,19 +5,20 @@ import (
|
|||
"fmt"
|
||||
"io/ioutil"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
|
||||
"github.com/hashicorp/consul/agent"
|
||||
"github.com/hashicorp/consul/api"
|
||||
"github.com/hashicorp/consul/lib"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/consul/sdk/testutil/retry"
|
||||
"github.com/mitchellh/cli"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/hashicorp/consul/agent"
|
||||
"github.com/hashicorp/consul/api"
|
||||
"github.com/hashicorp/consul/sdk/testutil"
|
||||
"github.com/hashicorp/consul/sdk/testutil/retry"
|
||||
)
|
||||
|
||||
func TestSnapshotSaveCommand_noTabs(t *testing.T) {
|
||||
|
@ -138,7 +139,7 @@ func TestSnapshotSaveCommand_TruncatedStream(t *testing.T) {
|
|||
var fakeResult atomic.Value
|
||||
|
||||
// Run a fake webserver to pretend to be the snapshot API.
|
||||
srv := lib.NewHTTPTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
|
||||
if req.URL.Path != "/v1/snapshot" {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
|
@ -157,6 +158,7 @@ func TestSnapshotSaveCommand_TruncatedStream(t *testing.T) {
|
|||
data := raw.([]byte)
|
||||
_, _ = w.Write(data)
|
||||
}))
|
||||
t.Cleanup(srv.Close)
|
||||
|
||||
// Wait until the server is actually listening.
|
||||
retry.Run(t, func(r *retry.R) {
|
||||
|
|
|
@ -8,10 +8,10 @@ import (
|
|||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/consul/connect"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
"github.com/mitchellh/go-testing-interface"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/hashicorp/consul/connect"
|
||||
)
|
||||
|
||||
// TestLocalAddr makes a localhost address on the given port
|
||||
|
@ -30,12 +30,10 @@ type TestTCPServer struct {
|
|||
// a TestTCPServer serving requests to it. The server is already started and can
|
||||
// be stopped by calling Close().
|
||||
func NewTestTCPServer(t testing.T) *TestTCPServer {
|
||||
addr := TestLocalAddr(freeport.Port(t))
|
||||
|
||||
l, err := net.Listen("tcp", addr)
|
||||
l, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
require.NoError(t, err)
|
||||
|
||||
log.Printf("test tcp server listening on %s", addr)
|
||||
log.Printf("test tcp server listening on %s", l.Addr())
|
||||
s := &TestTCPServer{l: l}
|
||||
go s.accept()
|
||||
|
||||
|
|
|
@ -24,10 +24,11 @@ import (
|
|||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/internal/strutil"
|
||||
"github.com/stretchr/testify/require"
|
||||
"gopkg.in/square/go-jose.v2"
|
||||
"gopkg.in/square/go-jose.v2/jwt"
|
||||
|
||||
"github.com/hashicorp/consul/internal/go-sso/oidcauth/internal/strutil"
|
||||
)
|
||||
|
||||
// Server is local server the mocks the endpoints used by the OIDC and
|
||||
|
@ -53,17 +54,6 @@ type Server struct {
|
|||
disableUserInfo bool
|
||||
}
|
||||
|
||||
type startOption struct {
|
||||
port int
|
||||
}
|
||||
|
||||
// WithPort is a option for Start that lets the caller control the port
|
||||
// allocation. The returnFunc parameter is used when the provider is stopped to
|
||||
// return the port in whatever bookkeeping system the caller wants to use.
|
||||
func WithPort(port int) startOption {
|
||||
return startOption{port: port}
|
||||
}
|
||||
|
||||
type TestingT interface {
|
||||
require.TestingT
|
||||
Helper()
|
||||
|
@ -73,7 +63,7 @@ type TestingT interface {
|
|||
// Start creates a disposable Server. If the port provided is
|
||||
// zero it will bind to a random free port, otherwise the provided port is
|
||||
// used.
|
||||
func Start(t TestingT, options ...startOption) *Server {
|
||||
func Start(t TestingT) *Server {
|
||||
t.Helper()
|
||||
s := &Server{
|
||||
allowedRedirectURIs: []string{
|
||||
|
@ -91,14 +81,7 @@ func Start(t TestingT, options ...startOption) *Server {
|
|||
require.NoError(t, err)
|
||||
s.jwks = jwks
|
||||
|
||||
var port int
|
||||
for _, option := range options {
|
||||
if option.port > 0 {
|
||||
port = option.port
|
||||
}
|
||||
}
|
||||
|
||||
s.httpServer = httptestNewUnstartedServerWithPort(s, port)
|
||||
s.httpServer = httptest.NewUnstartedServer(s)
|
||||
s.httpServer.Config.ErrorLog = log.New(ioutil.Discard, "", 0)
|
||||
s.httpServer.StartTLS()
|
||||
t.Cleanup(s.httpServer.Close)
|
||||
|
|
|
@ -1,33 +0,0 @@
|
|||
package lib
|
||||
|
||||
import (
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
|
||||
"github.com/hashicorp/consul/ipaddr"
|
||||
"github.com/hashicorp/consul/sdk/freeport"
|
||||
)
|
||||
|
||||
// NewHTTPTestServer starts and returns an httptest.Server that is listening
|
||||
// on a random port from freeport.Port. When the test case ends the server
|
||||
// will be stopped.
|
||||
//
|
||||
// We can't directly use httptest.Server here because that only thinks a port
|
||||
// is free if it's not bound. Consul tests frequently reserve ports via
|
||||
// `sdk/freeport` so you can have one part of the test try to use a port and
|
||||
// _know_ nothing is listening. If you simply assumed unbound ports were free
|
||||
// you'd end up with test cross-talk and weirdness.
|
||||
func NewHTTPTestServer(t freeport.TestingT, handler http.Handler) *httptest.Server {
|
||||
srv := httptest.NewUnstartedServer(handler)
|
||||
|
||||
addr := ipaddr.FormatAddressPort("127.0.0.1", freeport.Port(t))
|
||||
listener, err := net.Listen("tcp", addr)
|
||||
if err != nil {
|
||||
t.Fatalf("failed to listen on %v", addr)
|
||||
}
|
||||
srv.Listener = listener
|
||||
t.Cleanup(srv.Close)
|
||||
srv.Start()
|
||||
return srv
|
||||
}
|
Loading…
Reference in New Issue