diff --git a/AUTHORS b/AUTHORS index ea69b2987..f00b25832 100644 --- a/AUTHORS +++ b/AUTHORS @@ -84,3 +84,4 @@ List of contributors, in chronological order: * Zhang Xiao (https://github.com/xzhang1) * Tom Nguyen (https://github.com/lecafard) * Philip Cramer (https://github.com/PhilipCramer) +* James Munson (https://github.com/jmunson) diff --git a/api/api_test.go b/api/api_test.go index b6a5b11f1..b030d16c4 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -42,6 +42,7 @@ func createTestConfig() *os.File { jsonString, err := json.Marshal(gin.H{ "architectures": []string{}, "enableMetricsEndpoint": true, + "enablePprofEndpoint": false, "S3PublishEndpoints": map[string]map[string]string{ "test-s3": { "region": "us-east-1", diff --git a/api/pprof_integration_test.go b/api/pprof_integration_test.go new file mode 100644 index 000000000..36ab6663b --- /dev/null +++ b/api/pprof_integration_test.go @@ -0,0 +1,121 @@ +package api + +import ( + "encoding/json" + "fmt" + "net/http/httptest" + "os" + "os/exec" + "strings" + "testing" + + "github.com/aptly-dev/aptly/aptly" + ctx "github.com/aptly-dev/aptly/context" + "github.com/gin-gonic/gin" +) + +// pprofTestServer starts a real httptest.Server with the aptly router configured +// with enablePprofEndpoint: true. The caller is responsible for calling +// server.Close() and aptlyCtx.Shutdown(). +func pprofTestServer(t *testing.T) (*httptest.Server, *ctx.AptlyContext) { + t.Helper() + + aptly.Version = "testVersion" + + file, err := os.CreateTemp("", "aptly-pprof-integration") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Remove(file.Name()) }) + + jsonString, err := json.Marshal(gin.H{ + "architectures": []string{}, + "enablePprofEndpoint": true, + }) + if err != nil { + t.Fatal(err) + } + if _, err = file.Write(jsonString); err != nil { + t.Fatal(err) + } + _ = file.Close() + + aptlyCtx, router, err := newPprofContext(file.Name()) + if err != nil { + t.Fatal(err) + } + + return httptest.NewServer(router), aptlyCtx +} + +// runPprof runs "go tool pprof -top " and returns the combined output. +func runPprof(t *testing.T, url string) string { + t.Helper() + cmd := exec.Command("go", "tool", "pprof", "-top", url) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("go tool pprof -top %s failed: %v\noutput:\n%s", url, err, out) + } + return string(out) +} + +// TestPprofToolGoroutine verifies that go tool pprof can fetch and parse the +// goroutine profile from the /api/debug/pprof/goroutine endpoint. +func TestPprofToolGoroutine(t *testing.T) { + server, aptlyCtx := pprofTestServer(t) + defer server.Close() + defer aptlyCtx.Shutdown() + + url := fmt.Sprintf("%s/api/debug/pprof/goroutine", server.URL) + out := runPprof(t, url) + t.Logf("go tool pprof goroutine output:\n%s", out) + + if !strings.Contains(out, "Type: goroutine") { + t.Errorf("expected 'Type: goroutine' in output, got:\n%s", out) + } + if !strings.Contains(out, "flat") { + t.Errorf("expected 'flat' column header in output, got:\n%s", out) + } +} + +// TestPprofToolHeap verifies that go tool pprof can fetch and parse the heap +// profile from the /api/debug/pprof/heap endpoint. +func TestPprofToolHeap(t *testing.T) { + server, aptlyCtx := pprofTestServer(t) + defer server.Close() + defer aptlyCtx.Shutdown() + + url := fmt.Sprintf("%s/api/debug/pprof/heap", server.URL) + out := runPprof(t, url) + t.Logf("go tool pprof heap output:\n%s", out) + + // Heap profiles are typed as "inuse_space" by default. + if !strings.Contains(out, "Type: inuse_space") { + t.Errorf("expected 'Type: inuse_space' in output, got:\n%s", out) + } + if !strings.Contains(out, "flat") { + t.Errorf("expected 'flat' column header in output, got:\n%s", out) + } +} + +// TestPprofToolCmdline verifies that the cmdline endpoint is reachable. +// cmdline returns plain text (the process argv), not a pprof binary profile, +// so we confirm the HTTP fetch succeeds rather than parsing it as a profile. +func TestPprofToolCmdline(t *testing.T) { + server, aptlyCtx := pprofTestServer(t) + defer server.Close() + defer aptlyCtx.Shutdown() + + url := fmt.Sprintf("%s/api/debug/pprof/cmdline", server.URL) + + resp, err := server.Client().Get(url) + if err != nil { + t.Fatalf("GET %s: %v", url, err) + } + defer func() { _ = resp.Body.Close() }() + + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) + } + t.Logf("cmdline endpoint status: %d", resp.StatusCode) +} diff --git a/api/pprof_test.go b/api/pprof_test.go new file mode 100644 index 000000000..704c2eb9d --- /dev/null +++ b/api/pprof_test.go @@ -0,0 +1,183 @@ +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + + "github.com/aptly-dev/aptly/aptly" + ctx "github.com/aptly-dev/aptly/context" + "github.com/gin-gonic/gin" + "github.com/smira/flag" + . "gopkg.in/check.v1" +) + +// newPprofContext builds an aptly context and router from the given config file +// path. It returns an error if context initialisation fails. The caller is +// responsible for calling aptlyCtx.Shutdown() when done. +func newPprofContext(configPath string) (*ctx.AptlyContext, http.Handler, error) { + flags := flag.NewFlagSet("fakeFlags", flag.ContinueOnError) + flags.Bool("no-lock", false, "dummy") + flags.Int("db-open-attempts", 3, "dummy") + flags.String("config", configPath, "dummy") + flags.String("architectures", "", "dummy") + + aptlyCtx, err := ctx.NewContext(flags) + if err != nil { + return nil, nil, err + } + return aptlyCtx, Router(aptlyCtx), nil +} + +// PprofEnabledSuite tests pprof endpoints when EnablePprofEndpoint is true. +// It uses a dedicated router built from a config that enables only the pprof +// endpoint, ensuring no interaction with other optional features. +type PprofEnabledSuite struct { + configFile *os.File + aptlyCtx *ctx.AptlyContext + router http.Handler +} + +var _ = Suite(&PprofEnabledSuite{}) + +func (s *PprofEnabledSuite) SetUpSuite(c *C) { + aptly.Version = "testVersion" + + file, err := os.CreateTemp("", "aptly-pprof") + c.Assert(err, IsNil) + s.configFile = file + + jsonString, err := json.Marshal(gin.H{ + "architectures": []string{}, + "enablePprofEndpoint": true, + }) + c.Assert(err, IsNil) + _, err = file.Write(jsonString) + c.Assert(err, IsNil) + _ = file.Close() + + aptlyCtx, router, err := newPprofContext(file.Name()) + c.Assert(err, IsNil) + s.aptlyCtx = aptlyCtx + s.router = router +} + +func (s *PprofEnabledSuite) TearDownSuite(c *C) { + if s.configFile != nil { + _ = os.Remove(s.configFile.Name()) + } + if s.aptlyCtx != nil { + s.aptlyCtx.Shutdown() + } +} + +func (s *PprofEnabledSuite) request(method, url string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + req, err := http.NewRequest(method, url, nil) + if err != nil { + panic(err) + } + s.router.ServeHTTP(w, req) + return w +} + +// TestPprofIndexReturns200 verifies the pprof index page is served when the +// endpoint is enabled. +func (s *PprofEnabledSuite) TestPprofIndexReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/") + c.Check(w.Code, Equals, 200) + c.Check(w.Header().Get("Content-Type"), Matches, "text/html.*") +} + +// TestPprofGoroutineReturns200 verifies the goroutine sub-profile is served. +func (s *PprofEnabledSuite) TestPprofGoroutineReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/goroutine?debug=1") + c.Check(w.Code, Equals, 200) +} + +// TestPprofHeapReturns200 verifies the heap sub-profile is served. +func (s *PprofEnabledSuite) TestPprofHeapReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/heap?debug=1") + c.Check(w.Code, Equals, 200) +} + +// TestPprofCmdlineReturns200 verifies the cmdline handler is served. +func (s *PprofEnabledSuite) TestPprofCmdlineReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/cmdline") + c.Check(w.Code, Equals, 200) +} + +// TestPprofSymbolReturns200 verifies the symbol lookup handler responds (GET +// with no addresses returns 200 with a count of 0 symbols found). +func (s *PprofEnabledSuite) TestPprofSymbolReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/symbol") + c.Check(w.Code, Equals, 200) +} + +// TestPprofTraceReturns200 verifies the execution trace handler is wired +// correctly. The minimum trace duration is 1 second, so this test blocks +// briefly but confirms end-to-end handler registration. +func (s *PprofEnabledSuite) TestPprofTraceReturns200(c *C) { + w := s.request("GET", "/api/debug/pprof/trace?seconds=1") + c.Check(w.Code, Equals, 200) + c.Check(w.Header().Get("Content-Type"), Equals, "application/octet-stream") +} + +// TestNonPprofRoutesUnaffectedWhenEnabled verifies that existing API routes +// continue to work correctly when the pprof endpoint is enabled. +func (s *PprofEnabledSuite) TestNonPprofRoutesUnaffectedWhenEnabled(c *C) { + w := s.request("GET", "/api/version") + c.Check(w.Code, Equals, 200) +} + +// PprofDisabledSuite tests that pprof endpoints are NOT exposed when the config +// option is off. It embeds APISuite, whose config does not set +// enablePprofEndpoint, confirming the default state is safe. The inherited +// APISuite tests also run here, verifying that normal API operation is +// unaffected when pprof is disabled. +type PprofDisabledSuite struct { + APISuite +} + +var _ = Suite(&PprofDisabledSuite{}) + +// TestPprofIndexNotExposed verifies the pprof index is not reachable when +// enablePprofEndpoint is false (the default). +func (s *PprofDisabledSuite) TestPprofIndexNotExposed(c *C) { + response, err := s.HTTPRequest("GET", "/api/debug/pprof/", nil) + c.Assert(err, IsNil) + c.Check(response.Code, Equals, 404) +} + +// TestPprofGoroutineNotExposed verifies the goroutine profile is not reachable +// when disabled. +func (s *PprofDisabledSuite) TestPprofGoroutineNotExposed(c *C) { + response, err := s.HTTPRequest("GET", "/api/debug/pprof/goroutine", nil) + c.Assert(err, IsNil) + c.Check(response.Code, Equals, 404) +} + +// TestPprofHeapNotExposed verifies the heap profile is not reachable when +// disabled. +func (s *PprofDisabledSuite) TestPprofHeapNotExposed(c *C) { + response, err := s.HTTPRequest("GET", "/api/debug/pprof/heap", nil) + c.Assert(err, IsNil) + c.Check(response.Code, Equals, 404) +} + +// TestPprofCmdlineNotExposed verifies the cmdline endpoint is not reachable +// when disabled. +func (s *PprofDisabledSuite) TestPprofCmdlineNotExposed(c *C) { + response, err := s.HTTPRequest("GET", "/api/debug/pprof/cmdline", nil) + c.Assert(err, IsNil) + c.Check(response.Code, Equals, 404) +} + +// TestPprofTraceNotExposed verifies the trace endpoint is not reachable when +// disabled. +func (s *PprofDisabledSuite) TestPprofTraceNotExposed(c *C) { + response, err := s.HTTPRequest("GET", "/api/debug/pprof/trace", nil) + c.Assert(err, IsNil) + c.Check(response.Code, Equals, 404) +} diff --git a/api/router.go b/api/router.go index 1818b2a12..4d90db6b6 100644 --- a/api/router.go +++ b/api/router.go @@ -2,6 +2,8 @@ package api import ( "net/http" + "net/http/pprof" + "strings" "sync/atomic" "github.com/aptly-dev/aptly/aptly" @@ -82,6 +84,27 @@ func Router(c *ctx.AptlyContext) http.Handler { MetricsCollectorRegistrar.Register(router) } + if c.Config().EnablePprofEndpoint { + pprofMux := http.NewServeMux() + pprofMux.HandleFunc("/debug/pprof/", pprof.Index) + pprofMux.HandleFunc("/debug/pprof/cmdline", pprof.Cmdline) + pprofMux.HandleFunc("/debug/pprof/profile", pprof.Profile) + pprofMux.HandleFunc("/debug/pprof/symbol", pprof.Symbol) + pprofMux.HandleFunc("/debug/pprof/trace", pprof.Trace) + + router.Any("/api/debug/pprof/*any", func(c *gin.Context) { + origPath := c.Request.URL.Path + origRawPath := c.Request.URL.RawPath + c.Request.URL.Path = strings.TrimPrefix(origPath, "/api") + if origRawPath != "" { + c.Request.URL.RawPath = strings.TrimPrefix(origRawPath, "/api") + } + pprofMux.ServeHTTP(c.Writer, c.Request) + c.Request.URL.Path = origPath + c.Request.URL.RawPath = origRawPath + }) + } + if c.Config().ServeInAPIMode { router.GET("/repos/", reposListInAPIMode(c.Config().FileSystemPublishRoots)) router.GET("/repos/:storage/*pkgPath", reposServeInAPIMode) diff --git a/debian/aptly.conf b/debian/aptly.conf index 043b46ebe..bd009e913 100644 --- a/debian/aptly.conf +++ b/debian/aptly.conf @@ -86,6 +86,9 @@ enable_metrics_endpoint: false # Enable API documentation on /docs enable_swagger_endpoint: false +# Enable pprof profiling endpoints on /api/debug/pprof/* +enable_pprof_endpoint: false + # OBSOLETE: use via url param ?_async=true async_api: false diff --git a/system/t02_config/ConfigShowTest_gold b/system/t02_config/ConfigShowTest_gold index 8f12e639a..1dc6619ee 100644 --- a/system/t02_config/ConfigShowTest_gold +++ b/system/t02_config/ConfigShowTest_gold @@ -16,6 +16,7 @@ "serveInAPIMode": true, "enableMetricsEndpoint": true, "enableSwaggerEndpoint": false, + "enablePprofEndpoint": false, "AsyncAPI": false, "databaseBackend": { "type": "", diff --git a/system/t02_config/ConfigShowYAMLTest_gold b/system/t02_config/ConfigShowYAMLTest_gold index 942e4233b..1d746a584 100644 --- a/system/t02_config/ConfigShowYAMLTest_gold +++ b/system/t02_config/ConfigShowYAMLTest_gold @@ -15,6 +15,7 @@ ppa_baseurl: http://ppa.launchpad.net serve_in_api_mode: true enable_metrics_endpoint: true enable_swagger_endpoint: false +enable_pprof_endpoint: false async_api: false database_backend: type: "" diff --git a/system/t02_config/CreateConfigTest_gold b/system/t02_config/CreateConfigTest_gold index d8e39c9dc..13b5cd62d 100644 --- a/system/t02_config/CreateConfigTest_gold +++ b/system/t02_config/CreateConfigTest_gold @@ -86,6 +86,9 @@ enable_metrics_endpoint: false # Enable API documentation on /docs enable_swagger_endpoint: false +# Enable pprof profiling endpoints on /api/debug/pprof/* +enable_pprof_endpoint: false + # OBSOLETE: use via url param ?_async=true async_api: false diff --git a/utils/config.go b/utils/config.go index 1c148e4f1..c99fba86d 100644 --- a/utils/config.go +++ b/utils/config.go @@ -37,6 +37,7 @@ type ConfigStructure struct { // nolint: maligned ServeInAPIMode bool `json:"serveInAPIMode" yaml:"serve_in_api_mode"` EnableMetricsEndpoint bool `json:"enableMetricsEndpoint" yaml:"enable_metrics_endpoint"` EnableSwaggerEndpoint bool `json:"enableSwaggerEndpoint" yaml:"enable_swagger_endpoint"` + EnablePprofEndpoint bool `json:"enablePprofEndpoint" yaml:"enable_pprof_endpoint"` AsyncAPI bool `json:"AsyncAPI" yaml:"async_api"` // OBSOLETE // Database diff --git a/utils/config_test.go b/utils/config_test.go index abc59e492..c5f64df11 100644 --- a/utils/config_test.go +++ b/utils/config_test.go @@ -95,6 +95,7 @@ func (s *ConfigSuite) TestSaveConfig(c *C) { "serveInAPIMode": false, "enableMetricsEndpoint": false, "enableSwaggerEndpoint": false, + "enablePprofEndpoint": false, "AsyncAPI": false, "databaseBackend": { "type": "", @@ -291,6 +292,7 @@ func (s *ConfigSuite) TestSaveYAML2Config(c *C) { "serve_in_api_mode: false\n"+ "enable_metrics_endpoint: false\n"+ "enable_swagger_endpoint: false\n"+ + "enable_pprof_endpoint: false\n"+ "async_api: false\n"+ "database_backend:\n"+ " type: \"\"\n"+ @@ -350,6 +352,7 @@ ppa_baseurl: http://ppa.launchpad.net serve_in_api_mode: true enable_metrics_endpoint: true enable_swagger_endpoint: true +enable_pprof_endpoint: false async_api: true database_backend: type: etcd