Allow consul debug on non-ACL consul servers (#15155)

.changelog/15155.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+debug: fixed bug that caused consul debug CLI to error on ACL-disabled clusters
+```

agent/http.go

@@ -209,13 +209,6 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
 			var token string
 			s.parseToken(req, &token)
 
-			// If enableDebug is not set, and ACLs are disabled, write
-			// an unauthorized response
-			if !enableDebug && s.checkACLDisabled() {
-				resp.WriteHeader(http.StatusUnauthorized)
-				return
-			}
-
 			authz, err := s.agent.delegate.ResolveTokenAndDefaultMeta(token, nil, nil)
 			if err != nil {
 				resp.WriteHeader(http.StatusForbidden)
@@ -247,12 +240,14 @@ func (s *HTTPHandlers) handler(enableDebug bool) http.Handler {
 		handleFuncMetrics(pattern, s.wrap(bound, methods))
 	}
 
-	// Register wrapped pprof handlers
+	// If enableDebug or ACL enabled, register wrapped pprof handlers
+	if enableDebug || !s.checkACLDisabled() {
 		handlePProf("/debug/pprof/", pprof.Index)
 		handlePProf("/debug/pprof/cmdline", pprof.Cmdline)
 		handlePProf("/debug/pprof/profile", pprof.Profile)
 		handlePProf("/debug/pprof/symbol", pprof.Symbol)
 		handlePProf("/debug/pprof/trace", pprof.Trace)
+	}
 
 	if s.IsUIEnabled() {
 		// Note that we _don't_ support reloading ui_config.{enabled, content_dir,

agent/http_test.go

@@ -20,7 +20,7 @@ import (
 	"time"
 
 	"github.com/NYTimes/gziphandler"
-	cleanhttp "github.com/hashicorp/go-cleanhttp"
+	"github.com/hashicorp/go-cleanhttp"
 	"github.com/hashicorp/go-hclog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -982,7 +982,7 @@ func TestHTTPServer_PProfHandlers_DisableDebugNoACLs(t *testing.T) {
 	httpServer := &HTTPHandlers{agent: a.Agent}
 	httpServer.handler(false).ServeHTTP(resp, req)
 
-	require.Equal(t, http.StatusUnauthorized, resp.Code)
+	require.Equal(t, http.StatusNotFound, resp.Code)
 }
 
 func TestHTTPServer_PProfHandlers_ACLs(t *testing.T) {

api/api.go

@@ -7,7 +7,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"net"
 	"net/http"
 	"net/url"
@@ -744,7 +743,7 @@ func NewClient(config *Config) (*Client, error) {
 	// This is because when TokenFile is set it is read into the Token field.
 	// We want any derived clients to have to re-read the token file.
 	if config.TokenFile != "" {
-		data, err := ioutil.ReadFile(config.TokenFile)
+		data, err := os.ReadFile(config.TokenFile)
 		if err != nil {
 			return nil, fmt.Errorf("Error loading token file: %s", err)
 		}
@@ -1065,7 +1064,7 @@ func (c *Client) write(endpoint string, in, out interface{}, q *WriteOptions) (*
 		if err := decodeBody(resp, &out); err != nil {
 			return nil, err
 		}
-	} else if _, err := ioutil.ReadAll(resp.Body); err != nil {
+	} else if _, err := io.ReadAll(resp.Body); err != nil {
 		return nil, err
 	}
 	return wm, nil
@@ -1183,7 +1182,7 @@ func requireHttpCodes(resp *http.Response, httpCodes ...int) error {
 // is necessary to ensure that the http.Client's underlying RoundTripper is able
 // to re-use the TCP connection. See godoc on net/http.Client.Do.
 func closeResponseBody(resp *http.Response) error {
-	_, _ = io.Copy(ioutil.Discard, resp.Body)
+	_, _ = io.Copy(io.Discard, resp.Body)
 	return resp.Body.Close()
 }
 

api/debug.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"strconv"
 )
 
@@ -36,7 +35,7 @@ func (d *Debug) Heap() ([]byte, error) {
 
 	// We return a raw response because we're just passing through a response
 	// from the pprof handlers
-	body, err := ioutil.ReadAll(resp.Body)
+	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("error decoding body: %s", err)
 	}
@@ -62,7 +61,7 @@ func (d *Debug) Profile(seconds int) ([]byte, error) {
 
 	// We return a raw response because we're just passing through a response
 	// from the pprof handlers
-	body, err := ioutil.ReadAll(resp.Body)
+	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("error decoding body: %s", err)
 	}
@@ -107,7 +106,7 @@ func (d *Debug) Trace(seconds int) ([]byte, error) {
 
 	// We return a raw response because we're just passing through a response
 	// from the pprof handlers
-	body, err := ioutil.ReadAll(resp.Body)
+	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("error decoding body: %s", err)
 	}
@@ -130,7 +129,7 @@ func (d *Debug) Goroutine() ([]byte, error) {
 
 	// We return a raw response because we're just passing through a response
 	// from the pprof handlers
-	body, err := ioutil.ReadAll(resp.Body)
+	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("error decoding body: %s", err)
 	}

command/debug/debug.go

@@ -10,7 +10,6 @@ import (
 	"flag"
 	"fmt"
 	"io"
-	"io/ioutil"
 	"os"
 	"os/signal"
 	"path/filepath"
@@ -274,6 +273,22 @@ func (c *cmd) prepare() (version string, err error) {
 		c.capture = defaultTargets
 	}
 
+	// If EnableDebug is not true, skip collecting pprof
+	enableDebug, ok := self["DebugConfig"]["EnableDebug"].(bool)
+	if !ok {
+		return "", fmt.Errorf("agent response did not contain EnableDebug key")
+	}
+	if !enableDebug {
+		cs := c.capture
+		for i := 0; i < len(cs); i++ {
+			if cs[i] == "pprof" {
+				c.capture = append(cs[:i], cs[i+1:]...)
+				i--
+				c.UI.Warn("[WARN] Unable to capture pprof. Set enable_debug to true on target agent to enable profiling.")
+			}
+		}
+	}
+
 	for _, t := range c.capture {
 		if !allowedTarget(t) {
 			return version, fmt.Errorf("target not found: %s", t)
@@ -334,7 +349,7 @@ func writeJSONFile(filename string, content interface{}) error {
 	if err != nil {
 		return err
 	}
-	return ioutil.WriteFile(filename, marshaled, 0644)
+	return os.WriteFile(filename, marshaled, 0644)
 }
 
 // captureInterval blocks for the duration of the command
@@ -374,11 +389,12 @@ func (c *cmd) captureInterval(ctx context.Context) error {
 func captureShortLived(c *cmd) error {
 	g := new(errgroup.Group)
 
+	if c.captureTarget(targetProfiles) {
 		dir, err := makeIntervalDir(c.output, c.timeNow())
 		if err != nil {
 			return err
 		}
-	if c.captureTarget(targetProfiles) {
+
 		g.Go(func() error {
 			return c.captureHeap(dir)
 		})
@@ -436,7 +452,7 @@ func (c *cmd) captureGoRoutines(outputDir string) error {
 		return fmt.Errorf("failed to collect goroutine profile: %w", err)
 	}
 
-	return ioutil.WriteFile(filepath.Join(outputDir, "goroutine.prof"), gr, 0644)
+	return os.WriteFile(filepath.Join(outputDir, "goroutine.prof"), gr, 0644)
 }
 
 func (c *cmd) captureTrace(ctx context.Context, duration int) error {
@@ -479,7 +495,7 @@ func (c *cmd) captureHeap(outputDir string) error {
 		return fmt.Errorf("failed to collect heap profile: %w", err)
 	}
 
-	return ioutil.WriteFile(filepath.Join(outputDir, "heap.prof"), heap, 0644)
+	return os.WriteFile(filepath.Join(outputDir, "heap.prof"), heap, 0644)
 }
 
 func (c *cmd) captureLogs(ctx context.Context) error {
@@ -600,7 +616,7 @@ func (c *cmd) createArchiveTemp(path string) (tempName string, err error) {
 	dir := filepath.Dir(path)
 	name := filepath.Base(path)
 
-	f, err := ioutil.TempFile(dir, name+".tmp")
+	f, err := os.CreateTemp(dir, name+".tmp")
 	if err != nil {
 		return "", fmt.Errorf("failed to create compressed temp archive: %s", err)
 	}

command/debug/debug_test.go

@@ -519,6 +519,5 @@ func TestDebugCommand_DebugDisabled(t *testing.T) {
 	}
 
 	errOutput := ui.ErrorWriter.String()
-	require.Contains(t, errOutput, "failed to collect")
-
+	require.Contains(t, errOutput, "Unable to capture pprof")
 }