From 851606d245f8867dceb7bf179fb50377cc3792d8 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Thu, 3 Nov 2022 13:04:33 -0500 Subject: [PATCH] core/web: Router don't panic (#7847) --- core/cmd/client.go | 6 +++++- core/internal/cltest/cltest.go | 7 +------ core/web/auth/auth_test.go | 8 ++++---- core/web/helpers.go | 11 +++++++++++ core/web/router.go | 8 ++++---- core/web/router_test.go | 14 +++++++------- 6 files changed, 32 insertions(+), 22 deletions(-) diff --git a/core/cmd/client.go b/core/cmd/client.go index ed6beb6f787..4e32382f0e2 100644 --- a/core/cmd/client.go +++ b/core/cmd/client.go @@ -356,7 +356,11 @@ func (n ChainlinkRunner) Run(ctx context.Context, app chainlink.Application) err return errors.New("You must specify at least one port to listen on") } - server := server{handler: web.Router(app.(*chainlink.ChainlinkApplication), prometheus), lggr: app.GetLogger()} + handler, err := web.NewRouter(app, prometheus) + if err != nil { + return errors.Wrap(err, "failed to create web router") + } + server := server{handler: handler, lggr: app.GetLogger()} g, gCtx := errgroup.WithContext(ctx) if config.Port() != 0 { diff --git a/core/internal/cltest/cltest.go b/core/internal/cltest/cltest.go index 335711f43c2..96d22602344 100644 --- a/core/internal/cltest/cltest.go +++ b/core/internal/cltest/cltest.go @@ -549,7 +549,7 @@ func NewApplicationWithConfig(t testing.TB, cfg config.GeneralConfig, flagsAndDe ChainlinkApplication: app, Logger: lggr, } - ta.Server = newServer(ta) + ta.Server = httptest.NewServer(web.Router(t, app, nil)) if !useRealExternalInitiatorManager { app.ExternalInitiatorManager = externalInitiatorManager @@ -624,11 +624,6 @@ func NewEthMocksWithTransactionsOnBlocksAssertions(t testing.TB) *evmMocks.Clien return c } -func newServer(app chainlink.Application) *httptest.Server { - engine := web.Router(app, nil) - return httptest.NewServer(engine) -} - // Start starts the chainlink app and registers Stop to clean up at end of test. func (ta *TestApplication) Start(ctx context.Context) error { ta.t.Helper() diff --git a/core/web/auth/auth_test.go b/core/web/auth/auth_test.go index a12df231f00..3b6190e1519 100644 --- a/core/web/auth/auth_test.go +++ b/core/web/auth/auth_test.go @@ -318,7 +318,7 @@ func TestRBAC_Routemap_Admin(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -355,7 +355,7 @@ func TestRBAC_Routemap_Edit(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -401,7 +401,7 @@ func TestRBAC_Routemap_Run(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -447,7 +447,7 @@ func TestRBAC_Routemap_ViewOnly(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() diff --git a/core/web/helpers.go b/core/web/helpers.go index 82ddad99605..5063259a626 100644 --- a/core/web/helpers.go +++ b/core/web/helpers.go @@ -4,10 +4,15 @@ import ( "database/sql" "fmt" "net/http" + "testing" + "github.com/Depado/ginprom" "github.com/gin-gonic/gin" "github.com/manyminds/api2go/jsonapi" "github.com/pkg/errors" + "github.com/stretchr/testify/require" + + "github.com/smartcontractkit/chainlink/core/services/chainlink" "github.com/smartcontractkit/chainlink/core/store/models" ) @@ -68,3 +73,9 @@ func jsonAPIResponseWithStatus(c *gin.Context, resource interface{}, name string func jsonAPIResponse(c *gin.Context, resource interface{}, name string) { jsonAPIResponseWithStatus(c, resource, name, http.StatusOK) } + +func Router(t testing.TB, app chainlink.Application, prometheus *ginprom.Prometheus) *gin.Engine { + r, err := NewRouter(app, prometheus) + require.NoError(t, err) + return r +} diff --git a/core/web/router.go b/core/web/router.go index 25201cc58c4..4cb0062c6c5 100644 --- a/core/web/router.go +++ b/core/web/router.go @@ -38,13 +38,13 @@ import ( "github.com/smartcontractkit/chainlink/core/web/schema" ) -// Router listens and responds to requests to the node for valid paths. -func Router(app chainlink.Application, prometheus *ginprom.Prometheus) *gin.Engine { +// NewRouter returns *gin.Engine router that listens and responds to requests to the node for valid paths. +func NewRouter(app chainlink.Application, prometheus *ginprom.Prometheus) (*gin.Engine, error) { engine := gin.New() config := app.GetConfig() secret, err := app.SecretGenerator().Generate(config.RootDir()) if err != nil { - app.GetLogger().Panic(err) + return nil, err } sessionStore := sessions.NewCookieStore(secret) sessionStore.Options(config.SessionOptions()) @@ -87,7 +87,7 @@ func Router(app chainlink.Application, prometheus *ginprom.Prometheus) *gin.Engi graphqlHandler(app), ) - return engine + return engine, nil } // Defining the Graphql handler diff --git a/core/web/router_test.go b/core/web/router_test.go index a3da53a1426..d3ac15bc526 100644 --- a/core/web/router_test.go +++ b/core/web/router_test.go @@ -21,7 +21,7 @@ func TestTokenAuthRequired_NoCredentials(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -35,7 +35,7 @@ func TestTokenAuthRequired_SessionCredentials(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -50,7 +50,7 @@ func TestTokenAuthRequired_TokenCredentials(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -82,7 +82,7 @@ func TestTokenAuthRequired_BadTokenCredentials(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -114,7 +114,7 @@ func TestSessions_RateLimited(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -142,7 +142,7 @@ func TestRouter_LargePOSTBody(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() @@ -161,7 +161,7 @@ func TestRouter_GinHelmetHeaders(t *testing.T) { app := cltest.NewApplicationEVMDisabled(t) require.NoError(t, app.Start(testutils.Context(t))) - router := web.Router(app, nil) + router := web.Router(t, app, nil) ts := httptest.NewServer(router) defer ts.Close() res, err := http.Get(ts.URL)