debug api basic and full routing (#1358)

pkg/debugapi/debugapi.go

@@ -10,6 +10,7 @@ package debugapi
 import (
 	"crypto/ecdsa"
 	"net/http"
+	"sync"
 	"unicode/utf8"
 
 	"github.com/ethereum/go-ethereum/common"
@@ -30,33 +31,32 @@ import (
 
 type Service interface {
 	http.Handler
+	Configure(p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, storer storage.Storer, tags *tags.Tags, accounting accounting.Interface, settlement settlement.Interface, chequebookEnabled bool, swap swap.ApiInterface, chequebook chequebook.Service)
 	MustRegisterMetrics(cs ...prometheus.Collector)
 }
 
 type server struct {
-	Overlay           swarm.Address
-	PublicKey         ecdsa.PublicKey
-	PSSPublicKey      ecdsa.PublicKey
-	EthereumAddress   common.Address
-	P2P               p2p.DebugService
-	Pingpong          pingpong.Interface
-	TopologyDriver    topology.Driver
-	Storer            storage.Storer
-	Logger            logging.Logger
-	Tracer            *tracing.Tracer
-	Tags              *tags.Tags
-	Accounting        accounting.Interface
-	Settlement        settlement.Interface
-	ChequebookEnabled bool
-	Chequebook        chequebook.Service
-	Swap              swap.ApiInterface
-	metricsRegistry   *prometheus.Registry
-	Options
-	http.Handler
-}
-
-type Options struct {
+	Overlay            swarm.Address
+	PublicKey          ecdsa.PublicKey
+	PSSPublicKey       ecdsa.PublicKey
+	EthereumAddress    common.Address
+	P2P                p2p.DebugService
+	Pingpong           pingpong.Interface
+	TopologyDriver     topology.Driver
+	Storer             storage.Storer
+	Logger             logging.Logger
+	Tracer             *tracing.Tracer
+	Tags               *tags.Tags
+	Accounting         accounting.Interface
+	Settlement         settlement.Interface
+	ChequebookEnabled  bool
+	Chequebook         chequebook.Service
+	Swap               swap.ApiInterface
 	CORSAllowedOrigins []string
+	metricsRegistry    *prometheus.Registry
+	// handler is changed in the Configure method
+	handler   http.Handler
+	handlerMu sync.RWMutex
 }
 
 // checkOrigin returns true if the origin is not set or is equal to the request host.
@@ -103,28 +103,49 @@ func equalASCIIFold(s, t string) bool {
 	return s == t
 }
 
-func New(overlay swarm.Address, publicKey, pssPublicKey ecdsa.PublicKey, ethereumAddress common.Address, p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, storer storage.Storer, logger logging.Logger, tracer *tracing.Tracer, tags *tags.Tags, accounting accounting.Interface, settlement settlement.Interface, chequebookEnabled bool, swap swap.ApiInterface, chequebook chequebook.Service, o Options) Service {
-	s := &server{
-		Overlay:           overlay,
-		PublicKey:         publicKey,
-		PSSPublicKey:      pssPublicKey,
-		EthereumAddress:   ethereumAddress,
-		P2P:               p2p,
-		Pingpong:          pingpong,
-		TopologyDriver:    topologyDriver,
-		Storer:            storer,
-		Logger:            logger,
-		Tracer:            tracer,
-		Tags:              tags,
-		Accounting:        accounting,
-		Settlement:        settlement,
-		metricsRegistry:   newMetricsRegistry(),
-		ChequebookEnabled: chequebookEnabled,
-		Chequebook:        chequebook,
-		Swap:              swap,
-	}
+// New creates a new Debug API Service with only basic routers enabled in order
+// to expose /addresses, /health endpoints, Go metrics and pprof. It is useful to expose
+// these endpoints before all dependencies are configured and injected to have
+// access to basic debugging tools and /health endpoint.
+func New(overlay swarm.Address, publicKey, pssPublicKey ecdsa.PublicKey, ethereumAddress common.Address, logger logging.Logger, tracer *tracing.Tracer, corsAllowedOrigins []string) Service {
+	s := new(server)
+	s.Overlay = overlay
+	s.PublicKey = publicKey
+	s.PSSPublicKey = pssPublicKey
+	s.EthereumAddress = ethereumAddress
+	s.Logger = logger
+	s.Tracer = tracer
+	s.CORSAllowedOrigins = corsAllowedOrigins
+	s.metricsRegistry = newMetricsRegistry()
 
-	s.setupRouting()
+	s.setRouter(s.newBasicRouter())
 
 	return s
 }
+
+// Configure injects required dependencies and configuration parameters and
+// constructs HTTP routes that depend on them. It is intended and safe to call
+// this method only once.
+func (s *server) Configure(p2p p2p.DebugService, pingpong pingpong.Interface, topologyDriver topology.Driver, storer storage.Storer, tags *tags.Tags, accounting accounting.Interface, settlement settlement.Interface, chequebookEnabled bool, swap swap.ApiInterface, chequebook chequebook.Service) {
+	s.P2P = p2p
+	s.Pingpong = pingpong
+	s.TopologyDriver = topologyDriver
+	s.Storer = storer
+	s.Tags = tags
+	s.Accounting = accounting
+	s.Settlement = settlement
+	s.ChequebookEnabled = chequebookEnabled
+	s.Chequebook = chequebook
+	s.Swap = swap
+
+	s.setRouter(s.newRouter())
+}
+
+func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	// protect handler as it is changed by the Configure method
+	s.handlerMu.RLock()
+	h := s.handler
+	s.handlerMu.RUnlock()
+
+	h.ServeHTTP(w, r)
+}

pkg/debugapi/debugapi_test.go

@@ -6,6 +6,7 @@ package debugapi_test
 
 import (
 	"crypto/ecdsa"
+	"encoding/hex"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -13,9 +14,14 @@ import (
 	"testing"
 
 	"github.com/ethereum/go-ethereum/common"
+	"github.com/ethersphere/bee"
 	accountingmock "github.com/ethersphere/bee/pkg/accounting/mock"
+	"github.com/ethersphere/bee/pkg/crypto"
 	"github.com/ethersphere/bee/pkg/debugapi"
+	"github.com/ethersphere/bee/pkg/jsonhttp"
+	"github.com/ethersphere/bee/pkg/jsonhttp/jsonhttptest"
 	"github.com/ethersphere/bee/pkg/logging"
+	"github.com/ethersphere/bee/pkg/p2p/mock"
 	p2pmock "github.com/ethersphere/bee/pkg/p2p/mock"
 	"github.com/ethersphere/bee/pkg/pingpong"
 	"github.com/ethersphere/bee/pkg/resolver"
@@ -57,7 +63,8 @@ func newTestServer(t *testing.T, o testServerOptions) *testServer {
 	settlement := swapmock.New(o.SettlementOpts...)
 	chequebook := chequebookmock.NewChequebook(o.ChequebookOpts...)
 	swapserv := swapmock.NewApiInterface(o.SwapOpts...)
-	s := debugapi.New(o.Overlay, o.PublicKey, o.PSSPublicKey, o.EthereumAddress, o.P2P, o.Pingpong, topologyDriver, o.Storer, logging.New(ioutil.Discard, 0), nil, o.Tags, acc, settlement, true, swapserv, chequebook, debugapi.Options{})
+	s := debugapi.New(o.Overlay, o.PublicKey, o.PSSPublicKey, o.EthereumAddress, logging.New(ioutil.Discard, 0), nil, nil)
+	s.Configure(o.P2P, o.Pingpong, topologyDriver, o.Storer, o.Tags, acc, settlement, true, swapserv, chequebook)
 	ts := httptest.NewServer(s)
 	t.Cleanup(ts.Close)
 
@@ -86,3 +93,113 @@ func mustMultiaddr(t *testing.T, s string) multiaddr.Multiaddr {
 	}
 	return a
 }
+
+// TestServer_Configure validates that http routes are correct when server is
+// constructed with only basic routes and after it is configured with
+// dependencies.
+func TestServer_Configure(t *testing.T) {
+	privateKey, err := crypto.GenerateSecp256k1Key()
+	if err != nil {
+		t.Fatal(err)
+	}
+	pssPrivateKey, err := crypto.GenerateSecp256k1Key()
+	if err != nil {
+		t.Fatal(err)
+	}
+	overlay := swarm.MustParseHexAddress("ca1e9f3938cc1425c6061b96ad9eb93e134dfe8734ad490164ef20af9d1cf59c")
+	addresses := []multiaddr.Multiaddr{
+		mustMultiaddr(t, "/ip4/127.0.0.1/tcp/7071/p2p/16Uiu2HAmTBuJT9LvNmBiQiNoTsxE5mtNy6YG3paw79m94CRa9sRb"),
+		mustMultiaddr(t, "/ip4/192.168.0.101/tcp/7071/p2p/16Uiu2HAmTBuJT9LvNmBiQiNoTsxE5mtNy6YG3paw79m94CRa9sRb"),
+		mustMultiaddr(t, "/ip4/127.0.0.1/udp/7071/quic/p2p/16Uiu2HAmTBuJT9LvNmBiQiNoTsxE5mtNy6YG3paw79m94CRa9sRb"),
+	}
+
+	ethereumAddress := common.HexToAddress("abcd")
+
+	o := testServerOptions{
+		PublicKey:       privateKey.PublicKey,
+		PSSPublicKey:    pssPrivateKey.PublicKey,
+		Overlay:         overlay,
+		EthereumAddress: ethereumAddress,
+		P2P: mock.New(mock.WithAddressesFunc(func() ([]multiaddr.Multiaddr, error) {
+			return addresses, nil
+		})),
+	}
+	topologyDriver := topologymock.NewTopologyDriver(o.TopologyOpts...)
+	acc := accountingmock.NewAccounting(o.AccountingOpts...)
+	settlement := swapmock.New(o.SettlementOpts...)
+	chequebook := chequebookmock.NewChequebook(o.ChequebookOpts...)
+	swapserv := swapmock.NewApiInterface(o.SwapOpts...)
+	s := debugapi.New(o.Overlay, o.PublicKey, o.PSSPublicKey, o.EthereumAddress, logging.New(ioutil.Discard, 0), nil, nil)
+	ts := httptest.NewServer(s)
+	t.Cleanup(ts.Close)
+
+	client := &http.Client{
+		Transport: web.RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
+			u, err := url.Parse(ts.URL + r.URL.String())
+			if err != nil {
+				return nil, err
+			}
+			r.URL = u
+			return ts.Client().Transport.RoundTrip(r)
+		}),
+	}
+
+	testBasicRouter(t, client)
+	jsonhttptest.Request(t, client, http.MethodGet, "/readiness", http.StatusNotFound,
+		jsonhttptest.WithExpectedJSONResponse(jsonhttp.StatusResponse{
+			Message: http.StatusText(http.StatusNotFound),
+			Code:    http.StatusNotFound,
+		}),
+	)
+	jsonhttptest.Request(t, client, http.MethodGet, "/addresses", http.StatusOK,
+		jsonhttptest.WithExpectedJSONResponse(debugapi.AddressesResponse{
+			Overlay:      o.Overlay,
+			Underlay:     make([]multiaddr.Multiaddr, 0),
+			Ethereum:     o.EthereumAddress,
+			PublicKey:    hex.EncodeToString(crypto.EncodeSecp256k1PublicKey(&o.PublicKey)),
+			PSSPublicKey: hex.EncodeToString(crypto.EncodeSecp256k1PublicKey(&o.PSSPublicKey)),
+		}),
+	)
+
+	s.Configure(o.P2P, o.Pingpong, topologyDriver, o.Storer, o.Tags, acc, settlement, true, swapserv, chequebook)
+
+	testBasicRouter(t, client)
+	jsonhttptest.Request(t, client, http.MethodGet, "/readiness", http.StatusOK,
+		jsonhttptest.WithExpectedJSONResponse(debugapi.StatusResponse{
+			Status:  "ok",
+			Version: bee.Version,
+		}),
+	)
+	jsonhttptest.Request(t, client, http.MethodGet, "/addresses", http.StatusOK,
+		jsonhttptest.WithExpectedJSONResponse(debugapi.AddressesResponse{
+			Overlay:      o.Overlay,
+			Underlay:     addresses,
+			Ethereum:     o.EthereumAddress,
+			PublicKey:    hex.EncodeToString(crypto.EncodeSecp256k1PublicKey(&o.PublicKey)),
+			PSSPublicKey: hex.EncodeToString(crypto.EncodeSecp256k1PublicKey(&o.PSSPublicKey)),
+		}),
+	)
+}
+
+func testBasicRouter(t *testing.T, client *http.Client) {
+	t.Helper()
+
+	jsonhttptest.Request(t, client, http.MethodGet, "/health", http.StatusOK,
+		jsonhttptest.WithExpectedJSONResponse(debugapi.StatusResponse{
+			Status:  "ok",
+			Version: bee.Version,
+		}),
+	)
+
+	for _, path := range []string{
+		"/metrics",
+		"/debug/pprof",
+		"/debug/pprof/cmdline",
+		"/debug/pprof/profile?seconds=1", // profile for only 1 second to check only the status code
+		"/debug/pprof/symbol",
+		"/debug/pprof/trace",
+		"/debug/vars",
+	} {
+		jsonhttptest.Request(t, client, http.MethodGet, path, http.StatusOK)
+	}
+}

pkg/debugapi/p2p.go

@@ -24,11 +24,18 @@ type addressesResponse struct {
 }
 
 func (s *server) addressesHandler(w http.ResponseWriter, r *http.Request) {
-	underlay, err := s.P2P.Addresses()
-	if err != nil {
-		s.Logger.Debugf("debug api: p2p addresses: %v", err)
-		jsonhttp.InternalServerError(w, err)
-		return
+	// initialize variable to json encode as [] instead null if p2p is nil
+	underlay := make([]multiaddr.Multiaddr, 0)
+	// addresses endpoint is exposed before p2p service is configured
+	// to provide information about other addresses.
+	if s.P2P != nil {
+		u, err := s.P2P.Addresses()
+		if err != nil {
+			s.Logger.Debugf("debug api: p2p addresses: %v", err)
+			jsonhttp.InternalServerError(w, err)
+			return
+		}
+		underlay = u
 	}
 	jsonhttp.OK(w, addressesResponse{
 		Overlay:      s.Overlay,

pkg/debugapi/router.go

@@ -19,10 +19,17 @@ import (
 	"github.com/ethersphere/bee/pkg/logging/httpaccess"
 )
 
-func (s *server) setupRouting() {
-	baseRouter := http.NewServeMux()
+// newBasicRouter constructs only the routes that do not depend on the injected dependencies:
+// - /health
+// - pprof
+// - vars
+// - metrics
+// - /addresses
+func (s *server) newBasicRouter() *mux.Router {
+	router := mux.NewRouter()
+	router.NotFoundHandler = http.HandlerFunc(jsonhttp.NotFoundHandler)
 
-	baseRouter.Handle("/metrics", web.ChainHandlers(
+	router.Path("/metrics").Handler(web.ChainHandlers(
 		httpaccess.SetAccessLogLevelHandler(0), // suppress access log messages
 		web.FinalHandler(promhttp.InstrumentMetricHandler(
 			s.metricsRegistry,
@@ -30,9 +37,6 @@ func (s *server) setupRouting() {
 		)),
 	))
 
-	router := mux.NewRouter()
-	router.NotFoundHandler = http.HandlerFunc(jsonhttp.NotFoundHandler)
-
 	router.Handle("/debug/pprof", http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		u := r.URL
 		u.Path += "/"
@@ -48,20 +52,31 @@ func (s *server) setupRouting() {
 
 	router.Handle("/health", web.ChainHandlers(
 		httpaccess.SetAccessLogLevelHandler(0), // suppress access log messages
-		web.FinalHandlerFunc(s.statusHandler),
+		web.FinalHandlerFunc(statusHandler),
 	))
+
+	router.Handle("/addresses", jsonhttp.MethodHandler{
+		"GET": http.HandlerFunc(s.addressesHandler),
+	})
+
+	return router
+}
+
+// newRouter construct the complete set of routes after all of the dependencies
+// are injected and exposes /readiness endpoint to provide information that
+// Debug API is fully active.
+func (s *server) newRouter() *mux.Router {
+	router := s.newBasicRouter()
+
 	router.Handle("/readiness", web.ChainHandlers(
 		httpaccess.SetAccessLogLevelHandler(0), // suppress access log messages
-		web.FinalHandlerFunc(s.statusHandler),
+		web.FinalHandlerFunc(statusHandler),
 	))
 
 	router.Handle("/pingpong/{peer-id}", jsonhttp.MethodHandler{
 		"POST": http.HandlerFunc(s.pingpongHandler),
 	})
 
-	router.Handle("/addresses", jsonhttp.MethodHandler{
-		"GET": http.HandlerFunc(s.addressesHandler),
-	})
 	router.Handle("/connect/{multi-address:.+}", jsonhttp.MethodHandler{
 		"POST": http.HandlerFunc(s.peerConnectHandler),
 	})
@@ -149,7 +164,13 @@ func (s *server) setupRouting() {
 		"GET": http.HandlerFunc(s.getTagHandler),
 	})
 
-	baseRouter.Handle("/", web.ChainHandlers(
+	return router
+}
+
+// setRouter sets the base Debug API handler with common middlewares.
+func (s *server) setRouter(router http.Handler) {
+	h := http.NewServeMux()
+	h.Handle("/", web.ChainHandlers(
 		httpaccess.NewHTTPAccessLogHandler(s.Logger, logrus.InfoLevel, s.Tracer, "debug api access"),
 		handlers.CompressHandler,
 		func(h http.Handler) http.Handler {
@@ -164,10 +185,12 @@ func (s *server) setupRouting() {
 				h.ServeHTTP(w, r)
 			})
 		},
-		// todo: add recovery handler
 		web.NoCacheHeadersHandler,
 		web.FinalHandler(router),
 	))
 
-	s.Handler = baseRouter
+	s.handlerMu.Lock()
+	defer s.handlerMu.Unlock()
+
+	s.handler = h
 }

pkg/debugapi/status.go

@@ -16,7 +16,7 @@ type statusResponse struct {
 	Version string `json:"version"`
 }
 
-func (s *server) statusHandler(w http.ResponseWriter, r *http.Request) {
+func statusHandler(w http.ResponseWriter, r *http.Request) {
 	jsonhttp.OK(w, statusResponse{
 		Status:  "ok",
 		Version: bee.Version,

pkg/node/node.go

@@ -128,6 +128,39 @@ func NewBee(addr string, swarmAddress swarm.Address, publicKey ecdsa.PublicKey,
 		tracerCloser:   tracerCloser,
 	}
 
+	var debugAPIService debugapi.Service
+	if o.DebugAPIAddr != "" {
+		overlayEthAddress, err := signer.EthereumAddress()
+		if err != nil {
+			return nil, fmt.Errorf("eth address: %w", err)
+		}
+		// set up basic debug api endpoints for debugging and /health endpoint
+		debugAPIService = debugapi.New(swarmAddress, publicKey, pssPrivateKey.PublicKey, overlayEthAddress, logger, tracer, o.CORSAllowedOrigins)
+
+		debugAPIListener, err := net.Listen("tcp", o.DebugAPIAddr)
+		if err != nil {
+			return nil, fmt.Errorf("debug api listener: %w", err)
+		}
+
+		debugAPIServer := &http.Server{
+			IdleTimeout:       30 * time.Second,
+			ReadHeaderTimeout: 3 * time.Second,
+			Handler:           debugAPIService,
+			ErrorLog:          log.New(b.errorLogWriter, "", 0),
+		}
+
+		go func() {
+			logger.Infof("debug api address: %s", debugAPIListener.Addr())
+
+			if err := debugAPIServer.Serve(debugAPIListener); err != nil && err != http.ErrServerClosed {
+				logger.Debugf("debug api server: %v", err)
+				logger.Error("unable to serve debug api")
+			}
+		}()
+
+		b.debugAPIServer = debugAPIServer
+	}
+
 	stateStore, err := InitStateStore(logger, o.DataDir)
 	if err != nil {
 		return nil, err
@@ -438,12 +471,7 @@ func NewBee(addr string, swarmAddress swarm.Address, publicKey ecdsa.PublicKey,
 		b.apiCloser = apiService
 	}
 
-	if o.DebugAPIAddr != "" {
-		// Debug API server
-
-		debugAPIService := debugapi.New(swarmAddress, publicKey, pssPrivateKey.PublicKey, overlayEthAddress, p2ps, pingPong, kad, storer, logger, tracer, tagService, acc, settlement, o.SwapEnable, swapService, chequebookService, debugapi.Options{
-			CORSAllowedOrigins: o.CORSAllowedOrigins,
-		})
+	if debugAPIService != nil {
 		// register metrics from components
 		debugAPIService.MustRegisterMetrics(p2ps.Metrics()...)
 		debugAPIService.MustRegisterMetrics(pingPong.Metrics()...)
@@ -470,28 +498,8 @@ func NewBee(addr string, swarmAddress swarm.Address, publicKey ecdsa.PublicKey,
 			debugAPIService.MustRegisterMetrics(l.Metrics()...)
 		}
 
-		debugAPIListener, err := net.Listen("tcp", o.DebugAPIAddr)
-		if err != nil {
-			return nil, fmt.Errorf("debug api listener: %w", err)
-		}
-
-		debugAPIServer := &http.Server{
-			IdleTimeout:       30 * time.Second,
-			ReadHeaderTimeout: 3 * time.Second,
-			Handler:           debugAPIService,
-			ErrorLog:          log.New(b.errorLogWriter, "", 0),
-		}
-
-		go func() {
-			logger.Infof("debug api address: %s", debugAPIListener.Addr())
-
-			if err := debugAPIServer.Serve(debugAPIListener); err != nil && err != http.ErrServerClosed {
-				logger.Debugf("debug api server: %v", err)
-				logger.Error("unable to serve debug api")
-			}
-		}()
-
-		b.debugAPIServer = debugAPIServer
+		// inject dependencies and configure full debug api http path routes
+		debugAPIService.Configure(p2ps, pingPong, kad, storer, tagService, acc, settlement, o.SwapEnable, swapService, chequebookService)
 	}
 
 	if err := kad.Start(p2pCtx); err != nil {