diff --git a/internal/config/README.md b/internal/config/README.md index 41726b5..3cc47ac 100644 --- a/internal/config/README.md +++ b/internal/config/README.md @@ -33,30 +33,32 @@ is kept. | Key | Type | Default | Notes | |---|---|---|---| | `logging.level` | string | `warn` | One of `debug`, `info`, `warn`, `error` | -| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`); other commands log to the terminal | -| `database.file` | string | | Path to the SQLite database file, used by the `migrate` commands | +| `logging.file` | string | | Log file for `fetch service` (unless `--foreground`); other commands log to the terminal. Empty: log to the terminal | +| `database.file` | string | | Path to the SQLite database file, used by the `migrate` commands. Empty: commands that need the database fail | | `sources..enabled` | bool | `false` | For every source | -| `sources..interval` | duration | `10m` | For every source; e.g. `30s`, `1h` | +| `sources..interval` | duration | `10m` | For every source; e.g. `30s`, `1h`. Must be greater than 0; turn a source off with `enabled` | | `sources.find_a_tender.api_root` | string | `https://www.find-tender.service.gov.uk/api/1.0` | URL of the Find a Tender API | -| `sources.fts_mock.api_root` | string | | URL of the mock FTS server (see `internal/mock`) | +| `sources.fts_mock.api_root` | string | | URL of the mock FTS server (see `internal/mock`); must be set when the source is enabled | | `sources.sleeper.block_shutdown` | bool | `false` | Test source: if true, its sleep ignores shutdown | -| `server.host` | string | `127.0.0.1` | Address to listen on | -| `server.port` | int | `9393` | Port to listen on | -| `server.timeouts.shutdown` | duration | `15s` | How long shutdown waits for clients to close | -| `server.rate_limits.idle_ttl` | duration | `10m` | How long a client's rate limiter is kept after its last request | +| `server.host` | string | `127.0.0.1` | Address to listen on; empty: every interface | +| `server.port` | int | `9393` | Port to listen on; `0`: any free port | +| `server.timeouts.shutdown` | duration | `15s` | How long shutdown waits for clients to close; `0s`: until they have | +| `server.rate_limits.idle_ttl` | duration | `10m` | How long a client's rate limiter is kept after its last request; `0s`: for ever | | `server.websocket.enable_compression` | bool | `false` | permessage-deflate | -| `server.websocket.max_message_size` | byte size | `32KiB` | Largest message read from a client; `0B` for 32KiB | +| `server.websocket.max_message_size` | byte size | `32KiB` | Largest message read from a client; `0B` for 32KiB. `-1` (no limit) is rejected, as a client could then exhaust the server's memory | | `server.websocket.connection_limit.*` | | | A rate limit rule for new connections from each client IP, WebSocket and plain TCP alike; limiters are kept for `server.rate_limits.idle_ttl` | | `server.websocket.message_rate_limit.*` | | | A rate limit rule for messages from each connection | | `server.websocket.write_timeout` | duration | `10s` | `0s` for none | -| `server.websocket.ping_interval` | duration | `30s` | | +| `server.websocket.ping_interval` | duration | `30s` | How long a connection may be idle before it is pinged; `0s`: no pings | | `server.websocket.pong_timeout` | duration | `10s` | `0s` for none | -The current sources are `find_a_tender`, `fts_mock` and `sleeper`. See +The current sources are `find_a_tender`, `fts_mock` and `sleeper`. See "Addition of a new opportunities source" in the top level README for how to add a new source. A rate limit rule (`JSCRateLimitRule`) has `enabled`, `per_second` and `burst`. +It is off unless `enabled` and `per_second` is greater than 0; a `burst` below +1 means 1. It is used under `server.websocket.connection_limit` and `server.websocket.message_rate_limit`. @@ -97,8 +99,11 @@ per key: - `util.ByteSize` fields work the same way, with size strings such as `"32KiB"` or `"1MB"` (KB, MB, ... are powers of 1000; KiB, MiB, ... powers of 1024), and are given out exactly in the largest unit that divides them (`"1MB"`). + `-1` means no limit (`util.ByteSizeNoLimit`); no other size may be + negative. A setting for which no limit is unsafe rejects `-1`. - A section can reject values by implementing `validate(key, value) error` on - its pointer type. `JobSearchConfigLogging` does this for `level`. + its pointer type. `JSCLogging` does this for `level`, and + `JSCServerWebSocket` for `max_message_size` (rejecting `-1`). Every section implements the `Section` interface defined in `section.go`: `Map`, `Flatten`, `TypeOf`, `ValueOf` and `SetValue`, each a one-line call into diff --git a/internal/config/conf_database.go b/internal/config/conf_database.go index 54b9ecb..a8a3fe7 100644 --- a/internal/config/conf_database.go +++ b/internal/config/conf_database.go @@ -6,7 +6,8 @@ import "reflect" const JSCKDatabase string = "database" // JSCDatabase configures the SQLite database. File is the path to -// the database file. +// the database file; empty means none, and commands that need the database +// fail with a message saying so. type JSCDatabase struct { File string `cfg:"file"` } diff --git a/internal/config/conf_logging.go b/internal/config/conf_logging.go index f6b5538..a03ad07 100644 --- a/internal/config/conf_logging.go +++ b/internal/config/conf_logging.go @@ -10,7 +10,8 @@ import ( const JSCKLogging string = "logging" // JSCLogging configures logging. Level is one of debug, info, -// warn or error. File, if set, is where commands that log to a file write. +// warn or error. File, if set, is where commands that log to a file write; +// empty means they log to the terminal. type JSCLogging struct { Level string `cfg:"level" default:"warn"` File string `cfg:"file"` diff --git a/internal/config/conf_ratelimitrule.go b/internal/config/conf_ratelimitrule.go index b33f8b6..3e7bf0c 100644 --- a/internal/config/conf_ratelimitrule.go +++ b/internal/config/conf_ratelimitrule.go @@ -2,12 +2,14 @@ package config import "reflect" -// JSCRateLimitRule configures one rate limit. The type is used under several -// keys so it has no JSCK* constant and its Flatten keys are not prefixed. +// JSCRateLimitRule configures one rate limit: PerSecond sustained, with +// bursts of up to Burst. It is off unless Enabled and PerSecond is greater +// than 0; a Burst below 1 means 1. The type is used under several keys so it +// has no JSCK* constant and its Flatten keys are not prefixed. type JSCRateLimitRule struct { - Enabled bool `cfg:"enabled" default:"false"` + Enabled bool `cfg:"enabled" default:"false"` PerSecond int `cfg:"per_second" default:"50"` - Burst int `cfg:"burst" default:"100"` + Burst int `cfg:"burst" default:"100"` } // ---------- Section methods (see Section) ---------- diff --git a/internal/config/conf_sections_test.go b/internal/config/conf_sections_test.go index c9593ad..4ada0b5 100644 --- a/internal/config/conf_sections_test.go +++ b/internal/config/conf_sections_test.go @@ -527,3 +527,49 @@ func TestViperValue(t *testing.T) { } } } + +// max_message_size can't be -1 (no limit), directly or through the whole +// configuration; 0 (the default) and explicit sizes are accepted. +func TestWebSocketMaxMessageSizeNoLimit(t *testing.T) { + var c JSCServerWebSocket + if err := c.SetValue("max_message_size", "-1"); err == nil || + !strings.Contains(err.Error(), "can't be -1") { + t.Errorf("SetValue(max_message_size, -1) = %v; want rejected", err) + } + if c.MaxMessageSize != 0 { + t.Errorf("rejected size was stored: %v", c.MaxMessageSize) + } + for _, text := range []string{"0B", "64KiB"} { + if err := c.SetValue("max_message_size", text); err != nil { + t.Errorf("SetValue(max_message_size, %s): %v", text, err) + } + } + conf := defaultConfig(t) + if err := conf.SetValue("server.websocket.max_message_size", "-1"); err == nil { + t.Error("JobSearchConfig.SetValue(server.websocket.max_message_size, -1) succeeded") + } +} + +// A source's interval must be greater than 0, through the embedded section's +// promoted validate; a source is turned off with enabled. +func TestSourceIntervalMustBePositive(t *testing.T) { + conf := defaultConfig(t) + for _, key := range []string{ + "sources.find_a_tender.interval", + "sources.fts_mock.interval", + "sources.sleeper.interval", + } { + for _, value := range []string{"0s", "-1m"} { + err := conf.SetValue(key, value) + if err == nil || !strings.Contains(err.Error(), "greater than 0") { + t.Errorf("SetValue(%s, %s) = %v; want rejected", key, value, err) + } + } + if err := conf.SetValue(key, "1m"); err != nil { + t.Errorf("SetValue(%s, 1m): %v", key, err) + } + } + if conf.Sources.FindATender.Interval.Minutes() != 1 { + t.Errorf("FindATender.Interval = %v; want 1m", conf.Sources.FindATender.Interval) + } +} diff --git a/internal/config/conf_server.go b/internal/config/conf_server.go index 9f54df0..ea08ce4 100644 --- a/internal/config/conf_server.go +++ b/internal/config/conf_server.go @@ -7,7 +7,9 @@ import ( // JSCKServer is the server section's configuration key. const JSCKServer string = "server" -// JSCServer configures the outward server. +// JSCServer configures the outward server. Host is the address to listen +// on (empty means every interface) and Port the port (0 means any free +// port). type JSCServer struct { Host string `cfg:"host" default:"127.0.0.1"` Port int `cfg:"port" default:"9393"` diff --git a/internal/config/conf_server_ratelimits.go b/internal/config/conf_server_ratelimits.go index 58aa87e..ebc0e46 100644 --- a/internal/config/conf_server_ratelimits.go +++ b/internal/config/conf_server_ratelimits.go @@ -9,7 +9,8 @@ import ( const JSCKServerRateLimits string = "rate_limits" // JSCServerRateLimits configures the server's rate limiters. IdleTTL is how -// long a client's limiter is kept after its last request. +// long a client's limiter is kept after its last request; 0 or less means +// limiters are never evicted. type JSCServerRateLimits struct { IdleTTL time.Duration `cfg:"idle_ttl" default:"10m"` } diff --git a/internal/config/conf_server_timeouts.go b/internal/config/conf_server_timeouts.go index a9be776..ccf60e4 100644 --- a/internal/config/conf_server_timeouts.go +++ b/internal/config/conf_server_timeouts.go @@ -9,7 +9,8 @@ import ( const JSCKServerTimeouts string = "timeouts" // JSCServerTimeouts configures the server's timeouts. ShutdownTimeout is how -// long shutdown waits for clients to close. +// long shutdown waits for clients to close; 0 or less means it waits until +// they have. type JSCServerTimeouts struct { ShutdownTimeout time.Duration `cfg:"shutdown" default:"15s"` } diff --git a/internal/config/conf_server_websocket.go b/internal/config/conf_server_websocket.go index ede8957..cef9239 100644 --- a/internal/config/conf_server_websocket.go +++ b/internal/config/conf_server_websocket.go @@ -1,6 +1,7 @@ package config import ( + "errors" "reflect" "time" @@ -11,6 +12,10 @@ import ( const JSCKServerWebSocket string = "websocket" // JSCServerWebSocket configures the server's websocket connections. +// MaxMessageSize is the largest message read from a client; 0 means the +// default (32KiB), and -1 (no limit) is rejected (see validate). A +// WriteTimeout or PongTimeout of 0 or less means no limit, and a +// PingInterval of 0 or less turns pings off. type JSCServerWebSocket struct { EnableCompression bool `cfg:"enable_compression" default:"false"` MaxMessageSize util.ByteSize `cfg:"max_message_size" default:"32KiB"` @@ -40,3 +45,17 @@ func (c JSCServerWebSocket) ValueOf(key string) (any, error) { func (c *JSCServerWebSocket) SetValue(key string, value any) error { return setValue(c, key, value) } + +// ---------- Section specific methods ---------- + +// validate checks values before they are stored (see validator). +// max_message_size can't be -1 (no limit): a client could then send a +// message of any size, and the server holds each message in memory. +func (c *JSCServerWebSocket) validate(key string, value any) error { + if key == "max_message_size" && value.(util.ByteSize) == util.ByteSizeNoLimit { + return errors.New( + "max_message_size can't be -1 (no limit): set an explicit size, or 0 for the default", + ) + } + return nil +} diff --git a/internal/config/conf_source_ftsmock.go b/internal/config/conf_source_ftsmock.go index 9759e14..45a618e 100644 --- a/internal/config/conf_source_ftsmock.go +++ b/internal/config/conf_source_ftsmock.go @@ -6,7 +6,8 @@ import "reflect" const JSCKSourceFTSMock string = "fts_mock" // JSCSourceFTSMock configures fetching from a Find a Tender mock -// server (cmd/mock-fts) at APIRoot. +// server (cmd/mock-fts) at APIRoot, which has no default and must be set when +// the source is enabled. type JSCSourceFTSMock struct { JSCSourcesSource APIRoot string `cfg:"api_root"` diff --git a/internal/config/conf_sources.go b/internal/config/conf_sources.go index 0a82496..d1132f9 100644 --- a/internal/config/conf_sources.go +++ b/internal/config/conf_sources.go @@ -1,6 +1,7 @@ package config import ( + "errors" "reflect" "time" ) @@ -39,7 +40,9 @@ func (c *JSCSources) SetValue(key string, value any) error { // ========== Generic source ========== // JSCSourcesSource holds the settings every source has, and is embedded -// in each source's section. Interval is how often an enabled source fetches. +// in each source's section. Interval is how often an enabled source fetches; +// it must be greater than 0 (validate rejects 0 or less: a source is turned +// off with Enabled, not with its interval). type JSCSourcesSource struct { Enabled bool `cfg:"enabled"` Interval time.Duration `cfg:"interval" default:"10m"` @@ -67,3 +70,14 @@ func (c *JSCSourcesSource) SetValue(key string, value any) error { // Keys returns the keys directly in the section. func (c *JSCSourcesSource) Keys() []string { return keysOf(c) } + +// validate checks values before they are stored (see validator); it is +// promoted to each source's section. interval must be greater than 0. +func (c *JSCSourcesSource) validate(key string, value any) error { + if key == "interval" && value.(time.Duration) <= 0 { + return errors.New( + "interval must be greater than 0; set enabled to false to turn the source off", + ) + } + return nil +}