parent 10f500e

author absolutelightning <ashesh.vidyut@hashicorp.com> 1687352587 +0530
committer absolutelightning <ashesh.vidyut@hashicorp.com> 1687352592 +0530

init without tests

change log

fix tests

fix tests

added tests

change log breaking change

removed breaking change

fix test

keeping the test behaviour same

made enable debug atomic bool

fix lint

fix test true enable debug

using enable debug in agent as atomic bool

test fixes

fix tests

fix tests

added update on correct locaiton

fix tests

fix reloadable config enable debug

fix tests

fix init and acl 403

agent/agent.go

@@ -416,6 +416,8 @@ type Agent struct {
 
 	// enterpriseAgent embeds fields that we only access in consul-enterprise builds
 	enterpriseAgent
+
+	enableDebug atomic.Bool
 }
 
 // New process the desired options and creates a new Agent.
@@ -598,6 +600,8 @@ func (a *Agent) Start(ctx context.Context) error {
 	// Overwrite the configuration.
 	a.config = c
 
+	a.enableDebug.Store(c.EnableDebug)
+
 	if err := a.tlsConfigurator.Update(a.config.TLS); err != nil {
 		return fmt.Errorf("Failed to load TLS configurations after applying auto-config settings: %w", err)
 	}
@@ -4291,8 +4295,8 @@ func (a *Agent) reloadConfigInternal(newCfg *config.RuntimeConfig) error {
 
 	a.proxyConfig.SetUpdateRateLimit(newCfg.XDSUpdateRateLimit)
 
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(newCfg.EnableDebug.Load())
+	a.enableDebug.Store(newCfg.EnableDebug)
+	a.config.EnableDebug = newCfg.EnableDebug
 
 	return nil
 }

agent/agent_endpoint_test.go

@@ -17,7 +17,6 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -6009,8 +6008,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)
+			a.enableDebug.Store(true)
+
 			resp := httptest.NewRecorder()
 			handler := a.srv.handler()
 			go handler.ServeHTTP(resp, req)

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.Load())
+	require.Equal(t, true, a.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.Load())
+	require.Equal(t, false, a.enableDebug.Load())
 }
 
 func TestAgent_consulConfig_AutoEncryptAllowTLS(t *testing.T) {

agent/config/builder.go

@@ -18,7 +18,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"time"
 
 	"github.com/armon/go-metrics/prometheus"
@@ -1011,7 +1010,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:                *atomicBoolVal(c.EnableDebug),
+		EnableDebug:                boolVal(c.EnableDebug),
 		EnableRemoteScriptChecks:   enableRemoteScriptChecks,
 		EnableLocalScriptChecks:    enableLocalScriptChecks,
 		EncryptKey:                 stringVal(c.EncryptKey),
@@ -1943,21 +1942,6 @@ 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

agent/config/runtime.go

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

agent/config/runtime_test.go

@@ -17,7 +17,6 @@ import (
 	"reflect"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -326,7 +325,7 @@ func TestLoad_IntegrationWithFlags(t *testing.T) {
 			rt.DisableAnonymousSignature = true
 			rt.DisableKeyringFile = true
 			rt.Experiments = []string{"resource-apis"}
-			rt.EnableDebug.Store(true)
+			rt.EnableDebug = true
 			rt.UIConfig.Enabled = true
 			rt.LeaveOnTerm = false
 			rt.Logging.LogLevel = "DEBUG"
@@ -5977,8 +5976,7 @@ 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 = atomic.Bool{}
-			rt.EnableDebug.Store(true)
+			rt.EnableDebug = true
 		},
 	})
 }
@@ -6081,8 +6079,6 @@ func TestLoad_FullConfig(t *testing.T) {
 		_, n, _ := net.ParseCIDR(s)
 		return n
 	}
-	atomicBoolTrue := atomic.Bool{}
-	atomicBoolTrue.Store(true)
 
 	defaultEntMeta := structs.DefaultEnterpriseMetaInDefaultPartition()
 	nodeEntMeta := structs.NodeEnterpriseMetaInDefaultPartition()
