diff --git a/README.md b/README.md index 1574245..62259e0 100644 --- a/README.md +++ b/README.md @@ -219,7 +219,7 @@ image, `make run` builds it and runs it on port 8080. | `PORT` | `8080` | HTTP listen port | | `DATA_DIR` | `./data/reports` | Directory for compressed reports | | `DATA_DIR_MAX_BYTES` | `1073741824` (1 GiB) | Most bytes of report files kept in `DATA_DIR`, oldest deleted first; see [Report limits](#report-limits) | -| `DEBUG` | `false` | Enable debug logging | +| `DEBUG` | empty | Debug logging; any non-empty value, `false` and `0` included, turns it on | | `TRUSTED_PROXIES` | RFC 1918 ranges | Comma-separated CIDRs whose `X-Forwarded-For` / `X-Real-IP` headers are trusted for client IP resolution | | `REPORTS_PER_MINUTE` | `60` | Reports each client address may send a minute; see [Report limits](#report-limits) | | `CORS_ALLOWED_ORIGINS` | empty | Comma-separated origins whose pages may call the API; see [CORS](#cors) | @@ -242,10 +242,10 @@ an entry that is not an address at all, such as `unknown`, the direct peer is used. `X-Real-IP` is read only when `X-Forwarded-For` is missing or empty. The container image does not use this default; see [Deployment](#deployment). -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, -except `TRUSTED_PROXIES`. +A variable set to a value the server cannot use, such as `PORT=abc` 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, except +`TRUSTED_PROXIES`. No value of `DEBUG` stops the start. #### Report storage @@ -508,7 +508,8 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for netwatch needs: report files may take; the oldest are deleted to stay under it - `CORS_ALLOWED_ORIGINS`, default empty: other origins whose pages may call the API - - `DEBUG`, default `false`: debug logging + - `DEBUG`, default empty: debug logging; any non-empty value, `false` and + `0` included, turns it on - `DATA_DIR`, default `/data/reports`: the directory the reports are kept in: `/data` or a path below it, with no `.` or `..` part and no extra `/`. The container also stops if the path goes through a symbolic link that diff --git a/TODO.md b/TODO.md index 33acea5..fbec2d0 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,11 @@ Run `make frontend-viewport-test` in CI as its own step; it is not part of # Completed Steps +- 2026-10-19: `DEBUG` unset or empty leaves debug logging off, and any other + value, `false` and `0` included, turns it on, as the code style guide says + ([#159](https://git.eeqj.de/sneak/netwatch/issues/159)). No value of it stops + the start any more; only `true`, `false`, `1`, `0` and their short forms were + accepted before. `README.md` no longer gives `false` as its default - 2026-10-19: the three unit tests of a check whose result is discarded (its round given up, the user paused, or the first round) also check that the host's history is still empty, its status still `pending`, and nothing was diff --git a/internal/config/config.go b/internal/config/config.go index 69cca58..888afa1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -41,7 +41,6 @@ var ( "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") errMetricsCredentials = errors.New( @@ -101,7 +100,7 @@ func New( viper.SetDefault("CORS_ALLOWED_ORIGINS", "") viper.SetDefault("DATA_DIR", "./data/reports") viper.SetDefault("DATA_DIR_MAX_BYTES", defaultDataDirMaxBytes) - viper.SetDefault("DEBUG", "false") + viper.SetDefault("DEBUG", "") // An empty BIND_ADDRESS listens on every interface. viper.SetDefault("BIND_ADDRESS", "") viper.SetDefault("PORT", "8080") @@ -120,19 +119,17 @@ func New( } } - // Read with strconv: viper's GetInt and GetBool would read a value - // they cannot parse as 0 or false instead of failing. + // Read with strconv: viper's GetInt would read a value it cannot + // parse as 0 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) - } + // Any non-empty value turns debug logging on, "false" and "0" + // included, as the code style guide says. + debug := viper.GetString("DEBUG") != "" // An empty TRUSTED_PROXIES trusts no proxy, but viper reads an // empty environment variable as unset and would give the default, @@ -172,8 +169,8 @@ func New( } // 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. +// the server cannot use. New checks PORT as it reads it, 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. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 7de411a..43a15a5 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -127,12 +127,52 @@ func TestPortMustBeAPortNumber(t *testing.T) { } } -// TestDebugMustBeTrueOrFalse: viper reads any other value, such as -// "yes", as false. -func TestDebugMustBeTrueOrFalse(t *testing.T) { - t.Setenv("DEBUG", "yes") +// TestDebugIsOnForAnyNonEmptyValue: unset or empty, debug logging is +// off; any other value turns it on, "false" and "0" included, and none +// stops the start. +func TestDebugIsOnForAnyNonEmptyValue(t *testing.T) { + for _, tc := range []struct { + name string + value string + unset bool + want bool + }{ + {name: "unset", unset: true, want: false}, + {name: "empty", value: "", want: false}, + {name: "false", value: "false", want: true}, + {name: "0", value: "0", want: true}, + {name: "yes", value: "yes", want: true}, + } { + t.Run(tc.name, func(t *testing.T) { + // t.Setenv first, so the variable is put back as it was + // after the test. + t.Setenv("DEBUG", tc.value) - requireConfigError(t, "DEBUG") + if tc.unset { + err := os.Unsetenv("DEBUG") + if err != nil { + t.Fatal(err) + } + } + + 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.Debug != tc.want { + t.Fatalf("Debug = %t, want %t", cfg.Debug, tc.want) + } + }) + } } // TestBindAddressMustBeAnIPAddress: a host name would be looked up