# This is a combination of 12 commits.

# This is the 1st commit message: init without tests # This is the commit message #2: change log # This is the commit message #3: fix tests # This is the commit message #4: fix tests # This is the commit message #5: added tests # This is the commit message #6: change log breaking change # This is the commit message #7: removed breaking change # This is the commit message #8: fix test # This is the commit message #9: keeping the test behaviour same # This is the commit message #10: made enable debug atomic bool # This is the commit message #11: fix lint # This is the commit message #12: fix test true enable debug
hashicorp · Jun 21, 2023 · e08c309 · e08c309
1 parent 10f500e
commit e08c309
Show file tree

Hide file tree

Showing 11 changed files with 65 additions and 23 deletions.
diff --git a/agent/agent.go b/agent/agent.go
@@ -19,6 +19,7 @@ import (
 	"strconv"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/armon/go-metrics"
@@ -4290,7 +4291,8 @@ func (a *Agent) reloadConfigInternal(newCfg *config.RuntimeConfig) error {
 
 	a.proxyConfig.SetUpdateRateLimit(newCfg.XDSUpdateRateLimit)
 
-	a.config.EnableDebug = newCfg.EnableDebug
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(newCfg.EnableDebug.Load())
 
 	return nil
 }

diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go
@@ -17,6 +17,7 @@ import (
 	"os"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -6008,6 +6009,8 @@ func TestAgent_Monitor(t *testing.T) {
 			cancelCtx, cancelFunc := context.WithCancel(context.Background())
 			req = req.WithContext(cancelCtx)
 
+			a.config.EnableDebug = atomic.Bool{}
+			a.config.EnableDebug.Store(true)
 			resp := httptest.NewRecorder()
 			handler := a.srv.handler()
 			go handler.ServeHTTP(resp, req)

diff --git a/agent/agent_test.go b/agent/agent_test.go
@@ -4212,7 +4212,7 @@ func TestAgent_ReloadConfig_EnableDebug(t *testing.T) {
 		},
 	)
 	require.NoError(t, a.reloadConfigInternal(c))
-	require.Equal(t, true, a.config.EnableDebug)
+	require.Equal(t, true, a.config.EnableDebug.Load())
 
 	c = TestConfig(
 		testutil.Logger(t),
@@ -4223,7 +4223,7 @@ func TestAgent_ReloadConfig_EnableDebug(t *testing.T) {
 		},
 	)
 	require.NoError(t, a.reloadConfigInternal(c))
-	require.Equal(t, false, a.config.EnableDebug)
+	require.Equal(t, false, a.config.EnableDebug.Load())
 }
 
 func TestAgent_consulConfig_AutoEncryptAllowTLS(t *testing.T) {

diff --git a/agent/config/builder.go b/agent/config/builder.go
@@ -18,6 +18,7 @@ import (
 	"sort"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/armon/go-metrics/prometheus"
@@ -1010,7 +1011,7 @@ func (b *builder) build() (rt RuntimeConfig, err error) {
 		DiscoveryMaxStale:          b.durationVal("discovery_max_stale", c.DiscoveryMaxStale),
 		EnableAgentTLSForChecks:    boolVal(c.EnableAgentTLSForChecks),
 		EnableCentralServiceConfig: boolVal(c.EnableCentralServiceConfig),
-		EnableDebug:                boolVal(c.EnableDebug),
+		EnableDebug:                *atomicBoolVal(c.EnableDebug),
 		EnableRemoteScriptChecks:   enableRemoteScriptChecks,
 		EnableLocalScriptChecks:    enableLocalScriptChecks,
 		EncryptKey:                 stringVal(c.EncryptKey),
@@ -1942,6 +1943,21 @@ func boolValWithDefault(v *bool, defaultVal bool) bool {
 	return *v
 }
 
+func atomicBool(v bool) *atomic.Bool {
+	atomicBool := atomic.Bool{}
+	atomicBool.Store(v)
+	return &atomicBool
+}
+
+func atomicBoolVal(v *bool) *atomic.Bool {
+	if v == nil {
+		return &atomic.Bool{}
+	}
+	atomicBool := atomic.Bool{}
+	atomicBool.Store(*v)
+	return &atomicBool
+}
+
 func boolVal(v *bool) bool {
 	if v == nil {
 		return false

diff --git a/agent/config/runtime.go b/agent/config/runtime.go
@@ -8,6 +8,7 @@ import (
 	"net"
 	"reflect"
 	"strings"
+	"sync/atomic"
 	"time"
 
 	"github.com/hashicorp/go-uuid"
@@ -651,7 +652,7 @@ type RuntimeConfig struct {
 	// EnableDebug is used to enable various debugging features.
 	//
 	// hcl: enable_debug = (true|false)
-	EnableDebug bool
+	EnableDebug atomic.Bool
 
 	// EnableLocalScriptChecks controls whether health checks declared from the local
 	// config file which execute scripts are enabled. This includes regular script

diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go
@@ -17,6 +17,7 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -324,8 +325,8 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DevMode = true
 			rt.DisableAnonymousSignature = true
 			rt.DisableKeyringFile = true
-			rt.EnableDebug = true
 			rt.Experiments = []string{"resource-apis"}
+			rt.EnableDebug.Store(true)
 			rt.UIConfig.Enabled = true
 			rt.LeaveOnTerm = false
 			rt.Logging.LogLevel = "DEBUG"
@@ -5976,7 +5977,8 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			// The logstore settings from first file should not be overridden by a
 			// later file with nothing to say about logstores!
 			rt.RaftLogStoreConfig.Backend = consul.LogStoreBackendWAL
-			rt.EnableDebug = true
+			rt.EnableDebug = atomic.Bool{}
+			rt.EnableDebug.Store(true)
 		},
 	})
 }
@@ -6079,6 +6081,8 @@ func TestLoad_FullConfig(t *testing.T) {
 		_, n, _ := net.ParseCIDR(s)
 		return n
 	}
+	atomicBoolTrue := atomic.Bool{}
+	atomicBoolTrue.Store(true)
 
 	defaultEntMeta := structs.DefaultEnterpriseMetaInDefaultPartition()
 	nodeEntMeta := structs.NodeEnterpriseMetaInDefaultPartition()
@@ -6366,7 +6370,7 @@ func TestLoad_FullConfig(t *testing.T) {
 		DiscoveryMaxStale:                5 * time.Second,
 		EnableAgentTLSForChecks:          true,
 		EnableCentralServiceConfig:       false,
-		EnableDebug:                      true,
+		EnableDebug:                      *atomicBool(true),
 		EnableRemoteScriptChecks:         true,
 		EnableLocalScriptChecks:          true,
 		EncryptKey:                       "A4wELWqH",

diff --git a/agent/http.go b/agent/http.go
@@ -214,7 +214,7 @@ func (s *HTTPHandlers) handler() http.Handler {
 		wrapper := func(resp http.ResponseWriter, req *http.Request) {
 
 			// If enableDebug or ACL enabled, register wrapped pprof handlers
-			if !s.agent.config.EnableDebug && s.checkACLDisabled() {
+			if !s.agent.config.EnableDebug.Load() && s.checkACLDisabled() {
 				resp.WriteHeader(http.StatusNotFound)
 				return
 			}

diff --git a/agent/http_oss_test.go b/agent/http_oss_test.go
@@ -9,6 +9,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -144,7 +145,8 @@ func TestHTTPAPI_OptionMethod_OSS(t *testing.T) {
 			uri := fmt.Sprintf("http://%s%s", a.HTTPAddr(), path)
 			req, _ := http.NewRequest("OPTIONS", uri, nil)
 			resp := httptest.NewRecorder()
-			a.config.EnableDebug = true
+			a.config.EnableDebug = atomic.Bool{}
+			a.config.EnableDebug.Store(true)
 			a.srv.handler().ServeHTTP(resp, req)
 			allMethods := append([]string{"OPTIONS"}, methods...)
 
@@ -191,7 +193,8 @@ func TestHTTPAPI_AllowedNets_OSS(t *testing.T) {
 			req, _ := http.NewRequest(method, uri, nil)
 			req.RemoteAddr = "192.168.1.2:5555"
 			resp := httptest.NewRecorder()
-			a.config.EnableDebug = true
+			a.config.EnableDebug = atomic.Bool{}
+
 			a.srv.handler().ServeHTTP(resp, req)
 
 			require.Equal(t, http.StatusForbidden, resp.Code, "%s %s", method, path)

diff --git a/agent/http_test.go b/agent/http_test.go
@@ -20,6 +20,7 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -288,7 +289,8 @@ func TestSetupHTTPServer_HTTP2(t *testing.T) {
 	err = setupHTTPS(httpServer, noopConnState, time.Second)
 	require.NoError(t, err)
 
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	srvHandler := a.srv.handler()
 	mux, ok := srvHandler.(*wrappedMux)
 	require.True(t, ok, "expected a *wrappedMux, got %T", handler)
@@ -484,7 +486,8 @@ func TestHTTPAPI_Ban_Nonprintable_Characters(t *testing.T) {
 		t.Fatal(err)
 	}
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 	if got, want := resp.Code, http.StatusBadRequest; got != want {
 		t.Fatalf("bad response code got %d want %d", got, want)
@@ -508,7 +511,8 @@ func TestHTTPAPI_Allow_Nonprintable_Characters_With_Flag(t *testing.T) {
 		t.Fatal(err)
 	}
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 	// Key doesn't actually exist so we should get 404
 	if got, want := resp.Code, http.StatusNotFound; got != want {
@@ -648,7 +652,8 @@ func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) {
 
 	resp := httptest.NewRecorder()
 	req, _ := http.NewRequest("GET", path, nil)
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 
 	hdrs := resp.Header()
@@ -710,15 +715,17 @@ func TestAcceptEncodingGzip(t *testing.T) {
 	// negotiation, but since this call doesn't go through a real
 	// transport, the header has to be set manually
 	req.Header["Accept-Encoding"] = []string{"gzip"}
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 	require.Equal(t, 200, resp.Code)
 	require.Equal(t, "", resp.Header().Get("Content-Encoding"))
 
 	resp = httptest.NewRecorder()
 	req, _ = http.NewRequest("GET", "/v1/kv/long", nil)
 	req.Header["Accept-Encoding"] = []string{"gzip"}
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 	require.Equal(t, 200, resp.Code)
 	require.Equal(t, "gzip", resp.Header().Get("Content-Encoding"))
@@ -1074,7 +1081,8 @@ func TestHTTPServer_PProfHandlers_EnableDebug(t *testing.T) {
 	resp := httptest.NewRecorder()
 	req, _ := http.NewRequest("GET", "/debug/pprof/profile?seconds=1", nil)
 
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	httpServer := &HTTPHandlers{agent: a.Agent}
 	httpServer.handler().ServeHTTP(resp, req)
 
@@ -1175,7 +1183,8 @@ func TestHTTPServer_PProfHandlers_ACLs(t *testing.T) {
 		t.Run(fmt.Sprintf("case %d (%#v)", i, c), func(t *testing.T) {
 			req, _ := http.NewRequest("GET", fmt.Sprintf("%s?token=%s", c.endpoint, c.token), nil)
 			resp := httptest.NewRecorder()
-			a.config.EnableDebug = true
+			a.config.EnableDebug = atomic.Bool{}
+			a.config.EnableDebug.Store(true)
 			a.srv.handler().ServeHTTP(resp, req)
 			assert.Equal(t, c.code, resp.Code)
 		})
@@ -1486,7 +1495,8 @@ func TestEnableWebUI(t *testing.T) {
 
 	req, _ := http.NewRequest("GET", "/ui/", nil)
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.config.EnableDebug.Store(true)
 	a.srv.handler().ServeHTTP(resp, req)
 	require.Equal(t, http.StatusOK, resp.Code)
 
@@ -1516,7 +1526,8 @@ func TestEnableWebUI(t *testing.T) {
 	{
 		req, _ := http.NewRequest("GET", "/ui/", nil)
 		resp := httptest.NewRecorder()
-		a.config.EnableDebug = true
+		a.config.EnableDebug = atomic.Bool{}
+		a.config.EnableDebug.Store(true)
 		a.srv.handler().ServeHTTP(resp, req)
 		require.Equal(t, http.StatusOK, resp.Code)
 		require.Contains(t, resp.Body.String(), `<!-- CONSUL_VERSION:`)

diff --git a/agent/ui_endpoint_oss_test.go b/agent/ui_endpoint_oss_test.go
@@ -58,7 +58,8 @@ func TestUIEndpoint_MetricsProxy_ACLDeny(t *testing.T) {
 	`, backendURL))
 	defer a.Shutdown()
 
-	a.config.EnableDebug = true
+	a.config.EnableDebug = atomic.Bool{}
+	a.Config.EnableDebug.Store(true)
 	h := a.srv.handler()
 
 	testrpc.WaitForLeader(t, a.RPC, "dc1")

diff --git a/agent/ui_endpoint_test.go b/agent/ui_endpoint_test.go
@@ -2621,7 +2621,8 @@ func TestUIEndpoint_MetricsProxy(t *testing.T) {
 
 			// Now fetch the API handler to run requests against
 			h := a.srv.handler()
-			a.config.EnableDebug = true
+			a.config.EnableDebug = atomic.Bool{}
+			a.config.EnableDebug.Store(true)
 
 			req := httptest.NewRequest("GET", tc.path, nil)
 			rec := httptest.NewRecorder()