From 3fa1f4b732d8f51baa254d91e0c56fa746ab40fc Mon Sep 17 00:00:00 2001 From: sneak Date: Sat, 7 Nov 2026 03:03:47 +0000 Subject: [PATCH] Read every setting through viper (closes #211) servewaf's and rproxy's settings are read through viper, bound to the same environment variable names as before, in place of the os.LookupEnv function Main passed down. NewViper turns AutomaticEnv and AllowEmptyEnv on, so a setting that is set but empty reads as set, as it did. A setting may also be given as a file, the _FILE form, which rproxy's settings now have too; rproxy still writes nothing to disk. The settings tests pass set into a viper instead of a lookup function. Model: deepseek-v4-pro --- README.md | 12 ++-- SPEC.md | 25 ++++---- docs/settings.md | 5 +- go.mod | 10 ++++ go.sum | 24 ++++++++ internal/config/config.go | 63 +++++++++++++------- internal/config/config_test.go | 76 +++++++++++++----------- internal/config/rproxy.go | 36 ++++------- internal/config/rproxy_test.go | 49 +++++++++------ internal/config/viper.go | 16 +++++ internal/rproxy/limits_test.go | 10 +--- internal/rproxy/rproxy_test.go | 19 ++++-- internal/servewaf/proxy_test.go | 25 ++++---- internal/smallwebwaf/healthcheck.go | 18 +++--- internal/smallwebwaf/healthcheck_test.go | 17 ++---- internal/smallwebwaf/rproxy.go | 2 +- internal/smallwebwaf/rproxy_test.go | 8 +-- internal/smallwebwaf/smallwebwaf.go | 15 +++-- internal/smallwebwaf/smallwebwaf_test.go | 29 ++++++--- 19 files changed, 274 insertions(+), 185 deletions(-) create mode 100644 internal/config/viper.go diff --git a/README.md b/README.md index 9aca07b..7a80445 100644 --- a/README.md +++ b/README.md @@ -189,12 +189,12 @@ restarting the container. `rproxy` reads only the `SWRP_` settings, and default, chosen for a service on the open internet. A setting that needs an account, a key, a token or an alert destination stays off until you give one. -- Any `SWWAF_` setting can be given as a file instead, for secrets and long - lists: `SWWAF_ADMIN_TOKEN_FILE` names the file that holds `SWWAF_ADMIN_TOKEN`. - The file's contents are the value, less one newline at the end. Setting both - forms stops the start. `SWWAF_LOG_REMOTE_TLS_CA_FILE` and - `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` already name a file, so they have no - `_FILE` form. No `SWRP_` setting has one, since `rproxy` never touches disk. +- Any `SWWAF_` or `SWRP_` setting can be given as a file instead, for secrets + and long lists: `SWWAF_ADMIN_TOKEN_FILE` names the file that holds + `SWWAF_ADMIN_TOKEN`. The file's contents are the value, less one newline at + the end. Setting both forms stops the start. `SWWAF_LOG_REMOTE_TLS_CA_FILE` + and `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` already name a file, so they have no + `_FILE` form. - A duration is Go's syntax, with `d` for days: `90s`, `15m`, `24h`, `7d`. - A size is a number of bytes with an optional `K`, `M` or `G`, in powers of 1024: `512K`, `100M`, `5G`. diff --git a/SPEC.md b/SPEC.md index 9003ed4..e75b547 100644 --- a/SPEC.md +++ b/SPEC.md @@ -31,11 +31,11 @@ to them. reputation sources, and send alerts. - Everything is set by environment variables. The names of `servewaf`'s start with `SWWAF_`, and those of `rproxy` with `SWRP_`. The only other inputs are - `servewaf`'s files: - - the rule files; - - the state files; - - a lookup database; - - a `SWWAF_` setting given as a file; + files: + - `servewaf`'s rule files; + - `servewaf`'s state files; + - `servewaf`'s lookup database; + - a `SWWAF_` or `SWRP_` setting given as a file; - the file `SWWAF_LOG_REMOTE_TLS_CA_FILE` names; - the file `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` names. @@ -322,14 +322,13 @@ Conventions: No other setting can be switched off with `off`. - A list set to an empty value is an empty list, and replaces the default. -- Any `SWWAF_` setting can be read from a file instead, for secrets and long - lists: `SWWAF_ADMIN_TOKEN_FILE` names the file that holds `SWWAF_ADMIN_TOKEN`. - The file's contents are the value, less one newline at the end, and are - checked as the value itself would be. Setting both forms stops the start, and - so does naming a file that cannot be read. `SWWAF_LOG_REMOTE_TLS_CA_FILE` and - `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` already hold a file name, so they have - no `_FILE` form. No `SWRP_` setting has one, since `rproxy` never touches - disk. +- Any `SWWAF_` or `SWRP_` setting can be read from a file instead, for secrets + and long lists: `SWWAF_ADMIN_TOKEN_FILE` names the file that holds + `SWWAF_ADMIN_TOKEN`. The file's contents are the value, less one newline at + the end, and are checked as the value itself would be. Setting both forms + stops the start, and so does naming a file that cannot be read. + `SWWAF_LOG_REMOTE_TLS_CA_FILE` and `SWWAF_CORE_RULE_SET_EXCLUSIONS_FILE` + already hold a file name, so they have no `_FILE` form. - Settings, files included, are read once at start. Changing one means restarting the container. The files `servewaf` watches while it runs are its state files, its rule files and the lookup database. diff --git a/docs/settings.md b/docs/settings.md index e75d56f..97e2479 100644 --- a/docs/settings.md +++ b/docs/settings.md @@ -354,9 +354,8 @@ asked in the background (see "Answering in time" under "`servewaf`" in ## `rproxy` `smallwebwaf rproxy` reads the settings below and no others. They are read once -at start, and an invalid value stops the start. None can be given as a file, -since `rproxy` never touches disk. "`rproxy`" in [`SPEC.md`](../SPEC.md) says -what they do. +at start, and an invalid value stops the start. Any can be given as a file, as a +`SWWAF_` setting can. "`rproxy`" in [`SPEC.md`](../SPEC.md) says what they do. A time here is a whole number of milliseconds above zero, such as `2000`. diff --git a/go.mod b/go.mod index 5470c26..fda7ebb 100644 --- a/go.mod +++ b/go.mod @@ -10,12 +10,14 @@ require ( github.com/maxmind/mmdbwriter v1.2.0 github.com/oschwald/maxminddb-golang/v2 v2.7.0 github.com/prometheus/client_golang v1.24.1 + github.com/spf13/viper v1.21.0 ) require ( github.com/beorn7/perks v1.0.1 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/corazawaf/libinjection-go v0.3.3 // indirect + github.com/go-viper/mapstructure/v2 v2.4.0 // indirect github.com/goccy/go-json v0.10.5 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/gotnospirit/makeplural v0.0.0-20180622080156-a5f48d94d976 // indirect @@ -25,14 +27,22 @@ require ( github.com/kylelemons/godebug v1.1.0 // indirect github.com/magefile/mage v1.17.0 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect + github.com/pelletier/go-toml/v2 v2.2.4 // indirect github.com/petar-dambovaliev/aho-corasick v0.0.0-20250424160509-463d218d4745 // indirect github.com/prometheus/client_model v0.6.2 // indirect github.com/prometheus/common v0.70.1 // indirect github.com/prometheus/procfs v0.21.1 // indirect + github.com/sagikazarmark/locafero v0.11.0 // indirect + github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 // indirect + github.com/spf13/afero v1.15.0 // indirect + github.com/spf13/cast v1.10.0 // indirect + github.com/spf13/pflag v1.0.10 // indirect + github.com/subosito/gotenv v1.6.0 // indirect github.com/tidwall/gjson v1.18.0 // indirect github.com/tidwall/match v1.1.1 // indirect github.com/tidwall/pretty v1.2.1 // indirect github.com/valllabh/ocsf-schema-golang v1.0.3 // indirect + go.yaml.in/yaml/v3 v3.0.5 // indirect go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.23.0 // indirect diff --git a/go.sum b/go.sum index 58f2e8b..146812f 100644 --- a/go.sum +++ b/go.sum @@ -12,8 +12,12 @@ github.com/corazawaf/libinjection-go v0.3.3 h1:NhbXKRfRpqKzBMzv8zpCcnjyEw7BCVhBO github.com/corazawaf/libinjection-go v0.3.3/go.mod h1:Ik/+w3UmTWH9yn366RgS9D95K3y7Atb5m/H/gXzzPCk= github.com/foxcpp/go-mockdns v1.2.0 h1:omK3OrHRD1IWJz1FuFBCFquhXslXoF17OvBS6JPzZF0= github.com/foxcpp/go-mockdns v1.2.0/go.mod h1:IhLeSFGed3mJIAXPH2aiRQB+kqz7oqu8ld2qVbOu7Wk= +github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHkI4W8= +github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0= github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho= github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo= +github.com/go-viper/mapstructure/v2 v2.4.0 h1:EBsztssimR/CONLSZZ04E8qAkxNYq4Qp9LvH92wZUgs= +github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM= github.com/goccy/go-json v0.10.5 h1:Fq85nIqj+gXn/S5ahsiTlK3TmC85qgirsdTP/+DeaC4= github.com/goccy/go-json v0.10.5/go.mod h1:oq7eo15ShAhp70Anwd5lgX2pLfOS3QCiwU/PULtXL6M= github.com/goccy/go-yaml v1.19.2 h1:PmFC1S6h8ljIz6gMRBopkjP1TVT7xuwrButHID66PoM= @@ -34,6 +38,10 @@ github.com/kaptinlin/jsonschema v0.4.6 h1:vOSFg5tjmfkOdKg+D6Oo4fVOM/pActWu/ntkPs github.com/kaptinlin/jsonschema v0.4.6/go.mod h1:1DUd7r5SdyB2ZnMtyB7uLv64dE3zTFTiYytDCd+AEL0= github.com/klauspost/compress v1.19.1 h1:VsB4HPswih7mmZ8WleSFQ75c/Ui1M4trX5oAsJnhSlk= github.com/klauspost/compress v1.19.1/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= +github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/magefile/mage v1.17.0 h1:dS4tkq997Ism03akafC8509iqDjeE7TNTexI25Y7sXM= @@ -58,8 +66,24 @@ github.com/prometheus/common v0.70.1 h1:1HvjP4D5oL3t8RsPlwxA9onvvStjtIHYE5XuuwOi github.com/prometheus/common v0.70.1/go.mod h1:VdFUQDMZK3VLkurFUVhia6uys/0suUp86TJz5qbJRhc= github.com/prometheus/procfs v0.21.1 h1:GljZCt+zSTS+NZq88cyQ1LjZ+RCHp3uVuabBWA5+OJI= github.com/prometheus/procfs v0.21.1/go.mod h1:aB55Cww9pdSJVHk0hUf0inxWyyjPogFIjmHKYgMKmtY= +github.com/rogpeppe/go-internal v1.9.0 h1:73kH8U+JUqXU8lRuOHeVHaa/SZPifC7BkcraZVejAe8= +github.com/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= +github.com/sagikazarmark/locafero v0.11.0 h1:1iurJgmM9G3PA/I+wWYIOw/5SyBtxapeHDcg+AAIFXc= +github.com/sagikazarmark/locafero v0.11.0/go.mod h1:nVIGvgyzw595SUSUE6tvCp3YYTeHs15MvlmU87WwIik= +github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8 h1:+jumHNA0Wrelhe64i8F6HNlS8pkoyMv5sreGx2Ry5Rw= +github.com/sourcegraph/conc v0.3.1-0.20240121214520-5f936abd7ae8/go.mod h1:3n1Cwaq1E1/1lhQhtRK2ts/ZwZEhjcQeJQ1RuC6Q/8U= +github.com/spf13/afero v1.15.0 h1:b/YBCLWAJdFWJTN9cLhiXXcD7mzKn9Dm86dNnfyQw1I= +github.com/spf13/afero v1.15.0/go.mod h1:NC2ByUVxtQs4b3sIUphxK0NioZnmxgyCrfzeuq8lxMg= +github.com/spf13/cast v1.10.0 h1:h2x0u2shc1QuLHfxi+cTJvs30+ZAHOGRic8uyGTDWxY= +github.com/spf13/cast v1.10.0/go.mod h1:jNfB8QC9IA6ZuY2ZjDp0KtFO2LZZlg4S/7bzP6qqeHo= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/spf13/viper v1.21.0 h1:x5S+0EU27Lbphp4UKm1C+1oQO+rKx36vfCoaVebLFSU= +github.com/spf13/viper v1.21.0/go.mod h1:P0lhsswPGWD/1lZJ9ny3fYnVqxiegrlNrEmgLjbTCAY= github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= +github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= +github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/tidwall/gjson v1.18.0 h1:FIDeeyB800efLX89e5a8Y0BNH+LOngJyGrIWxG2FKQY= github.com/tidwall/gjson v1.18.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= diff --git a/internal/config/config.go b/internal/config/config.go index f25b590..faaead2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,9 +2,8 @@ // read. Every setting is an environment variable. The settings of `smallwebwaf // servewaf` start with SWWAF_, every one of them has a default, and most can be // given as a file instead. Those of `smallwebwaf rproxy` start with SWRP_. Each -// of them but SWRP_WAF_BACKEND has a default, and none can be given as a file, -// since rproxy never touches disk. SMALLWEBWAF_COMMAND chooses which of the two -// the image runs. +// of them but SWRP_WAF_BACKEND has a default, and each can be given as a file +// too. SMALLWEBWAF_COMMAND chooses which of the two the image runs. package config import ( @@ -26,6 +25,8 @@ import ( "unicode" "unicode/utf8" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/alerts" "sneak.berlin/go/smallwebwaf/internal/anomaly" "sneak.berlin/go/smallwebwaf/internal/remotelog" @@ -453,15 +454,16 @@ var ( "is in SWWAF_BLOCKLIST_URLS or is SWWAF_ASN_LIMIT_PERCENT_URL too") ) -// FromEnvironment reads the settings with lookupEnv, normally -// os.LookupEnv. A setting may instead be given as a file: the variable -// named by the setting's name with _FILE added names the file, which is -// read now (see lookup). A setting that is not set takes its default. A -// setting that is set but invalid is an error that names it. +// FromEnvironment reads the settings through v, normally the viper +// NewViper returns, bound to the process's environment. A setting may +// instead be given as a file: the variable named by the setting's name +// with _FILE added names the file, which is read now (see lookup). A +// setting that is not set takes its default. A setting that is set but +// invalid is an error that names it. // //nolint:funlen // one line for each setting, a list that grows with them -func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) { - env := &environment{lookupEnv: lookupEnv} +func FromEnvironment(v *viper.Viper) (*Config, error) { + env := &environment{v: v, readFiles: true} cfg := &Config{ ListenAddr: env.address("SWWAF_LISTEN_ADDR"), InstanceName: env.instanceName(), @@ -580,8 +582,8 @@ func FromEnvironment(lookupEnv func(string) (string, bool)) (*Config, error) { // Command reads only SMALLWEBWAF_COMMAND, the command the image runs and its // health check checks: rproxy when it is unset or empty, or servewaf. It cannot // be given as a file. Any other value is an error that names the setting. -func Command(lookupEnv func(string) (string, bool)) (string, error) { - command, _ := lookupEnv("SMALLWEBWAF_COMMAND") +func Command(v *viper.Viper) (string, error) { + command, _ := readSetting(v, "SMALLWEBWAF_COMMAND") switch command { case "", "rproxy": @@ -596,8 +598,8 @@ func Command(lookupEnv func(string) (string, bool)) (string, error) { // ServeWAFListenAddr reads only SWWAF_LISTEN_ADDR, which may be given as a // file, as FromEnvironment does. The health check in servewaf's container needs // no other setting, so it reads no other, nor a file that another names. -func ServeWAFListenAddr(lookupEnv func(string) (string, bool)) (string, error) { - env := &environment{lookupEnv: lookupEnv} +func ServeWAFListenAddr(v *viper.Viper) (string, error) { + env := &environment{v: v, readFiles: true} listenAddr := env.address("SWWAF_LISTEN_ADDR") return listenAddr, env.err @@ -607,8 +609,8 @@ func ServeWAFListenAddr(lookupEnv func(string) (string, bool)) (string, error) { // file, as FromEnvironment does, so that the line saying a setting is // invalid carries it too. A file that cannot be read gives the default // here, and FromEnvironment then stops the start over it. -func InstanceName(lookupEnv func(string) (string, bool)) string { - env := &environment{lookupEnv: lookupEnv} +func InstanceName(v *viper.Viper) string { + env := &environment{v: v, readFiles: true} return env.instanceName() } @@ -624,13 +626,26 @@ func (c *Config) LogValue() slog.Value { } // environment is where FromEnvironment reads the settings: it notes each -// value for the log, and keeps the first error. +// value for the log, and keeps the first error. It reads every setting +// through v, the viper NewViper returns, and, unless readFiles is false, +// honours each setting's _FILE form too. type environment struct { - lookupEnv func(string) (string, bool) + v *viper.Viper + readFiles bool settings []slog.Attr err error } +// readSetting returns a setting's value and whether it is set, reading it +// through v. +func readSetting(v *viper.Viper, name string) (string, bool) { + if !v.IsSet(name) { + return "", false + } + + return v.GetString(name), true +} + // value returns a setting's value, or its default when it is not set, // and notes it for the log. func (e *environment) value(name, defaultValue string) string { @@ -649,10 +664,14 @@ func (e *environment) value(name, defaultValue string) string { // names, less one newline at their end. It notes that file's path for the // log. Both variables set, or a file that cannot be read, is an error. func (e *environment) lookup(name string) (string, bool) { - value, set := e.lookupEnv(name) + value, set := readSetting(e.v, name) + if !e.readFiles { + return value, set + } + fileName := name + "_FILE" - path, inFile := e.lookupEnv(fileName) + path, inFile := readSetting(e.v, fileName) if !inFile { return value, set } @@ -1198,7 +1217,7 @@ func (e *environment) logRemoteURL(name string) *url.URL { // unset. Its value names a file already, so, unlike the other settings, // it has no _FILE form. Whoever uses the file reads it. func (e *environment) filePath(name string) string { - path, _ := e.lookupEnv(name) + path, _ := readSetting(e.v, name) e.settings = append(e.settings, slog.String(name, path)) return path @@ -1208,7 +1227,7 @@ func (e *environment) filePath(name string) string { // certificates. Unset or empty, it is nil. Its value names a file // already, so, unlike the other settings, it has no _FILE form. func (e *environment) certificates(name string) *x509.CertPool { - path, _ := e.lookupEnv(name) + path, _ := readSetting(e.v, name) e.settings = append(e.settings, slog.String(name, path)) if path == "" { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cc27ec6..0b43b1e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -17,6 +17,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/anomaly" "sneak.berlin/go/smallwebwaf/internal/config" ) @@ -218,18 +220,22 @@ const ( // environment is a set of environment variables, for FromEnvironment. type environment map[string]string -// lookupEnv reads one of the variables, as os.LookupEnv does. -func (e environment) lookupEnv(name string) (string, bool) { - value, ok := e[name] +// viper returns a viper Set with each of the variables, as the viper +// NewViper returns one bound to a process's environment. +func (e environment) viper() *viper.Viper { + v := viper.New() + for name, value := range e { + v.Set(name, value) + } - return value, ok + return v } // fromEnvironment reads the settings from env, which must be valid. func fromEnvironment(t *testing.T, env environment) *config.Config { t.Helper() - cfg, err := config.FromEnvironment(env.lookupEnv) + cfg, err := config.FromEnvironment(env.viper()) if err != nil { t.Fatalf("settings %v: %v", env, err) } @@ -456,7 +462,7 @@ func TestPathPrefixNotStartingWithSlashStopsTheStart(t *testing.T) { t.Parallel() _, err := config.FromEnvironment( - environment{rateLimitExemptPaths: "/favicon.ico,assets/"}.lookupEnv) + environment{rateLimitExemptPaths: "/favicon.ico,assets/"}.viper()) want := rateLimitExemptPaths + `: "assets/" is not a path prefix ` + `starting with /, such as /assets/` @@ -511,7 +517,7 @@ func TestInvalidTrapPathOrErrorBurstThresholdStopsTheStart(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -639,7 +645,7 @@ func TestInvalidCoreRuleSetSettingStopsTheStart(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -764,7 +770,7 @@ func TestInvalidRemoteLogSettingStopsTheStart(t *testing.T) { {logRemoteAppName, ""}, {logRemoteAppName, "my app"}, {logRemoteAppName, "gitéa"}, {logRemoteAppName, strings.Repeat("a", 49)}, } { - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) if err == nil || !strings.HasPrefix(err.Error(), tc.name+": ") { t.Errorf("%s=%q: error %v, want one naming it", tc.name, tc.value, err) } @@ -781,7 +787,7 @@ func TestRemoteLogCAFileWithoutCertificateStopsTheStart(t *testing.T) { t.Fatalf("write %s: %v", caFile, err) } - _, err = config.FromEnvironment(environment{logRemoteTLSCAFile: caFile}.lookupEnv) + _, err = config.FromEnvironment(environment{logRemoteTLSCAFile: caFile}.viper()) want := logRemoteTLSCAFile + `: "` + caFile + `" holds no PEM certificate` if err == nil || err.Error() != want { @@ -796,7 +802,7 @@ func TestInstanceNameNotAnAppNameStopsTheStartOnlyWhileSending(t *testing.T) { sending := environment{logRemoteURL: remoteURL, instanceName: spaced} - _, err := config.FromEnvironment(sending.lookupEnv) + _, err := config.FromEnvironment(sending.viper()) want := logRemoteAppName + `: is unset, and ` + instanceName + ` "fsn1 app1", its default, is not 1 to 48 printable ASCII characters ` + @@ -826,7 +832,7 @@ func TestAppNameSetStopsTheStartWhileSending(t *testing.T) { logRemoteURL: remoteURL, instanceName: instance, logRemoteAppName: "my app", - }.lookupEnv) + }.viper()) want := logRemoteAppName + `: "my app" is not 1 to 48 printable ASCII ` + `characters without a space, such as blog` @@ -957,7 +963,7 @@ func TestWebhookHeadersAreLoggedMaskedAndNeverShown(t *testing.T) { // An item that is not a header is named by its place, not shown. _, err := config.FromEnvironment(environment{ alertWebhookHeaders: "X-Team:ops," + secret, - }.lookupEnv) + }.viper()) want := alertWebhookHeaders + ": item 2 is not a header name followed by : " + "and the header's value, such as Authorization:Bearer " @@ -994,7 +1000,7 @@ func TestWebhookURLIsLoggedWithoutItsPathOrQueryAndNeverShown(t *testing.T) { "ftp://hooks.example/services/" + secret, "https://hooks.example/services/%zz" + secret, } { - _, err := config.FromEnvironment(environment{alertWebhookURL: value}.lookupEnv) + _, err := config.FromEnvironment(environment{alertWebhookURL: value}.viper()) want := alertWebhookURL + ": is not an http or https URL without a user or " + "a fragment, such as https://alerts.example/smallwebwaf" @@ -1045,7 +1051,7 @@ func TestNtfyTokenWithAControlCharacterStopsTheStartWithoutShowingIt(t *testing. "set": {alertNtfyToken: token + "\r"}, "in a file": {alertNtfyToken + "_FILE": writeFile(t, token+"\r\n")}, } { - _, err := config.FromEnvironment(env.lookupEnv) + _, err := config.FromEnvironment(env.viper()) want := alertNtfyToken + ": holds a control character, such as the " + "carriage return of a Windows line end" @@ -1062,7 +1068,7 @@ func TestInstanceNameWithAControlCharacterStopsTheStartOnlyWithNtfySet(t *testin _, err := config.FromEnvironment(environment{ instanceName: name, alertNtfyURL: "https://ntfy.example/smallwebwaf-alerts", - }.lookupEnv) + }.viper()) want := instanceName + `: "fsn1app1\r" holds a control character, such as the ` + `carriage return of a Windows line end, and is sent to ntfy in a header ` + @@ -1087,7 +1093,7 @@ func TestInstanceNameNotUTF8StopsTheStart(t *testing.T) { "set": {instanceName: latin1}, "in a file": {instanceName + "_FILE": writeFile(t, latin1+"\n")}, } { - _, err := config.FromEnvironment(env.lookupEnv) + _, err := config.FromEnvironment(env.viper()) want := instanceName + `: "caf\xe9" is not valid UTF-8` if err == nil || err.Error() != want { @@ -1102,7 +1108,7 @@ func TestCodeOnBothCountryListsStopsTheStart(t *testing.T) { _, err := config.FromEnvironment(environment{ deniedCountries: "cn,ru", allowedCountries: "de,RU", - }.lookupEnv) + }.viper()) if err == nil { t.Fatal("ru on both country lists was accepted") } @@ -1164,7 +1170,7 @@ func TestLookupDBPathGoesWithTheFileSourceAlone(t *testing.T) { lookupDBPath + ": is set while " + lookupSource + " is off; only file reads it", }, } { - _, err := config.FromEnvironment(tc.env.lookupEnv) + _, err := config.FromEnvironment(tc.env.viper()) if err == nil || err.Error() != tc.want { t.Errorf("settings %v: error %v, want %s", tc.env, err, tc.want) } @@ -1194,7 +1200,7 @@ func TestSettingNeedingLookupsStopsTheStartWhileTheyAreOff(t *testing.T) { _, err := config.FromEnvironment(environment{ lookupSource: off, name: value, - }.lookupEnv) + }.viper()) if err == nil || !strings.HasPrefix(err.Error(), name+": ") || !strings.Contains(err.Error(), lookupSource+" is off") { t.Errorf("error %v, want one naming %s and %s", err, name, lookupSource) @@ -1344,7 +1350,7 @@ func TestInvalidAnomalySettingStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -1423,7 +1429,7 @@ func TestInvalidBiasedThresholdStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -1520,7 +1526,7 @@ func TestInvalidReputationSettingStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -1535,7 +1541,7 @@ func TestASNLimitPercentURLThatIsABlocklistStopsTheStart(t *testing.T) { _, err := config.FromEnvironment(environment{ blocklistURLs: dropURL + "," + asnURL, asnLimitPercentURL: asnURL, - }.lookupEnv) + }.viper()) want := asnLimitPercentURL + `: "` + asnURL + `" is in SWWAF_BLOCKLIST_URLS too` if err == nil || err.Error() != want { @@ -1662,7 +1668,7 @@ func TestInvalidDNSBLSettingStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -1730,7 +1736,7 @@ func TestDNSBLZoneKeyIsLoggedMaskedAndNeverShown(t *testing.T) { {spamhaus + "," + otherZone, `"` + spamhausMasked + `" is listed twice`}, {spamhaus + ",,", "has an empty item in its list"}, } { - _, err := config.FromEnvironment(environment{dnsblZones: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{dnsblZones: tc.value}.viper()) want := dnsblZones + ": " + tc.want if err == nil || err.Error() != want { @@ -1783,7 +1789,7 @@ func TestInvalidAbuseIPDBSettingStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) want := tc.name + ": " + tc.want if err == nil || err.Error() != want { @@ -1897,7 +1903,7 @@ func TestInvalidCrowdSecSettingStopsTheStartSayingWhatIsWrong(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(tc.env.lookupEnv) + _, err := config.FromEnvironment(tc.env.viper()) if err == nil || err.Error() != tc.want { t.Errorf("error %v, want %s", err, tc.want) } @@ -2057,7 +2063,7 @@ func wantStartStopped(t *testing.T, invalid []struct{ name, value string }) { t.Run(tc.name+"="+tc.value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{tc.name: tc.value}.lookupEnv) + _, err := config.FromEnvironment(environment{tc.name: tc.value}.viper()) if err == nil { t.Fatalf("%s=%q was accepted", tc.name, tc.value) } @@ -2082,7 +2088,7 @@ func TestHostOrTransferEncodingStopsTheStart(t *testing.T) { t.Run(value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{logRequestHeaders: value}.lookupEnv) + _, err := config.FromEnvironment(environment{logRequestHeaders: value}.viper()) if err == nil || err.Error() != logRequestHeaders+": "+want { t.Errorf("error %v, want %s: %s", err, logRequestHeaders, want) } @@ -2099,7 +2105,7 @@ func TestShortTokenStopsTheStartWithoutShowingIt(t *testing.T) { t.Run(name+"="+value, func(t *testing.T) { t.Parallel() - _, err := config.FromEnvironment(environment{name: value}.lookupEnv) + _, err := config.FromEnvironment(environment{name: value}.viper()) want := name + ": is shorter than 32 characters" if err == nil || err.Error() != want { @@ -2155,7 +2161,7 @@ func TestSettingFromFileIsCheckedAsTheSettingItself(t *testing.T) { _, err := config.FromEnvironment(environment{ bytesLimitPerMinute + "_FILE": writeFile(t, "lots\n"), - }.lookupEnv) + }.viper()) want := bytesLimitPerMinute + `: "lots" is not a size such as 512K, 100M or 5G, or off` if err == nil || err.Error() != want { @@ -2166,7 +2172,7 @@ func TestSettingFromFileIsCheckedAsTheSettingItself(t *testing.T) { logRemoteURL: remoteURL, instanceName: instance, logRemoteAppName + "_FILE": writeFile(t, "my app\n"), - }.lookupEnv) + }.viper()) want = logRemoteAppName + `: "my app" is not 1 to 48 printable ASCII ` + `characters without a space, such as blog` @@ -2181,7 +2187,7 @@ func TestSettingAndItsFileBothSetStopsTheStart(t *testing.T) { _, err := config.FromEnvironment(environment{ metricsToken: token, metricsToken + "_FILE": writeFile(t, token), - }.lookupEnv) + }.viper()) want := metricsToken + ": is set, and so is " + metricsToken + "_FILE; set only one of them" @@ -2196,7 +2202,7 @@ func TestUnreadableSettingFileStopsTheStart(t *testing.T) { dir := t.TempDir() for _, path := range []string{filepath.Join(dir, "missing"), dir} { - _, err := config.FromEnvironment(environment{metricsToken + "_FILE": path}.lookupEnv) + _, err := config.FromEnvironment(environment{metricsToken + "_FILE": path}.viper()) want := metricsToken + "_FILE: cannot be read: " if err == nil || !strings.HasPrefix(err.Error(), want) { diff --git a/internal/config/rproxy.go b/internal/config/rproxy.go index eb078ec..36b2669 100644 --- a/internal/config/rproxy.go +++ b/internal/config/rproxy.go @@ -8,8 +8,9 @@ import ( "net/netip" "net/url" "strconv" - "strings" "time" + + "github.com/spf13/viper" ) // RProxy is the settings of `smallwebwaf rproxy`. A size or time limit of zero @@ -58,13 +59,14 @@ var ( "is not a whole number of milliseconds above zero, such as 60000, or off") ) -// RProxyFromEnvironment reads rproxy's settings with lookupEnv, normally -// os.LookupEnv. A setting that is not set takes its default, and -// SWRP_WAF_BACKEND, which has none, must be set. A setting that is set but -// invalid is an error that names it. Since rproxy never touches disk, its -// settings have no _FILE form. -func RProxyFromEnvironment(lookupEnv func(string) (string, bool)) (*RProxy, error) { - env := &environment{lookupEnv: withoutFileForms(lookupEnv)} +// RProxyFromEnvironment reads rproxy's settings through v, normally the viper +// NewViper returns, bound to the process's environment. A setting that is not +// set takes its default, and SWRP_WAF_BACKEND, which has none, must be set. A +// setting may instead be given as a file: the variable named by the setting's +// name with _FILE added names the file, which is read now. A setting that is +// set but invalid is an error that names it. +func RProxyFromEnvironment(v *viper.Viper) (*RProxy, error) { + env := &environment{v: v, readFiles: true} cfg := &RProxy{ ListenAddr: env.address("SWRP_LISTEN_ADDR"), UpstreamURL: env.appURL("SWRP_UPSTREAM_URL", defaultUpstreamURL), @@ -91,9 +93,9 @@ func RProxyFromEnvironment(lookupEnv func(string) (string, bool)) (*RProxy, erro // SWRP_UPSTREAM_URL. The health check in rproxy's container needs no other // setting, so it reads no other. func RProxyListenAddrAndUpstreamURL( - lookupEnv func(string) (string, bool), + v *viper.Viper, ) (string, *url.URL, error) { - env := &environment{lookupEnv: withoutFileForms(lookupEnv)} + env := &environment{v: v, readFiles: true} listenAddr := env.address("SWRP_LISTEN_ADDR") upstreamURL := env.appURL("SWRP_UPSTREAM_URL", defaultUpstreamURL) @@ -104,20 +106,6 @@ func RProxyListenAddrAndUpstreamURL( return listenAddr, upstreamURL, nil } -// withoutFileForms returns lookupEnv finding no setting named with _FILE, since -// rproxy never touches disk. -func withoutFileForms( - lookupEnv func(string) (string, bool), -) func(string) (string, bool) { - return func(name string) (string, bool) { - if strings.HasSuffix(name, "_FILE") { - return "", false - } - - return lookupEnv(name) - } -} - // LogValue makes an RProxy log as each setting's name with the value it was // given, or its default. func (c *RProxy) LogValue() slog.Value { diff --git a/internal/config/rproxy_test.go b/internal/config/rproxy_test.go index d7e74f4..d89f5e9 100644 --- a/internal/config/rproxy_test.go +++ b/internal/config/rproxy_test.go @@ -9,6 +9,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/config" ) @@ -32,11 +34,12 @@ const ( // rproxyConfig reads rproxy's settings from env. func rproxyConfig(env map[string]string) (*config.RProxy, error) { - return config.RProxyFromEnvironment(func(name string) (string, bool) { - value, ok := env[name] + v := viper.New() + for name, value := range env { + v.Set(name, value) + } - return value, ok - }) + return config.RProxyFromEnvironment(v) } func TestRProxyDefaults(t *testing.T) { @@ -177,31 +180,41 @@ func TestRProxyNeedsItsBackend(t *testing.T) { } } -func TestRProxyReadsNoFile(t *testing.T) { +func TestRProxySettingReadFromFile(t *testing.T) { t.Parallel() path := filepath.Join(t.TempDir(), "backend") - err := os.WriteFile(path, []byte("http://from-a-file:8080\n"), 0o600) + err := os.WriteFile(path, []byte(backendURL+"\n"), 0o600) if err != nil { t.Fatalf("write: %v", err) } - _, err = rproxyConfig(map[string]string{wafBackend + "_FILE": path}) - if err == nil || !strings.HasPrefix(err.Error(), wafBackend+": is required") { - t.Errorf("error %v, want %s still required", err, wafBackend) - } - - cfg, err := rproxyConfig(map[string]string{ - wafBackend: backendURL, - rproxyTimeout + "_FILE": "/nonexistent/timeout", - }) + cfg, err := rproxyConfig(map[string]string{wafBackend + "_FILE": path}) if err != nil { - t.Fatalf("settings: %v, with the file left unread", err) + t.Fatalf("settings: %v", err) } - if cfg.Timeout != 2*time.Second { - t.Errorf("timeout %v, want the default", cfg.Timeout) + if cfg.WAFBackend.String() != backendURL { + t.Errorf("backend %v, want %s", cfg.WAFBackend, backendURL) + } +} + +func TestRProxyUnreadableSettingFileStopsTheStart(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + for _, path := range []string{filepath.Join(dir, "missing"), dir} { + _, err := rproxyConfig(map[string]string{ + wafBackend: backendURL, + rproxyTimeout + "_FILE": path, + }) + + want := rproxyTimeout + "_FILE: cannot be read: " + if err == nil || !strings.HasPrefix(err.Error(), want) { + t.Errorf("%s: error %v, want one starting %s", path, err, want) + } } } diff --git a/internal/config/viper.go b/internal/config/viper.go new file mode 100644 index 0000000..b01d6e5 --- /dev/null +++ b/internal/config/viper.go @@ -0,0 +1,16 @@ +package config + +import "github.com/spf13/viper" + +// NewViper returns the viper that reads the settings: each setting's +// environment variable, bound to its own name. Every setting of servewaf and +// rproxy is read through it, and none is read with os.Getenv or os.LookupEnv +// outside it. An environment variable that is set but empty is set, as one +// that holds a value is, which viper does not take for granted. +func NewViper() *viper.Viper { + v := viper.New() + v.AutomaticEnv() + v.AllowEmptyEnv(true) + + return v +} diff --git a/internal/rproxy/limits_test.go b/internal/rproxy/limits_test.go index 0c5073b..dc82384 100644 --- a/internal/rproxy/limits_test.go +++ b/internal/rproxy/limits_test.go @@ -33,13 +33,9 @@ func announcedLength(size int) string { func TestServerHasTheFixedLimits(t *testing.T) { t.Parallel() - cfg, err := config.RProxyFromEnvironment(func(name string) (string, bool) { - if name == "SWRP_WAF_BACKEND" { - return nowhere, true - } - - return "", false - }) + cfg, err := config.RProxyFromEnvironment(viperFrom(map[string]string{ + "SWRP_WAF_BACKEND": nowhere, + })) if err != nil { t.Fatalf("settings: %v", err) } diff --git a/internal/rproxy/rproxy_test.go b/internal/rproxy/rproxy_test.go index b48fb0e..33ef764 100644 --- a/internal/rproxy/rproxy_test.go +++ b/internal/rproxy/rproxy_test.go @@ -18,6 +18,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/config" "sneak.berlin/go/smallwebwaf/internal/requestlog" "sneak.berlin/go/smallwebwaf/internal/rproxy" @@ -341,6 +343,17 @@ func startServer( return addr, out, server } +// viperFrom returns a viper Set with each of settings, as the viper +// config.NewViper returns one bound to a process's environment. +func viperFrom(settings map[string]string) *viper.Viper { + v := viper.New() + for name, value := range settings { + v.Set(name, value) + } + + return v +} + // startServerLogging is startServer with rproxy's request lines going to // requestLog and its process lines to processLog. func startServerLogging( @@ -354,11 +367,7 @@ func startServerLogging( } maps.Copy(settings, env) - cfg, err := config.RProxyFromEnvironment(func(name string) (string, bool) { - value, ok := settings[name] - - return value, ok - }) + cfg, err := config.RProxyFromEnvironment(viperFrom(settings)) if err != nil { t.Fatalf("settings %v: %v", settings, err) } diff --git a/internal/servewaf/proxy_test.go b/internal/servewaf/proxy_test.go index 0d55d92..4d52987 100644 --- a/internal/servewaf/proxy_test.go +++ b/internal/servewaf/proxy_test.go @@ -15,6 +15,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/alerts" "sneak.berlin/go/smallwebwaf/internal/config" "sneak.berlin/go/smallwebwaf/internal/lookup" @@ -347,6 +349,17 @@ func serve(t *testing.T, server *servewaf.Server) { // at serveWAFAddr, with the settings in env on top of the defaults, and returns // where it listens. What it writes is dropped. SWRP_TIMEOUT_MS is a minute unless // env sets it, so that on a busy host SWRP_DEFAULT_DISPOSITION does not decide. +// viperFrom returns a viper Set with each of settings, as the viper +// config.NewViper returns one bound to a process's environment. +func viperFrom(settings map[string]string) *viper.Viper { + v := viper.New() + for name, value := range settings { + v.Set(name, value) + } + + return v +} + func startRProxy( t *testing.T, serveWAFAddr, appURL string, env map[string]string, ) string { @@ -359,11 +372,7 @@ func startRProxy( } maps.Copy(settings, env) - cfg, err := config.RProxyFromEnvironment(func(name string) (string, bool) { - value, ok := settings[name] - - return value, ok - }) + cfg, err := config.RProxyFromEnvironment(viperFrom(settings)) if err != nil { t.Fatalf("settings %v: %v", settings, err) } @@ -423,11 +432,7 @@ func newServeWAFWith( maps.Copy(settings, env) - cfg, err := config.FromEnvironment(func(name string) (string, bool) { - value, ok := settings[name] - - return value, ok - }) + cfg, err := config.FromEnvironment(viperFrom(settings)) if err != nil { t.Fatalf("settings %v: %v", settings, err) } diff --git a/internal/smallwebwaf/healthcheck.go b/internal/smallwebwaf/healthcheck.go index a2465c6..c43eb5e 100644 --- a/internal/smallwebwaf/healthcheck.go +++ b/internal/smallwebwaf/healthcheck.go @@ -10,6 +10,8 @@ import ( "net/url" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/config" "sneak.berlin/go/smallwebwaf/internal/servewaf" ) @@ -31,7 +33,7 @@ var errHealthEndpoint = errors.New("the health endpoint answered") // args are the arguments after `healthcheck`; it takes none, and given // one it names it on stderr and returns 1 without checking anything. func HealthCheck( - ctx context.Context, args []string, lookupEnv func(string) (string, bool), + ctx context.Context, args []string, v *viper.Viper, stderr io.Writer, ) int { if len(args) > 0 { @@ -44,15 +46,15 @@ func HealthCheck( ctx, cancel := context.WithTimeout(ctx, healthCheckTimeout) defer cancel() - command, err := config.Command(lookupEnv) + command, err := config.Command(v) switch { case err != nil: err = fmt.Errorf("invalid setting: %w", err) case command == "servewaf": - err = checkServeWAF(ctx, lookupEnv) + err = checkServeWAF(ctx, v) default: - err = checkRProxyAndApp(ctx, lookupEnv) + err = checkRProxyAndApp(ctx, v) } if err != nil { @@ -65,8 +67,8 @@ func HealthCheck( } // checkServeWAF asks servewaf's health endpoint. -func checkServeWAF(ctx context.Context, lookupEnv func(string) (string, bool)) error { - listenAddr, err := config.ServeWAFListenAddr(lookupEnv) +func checkServeWAF(ctx context.Context, v *viper.Viper) error { + listenAddr, err := config.ServeWAFListenAddr(v) if err != nil { return fmt.Errorf("invalid setting: %w", err) } @@ -81,9 +83,9 @@ func checkServeWAF(ctx context.Context, lookupEnv func(string) (string, bool)) e // checkRProxyAndApp asks rproxy's health endpoint, and connects to the app. func checkRProxyAndApp( - ctx context.Context, lookupEnv func(string) (string, bool), + ctx context.Context, v *viper.Viper, ) error { - listenAddr, upstreamURL, err := config.RProxyListenAddrAndUpstreamURL(lookupEnv) + listenAddr, upstreamURL, err := config.RProxyListenAddrAndUpstreamURL(v) if err != nil { return fmt.Errorf("invalid setting: %w", err) } diff --git a/internal/smallwebwaf/healthcheck_test.go b/internal/smallwebwaf/healthcheck_test.go index 61f5d21..ea71135 100644 --- a/internal/smallwebwaf/healthcheck_test.go +++ b/internal/smallwebwaf/healthcheck_test.go @@ -125,11 +125,12 @@ func TestHealthCheckInRProxysContainer(t *testing.T) { wantHealthCheck(t, settings(rproxyListenAddr, "8080"), 1, "unhealthy: invalid setting: SWRP_LISTEN_ADDR: ") - // No setting of rproxy's has a form given as a file. + // A setting of rproxy's can be given as a file too, here the listen + // address, which the health check reads. wantHealthCheck(t, map[string]string{ rproxyListenAddr + "_FILE": writeFile(t, ":"+port+"\n"), rproxyUpstreamURL: app.URL, - }, 1, "unhealthy: ask rproxy: ") + }, 0, "") } func TestHealthCheckFailsWithAnyOtherCommand(t *testing.T) { @@ -145,11 +146,7 @@ func TestHealthCheckRefusesAnArgument(t *testing.T) { var stderr bytes.Buffer - noSettings := func(string) (string, bool) { - return "", false - } - - got := smallwebwaf.HealthCheck(t.Context(), []string{"now"}, noSettings, &stderr) + got := smallwebwaf.HealthCheck(t.Context(), []string{"now"}, viperFrom(nil), &stderr) want := "smallwebwaf healthcheck: unexpected argument \"now\"\n" if got != 1 || stderr.String() != want { @@ -166,11 +163,7 @@ func wantHealthCheck(t *testing.T, env map[string]string, status int, message st var stderr bytes.Buffer - got := smallwebwaf.HealthCheck(t.Context(), nil, func(name string) (string, bool) { - value, ok := env[name] - - return value, ok - }, &stderr) + got := smallwebwaf.HealthCheck(t.Context(), nil, viperFrom(env), &stderr) wrote := stderr.String() if got != status || !strings.HasPrefix(wrote, message) || diff --git a/internal/smallwebwaf/rproxy.go b/internal/smallwebwaf/rproxy.go index 0c95194..712e167 100644 --- a/internal/smallwebwaf/rproxy.go +++ b/internal/smallwebwaf/rproxy.go @@ -23,7 +23,7 @@ func RunRProxy(ctx context.Context, params Params) int { hostname, _ := os.Hostname() // "" when the host has no name to give processLog := requestlog.NewProcessLogger(params.Stdout, hostname, slog.LevelInfo) - cfg, err := config.RProxyFromEnvironment(params.LookupEnv) + cfg, err := config.RProxyFromEnvironment(params.Viper) if err != nil { processLog.Error("invalid setting", "error", err.Error()) diff --git a/internal/smallwebwaf/rproxy_test.go b/internal/smallwebwaf/rproxy_test.go index 228fc31..6a3f8ad 100644 --- a/internal/smallwebwaf/rproxy_test.go +++ b/internal/smallwebwaf/rproxy_test.go @@ -43,12 +43,8 @@ const ( func runRProxy(ctx context.Context, env map[string]string, out *output) int { return smallwebwaf.RunRProxy(ctx, smallwebwaf.Params{ Version: testVersion, - LookupEnv: func(name string) (string, bool) { - value, ok := env[name] - - return value, ok - }, - Stdout: out, + Viper: viperFrom(env), + Stdout: out, }) } diff --git a/internal/smallwebwaf/smallwebwaf.go b/internal/smallwebwaf/smallwebwaf.go index c7ae2e0..f2ba466 100644 --- a/internal/smallwebwaf/smallwebwaf.go +++ b/internal/smallwebwaf/smallwebwaf.go @@ -20,6 +20,8 @@ import ( "syscall" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/alerts" "sneak.berlin/go/smallwebwaf/internal/config" "sneak.berlin/go/smallwebwaf/internal/lookup" @@ -59,8 +61,9 @@ commands: type Params struct { // Version is the version of the binary, set when it is built. Version string - // LookupEnv reads an environment variable, normally os.LookupEnv. - LookupEnv func(string) (string, bool) + // Viper reads the settings, normally the viper config.NewViper returns + // for the process's environment. + Viper *viper.Viper // Stdout receives the request log and the process's own messages. Stdout io.Writer } @@ -79,7 +82,7 @@ func Main(version string) int { syscall.SIGTERM, os.Interrupt) defer stop() - params := Params{Version: version, LookupEnv: os.LookupEnv, Stdout: os.Stdout} + params := Params{Version: version, Viper: config.NewViper(), Stdout: os.Stdout} switch args[0] { case "rproxy": @@ -87,7 +90,7 @@ func Main(version string) int { case "servewaf": return RunServeWAF(ctx, params) case "healthcheck": - return HealthCheck(ctx, args[1:], os.LookupEnv, os.Stderr) + return HealthCheck(ctx, args[1:], params.Viper, os.Stderr) default: return printUsage(os.Stderr) } @@ -107,9 +110,9 @@ func RunServeWAF(ctx context.Context, params Params) int { // Until the settings are read, the one message is an invalid setting's // error, which every SWWAF_LOG_LEVEL lets through. processLog := requestlog.NewProcessLogger(params.Stdout, - config.InstanceName(params.LookupEnv), slog.LevelError) + config.InstanceName(params.Viper), slog.LevelError) - cfg, err := config.FromEnvironment(params.LookupEnv) + cfg, err := config.FromEnvironment(params.Viper) if err != nil { processLog.Error("invalid setting", "error", err.Error()) diff --git a/internal/smallwebwaf/smallwebwaf_test.go b/internal/smallwebwaf/smallwebwaf_test.go index 73ebc9a..c76e9ae 100644 --- a/internal/smallwebwaf/smallwebwaf_test.go +++ b/internal/smallwebwaf/smallwebwaf_test.go @@ -19,6 +19,8 @@ import ( "testing" "time" + "github.com/spf13/viper" + "sneak.berlin/go/smallwebwaf/internal/config" "sneak.berlin/go/smallwebwaf/internal/lookup/lookuptest" "sneak.berlin/go/smallwebwaf/internal/servewaf" @@ -123,21 +125,30 @@ func (o *output) text() string { return o.buf.String() } +// viperFrom returns a viper Set with each of settings, as the viper +// config.NewViper returns one bound to a process's environment. +func viperFrom(settings map[string]string) *viper.Viper { + v := viper.New() + for name, value := range settings { + v.Set(name, value) + } + + return v +} + // run runs `smallwebwaf servewaf` with the settings in env until ctx is done, // and returns its exit status. SWWAF_LOOKUP_SOURCE is off unless env sets it, // so that no test sends GeoJS its clients' addresses. func run(ctx context.Context, env map[string]string, out *output) int { + v := viperFrom(env) + if _, set := env["SWWAF_LOOKUP_SOURCE"]; !set { + v.Set("SWWAF_LOOKUP_SOURCE", "off") + } + return smallwebwaf.RunServeWAF(ctx, smallwebwaf.Params{ Version: testVersion, - LookupEnv: func(name string) (string, bool) { - value, ok := env[name] - if !ok && name == "SWWAF_LOOKUP_SOURCE" { - return "off", true - } - - return value, ok - }, - Stdout: out, + Viper: v, + Stdout: out, }) }