From 970f5d78ec1f0704a2b5dd6593e385fabacce2f8 Mon Sep 17 00:00:00 2001
From: Daniel Nephin <dnephin@hashicorp.com>
Date: Tue, 8 Jun 2021 19:11:34 -0400
Subject: [PATCH] agent: fix two data race in agent tests

The LogOutput io.Writer used by TestAgent must allow concurrent reads and writes, and a
bytes.Buffer does not allow this. The bytes.Buffer must be wrapped with a lock to make this safe.
---
 agent/agent_test.go | 24 +++++++++++++++++++++---
 agent/http_test.go  | 11 ++++++-----
 agent/testagent.go  |  2 ++
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/agent/agent_test.go b/agent/agent_test.go
index 9570bec08b..67a6adf336 100644
--- a/agent/agent_test.go
+++ b/agent/agent_test.go
@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
@@ -3729,10 +3730,27 @@ func TestAgent_SecurityChecks(t *testing.T) {
 	defer a.Shutdown()
 
 	data := make([]byte, 0, 8192)
-	bytesBuffer := bytes.NewBuffer(data)
-	a.LogOutput = bytesBuffer
+	buf := &syncBuffer{b: bytes.NewBuffer(data)}
+	a.LogOutput = buf
 	assert.NoError(t, a.Start(t))
-	assert.Contains(t, bytesBuffer.String(), "using enable-script-checks without ACLs and without allow_write_http_from is DANGEROUS")
+	assert.Contains(t, buf.String(), "using enable-script-checks without ACLs and without allow_write_http_from is DANGEROUS")
+}
+
+type syncBuffer struct {
+	lock sync.RWMutex
+	b    *bytes.Buffer
+}
+
+func (b *syncBuffer) Write(data []byte) (int, error) {
+	b.lock.Lock()
+	defer b.lock.Unlock()
+	return b.b.Write(data)
+}
+
+func (b *syncBuffer) String() string {
+	b.lock.Lock()
+	defer b.lock.Unlock()
+	return b.b.String()
 }
 
 func TestAgent_ReloadConfigOutgoingRPCConfig(t *testing.T) {
diff --git a/agent/http_test.go b/agent/http_test.go
index cddded0771..d391af3c11 100644
--- a/agent/http_test.go
+++ b/agent/http_test.go
@@ -20,6 +20,11 @@ import (
 	"time"
 
 	"github.com/NYTimes/gziphandler"
+	cleanhttp "github.com/hashicorp/go-cleanhttp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"golang.org/x/net/http2"
+
 	"github.com/hashicorp/consul/agent/config"
 	"github.com/hashicorp/consul/agent/structs"
 	tokenStore "github.com/hashicorp/consul/agent/token"
@@ -27,10 +32,6 @@ import (
 	"github.com/hashicorp/consul/sdk/testutil"
 	"github.com/hashicorp/consul/sdk/testutil/retry"
 	"github.com/hashicorp/consul/testrpc"
-	cleanhttp "github.com/hashicorp/go-cleanhttp"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-	"golang.org/x/net/http2"
 )
 
 func TestHTTPServer_UnixSocket(t *testing.T) {
@@ -632,7 +633,7 @@ func TestHTTP_wrap_obfuscateLog(t *testing.T) {
 	}
 
 	t.Parallel()
-	buf := new(bytes.Buffer)
+	buf := &syncBuffer{b: new(bytes.Buffer)}
 	a := StartTestAgent(t, TestAgent{LogOutput: buf})
 	defer a.Shutdown()
 
diff --git a/agent/testagent.go b/agent/testagent.go
index 11410f2080..e4cbae7ce3 100644
--- a/agent/testagent.go
+++ b/agent/testagent.go
@@ -53,6 +53,8 @@ type TestAgent struct {
 	Config *config.RuntimeConfig
 
 	// LogOutput is the sink for the logs. If nil, logs are written to os.Stderr.
+	// The io.Writer must allow concurrent reads and writes. Note that
+	// bytes.Buffer is not safe for concurrent reads and writes.
 	LogOutput io.Writer
 
 	// DataDir may be set to a directory which exists. If is it not set,