Merge pull request #9160 from hashicorp/dnephin/go-test-race-in-to-out-list

ci: change go-test-race package list to exclude list

.circleci/config.yml

@@ -236,16 +236,15 @@ jobs:
           name: go test -race
           command: |
             mkdir -p $TEST_RESULTS_DIR /tmp/jsonfile
+            pkgs="$(go list ./... | \
+              grep -E -v '^github.com/hashicorp/consul/agent(/consul|/local|/xds|/routine-leak-checker)?$' | \
+              grep -E -v '^github.com/hashicorp/consul/command/')"
             gotestsum \
-              --format=short-verbose \
               --jsonfile /tmp/jsonfile/go-test-race.log \
               --junitfile $TEST_RESULTS_DIR/gotestsum-report.xml -- \
               -tags="$GOTAGS" -p 2 \
               -race -gcflags=all=-d=checkptr=0 \
-              ./agent/{ae,cache,cache-types,checks,config,pool,proxycfg,router}/... \
-              ./agent/consul/{authmethod,fsm,state,stream}/... \
-              ./agent/{grpc,rpc,rpcclient,submatview}/... \
-              ./snapshot
+              $pkgs
 
       - store_test_results:
           path: *TEST_RESULTS_DIR

acl/authorizer_test.go

@@ -195,8 +195,6 @@ func (m *mockAuthorizer) Snapshot(ctx *AuthorizerContext) EnforcementDecision {
 }
 
 func TestACL_Enforce(t *testing.T) {
-	t.Parallel()
-
 	type testCase struct {
 		method   string
 		resource Resource

acl/chained_authorizer_test.go

@@ -94,11 +94,7 @@ func (authz testAuthorizer) Snapshot(*AuthorizerContext) EnforcementDecision {
 }
 
 func TestChainedAuthorizer(t *testing.T) {
-	t.Parallel()
-
 	t.Run("No Authorizers", func(t *testing.T) {
-		t.Parallel()
-
 		authz := NewChainedAuthorizer([]Authorizer{})
 		checkDenyACLRead(t, authz, "foo", nil)
 		checkDenyACLWrite(t, authz, "foo", nil)
@@ -129,8 +125,6 @@ func TestChainedAuthorizer(t *testing.T) {
 	})
 
 	t.Run("Authorizer Defaults", func(t *testing.T) {
-		t.Parallel()
-
 		authz := NewChainedAuthorizer([]Authorizer{testAuthorizer(Default)})
 		checkDenyACLRead(t, authz, "foo", nil)
 		checkDenyACLWrite(t, authz, "foo", nil)
@@ -161,8 +155,6 @@ func TestChainedAuthorizer(t *testing.T) {
 	})
 
 	t.Run("Authorizer No Defaults", func(t *testing.T) {
-		t.Parallel()
-
 		authz := NewChainedAuthorizer([]Authorizer{testAuthorizer(Allow)})
 		checkAllowACLRead(t, authz, "foo", nil)
 		checkAllowACLWrite(t, authz, "foo", nil)
@@ -193,8 +185,6 @@ func TestChainedAuthorizer(t *testing.T) {
 	})
 
 	t.Run("First Found", func(t *testing.T) {
-		t.Parallel()
-
 		authz := NewChainedAuthorizer([]Authorizer{testAuthorizer(Deny), testAuthorizer(Allow)})
 		checkDenyACLRead(t, authz, "foo", nil)
 		checkDenyACLWrite(t, authz, "foo", nil)

acl/policy_authorizer_test.go

@@ -13,8 +13,6 @@ import (
 // ensure compatibility from version to version those tests have been only minimally altered. The tests in this
 // file are specific to the newer functionality.
 func TestPolicyAuthorizer(t *testing.T) {
-	t.Parallel()
-
 	type aclCheck struct {
 		name   string
 		prefix string
@@ -446,8 +444,6 @@ func TestPolicyAuthorizer(t *testing.T) {
 		name := name
 		tcase := tcase
 		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
 			authz, err := NewPolicyAuthorizer([]*Policy{tcase.policy}, nil)
 			require.NoError(t, err)
 
@@ -458,7 +454,6 @@ func TestPolicyAuthorizer(t *testing.T) {
 				}
 				t.Run(checkName, func(t *testing.T) {
 					check := check
-					t.Parallel()
 
 					check.check(t, authz, check.prefix, nil)
 				})
@@ -468,8 +463,6 @@ func TestPolicyAuthorizer(t *testing.T) {
 }
 
 func TestAnyAllowed(t *testing.T) {
-	t.Parallel()
-
 	type radixInsertion struct {
 		segment string
 		value   *policyAuthorizerRadixLeaf
@@ -719,8 +712,6 @@ func TestAnyAllowed(t *testing.T) {
 }
 
 func TestAllAllowed(t *testing.T) {
-	t.Parallel()
-
 	type radixInsertion struct {
 		segment string
 		value   *policyAuthorizerRadixLeaf

acl/static_authorizer_test.go

@@ -5,11 +5,7 @@ import (
 )
 
 func TestStaticAuthorizer(t *testing.T) {
-	t.Parallel()
-
 	t.Run("AllowAll", func(t *testing.T) {
-		t.Parallel()
-
 		authz := AllowAll()
 		checkDenyACLRead(t, authz, "foo", nil)
 		checkDenyACLWrite(t, authz, "foo", nil)
@@ -40,7 +36,6 @@ func TestStaticAuthorizer(t *testing.T) {
 	})
 
 	t.Run("DenyAll", func(t *testing.T) {
-		t.Parallel()
 		authz := DenyAll()
 		checkDenyACLRead(t, authz, "foo", nil)
 		checkDenyACLWrite(t, authz, "foo", nil)
@@ -71,7 +66,6 @@ func TestStaticAuthorizer(t *testing.T) {
 	})
 
 	t.Run("ManageAll", func(t *testing.T) {
-		t.Parallel()
 		authz := ManageAll()
 		checkAllowACLRead(t, authz, "foo", nil)
 		checkAllowACLWrite(t, authz, "foo", nil)

agent/auto-config/auto_config_test.go

@@ -634,6 +634,7 @@ type testAutoConfig struct {
 	ac            *AutoConfig
 	tokenUpdates  chan struct{}
 	originalToken string
+	stop          func()
 
 	initialRoots *structs.IndexedCARoots
 	initialCert  *structs.IssuedCert
@@ -835,6 +836,7 @@ func startedAutoConfig(t *testing.T, autoEncrypt bool) testAutoConfig {
 		initialRoots:  indexedRoots,
 		initialCert:   cert,
 		extraCerts:    extraCerts,
+		stop:          cancel,
 	}
 }
 
@@ -1098,16 +1100,15 @@ func TestFallback(t *testing.T) {
 	// now wait for the fallback routine to be invoked
 	require.True(t, waitForChans(100*time.Millisecond, fallbackCtx.Done()), "fallback routines did not get invoked within the alotted time")
 
-	// persisting these to disk happens after the RPC we waited on above will have fired
-	// There is no deterministic way to know once its been written so we wrap this in a retry.
-	testretry.Run(t, func(r *testretry.R) {
-		resp, err := testAC.ac.readPersistedAutoConfig()
-		require.NoError(r, err)
+	testAC.stop()
+	<-testAC.ac.done
 
-		// ensure the roots got persisted to disk
-		require.Equal(r, thirdCert.CertPEM, resp.Certificate.GetCertPEM())
-		require.Equal(r, secondRoots.ActiveRootID, resp.CARoots.GetActiveRootID())
-	})
+	resp, err := testAC.ac.readPersistedAutoConfig()
+	require.NoError(t, err)
+
+	// ensure the roots got persisted to disk
+	require.Equal(t, thirdCert.CertPEM, resp.Certificate.GetCertPEM())
+	require.Equal(t, secondRoots.ActiveRootID, resp.CARoots.GetActiveRootID())
 }
 
 func TestIntroToken(t *testing.T) {

connect/proxy/config_test.go

@@ -5,12 +5,12 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/connect"
 	"github.com/hashicorp/consul/sdk/testutil"
-	"github.com/stretchr/testify/require"
 )
 
 func TestUpstreamResolverFuncFromClient(t *testing.T) {
@@ -79,8 +79,6 @@ func TestUpstreamResolverFuncFromClient(t *testing.T) {
 }
 
 func TestAgentConfigWatcherSidecarProxy(t *testing.T) {
-	t.Parallel()
-
 	a := agent.StartTestAgent(t, agent.TestAgent{Name: "agent_smith"})
 	defer a.Shutdown()
 

connect/proxy/proxy_test.go

@@ -5,7 +5,7 @@ import (
 	"net"
 	"testing"
 
-	"github.com/hashicorp/consul/testrpc"
+	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent"
 	agConnect "github.com/hashicorp/consul/agent/connect"
@@ -14,12 +14,10 @@ import (
 	"github.com/hashicorp/consul/sdk/freeport"
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
-	"github.com/stretchr/testify/require"
+	"github.com/hashicorp/consul/testrpc"
 )
 
 func TestProxy_public(t *testing.T) {
-	t.Parallel()
-
 	require := require.New(t)
 
 	ports := freeport.MustTake(1)

connect/service_test.go

@@ -14,13 +14,13 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	"github.com/hashicorp/consul/agent"
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
-	"github.com/stretchr/testify/require"
 )
 
 // Assert io.Closer implementation
@@ -89,8 +89,8 @@ func TestService_Dial(t *testing.T) {
 					err := testSvr.Serve()
 					require.NoError(err)
 				}()
-				defer testSvr.Close()
 				<-testSvr.Listening
+				defer testSvr.Close()
 			}
 
 			// Always expect to be connecting to a "DB"

connect/testing.go

@@ -10,11 +10,12 @@ import (
 	"net/http"
 	"sync/atomic"
 
+	"github.com/hashicorp/go-hclog"
+	testing "github.com/mitchellh/go-testing-interface"
+
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/agent/structs"
 	"github.com/hashicorp/consul/sdk/freeport"
-	"github.com/hashicorp/go-hclog"
-	testing "github.com/mitchellh/go-testing-interface"
 )
 
 // TestService returns a Service instance based on a static TLS Config.
@@ -124,8 +125,8 @@ func (s *TestServer) Serve() error {
 	if err != nil {
 		return err
 	}
-	close(s.Listening)
 	s.l = l
+	close(s.Listening)
 	log.Printf("test connect service listening on %s", s.Addr)
 
 	for {

connect/tls.go

@@ -13,9 +13,10 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/hashicorp/go-hclog"
+
 	"github.com/hashicorp/consul/agent/connect"
 	"github.com/hashicorp/consul/api"
-	"github.com/hashicorp/go-hclog"
 )
 
 // parseLeafX509Cert will parse an X509 certificate
@@ -460,5 +461,7 @@ func (cfg *dynamicTLSConfig) Ready() bool {
 // method will not stop returning a nil chan in that case. It is only useful
 // for initial startup. For ongoing health Ready() should be used.
 func (cfg *dynamicTLSConfig) ReadyWait() <-chan struct{} {
+	cfg.RLock()
+	defer cfg.RUnlock()
 	return cfg.readyCh
 }