From bd157ac9701a64073042e03dda8313c8337fd981 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 19 Oct 2026 04:19:31 +0000 Subject: [PATCH] main() is one call into internal/cli (closes #160) cmd/netwatch-server/main.go parsed the command line, ran the check-cidr and prepare-data-dir subcommands and built the fx application itself; the Go style guide puts all of that in internal/ or pkg/. The new package internal/cli does it, with the same arguments, messages and exit codes. main() is now os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer)): Main takes the writer for errors and the function that runs the server, so its tests give it a buffer and a function that fails them. Appname and Version stay in main, where the build sets Version. The function behind prepare-data-dir also takes the volume and the user's name, so its own tests give it a temporary directory and the user they run as. Model: opus-5-5 --- README.md | 3 + TODO.md | 14 +++ cmd/netwatch-server/main.go | 78 +------------ internal/cli/cli.go | 129 +++++++++++++++++++++ internal/cli/cli_test.go | 164 +++++++++++++++++++++++++++ internal/cli/export_test.go | 16 +++ internal/config/config_test.go | 8 +- internal/globals/globals.go | 2 +- internal/reportbuf/reportbuf_test.go | 14 +-- internal/server/routes_test.go | 6 +- 10 files changed, 345 insertions(+), 89 deletions(-) create mode 100644 internal/cli/cli.go create mode 100644 internal/cli/cli_test.go create mode 100644 internal/cli/export_test.go diff --git a/README.md b/README.md index 6e2778e..f422108 100644 --- a/README.md +++ b/README.md @@ -218,6 +218,9 @@ stops. Its routes: It is an `fx`-wired Go application: `cmd/netwatch-server/` starts it, and its packages are in `internal/`: +- **`cli`**: Runs `netwatch-server` from its command line: the `check-cidr` and + `prepare-data-dir` subcommands, which `bin/entrypoint.sh` runs before it + starts the server, and otherwise the server, with the `fx` wiring. - **`config`**: Loads configuration from environment variables and config files via Viper. - **`handlers`**: HTTP request handlers for the API (health check, report diff --git a/TODO.md b/TODO.md index 9f120e6..1431c2c 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,20 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of # Completed Steps +- 2026-10-19: `main()` in `cmd/netwatch-server/main.go` is one call into a new + package, `internal/cli`, as the Go style guide asks + ([#160](https://git.eeqj.de/sneak/netwatch/issues/160)). `internal/cli` runs + the `check-cidr` and `prepare-data-dir` subcommands, with the same arguments, + messages and exit codes, and otherwise the server, with the `fx` wiring. + `Appname` and `Version` stay in `main`, where the build sets `Version`, and + `main` gives `cli.Main` stderr and the function that runs the server, which + the tests replace with a buffer and a function that fails them. Unit tests + call `cli.Main` with `check-cidr` and a CIDR the server takes and one it + refuses, and check the exit code and what it writes to stderr, and with + `prepare-data-dir` and a path outside `/data`, and check that it exits 1 with + the error from looking up the `netwatch` user, which the test image does not + have. Others give the functions behind the subcommands input they take and + input they refuse, and check the exit code and what they write to stderr - 2026-10-19: when `VERSION` is unset or empty, `script/build` stamps the version from `git describe --tags --always --dirty`, as `script/docker` does ([#156](https://git.eeqj.de/sneak/netwatch/issues/156)). Without `--tags` it diff --git a/cmd/netwatch-server/main.go b/cmd/netwatch-server/main.go index d9f3c17..5eefc12 100644 --- a/cmd/netwatch-server/main.go +++ b/cmd/netwatch-server/main.go @@ -1,22 +1,11 @@ -// Package main is the entry point for netwatch-server. +// Package main is the entry point for netwatch-server; package cli +// does the work. package main import ( - "fmt" "os" - "os/user" - "sneak.berlin/go/netwatch/internal/config" - "sneak.berlin/go/netwatch/internal/globals" - "sneak.berlin/go/netwatch/internal/handlers" - "sneak.berlin/go/netwatch/internal/healthcheck" - "sneak.berlin/go/netwatch/internal/logger" - "sneak.berlin/go/netwatch/internal/middleware" - "sneak.berlin/go/netwatch/internal/reportbuf" - "sneak.berlin/go/netwatch/internal/server" - - "go.uber.org/fx" - "go.uber.org/fx/fxevent" + "sneak.berlin/go/netwatch/internal/cli" ) //nolint:gochecknoglobals // set via ldflags at build time @@ -26,64 +15,5 @@ var ( ) func main() { - // "netwatch-server check-cidr CIDR" exits 1, with the error, if - // this server would refuse CIDR in its TRUSTED_PROXIES. - // bin/entrypoint.sh runs it on each entry it gives nginx. - if len(os.Args) == 3 && os.Args[1] == "check-cidr" { - _, err := middleware.ParseTrustedProxies(os.Args[2:]) - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - - return - } - - // "netwatch-server prepare-data-dir DATA_DIR" gets DATA_DIR ready - // for the netwatch user, or exits 1 with the error; see - // reportbuf.PrepareDataDir. bin/entrypoint.sh runs it as root - // before it starts this server as that user. - if len(os.Args) == 3 && os.Args[1] == "prepare-data-dir" { - netwatch, err := user.Lookup("netwatch") - if err != nil { - fmt.Fprintln(os.Stderr, err) - os.Exit(1) - } - - err = reportbuf.PrepareDataDir("/data", os.Args[2], netwatch) - if err != nil { - fmt.Fprintf(os.Stderr, "DATA_DIR '%s': %v\n", os.Args[2], err) - os.Exit(1) - } - - return - } - - globals.Appname = Appname - globals.Version = Version - - fx.New( - // fx logs each step of starting and stopping through the - // server's own logger, so off a terminal those lines are - // JSON like every other line. - fx.WithLogger(func(log *logger.Logger) fxevent.Logger { - return &fxevent.SlogLogger{Logger: log.Get()} - }), - fx.Provide( - config.New, - globals.New, - handlers.New, - healthcheck.New, - logger.New, - middleware.New, - reportbuf.New, - server.New, - ), - fx.Invoke( - // First, so the name and version are logged even when - // a setting stops the start. - func(log *logger.Logger) { log.Identify() }, - func(*server.Server) {}, - ), - ).Run() + os.Exit(cli.Main(os.Stderr, Appname, Version, os.Args[1:], cli.RunServer)) } diff --git a/internal/cli/cli.go b/internal/cli/cli.go new file mode 100644 index 0000000..c53793d --- /dev/null +++ b/internal/cli/cli.go @@ -0,0 +1,129 @@ +// Package cli runs netwatch-server from its command line: the +// check-cidr and prepare-data-dir subcommands, which bin/entrypoint.sh +// runs before it starts the server, and otherwise the server, with the +// fx wiring. +package cli + +import ( + "fmt" + "io" + "os/user" + + "sneak.berlin/go/netwatch/internal/config" + "sneak.berlin/go/netwatch/internal/globals" + "sneak.berlin/go/netwatch/internal/handlers" + "sneak.berlin/go/netwatch/internal/healthcheck" + "sneak.berlin/go/netwatch/internal/logger" + "sneak.berlin/go/netwatch/internal/middleware" + "sneak.berlin/go/netwatch/internal/reportbuf" + "sneak.berlin/go/netwatch/internal/server" + + "go.uber.org/fx" + "go.uber.org/fx/fxevent" +) + +// Main runs netwatch-server with args, its command line after the +// program's name, and returns the exit code; the subcommands write +// their errors to stderr. Unless args are "check-cidr CIDR" or +// "prepare-data-dir DATA_DIR", Main runs the server by calling serve +// with appname and version, the name and version the server logs and +// reports. main passes os.Stderr and RunServer; the tests pass a +// buffer and a function that fails them, so that no test starts the +// server. When serve is RunServer and the server fails to start, to +// listen or to stop, fx's Run exits the process with status 1 and Main +// never returns. +func Main( + stderr io.Writer, + appname, version string, + args []string, + serve func(appname, version string), +) int { + // "netwatch-server check-cidr CIDR" exits 1, with the error, if + // this server would refuse CIDR in its TRUSTED_PROXIES. + // bin/entrypoint.sh runs it on each entry it gives nginx. + if len(args) == 2 && args[0] == "check-cidr" { + return checkCIDR(stderr, args[1]) + } + + // "netwatch-server prepare-data-dir DATA_DIR" gets DATA_DIR ready + // for the netwatch user, or exits 1 with the error; see + // reportbuf.PrepareDataDir. bin/entrypoint.sh runs it as root + // before it starts this server as that user. + if len(args) == 2 && args[0] == "prepare-data-dir" { + return prepareDataDir(stderr, "/data", args[1], "netwatch") + } + + serve(appname, version) + + return 0 +} + +// checkCIDR returns 0 if the server would take cidr in its +// TRUSTED_PROXIES, and otherwise writes the error to stderr and +// returns 1. +func checkCIDR(stderr io.Writer, cidr string) int { + _, err := middleware.ParseTrustedProxies([]string{cidr}) + if err != nil { + _, _ = fmt.Fprintln(stderr, err) + + return 1 + } + + return 0 +} + +// prepareDataDir gets dataDir ready for the user named username with +// reportbuf.PrepareDataDir, volume being where the data volume is +// mounted, and returns 0. Otherwise it writes the error to stderr and +// returns 1. +func prepareDataDir(stderr io.Writer, volume, dataDir, username string) int { + owner, err := user.Lookup(username) + if err != nil { + _, _ = fmt.Fprintln(stderr, err) + + return 1 + } + + err = reportbuf.PrepareDataDir(volume, dataDir, owner) + if err != nil { + _, _ = fmt.Fprintf(stderr, "DATA_DIR '%s': %v\n", dataDir, err) + + return 1 + } + + return 0 +} + +// RunServer runs the server until it is told to stop, with appname and +// version as the name and version it logs and reports. If the server +// fails to start, to listen or to stop, fx's Run exits the process +// with status 1 itself. +func RunServer(appname, version string) { + globals.Appname = appname + globals.Version = version + + fx.New( + // fx logs each step of starting and stopping through the + // server's own logger, so off a terminal those lines are + // JSON like every other line. + fx.WithLogger(func(log *logger.Logger) fxevent.Logger { + return &fxevent.SlogLogger{Logger: log.Get()} + }), + fx.Provide( + config.New, + globals.New, + handlers.New, + healthcheck.New, + logger.New, + middleware.New, + reportbuf.New, + server.New, + ), + fx.Invoke( + // First, so the name and version are logged even when + // a setting stops the start. + func(log *logger.Logger) { log.Identify() }, + func(*server.Server) {}, + ), + ).Run() +} diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go new file mode 100644 index 0000000..de6c46a --- /dev/null +++ b/internal/cli/cli_test.go @@ -0,0 +1,164 @@ +package cli_test + +import ( + "bytes" + "os" + "os/user" + "path/filepath" + "strconv" + "strings" + "testing" + + "sneak.berlin/go/netwatch/internal/cli" + "sneak.berlin/go/netwatch/internal/reportbuf" +) + +// TestMainRunsCheckCIDR: Main given check-cidr returns 0, writing +// nothing, for a CIDR the server takes in its TRUSTED_PROXIES, and 1, +// with an error naming TRUSTED_PROXIES and the CIDR, for one it +// refuses, and does not start the server. +func TestMainRunsCheckCIDR(t *testing.T) { + t.Parallel() + + var stderr bytes.Buffer + + code := cli.Main(&stderr, "netwatch", "test", + []string{"check-cidr", "10.0.0.0/8"}, failIfServerStarts(t)) + if code != 0 || stderr.Len() != 0 { + t.Errorf("check-cidr 10.0.0.0/8: exit %d, stderr %q; want exit 0 and no output", + code, stderr.String()) + } + + stderr.Reset() + + code = cli.Main(&stderr, "netwatch", "test", + []string{"check-cidr", "1.2.3/32"}, failIfServerStarts(t)) + + want := `TRUSTED_PROXIES "1.2.3/32": ` + if code != 1 || !strings.HasPrefix(stderr.String(), want) { + t.Errorf("check-cidr 1.2.3/32: exit %d, stderr %q; want exit 1 and %q...", + code, stderr.String(), want) + } +} + +// TestMainRunsPrepareDataDir: Main given prepare-data-dir runs the +// prepare-data-dir subcommand for the netwatch user, and does not start +// the server. The test image has no netwatch user, so that subcommand +// exits 1 with the error from looking the user up; +// TestPrepareDataDirRefusesPathOutsideVolume checks the path. +func TestMainRunsPrepareDataDir(t *testing.T) { + t.Parallel() + + var stderr bytes.Buffer + + code := cli.Main(&stderr, "netwatch", "test", + []string{"prepare-data-dir", "/etc"}, failIfServerStarts(t)) + + want := user.UnknownUserError("netwatch").Error() + "\n" + if code != 1 || stderr.String() != want { + t.Errorf("prepare-data-dir /etc: exit %d, stderr %q; want exit 1 and %q", + code, stderr.String(), want) + } +} + +// TestCheckCIDRTakesCIDR: check-cidr exits 0, writing nothing, for a +// CIDR the server takes in its TRUSTED_PROXIES. +func TestCheckCIDRTakesCIDR(t *testing.T) { + t.Parallel() + + for _, cidr := range []string{"10.0.0.0/8", "192.0.2.1/32", "2001:db8::/32"} { + var stderr bytes.Buffer + + code := cli.CheckCIDR(&stderr, cidr) + if code != 0 || stderr.Len() != 0 { + t.Errorf("%q: exit %d, stderr %q; want exit 0 and no output", + cidr, code, stderr.String()) + } + } +} + +// TestCheckCIDRRefusesOtherInput: check-cidr exits 1, with an error +// naming TRUSTED_PROXIES and the input, for anything the server would +// refuse in its TRUSTED_PROXIES, such as 1.2.3/32, which nginx would +// look up as a hostname. +func TestCheckCIDRRefusesOtherInput(t *testing.T) { + t.Parallel() + + for _, cidr := range []string{ + "1.2.3/32", "example.com/32", "10.0.0.0/33", "10.0.0.1", "", + } { + var stderr bytes.Buffer + + code := cli.CheckCIDR(&stderr, cidr) + + want := "TRUSTED_PROXIES " + strconv.Quote(cidr) + ": " + if code != 1 || !strings.HasPrefix(stderr.String(), want) { + t.Errorf("%q: exit %d, stderr %q; want exit 1 and %q...", + cidr, code, stderr.String(), want) + } + } +} + +// TestPrepareDataDirTakesPathInVolume: prepare-data-dir exits 0, +// writing nothing, for a DATA_DIR below the volume, and creates it. +func TestPrepareDataDirTakesPathInVolume(t *testing.T) { + t.Parallel() + + volume := t.TempDir() + dataDir := filepath.Join(volume, "reports") + + var stderr bytes.Buffer + + code := cli.PrepareDataDir(&stderr, volume, dataDir, currentUsername(t)) + if code != 0 || stderr.Len() != 0 { + t.Errorf("exit %d, stderr %q; want exit 0 and no output", + code, stderr.String()) + } + + info, err := os.Stat(dataDir) + if err != nil || !info.IsDir() { + t.Errorf("DATA_DIR not created: %v", err) + } +} + +// TestPrepareDataDirRefusesPathOutsideVolume: prepare-data-dir exits +// 1, with an error naming DATA_DIR and its value, for a DATA_DIR +// outside the volume. +func TestPrepareDataDirRefusesPathOutsideVolume(t *testing.T) { + t.Parallel() + + volume := t.TempDir() + + var stderr bytes.Buffer + + code := cli.PrepareDataDir(&stderr, volume, "/etc", currentUsername(t)) + + want := "DATA_DIR '/etc': " + reportbuf.ErrDataDirOutsideVolume.Error() + "\n" + if code != 1 || stderr.String() != want { + t.Errorf("exit %d, stderr %q; want exit 1 and %q", + code, stderr.String(), want) + } +} + +// currentUsername is the name of the user the test runs as, the only +// owner a test not run as root can give files to. +func currentUsername(t *testing.T) string { + t.Helper() + + current, err := user.Current() + if err != nil { + t.Fatal(err) + } + + return current.Username +} + +// failIfServerStarts is what the tests give Main in place of +// cli.RunServer: it fails the test instead of starting the server. +func failIfServerStarts(t *testing.T) func(appname, version string) { + t.Helper() + + return func(string, string) { + t.Error("Main started the server") + } +} diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go new file mode 100644 index 0000000..24522b6 --- /dev/null +++ b/internal/cli/export_test.go @@ -0,0 +1,16 @@ +package cli + +import "io" + +// CheckCIDR exposes the check-cidr subcommand to the external tests, +// which read what it writes to stderr. +func CheckCIDR(stderr io.Writer, cidr string) int { + return checkCIDR(stderr, cidr) +} + +// PrepareDataDir exposes the prepare-data-dir subcommand to the +// external tests, which give it a volume and a user of their own in +// place of /data and netwatch. +func PrepareDataDir(stderr io.Writer, volume, dataDir, username string) int { + return prepareDataDir(stderr, volume, dataDir, username) +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 43a15a5..7396ff0 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -13,8 +13,8 @@ import ( "go.uber.org/fx" ) -// requireConfigError builds the config as main does and fails the -// test unless that fails with an error naming setting. It uses +// requireConfigError builds the config as cli.RunServer does and fails +// the test unless that fails with an error naming setting. It uses // fx.New, because fxtest.New fails the test itself on an error. func requireConfigError(t *testing.T, setting string) { t.Helper() @@ -31,8 +31,8 @@ func requireConfigError(t *testing.T, setting string) { } } -// requireTrustedProxies builds the config as main does and fails the -// test unless its TrustedProxies are want. +// requireTrustedProxies builds the config as cli.RunServer does and +// fails the test unless its TrustedProxies are want. func requireTrustedProxies(t *testing.T, want []string) { t.Helper() diff --git a/internal/globals/globals.go b/internal/globals/globals.go index c6a48a2..8920243 100644 --- a/internal/globals/globals.go +++ b/internal/globals/globals.go @@ -4,7 +4,7 @@ package globals import "go.uber.org/fx" -//nolint:gochecknoglobals // set from main before fx starts +//nolint:gochecknoglobals // set by cli.RunServer before fx starts var ( // Appname is the application name. Appname string diff --git a/internal/reportbuf/reportbuf_test.go b/internal/reportbuf/reportbuf_test.go index ffb6128..ddb6d52 100644 --- a/internal/reportbuf/reportbuf_test.go +++ b/internal/reportbuf/reportbuf_test.go @@ -334,9 +334,9 @@ func stopWhileHeld( return <-stopped } -// startApp starts a Buffer through fx, as main does, with the DATA_DIR -// and DATA_DIR_MAX_BYTES the calling test has set, and returns it with -// the app, for tests that stop the app themselves. +// startApp starts a Buffer through fx, as cli.RunServer does, with the +// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set, and returns +// it with the app, for tests that stop the app themselves. func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) { t.Helper() @@ -358,8 +358,8 @@ func startApp(t *testing.T) (*reportbuf.Buffer, *fxtest.App) { return buf, app } -// startBuffer starts a Buffer through fx, as main does, with the -// DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set. +// startBuffer starts a Buffer through fx, as cli.RunServer does, with +// the DATA_DIR and DATA_DIR_MAX_BYTES the calling test has set. func startBuffer(t *testing.T) *reportbuf.Buffer { t.Helper() @@ -1120,8 +1120,8 @@ func TestHealthCheckFailsWhileWritesFail(t *testing.T) { } // startBufferWithHandlers starts a Buffer and the Handlers that store -// reports in it through fx, as main does, with the DATA_DIR the calling -// test has set. +// reports in it through fx, as cli.RunServer does, with the DATA_DIR +// the calling test has set. func startBufferWithHandlers( t *testing.T, ) (*reportbuf.Buffer, *handlers.Handlers) { diff --git a/internal/server/routes_test.go b/internal/server/routes_test.go index fd805f3..a0938f1 100644 --- a/internal/server/routes_test.go +++ b/internal/server/routes_test.go @@ -22,9 +22,9 @@ import ( "go.uber.org/fx/fxtest" ) -// newServer builds a Server from the same constructors as main, -// configured from the environment. It is never started, so nothing -// listens. +// newServer builds a Server from the same constructors as +// cli.RunServer, configured from the environment. It is never started, +// so nothing listens. func newServer(t *testing.T) *server.Server { t.Helper() -- 2.54.0