From 0e176dd6aa7074bbffb0eb97cb4013704ef0acf0 Mon Sep 17 00:00:00 2001 From: "Chris S. Kim" Date: Thu, 27 Oct 2022 09:25:18 -0400 Subject: [PATCH] Allow consul debug on non-ACL consul servers (#15155) --- .changelog/15155.txt | 3 +++ agent/http.go | 21 ++++++++------------- agent/http_test.go | 4 ++-- api/api.go | 7 +++---- api/debug.go | 9 ++++----- command/debug/debug.go | 34 +++++++++++++++++++++++++--------- command/debug/debug_test.go | 3 +-- 7 files changed, 46 insertions(+), 35 deletions(-) create mode 100644 .changelog/15155.txt diff --git a/.changelog/15155.txt b/.changelog/15155.txt new file mode 100644 index 0000000000..1b51c2b96c --- /dev/null +++ b/.changelog/15155.txt @@ -0,0 +1,3 @@ +```release-note:bug +debug: fixed bug that caused consul debug CLI to error on ACL-disabled clusters +``` \ No newline at end of file diff --git a/agent/http.go b/agent/http.go index 265f32e270..a3529a67a7 100644 --- a/agent/http.go +++ b/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 - 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 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, diff --git a/agent/http_test.go b/agent/http_test.go index 60b0bd97f7..f13e8143da 100644 --- a/agent/http_test.go +++ b/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) { diff --git a/api/api.go b/api/api.go index fcb7c37306..84b850eb30 100644 --- a/api/api.go +++ b/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() } diff --git a/api/debug.go b/api/debug.go index 0dfbfd846b..b7e80b88d0 100644 --- a/api/debug.go +++ b/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) } diff --git a/command/debug/debug.go b/command/debug/debug.go index 587462a4b7..017f42b77a 100644 --- a/command/debug/debug.go +++ b/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) - dir, err := makeIntervalDir(c.output, c.timeNow()) - if err != nil { - return err - } if c.captureTarget(targetProfiles) { + dir, err := makeIntervalDir(c.output, c.timeNow()) + if err != nil { + return err + } + 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) } diff --git a/command/debug/debug_test.go b/command/debug/debug_test.go index e36cc58b15..612a243cc5 100644 --- a/command/debug/debug_test.go +++ b/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") }