@@ -6370,7 +6366,7 @@ func TestLoad_FullConfig(t *testing.T) {
 		DiscoveryMaxStale:                5 * time.Second,
 		EnableAgentTLSForChecks:          true,
 		EnableCentralServiceConfig:       false,
-		EnableDebug:                      *atomicBool(true),
+		EnableDebug:                      true,
 		EnableRemoteScriptChecks:         true,
 		EnableLocalScriptChecks:          true,
 		EncryptKey:                       "A4wELWqH",

agent/http.go

@@ -213,8 +213,13 @@ 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.Load() && s.checkACLDisabled() {
+			if s.checkACLDisabled() {
+				resp.WriteHeader(http.StatusForbidden)
+				return
+			}
+
+			// If enableDebug register wrapped pprof handlers
+			if !s.agent.enableDebug.Load() {
 				resp.WriteHeader(http.StatusNotFound)
 				return
 			}

agent/http_oss_test.go

@@ -9,7 +9,6 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -145,8 +144,7 @@ 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 = atomic.Bool{}
-			a.config.EnableDebug.Store(true)
+			a.enableDebug.Store(true)
 			a.srv.handler().ServeHTTP(resp, req)
 			allMethods := append([]string{"OPTIONS"}, methods...)
 
@@ -193,8 +191,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 = atomic.Bool{}
-			
+			a.enableDebug.Store(true)
+
 			a.srv.handler().ServeHTTP(resp, req)
 
 			require.Equal(t, http.StatusForbidden, resp.Code, "%s %s", method, path)

agent/http_test.go

@@ -20,7 +20,6 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -289,8 +288,8 @@ func TestSetupHTTPServer_HTTP2(t *testing.T) {
 	err = setupHTTPS(httpServer, noopConnState, time.Second)
 	require.NoError(t, err)
 
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.enableDebug.Store(true)
+
 	srvHandler := a.srv.handler()
 	mux, ok := srvHandler.(*wrappedMux)
 	require.True(t, ok, "expected a *wrappedMux, got %T", handler)
@@ -486,8 +485,8 @@ func TestHTTPAPI_Ban_Nonprintable_Characters(t *testing.T) {
 		t.Fatal(err)
 	}
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.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)
@@ -511,8 +510,8 @@ func TestHTTPAPI_Allow_Nonprintable_Characters_With_Flag(t *testing.T) {
 		t.Fatal(err)
 	}
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.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 {
@@ -652,8 +651,8 @@ func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) {
 
 	resp := httptest.NewRecorder()
 	req, _ := http.NewRequest("GET", path, nil)
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.enableDebug.Store(true)
+
 	a.srv.handler().ServeHTTP(resp, req)
 
 	hdrs := resp.Header()
@@ -715,17 +714,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 = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.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 = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.enableDebug.Store(true)
+
 	a.srv.handler().ServeHTTP(resp, req)
 	require.Equal(t, 200, resp.Code)
 	require.Equal(t, "gzip", resp.Header().Get("Content-Encoding"))
@@ -1081,8 +1080,7 @@ func TestHTTPServer_PProfHandlers_EnableDebug(t *testing.T) {
 	resp := httptest.NewRecorder()
 	req, _ := http.NewRequest("GET", "/debug/pprof/profile?seconds=1", nil)
 
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.enableDebug.Store(true)
 	httpServer := &HTTPHandlers{agent: a.Agent}
 	httpServer.handler().ServeHTTP(resp, req)
 
@@ -1183,8 +1181,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 = atomic.Bool{}
-			a.config.EnableDebug.Store(true)
+			a.enableDebug.Store(true)
+
 			a.srv.handler().ServeHTTP(resp, req)
 			assert.Equal(t, c.code, resp.Code)
 		})
@@ -1495,8 +1493,8 @@ func TestEnableWebUI(t *testing.T) {
 
 	req, _ := http.NewRequest("GET", "/ui/", nil)
 	resp := httptest.NewRecorder()
-	a.config.EnableDebug = atomic.Bool{}
-	a.config.EnableDebug.Store(true)
+	a.enableDebug.Store(true)
+
 	a.srv.handler().ServeHTTP(resp, req)
 	require.Equal(t, http.StatusOK, resp.Code)
 
@@ -1526,8 +1524,8 @@ func TestEnableWebUI(t *testing.T) {
 	{
 		req, _ := http.NewRequest("GET", "/ui/", nil)
 		resp := httptest.NewRecorder()
-		a.config.EnableDebug = atomic.Bool{}
-		a.config.EnableDebug.Store(true)
+		a.enableDebug.Store(true)
+
 		a.srv.handler().ServeHTTP(resp, req)
 		require.Equal(t, http.StatusOK, resp.Code)
 		require.Contains(t, resp.Body.String(), `<!-- CONSUL_VERSION:`)

agent/ui_endpoint_oss_test.go

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

agent/ui_endpoint_test.go

@@ -2620,9 +2620,9 @@ func TestUIEndpoint_MetricsProxy(t *testing.T) {
 			require.NoError(t, a.Agent.reloadConfigInternal(&cfg))
 
 			// Now fetch the API handler to run requests against
+			a.enableDebug.Store(true)
+
 			h := a.srv.handler()
-			a.config.EnableDebug = atomic.Bool{}
-			a.config.EnableDebug.Store(true)
 
 			req := httptest.NewRequest("GET", tc.path, nil)
 			rec := httptest.NewRecorder()