From c34185d893311e31ecfbc72d4e306fb57e56e8fa Mon Sep 17 00:00:00 2001 From: Mike Crute Date: Sun, 22 May 2022 00:59:57 -0700 Subject: echo: completely decouple new/config --- echo/echo_default.go | 299 ++++++++++++++++++++++++++++----------------------- 1 file changed, 166 insertions(+), 133 deletions(-) diff --git a/echo/echo_default.go b/echo/echo_default.go index 6977eed..dcdda63 100644 --- a/echo/echo_default.go +++ b/echo/echo_default.go @@ -7,15 +7,13 @@ import ( "html/template" "io/fs" "net/http" - "os" "sync" "code.crute.us/mcrute/golib/clients/netbox" - glautocert "code.crute.us/mcrute/golib/crypto/acme/autocert" + "code.crute.us/mcrute/golib/crypto/acme/autocert" glmw "code.crute.us/mcrute/golib/echo/middleware" "code.crute.us/mcrute/golib/echo/prometheus" "code.crute.us/mcrute/golib/service" - glservice "code.crute.us/mcrute/golib/service" "github.com/labstack/echo/v4" "github.com/labstack/echo/v4/middleware" @@ -54,13 +52,18 @@ func makeMiddlewareSkipper(c *prometheus.PrometheusConfig) middleware.Skipper { } } +// EchoConfig is the configuration for an EchoWrapper +// +// Service clients are generally passed in an only by interface even +// though we could create these internally so as to provide some +// isolation between what is (theoretically) a re-usable, though +// opinionated, Echo instance vs something that is completely custom to +// internal use-cases. type EchoConfig struct { - Autocert glautocert.PrimingCertProvider + Autocert autocert.PrimingCertProvider NetboxClient netbox.NetboxClient ApplicationName string ApplicationVersion string - DisableServerHeader bool - Debug bool BindAddresses []string BodySizeLimit string TrustedProxyIPRanges []string @@ -77,12 +80,36 @@ type EchoConfig struct { type EchoWrapper struct { *echo.Echo - Autocert glautocert.PrimingCertProvider - middlewareJobs []glservice.RunnerFunc - middlewareInitJobs []glservice.SyncRunnerFunc - servers []*http.Server - tlsServers []*http.Server - templateFS fs.FS + runner *service.AppRunner + autocert autocert.PrimingCertProvider + templateFS fs.FS +} + +// NewEchoWrapper creates a new instance of Echo and wraps it in an +// internal wrapper that does configuration and service running. +// +// It is expected that Configure is called before using this Echo +// instance. +func NewEchoWrapper(ctx context.Context, debug bool) (*EchoWrapper, error) { + e := echo.New() + e.Debug = debug + + e.Logger.SetLevel(log.INFO) + if debug { + e.Logger.SetLevel(log.DEBUG) + } + + e.Use(middleware.Logger()) + e.Use(glmw.Recover()) + + return &EchoWrapper{ + Echo: e, + runner: service.NewAppRunner(ctx, e.Logger), + }, nil +} + +func (w *EchoWrapper) Runner() *service.AppRunner { + return w.runner } func (w *EchoWrapper) CachedStaticRoute(prefix, path string) { @@ -92,214 +119,230 @@ func (w *EchoWrapper) CachedStaticRoute(prefix, path string) { StaticFS(w.GET, w.templateFS, prefix, path, glmw.CacheOneMonthMiddleware) } -func (w *EchoWrapper) makeServerJob(s *http.Server, echoInit bool) glservice.RunnerFunc { - return func(ctx context.Context, wg *sync.WaitGroup) error { - wg.Add(1) - defer wg.Done() +func (w *EchoWrapper) RunForever(enableBackgroundJobs bool) { + w.runner.RunForever(enableBackgroundJobs) +} - w.Logger.Infof("Starting server with address: %s", s.Addr) +func (w *EchoWrapper) GetTemplateFS() fs.FS { + return w.templateFS +} - err := make(chan error) - go func() { - if s.TLSConfig == nil && echoInit { - err <- w.Echo.StartServer(s) - } else if s.TLSConfig == nil && !echoInit { - err <- s.ListenAndServe() - } else { - err <- s.ListenAndServeTLS("", "") - } - }() - select { - case e := <-err: - return e - default: - } +func (w *EchoWrapper) Configure(c EchoConfig) error { + w.configureAutocert(&c) - select { - case <-ctx.Done(): - w.Logger.Info("Shutting down web server") - return s.Shutdown(ctx) - } + if err := w.configureIpExtractor(&c); err != nil { + return err } -} -func (w *EchoWrapper) makeServerJobs() []glservice.RunnerFunc { - out := []glservice.RunnerFunc{} + if err := w.configureTemplates(&c); err != nil { + return err + } - for i, s := range w.servers { - // The first http (not https) server should do an echo.StartServer to - // configure some internal echo state and print the banner (if - // configured). - out = append(out, w.makeServerJob(s, i == 0)) + if err := w.configureCombinedLogging(&c); err != nil { + return err } - for _, s := range w.tlsServers { - out = append(out, w.makeServerJob(s, false)) + bindings, err := ParseAddressPortBindings(c.BindAddresses) + if err != nil { + return fmt.Errorf("Error parsing address/port bindings") } - return out -} + w.buildServers(&c, bindings) + w.configureBodyLimit(&c) + w.configureRedirects(&c, bindings) + w.configureCompression(&c) -func (w *EchoWrapper) AddJobsToRunner(r *service.AppRunner) { - r.AddInitJob(w.Autocert.PrimeCache) - r.AddJob(w.Autocert.PrimingReporter(w.Logger)) + w.Use(glmw.StrictSecure()) - r.AddJobs(w.makeServerJobs()) + w.configureCORS(&c) + w.configureCSP(&c) + w.configureServerHeader(&c) + w.configurePrometheus(&c) - r.AddInitJobs(w.middlewareInitJobs) - r.AddJobs(w.middlewareJobs) + return nil } -func (w *EchoWrapper) GetTemplateFS() fs.FS { - return w.templateFS +func (w *EchoWrapper) configureAutocert(c *EchoConfig) { + w.autocert = c.Autocert + w.runner.AddInitJob(w.autocert.PrimeCache) + w.runner.AddJob(w.autocert.PrimingReporter(w.Logger)) } -// NewDefaultEchoWithConfig builds a wrapper around an Echo instance and -// configures it in the default way that it should probably be configured in -// all cases. The struct returned from this function can be treated like a -// normal Echo instance (because, for the most part it is). -func NewDefaultEchoWithConfig(c EchoConfig) (*EchoWrapper, error) { +func (w *EchoWrapper) configureIpExtractor(c *EchoConfig) error { var err error - - mwjobs := []glservice.RunnerFunc{} - mwinitjobs := []glservice.SyncRunnerFunc{} - - if os.Getenv("POMONA_DC_SITE") == "" { - return nil, fmt.Errorf("POMONA_DC_SITE must be in the environment") - } - - e := echo.New() - e.Debug = c.Debug - // This is only required if the app is behind a proxy, for apps that // take traffic directly from the internet the IP should just be // extracted from the requests RemoteAddr. if c.TrustedProxyIPRanges == nil { - e.IPExtractor, err = glmw.ExtractIPFromXFFHeaders(false, c.TrustedProxyIPRanges) + w.IPExtractor, err = glmw.ExtractIPFromXFFHeaders(false, c.TrustedProxyIPRanges) if err != nil { - return nil, fmt.Errorf("Error building XFF IP extractor: %w", err) + return fmt.Errorf("Error building XFF IP extractor: %w", err) } } else { - e.IPExtractor = echo.ExtractIPDirect() + w.IPExtractor = echo.ExtractIPDirect() } + return nil +} +func (w *EchoWrapper) configureTemplates(c *EchoConfig) error { // Use templates from disk in debug mode and the embedded ones that are // built-in to the binary for prod mode. var templates fs.FS - if c.DiskTemplates != nil && c.Debug { // Debug Mode + + if c.DiskTemplates != nil && w.Debug { // Debug Mode templates = c.DiskTemplates - } else if c.EmbeddedTemplates != nil && !c.Debug { // Prod Mode + } else if c.EmbeddedTemplates != nil && !w.Debug { // Prod Mode templates = c.EmbeddedTemplates } // Only install template handlers if the path and glob are set if templates != nil { - e.HTTPErrorHandler = ErrorHandler(templates, c.TemplateFunctions) + w.HTTPErrorHandler = ErrorHandler(templates, c.TemplateFunctions) tr, err := NewTemplateRenderer(templates, "*.tpl", c.TemplateFunctions) if err != nil { - return nil, fmt.Errorf("Error loading template renderer: %w", err) + return fmt.Errorf("Error loading template renderer: %w", err) } - e.Renderer = tr + w.Renderer = tr for _, t := range mandatoryTemplates { - if !tr.HaveTemplate(e.NewContext(nil, nil), t) { - return nil, fmt.Errorf("Tempalate renderer is missing required template %s", t) + if !tr.HaveTemplate(w.NewContext(nil, nil), t) { + return fmt.Errorf("Tempalate renderer is missing required template %s", t) } } } - e.Logger.SetLevel(log.INFO) - if c.Debug { - e.Logger.SetLevel(log.DEBUG) - } + w.templateFS = templates + return nil +} +func (w *EchoWrapper) configureCombinedLogging(c *EchoConfig) error { if c.CombinedHostLogFile != "" { lc, err := NginxCombinedHostConfigToFile(c.CombinedHostLogFile) if err != nil { - return nil, fmt.Errorf("Error opening log file: %w", err) + return fmt.Errorf("Error opening log file: %w", err) } - e.Use(middleware.LoggerWithConfig(lc)) + w.Use(middleware.LoggerWithConfig(lc)) } + return nil +} - bindings, err := ParseAddressPortBindings(c.BindAddresses) - if err != nil { - return nil, fmt.Errorf("Error parsing address/port bindings") +func (w *EchoWrapper) makeServerJob(s *http.Server, echoInit bool) service.RunnerFunc { + return func(ctx context.Context, wg *sync.WaitGroup) error { + wg.Add(1) + defer wg.Done() + + w.Logger.Infof("Starting server with address: %s", s.Addr) + + err := make(chan error) + go func() { + if s.TLSConfig == nil && echoInit { + err <- w.Echo.StartServer(s) + } else if s.TLSConfig == nil && !echoInit { + err <- s.ListenAndServe() + } else { + err <- s.ListenAndServeTLS("", "") + } + }() + select { + case e := <-err: + return e + default: + } + + select { + case <-ctx.Done(): + w.Logger.Info("Shutting down web server") + return s.Shutdown(ctx) + } } +} - servers := make([]*http.Server, len(c.BindAddresses)) +func (w *EchoWrapper) buildServers(c *EchoConfig, bindings *AddressPortConfig) { for i, a := range bindings.HttpBindings() { - servers[i] = &http.Server{ + s := &http.Server{ Addr: a, - Handler: e, + Handler: w, } + w.runner.AddJob(w.makeServerJob(s, i == 0)) } - tlsServers := make([]*http.Server, len(c.BindAddresses)) - for i, a := range bindings.TlsBindings() { - tlsServers[i] = &http.Server{ + for _, a := range bindings.TlsBindings() { + s := &http.Server{ Addr: a, TLSConfig: &tls.Config{ MinVersion: tls.VersionTLS12, GetCertificate: c.Autocert.GetCertificate, NextProtos: []string{"h2", "http/1.1"}, // enable HTTP/2 }, - Handler: e, + Handler: w, } + w.runner.AddJob(w.makeServerJob(s, false)) } +} - metricsSkipper := makeMiddlewareSkipper(c.PrometheusConfig) - - e.Use(middleware.Logger()) - e.Use(glmw.Recover()) - +func (w *EchoWrapper) configureBodyLimit(c *EchoConfig) { if c.BodySizeLimit == "" { - e.Use(middleware.BodyLimit(defaultBodySizeLimit)) + w.Use(middleware.BodyLimit(defaultBodySizeLimit)) } else if c.BodySizeLimit != "0" { - e.Use(middleware.BodyLimit(c.BodySizeLimit)) + w.Use(middleware.BodyLimit(c.BodySizeLimit)) } +} + +func (w *EchoWrapper) configureRedirects(c *EchoConfig, bindings *AddressPortConfig) { + metricsSkipper := makeMiddlewareSkipper(c.PrometheusConfig) - e.Use(glmw.HTTPSRedirectWithConfig(glmw.HTTPSRedirectConfig{ + w.Use(glmw.HTTPSRedirectWithConfig(glmw.HTTPSRedirectConfig{ Skipper: metricsSkipper, Port: bindings.TlsPort, })) if c.RedirectToWWW { - e.Use(middleware.WWWRedirectWithConfig(middleware.RedirectConfig{ + w.Use(middleware.WWWRedirectWithConfig(middleware.RedirectConfig{ Skipper: metricsSkipper, })) } +} - e.Use(middleware.Decompress()) +func (w *EchoWrapper) configureCompression(c *EchoConfig) { + metricsSkipper := makeMiddlewareSkipper(c.PrometheusConfig) + + w.Use(middleware.Decompress()) // TODO: This mangles responses but only for Accept: */* (browsers). Why? - e.Use(middleware.GzipWithConfig(middleware.GzipConfig{ + w.Use(middleware.GzipWithConfig(middleware.GzipConfig{ Skipper: metricsSkipper, Level: 5, })) +} - e.Use(glmw.StrictSecure()) - +func (w *EchoWrapper) configureCORS(c *EchoConfig) { if c.CORSConfig != nil { - e.Use(middleware.CORSWithConfig(*c.CORSConfig)) + w.Use(middleware.CORSWithConfig(*c.CORSConfig)) } else { - e.Use(middleware.CORS()) + w.Use(middleware.CORS()) } +} +func (w *EchoWrapper) configureCSP(c *EchoConfig) { if c.ContentSecurityPolicy != nil { - e.Use(glmw.ContentSecurityPolicyWithConfig(*c.ContentSecurityPolicy)) + w.Use(glmw.ContentSecurityPolicyWithConfig(*c.ContentSecurityPolicy)) } else { - e.Use(glmw.ContentSecurityPolicyWithConfig(glmw.ContentSecurityPolicyConfig{ + w.Use(glmw.ContentSecurityPolicyWithConfig(glmw.ContentSecurityPolicyConfig{ DefaultSrc: []glmw.CSPDirective{ glmw.CSPSelf, }, })) } +} - if c.ApplicationName != "" && c.ApplicationVersion != "" && !c.DisableServerHeader { - e.Use(glmw.AddServerHeader(c.ApplicationName, c.ApplicationVersion)) +func (w *EchoWrapper) configureServerHeader(c *EchoConfig) { + if c.ApplicationName != "" && c.ApplicationVersion != "" { + w.Use(glmw.AddServerHeader(c.ApplicationName, c.ApplicationVersion)) } +} +func (w *EchoWrapper) configurePrometheus(c *EchoConfig) { if !c.DisablePrometheus && c.NetboxClient != nil { // TODO: Should constrain this by site probably but monitoring happens // across sites so those prefixes need to be included @@ -307,10 +350,10 @@ func NewDefaultEchoWithConfig(c EchoConfig) (*EchoWrapper, error) { NetboxClient: c.NetboxClient, Tag: "management", IncludeLocalhost: true, - Logger: e.Logger, + Logger: w.Logger, } - mwinitjobs = append(mwinitjobs, f.Init) - mwjobs = append(mwjobs, f.RunRefresh) + w.runner.AddInitJob(f.Init) + w.runner.AddJob(f.RunRefresh) var prom *prometheus.Prometheus if c.PrometheusConfig != nil { @@ -319,17 +362,7 @@ func NewDefaultEchoWithConfig(c EchoConfig) (*EchoWrapper, error) { prom = prometheus.NewPrometheus() } - e.Use(prom.MiddlewareHandler) - e.GET(prom.Config.MetricsPath, prom.MetricsHandler, f.Middleware) + w.Use(prom.MiddlewareHandler) + w.GET(prom.Config.MetricsPath, prom.MetricsHandler, f.Middleware) } - - return &EchoWrapper{ - Echo: e, - Autocert: c.Autocert, - servers: servers, - tlsServers: tlsServers, - templateFS: templates, - middlewareJobs: mwjobs, - middlewareInitJobs: mwinitjobs, - }, nil } -- cgit v1.2.3