From 93d41083cdaa73976cd5d03fa5d3b2ca3b94a93c Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 28 Sep 2026 09:32:50 +0000 Subject: [PATCH] test: every setting from its environment variable (closes #128) Tests, written before the change, for the PIXA_ variables and PORT: every key set from the environment with no config file, PORT over the file's port, a list variable replacing the file's list, an empty variable as a set value, invalid values aborting startup naming the variable, and the signing key and metrics password never printed. All fail until the change lands, except the check that a config file alone behaves as before. TestMain unsets PORT and every PIXA_ variable so the shell running the tests cannot change their result. Model: opus-5-5 --- internal/config/env_internal_test.go | 255 +++++++++++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 internal/config/env_internal_test.go diff --git a/internal/config/env_internal_test.go b/internal/config/env_internal_test.go new file mode 100644 index 0000000..ea1e361 --- /dev/null +++ b/internal/config/env_internal_test.go @@ -0,0 +1,255 @@ +package config + +import ( + "net/netip" + "os" + "reflect" + "slices" + "strings" + "testing" +) + +// TestMain unsets PORT and every PIXA_ environment variable before the +// tests run, so each test sees only the variables it sets itself, not +// whatever the shell running the tests exports. +func TestMain(m *testing.M) { + for _, entry := range os.Environ() { + name, _, _ := strings.Cut(entry, "=") + if name != "PORT" && !strings.HasPrefix(name, "PIXA_") { + continue + } + + err := os.Unsetenv(name) + if err != nil { + panic(err) + } + } + + m.Run() +} + +// wantStartupError fails the test unless err is a startup error that +// mentions every one of wants. +func wantStartupError(t *testing.T, err error, wants ...string) { + t.Helper() + + if err == nil { + t.Fatalf("want a startup error mentioning %q, got none", wants) + } + + t.Logf("got expected error: %v", err) + + for _, want := range wants { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not mention %q", err.Error(), want) + } + } +} + +// TestEnvironmentSetsEveryKey sets every key from its environment +// variable, with no config file at all: PORT for the port, and PIXA_ +// plus the key in upper case, "." written as "_", for every other key. +func TestEnvironmentSetsEveryKey(t *testing.T) { + t.Setenv("PIXA_DEBUG", "true") + t.Setenv("PIXA_MAINTENANCE_MODE", "1") + t.Setenv("PORT", "9090") + t.Setenv("PIXA_STATE_DIR", "/srv/pixa-env") + t.Setenv("PIXA_SENTRY_DSN", "https://abc123@sentry.example.com/42") + t.Setenv("PIXA_DB_URL", "file:/srv/pixa-env/other.sqlite3") + t.Setenv("PIXA_METRICS_USERNAME", "metricsuser") + t.Setenv("PIXA_METRICS_PASSWORD", "metricspass") + t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey) + t.Setenv("PIXA_ALLOWLIST_HOSTS", "s3.sneak.cloud,.example.com") + t.Setenv("PIXA_ALLOW_HTTP", "true") + t.Setenv("PIXA_UPSTREAM_CONNECTIONS_PER_HOST", "5") + t.Setenv("PIXA_CACHE_MAX_BYTES", "1024") + t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24") + t.Setenv("PIXA_TRUSTED_PROXIES", "192.0.2.0/24") + + c, err := newFromSmartConfig(nil) + if err != nil { + t.Fatalf("configuration from the environment alone should load: %v", err) + } + + want := Config{ + Debug: true, + MaintenanceMode: true, + Port: 9090, + StateDir: "/srv/pixa-env", + SentryDSN: "https://abc123@sentry.example.com/42", + DBURL: "file:/srv/pixa-env/other.sqlite3", + MetricsUsername: "metricsuser", + MetricsPassword: "metricspass", + SigningKey: validTestSigningKey, + AllowlistHosts: []string{testHostS3, ".example.com"}, + AllowHTTP: true, + UpstreamConnectionsPerHost: 5, + CacheMaxBytes: 1024, + cacheMaxBytesExplicit: true, + BlockedNetworks: []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")}, + TrustedProxies: []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")}, + } + + if !reflect.DeepEqual(*c, want) { + t.Errorf("config from the environment =\n%+v\nwant\n%+v", *c, want) + } +} + +// TestPortFromEnvironmentOverridesConfigFile checks that PORT wins over +// the port in the config file. +func TestPortFromEnvironmentOverridesConfigFile(t *testing.T) { + t.Setenv("PORT", "9090") + + c, err := configFromYAML(t, signingKeyLine+"port: 8080\n") + if err != nil { + t.Fatalf("PORT=9090 with port 8080 in the file should load: %v", err) + } + + if c.Port != 9090 { + t.Errorf("Port = %d, want 9090 from PORT, not 8080 from the file", c.Port) + } +} + +// TestInvalidPortFromEnvironmentAbortsStartup checks that a PORT that is +// not a number, or is outside the port range, aborts startup naming PORT +// and the value, even though the file's port is valid. +func TestInvalidPortFromEnvironmentAbortsStartup(t *testing.T) { + t.Setenv("PORT", "banana") + + _, err := configFromYAML(t, signingKeyLine+"port: 8080\n") + wantStartupError(t, err, "PORT", "banana") + + t.Setenv("PORT", "70000") + + _, err = configFromYAML(t, signingKeyLine+"port: 8080\n") + wantStartupError(t, err, "PORT", "70000") +} + +// TestListFromEnvironmentReplacesConfigFileList checks that a list +// variable replaces the file's list, split on commas with the spaces +// around each entry trimmed. +func TestListFromEnvironmentReplacesConfigFileList(t *testing.T) { + t.Setenv("PIXA_ALLOWLIST_HOSTS", " cdn.example.com , .example.org ") + + c, err := configFromYAML(t, signingKeyLine+ + "allowlist_hosts:\n - s3.sneak.cloud\n - sneak.berlin\n") + if err != nil { + t.Fatalf("PIXA_ALLOWLIST_HOSTS should load: %v", err) + } + + want := []string{"cdn.example.com", ".example.org"} + if !slices.Equal(c.AllowlistHosts, want) { + t.Errorf("AllowlistHosts = %v, want %v from PIXA_ALLOWLIST_HOSTS", + c.AllowlistHosts, want) + } +} + +// TestInvalidBlockedNetworksFromEnvironmentAbortsStartup checks that an +// invalid CIDR, or an empty entry, in PIXA_BLOCKED_NETWORKS aborts +// startup naming the variable, as the same list in the file does. +func TestInvalidBlockedNetworksFromEnvironmentAbortsStartup(t *testing.T) { + t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24,not-a-cidr") + + _, err := configFromYAML(t, signingKeyLine) + wantStartupError(t, err, "PIXA_BLOCKED_NETWORKS", "not-a-cidr") + + t.Setenv("PIXA_BLOCKED_NETWORKS", "203.0.113.0/24,,198.51.100.0/24") + + _, err = configFromYAML(t, signingKeyLine) + wantStartupError(t, err, "PIXA_BLOCKED_NETWORKS") +} + +// TestInvalidDebugFromEnvironmentAbortsStartup checks that a PIXA_DEBUG +// that strconv.ParseBool rejects aborts startup instead of defaulting. +func TestInvalidDebugFromEnvironmentAbortsStartup(t *testing.T) { + t.Setenv("PIXA_DEBUG", "maybe") + + _, err := configFromYAML(t, signingKeyLine) + wantStartupError(t, err, "PIXA_DEBUG", "maybe") +} + +// TestConfigFileAloneBehavesAsBefore checks that with no variables set +// (TestMain unsets them) the config file's values are used and omitted +// keys take their defaults. +func TestConfigFileAloneBehavesAsBefore(t *testing.T) { + t.Parallel() + + c, err := configFromYAML(t, signingKeyLine+"port: 9191\n") + if err != nil { + t.Fatalf("config file should load: %v", err) + } + + if c.Port != 9191 { + t.Errorf("Port = %d, want 9191 from the file", c.Port) + } + + if c.StateDir != DefaultStateDir { + t.Errorf("StateDir = %q, want default %q", c.StateDir, DefaultStateDir) + } + + if !slices.Equal(c.TrustedProxies, defaultTrustedProxies()) { + t.Errorf("TrustedProxies = %v, want default %v", + c.TrustedProxies, defaultTrustedProxies()) + } +} + +// TestEmptyTrustedProxiesFromEnvironmentTrustsNoOne checks that an empty +// PIXA_TRUSTED_PROXIES is an empty list, like [] in the file: it trusts +// no proxy instead of taking the default ranges. +func TestEmptyTrustedProxiesFromEnvironmentTrustsNoOne(t *testing.T) { + t.Setenv("PIXA_TRUSTED_PROXIES", "") + + c, err := configFromYAML(t, signingKeyLine) + if err != nil { + t.Fatalf("empty PIXA_TRUSTED_PROXIES should load: %v", err) + } + + if len(c.TrustedProxies) != 0 { + t.Errorf("TrustedProxies = %v, want none", c.TrustedProxies) + } +} + +// TestEmptyVariableDoesNotFallBackToConfigFile checks that a variable +// that is present but empty is a set value: an empty PIXA_STATE_DIR +// aborts startup like state_dir: "" in the file, instead of falling +// through to the file's state_dir. +func TestEmptyVariableDoesNotFallBackToConfigFile(t *testing.T) { + t.Setenv("PIXA_STATE_DIR", "") + + _, err := configFromYAML(t, signingKeyLine+"state_dir: /srv/pixa-file\n") + wantStartupError(t, err, "PIXA_STATE_DIR") +} + +// TestMissingSigningKeyNamesItsVariable checks that with no config file +// and no PIXA_SIGNING_KEY, startup aborts naming the variable, which is +// how a container started without it reports the problem. +func TestMissingSigningKeyNamesItsVariable(t *testing.T) { + t.Parallel() + + _, err := newFromSmartConfig(nil) + wantStartupError(t, err, "PIXA_SIGNING_KEY") +} + +// TestSecretsFromEnvironmentAreNotPrinted checks that errors about the +// signing key and the metrics password name their variables but never +// print their values. +func TestSecretsFromEnvironmentAreNotPrinted(t *testing.T) { + t.Setenv("PIXA_SIGNING_KEY", "short-signing-secret") + + _, err := newFromSmartConfig(nil) + wantStartupError(t, err, "PIXA_SIGNING_KEY") + + if strings.Contains(err.Error(), "short-signing-secret") { + t.Errorf("error %q prints the signing key", err.Error()) + } + + t.Setenv("PIXA_SIGNING_KEY", validTestSigningKey) + t.Setenv("PIXA_METRICS_PASSWORD", "metrics-password-secret") + + _, err = newFromSmartConfig(nil) + wantStartupError(t, err, "PIXA_METRICS_PASSWORD") + + if strings.Contains(err.Error(), "metrics-password-secret") { + t.Errorf("error %q prints the metrics password", err.Error()) + } +}