all: avoid mutating global state in parallel tests (#3687)

The SetupSuiteTargetFilters globally mutates the function used for logic
inside the target handler service. This means that we could get
inconsistent logical behavior in certain tests that were run in parallel,
since the logic can change at runtime. Ensure tests that make use
of this function are never run in parallel with other tests.

internal/cmd/commands/server/worker_shutdown_reload_test.go

@@ -95,6 +95,7 @@ func TestServer_ShutdownWorker(t *testing.T) {
 	require.NotNil(tgtR)
 
 	// Authorize and connect
+	// This prevents us from running tests in parallel.
 	tg.SetupSuiteTargetFilters(t)
 	sess := helper.NewTestSession(ctx, t, tcl, tgt.Id)
 	sConn := sess.Connect(ctx, t)

internal/daemon/controller/handlers/targets/target_service_test.go

@@ -41,8 +41,8 @@ func TestWorkerList_Addresses(t *testing.T) {
 }
 
 func TestWorkerList_EgressFilter(t *testing.T) {
-	t.Parallel()
 	ctx := context.Background()
+	// This prevents us from running tests in parallel.
 	SetupSuiteTargetFilters(t)
 	conn, _ := db.TestSetup(t, "postgres")
 	wrapper := db.TestWrapper(t)

internal/daemon/controller/handlers/targets/tcp/target_service_test.go

@@ -2415,8 +2415,8 @@ func TestRemoveTargetCredentialSources(t *testing.T) {
 }
 
 func TestAuthorizeSession(t *testing.T) {
-	t.Parallel()
 	ctx := context.Background()
+	// This prevents us from running tests in parallel.
 	targets.SetupSuiteTargetFilters(t)
 	conn, _ := db.TestSetup(t, "postgres")
 	rw := db.New(conn)
@@ -2712,13 +2712,13 @@ func TestAuthorizeSession(t *testing.T) {
 }
 
 func TestAuthorizeSessionTypedCredentials(t *testing.T) {
-	t.Parallel()
 	ctx := context.Background()
 	conn, _ := db.TestSetup(t, "postgres")
 	rw := db.New(conn)
 	wrapper := db.TestWrapper(t)
 	kms := kms.TestKms(t, conn, wrapper)
 
+	// This prevents us from running tests in parallel.
 	targets.SetupSuiteTargetFilters(t)
 
 	sche := scheduler.TestScheduler(t, conn, wrapper)
@@ -3303,8 +3303,8 @@ func TestAuthorizeSessionTypedCredentials(t *testing.T) {
 }
 
 func TestAuthorizeSession_Errors(t *testing.T) {
-	t.Parallel()
 	ctx := context.Background()
+	// This prevents us from running tests in parallel.
 	targets.SetupSuiteTargetFilters(t)
 	conn, _ := db.TestSetup(t, "postgres")
 	rw := db.New(conn)

internal/daemon/controller/handlers/targets/testing.go

@@ -8,6 +8,7 @@ import (
 )
 
 // SetupSuiteTargetFilters is used to ensure that OSS tests run from the ENT repo use the OSS level of target filtering
+// WARNING: Do NOT run tests in parallel when using this.
 func SetupSuiteTargetFilters(t *testing.T) {
 	oldFn := AuthorizeSessionWorkerFilterFn
 	AuthorizeSessionWorkerFilterFn = AuthorizeSessionWithWorkerFilter

internal/tests/api/credentials/credentials_test.go

@@ -412,6 +412,7 @@ func TestUpdateAfterKeyRotation(t *testing.T) {
 		Level: hclog.Trace,
 	})
 
+	// This prevents us from running tests in parallel.
 	tg.SetupSuiteTargetFilters(t)
 
 	tc := controller.NewTestController(t, &controller.TestControllerOpts{SchedulerRunJobInterval: 100 * time.Millisecond})

internal/tests/cluster/multi_controller_worker_test.go

@@ -123,7 +123,6 @@ func TestWorkerAppendInitialUpstreams(t *testing.T) {
 	for {
 		select {
 		case <-time.After(500 * time.Millisecond):
-			break
 		case <-cancelCtx.Done():
 			require.FailNow("No worker found after 10 seconds")
 		}

internal/tests/cluster/session_cleanup_test.go

@@ -63,11 +63,9 @@ func workerGracePeriod(ty timeoutBurdenType) time.Duration {
 // TestSessionCleanup is the main test for session cleanup, and
 // dispatches to the individual subtests.
 func TestSessionCleanup(t *testing.T) {
-	t.Parallel()
 	for _, burdenCase := range timeoutBurdenCases {
 		burdenCase := burdenCase
 		t.Run(string(burdenCase), func(t *testing.T) {
-			t.Parallel()
 			t.Run("single_controller", testWorkerSessionCleanupSingle(burdenCase))
 			t.Run("multi_controller", testWorkerSessionCleanupMulti(burdenCase))
 		})
@@ -77,8 +75,8 @@ func TestSessionCleanup(t *testing.T) {
 func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testing.T) {
 	const op = "cluster.testWorkerSessionCleanupSingle"
 	return func(t *testing.T) {
-		t.Parallel()
 		require := require.New(t)
+		// This prevents us from running tests in parallel.
 		tg.SetupSuiteTargetFilters(t)
 		logger := hclog.New(&hclog.LoggerOptions{
 			Name:  t.Name(),
@@ -203,7 +201,7 @@ func testWorkerSessionCleanupSingle(burdenCase timeoutBurdenType) func(t *testin
 func testWorkerSessionCleanupMulti(burdenCase timeoutBurdenType) func(t *testing.T) {
 	const op = "cluster.testWorkerSessionCleanupMulti"
 	return func(t *testing.T) {
-		t.Parallel()
+		// This prevents us from running tests in parallel.
 		tg.SetupSuiteTargetFilters(t)
 		require := require.New(t)
 		logger := hclog.New(&hclog.LoggerOptions{

internal/tests/cluster/worker_bytesupdown_test.go

@@ -21,9 +21,9 @@ import (
 )
 
 func TestWorkerBytesUpDown(t *testing.T) {
-	t.Parallel()
 	require := require.New(t)
 
+	// This prevents us from running tests in parallel.
 	tg.SetupSuiteTargetFilters(t)
 
 	logger := hclog.New(&hclog.LoggerOptions{

internal/tests/cluster/worker_proxy_test.go

@@ -22,8 +22,8 @@ import (
 
 func TestWorkerSessionProxyMultipleConnections(t *testing.T) {
 	const op = "cluster.TestWorkerSessionMultipleConnections"
-	t.Parallel()
 
+	// This prevents us from running tests in parallel.
 	tg.SetupSuiteTargetFilters(t)
 
 	require := require.New(t)

internal/tests/cluster/x509_verification_test.go

@@ -33,6 +33,7 @@ func TestCustomX509Verification_Client(t *testing.T) {
 	ctx := context.Background()
 	ec := event.TestEventerConfig(t, "TestWorkerReplay", event.TestWithObservationSink(t), event.TestWithSysSink(t))
 	testLock := &sync.Mutex{}
+	// This prevents us from running tests in parallel.
 	tg.SetupSuiteTargetFilters(t)
 	logger := hclog.New(&hclog.LoggerOptions{
 		Mutex: testLock,
@@ -174,9 +175,9 @@ func TestCustomX509Verification_Server(t *testing.T) {
 
 func testCustomX509Verification_Server(ec event.TestConfig, certPool *x509.CertPool, dnsName, wantErrContains string) func(t *testing.T) {
 	return func(t *testing.T) {
-		t.Parallel()
 		req := require.New(t)
 		ctx := context.Background()
+		// This prevents us from running tests in parallel.
 		tg.SetupSuiteTargetFilters(t)
 
 		conf, err := config.DevController()