diff --git a/agent/agent.go b/agent/agent.go index 1c5ddb7c2cc4..fa75a1cd1cf4 100644 --- a/agent/agent.go +++ b/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 } diff --git a/agent/agent_endpoint_test.go b/agent/agent_endpoint_test.go index 6f57054a3035..9f4210ac892a 100644 --- a/agent/agent_endpoint_test.go +++ b/agent/agent_endpoint_test.go @@ -17,7 +17,6 @@ import ( "os" "strconv" "strings" - "sync/atomic" "testing" "time" @@ -6011,8 +6010,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) diff --git a/agent/agent_test.go b/agent/agent_test.go index 108f970f9fec..a2e27feaf4fd 100644 --- 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.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) { diff --git a/agent/config/builder.go b/agent/config/builder.go index 96e25077c984..5d191ce8b3ac 100644 --- a/agent/config/builder.go +++ b/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), @@ -1939,21 +1938,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 diff --git a/agent/config/runtime.go b/agent/config/runtime.go index 91617d67b5b9..1a8dc13794d3 100644 --- a/agent/config/runtime.go +++ b/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 diff --git a/agent/config/runtime_test.go b/agent/config/runtime_test.go index 6d02f744efe5..c4d598c10fc3 100644 --- a/agent/config/runtime_test.go +++ b/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" @@ -5970,8 +5969,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 }, }) } @@ -6074,8 +6072,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() @@ -6363,7 +6359,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", diff --git a/agent/http.go b/agent/http.go index 7a4554273111..1d1ca8e48d58 100644 --- a/agent/http.go +++ b/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 } diff --git a/agent/http_oss_test.go b/agent/http_oss_test.go index 1ad58239135e..5ba36320f628 100644 --- a/agent/http_oss_test.go +++ b/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) diff --git a/agent/http_test.go b/agent/http_test.go index 212d8ef7e18e..99100c5fbc8e 100644 --- a/agent/http_test.go +++ b/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,8 +714,8 @@ 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")) @@ -724,8 +723,8 @@ func TestAcceptEncodingGzip(t *testing.T) { 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(), `