diff --git a/src/code.cloudfoundry.org/gorouter/common/component_test.go b/src/code.cloudfoundry.org/gorouter/common/component_test.go index e36c0e168..3e4c7bf2a 100644 --- a/src/code.cloudfoundry.org/gorouter/common/component_test.go +++ b/src/code.cloudfoundry.org/gorouter/common/component_test.go @@ -167,8 +167,9 @@ var _ = Describe("Component", func() { var natsRunner *test_util.NATSRunner BeforeEach(func() { - natsPort := test_util.NextAvailPort() + natsPort := test_util.ReservePort() natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() mbusClient = natsRunner.MessageBus mbusClient.Opts.SkipSubjectValidation = true diff --git a/src/code.cloudfoundry.org/gorouter/handlers/reporter_test.go b/src/code.cloudfoundry.org/gorouter/handlers/reporter_test.go index 9cc4afb1c..a49a21683 100644 --- a/src/code.cloudfoundry.org/gorouter/handlers/reporter_test.go +++ b/src/code.cloudfoundry.org/gorouter/handlers/reporter_test.go @@ -88,7 +88,9 @@ var _ = Describe("Reporter Handler", func() { }) It("emits routing response metrics", func() { + before := time.Now() handler.ServeHTTP(resp, req) + after := time.Now() Expect(fakeReporter.CaptureBadGatewayCallCount()).To(Equal(0)) @@ -102,7 +104,11 @@ var _ = Describe("Reporter Handler", func() { Expect(capturedEndpoint.PrivateInstanceId).To(Equal("id")) Expect(capturedEndpoint.PrivateInstanceIndex).To(Equal("1")) Expect(capturedRespCode).To(Equal(http.StatusTeapot)) - Expect(startTime).To(BeTemporally("~", time.Now(), 100*time.Millisecond)) + // ReceivedAt is set to timeNow-1ms where timeNow is captured inside + // the handler (between before and after), so the exact bracket is: + // before-1ms <= startTime <= after-1ms + Expect(startTime).To(BeTemporally(">=", before.Add(-1*time.Millisecond))) + Expect(startTime).To(BeTemporally("<=", after.Add(-1*time.Millisecond))) Expect(latency).To(BeNumerically(">", 0)) Expect(latency).To(BeNumerically("<", 10*time.Millisecond)) diff --git a/src/code.cloudfoundry.org/gorouter/handlers/requestinfo_test.go b/src/code.cloudfoundry.org/gorouter/handlers/requestinfo_test.go index 1841aa4f6..0f363918c 100644 --- a/src/code.cloudfoundry.org/gorouter/handlers/requestinfo_test.go +++ b/src/code.cloudfoundry.org/gorouter/handlers/requestinfo_test.go @@ -59,16 +59,17 @@ var _ = Describe("RequestInfoHandler", func() { }) It("sets RequestInfo with StartTime on the context", func() { + before := time.Now() handler.ServeHTTP(resp, req, nextHandler) var contextReq *http.Request Eventually(reqChan).Should(Receive(&contextReq)) - - expectedStartTime := time.Now() + after := time.Now() ri, err := handlers.ContextRequestInfo(contextReq) Expect(err).ToNot(HaveOccurred()) Expect(ri).ToNot(BeNil()) - Expect(ri.ReceivedAt).To(BeTemporally("~", expectedStartTime, 10*time.Millisecond)) + Expect(ri.ReceivedAt).To(BeTemporally(">=", before)) + Expect(ri.ReceivedAt).To(BeTemporally("<=", after)) }) }) diff --git a/src/code.cloudfoundry.org/gorouter/integration/common_integration_test.go b/src/code.cloudfoundry.org/gorouter/integration/common_integration_test.go index 74ec5d238..41cf16cd5 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/common_integration_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/common_integration_test.go @@ -63,7 +63,9 @@ func (s *testState) SetOnlyTrustClientCACertsTrue() { func NewTestState() *testState { // TODO: don't hide so much behind these test_util methods - cfg, clientTLSConfig := test_util.SpecSSLConfig(test_util.NextAvailPort(), test_util.NextAvailPort(), test_util.NextAvailPort(), test_util.NextAvailPort(), test_util.NextAvailPort(), test_util.NextAvailPort(), test_util.NextAvailPort()) + // Use ReservePort to keep listeners open until the gorouter process + // starts, preventing other processes from grabbing these ports. + cfg, clientTLSConfig := test_util.SpecSSLConfig(test_util.ReservePort(), test_util.ReservePort(), test_util.ReservePort(), test_util.ReservePort(), test_util.ReservePort(), test_util.ReservePort(), test_util.ReservePort()) cfg.SkipSSLValidation = false cfg.RouteServicesHairpinning = false cfg.CipherString = "ECDHE-RSA-AES128-GCM-SHA256:ECDHE-RSA-AES256-GCM-SHA384" @@ -71,7 +73,7 @@ func NewTestState() *testState { // TODO: why these magic numbers? cfg.PruneStaleDropletsInterval = 2 * time.Second cfg.DropletStaleThreshold = 10 * time.Second - cfg.StartResponseDelayInterval = 1 * time.Second + cfg.StartResponseDelayInterval = 0 cfg.EndpointTimeout = 15 * time.Second cfg.EndpointDialTimeout = 500 * time.Millisecond cfg.DrainTimeout = 200 * time.Millisecond @@ -258,6 +260,10 @@ func (s *testState) registerAndWait(rm mbus.RegistryMessage) { func (s *testState) StartGorouter() *Session { Expect(s.cfg).NotTo(BeNil(), "set up test cfg before calling this function") + // Release NATS port first so the NATS server can bind it, while keeping + // the other ports reserved until the gorouter starts. + test_util.ReleasePort(s.cfg.Nats.Hosts[0].Port) + s.natsRunner = test_util.NewNATSRunner(int(s.cfg.Nats.Hosts[0].Port)) s.natsRunner.Start() @@ -271,6 +277,10 @@ func (s *testState) StartGorouter() *Session { Expect(err).ToNot(HaveOccurred()) Expect(os.WriteFile(cfgFile, cfgBytes, 0644)).To(Succeed()) + // Release remaining reserved ports just before the gorouter process + // starts, minimizing the TOCTOU window between release and bind. + test_util.ReleaseAllPorts() + cmd := exec.Command(gorouterPath, "-c", cfgFile) s.gorouterSession, err = Start(cmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) @@ -297,6 +307,12 @@ func (s *testState) StartGorouterOrFail() { } func (s *testState) StopAndCleanup() { + // Stop router before NATS to prevent subscriber's ClosedCB from + // firing log.Fatal → os.Exit(1), which kills the test proc. + if s.gorouterSession != nil && s.gorouterSession.ExitCode() == -1 { + Eventually(s.gorouterSession.Terminate(), 5).Should(Exit(0)) + } + if s.natsRunner != nil { s.natsRunner.Stop() } @@ -308,10 +324,6 @@ func (s *testState) StopAndCleanup() { os.RemoveAll(s.tmpdir) - if s.gorouterSession != nil && s.gorouterSession.ExitCode() == -1 { - Eventually(s.gorouterSession.Terminate(), 5).Should(Exit(0)) - } - if s.fakeMetron != nil { s.StopMetron() } diff --git a/src/code.cloudfoundry.org/gorouter/integration/gdpr_test.go b/src/code.cloudfoundry.org/gorouter/integration/gdpr_test.go index 521dd83b7..dbeed7c4d 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/gdpr_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/gdpr_test.go @@ -65,7 +65,7 @@ var _ = Describe("GDPR", func() { testState.EnableAccessLog() testState.cfg.Status.Pass = "pass" testState.cfg.Status.User = "user" - testState.cfg.Status.Routes.Port = 6705 + testState.cfg.Status.Routes.Port = test_util.ReservePort() testState.cfg.Logging.DisableLogForwardedFor = true testState.StartGorouterOrFail() @@ -136,7 +136,7 @@ var _ = Describe("GDPR", func() { testState.EnableAccessLog() testState.cfg.Status.Pass = "pass" testState.cfg.Status.User = "user" - testState.cfg.Status.Routes.Port = 6706 + testState.cfg.Status.Routes.Port = test_util.ReservePort() testState.cfg.Logging.DisableLogSourceIP = true testState.StartGorouterOrFail() diff --git a/src/code.cloudfoundry.org/gorouter/integration/main_test.go b/src/code.cloudfoundry.org/gorouter/integration/main_test.go index 846974a43..c6868a187 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/main_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/main_test.go @@ -65,29 +65,33 @@ var _ = Describe("Router Integration", func() { Expect(err).ToNot(HaveOccurred()) cfgFile = filepath.Join(tmpdir, "config.yml") - statusPort = test_util.NextAvailPort() - statusTLSPort = test_util.NextAvailPort() - statusRoutesPort = test_util.NextAvailPort() - proxyPort = test_util.NextAvailPort() - natsPort = test_util.NextAvailPort() - sslPort = test_util.NextAvailPort() - routeServiceServerPort = test_util.NextAvailPort() + statusPort = test_util.ReservePort() + statusTLSPort = test_util.ReservePort() + statusRoutesPort = test_util.ReservePort() + proxyPort = test_util.ReservePort() + natsPort = test_util.ReservePort() + sslPort = test_util.ReservePort() + routeServiceServerPort = test_util.ReservePort() natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() oauthServerURL = oauthServer.Addr() }) AfterEach(func() { + test_util.ReleaseAllPorts() + // Stop router before NATS to prevent subscriber's ClosedCB from + // firing log.Fatal → os.Exit(1), which kills the test proc. + if gorouterSession != nil && gorouterSession.ExitCode() == -1 { + stopGorouter(gorouterSession) + } + if natsRunner != nil { natsRunner.Stop() } os.RemoveAll(tmpdir) - - if gorouterSession != nil && gorouterSession.ExitCode() == -1 { - stopGorouter(gorouterSession) - } }) Context("when config is invalid", func() { @@ -609,6 +613,7 @@ var _ = Describe("Router Integration", func() { tempCfg.Logging.MetronAddress = "" writeConfig(tempCfg, cfgFile) + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) gorouterSession, _ = Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Eventually(gorouterSession, 5*time.Second).Should(Exit(1)) @@ -635,7 +640,7 @@ var _ = Describe("Router Integration", func() { BeforeEach(func() { testState = NewTestState() - testState.cfg.DebugAddr = fmt.Sprintf("127.0.0.1:%d", test_util.NextAvailPort()) + testState.cfg.DebugAddr = fmt.Sprintf("127.0.0.1:%d", test_util.ReservePort()) testState.StartGorouterOrFail() gorouterSession = testState.gorouterSession @@ -1047,7 +1052,7 @@ var _ = Describe("Router Integration", func() { Describe("prometheus metrics", func() { It("starts a prometheus https server", func() { c := createConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, routeServiceServerPort, cfgFile, defaultPruneInterval, defaultPruneThreshold, 0, false, 0, natsPort) - metricsPort := test_util.NextAvailPort() + metricsPort := test_util.ReservePort() serverCAPath, serverCertPath, serverKeyPath, clientCert := tls_helpers.GenerateCaAndMutualTlsCerts() c.Prometheus.Enabled = true @@ -1421,6 +1426,7 @@ var _ = Describe("Router Integration", func() { It("does not exit", func() { writeConfig(cfg, cfgFile) + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) session, err := Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) @@ -1436,6 +1442,7 @@ var _ = Describe("Router Integration", func() { It("gorouter exits with non-zero code", func() { writeConfig(cfg, cfgFile) + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) session, err := Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) @@ -1453,6 +1460,7 @@ var _ = Describe("Router Integration", func() { routingApiServer.Close() writeConfig(cfg, cfgFile) + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) session, err := Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) @@ -1468,6 +1476,7 @@ var _ = Describe("Router Integration", func() { cfg.OAuth.Port = 0 writeConfig(cfg, cfgFile) + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) session, err := Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) diff --git a/src/code.cloudfoundry.org/gorouter/integration/nats_test.go b/src/code.cloudfoundry.org/gorouter/integration/nats_test.go index 0067d21a3..e095eed60 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/nats_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/nats_test.go @@ -36,27 +36,31 @@ var _ = Describe("NATS Integration", func() { Expect(err).ToNot(HaveOccurred()) cfgFile = filepath.Join(tmpdir, "config.yml") - statusPort = test_util.NextAvailPort() - statusTLSPort = test_util.NextAvailPort() - statusRoutesPort = test_util.NextAvailPort() - proxyPort = test_util.NextAvailPort() - natsPort = test_util.NextAvailPort() - routeServiceServerPort = test_util.NextAvailPort() + statusPort = test_util.ReservePort() + statusTLSPort = test_util.ReservePort() + statusRoutesPort = test_util.ReservePort() + proxyPort = test_util.ReservePort() + natsPort = test_util.ReservePort() + routeServiceServerPort = test_util.ReservePort() natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() }) AfterEach(func() { + test_util.ReleaseAllPorts() + // Stop router before NATS to prevent subscriber's ClosedCB from + // firing log.Fatal → os.Exit(1), which kills the test proc. + if gorouterSession != nil && gorouterSession.ExitCode() == -1 { + stopGorouter(gorouterSession) + } + if natsRunner != nil { natsRunner.Stop() } os.RemoveAll(tmpdir) - - if gorouterSession != nil && gorouterSession.ExitCode() == -1 { - stopGorouter(gorouterSession) - } }) It("has Nats connectivity", func() { @@ -162,7 +166,7 @@ var _ = Describe("NATS Integration", func() { ) BeforeEach(func() { - natsPort2 = test_util.NextAvailPort() + natsPort2 = test_util.ReservePort() natsRunner2 = test_util.NewNATSRunner(int(natsPort2)) pruneInterval = 2 * time.Second @@ -206,6 +210,7 @@ var _ = Describe("NATS Integration", func() { time.Sleep(heartbeatInterval * 2) natsRunner.Stop() + test_util.ReleasePort(natsPort2) natsRunner2.Start() // Give router time to make a bad decision (i.e. prune routes) @@ -222,7 +227,7 @@ var _ = Describe("NATS Integration", func() { Context("when suspend_pruning_if_nats_unavailable enabled", func() { BeforeEach(func() { - natsPort2 = test_util.NextAvailPort() + natsPort2 = test_util.ReservePort() natsRunner2 = test_util.NewNATSRunner(int(natsPort2)) pruneInterval = 200 * time.Millisecond diff --git a/src/code.cloudfoundry.org/gorouter/integration/test_utils_test.go b/src/code.cloudfoundry.org/gorouter/integration/test_utils_test.go index c8d8d0978..3b3a8c65b 100644 --- a/src/code.cloudfoundry.org/gorouter/integration/test_utils_test.go +++ b/src/code.cloudfoundry.org/gorouter/integration/test_utils_test.go @@ -46,7 +46,7 @@ func configDrainSetup(cfg *config.Config, pruneInterval, pruneThreshold time.Dur // as part of pausing cfg.PruneStaleDropletsInterval = pruneInterval cfg.DropletStaleThreshold = pruneThreshold - cfg.StartResponseDelayInterval = 1 * time.Second + cfg.StartResponseDelayInterval = 0 cfg.EndpointTimeout = 5 * time.Second cfg.EndpointDialTimeout = 500 * time.Millisecond cfg.DrainTimeout = 200 * time.Millisecond @@ -60,6 +60,7 @@ func writeConfig(cfg *config.Config, cfgFile string) { } func startGorouterSession(cfgFile string) *Session { + test_util.ReleaseAllPorts() gorouterCmd := exec.Command(gorouterPath, "-c", cfgFile) session, err := Start(gorouterCmd, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) diff --git a/src/code.cloudfoundry.org/gorouter/logger/logger.go b/src/code.cloudfoundry.org/gorouter/logger/logger.go index d41c57144..cb5aedba3 100644 --- a/src/code.cloudfoundry.org/gorouter/logger/logger.go +++ b/src/code.cloudfoundry.org/gorouter/logger/logger.go @@ -1,6 +1,7 @@ package logger import ( + "fmt" "io" "log/slog" "os" @@ -235,6 +236,8 @@ via os.Exit(1) after writing the log message. */ func Fatal(logger *slog.Logger, message string, slogAttrs ...any) { logger.Error(message, slogAttrs...) + // Write to stderr so the message survives os.Exit (stderr is unbuffered). + fmt.Fprintf(os.Stderr, "FATAL: %s %v\n", message, slogAttrs) os.Exit(1) } diff --git a/src/code.cloudfoundry.org/gorouter/mbus/subscriber_test.go b/src/code.cloudfoundry.org/gorouter/mbus/subscriber_test.go index 790307f7e..836ff36bc 100644 --- a/src/code.cloudfoundry.org/gorouter/mbus/subscriber_test.go +++ b/src/code.cloudfoundry.org/gorouter/mbus/subscriber_test.go @@ -38,9 +38,9 @@ var _ = Describe("Subscriber", func() { ) BeforeEach(func() { - natsPort = test_util.NextAvailPort() - + natsPort = test_util.ReservePort() natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() natsClient = natsRunner.MessageBus @@ -60,13 +60,13 @@ var _ = Describe("Subscriber", func() { }) AfterEach(func() { - if natsRunner != nil { - natsRunner.Stop() - } if process != nil { process.Signal(os.Interrupt) } process = nil + if natsRunner != nil { + natsRunner.Stop() + } }) It("exits when signaled", func() { diff --git a/src/code.cloudfoundry.org/gorouter/proxy/proxy_test.go b/src/code.cloudfoundry.org/gorouter/proxy/proxy_test.go index 5d5842181..85bb6c470 100644 --- a/src/code.cloudfoundry.org/gorouter/proxy/proxy_test.go +++ b/src/code.cloudfoundry.org/gorouter/proxy/proxy_test.go @@ -3,8 +3,6 @@ package proxy_test import ( "bufio" "bytes" - "code.cloudfoundry.org/gorouter/proxy" - "code.cloudfoundry.org/gorouter/routeservice" "crypto/tls" "crypto/x509" "fmt" @@ -22,6 +20,9 @@ import ( "sync/atomic" "time" + "code.cloudfoundry.org/gorouter/proxy" + "code.cloudfoundry.org/gorouter/routeservice" + "github.com/cloudfoundry/dropsonde/factories" "github.com/cloudfoundry/sonde-go/events" uuid "github.com/nu7hatch/gouuid" @@ -2021,8 +2022,11 @@ var _ = Describe("Proxy", func() { Expect(body).To(Equal("ABCD")) - expectRsp := test_util.NewResponse(100) - conn.WriteResponse(expectRsp) + conn.WriteResponse(&http.Response{ + StatusCode: http.StatusContinue, + ProtoMajor: 1, + ProtoMinor: 1, + }) rsp := test_util.NewResponse(200) rsp.Body = io.NopCloser(strings.NewReader("valid-but-unimportant-response-data")) @@ -2076,8 +2080,11 @@ var _ = Describe("Proxy", func() { Expect(body).To(Equal("ABCD")) - expectRsp := test_util.NewResponse(100) - conn.WriteResponse(expectRsp) + conn.WriteResponse(&http.Response{ + StatusCode: http.StatusContinue, + ProtoMajor: 1, + ProtoMinor: 1, + }) rsp := test_util.NewResponse(201) rsp.Body = io.NopCloser(strings.NewReader("valid-but-unimportant-response-data")) @@ -2888,9 +2895,11 @@ var _ = Describe("Proxy", func() { conn := dialProxy(proxyServer) req := test_util.NewRequest("GET", "reporter-test", "/", nil) + before := time.Now() conn.WriteRequest(req) resp, _ := conn.ReadResponse() + after := time.Now() Expect(resp.StatusCode).To(Equal(http.StatusOK)) Expect(fakeReporter.CaptureBadGatewayCallCount()).To(Equal(0)) @@ -2906,7 +2915,8 @@ var _ = Describe("Proxy", func() { Expect(capturedEndpoint.PrivateInstanceId).To(Equal("")) Expect(capturedEndpoint.PrivateInstanceIndex).To(Equal("2")) Expect(capturedRespCode).To(Equal(http.StatusOK)) - Expect(startTime).To(BeTemporally("~", time.Now(), 100*time.Millisecond)) + Expect(startTime).To(BeTemporally(">=", before)) + Expect(startTime).To(BeTemporally("<=", after)) Expect(latency).To(BeNumerically(">", 0)) Expect(fakeReporter.CaptureRoutingRequestCallCount()).To(Equal(1)) diff --git a/src/code.cloudfoundry.org/gorouter/proxy/round_tripper/proxy_round_tripper_test.go b/src/code.cloudfoundry.org/gorouter/proxy/round_tripper/proxy_round_tripper_test.go index e4187b506..fa59b63b3 100644 --- a/src/code.cloudfoundry.org/gorouter/proxy/round_tripper/proxy_round_tripper_test.go +++ b/src/code.cloudfoundry.org/gorouter/proxy/round_tripper/proxy_round_tripper_test.go @@ -3028,13 +3028,14 @@ var _ = Describe("ProxyRoundTripper", func() { } }) It("sets a http/1 timeout on the request context", func() { + before := time.Now() proxyRoundTripper.RoundTrip(req) var request *http.Request Eventually(reqCh).Should(Receive(&request)) deadLine, deadlineSet := request.Context().Deadline() Expect(deadlineSet).To(BeTrue()) - Expect(deadLine).To(BeTemporally("~", time.Now().Add(20*time.Millisecond), 11*time.Millisecond)) + Expect(deadLine).To(BeTemporally("~", before.Add(20*time.Millisecond), 20*time.Millisecond)) Eventually(func() string { err := request.Context().Err() if err != nil { @@ -3053,13 +3054,14 @@ var _ = Describe("ProxyRoundTripper", func() { } }) It("sets a http/2 timeout on the request context", func() { + before := time.Now() proxyRoundTripper.RoundTrip(req) var request *http.Request Eventually(reqCh).Should(Receive(&request)) deadLine, deadlineSet := request.Context().Deadline() Expect(deadlineSet).To(BeTrue()) - Expect(deadLine).To(BeTemporally("~", time.Now().Add(15*time.Millisecond), 6*time.Millisecond)) + Expect(deadLine).To(BeTemporally("~", before.Add(15*time.Millisecond), 15*time.Millisecond)) Eventually(func() string { err := request.Context().Err() if err != nil { diff --git a/src/code.cloudfoundry.org/gorouter/router/route_service_server_test.go b/src/code.cloudfoundry.org/gorouter/router/route_service_server_test.go index c2af6c3ab..9992ddb8c 100644 --- a/src/code.cloudfoundry.org/gorouter/router/route_service_server_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/route_service_server_test.go @@ -7,6 +7,7 @@ import ( "net/http" "time" + "code.cloudfoundry.org/gorouter/test_util" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -31,6 +32,7 @@ var _ = Describe("RouteServicesServer", func() { var err error cfg, err = config.DefaultConfig() Expect(err).NotTo(HaveOccurred()) + cfg.RouteServicesServerPort = test_util.NextAvailPort() req, err = http.NewRequest("GET", "/foo", nil) Expect(err).NotTo(HaveOccurred()) diff --git a/src/code.cloudfoundry.org/gorouter/router/router_drain_test.go b/src/code.cloudfoundry.org/gorouter/router/router_drain_test.go index 64d42b664..3fa59ff29 100644 --- a/src/code.cloudfoundry.org/gorouter/router/router_drain_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/router_drain_test.go @@ -151,8 +151,9 @@ var _ = Describe("Router", func() { BeforeEach(func() { logger = test_util.NewTestLogger("test") - natsPort = test_util.NextAvailPort() + natsPort = test_util.ReservePort() natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() proxyPort := test_util.NextAvailPort() @@ -161,11 +162,12 @@ var _ = Describe("Router", func() { statusRoutesPort := test_util.NextAvailPort() sslPort := test_util.NextAvailPort() + routeServiceServerPort := test_util.NextAvailPort() defaultCert := test_util.CreateCert("default") cert2 := test_util.CreateCert("default") - config = test_util.SpecConfig(statusPort, statusTlsPort, statusRoutesPort, proxyPort, natsPort) + config = test_util.SpecConfig(statusPort, statusTlsPort, statusRoutesPort, proxyPort, routeServiceServerPort, natsPort) config.EnableSSL = true config.SSLPort = sslPort config.SSLCertificates = []tls.Certificate{defaultCert, cert2} @@ -202,13 +204,13 @@ var _ = Describe("Router", func() { }) AfterEach(func() { - if natsRunner != nil { - natsRunner.Stop() - } if subscriber != nil { subscriber.Signal(os.Interrupt) <-subscriber.Wait() } + if natsRunner != nil { + natsRunner.Stop() + } }) Context("Drain", func() { diff --git a/src/code.cloudfoundry.org/gorouter/router/router_test.go b/src/code.cloudfoundry.org/gorouter/router/router_test.go index b9217227d..9e8fbb61d 100644 --- a/src/code.cloudfoundry.org/gorouter/router/router_test.go +++ b/src/code.cloudfoundry.org/gorouter/router/router_test.go @@ -85,8 +85,9 @@ var _ = Describe("Router", func() { statusPort = test_util.NextAvailPort() statusTLSPort = test_util.NextAvailPort() statusRoutesPort = test_util.NextAvailPort() - natsPort = test_util.NextAvailPort() - config = test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, natsPort) + natsPort = test_util.ReservePort() + routeServiceServerPort := test_util.NextAvailPort() + config = test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, routeServiceServerPort, natsPort) backendIdleTimeout = config.EndpointTimeout requestTimeout = config.EndpointTimeout config.EnableSSL = true @@ -102,6 +103,7 @@ var _ = Describe("Router", func() { } natsRunner = test_util.NewNATSRunner(int(natsPort)) + test_util.ReleasePort(natsPort) natsRunner.Start() routeServicesServer = &sharedfakes.RouteServicesServer{} @@ -163,8 +165,9 @@ var _ = Describe("Router", func() { statusPort = test_util.NextAvailPort() statusTLSPort = test_util.NextAvailPort() statusRoutesPort = test_util.NextAvailPort() + routeServiceServerPort := test_util.NextAvailPort() - c := test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, natsPort) + c := test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, routeServiceServerPort, natsPort) c.StartResponseDelayInterval = 1 * time.Second rtr, err := initializeRouter(c, c.EndpointTimeout, c.EndpointTimeout, registry, varz, mbusClient, logger.Logger, rss) @@ -185,8 +188,9 @@ var _ = Describe("Router", func() { statusPort = test_util.NextAvailPort() statusTLSPort = test_util.NextAvailPort() statusRoutesPort = test_util.NextAvailPort() + routeServiceServerPort := test_util.NextAvailPort() - c := test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, natsPort) + c := test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, routeServiceServerPort, natsPort) c.StartResponseDelayInterval = 1 * time.Second rss := &sharedfakes.RouteServicesServer{} @@ -219,7 +223,8 @@ var _ = Describe("Router", func() { statusPort = test_util.NextAvailPort() statusTLSPort = test_util.NextAvailPort() statusRoutesPort = test_util.NextAvailPort() - c = test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, natsPort) + routeServiceServerPort := test_util.NextAvailPort() + c = test_util.SpecConfig(statusPort, statusTLSPort, statusRoutesPort, proxyPort, routeServiceServerPort, natsPort) c.StartResponseDelayInterval = 1 * time.Second }) @@ -2312,7 +2317,7 @@ var _ = Describe("Router", func() { }) - Describe("frontend timeouts", func() { + Context("frontend timeouts", func() { Context("when the frontend connection idles for more than the configured IdleTimeout", func() { BeforeEach(func() { config.FrontendIdleTimeout = 500 * time.Millisecond diff --git a/src/code.cloudfoundry.org/gorouter/test/common/app.go b/src/code.cloudfoundry.org/gorouter/test/common/app.go index 689090427..8283ea3ac 100644 --- a/src/code.cloudfoundry.org/gorouter/test/common/app.go +++ b/src/code.cloudfoundry.org/gorouter/test/common/app.go @@ -68,8 +68,11 @@ func (a *TestApp) Endpoint() string { } func (a *TestApp) TlsListen(tlsConfig *tls.Config) chan error { + ln, err := tls.Listen("tcp", fmt.Sprintf(":%d", a.port), tlsConfig) + if err != nil { + panic("TestApp.TlsListen: " + err.Error()) + } a.server = &http.Server{ - Addr: fmt.Sprintf(":%d", a.port), Handler: a.mux, TLSConfig: tlsConfig, ReadHeaderTimeout: 5 * time.Second, @@ -77,7 +80,7 @@ func (a *TestApp) TlsListen(tlsConfig *tls.Config) chan error { errChan := make(chan error, 1) go func() { - err := a.server.ListenAndServeTLS("", "") + err := a.server.Serve(ln) errChan <- err }() return errChan @@ -89,12 +92,15 @@ func (a *TestApp) RegisterAndListen() { } func (a *TestApp) Listen() { + ln, err := net.Listen("tcp", fmt.Sprintf(":%d", a.port)) + if err != nil { + panic("TestApp.Listen: " + err.Error()) + } server := &http.Server{ - Addr: fmt.Sprintf(":%d", a.port), Handler: a.mux, ReadHeaderTimeout: 5 * time.Second, } - go server.ListenAndServe() + go server.Serve(ln) } func (a *TestApp) RegisterRepeatedly(duration time.Duration) { diff --git a/src/code.cloudfoundry.org/gorouter/test_util/ports.go b/src/code.cloudfoundry.org/gorouter/test_util/ports.go index aaeec5d70..d728c81a0 100644 --- a/src/code.cloudfoundry.org/gorouter/test_util/ports.go +++ b/src/code.cloudfoundry.org/gorouter/test_util/ports.go @@ -1,18 +1,122 @@ package test_util import ( + "fmt" "net" + "sync" + + . "github.com/onsi/ginkgo/v2" +) + +var ( + allocatedPorts = make(map[uint16]bool) + reservedListeners = make(map[uint16]net.Listener) + portMu sync.Mutex ) -// NextAvailPort asks the OS for a free port by binding to :0, then closing -// the listener and returning the assigned port. This avoids cross-suite port -// collisions that occur when multiple suites reuse the same static port range. +// portRange returns the base port and size of the range reserved for the +// current Ginkgo parallel process. It divides the port space [61000,65534] +// evenly across GinkgoConfiguration().ParallelTotal procs. +// +// Port space starts at 61000 to stay entirely above the Linux kernel's default +// ephemeral port range (32768–60999, see /proc/sys/net/ipv4/ip_local_port_range). +// Ports inside the ephemeral range can be grabbed by the OS for outgoing +// connections in the window between ReleaseAllPorts() and the moment the +// external process (gorouter) actually calls listen(), causing "address already +// in use" failures on loaded systems such as Docker VMs. +func portRange() (base, size uint16) { + suiteConfig, _ := GinkgoConfiguration() + total := suiteConfig.ParallelTotal + if total <= 0 { + total = 1 + } + // Stay above the Linux ephemeral range (32768-60999). + const portSpaceStart = 61000 + const portSpaceEnd = 65534 + size = uint16((portSpaceEnd - portSpaceStart) / total) + base = portSpaceStart + uint16(GinkgoParallelProcess()-1)*size + return +} + +// nextPortInRange returns the next free port in this process's dedicated range. +// Must be called with portMu held. +func nextPortInRange() uint16 { + base, size := portRange() + for port := base; port < base+size; port++ { + if allocatedPorts[port] { + continue + } + l, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) + if err != nil { + // Port in use by something outside our process – skip it. + allocatedPorts[port] = true + continue + } + l.Close() + allocatedPorts[port] = true + return port + } + panic(fmt.Sprintf("nextPortInRange: exhausted %d-port range starting at %d for Ginkgo proc %d", size, base, GinkgoParallelProcess())) +} + +// NextAvailPort returns a free port from the current Ginkgo process's dedicated +// port range. Using per-process ranges eliminates cross-process collisions when +// running with --nodes=N, removing the need for the ReservePort/ReleaseAllPorts +// dance for in-process port bindings. func NextAvailPort() uint16 { - l, err := net.Listen("tcp", "127.0.0.1:0") - if err != nil { - panic("NextAvailPort: " + err.Error()) + portMu.Lock() + defer portMu.Unlock() + return nextPortInRange() +} + +// ReservePort returns a free port and keeps the listener open so that no other +// process can grab it before the caller is ready. Call ReleaseAllPorts to +// close all held listeners just before starting the process that will bind +// these ports. This eliminates the TOCTOU race between port allocation and +// binding when ports are used by external processes (e.g. integration tests +// that spawn gorouter as a separate binary). +func ReservePort() uint16 { + portMu.Lock() + defer portMu.Unlock() + + base, size := portRange() + for port := base; port < base+size; port++ { + if allocatedPorts[port] { + continue + } + l, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", port)) + if err != nil { + allocatedPorts[port] = true + continue + } + allocatedPorts[port] = true + reservedListeners[port] = l // keep open! + return port + } + panic(fmt.Sprintf("ReservePort: exhausted %d-port range starting at %d for Ginkgo proc %d", size, base, GinkgoParallelProcess())) +} + +// ReleaseAllPorts closes all listeners held by ReservePort. Call this just +// before starting an external process that needs to bind the reserved ports. +func ReleaseAllPorts() { + portMu.Lock() + defer portMu.Unlock() + + for port, l := range reservedListeners { + l.Close() + delete(reservedListeners, port) + } +} + +// ReleasePort closes the reservation listener for a single port. Use this +// when only one reserved port needs to be freed (e.g. before starting NATS +// while keeping the other ports reserved for the gorouter). +func ReleasePort(port uint16) { + portMu.Lock() + defer portMu.Unlock() + + if l, ok := reservedListeners[port]; ok { + l.Close() + delete(reservedListeners, port) } - defer l.Close() - // #nosec G115 - ephemeral ports are always in uint16 range - return uint16(l.Addr().(*net.TCPAddr).Port) }