diff --git a/.circleci/config.yml b/.circleci/config.yml index 1b8eb39ec..af00fc918 100644 --- a/.circleci/config.yml +++ b/.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 diff --git a/acl/authorizer_test.go b/acl/authorizer_test.go index b5534b00c..b1833efd1 100644 --- a/acl/authorizer_test.go +++ b/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 diff --git a/acl/chained_authorizer_test.go b/acl/chained_authorizer_test.go index 4ff7b2e4f..870a00f9c 100644 --- a/acl/chained_authorizer_test.go +++ b/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) diff --git a/acl/policy_authorizer_test.go b/acl/policy_authorizer_test.go index 0f074e49a..d303eea92 100644 --- a/acl/policy_authorizer_test.go +++ b/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 diff --git a/acl/static_authorizer_test.go b/acl/static_authorizer_test.go index a2865754e..b9ed59093 100644 --- a/acl/static_authorizer_test.go +++ b/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) diff --git a/agent/auto-config/auto_config_test.go b/agent/auto-config/auto_config_test.go index b8ab0caf4..dc413ed4d 100644 --- a/agent/auto-config/auto_config_test.go +++ b/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) { diff --git a/connect/proxy/config_test.go b/connect/proxy/config_test.go index 6ba560308..f076d626c 100644 --- a/connect/proxy/config_test.go +++ b/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() diff --git a/connect/proxy/proxy_test.go b/connect/proxy/proxy_test.go index 64765ca8a..f280387a1 100644 --- a/connect/proxy/proxy_test.go +++ b/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) diff --git a/connect/service_test.go b/connect/service_test.go index 49c487700..1ee66ce83 100644 --- a/connect/service_test.go +++ b/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" diff --git a/connect/testing.go b/connect/testing.go index 4554add0e..30a517b61 100644 --- a/connect/testing.go +++ b/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 { diff --git a/connect/tls.go b/connect/tls.go index e465d3a10..a79fe7c8a 100644 --- a/connect/tls.go +++ b/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 }