upaas: health check, settings checked at start, README section (closes #59)

The image's HEALTHCHECK requests /.well-known/healthcheck through
nginx on the port from PORT, so it fails unless both processes answer.
The backend reads PORT and DEBUG with strconv instead of viper, which
turned a bad PORT into 0 and a bad DEBUG into false. Those, and a
BIND_ADDRESS that is not an IP address, now stop the start with an
error naming the variable; the TRUSTED_PROXIES error names it too.
README.md gains "Running under upaas". Its first-run steps create the
host directory owned by uid 1000, so the image changes no ownership.

Model: opus-5-5

backend/README.md

@@ -91,6 +91,10 @@ The loopback entries cover the reverse proxy that shares the container; the
 RFC1918 ranges match `nginx.conf`. A request whose direct peer is outside this
 set has its forwarded headers ignored, and the direct peer is logged instead.
 
+A variable set to a value the server cannot use, such as `PORT=abc`,
+`DEBUG=maybe` or a `BIND_ADDRESS` that is not an IP address, stops it from
+starting, with an error naming the variable. An empty variable counts as unset.
+
 ### Container image
 
 The root `Dockerfile` builds one image in which nginx listens on the public port

backend/internal/config/config.go

@@ -6,7 +6,10 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"math"
+	"net/netip"
 	"net/url"
+	"strconv"
 	"strings"
 
 	"sneak.berlin/go/netwatch/internal/globals"
@@ -37,6 +40,9 @@ var (
 	errNotOrigin   = errors.New(
 		"must be an origin, scheme://host with an optional port",
 	)
+	errNotPort = errors.New("must be a port number, 1 to 65535")
+	errNotBool = errors.New("must be true or false")
+	errNotIP   = errors.New("must be an IP address, or empty")
 )
 
 // Params defines the dependencies for Config.
@@ -65,7 +71,8 @@ type Config struct {
 }
 
 // New loads configuration from env, .env files, and config
-// files, returning a fully resolved Config.
+// files, returning a fully resolved Config. It fails, with an error
+// naming the setting, on a value the server cannot use.
 func New(
 	_ fx.Lifecycle,
 	params Params,
@@ -103,15 +110,29 @@ func New(
 		}
 	}
 
+	// Read with strconv: viper's GetInt and GetBool would read a value
+	// they cannot parse as 0 or false instead of failing.
+	port, err := strconv.Atoi(viper.GetString("PORT"))
+	if err != nil || port < 1 || port > math.MaxUint16 {
+		return nil, fmt.Errorf("PORT %q: %w",
+			viper.GetString("PORT"), errNotPort)
+	}
+
+	debug, err := strconv.ParseBool(viper.GetString("DEBUG"))
+	if err != nil {
+		return nil, fmt.Errorf("DEBUG %q: %w",
+			viper.GetString("DEBUG"), errNotBool)
+	}
+
 	s := &Config{
 		BindAddress:        viper.GetString("BIND_ADDRESS"),
 		CORSAllowedOrigins: splitList(viper.GetString("CORS_ALLOWED_ORIGINS")),
 		DataDir:            viper.GetString("DATA_DIR"),
 		DataDirMaxBytes:    viper.GetInt64("DATA_DIR_MAX_BYTES"),
-		Debug:              viper.GetBool("DEBUG"),
+		Debug:              debug,
 		MetricsPassword:    viper.GetString("METRICS_PASSWORD"),
 		MetricsUsername:    viper.GetString("METRICS_USERNAME"),
-		Port:               viper.GetInt("PORT"),
+		Port:               port,
 		ReportsPerMinute:   viper.GetInt("REPORTS_PER_MINUTE"),
 		SentryDSN:          viper.GetString("SENTRY_DSN"),
 		TrustedProxies:     splitList(viper.GetString("TRUSTED_PROXIES")),
@@ -119,19 +140,7 @@ func New(
 		params:             &params,
 	}
 
-	// viper reads a value that is not a number as 0, so this also
-	// catches a mistyped setting.
-	if s.ReportsPerMinute <= 0 {
-		return nil, fmt.Errorf("REPORTS_PER_MINUTE %q: %w",
-			viper.GetString("REPORTS_PER_MINUTE"), errNotPositive)
-	}
-
-	if s.DataDirMaxBytes <= 0 {
-		return nil, fmt.Errorf("DATA_DIR_MAX_BYTES %q: %w",
-			viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive)
-	}
-
-	err = checkOrigins(s.CORSAllowedOrigins)
+	err = s.check()
 	if err != nil {
 		return nil, err
 	}
@@ -144,6 +153,32 @@ func New(
 	return s, nil
 }
 
+// check fails with an error naming the first setting here whose value
+// the server cannot use. New checks PORT and DEBUG as it reads them,
+// and the middleware checks TRUSTED_PROXIES as it parses it.
+func (s *Config) check() error {
+	// viper reads a value that is not a number as 0, so this also
+	// catches a mistyped setting.
+	if s.ReportsPerMinute <= 0 {
+		return fmt.Errorf("REPORTS_PER_MINUTE %q: %w",
+			viper.GetString("REPORTS_PER_MINUTE"), errNotPositive)
+	}
+
+	if s.DataDirMaxBytes <= 0 {
+		return fmt.Errorf("DATA_DIR_MAX_BYTES %q: %w",
+			viper.GetString("DATA_DIR_MAX_BYTES"), errNotPositive)
+	}
+
+	if s.BindAddress != "" {
+		_, err := netip.ParseAddr(s.BindAddress)
+		if err != nil {
+			return fmt.Errorf("BIND_ADDRESS %q: %w", s.BindAddress, errNotIP)
+		}
+	}
+
+	return checkOrigins(s.CORSAllowedOrigins)
+}
+
 // checkOrigins fails on the first CORS_ALLOWED_ORIGINS entry that is
 // not a plain origin, scheme://host with an optional port, as browsers
 // send it; anything more, such as a trailing "/", would match no page.

backend/internal/config/config_test.go

@@ -29,6 +29,63 @@ func requireConfigError(t *testing.T, setting string) {
 	}
 }
 
+// TestSettingsLoadAsGiven: valid values pass the checks and are used
+// as given. bin/entrypoint.sh starts the server with these
+// BIND_ADDRESS and PORT values.
+func TestSettingsLoadAsGiven(t *testing.T) {
+	t.Setenv("BIND_ADDRESS", "127.0.0.1")
+	t.Setenv("PORT", "8081")
+	t.Setenv("DEBUG", "true")
+
+	var cfg *config.Config
+
+	app := fx.New(
+		fx.NopLogger,
+		fx.Provide(globals.New, logger.New, config.New),
+		fx.Populate(&cfg),
+	)
+
+	err := app.Err()
+	if err != nil {
+		t.Fatalf("config error = %v", err)
+	}
+
+	if cfg.BindAddress != "127.0.0.1" || cfg.Port != 8081 || !cfg.Debug {
+		t.Fatalf("BindAddress, Port, Debug = %q, %d, %t; "+
+			"want \"127.0.0.1\", 8081, true",
+			cfg.BindAddress, cfg.Port, cfg.Debug)
+	}
+}
+
+// TestPortMustBeAPortNumber: viper reads a value that is not a number
+// as 0, on which the server would listen on a random port.
+func TestPortMustBeAPortNumber(t *testing.T) {
+	for _, value := range []string{"abc", "0", "65536", "8080.5"} {
+		t.Run(value, func(t *testing.T) {
+			t.Setenv("PORT", value)
+
+			requireConfigError(t, "PORT")
+		})
+	}
+}
+
+// TestDebugMustBeTrueOrFalse: viper reads any other value, such as
+// "yes", as false.
+func TestDebugMustBeTrueOrFalse(t *testing.T) {
+	t.Setenv("DEBUG", "yes")
+
+	requireConfigError(t, "DEBUG")
+}
+
+// TestBindAddressMustBeAnIPAddress: a host name would be looked up
+// only once the server starts listening, and a mistyped one would stop
+// it then with an error that does not name the setting.
+func TestBindAddressMustBeAnIPAddress(t *testing.T) {
+	t.Setenv("BIND_ADDRESS", "localhost")
+
+	requireConfigError(t, "BIND_ADDRESS")
+}
+
 // TestReportsPerMinuteMustBePositive: unchecked, zero would panic
 // when the routes are built, and a negative rate would lift the
 // limit.

backend/internal/middleware/middleware.go

@@ -77,8 +77,8 @@ func New(
 	return s, nil
 }
 
-// parseTrustedProxies converts CIDR strings into prefixes,
-// failing fast on any malformed entry.
+// parseTrustedProxies converts the TRUSTED_PROXIES entries into
+// prefixes, failing fast on any malformed entry.
 func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
 	prefixes := make([]netip.Prefix, 0, len(cidrs))
 
@@ -86,7 +86,7 @@ func parseTrustedProxies(cidrs []string) ([]netip.Prefix, error) {
 		prefix, err := netip.ParsePrefix(cidr)
 		if err != nil {
 			return nil, fmt.Errorf(
-				"trusted proxy %q: %w", cidr, err,
+				"TRUSTED_PROXIES %q: %w", cidr, err,
 			)
 		}
 

backend/internal/middleware/middleware_test.go

@@ -40,8 +40,8 @@ func TestParseTrustedProxiesRejectsMalformed(t *testing.T) {
 	t.Parallel()
 
 	_, err := middleware.ParseTrustedProxies([]string{"not-a-cidr"})
-	if err == nil {
-		t.Fatal("expected error for malformed CIDR, got nil")
+	if err == nil || !strings.Contains(err.Error(), "TRUSTED_PROXIES") {
+		t.Fatalf("error = %v, want one naming TRUSTED_PROXIES", err)
 	}
 }