Compare commits
2 Commits
1230ae990e
...
a5bc29f433
| Author | SHA1 | Date | |
|---|---|---|---|
| a5bc29f433 | |||
| b9f7db6901 |
71
README.md
71
README.md
@@ -71,8 +71,18 @@ make clean # Remove bin/
|
|||||||
### Configuration
|
### Configuration
|
||||||
|
|
||||||
All configuration is via environment variables. For local development,
|
All configuration is via environment variables. For local development,
|
||||||
you can place variables in a `.env` file in the project root (loaded
|
you can place variables in a `.env` file in the process working
|
||||||
automatically via `godotenv/autoload`).
|
directory, read once at startup before anything else looks at the
|
||||||
|
environment.
|
||||||
|
|
||||||
|
The file is optional and having none is the normal case for a
|
||||||
|
deployment. A file that is there but cannot be parsed aborts startup
|
||||||
|
with a message naming it, because a single malformed line makes none
|
||||||
|
of the file apply: every variable in it silently reverts to its
|
||||||
|
default, which is exactly the failure [Invalid values abort
|
||||||
|
startup](#invalid-values-abort-startup) exists to prevent, for all of
|
||||||
|
them at once. A variable already present in the real environment wins
|
||||||
|
over the file's value for the same name.
|
||||||
|
|
||||||
The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
|
The environment is selected by setting `WEBHOOKER_ENVIRONMENT` to `dev`
|
||||||
or `prod` (default: `dev`). The setting controls exactly one behavior:
|
or `prod` (default: `dev`). The setting controls exactly one behavior:
|
||||||
@@ -125,7 +135,7 @@ TTY detection, and security headers are always applied.
|
|||||||
| `MAINTENANCE_MODE` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
|
| `MAINTENANCE_MODE` | Report `maintenanceMode: true` in the healthcheck JSON. It does not change how any request is served — no maintenance page exists | `false` |
|
||||||
| `METRICS_USERNAME` | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""` |
|
| `METRICS_USERNAME` | Basic auth username for `/metrics`. Must be set together with `METRICS_PASSWORD`; one without the other fails startup | `""` |
|
||||||
| `METRICS_PASSWORD` | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""` |
|
| `METRICS_PASSWORD` | Basic auth password for `/metrics`. Must be set together with `METRICS_USERNAME`; one without the other fails startup | `""` |
|
||||||
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
|
| `SENTRY_DSN` | Sentry error reporting DSN. Unset leaves error reporting off; a value the Sentry SDK cannot parse fails startup rather than serving with reporting silently off | `""` |
|
||||||
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
|
| `RETENTION_SWEEP_INTERVAL` | How often the retention reaper and archive sweeper run (Go duration, must be positive) | `1h` |
|
||||||
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
| `SESSION_IDLE_TIMEOUT` | Idle session timeout (Go duration) | `24h` |
|
||||||
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
|
| `RECEIVER_RATE_LIMIT` | Receiver requests/minute per IP per entrypoint (10x that per IP across the route) | `120` |
|
||||||
@@ -464,10 +474,20 @@ startup), every entry in `TRUSTED_PROXIES` and
|
|||||||
`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address, and
|
`ALLOWED_EGRESS_CIDRS` must be a CIDR block or a bare IP address, and
|
||||||
`BIND_ADDRESS` must be an IP address literal — `localhost`,
|
`BIND_ADDRESS` must be an IP address literal — `localhost`,
|
||||||
`127.0.0.1:8080` and `10.0.0.0/8` are each rejected rather than
|
`127.0.0.1:8080` and `10.0.0.0/8` are each rejected rather than
|
||||||
resolved, split, or narrowed to something they do not say.
|
resolved, split, or narrowed to something they do not say — and
|
||||||
|
`SENTRY_DSN` must parse as a Sentry DSN.
|
||||||
`SESSION_IDLE_TIMEOUT` is the exception: a
|
`SESSION_IDLE_TIMEOUT` is the exception: a
|
||||||
non-positive value there means idle expiry is disabled, not invalid.
|
non-positive value there means idle expiry is disabled, not invalid.
|
||||||
|
|
||||||
|
`SENTRY_DSN` is checked with the Sentry SDK's own parser, the same call
|
||||||
|
the SDK makes on the DSN it is later handed, so what configuration
|
||||||
|
accepts is exactly what will initialise. A typo in it is the one
|
||||||
|
configuration mistake nothing downstream can ever notice — the variable
|
||||||
|
is still set, so every later signal reports error reporting as on while
|
||||||
|
no report is being sent — which is why it aborts rather than starting
|
||||||
|
with reporting off. Leaving it unset is not a mistake and not affected:
|
||||||
|
error reporting is simply off and startup is normal.
|
||||||
|
|
||||||
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
|
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
|
||||||
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
|
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
|
||||||
`true`, `True`, `0`, `f`, `F`, `FALSE`, `false`, `False` — and nothing
|
`true`, `True`, `0`, `f`, `F`, `FALSE`, `false`, `False` — and nothing
|
||||||
@@ -666,6 +686,44 @@ databases written by `database` targets (`archive-{uuid}.db`). Mount
|
|||||||
this as a persistent volume to preserve data across container
|
this as a persistent volume to preserve data across container
|
||||||
restarts.
|
restarts.
|
||||||
|
|
||||||
|
**The bind-mounted directory must be owned by UID 1000, or the
|
||||||
|
container does not start.** Docker creates a `-v` source path that
|
||||||
|
does not exist yet as `root:root`, and the process runs as UID 1000,
|
||||||
|
so it cannot take its `DATA_DIR` lock:
|
||||||
|
|
||||||
|
```
|
||||||
|
webhooker: locking data directory /var/lib/webhooker: open
|
||||||
|
/var/lib/webhooker/webhooker.lock: permission denied
|
||||||
|
```
|
||||||
|
|
||||||
|
It exits non-zero at that point, before opening any database. Create
|
||||||
|
the directory ahead of the first `docker run`:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
mkdir -p /path/to/data
|
||||||
|
chown 1000:1000 /path/to/data
|
||||||
|
chmod 750 /path/to/data
|
||||||
|
```
|
||||||
|
|
||||||
|
The same `chown` is what a restore needs — see step 4 of
|
||||||
|
[Restore](#restore). A **named volume** does not have this problem:
|
||||||
|
Docker copies the image's ownership onto a volume it initializes, and
|
||||||
|
the image creates `/var/lib/webhooker` owned by `webhooker`.
|
||||||
|
|
||||||
|
**The file modes are not yours to set, and do not depend on the
|
||||||
|
directory.** `webhooker.db` holds target configuration in plaintext —
|
||||||
|
bearer tokens, API keys, Slack webhook URLs — along with the session
|
||||||
|
encryption key, so webhooker creates every SQLite file it owns `0600`:
|
||||||
|
each database and both of its `-wal` and `-shm` sidecars, across all
|
||||||
|
three tiers. Files an earlier build left `0644` are tightened when
|
||||||
|
they are opened. A `DATA_DIR` webhooker creates itself is `0750`, but
|
||||||
|
a bind mount supplies its own directory and Docker's default for one
|
||||||
|
it creates is `0755`; the `0600` files hold there regardless. The
|
||||||
|
`chmod 750` above is defence in depth — it stops other local users
|
||||||
|
listing the directory and learning your webhook UUIDs from the
|
||||||
|
`events-{uuid}.db` filenames — not the barrier protecting the
|
||||||
|
credentials.
|
||||||
|
|
||||||
## Deployment behind a reverse proxy
|
## Deployment behind a reverse proxy
|
||||||
|
|
||||||
webhooker terminates no TLS of its own. It serves plaintext HTTP and
|
webhooker terminates no TLS of its own. It serves plaintext HTTP and
|
||||||
@@ -1622,6 +1680,11 @@ webhooker uses **separate SQLite database files**: a main application
|
|||||||
database for configuration data and per-webhook databases for event
|
database for configuration data and per-webhook databases for event
|
||||||
storage. All database files live in the `DATA_DIR` directory.
|
storage. All database files live in the `DATA_DIR` directory.
|
||||||
|
|
||||||
|
Every one of them is created `0600`, and so is each `-wal` and `-shm`
|
||||||
|
sidecar. See
|
||||||
|
[Running with Docker](#running-with-docker) for what that does and
|
||||||
|
does not protect.
|
||||||
|
|
||||||
**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
|
**Main Application Database** (`{DATA_DIR}/webhooker.db`) — stores
|
||||||
configuration and application state:
|
configuration and application state:
|
||||||
|
|
||||||
|
|||||||
107
cmd/webhooker/dotenv_test.go
Normal file
107
cmd/webhooker/dotenv_test.go
Normal file
@@ -0,0 +1,107 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
import (
|
||||||
|
"bytes"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
)
|
||||||
|
|
||||||
|
// dotEnvKey is a throwaway variable name these tests write and read,
|
||||||
|
// so they cannot disturb real configuration.
|
||||||
|
const dotEnvKey = "WEBHOOKER_TEST_DISPATCH_VALUE"
|
||||||
|
|
||||||
|
// writeDotEnvInWorkingDir puts contents in a .env file in a fresh
|
||||||
|
// temporary directory and moves the process there.
|
||||||
|
//
|
||||||
|
// The callers are deliberately not parallel and must stay that way:
|
||||||
|
// t.Chdir moves the whole process. Go releases parallel tests only
|
||||||
|
// after every sequential test in the package has finished, so nothing
|
||||||
|
// else runs while these do.
|
||||||
|
func writeDotEnvInWorkingDir(t *testing.T, contents string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
require.NoError(t, os.WriteFile(
|
||||||
|
filepath.Join(dir, config.DotEnvPath),
|
||||||
|
[]byte(contents), 0o600,
|
||||||
|
))
|
||||||
|
t.Chdir(dir)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDispatch_MalformedDotEnvRefuses pins the second half of the
|
||||||
|
// defect. godotenv applies nothing at all when a file will not parse,
|
||||||
|
// so one mistyped line used to revert every variable in it to its
|
||||||
|
// default and start the server anyway, with no log line naming the
|
||||||
|
// file. The refusal has to arrive before any subcommand runs, which
|
||||||
|
// is why `help` — the one subcommand that touches nothing — is still
|
||||||
|
// refused here.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // t.Chdir moves the whole process.
|
||||||
|
func TestDispatch_MalformedDotEnvRefuses(t *testing.T) {
|
||||||
|
writeDotEnvInWorkingDir(t, "PORT 19615\n")
|
||||||
|
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
|
||||||
|
code := dispatch(
|
||||||
|
[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
|
||||||
|
)
|
||||||
|
|
||||||
|
require.Equal(t, 1, code, "a broken .env must exit non-zero")
|
||||||
|
assert.Contains(
|
||||||
|
t, stderr.String(), config.DotEnvPath,
|
||||||
|
"the refusal must name the file",
|
||||||
|
)
|
||||||
|
assert.Empty(
|
||||||
|
t, stdout.String(),
|
||||||
|
"the subcommand must not have run",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDispatch_LoadsDotEnvBeforeSubcommands pins the ordering the
|
||||||
|
// godotenv/autoload import used to provide for free. It ran in an
|
||||||
|
// init(), so .env was in the environment before anything read it —
|
||||||
|
// including config.DataDir, which both the DATA_DIR lock and resetpw
|
||||||
|
// call outside the fx graph. Loading any later would let a .env that
|
||||||
|
// sets DATA_DIR lock one directory while the config opened databases
|
||||||
|
// in another.
|
||||||
|
func TestDispatch_LoadsDotEnvBeforeSubcommands(t *testing.T) {
|
||||||
|
t.Setenv(dotEnvKey, "placeholder")
|
||||||
|
require.NoError(t, os.Unsetenv(dotEnvKey))
|
||||||
|
|
||||||
|
writeDotEnvInWorkingDir(t, dotEnvKey+"=from-dot-env\n")
|
||||||
|
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
|
||||||
|
code := dispatch(
|
||||||
|
[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
|
||||||
|
)
|
||||||
|
|
||||||
|
require.Equal(t, 0, code)
|
||||||
|
assert.Equal(
|
||||||
|
t, "from-dot-env", os.Getenv(dotEnvKey),
|
||||||
|
"the file must be applied before the subcommand runs",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestDispatch_MissingDotEnvIsFine pins the case most deployments are
|
||||||
|
// in: no .env at all, which must stay a normal start.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // t.Chdir moves the whole process.
|
||||||
|
func TestDispatch_MissingDotEnvIsFine(t *testing.T) {
|
||||||
|
t.Chdir(t.TempDir())
|
||||||
|
|
||||||
|
var stdout, stderr bytes.Buffer
|
||||||
|
|
||||||
|
code := dispatch(
|
||||||
|
[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
|
||||||
|
)
|
||||||
|
|
||||||
|
require.Equal(t, 0, code)
|
||||||
|
assert.Empty(t, stderr.String())
|
||||||
|
}
|
||||||
@@ -54,6 +54,11 @@ const stopTimeout = 5 * time.Second
|
|||||||
// caller can tell "called wrong" from "declined".
|
// caller can tell "called wrong" from "declined".
|
||||||
const exitUsage = 2
|
const exitUsage = 2
|
||||||
|
|
||||||
|
// helpCommand is the subcommand that prints usage. The flag spellings
|
||||||
|
// beside it in the switch are aliases; this is the name the usage text
|
||||||
|
// documents and the one tests invoke.
|
||||||
|
const helpCommand = "help"
|
||||||
|
|
||||||
// Build-time variables set via -ldflags.
|
// Build-time variables set via -ldflags.
|
||||||
//
|
//
|
||||||
//nolint:gochecknoglobals // Build-time variables injected by the linker.
|
//nolint:gochecknoglobals // Build-time variables injected by the linker.
|
||||||
@@ -75,11 +80,27 @@ func main() {
|
|||||||
// every existing deployment invoke; that path is unchanged, including
|
// every existing deployment invoke; that path is unchanged, including
|
||||||
// where the DATA_DIR lock is taken relative to building the fx graph
|
// where the DATA_DIR lock is taken relative to building the fx graph
|
||||||
// and how fx propagates a non-zero exit itself.
|
// and how fx propagates a non-zero exit itself.
|
||||||
|
//
|
||||||
|
// The optional .env file is read here, before any subcommand and so
|
||||||
|
// before anything reads the environment — config.DataDir, which both
|
||||||
|
// the DATA_DIR lock and resetpw call outside the fx graph, above all.
|
||||||
|
// It used to be read from an init() in internal/config, which put it
|
||||||
|
// earlier still but threw the error away: a single malformed line
|
||||||
|
// applied none of the file and said nothing about it. A file that is
|
||||||
|
// not there stays fine, since .env is optional and most deployments
|
||||||
|
// do not have one.
|
||||||
func dispatch(
|
func dispatch(
|
||||||
args []string,
|
args []string,
|
||||||
stdin io.Reader,
|
stdin io.Reader,
|
||||||
stdout, stderr io.Writer,
|
stdout, stderr io.Writer,
|
||||||
) int {
|
) int {
|
||||||
|
err := config.LoadDotEnv()
|
||||||
|
if err != nil {
|
||||||
|
_, _ = fmt.Fprintf(stderr, "%s: %v\n", appname, err)
|
||||||
|
|
||||||
|
return 1
|
||||||
|
}
|
||||||
|
|
||||||
if len(args) == 0 {
|
if len(args) == 0 {
|
||||||
return run(stderr)
|
return run(stderr)
|
||||||
}
|
}
|
||||||
@@ -87,7 +108,7 @@ func dispatch(
|
|||||||
switch args[0] {
|
switch args[0] {
|
||||||
case resetpw.Name:
|
case resetpw.Name:
|
||||||
return resetpw.Run(args[1:], stdin, stdout, stderr)
|
return resetpw.Run(args[1:], stdin, stdout, stderr)
|
||||||
case "help", "-h", "-help", "--help":
|
case helpCommand, "-h", "-help", "--help":
|
||||||
usage(stdout)
|
usage(stdout)
|
||||||
|
|
||||||
return 0
|
return 0
|
||||||
|
|||||||
@@ -121,7 +121,7 @@ func TestDispatch_Help(t *testing.T) {
|
|||||||
var stdout, stderr bytes.Buffer
|
var stdout, stderr bytes.Buffer
|
||||||
|
|
||||||
code := dispatch(
|
code := dispatch(
|
||||||
[]string{"help"}, strings.NewReader(""), &stdout, &stderr,
|
[]string{helpCommand}, strings.NewReader(""), &stdout, &stderr,
|
||||||
)
|
)
|
||||||
|
|
||||||
require.Equal(t, 0, code)
|
require.Equal(t, 0, code)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ package config
|
|||||||
import (
|
import (
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io/fs"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
"net/netip"
|
"net/netip"
|
||||||
"os"
|
"os"
|
||||||
@@ -11,13 +12,11 @@ import (
|
|||||||
"strings"
|
"strings"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/getsentry/sentry-go"
|
||||||
|
"github.com/joho/godotenv"
|
||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"sneak.berlin/go/webhooker/internal/globals"
|
"sneak.berlin/go/webhooker/internal/globals"
|
||||||
"sneak.berlin/go/webhooker/internal/logger"
|
"sneak.berlin/go/webhooker/internal/logger"
|
||||||
|
|
||||||
// Populates the environment from a ./.env file automatically for
|
|
||||||
// development configuration. Kept in one place only (here).
|
|
||||||
_ "github.com/joho/godotenv/autoload"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@@ -84,6 +83,12 @@ const (
|
|||||||
// IPv6 prefix spends on the ::ffff:0:0/96 wrapper, so a /104
|
// IPv6 prefix spends on the ::ffff:0:0/96 wrapper, so a /104
|
||||||
// covers the same addresses as an IPv4 /8.
|
// covers the same addresses as an IPv4 /8.
|
||||||
mappedV4Offset = 96
|
mappedV4Offset = 96
|
||||||
|
|
||||||
|
// DotEnvPath is the optional file of KEY=value lines read into the
|
||||||
|
// environment at startup, relative to the process working
|
||||||
|
// directory. Exported so that documentation and tests name the
|
||||||
|
// same path the loader opens.
|
||||||
|
DotEnvPath = ".env"
|
||||||
)
|
)
|
||||||
|
|
||||||
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
|
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
|
||||||
@@ -107,6 +112,15 @@ var ErrInvalidCIDR = errors.New("invalid CIDR")
|
|||||||
// something that is not an IP address literal.
|
// something that is not an IP address literal.
|
||||||
var ErrInvalidBindAddress = errors.New("invalid bind address")
|
var ErrInvalidBindAddress = errors.New("invalid bind address")
|
||||||
|
|
||||||
|
// ErrInvalidSentryDSN is returned when SENTRY_DSN is set to something
|
||||||
|
// the Sentry SDK cannot parse as a DSN.
|
||||||
|
var ErrInvalidSentryDSN = errors.New("invalid Sentry DSN")
|
||||||
|
|
||||||
|
// ErrDotEnvUnreadable is returned when the optional .env file exists
|
||||||
|
// but cannot be read or parsed. A file that is not there is not an
|
||||||
|
// error; a file that is there and broken is.
|
||||||
|
var ErrDotEnvUnreadable = errors.New("unreadable .env file")
|
||||||
|
|
||||||
// ErrIncompleteMetricsAuth is returned when exactly one of
|
// ErrIncompleteMetricsAuth is returned when exactly one of
|
||||||
// METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
|
// METRICS_USERNAME and METRICS_PASSWORD carries a value. Neither
|
||||||
// fallback is acceptable: serving /metrics on the username alone
|
// fallback is acceptable: serving /metrics on the username alone
|
||||||
@@ -212,12 +226,62 @@ func (c *Config) MetricsAuthEnabled() bool {
|
|||||||
return c.MetricsUsername != "" && c.MetricsPassword != ""
|
return c.MetricsUsername != "" && c.MetricsPassword != ""
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SentryEnabled reports whether error reporting is shipped to Sentry.
|
||||||
|
// It is the only answer to that question in the codebase: the SDK
|
||||||
|
// initialisation, the sentryhttp middleware registration and the
|
||||||
|
// startup log's sentryEnabled field all read this one method, so the
|
||||||
|
// log cannot report reporting as on while nothing is sending.
|
||||||
|
//
|
||||||
|
// A non-empty DSN is enough because loadFromEnv already parsed it with
|
||||||
|
// the SDK's own parser and refused to build a Config around one the
|
||||||
|
// SDK would reject, and because initialising the SDK with a DSN that
|
||||||
|
// parsed and failed anyway aborts the process rather than leaving this
|
||||||
|
// true and the client absent.
|
||||||
|
func (c *Config) SentryEnabled() bool {
|
||||||
|
return c.SentryDSN != ""
|
||||||
|
}
|
||||||
|
|
||||||
// envString returns the value of the named environment variable,
|
// envString returns the value of the named environment variable,
|
||||||
// or an empty string if not set.
|
// or an empty string if not set.
|
||||||
func envString(key string) string {
|
func envString(key string) string {
|
||||||
return os.Getenv(key)
|
return os.Getenv(key)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// LoadDotEnv reads DotEnvPath into the environment when that file is
|
||||||
|
// present, and reports a file that is present but broken.
|
||||||
|
//
|
||||||
|
// It has to run before anything reads the environment, so that every
|
||||||
|
// reader agrees on what the environment holds — the DATA_DIR lock
|
||||||
|
// taken before the fx graph exists as much as loadFromEnv itself. A
|
||||||
|
// variable already set in the real environment wins: godotenv never
|
||||||
|
// overwrites one.
|
||||||
|
//
|
||||||
|
// A missing file is not an error. It is a development convenience and
|
||||||
|
// most deployments set the environment directly.
|
||||||
|
//
|
||||||
|
// Any other failure is. godotenv parses the whole file before setting
|
||||||
|
// anything, so a single malformed line applies none of it: every
|
||||||
|
// variable in the file silently reverts to its default, which defeats
|
||||||
|
// the fail-loud guarantee for all of them at once.
|
||||||
|
func LoadDotEnv() error {
|
||||||
|
return loadDotEnvFile(DotEnvPath)
|
||||||
|
}
|
||||||
|
|
||||||
|
// loadDotEnvFile is LoadDotEnv over a named file, so tests can point
|
||||||
|
// at a temporary one instead of the process working directory.
|
||||||
|
func loadDotEnvFile(path string) error {
|
||||||
|
err := godotenv.Load(path)
|
||||||
|
if err == nil || errors.Is(err, fs.ErrNotExist) {
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
return fmt.Errorf(
|
||||||
|
"%w: %s: %w; nothing in it was applied, so fix the file or "+
|
||||||
|
"remove it",
|
||||||
|
ErrDotEnvUnreadable, path, err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
// DataDir resolves DATA_DIR, applying DefaultDataDir when it is unset
|
// DataDir resolves DATA_DIR, applying DefaultDataDir when it is unset
|
||||||
// or empty. It is exported so that entry points which must act on the
|
// or empty. It is exported so that entry points which must act on the
|
||||||
// data directory before the fx graph exists — taking the exclusive
|
// data directory before the fx graph exists — taking the exclusive
|
||||||
@@ -462,6 +526,41 @@ func envBindAddress(key, defaultValue string) (string, error) {
|
|||||||
return addr.String(), nil
|
return addr.String(), nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// envSentryDSN returns the value of the named environment variable
|
||||||
|
// checked as a Sentry DSN. An unset (or empty, or whitespace-only)
|
||||||
|
// value yields "", which means error reporting stays off — the common
|
||||||
|
// case, and a normal start.
|
||||||
|
//
|
||||||
|
// A set value is parsed with sentry.NewDsn, which is the call
|
||||||
|
// sentry.Init makes on the DSN it is handed, so what passes here is
|
||||||
|
// exactly what the SDK will accept later and the two cannot disagree.
|
||||||
|
// Reproducing the check by hand instead would cost this package its
|
||||||
|
// dependency on the SDK — already a module dependency, already linked
|
||||||
|
// into the binary — in exchange for a second definition of "valid DSN"
|
||||||
|
// free to drift from the one that decides.
|
||||||
|
//
|
||||||
|
// A set value that does not parse is a hard error naming the key, so
|
||||||
|
// startup fails loudly. Losing error reporting is the failure this
|
||||||
|
// variable exists to prevent, and a typo in a DSN is silent forever:
|
||||||
|
// nothing later in the process can notice that reports are going
|
||||||
|
// nowhere. The bad value is quoted because it is a URL to a public
|
||||||
|
// endpoint carrying a public key, not a secret.
|
||||||
|
func envSentryDSN(key string) (string, error) {
|
||||||
|
v := strings.TrimSpace(os.Getenv(key))
|
||||||
|
if v == "" {
|
||||||
|
return "", nil
|
||||||
|
}
|
||||||
|
|
||||||
|
_, err := sentry.NewDsn(v)
|
||||||
|
if err != nil {
|
||||||
|
return "", fmt.Errorf(
|
||||||
|
"%w: %s: %q: %w", ErrInvalidSentryDSN, key, v, err,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
return v, nil
|
||||||
|
}
|
||||||
|
|
||||||
// resolveMetricsAuth reads the /metrics basic-auth credentials and
|
// resolveMetricsAuth reads the /metrics basic-auth credentials and
|
||||||
// rejects a half-set pair, naming both variables either way. The
|
// rejects a half-set pair, naming both variables either way. The
|
||||||
// error carries neither value: the password is a secret.
|
// error carries neither value: the password is a secret.
|
||||||
@@ -594,6 +693,11 @@ func loadFromEnv() (*Config, error) {
|
|||||||
return nil, err
|
return nil, err
|
||||||
}
|
}
|
||||||
|
|
||||||
|
sentryDSN, err := envSentryDSN("SENTRY_DSN")
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
return &Config{
|
return &Config{
|
||||||
DataDir: DataDir(),
|
DataDir: DataDir(),
|
||||||
Debug: debug,
|
Debug: debug,
|
||||||
@@ -603,7 +707,7 @@ func loadFromEnv() (*Config, error) {
|
|||||||
MetricsPassword: metricsPassword,
|
MetricsPassword: metricsPassword,
|
||||||
Port: port,
|
Port: port,
|
||||||
BindAddress: bindAddress,
|
BindAddress: bindAddress,
|
||||||
SentryDSN: envString("SENTRY_DSN"),
|
SentryDSN: sentryDSN,
|
||||||
RetentionSweepInterval: retentionSweepInterval,
|
RetentionSweepInterval: retentionSweepInterval,
|
||||||
SessionIdleTimeout: sessionIdleTimeout,
|
SessionIdleTimeout: sessionIdleTimeout,
|
||||||
ReceiverRateLimit: receiverRateLimit,
|
ReceiverRateLimit: receiverRateLimit,
|
||||||
@@ -738,7 +842,7 @@ func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
|
|||||||
"receiverRateLimit", s.ReceiverRateLimit,
|
"receiverRateLimit", s.ReceiverRateLimit,
|
||||||
"trustedProxies", len(s.TrustedProxies),
|
"trustedProxies", len(s.TrustedProxies),
|
||||||
"allowedEgressCIDRs", len(s.AllowedEgressCIDRs),
|
"allowedEgressCIDRs", len(s.AllowedEgressCIDRs),
|
||||||
"hasSentryDSN", s.SentryDSN != "",
|
"sentryEnabled", s.SentryEnabled(),
|
||||||
"hasMetricsAuth", s.MetricsAuthEnabled(),
|
"hasMetricsAuth", s.MetricsAuthEnabled(),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|||||||
158
internal/config/dotenv_test.go
Normal file
158
internal/config/dotenv_test.go
Normal file
@@ -0,0 +1,158 @@
|
|||||||
|
package config_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
)
|
||||||
|
|
||||||
|
// dotEnvKey is a throwaway variable name the .env tests write and
|
||||||
|
// read, so they cannot disturb real configuration.
|
||||||
|
const dotEnvKey = "WEBHOOKER_TEST_DOTENV_VALUE"
|
||||||
|
|
||||||
|
// malformedDotEnv is a file godotenv cannot parse. The first line is
|
||||||
|
// the realistic typo — a space where the `=` belongs — and the rest
|
||||||
|
// make sure nothing downstream treats the file as salvageable line by
|
||||||
|
// line.
|
||||||
|
const malformedDotEnv = "PORT 19615\n" +
|
||||||
|
"this is not = valid ! syntax\n" +
|
||||||
|
"\"unclosed\n"
|
||||||
|
|
||||||
|
// unsetDotEnvKey makes dotEnvKey genuinely absent for the duration of
|
||||||
|
// the test and restores it afterwards. t.Setenv registers the restore;
|
||||||
|
// the Unsetenv that follows is what the test actually needs, because a
|
||||||
|
// variable set to the empty string is still present in os.Environ and
|
||||||
|
// godotenv would refuse to overwrite it.
|
||||||
|
func unsetDotEnvKey(t *testing.T) {
|
||||||
|
t.Helper()
|
||||||
|
t.Setenv(dotEnvKey, "placeholder")
|
||||||
|
require.NoError(t, os.Unsetenv(dotEnvKey))
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeDotEnv writes contents to a .env file in a fresh temporary
|
||||||
|
// directory and returns its path.
|
||||||
|
func writeDotEnv(t *testing.T, contents string) string {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
path := filepath.Join(t.TempDir(), config.DotEnvPath)
|
||||||
|
require.NoError(t, os.WriteFile(path, []byte(contents), 0o600))
|
||||||
|
|
||||||
|
return path
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_MissingFileIsFine pins the case most deployments are
|
||||||
|
// in. The file is optional: it is a development convenience, and a
|
||||||
|
// deployment that configures the environment directly must start
|
||||||
|
// normally rather than be refused for a file it was never meant to
|
||||||
|
// have.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
|
||||||
|
func TestLoadDotEnv_MissingFileIsFine(t *testing.T) {
|
||||||
|
unsetDotEnvKey(t)
|
||||||
|
|
||||||
|
absent := filepath.Join(t.TempDir(), config.DotEnvPath)
|
||||||
|
require.NoError(t, config.LoadDotEnvFileForTest(absent))
|
||||||
|
|
||||||
|
_, present := os.LookupEnv(dotEnvKey)
|
||||||
|
assert.False(t, present, "nothing may be set from an absent file")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_AppliesValues pins that a well-formed file still
|
||||||
|
// reaches the environment, which is the whole reason the file is read
|
||||||
|
// at all.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
|
||||||
|
func TestLoadDotEnv_AppliesValues(t *testing.T) {
|
||||||
|
unsetDotEnvKey(t)
|
||||||
|
|
||||||
|
path := writeDotEnv(t, "# a comment\n"+dotEnvKey+"=from-dot-env\n")
|
||||||
|
|
||||||
|
require.NoError(t, config.LoadDotEnvFileForTest(path))
|
||||||
|
assert.Equal(t, "from-dot-env", os.Getenv(dotEnvKey))
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_RealEnvironmentWins pins that the file cannot
|
||||||
|
// override a variable the process was actually started with. A
|
||||||
|
// deployment that sets DATA_DIR in its unit file must not have it
|
||||||
|
// silently replaced by a stale .env left in the working directory.
|
||||||
|
func TestLoadDotEnv_RealEnvironmentWins(t *testing.T) {
|
||||||
|
t.Setenv(dotEnvKey, "from-environment")
|
||||||
|
|
||||||
|
path := writeDotEnv(t, dotEnvKey+"=from-dot-env\n")
|
||||||
|
|
||||||
|
require.NoError(t, config.LoadDotEnvFileForTest(path))
|
||||||
|
assert.Equal(t, "from-environment", os.Getenv(dotEnvKey))
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_MalformedFileAborts is the defect this fixes. One bad
|
||||||
|
// line makes godotenv apply none of the file, so every variable in it
|
||||||
|
// reverts to its default; the process used to start that way with no
|
||||||
|
// log line naming the file at all.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // unsetDotEnvKey uses t.Setenv.
|
||||||
|
func TestLoadDotEnv_MalformedFileAborts(t *testing.T) {
|
||||||
|
unsetDotEnvKey(t)
|
||||||
|
|
||||||
|
path := writeDotEnv(
|
||||||
|
t, malformedDotEnv+dotEnvKey+"=from-dot-env\n",
|
||||||
|
)
|
||||||
|
|
||||||
|
err := config.LoadDotEnvFileForTest(path)
|
||||||
|
|
||||||
|
require.Error(t, err)
|
||||||
|
require.ErrorIs(t, err, config.ErrDotEnvUnreadable)
|
||||||
|
assert.Contains(
|
||||||
|
t, err.Error(), config.DotEnvPath,
|
||||||
|
"the failure must name the file it could not read",
|
||||||
|
)
|
||||||
|
|
||||||
|
_, present := os.LookupEnv(dotEnvKey)
|
||||||
|
assert.False(
|
||||||
|
t, present,
|
||||||
|
"a rejected file must apply nothing, not part of itself",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_UnreadableFileAborts pins that only absence is
|
||||||
|
// tolerated. A .env that exists but cannot be read is a file the
|
||||||
|
// operator meant to be applied, so it fails like a malformed one
|
||||||
|
// rather than being treated as though it were not there.
|
||||||
|
func TestLoadDotEnv_UnreadableFileAborts(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
// A directory in the file's place: open succeeds and the read
|
||||||
|
// fails, which no umask or root-ness can turn back into success
|
||||||
|
// the way a chmod could.
|
||||||
|
path := filepath.Join(t.TempDir(), config.DotEnvPath)
|
||||||
|
require.NoError(t, os.Mkdir(path, 0o750))
|
||||||
|
|
||||||
|
err := config.LoadDotEnvFileForTest(path)
|
||||||
|
|
||||||
|
require.Error(t, err)
|
||||||
|
require.ErrorIs(t, err, config.ErrDotEnvUnreadable)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestLoadDotEnv_ReadsTheWorkingDirectory pins the path LoadDotEnv
|
||||||
|
// itself opens, which the tests above bypass. It is relative to the
|
||||||
|
// process working directory, as it was under godotenv/autoload and as
|
||||||
|
// the README documents.
|
||||||
|
//
|
||||||
|
//nolint:paralleltest // t.Chdir moves the whole process.
|
||||||
|
func TestLoadDotEnv_ReadsTheWorkingDirectory(t *testing.T) {
|
||||||
|
unsetDotEnvKey(t)
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
require.NoError(t, os.WriteFile(
|
||||||
|
filepath.Join(dir, config.DotEnvPath),
|
||||||
|
[]byte(dotEnvKey+"=from-working-directory\n"),
|
||||||
|
0o600,
|
||||||
|
))
|
||||||
|
t.Chdir(dir)
|
||||||
|
|
||||||
|
require.NoError(t, config.LoadDotEnv())
|
||||||
|
assert.Equal(t, "from-working-directory", os.Getenv(dotEnvKey))
|
||||||
|
}
|
||||||
@@ -518,8 +518,20 @@ type badEnvValueCase struct {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// badEnvValueCases is the config.New table, kept out of the test body
|
// badEnvValueCases is the config.New table, kept out of the test body
|
||||||
// so the test itself stays readable.
|
// so the test itself stays readable. It is assembled from per-variable
|
||||||
|
// groups because one literal covering every variable outgrew the
|
||||||
|
// function-length budget.
|
||||||
func badEnvValueCases() []badEnvValueCase {
|
func badEnvValueCases() []badEnvValueCase {
|
||||||
|
cases := listenerEnvValueCases()
|
||||||
|
cases = append(cases, flagEnvValueCases()...)
|
||||||
|
cases = append(cases, sentryEnvValueCases()...)
|
||||||
|
|
||||||
|
return cases
|
||||||
|
}
|
||||||
|
|
||||||
|
// listenerEnvValueCases covers the two variables that describe the
|
||||||
|
// HTTP listener.
|
||||||
|
func listenerEnvValueCases() []badEnvValueCase {
|
||||||
return []badEnvValueCase{
|
return []badEnvValueCase{
|
||||||
{
|
{
|
||||||
name: "valid PORT is used",
|
name: "valid PORT is used",
|
||||||
@@ -542,27 +554,6 @@ func badEnvValueCases() []badEnvValueCase {
|
|||||||
value: "70000",
|
value: "70000",
|
||||||
expectError: true,
|
expectError: true,
|
||||||
},
|
},
|
||||||
{
|
|
||||||
name: "valid DEBUG is used",
|
|
||||||
key: envKeyDebug,
|
|
||||||
value: "true",
|
|
||||||
check: func(t *testing.T, cfg *config.Config) {
|
|
||||||
t.Helper()
|
|
||||||
assert.True(t, cfg.Debug)
|
|
||||||
},
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "unparseable DEBUG aborts startup",
|
|
||||||
key: envKeyDebug,
|
|
||||||
value: "ture",
|
|
||||||
expectError: true,
|
|
||||||
},
|
|
||||||
{
|
|
||||||
name: "unparseable MAINTENANCE_MODE aborts startup",
|
|
||||||
key: envKeyMaintenanceMode,
|
|
||||||
value: "sometimes",
|
|
||||||
expectError: true,
|
|
||||||
},
|
|
||||||
{
|
{
|
||||||
name: "valid BIND_ADDRESS is used",
|
name: "valid BIND_ADDRESS is used",
|
||||||
key: envKeyBindAddress,
|
key: envKeyBindAddress,
|
||||||
@@ -595,6 +586,69 @@ func badEnvValueCases() []badEnvValueCase {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// flagEnvValueCases covers the boolean variables.
|
||||||
|
func flagEnvValueCases() []badEnvValueCase {
|
||||||
|
return []badEnvValueCase{
|
||||||
|
{
|
||||||
|
name: "valid DEBUG is used",
|
||||||
|
key: envKeyDebug,
|
||||||
|
value: "true",
|
||||||
|
check: func(t *testing.T, cfg *config.Config) {
|
||||||
|
t.Helper()
|
||||||
|
assert.True(t, cfg.Debug)
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "unparseable DEBUG aborts startup",
|
||||||
|
key: envKeyDebug,
|
||||||
|
value: "ture",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "unparseable MAINTENANCE_MODE aborts startup",
|
||||||
|
key: envKeyMaintenanceMode,
|
||||||
|
value: "sometimes",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// sentryEnvValueCases covers SENTRY_DSN. The three rejected values are
|
||||||
|
// the ones measured on the defect: each initialised the SDK with an
|
||||||
|
// error and left the process serving with error reporting off.
|
||||||
|
func sentryEnvValueCases() []badEnvValueCase {
|
||||||
|
return []badEnvValueCase{
|
||||||
|
{
|
||||||
|
name: "valid SENTRY_DSN is used",
|
||||||
|
key: envKeySentryDSN,
|
||||||
|
value: validSentryDSN,
|
||||||
|
check: func(t *testing.T, cfg *config.Config) {
|
||||||
|
t.Helper()
|
||||||
|
assert.Equal(t, validSentryDSN, cfg.SentryDSN)
|
||||||
|
assert.True(t, cfg.SentryEnabled())
|
||||||
|
},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "unparseable SENTRY_DSN aborts startup",
|
||||||
|
key: envKeySentryDSN,
|
||||||
|
value: "not-a-dsn",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "SENTRY_DSN that is not a URL aborts startup",
|
||||||
|
key: envKeySentryDSN,
|
||||||
|
value: "%%%",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "keyless SENTRY_DSN aborts startup",
|
||||||
|
key: envKeySentryDSN,
|
||||||
|
value: "https://example.invalid/1",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
|
// TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
|
||||||
// break the legitimate unset case: absent variables still get their
|
// break the legitimate unset case: absent variables still get their
|
||||||
// documented defaults.
|
// documented defaults.
|
||||||
@@ -603,7 +657,7 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
|
|||||||
|
|
||||||
for _, key := range []string{
|
for _, key := range []string{
|
||||||
envKeyPort, envKeyDebug, envKeyMaintenanceMode,
|
envKeyPort, envKeyDebug, envKeyMaintenanceMode,
|
||||||
envKeyBindAddress,
|
envKeyBindAddress, envKeySentryDSN,
|
||||||
} {
|
} {
|
||||||
require.NoError(t, os.Unsetenv(key))
|
require.NoError(t, os.Unsetenv(key))
|
||||||
}
|
}
|
||||||
@@ -626,4 +680,9 @@ func TestNewUsesDefaultsWhenUnset(t *testing.T) {
|
|||||||
t, config.DefaultBindAddressForTest, cfg.BindAddress,
|
t, config.DefaultBindAddressForTest, cfg.BindAddress,
|
||||||
)
|
)
|
||||||
assert.Equal(t, bindAddressDefault, cfg.BindAddress)
|
assert.Equal(t, bindAddressDefault, cfg.BindAddress)
|
||||||
|
|
||||||
|
// An absent SENTRY_DSN is the common case and must stay a normal
|
||||||
|
// start with error reporting off, not a refusal.
|
||||||
|
assert.Empty(t, cfg.SentryDSN)
|
||||||
|
assert.False(t, cfg.SentryEnabled())
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -51,6 +51,18 @@ func EnvPortForTest(key string, defaultValue int) (int, error) {
|
|||||||
return envPort(key, defaultValue)
|
return envPort(key, defaultValue)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// EnvSentryDSNForTest exposes envSentryDSN.
|
||||||
|
func EnvSentryDSNForTest(key string) (string, error) {
|
||||||
|
return envSentryDSN(key)
|
||||||
|
}
|
||||||
|
|
||||||
|
// LoadDotEnvFileForTest exposes the loader LoadDotEnv runs, over a
|
||||||
|
// caller-named file rather than the process working directory, so
|
||||||
|
// each .env state can be covered without moving the test process.
|
||||||
|
func LoadDotEnvFileForTest(path string) error {
|
||||||
|
return loadDotEnvFile(path)
|
||||||
|
}
|
||||||
|
|
||||||
// EnvBindAddressForTest exposes envBindAddress.
|
// EnvBindAddressForTest exposes envBindAddress.
|
||||||
func EnvBindAddressForTest(key, defaultValue string) (string, error) {
|
func EnvBindAddressForTest(key, defaultValue string) (string, error) {
|
||||||
return envBindAddress(key, defaultValue)
|
return envBindAddress(key, defaultValue)
|
||||||
|
|||||||
141
internal/config/sentry_test.go
Normal file
141
internal/config/sentry_test.go
Normal file
@@ -0,0 +1,141 @@
|
|||||||
|
package config_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"os"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
)
|
||||||
|
|
||||||
|
// envKeySentryDSN is the variable envSentryDSN reads in production.
|
||||||
|
const envKeySentryDSN = "SENTRY_DSN"
|
||||||
|
|
||||||
|
// validSentryDSN is a syntactically complete DSN. The host is under
|
||||||
|
// .invalid (RFC 2606), so nothing a test builds around it can reach a
|
||||||
|
// real Sentry installation.
|
||||||
|
const validSentryDSN = "https://abc123@sentry.invalid/42"
|
||||||
|
|
||||||
|
// envSentryDSNCase is one row of the envSentryDSN table.
|
||||||
|
type envSentryDSNCase struct {
|
||||||
|
name string
|
||||||
|
set bool
|
||||||
|
value string
|
||||||
|
expectError bool
|
||||||
|
expected string
|
||||||
|
}
|
||||||
|
|
||||||
|
// envSentryDSNCases is the envSentryDSN table. The three invalid
|
||||||
|
// values are the ones measured on the defect: each initialised the SDK
|
||||||
|
// with an error and left the process serving with reporting off.
|
||||||
|
func envSentryDSNCases() []envSentryDSNCase {
|
||||||
|
return []envSentryDSNCase{
|
||||||
|
{
|
||||||
|
name: "unset means reporting off",
|
||||||
|
expected: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty means reporting off",
|
||||||
|
set: true,
|
||||||
|
value: "",
|
||||||
|
expected: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "whitespace means reporting off",
|
||||||
|
set: true,
|
||||||
|
value: " ",
|
||||||
|
expected: "",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a valid DSN is kept",
|
||||||
|
set: true,
|
||||||
|
value: validSentryDSN,
|
||||||
|
expected: validSentryDSN,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "surrounding whitespace is trimmed",
|
||||||
|
set: true,
|
||||||
|
value: " " + validSentryDSN + "\t",
|
||||||
|
expected: validSentryDSN,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a value that is not a URL is rejected",
|
||||||
|
set: true,
|
||||||
|
value: "not-a-dsn",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "an unparseable URL is rejected",
|
||||||
|
set: true,
|
||||||
|
value: "%%%",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a DSN without a public key is rejected",
|
||||||
|
set: true,
|
||||||
|
value: "https://example.invalid/1",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a DSN without a project id is rejected",
|
||||||
|
set: true,
|
||||||
|
value: "https://abc123@sentry.invalid/",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "a non-HTTP scheme is rejected",
|
||||||
|
set: true,
|
||||||
|
value: "ftp://abc123@sentry.invalid/42",
|
||||||
|
expectError: true,
|
||||||
|
},
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEnvSentryDSN covers the helper directly. What it pins beyond the
|
||||||
|
// value is the failure shape: a set-but-unparseable DSN names the
|
||||||
|
// variable and the value, exactly as the other fail-loud helpers do,
|
||||||
|
// so an operator reads the fix off the message.
|
||||||
|
func TestEnvSentryDSN(t *testing.T) {
|
||||||
|
for _, tt := range envSentryDSNCases() {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
// Cannot use t.Parallel() here because t.Setenv
|
||||||
|
// is incompatible with parallel subtests.
|
||||||
|
if tt.set {
|
||||||
|
t.Setenv(envKeySentryDSN, tt.value)
|
||||||
|
} else {
|
||||||
|
require.NoError(t, os.Unsetenv(envKeySentryDSN))
|
||||||
|
}
|
||||||
|
|
||||||
|
got, err := config.EnvSentryDSNForTest(envKeySentryDSN)
|
||||||
|
|
||||||
|
if tt.expectError {
|
||||||
|
require.Error(t, err)
|
||||||
|
require.ErrorIs(t, err, config.ErrInvalidSentryDSN)
|
||||||
|
assert.Contains(t, err.Error(), envKeySentryDSN)
|
||||||
|
assert.Contains(t, err.Error(), tt.value)
|
||||||
|
assert.Empty(t, got)
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, tt.expected, got)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestSentryEnabled_TracksTheDSN pins that the one method answering
|
||||||
|
// "is anything being reported" agrees with the DSN in every state. The
|
||||||
|
// startup log, the SDK initialisation and the sentryhttp middleware
|
||||||
|
// all read it, so a log field cannot report reporting as on while
|
||||||
|
// nothing is sending.
|
||||||
|
func TestSentryEnabled_TracksTheDSN(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
assert.False(t, (&config.Config{}).SentryEnabled())
|
||||||
|
assert.True(
|
||||||
|
t,
|
||||||
|
(&config.Config{SentryDSN: validSentryDSN}).SentryEnabled(),
|
||||||
|
)
|
||||||
|
}
|
||||||
240
internal/database/sqlite_mode_test.go
Normal file
240
internal/database/sqlite_mode_test.go
Normal file
@@ -0,0 +1,240 @@
|
|||||||
|
package database_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"io/fs"
|
||||||
|
"net/http"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"go.uber.org/fx/fxtest"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
"sneak.berlin/go/webhooker/internal/database"
|
||||||
|
"sneak.berlin/go/webhooker/internal/globals"
|
||||||
|
"sneak.berlin/go/webhooker/internal/logger"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ownerOnly is the mode every SQLite file the service owns must have.
|
||||||
|
// Spelled out rather than referencing database.SQLiteFilePerm so the
|
||||||
|
// test fails if the constant itself is loosened.
|
||||||
|
const ownerOnly fs.FileMode = 0o600
|
||||||
|
|
||||||
|
// requireOwnerOnly asserts that path exists and is readable and
|
||||||
|
// writable by its owner and by nobody else.
|
||||||
|
func requireOwnerOnly(t *testing.T, path string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
info, err := os.Stat(path)
|
||||||
|
require.NoError(t, err, "%s must exist", path)
|
||||||
|
assert.Equal(
|
||||||
|
t,
|
||||||
|
ownerOnly,
|
||||||
|
info.Mode().Perm(),
|
||||||
|
"%s holds credentials and must not be readable by "+
|
||||||
|
"anyone but its owner",
|
||||||
|
path,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// requireDatabaseSetOwnerOnly asserts the mode of a database file and
|
||||||
|
// of both WAL sidecars. The sidecars carry the same rows as the
|
||||||
|
// database, so tightening only the main file fixes nothing.
|
||||||
|
func requireDatabaseSetOwnerOnly(t *testing.T, dbPath string) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
requireOwnerOnly(t, dbPath)
|
||||||
|
requireOwnerOnly(t, dbPath+"-wal")
|
||||||
|
requireOwnerOnly(t, dbPath+"-shm")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestMainDatabaseFilesAreOwnerOnly covers the tier the defect was
|
||||||
|
// reported against: webhooker.db holds targets.config in plaintext —
|
||||||
|
// bearer tokens, API keys, Slack webhook URLs — and the session
|
||||||
|
// encryption key.
|
||||||
|
func TestMainDatabaseFilesAreOwnerOnly(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
lc := fxtest.NewLifecycle(t)
|
||||||
|
|
||||||
|
l, err := logger.New(lc, logger.LoggerParams{
|
||||||
|
Globals: &globals.Globals{
|
||||||
|
Appname: testAppname,
|
||||||
|
Version: testVersion,
|
||||||
|
},
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
// A directory the application creates itself, not one t.TempDir
|
||||||
|
// made at 0700, so the mode below is the application's.
|
||||||
|
dataDir := filepath.Join(t.TempDir(), "data")
|
||||||
|
|
||||||
|
db, err := database.New(lc, database.DatabaseParams{
|
||||||
|
Config: &config.Config{DataDir: dataDir},
|
||||||
|
Logger: l,
|
||||||
|
})
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
require.NoError(t, lc.Start(ctx))
|
||||||
|
|
||||||
|
defer func() { require.NoError(t, lc.Stop(ctx)) }()
|
||||||
|
|
||||||
|
// Write through the real model so the WAL is populated and both
|
||||||
|
// sidecars are on disk while the handle is open.
|
||||||
|
require.NoError(t, db.DB().Create(&database.Webhook{
|
||||||
|
Name: testWebhookName,
|
||||||
|
}).Error)
|
||||||
|
|
||||||
|
requireDatabaseSetOwnerOnly(
|
||||||
|
t, filepath.Join(dataDir, database.MainDBFileName),
|
||||||
|
)
|
||||||
|
|
||||||
|
// The data directory grants nothing to `other`. Asserted as a
|
||||||
|
// property rather than as an exact 0750, because MkdirAll applies
|
||||||
|
// the ambient umask: the exact mode is the developer's umask as
|
||||||
|
// much as the application's request, and pinning it would make
|
||||||
|
// `make check` pass or fail on where it is run. The group bits are
|
||||||
|
// deliberately left unasserted — deployments may rely on them.
|
||||||
|
info, err := os.Stat(dataDir)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Zero(
|
||||||
|
t,
|
||||||
|
info.Mode().Perm()&0o007,
|
||||||
|
"the data directory must not be world-accessible",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestPerWebhookEventDatabaseFilesAreOwnerOnly covers the events-*.db
|
||||||
|
// tier. These carry no credential canaries since
|
||||||
|
// https://git.eeqj.de/sneak/webhooker/issues/206, but they hold every
|
||||||
|
// received request body and header.
|
||||||
|
func TestPerWebhookEventDatabaseFilesAreOwnerOnly(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
mgr, lc := setupTestWebhookDBManager(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
require.NoError(t, lc.Start(ctx))
|
||||||
|
|
||||||
|
defer func() { require.NoError(t, lc.Stop(ctx)) }()
|
||||||
|
|
||||||
|
webhookID := uuid.New().String()
|
||||||
|
|
||||||
|
db, err := mgr.GetDB(webhookID)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.NoError(t, db.Create(&database.Event{
|
||||||
|
WebhookID: webhookID,
|
||||||
|
EntrypointID: uuid.New().String(),
|
||||||
|
Method: http.MethodPost,
|
||||||
|
Body: "{}",
|
||||||
|
}).Error)
|
||||||
|
|
||||||
|
requireDatabaseSetOwnerOnly(t, mgr.DBPath(webhookID))
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestArchiveDatabaseFilesAreOwnerOnly covers the archive-*.db tier.
|
||||||
|
// internal/delivery builds that path and opens it through OpenSQLite,
|
||||||
|
// the same single open path exercised here, so the mode is settled for
|
||||||
|
// all three tiers in one place.
|
||||||
|
func TestArchiveDatabaseFilesAreOwnerOnly(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
path := filepath.Join(
|
||||||
|
t.TempDir(), "archive-"+uuid.New().String()+".db",
|
||||||
|
)
|
||||||
|
|
||||||
|
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
defer func() { require.NoError(t, sqlDB.Close()) }()
|
||||||
|
|
||||||
|
_, err = sqlDB.ExecContext(ctx, "create table t (id integer)")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
requireDatabaseSetOwnerOnly(t, path)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestOpenSQLiteTightensFilesLeftWorldReadable is the upgrade case: a
|
||||||
|
// data directory an earlier build left at 0644, including a
|
||||||
|
// developer's own scratch directory, is fixed when it is opened rather
|
||||||
|
// than staying exposed until it is recreated.
|
||||||
|
func TestOpenSQLiteTightensFilesLeftWorldReadable(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, database.MainDBFileName)
|
||||||
|
|
||||||
|
// A database and both sidecars as the pre-fix build left them.
|
||||||
|
for _, p := range []string{path, path + "-wal", path + "-shm"} {
|
||||||
|
require.NoError(t, os.WriteFile(p, nil, 0o644)) //nolint:gosec // the mode under test
|
||||||
|
}
|
||||||
|
|
||||||
|
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.NoError(t, sqlDB.Close())
|
||||||
|
|
||||||
|
requireDatabaseSetOwnerOnly(t, path)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestOpenSQLiteExistingModeDoesNotCreateTheFile guards the mechanism
|
||||||
|
// the fix uses: OpenSQLite now creates the database file itself, and
|
||||||
|
// must not do so for a caller that asked for an existing database. An
|
||||||
|
// empty file materialized here would turn a missing-database error
|
||||||
|
// into a silently empty one.
|
||||||
|
func TestOpenSQLiteExistingModeDoesNotCreateTheFile(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
path := filepath.Join(t.TempDir(), "absent.db")
|
||||||
|
|
||||||
|
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeExisting)
|
||||||
|
if err == nil {
|
||||||
|
// sql.Open is lazy: force the connection that fails.
|
||||||
|
require.Error(t, sqlDB.PingContext(ctx))
|
||||||
|
require.NoError(t, sqlDB.Close())
|
||||||
|
}
|
||||||
|
|
||||||
|
_, statErr := os.Stat(path)
|
||||||
|
assert.ErrorIs(t, statErr, fs.ErrNotExist)
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestReopenAfterRestartKeepsFilesOwnerOnly is the restart case: a
|
||||||
|
// process that closed its files must be able to open them again at
|
||||||
|
// 0600, including through a gorm handle, and the sidecars must come
|
||||||
|
// back at 0600 too rather than at SQLite's own default.
|
||||||
|
func TestReopenAfterRestartKeepsFilesOwnerOnly(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
ctx := context.Background()
|
||||||
|
dir := t.TempDir()
|
||||||
|
path := filepath.Join(dir, database.MainDBFileName)
|
||||||
|
|
||||||
|
first, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
_, err = first.ExecContext(ctx, "create table t (id integer)")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NoError(t, first.Close())
|
||||||
|
|
||||||
|
second, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
defer func() { require.NoError(t, second.Close()) }()
|
||||||
|
|
||||||
|
_, err = second.ExecContext(ctx, "insert into t (id) values (1)")
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
requireDatabaseSetOwnerOnly(t, path)
|
||||||
|
|
||||||
|
var got int
|
||||||
|
|
||||||
|
require.NoError(t,
|
||||||
|
second.QueryRowContext(ctx, "select id from t").Scan(&got))
|
||||||
|
assert.Equal(t, 1, got)
|
||||||
|
}
|
||||||
@@ -2,8 +2,11 @@ package database
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"database/sql"
|
"database/sql"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io/fs"
|
||||||
"net/url"
|
"net/url"
|
||||||
|
"os"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
_ "modernc.org/sqlite" // Pure Go SQLite driver
|
_ "modernc.org/sqlite" // Pure Go SQLite driver
|
||||||
@@ -72,6 +75,90 @@ const (
|
|||||||
sqliteConnMaxIdleTime = time.Minute
|
sqliteConnMaxIdleTime = time.Minute
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// SQLiteFilePerm is the mode every SQLite file this service owns is
|
||||||
|
// created with and held at: owner read/write, nothing for group or
|
||||||
|
// other.
|
||||||
|
//
|
||||||
|
// These files hold credentials in plaintext. The main database stores
|
||||||
|
// `targets.config` — bearer tokens, API keys, Slack webhook URLs — and
|
||||||
|
// the session encryption key. SQLite left to itself creates them 0644
|
||||||
|
// (see reserveSQLiteFile), which made the 0750 data directory the only
|
||||||
|
// barrier; a bind-mounted directory supplied at 0755 removes it and
|
||||||
|
// every local user on the host can read every stored credential.
|
||||||
|
//
|
||||||
|
// This is a file-mode fix and not encryption at rest. An unattended
|
||||||
|
// process needs a key it can read without a human, so the key lands
|
||||||
|
// beside the data and an attacker who can read the database can read
|
||||||
|
// it too. See https://git.eeqj.de/sneak/webhooker/issues/212.
|
||||||
|
const SQLiteFilePerm fs.FileMode = 0o600
|
||||||
|
|
||||||
|
// reserveSQLiteFile puts path at SQLiteFilePerm before the driver ever
|
||||||
|
// touches it, and tightens any sidecar already on disk.
|
||||||
|
//
|
||||||
|
// The mode has to be settled here rather than by a chmod after opening,
|
||||||
|
// because SQLite picks it: robust_open substitutes
|
||||||
|
// SQLITE_DEFAULT_FILE_PERMISSIONS (0644) whenever it is handed mode 0,
|
||||||
|
// and findCreateFileMode yields 0 for a main database opened by URI
|
||||||
|
// with no `modeof` parameter. A chmod afterwards would leave a window
|
||||||
|
// in which the credentials are on disk world-readable.
|
||||||
|
//
|
||||||
|
// Creating the file ourselves also settles the sidecars, which is the
|
||||||
|
// half that could quietly not work. SQLite does not create those at a
|
||||||
|
// mode we choose — it derives both from the main database file:
|
||||||
|
// `-wal` through findCreateFileMode, which stats the path with the
|
||||||
|
// suffix stripped, and `-shm` in unixOpenSharedMemory from an fstat of
|
||||||
|
// the already-open database descriptor. A main file at 0600 therefore
|
||||||
|
// produces sidecars at 0600. A zero-length file is a valid empty
|
||||||
|
// database, so reserving it changes nothing else.
|
||||||
|
//
|
||||||
|
// create says whether the caller is opening in a mode that may create
|
||||||
|
// the database. When it is false a missing file is left missing, so
|
||||||
|
// SQLite still reports the absence rather than this function
|
||||||
|
// materializing an empty database the caller asked not to create.
|
||||||
|
//
|
||||||
|
// Chmod of a file that already exists is what tightens a data
|
||||||
|
// directory an earlier build left at 0644 — including a developer's
|
||||||
|
// own scratch directory — without any migration machinery.
|
||||||
|
func reserveSQLiteFile(path string, create bool) error {
|
||||||
|
if create {
|
||||||
|
// gosec G304: the path is the database file the caller asked
|
||||||
|
// to open, and the driver is about to open the same path
|
||||||
|
// anyway. Creating it here is what fixes its mode.
|
||||||
|
f, err := os.OpenFile( //nolint:gosec // see above
|
||||||
|
path, os.O_RDWR|os.O_CREATE, SQLiteFilePerm,
|
||||||
|
)
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("creating %s: %w", path, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
err = f.Close()
|
||||||
|
if err != nil {
|
||||||
|
return fmt.Errorf("closing %s: %w", path, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// O_CREATE leaves an existing file's mode alone, and umask can only
|
||||||
|
// have narrowed a new one. Chmod settles both cases at exactly
|
||||||
|
// SQLiteFilePerm.
|
||||||
|
for _, p := range append(
|
||||||
|
[]string{path}, sqliteSidecarPaths(path)...,
|
||||||
|
) {
|
||||||
|
err := os.Chmod(p, SQLiteFilePerm)
|
||||||
|
if err != nil && !errors.Is(err, fs.ErrNotExist) {
|
||||||
|
return fmt.Errorf("securing %s: %w", p, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|
||||||
|
// sqliteSidecarPaths returns the files SQLite maintains beside a
|
||||||
|
// database under WAL. They carry the same rows as the database itself,
|
||||||
|
// so a fix that tightens only the main file has fixed nothing.
|
||||||
|
func sqliteSidecarPaths(path string) []string {
|
||||||
|
return []string{path + "-wal", path + "-shm"}
|
||||||
|
}
|
||||||
|
|
||||||
// SQLiteDSN builds the connection string for one database file.
|
// SQLiteDSN builds the connection string for one database file.
|
||||||
//
|
//
|
||||||
// mode is the SQLite URI open mode: "rwc" to create the file when it
|
// mode is the SQLite URI open mode: "rwc" to create the file when it
|
||||||
@@ -138,9 +225,17 @@ func SQLiteDSN(path, mode string) string {
|
|||||||
// durability settings and pool bounds applied. mode is the SQLite URI
|
// durability settings and pool bounds applied. mode is the SQLite URI
|
||||||
// open mode ("rwc" or "rw").
|
// open mode ("rwc" or "rw").
|
||||||
//
|
//
|
||||||
|
// The file and its WAL sidecars are settled at SQLiteFilePerm before
|
||||||
|
// the driver sees the path; see reserveSQLiteFile.
|
||||||
|
//
|
||||||
// The handle is returned rather than a *gorm.DB because the callers
|
// The handle is returned rather than a *gorm.DB because the callers
|
||||||
// wrap it in gorm themselves with their own logger.
|
// wrap it in gorm themselves with their own logger.
|
||||||
func OpenSQLite(path, mode string) (*sql.DB, error) {
|
func OpenSQLite(path, mode string) (*sql.DB, error) {
|
||||||
|
err := reserveSQLiteFile(path, mode == SQLiteModeCreate)
|
||||||
|
if err != nil {
|
||||||
|
return nil, err
|
||||||
|
}
|
||||||
|
|
||||||
sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
|
sqlDB, err := sql.Open("sqlite", SQLiteDSN(path, mode))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return nil, fmt.Errorf(
|
return nil, fmt.Errorf(
|
||||||
|
|||||||
@@ -70,7 +70,7 @@ func (s *Server) serveUntilShutdown() {
|
|||||||
err := s.httpServer.ListenAndServe()
|
err := s.httpServer.ListenAndServe()
|
||||||
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
if err != nil && !errors.Is(err, http.ErrServerClosed) {
|
||||||
s.log.Error("listen error", "error", err)
|
s.log.Error("listen error", "error", err)
|
||||||
s.shutdownOnListenFailure()
|
s.shutdownWithFailure()
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -93,7 +93,7 @@ func requireListenFailureExit(t *testing.T, env *testEnv) {
|
|||||||
select {
|
select {
|
||||||
case sig := <-app.Wait():
|
case sig := <-app.Wait():
|
||||||
require.Equal(
|
require.Equal(
|
||||||
t, server.ListenFailureExitCode, sig.ExitCode,
|
t, server.StartupFailureExitCode, sig.ExitCode,
|
||||||
"listen failure must exit non-zero",
|
"listen failure must exit non-zero",
|
||||||
)
|
)
|
||||||
case <-time.After(listenFailureDeadline):
|
case <-time.After(listenFailureDeadline):
|
||||||
|
|||||||
99
internal/server/sentry_failure_test.go
Normal file
99
internal/server/sentry_failure_test.go
Normal file
@@ -0,0 +1,99 @@
|
|||||||
|
package server_test
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net"
|
||||||
|
"strconv"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
"go.uber.org/fx"
|
||||||
|
"sneak.berlin/go/webhooker/internal/config"
|
||||||
|
"sneak.berlin/go/webhooker/internal/globals"
|
||||||
|
"sneak.berlin/go/webhooker/internal/server"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestSentryInitFailure_ShutsDownTheApp pins that error reporting
|
||||||
|
// which is configured and cannot be started ends the application
|
||||||
|
// instead of serving without it.
|
||||||
|
//
|
||||||
|
// The measured defect logged `sentry init failure` and kept running,
|
||||||
|
// so the deployment served traffic with reporting off while every
|
||||||
|
// other signal — SENTRY_DSN still set, the startup summary's own
|
||||||
|
// field — said it was on. Nothing later in the process can notice
|
||||||
|
// that reports are going nowhere, which is why this exits rather than
|
||||||
|
// degrades.
|
||||||
|
//
|
||||||
|
// The DSN is placed on a hand-built Config, which is the only way to
|
||||||
|
// reach this branch at all: loadFromEnv now parses SENTRY_DSN with
|
||||||
|
// sentry.NewDsn, the same call sentry.Init makes, so a DSN that
|
||||||
|
// survives configuration cannot fail initialisation in the SDK
|
||||||
|
// version this pins. The branch stays because that is a property of
|
||||||
|
// the SDK's current implementation rather than of its contract.
|
||||||
|
func TestSentryInitFailure_ShutsDownTheApp(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
|
||||||
|
port := freePort(t)
|
||||||
|
|
||||||
|
env := newTestEnvWithConfig(t, &config.Config{
|
||||||
|
DataDir: t.TempDir(),
|
||||||
|
Environment: config.EnvironmentDev,
|
||||||
|
BindAddress: loopbackV4,
|
||||||
|
Port: port,
|
||||||
|
SentryDSN: "not-a-dsn",
|
||||||
|
})
|
||||||
|
|
||||||
|
app := fx.New(
|
||||||
|
fx.NopLogger,
|
||||||
|
fx.Supply(env.log, env.cfg, env.mw, env.hnd),
|
||||||
|
fx.Provide(globals.New, server.New),
|
||||||
|
fx.Invoke(func(*server.Server) {}),
|
||||||
|
)
|
||||||
|
|
||||||
|
startCtx, cancelStart := context.WithTimeout(
|
||||||
|
context.Background(), lifecycleTimeout,
|
||||||
|
)
|
||||||
|
defer cancelStart()
|
||||||
|
|
||||||
|
require.NoError(t, app.Start(startCtx))
|
||||||
|
|
||||||
|
select {
|
||||||
|
case sig := <-app.Wait():
|
||||||
|
require.Equal(
|
||||||
|
t, server.StartupFailureExitCode, sig.ExitCode,
|
||||||
|
"a sentry failure must exit non-zero",
|
||||||
|
)
|
||||||
|
case <-time.After(listenFailureDeadline):
|
||||||
|
t.Fatal("a sentry failure left the app running")
|
||||||
|
}
|
||||||
|
|
||||||
|
// The stop sequence still has to complete: the failure must reach
|
||||||
|
// shutdown through fx rather than around it.
|
||||||
|
stopCtx, cancelStop := context.WithTimeout(
|
||||||
|
context.Background(), lifecycleTimeout,
|
||||||
|
)
|
||||||
|
defer cancelStop()
|
||||||
|
|
||||||
|
require.NoError(t, app.Stop(stopCtx))
|
||||||
|
|
||||||
|
// And it must give up before it listens. A process that bound the
|
||||||
|
// port and then exited would have accepted requests it could not
|
||||||
|
// report on, which is the state under test in miniature.
|
||||||
|
requireBindable(t, port)
|
||||||
|
}
|
||||||
|
|
||||||
|
// requireBindable asserts that the port is free, which it is only if
|
||||||
|
// the server under test never claimed it.
|
||||||
|
func requireBindable(t *testing.T, port int) {
|
||||||
|
t.Helper()
|
||||||
|
|
||||||
|
var listenCfg net.ListenConfig
|
||||||
|
|
||||||
|
listener, err := listenCfg.Listen(
|
||||||
|
t.Context(), "tcp",
|
||||||
|
net.JoinHostPort(loopbackV4, strconv.Itoa(port)),
|
||||||
|
)
|
||||||
|
require.NoError(t, err, "the server bound a port it then gave up")
|
||||||
|
require.NoError(t, listener.Close())
|
||||||
|
}
|
||||||
@@ -51,12 +51,13 @@ const (
|
|||||||
minSentryFlush = 250 * time.Millisecond
|
minSentryFlush = 250 * time.Millisecond
|
||||||
)
|
)
|
||||||
|
|
||||||
// ListenFailureExitCode is the status the process exits with when the
|
// StartupFailureExitCode is the status the process exits with when
|
||||||
// HTTP listener cannot be established, or dies for a reason other
|
// the serving goroutine gives up: the HTTP listener cannot be
|
||||||
// than a requested shutdown. It must stay non-zero: systemd
|
// established or dies for a reason other than a requested shutdown, or
|
||||||
// `Restart=on-failure` and Docker's restart policies key off it, and a
|
// error reporting is configured and cannot be started. It must stay
|
||||||
// zero exit would read as a deliberate stop.
|
// non-zero: systemd `Restart=on-failure` and Docker's restart policies
|
||||||
const ListenFailureExitCode = 1
|
// key off it, and a zero exit would read as a deliberate stop.
|
||||||
|
const StartupFailureExitCode = 1
|
||||||
|
|
||||||
// SentryFlushBudget reports how long the Sentry flush may run when
|
// SentryFlushBudget reports how long the Sentry flush may run when
|
||||||
// remaining is the time left on the fx stop context after the HTTP
|
// remaining is the time left on the fx stop context after the HTTP
|
||||||
@@ -135,11 +136,25 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Run configures Sentry and starts serving HTTP requests.
|
// Run configures Sentry and starts serving HTTP requests.
|
||||||
|
//
|
||||||
|
// A Sentry failure ends the application instead of listening. It runs
|
||||||
|
// before the listener rather than after it so that the process never
|
||||||
|
// binds a port it is about to give up.
|
||||||
func (s *Server) Run() {
|
func (s *Server) Run() {
|
||||||
s.configure()
|
s.configure()
|
||||||
|
|
||||||
// logging before sentry, because sentry logs
|
// logging before sentry, because sentry logs
|
||||||
s.enableSentry()
|
err := s.enableSentry()
|
||||||
|
if err != nil {
|
||||||
|
s.log.Error(
|
||||||
|
"SENTRY_DSN is set but error reporting could not be "+
|
||||||
|
"started; refusing to serve with it off",
|
||||||
|
"error", err,
|
||||||
|
)
|
||||||
|
s.shutdownWithFailure()
|
||||||
|
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
s.serve()
|
s.serve()
|
||||||
}
|
}
|
||||||
@@ -150,11 +165,23 @@ func (s *Server) MaintenanceMode() bool {
|
|||||||
return s.params.Config.MaintenanceMode
|
return s.params.Config.MaintenanceMode
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Server) enableSentry() {
|
// enableSentry initialises the Sentry SDK when error reporting is
|
||||||
|
// configured, and reports the failure when it is configured and cannot
|
||||||
|
// be initialised. A DSN that is not set is not a failure: reporting
|
||||||
|
// stays off and the server starts normally.
|
||||||
|
//
|
||||||
|
// There is no fallback to running with reporting off. An operator who
|
||||||
|
// set SENTRY_DSN asked for failures to be visible, and serving traffic
|
||||||
|
// with reporting quietly off is the one state nothing can ever tell
|
||||||
|
// them about — the DSN is still set, so every later signal says it is
|
||||||
|
// on. Config already refused a DSN the SDK cannot parse, which is what
|
||||||
|
// a typo produces, so reaching this branch means the SDK refused
|
||||||
|
// something that parsed: not a condition to guess at either.
|
||||||
|
func (s *Server) enableSentry() error {
|
||||||
s.sentryEnabled.Store(false)
|
s.sentryEnabled.Store(false)
|
||||||
|
|
||||||
if s.params.Config.SentryDSN == "" {
|
if !s.params.Config.SentryEnabled() {
|
||||||
return
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
err := sentry.Init(sentryClientOptions(
|
err := sentry.Init(sentryClientOptions(
|
||||||
@@ -166,19 +193,19 @@ func (s *Server) enableSentry() {
|
|||||||
),
|
),
|
||||||
))
|
))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Error("sentry init failure", "error", err)
|
return fmt.Errorf("initialising sentry: %w", err)
|
||||||
// Don't use fatal since we still want the service to run
|
|
||||||
return
|
|
||||||
}
|
}
|
||||||
|
|
||||||
s.log.Info("sentry error reporting activated")
|
s.log.Info("sentry error reporting activated")
|
||||||
s.sentryEnabled.Store(true)
|
s.sentryEnabled.Store(true)
|
||||||
|
|
||||||
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// serve installs the signal watcher, starts the listener and blocks
|
// serve installs the signal watcher, starts the listener and blocks
|
||||||
// until the server's context is cancelled. The process exit status is
|
// until the server's context is cancelled. The process exit status is
|
||||||
// fx's to decide — from a signal, or from the code
|
// fx's to decide — from a signal, or from the code
|
||||||
// shutdownOnListenFailure hands the Shutdowner — so this reports
|
// shutdownWithFailure hands the Shutdowner — so this reports
|
||||||
// nothing back to its caller.
|
// nothing back to its caller.
|
||||||
func (s *Server) serve() {
|
func (s *Server) serve() {
|
||||||
ctx, cancelFunc := context.WithCancel(context.Background())
|
ctx, cancelFunc := context.WithCancel(context.Background())
|
||||||
@@ -208,20 +235,24 @@ func (s *Server) serve() {
|
|||||||
// Do not call cleanShutdown() here to avoid double invocation.
|
// Do not call cleanShutdown() here to avoid double invocation.
|
||||||
}
|
}
|
||||||
|
|
||||||
// shutdownOnListenFailure ends the application after the HTTP
|
// shutdownWithFailure ends the application non-zero from the serving
|
||||||
// listener failed. The fx OnStart hook returns as soon as the serving
|
// goroutine. It is how anything on that goroutine fails fatally: the
|
||||||
// goroutine is spawned, so nothing downstream of it ever learns that
|
// fx OnStart hook returns as soon as the goroutine is spawned, so
|
||||||
// the listen failed: fx reports RUNNING and the process sits alive
|
// nothing downstream of it ever learns that the goroutine gave up. fx
|
||||||
// with nothing bound, which is invisible to systemd and Docker
|
// reports RUNNING and the process sits alive having done neither what
|
||||||
// restart policies. Asking the Shutdowner to stop the app with a
|
// it was asked nor anything visible instead, which systemd and
|
||||||
// non-zero code is what turns that into a visible failure.
|
// Docker restart policies cannot see. Asking the Shutdowner to stop
|
||||||
|
// the app with a non-zero code is what turns that into a visible
|
||||||
|
// failure, and it is the whole of "fatal" here — no panic, no
|
||||||
|
// os.Exit, and every stop hook still runs.
|
||||||
//
|
//
|
||||||
// The context cancel that follows only unwinds serve()'s own wait.
|
// The context cancel that follows only unwinds serve()'s own wait,
|
||||||
// The shutdown itself runs through fx's normal stop sequence, so the
|
// and is skipped before serve has installed one. The shutdown itself
|
||||||
// clean-shutdown drain in cleanShutdown is reached unchanged.
|
// runs through fx's normal stop sequence, so the clean-shutdown drain
|
||||||
func (s *Server) shutdownOnListenFailure() {
|
// in cleanShutdown is reached unchanged.
|
||||||
|
func (s *Server) shutdownWithFailure() {
|
||||||
err := s.params.Shutdowner.Shutdown(
|
err := s.params.Shutdowner.Shutdown(
|
||||||
fx.ExitCode(ListenFailureExitCode),
|
fx.ExitCode(StartupFailureExitCode),
|
||||||
)
|
)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.log.Error("shutdown request failed", "error", err)
|
s.log.Error("shutdown request failed", "error", err)
|
||||||
|
|||||||
Reference in New Issue
Block a user