From 342497a86901ec7ce8b290b472c3cca2c25ec97a Mon Sep 17 00:00:00 2001 From: James Elliott Date: Tue, 9 Aug 2022 07:50:12 +1000 Subject: [PATCH] refactor(server): use errgroup to supervise services (#3755) Uses the errgroup package and pattern for supervising services like servers etc. --- config.template.yml | 54 ++++++- .../en/configuration/methods/environment.md | 5 +- .../en/configuration/miscellaneous/server.md | 31 ++-- .../en/configuration/prologue/common.md | 43 +++++ .../en/configuration/security/regulation.md | 4 +- .../en/configuration/telemetry/metrics.md | 17 ++ .../en/reference/guides/log-messages.md | 6 +- go.mod | 1 + go.sum | 3 +- internal/commands/root.go | 153 +++++++++++------- internal/commands/util.go | 18 +++ internal/configuration/config.template.yml | 54 ++++++- internal/configuration/deprecation.go | 21 +++ internal/configuration/schema/keys.go | 13 +- internal/configuration/schema/server.go | 24 ++- internal/configuration/schema/shared.go | 17 ++ internal/configuration/schema/storage.go | 3 - internal/configuration/schema/telemetry.go | 13 ++ internal/configuration/validator/const.go | 1 - internal/configuration/validator/server.go | 24 ++- .../configuration/validator/server_test.go | 32 ++-- internal/configuration/validator/storage.go | 5 - .../configuration/validator/storage_test.go | 20 --- internal/configuration/validator/telemetry.go | 20 +++ internal/server/const.go | 9 ++ internal/server/server.go | 29 ++-- internal/server/server_test.go | 1 - 27 files changed, 459 insertions(+), 162 deletions(-) create mode 100644 internal/commands/util.go diff --git a/config.template.yml b/config.template.yml index 70da4ffa3..a878e05f4 100644 --- a/config.template.yml +++ b/config.template.yml @@ -51,13 +51,6 @@ server: ## Useful to allow overriding of specific static assets. # asset_path: /config/assets/ - ## Buffers usually should be configured to be the same value. - ## Explanation at https://www.authelia.com/c/server#buffer-sizes - ## Read buffer size adjusts the server's max incoming request size in bytes. - ## Write buffer size does the same for outgoing responses. - read_buffer_size: 4096 - write_buffer_size: 4096 - ## Enables the pprof endpoint. enable_pprof: false @@ -85,6 +78,32 @@ server: ## The CSP Template. Read the docs. csp_template: "" + ## Server Buffers configuration. + # buffers: + + ## Buffers usually should be configured to be the same value. + ## Explanation at https://www.authelia.com/c/server#buffer-sizes + ## Read buffer size adjusts the server's max incoming request size in bytes. + ## Write buffer size does the same for outgoing responses. + + ## Read buffer. + # read: 4096 + + ## Write buffer. + # write: 4096 + + ## Server Timeouts configuration. + # timeouts: + + ## Read timeout. + # read: 2s + + ## Write timeout. + # write: 2s + + ## Idle timeout. + # idle: 30s + ## ## Log Configuration ## @@ -116,6 +135,27 @@ telemetry: ## The address to listen on for metrics. This should be on a different port to the main server.port value. address: tcp://0.0.0.0:9959 + ## Metrics Server Buffers configuration. + # buffers: + + ## Read buffer. + # read: 4096 + + ## Write buffer. + # write: 4096 + + ## Metrics Server Timeouts configuration. + # timeouts: + + ## Read timeout. + # read: 2s + + ## Write timeout. + # write: 2s + + ## Idle timeout. + # idle: 30s + ## ## TOTP Configuration ## diff --git a/docs/content/en/configuration/methods/environment.md b/docs/content/en/configuration/methods/environment.md index e22f539bf..4fc3ccd49 100644 --- a/docs/content/en/configuration/methods/environment.md +++ b/docs/content/en/configuration/methods/environment.md @@ -40,12 +40,13 @@ For example this YAML configuration: log: level: info server: - read_buffer_size: 4096 + buffers: + read: 4096 ``` Can be replaced by this environment variable configuration: ```bash AUTHELIA_LOG_LEVEL=info -AUTHELIA_SERVER_READ_BUFFER_SIZE=4096 +AUTHELIA_SERVER_BUFFERS_READ=4096 ``` diff --git a/docs/content/en/configuration/miscellaneous/server.md b/docs/content/en/configuration/miscellaneous/server.md index b763e99af..dcab6243e 100644 --- a/docs/content/en/configuration/miscellaneous/server.md +++ b/docs/content/en/configuration/miscellaneous/server.md @@ -22,8 +22,6 @@ server: host: 0.0.0.0 port: 9091 path: "" - read_buffer_size: 4096 - write_buffer_size: 4096 enable_pprof: false enable_expvars: false disable_healthcheck: false @@ -33,6 +31,13 @@ server: client_certificates: [] headers: csp_template: "" + buffers: + read: 4096 + write: 4096 + timeouts: + read: 10s + write: 10s + idle: 10s ``` ## Options @@ -95,18 +100,6 @@ assets that can be overridden must be placed in the `asset_path`. The structure can be overriden is documented in the [Sever Asset Overrides Reference Guide](../../reference/guides/server-asset-overrides.md). -### read_buffer_size - -{{< confkey type="integer " default="4096" required="no" >}} - -Configures the maximum request size. The default of 4096 is generally sufficient for most use cases. - -### write_buffer_size - -{{< confkey type="integer " default="4096" required="no" >}} - -Configures the maximum response size. The default of 4096 is generally sufficient for most use cases. - ### enable_pprof {{< confkey type="boolean" default="false" required="no" >}} @@ -174,6 +167,16 @@ about how browsers utilize and understand this header before attempting to custo For example, the default CSP template is `default-src 'self'; object-src 'none'; style-src 'self' 'nonce-${NONCE}'`. +### buffers + +Configures the server buffers. See the [Server Buffers](../prologue/common.md#server-buffers) documentation for more +information. + +### timeouts + +Configures the server timeouts. See the [Server Timeouts](../prologue/common.md#server-timeouts) documentation for more +information. + ## Additional Notes ### Buffer Sizes diff --git a/docs/content/en/configuration/prologue/common.md b/docs/content/en/configuration/prologue/common.md index ce9b113d6..fc3469331 100644 --- a/docs/content/en/configuration/prologue/common.md +++ b/docs/content/en/configuration/prologue/common.md @@ -133,3 +133,46 @@ The key `minimum_version` controls the minimum TLS version Authelia will use whe The possible values are `TLS1.3`, `TLS1.2`, `TLS1.1`, `TLS1.0`. Anything other than `TLS1.3` or `TLS1.2` are very old and deprecated. You should avoid using these and upgrade your backend service instead of decreasing this value. + +## Server Buffers + +### read + +{{< confkey type="integer" default="4096" required="no" >}} + +Configures the maximum request size. The default of 4096 is generally sufficient for most use cases. + +### write + +{{< confkey type="integer" default="4096" required="no" >}} + +Configures the maximum response size. The default of 4096 is generally sufficient for most use cases. + +## Server Timeouts + +### read + +{{< confkey type="duration" default="2s" required="no" >}} + +*__Note:__ This setting uses the [duration notation format](#duration-notation-format). Please see the +[common options](#duration-notation-format) documentation for information on this format.* + +Configures the server read timeout. + +### write + +{{< confkey type="duration" default="2s" required="no" >}} + +*__Note:__ This setting uses the [duration notation format](#duration-notation-format). Please see the +[common options](#duration-notation-format) documentation for information on this format.* + +Configures the server write timeout. + +### idle + +{{< confkey type="duration" default="30s" required="no" >}} + +*__Note:__ This setting uses the [duration notation format](#duration-notation-format). Please see the +[common options](#duration-notation-format) documentation for information on this format.* + +Configures the server write timeout. diff --git a/docs/content/en/configuration/security/regulation.md b/docs/content/en/configuration/security/regulation.md index 93bcc3077..705c7e718 100644 --- a/docs/content/en/configuration/security/regulation.md +++ b/docs/content/en/configuration/security/regulation.md @@ -31,13 +31,13 @@ regulation: ### max_retries -{{< confkey type="integer " default="3" required="no" >}} +{{< confkey type="integer" default="3" required="no" >}} The number of failed login attempts before a user may be banned. Setting this option to 0 disables regulation entirely. ### find_time -{{< confkey type="duration " default="2m" required="no" >}} +{{< confkey type="duration" default="2m" required="no" >}} *__Note:__ This setting uses the [duration notation format](../prologue/common.md#duration-notation-format). Please see the [common options](../prologue/common.md#duration-notation-format) documentation for information on this format.* diff --git a/docs/content/en/configuration/telemetry/metrics.md b/docs/content/en/configuration/telemetry/metrics.md index 9db3ba243..a8d347ea8 100644 --- a/docs/content/en/configuration/telemetry/metrics.md +++ b/docs/content/en/configuration/telemetry/metrics.md @@ -21,6 +21,13 @@ telemetry: metrics: enabled: false address: "tcp://0.0.0.0:9959" + buffers: + read: 4096 + write: 4096 + timeouts: + read: 10s + write: 10s + idle: 10s ``` ## Options @@ -38,6 +45,16 @@ Determines if the [Prometheus] HTTP Metrics Exporter is enabled. Configures the listener address for the [Prometheus] HTTP Metrics Exporter. This configuration key uses the [Address](../prologue/common.md#address) format. The scheme must be `tcp://` or empty. +### buffers + +Configures the server buffers. See the [Server Buffers](../prologue/common.md#server-buffers) documentation for more +information. + +### timeouts + +Configures the server timeouts. See the [Server Timeouts](../prologue/common.md#server-timeouts) documentation for more +information. + ## See More - [Telemetry Reference Documentation](../../reference/guides/metrics.md) diff --git a/docs/content/en/reference/guides/log-messages.md b/docs/content/en/reference/guides/log-messages.md index 6d5d2c116..53822998e 100644 --- a/docs/content/en/reference/guides/log-messages.md +++ b/docs/content/en/reference/guides/log-messages.md @@ -15,15 +15,15 @@ toc: true ## Request Header Too Large The `request header too large` error with a status code of `431` indicates the HTTP request made to *Authelia* had -headers exceeding the server [read_buffer_size](../../configuration/miscellaneous/server.md#read_buffer_size) parameter. +headers exceeding the server [read buffer](../../configuration/miscellaneous/server.md#buffers) parameter. Usually the defaults are sufficient however some applications cause fairly large headers to be added to requests. -It's suggested you increase the [read_buffer_size](../../configuration/miscellaneous/server.md#read_buffer_size) +It's suggested you increase the [read buffer](../../configuration/miscellaneous/server.md#buffers) configuration option (by either doubling or quadrupling it) in order to alleviate this issue or use the reverse proxy to remove the excessive headers which are causing this issue. -It's generally recommended the [write_buffer_size](../../configuration/miscellaneous/server.md#write_buffer_size) is +It's generally recommended the [write buffer](../../configuration/miscellaneous/server.md#buffers) is also increased. ## User Has Been Inactive Too Long diff --git a/go.mod b/go.mod index b11cac09a..46f14559d 100644 --- a/go.mod +++ b/go.mod @@ -36,6 +36,7 @@ require ( github.com/stretchr/testify v1.8.0 github.com/trustelem/zxcvbn v1.0.1 github.com/valyala/fasthttp v1.38.0 + golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 golang.org/x/text v0.3.7 gopkg.in/square/go-jose.v2 v2.6.0 gopkg.in/yaml.v3 v3.0.1 diff --git a/go.sum b/go.sum index 0227cd986..32b4ec065 100644 --- a/go.sum +++ b/go.sum @@ -1587,7 +1587,8 @@ golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJ golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20210220032951-036812b2e83c/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f h1:Ax0t5p6N38Ga0dThY21weqDEyz2oklo4IvDkpigvkD8= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4 h1:uVc8UZUe6tr40fFVnUP5Oj+veunVezqYl9z7DYw9xzw= +golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sys v0.0.0-20180816055513-1c9583448a9c/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180823144017-11551d06cbcc/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= diff --git a/internal/commands/root.go b/internal/commands/root.go index 783fc120f..f23dae3d3 100644 --- a/internal/commands/root.go +++ b/internal/commands/root.go @@ -1,13 +1,18 @@ package commands import ( + "context" "fmt" + "net" "os" + "os/signal" "strings" - "sync" + "syscall" "github.com/sirupsen/logrus" "github.com/spf13/cobra" + "github.com/valyala/fasthttp" + "golang.org/x/sync/errgroup" "github.com/authelia/authelia/v4/internal/configuration/schema" "github.com/authelia/authelia/v4/internal/logging" @@ -74,99 +79,127 @@ func cmdRootRun(_ *cobra.Command, _ []string) { logger.Fatalf("Errors occurred provisioning providers.") } - doStartupChecks(config, &providers) + doStartupChecks(config, &providers, logger) runServers(config, providers, logger) } -func runServers(config *schema.Configuration, providers middlewares.Providers, logger *logrus.Logger) { - wg := new(sync.WaitGroup) +func runServers(config *schema.Configuration, providers middlewares.Providers, log *logrus.Logger) { + ctx := context.Background() - wg.Add(2) + ctx, cancel := context.WithCancel(ctx) - go func() { - err := startDefaultServer(config, providers) - if err != nil { - logger.Fatal(err) + defer cancel() + + quit := make(chan os.Signal, 1) + + signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM) + + defer signal.Stop(quit) + + g, ctx := errgroup.WithContext(ctx) + + var ( + mainServer, metricsServer *fasthttp.Server + mainListener, metricsListener net.Listener + ) + + g.Go(func() (err error) { + defer func() { + if rec := recover(); rec != nil { + log.WithError(recoverErr(rec)).Errorf("Critical error in server caught (recovered)") + } + }() + + if mainServer, mainListener, err = server.CreateDefaultServer(*config, providers); err != nil { + return err } - wg.Done() - }() - - go func() { - err := startMetricsServer(config, providers) - if err != nil { - logger.Fatal(err) + if err = mainServer.Serve(mainListener); err != nil { + return err } - }() - wg.Wait() -} - -func startDefaultServer(config *schema.Configuration, providers middlewares.Providers) (err error) { - svr, listener, err := server.CreateDefaultServer(*config, providers) - - switch err { - case nil: - if err = svr.Serve(listener); err != nil { - return fmt.Errorf("error occurred during default server operation: %w", err) - } - default: - return fmt.Errorf("error occurred during default server startup: %w", err) - } - - return nil -} - -func startMetricsServer(config *schema.Configuration, providers middlewares.Providers) (err error) { - if providers.Metrics == nil { return nil - } + }) - svr, listener, err := server.CreateMetricsServer(config.Telemetry.Metrics) - - switch err { - case nil: - if err = svr.Serve(listener); err != nil { - return fmt.Errorf("error occurred during metrics server operation: %w", err) + g.Go(func() (err error) { + if providers.Metrics == nil { + return nil } - default: - return fmt.Errorf("error occurred during metrics server startup: %w", err) + + defer func() { + if rec := recover(); rec != nil { + log.WithError(recoverErr(rec)).Errorf("Critical error in metrics server caught (recovered)") + } + }() + + if metricsServer, metricsListener, err = server.CreateMetricsServer(config.Telemetry.Metrics); err != nil { + return err + } + + if err = metricsServer.Serve(metricsListener); err != nil { + return err + } + + return nil + }) + + select { + case <-quit: + break + case <-ctx.Done(): + break } - return nil + cancel() + + log.Infof("Shutting down") + + var err error + + if err = mainServer.Shutdown(); err != nil { + log.WithError(err).Errorf("Error occurred shutting down the server") + } + + if metricsServer != nil { + if err = metricsServer.Shutdown(); err != nil { + log.WithError(err).Errorf("Error occurred shutting down the metrics server") + } + } + + if err = g.Wait(); err != nil { + log.WithError(err).Errorf("Error occurred waiting for shutdown") + } } -func doStartupChecks(config *schema.Configuration, providers *middlewares.Providers) { - logger := logging.Logger() - +func doStartupChecks(config *schema.Configuration, providers *middlewares.Providers, log *logrus.Logger) { var ( failures []string err error ) - if err = doStartupCheck(logger, "storage", providers.StorageProvider, false); err != nil { - logger.Errorf("Failure running the storage provider startup check: %+v", err) + if err = doStartupCheck(log, "storage", providers.StorageProvider, false); err != nil { + log.Errorf("Failure running the storage provider startup check: %+v", err) failures = append(failures, "storage") } - if err = doStartupCheck(logger, "user", providers.UserProvider, false); err != nil { - logger.Errorf("Failure running the user provider startup check: %+v", err) + if err = doStartupCheck(log, "user", providers.UserProvider, false); err != nil { + log.Errorf("Failure running the user provider startup check: %+v", err) failures = append(failures, "user") } - if err = doStartupCheck(logger, "notification", providers.Notifier, config.Notifier.DisableStartupCheck); err != nil { - logger.Errorf("Failure running the notification provider startup check: %+v", err) + if err = doStartupCheck(log, "notification", providers.Notifier, config.Notifier.DisableStartupCheck); err != nil { + log.Errorf("Failure running the notification provider startup check: %+v", err) failures = append(failures, "notification") } if !config.NTP.DisableStartupCheck && !providers.Authorizer.IsSecondFactorEnabled() { - logger.Debug("The NTP startup check was skipped due to there being no configured 2FA access control rules") - } else if err = doStartupCheck(logger, "ntp", providers.NTP, config.NTP.DisableStartupCheck); err != nil { - logger.Errorf("Failure running the ntp provider startup check: %+v", err) + log.Debug("The NTP startup check was skipped due to there being no configured 2FA access control rules") + } else if err = doStartupCheck(log, "ntp", providers.NTP, config.NTP.DisableStartupCheck); err != nil { + log.Errorf("Failure running the ntp provider startup check: %+v", err) if !config.NTP.DisableFailure { failures = append(failures, "ntp") @@ -174,7 +207,7 @@ func doStartupChecks(config *schema.Configuration, providers *middlewares.Provid } if len(failures) != 0 { - logger.Fatalf("The following providers had fatal failures during startup: %s", strings.Join(failures, ", ")) + log.Fatalf("The following providers had fatal failures during startup: %s", strings.Join(failures, ", ")) } } diff --git a/internal/commands/util.go b/internal/commands/util.go new file mode 100644 index 000000000..26c308648 --- /dev/null +++ b/internal/commands/util.go @@ -0,0 +1,18 @@ +package commands + +import ( + "fmt" +) + +func recoverErr(i interface{}) error { + switch v := i.(type) { + case nil: + return nil + case string: + return fmt.Errorf("recovered panic: %s", v) + case error: + return fmt.Errorf("recovered panic: %w", v) + default: + return fmt.Errorf("recovered panic with unknown type: %v", v) + } +} diff --git a/internal/configuration/config.template.yml b/internal/configuration/config.template.yml index 70da4ffa3..a878e05f4 100644 --- a/internal/configuration/config.template.yml +++ b/internal/configuration/config.template.yml @@ -51,13 +51,6 @@ server: ## Useful to allow overriding of specific static assets. # asset_path: /config/assets/ - ## Buffers usually should be configured to be the same value. - ## Explanation at https://www.authelia.com/c/server#buffer-sizes - ## Read buffer size adjusts the server's max incoming request size in bytes. - ## Write buffer size does the same for outgoing responses. - read_buffer_size: 4096 - write_buffer_size: 4096 - ## Enables the pprof endpoint. enable_pprof: false @@ -85,6 +78,32 @@ server: ## The CSP Template. Read the docs. csp_template: "" + ## Server Buffers configuration. + # buffers: + + ## Buffers usually should be configured to be the same value. + ## Explanation at https://www.authelia.com/c/server#buffer-sizes + ## Read buffer size adjusts the server's max incoming request size in bytes. + ## Write buffer size does the same for outgoing responses. + + ## Read buffer. + # read: 4096 + + ## Write buffer. + # write: 4096 + + ## Server Timeouts configuration. + # timeouts: + + ## Read timeout. + # read: 2s + + ## Write timeout. + # write: 2s + + ## Idle timeout. + # idle: 30s + ## ## Log Configuration ## @@ -116,6 +135,27 @@ telemetry: ## The address to listen on for metrics. This should be on a different port to the main server.port value. address: tcp://0.0.0.0:9959 + ## Metrics Server Buffers configuration. + # buffers: + + ## Read buffer. + # read: 4096 + + ## Write buffer. + # write: 4096 + + ## Metrics Server Timeouts configuration. + # timeouts: + + ## Read timeout. + # read: 2s + + ## Write timeout. + # write: 2s + + ## Idle timeout. + # idle: 30s + ## ## TOTP Configuration ## diff --git a/internal/configuration/deprecation.go b/internal/configuration/deprecation.go index f0850bf74..56615cbc1 100644 --- a/internal/configuration/deprecation.go +++ b/internal/configuration/deprecation.go @@ -106,6 +106,13 @@ var deprecations = map[string]Deprecation{ AutoMap: true, MapFunc: nil, }, + "storage.postgres.sslmode": { + Version: model.SemanticVersion{Major: 4, Minor: 36}, + Key: "storage.postgres.sslmode", + NewKey: "storage.postgres.ssl.mode", + AutoMap: true, + MapFunc: nil, + }, "authentication_backend.disable_reset_password": { Version: model.SemanticVersion{Major: 4, Minor: 36}, Key: "authentication_backend.disable_reset_password", @@ -113,4 +120,18 @@ var deprecations = map[string]Deprecation{ AutoMap: true, MapFunc: nil, }, + "server.read_buffer_size": { + Version: model.SemanticVersion{Major: 4, Minor: 37}, + Key: "server.read_buffer_size", + NewKey: "server.buffers.read", + AutoMap: true, + MapFunc: nil, + }, + "server.write_buffer_size": { + Version: model.SemanticVersion{Major: 4, Minor: 37}, + Key: "server.write_buffer_size", + NewKey: "server.buffers.write", + AutoMap: true, + MapFunc: nil, + }, } diff --git a/internal/configuration/schema/keys.go b/internal/configuration/schema/keys.go index 63896973a..07639a456 100644 --- a/internal/configuration/schema/keys.go +++ b/internal/configuration/schema/keys.go @@ -150,7 +150,6 @@ var Keys = []string{ "storage.postgres.ssl.root_certificate", "storage.postgres.ssl.certificate", "storage.postgres.ssl.key", - "storage.postgres.sslmode", "storage.encryption_key", "notifier.disable_startup_check", "notifier.filesystem.filename", @@ -173,8 +172,6 @@ var Keys = []string{ "server.port", "server.path", "server.asset_path", - "server.read_buffer_size", - "server.write_buffer_size", "server.enable_pprof", "server.enable_expvars", "server.disable_healthcheck", @@ -182,8 +179,18 @@ var Keys = []string{ "server.tls.key", "server.tls.client_certificates", "server.headers.csp_template", + "server.buffers.read", + "server.buffers.write", + "server.timeouts.read", + "server.timeouts.write", + "server.timeouts.idle", "telemetry.metrics.enabled", "telemetry.metrics.address", + "telemetry.metrics.buffers.read", + "telemetry.metrics.buffers.write", + "telemetry.metrics.timeouts.read", + "telemetry.metrics.timeouts.write", + "telemetry.metrics.timeouts.idle", "webauthn.disable", "webauthn.display_name", "webauthn.attestation_conveyance_preference", diff --git a/internal/configuration/schema/server.go b/internal/configuration/schema/server.go index 77146a454..57073ab1a 100644 --- a/internal/configuration/schema/server.go +++ b/internal/configuration/schema/server.go @@ -1,19 +1,24 @@ package schema +import ( + "time" +) + // ServerConfiguration represents the configuration of the http server. type ServerConfiguration struct { Host string `koanf:"host"` Port int `koanf:"port"` Path string `koanf:"path"` AssetPath string `koanf:"asset_path"` - ReadBufferSize int `koanf:"read_buffer_size"` - WriteBufferSize int `koanf:"write_buffer_size"` EnablePprof bool `koanf:"enable_pprof"` EnableExpvars bool `koanf:"enable_expvars"` DisableHealthcheck bool `koanf:"disable_healthcheck"` TLS ServerTLSConfiguration `koanf:"tls"` Headers ServerHeadersConfiguration `koanf:"headers"` + + Buffers ServerBuffers `koanf:"buffers"` + Timeouts ServerTimeouts `koanf:"timeouts"` } // ServerTLSConfiguration represents the configuration of the http servers TLS options. @@ -30,8 +35,15 @@ type ServerHeadersConfiguration struct { // DefaultServerConfiguration represents the default values of the ServerConfiguration. var DefaultServerConfiguration = ServerConfiguration{ - Host: "0.0.0.0", - Port: 9091, - ReadBufferSize: 4096, - WriteBufferSize: 4096, + Host: "0.0.0.0", + Port: 9091, + Buffers: ServerBuffers{ + Read: 4096, + Write: 4096, + }, + Timeouts: ServerTimeouts{ + Read: time.Second * 2, + Write: time.Second * 2, + Idle: time.Second * 30, + }, } diff --git a/internal/configuration/schema/shared.go b/internal/configuration/schema/shared.go index 46cf5784b..336121839 100644 --- a/internal/configuration/schema/shared.go +++ b/internal/configuration/schema/shared.go @@ -1,8 +1,25 @@ package schema +import ( + "time" +) + // TLSConfig is a representation of the TLS configuration. type TLSConfig struct { MinimumVersion string `koanf:"minimum_version"` SkipVerify bool `koanf:"skip_verify"` ServerName string `koanf:"server_name"` } + +// ServerTimeouts represents server timeout configurations. +type ServerTimeouts struct { + Read time.Duration `koanf:"read"` + Write time.Duration `koanf:"write"` + Idle time.Duration `koanf:"idle"` +} + +// ServerBuffers represents server buffer configurations. +type ServerBuffers struct { + Read int `koanf:"read"` + Write int `koanf:"write"` +} diff --git a/internal/configuration/schema/storage.go b/internal/configuration/schema/storage.go index 3503689db..de4eae7bb 100644 --- a/internal/configuration/schema/storage.go +++ b/internal/configuration/schema/storage.go @@ -28,9 +28,6 @@ type PostgreSQLStorageConfiguration struct { Schema string `koanf:"schema"` SSL PostgreSQLSSLStorageConfiguration `koanf:"ssl"` - - // Deprecated. TODO: Remove in v4.36.0. - SSLMode string `koanf:"sslmode"` } // PostgreSQLSSLStorageConfiguration represents the SSL configuration of a PostgreSQL database. diff --git a/internal/configuration/schema/telemetry.go b/internal/configuration/schema/telemetry.go index 594d193af..3de20bc3d 100644 --- a/internal/configuration/schema/telemetry.go +++ b/internal/configuration/schema/telemetry.go @@ -2,6 +2,7 @@ package schema import ( "net" + "time" ) // TelemetryConfig represents the telemetry config. @@ -13,11 +14,23 @@ type TelemetryConfig struct { type TelemetryMetricsConfig struct { Enabled bool `koanf:"enabled"` Address *Address `koanf:"address"` + + Buffers ServerBuffers `koanf:"buffers"` + Timeouts ServerTimeouts `koanf:"timeouts"` } // DefaultTelemetryConfig is the default telemetry configuration. var DefaultTelemetryConfig = TelemetryConfig{ Metrics: TelemetryMetricsConfig{ Address: &Address{true, "tcp", net.ParseIP("0.0.0.0"), 9959}, + Buffers: ServerBuffers{ + Read: 4096, + Write: 4096, + }, + Timeouts: ServerTimeouts{ + Read: time.Second * 2, + Write: time.Second * 2, + Idle: time.Second * 30, + }, }, } diff --git a/internal/configuration/validator/const.go b/internal/configuration/validator/const.go index 8f89dd7b6..78797a283 100644 --- a/internal/configuration/validator/const.go +++ b/internal/configuration/validator/const.go @@ -247,7 +247,6 @@ const ( errFmtServerPathNoForwardSlashes = "server: option 'path' must not contain any forward slashes" errFmtServerPathAlphaNum = "server: option 'path' must only contain alpha numeric characters" - errFmtServerBufferSize = "server: option '%s_buffer_size' must be above 0 but it is configured as '%d'" ) const ( diff --git a/internal/configuration/validator/server.go b/internal/configuration/validator/server.go index 06f26e051..b0529ab0e 100644 --- a/internal/configuration/validator/server.go +++ b/internal/configuration/validator/server.go @@ -70,15 +70,23 @@ func ValidateServer(config *schema.Configuration, validator *schema.StructValida config.Server.Path = path.Clean("/" + config.Server.Path) } - if config.Server.ReadBufferSize == 0 { - config.Server.ReadBufferSize = schema.DefaultServerConfiguration.ReadBufferSize - } else if config.Server.ReadBufferSize < 0 { - validator.Push(fmt.Errorf(errFmtServerBufferSize, "read", config.Server.ReadBufferSize)) + if config.Server.Buffers.Read <= 0 { + config.Server.Buffers.Read = schema.DefaultServerConfiguration.Buffers.Read } - if config.Server.WriteBufferSize == 0 { - config.Server.WriteBufferSize = schema.DefaultServerConfiguration.WriteBufferSize - } else if config.Server.WriteBufferSize < 0 { - validator.Push(fmt.Errorf(errFmtServerBufferSize, "write", config.Server.WriteBufferSize)) + if config.Server.Buffers.Write <= 0 { + config.Server.Buffers.Write = schema.DefaultServerConfiguration.Buffers.Write + } + + if config.Server.Timeouts.Read <= 0 { + config.Server.Timeouts.Read = schema.DefaultServerConfiguration.Timeouts.Read + } + + if config.Server.Timeouts.Write <= 0 { + config.Server.Timeouts.Write = schema.DefaultServerConfiguration.Timeouts.Write + } + + if config.Server.Timeouts.Idle <= 0 { + config.Server.Timeouts.Idle = schema.DefaultServerConfiguration.Timeouts.Idle } } diff --git a/internal/configuration/validator/server_test.go b/internal/configuration/validator/server_test.go index 7349465cd..bbbdb4010 100644 --- a/internal/configuration/validator/server_test.go +++ b/internal/configuration/validator/server_test.go @@ -3,6 +3,7 @@ package validator import ( "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -23,8 +24,8 @@ func TestShouldSetDefaultServerValues(t *testing.T) { assert.Equal(t, schema.DefaultServerConfiguration.Host, config.Server.Host) assert.Equal(t, schema.DefaultServerConfiguration.Port, config.Server.Port) - assert.Equal(t, schema.DefaultServerConfiguration.ReadBufferSize, config.Server.ReadBufferSize) - assert.Equal(t, schema.DefaultServerConfiguration.WriteBufferSize, config.Server.WriteBufferSize) + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Read, config.Server.Buffers.Read) + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Write, config.Server.Buffers.Write) assert.Equal(t, schema.DefaultServerConfiguration.TLS.Key, config.Server.TLS.Key) assert.Equal(t, schema.DefaultServerConfiguration.TLS.Certificate, config.Server.TLS.Certificate) assert.Equal(t, schema.DefaultServerConfiguration.Path, config.Server.Path) @@ -41,8 +42,8 @@ func TestShouldSetDefaultConfig(t *testing.T) { assert.Len(t, validator.Errors(), 0) assert.Len(t, validator.Warnings(), 0) - assert.Equal(t, schema.DefaultServerConfiguration.ReadBufferSize, config.Server.ReadBufferSize) - assert.Equal(t, schema.DefaultServerConfiguration.WriteBufferSize, config.Server.WriteBufferSize) + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Read, config.Server.Buffers.Read) + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Write, config.Server.Buffers.Write) } func TestShouldParsePathCorrectly(t *testing.T) { @@ -61,21 +62,32 @@ func TestShouldParsePathCorrectly(t *testing.T) { assert.Equal(t, "/apple", config.Server.Path) } -func TestShouldRaiseOnNegativeValues(t *testing.T) { +func TestShouldDefaultOnNegativeValues(t *testing.T) { validator := schema.NewStructValidator() config := &schema.Configuration{ Server: schema.ServerConfiguration{ - ReadBufferSize: -1, - WriteBufferSize: -1, + Buffers: schema.ServerBuffers{ + Read: -1, + Write: -1, + }, + Timeouts: schema.ServerTimeouts{ + Read: time.Second * -1, + Write: time.Second * -1, + Idle: time.Second * -1, + }, }, } ValidateServer(config, validator) - require.Len(t, validator.Errors(), 2) + require.Len(t, validator.Errors(), 0) - assert.EqualError(t, validator.Errors()[0], "server: option 'read_buffer_size' must be above 0 but it is configured as '-1'") - assert.EqualError(t, validator.Errors()[1], "server: option 'write_buffer_size' must be above 0 but it is configured as '-1'") + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Read, config.Server.Buffers.Read) + assert.Equal(t, schema.DefaultServerConfiguration.Buffers.Write, config.Server.Buffers.Write) + + assert.Equal(t, schema.DefaultServerConfiguration.Timeouts.Read, config.Server.Timeouts.Read) + assert.Equal(t, schema.DefaultServerConfiguration.Timeouts.Write, config.Server.Timeouts.Write) + assert.Equal(t, schema.DefaultServerConfiguration.Timeouts.Idle, config.Server.Timeouts.Idle) } func TestShouldRaiseOnNonAlphanumericCharsInPath(t *testing.T) { diff --git a/internal/configuration/validator/storage.go b/internal/configuration/validator/storage.go index 0ff673eae..a545113bf 100644 --- a/internal/configuration/validator/storage.go +++ b/internal/configuration/validator/storage.go @@ -56,11 +56,6 @@ func validatePostgreSQLConfiguration(config *schema.PostgreSQLStorageConfigurati config.Schema = schema.DefaultPostgreSQLStorageConfiguration.Schema } - // Deprecated. TODO: Remove in v4.36.0. - if config.SSLMode != "" && config.SSL.Mode == "" { - config.SSL.Mode = config.SSLMode - } - if config.SSL.Mode == "" { config.SSL.Mode = schema.DefaultPostgreSQLStorageConfiguration.SSL.Mode } else if !utils.IsStringInSlice(config.SSL.Mode, validStoragePostgreSQLSSLModes) { diff --git a/internal/configuration/validator/storage_test.go b/internal/configuration/validator/storage_test.go index b892c60fb..bf0d0a67c 100644 --- a/internal/configuration/validator/storage_test.go +++ b/internal/configuration/validator/storage_test.go @@ -166,26 +166,6 @@ func (suite *StorageSuite) TestShouldValidatePostgresSSLModeMustBeValid() { suite.Assert().EqualError(suite.validator.Errors()[0], "storage: postgres: ssl: option 'mode' must be one of 'disable', 'require', 'verify-ca', 'verify-full' but it is configured as 'unknown'") } -// Deprecated. TODO: Remove in v4.36.0. -func (suite *StorageSuite) TestShouldValidatePostgresSSLModeMustBeMappedForDeprecations() { - suite.config.PostgreSQL = &schema.PostgreSQLStorageConfiguration{ - SQLStorageConfiguration: schema.SQLStorageConfiguration{ - Host: "pg", - Username: "myuser", - Password: "pass", - Database: "database", - }, - SSLMode: "require", - } - - ValidateStorage(suite.config, suite.validator) - - suite.Assert().Len(suite.validator.Warnings(), 0) - suite.Assert().Len(suite.validator.Errors(), 0) - - suite.Assert().Equal(suite.config.PostgreSQL.SSL.Mode, "require") -} - func (suite *StorageSuite) TestShouldRaiseErrorOnNoEncryptionKey() { suite.config.EncryptionKey = "" suite.config.Local = &schema.LocalStorageConfiguration{ diff --git a/internal/configuration/validator/telemetry.go b/internal/configuration/validator/telemetry.go index d546cc523..8dc592e22 100644 --- a/internal/configuration/validator/telemetry.go +++ b/internal/configuration/validator/telemetry.go @@ -22,4 +22,24 @@ func ValidateTelemetry(config *schema.Configuration, validator *schema.StructVal if config.Telemetry.Metrics.Address.Port == 0 { config.Telemetry.Metrics.Address.Port = schema.DefaultTelemetryConfig.Metrics.Address.Port } + + if config.Telemetry.Metrics.Buffers.Read <= 0 { + config.Telemetry.Metrics.Buffers.Read = schema.DefaultTelemetryConfig.Metrics.Buffers.Read + } + + if config.Telemetry.Metrics.Buffers.Write <= 0 { + config.Telemetry.Metrics.Buffers.Write = schema.DefaultTelemetryConfig.Metrics.Buffers.Write + } + + if config.Telemetry.Metrics.Timeouts.Read <= 0 { + config.Telemetry.Metrics.Timeouts.Read = schema.DefaultTelemetryConfig.Metrics.Timeouts.Read + } + + if config.Telemetry.Metrics.Timeouts.Write <= 0 { + config.Telemetry.Metrics.Timeouts.Write = schema.DefaultTelemetryConfig.Metrics.Timeouts.Write + } + + if config.Telemetry.Metrics.Timeouts.Idle <= 0 { + config.Telemetry.Metrics.Timeouts.Idle = schema.DefaultTelemetryConfig.Metrics.Timeouts.Idle + } } diff --git a/internal/server/const.go b/internal/server/const.go index 483a3ab0f..9c9a34098 100644 --- a/internal/server/const.go +++ b/internal/server/const.go @@ -74,3 +74,12 @@ const ( cspDefaultTemplate = "default-src 'self'%s; frame-src 'none'; object-src 'none'; style-src 'self' 'nonce-%s'; frame-ancestors 'none'; base-uri 'self'" cspNoncePlaceholder = "${NONCE}" ) + +const ( + connNonTLS = "non-TLS" + connTLS = "TLS" +) + +const ( + fmtLogServerInit = "Initializing %s for %s connections on '%s' path '%s'" +) diff --git a/internal/server/server.go b/internal/server/server.go index a71a36498..37dc41c86 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -7,6 +7,7 @@ import ( "net" "os" "strconv" + "strings" "github.com/valyala/fasthttp" @@ -21,8 +22,11 @@ func CreateDefaultServer(config schema.Configuration, providers middlewares.Prov ErrorHandler: handleError(), Handler: handleRouter(config, providers), NoDefaultServerHeader: true, - ReadBufferSize: config.Server.ReadBufferSize, - WriteBufferSize: config.Server.WriteBufferSize, + ReadBufferSize: config.Server.Buffers.Read, + WriteBufferSize: config.Server.Buffers.Write, + ReadTimeout: config.Server.Timeouts.Read, + WriteTimeout: config.Server.Timeouts.Write, + IdleTimeout: config.Server.Timeouts.Idle, } address := net.JoinHostPort(config.Server.Host, strconv.Itoa(config.Server.Port)) @@ -33,7 +37,7 @@ func CreateDefaultServer(config schema.Configuration, providers middlewares.Prov ) if config.Server.TLS.Certificate != "" && config.Server.TLS.Key != "" { - connectionType, connectionScheme = "TLS", schemeHTTPS + connectionType, connectionScheme = connTLS, schemeHTTPS if err = server.AppendCert(config.Server.TLS.Certificate, config.Server.TLS.Key); err != nil { return nil, nil, fmt.Errorf("unable to load tls server certificate '%s' or private key '%s': %w", config.Server.TLS.Certificate, config.Server.TLS.Key, err) @@ -62,7 +66,7 @@ func CreateDefaultServer(config schema.Configuration, providers middlewares.Prov return nil, nil, fmt.Errorf("unable to initialize tcp listener: %w", err) } } else { - connectionType, connectionScheme = "non-TLS", schemeHTTP + connectionType, connectionScheme = connNonTLS, schemeHTTP if listener, err = net.Listen("tcp", address); err != nil { return nil, nil, fmt.Errorf("unable to initialize tcp listener: %w", err) @@ -74,14 +78,14 @@ func CreateDefaultServer(config schema.Configuration, providers middlewares.Prov return nil, nil, fmt.Errorf("unable to configure healthcheck: %w", err) } - logger := logging.Logger() + paths := []string{"/"} - if config.Server.Path == "" { - logger.Infof("Initializing server for %s connections on '%s' path '/'", connectionType, listener.Addr().String()) - } else { - logger.Infof("Initializing server for %s connections on '%s' paths '/' and '%s'", connectionType, listener.Addr().String(), config.Server.Path) + if config.Server.Path != "" { + paths = append(paths, config.Server.Path) } + logging.Logger().Infof(fmtLogServerInit, "server", connectionType, listener.Addr().String(), strings.Join(paths, "' and '")) + return server, listener, nil } @@ -95,7 +99,14 @@ func CreateMetricsServer(config schema.TelemetryMetricsConfig) (server *fasthttp ErrorHandler: handleError(), NoDefaultServerHeader: true, Handler: handleMetrics(), + ReadBufferSize: config.Buffers.Read, + WriteBufferSize: config.Buffers.Write, + ReadTimeout: config.Timeouts.Read, + WriteTimeout: config.Timeouts.Write, + IdleTimeout: config.Timeouts.Idle, } + logging.Logger().Infof(fmtLogServerInit, "server (metrics)", connNonTLS, listener.Addr().String(), "/metrics") + return server, listener, nil } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index ad5bf174d..d7f62ff43 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -192,7 +192,6 @@ func TestShouldRaiseErrorWhenClientDoesNotSkipVerify(t *testing.T) { defer tlsServerContext.Close() - fmt.Println(tlsServerContext.Port()) req, err := http.NewRequest("GET", fmt.Sprintf("https://local.example.com:%d", tlsServerContext.Port()), nil) require.NoError(t, err)