From 68200aaac903d9e02c50a64ee6c55f600fdbf3f6 Mon Sep 17 00:00:00 2001 From: Atharva0506 Date: Thu, 20 Aug 2026 12:48:41 +0530 Subject: [PATCH 1/2] fix: load config.yaml from a configurable path main.go loaded configuration with the relative path "config.yaml". The runtime stage of the Dockerfile sets no WORKDIR, so the process ran from / and looked for /config.yaml, while the Dockerfile copies the file to /etc/relay/config.yaml. config.Load treats a missing file as "use defaults", so this failed silently: containerised deployments ran on hardcoded defaults and any edit to config.yaml was ignored. It was invisible only because the defaults happened to match the shipped file. Resolve the path from RELAY_CONFIG_PATH, defaulting to config.yaml so a source checkout is unaffected, and set it to /etc/relay/config.yaml in the image. Operators can now mount their own file over that path. Config gains a Source field recording which file was actually read, so startup logs the resolved path -- and warns explicitly when no file was found and defaults are in use. The silence was what made this hard to notice. Closes #27 --- Dockerfile | 5 ++ README.md | 20 +++++ cmd/relay/main.go | 27 ++++++- internal/config/config.go | 9 ++- internal/config/load_test.go | 144 +++++++++++++++++++++++++++++++++++ 5 files changed, 201 insertions(+), 4 deletions(-) create mode 100644 internal/config/load_test.go diff --git a/Dockerfile b/Dockerfile index 3485caa..aaafe40 100644 --- a/Dockerfile +++ b/Dockerfile @@ -39,6 +39,11 @@ RUN mkdir -p /data && chown -R relay:relay /data # Set default storage path to the mounted volume ENV RELAY_STORAGE_PATH=/data/relay.db +# Point the binary at the config file copied in above. Without this the +# process starts from / and silently falls back to built-in defaults, +# ignoring the bundled config.yaml entirely. +ENV RELAY_CONFIG_PATH=/etc/relay/config.yaml + USER relay EXPOSE 3000 diff --git a/README.md b/README.md index a263a55..2bec44e 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,25 @@ The server starts on `http://localhost:3000` with a SQLite database that auto-cr docker compose up -d ``` +ThruBox reads `config.yaml` from the working directory. Set `RELAY_CONFIG_PATH` +to load it from anywhere else: + +```bash +RELAY_CONFIG_PATH=/etc/relay/config.yaml ./relay-server +``` + +The image sets this for you and ships the file at `/etc/relay/config.yaml`, so +you can mount your own over it: + +```bash +docker run -v ./my-config.yaml:/etc/relay/config.yaml:ro ghcr.io/aossie-org/thrubox-server +``` + +If no config file is found at the resolved path the server logs a warning at +startup and runs on built-in defaults rather than failing. Precedence is +**built-in defaults → `config.yaml` → environment variables**, so an +environment variable always wins over the file. + #### 5. Run Tests ```bash @@ -196,6 +215,7 @@ Edit `config.yaml` or use environment variables: | Setting | YAML Key | Env Variable | Default | |---------|----------|-------------|---------| +| Config file path | — | `RELAY_CONFIG_PATH` | `config.yaml` (`/etc/relay/config.yaml` in the Docker image) | | Server port | `server.port` | `RELAY_SERVER_PORT` | `3000` | | Server host | `server.host` | `RELAY_SERVER_HOST` | `0.0.0.0` | | Storage path | `storage.path` | `RELAY_STORAGE_PATH` | `./data/relay.db` | diff --git a/cmd/relay/main.go b/cmd/relay/main.go index 8801bef..3e36734 100644 --- a/cmd/relay/main.go +++ b/cmd/relay/main.go @@ -15,20 +15,41 @@ import ( "github.com/AOSSIE-Org/ThruBox-Server/internal/store" ) +// defaultConfigPath is the config file location used when RELAY_CONFIG_PATH +// is not set. It is relative, so it resolves against the working directory. +const defaultConfigPath = "config.yaml" + func main() { // Set up structured logging slog.SetDefault(slog.New(slog.NewTextHandler(os.Stdout, &slog.HandlerOptions{ Level: slog.LevelInfo, }))) - // Load configuration - cfg, err := config.Load("config.yaml") + // Load configuration. The path is overridable so the same binary works + // from a source checkout (./config.yaml) and from the container image, + // where the file lives at /etc/relay/config.yaml. + configPath := os.Getenv("RELAY_CONFIG_PATH") + if configPath == "" { + configPath = defaultConfigPath + } + + cfg, err := config.Load(configPath) if err != nil { - slog.Error("failed to load configuration", "error", err) + slog.Error("failed to load configuration", "error", err, "config_file", configPath) os.Exit(1) } + configSource := cfg.Source + if configSource == "" { + configSource = "(none)" + slog.Warn("no config file found, falling back to built-in defaults", + "looked_for", configPath, + "hint", "set RELAY_CONFIG_PATH to point at your config.yaml", + ) + } + slog.Info("configuration loaded", + "config_file", configSource, "port", cfg.Server.Port, "storage_driver", cfg.Storage.Driver, "storage_path", cfg.Storage.Path, diff --git a/internal/config/config.go b/internal/config/config.go index 46afad3..8119be7 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -15,6 +15,11 @@ type Config struct { Storage StorageConfig `yaml:"storage"` Messages MessageConfig `yaml:"messages"` Security SecurityConfig `yaml:"security"` + + // Source is the path of the YAML file this config was read from, or the + // empty string when no file was found and the built-in defaults are in + // use. It is populated by Load and is never read from the YAML itself. + Source string `yaml:"-"` } // ServerConfig holds HTTP server settings. @@ -73,7 +78,8 @@ func Load(path string) (*Config, error) { data, err := os.ReadFile(path) if err != nil { if os.IsNotExist(err) { - // No config file — use defaults + env overrides + // No config file — use defaults + env overrides. + // Source stays empty so the caller can report that fact. applyEnvOverrides(cfg) if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("invalid default configuration: %w", err) @@ -87,6 +93,7 @@ func Load(path string) (*Config, error) { return nil, fmt.Errorf("parsing config file: %w", err) } + cfg.Source = path applyEnvOverrides(cfg) if err := cfg.Validate(); err != nil { return nil, fmt.Errorf("invalid configuration: %w", err) diff --git a/internal/config/load_test.go b/internal/config/load_test.go new file mode 100644 index 0000000..051aa3b --- /dev/null +++ b/internal/config/load_test.go @@ -0,0 +1,144 @@ +package config + +import ( + "os" + "path/filepath" + "testing" +) + +// clearEnvOverrides neutralises the environment so a test observes only the +// file and the defaults. Empty is equivalent to unset for applyEnvOverrides, +// and t.Setenv restores whatever the developer's shell had. +func clearEnvOverrides(t *testing.T) { + t.Helper() + for _, k := range []string{ + "RELAY_SERVER_PORT", + "PORT", + "RELAY_SERVER_HOST", + "RELAY_STORAGE_DRIVER", + "RELAY_STORAGE_PATH", + "RELAY_MESSAGES_TTL_DAYS", + "RELAY_MESSAGES_MAX_PAYLOAD_SIZE", + "RELAY_SECURITY_RATE_LIMIT", + "RELAY_SECURITY_API_KEY", + } { + t.Setenv(k, "") + } +} + +// writeConfig drops a YAML file in a temp dir and returns its path. +func writeConfig(t *testing.T, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "config.yaml") + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("writing temp config: %v", err) + } + return path +} + +// TestLoad_ReadsFileAtGivenPath is the regression test for the bug: a config +// file sitting somewhere other than ./config.yaml must actually be applied. +func TestLoad_ReadsFileAtGivenPath(t *testing.T) { + clearEnvOverrides(t) + + path := writeConfig(t, "server:\n port: 8123\nmessages:\n ttl_days: 42\n") + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Server.Port != 8123 { + t.Errorf("Server.Port = %d, want 8123 (file value ignored)", cfg.Server.Port) + } + if cfg.Messages.TTLDays != 42 { + t.Errorf("Messages.TTLDays = %d, want 42 (file value ignored)", cfg.Messages.TTLDays) + } + if cfg.Source != path { + t.Errorf("Source = %q, want %q", cfg.Source, path) + } +} + +// TestLoad_MissingFileUsesDefaults keeps the documented behaviour: a missing +// file is not an error, it just means defaults. Source must be empty so the +// caller can say so out loud. +func TestLoad_MissingFileUsesDefaults(t *testing.T) { + clearEnvOverrides(t) + + path := filepath.Join(t.TempDir(), "does-not-exist.yaml") + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if cfg.Server.Port != 3000 { + t.Errorf("Server.Port = %d, want 3000", cfg.Server.Port) + } + if cfg.Source != "" { + t.Errorf("Source = %q, want empty string for a missing file", cfg.Source) + } +} + +// TestLoad_EnvOverridesFile pins the precedence order: +// defaults -> file -> environment. +func TestLoad_EnvOverridesFile(t *testing.T) { + clearEnvOverrides(t) + + path := writeConfig(t, "server:\n port: 8123\nsecurity:\n api_key: \"from-file\"\n") + + t.Setenv("RELAY_SERVER_PORT", "9999") + t.Setenv("RELAY_SECURITY_API_KEY", "from-env") + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Server.Port != 9999 { + t.Errorf("Server.Port = %d, want 9999 (env must beat file)", cfg.Server.Port) + } + if cfg.Security.APIKey != "from-env" { + t.Errorf("Security.APIKey = %q, want \"from-env\"", cfg.Security.APIKey) + } + if cfg.Source != path { + t.Errorf("Source = %q, want %q", cfg.Source, path) + } +} + +// TestLoad_PartialFileKeepsDefaults confirms unset keys are not zeroed. +func TestLoad_PartialFileKeepsDefaults(t *testing.T) { + clearEnvOverrides(t) + + path := writeConfig(t, "server:\n port: 8123\n") + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v", err) + } + if cfg.Server.Host != "0.0.0.0" { + t.Errorf("Server.Host = %q, want \"0.0.0.0\"", cfg.Server.Host) + } + if cfg.Storage.Driver != "sqlite" { + t.Errorf("Storage.Driver = %q, want \"sqlite\"", cfg.Storage.Driver) + } +} + +// TestLoad_InvalidYAMLIsAnError distinguishes "malformed" from "absent". +func TestLoad_InvalidYAMLIsAnError(t *testing.T) { + clearEnvOverrides(t) + + path := writeConfig(t, "server:\n\tport: [unbalanced\n") + + if _, err := Load(path); err == nil { + t.Fatal("Load() error = nil, want a parse error for malformed YAML") + } +} + +// TestLoad_InvalidValueIsAnError checks Validate still runs on file input. +func TestLoad_InvalidValueIsAnError(t *testing.T) { + clearEnvOverrides(t) + + path := writeConfig(t, "server:\n port: 70000\n") + + if _, err := Load(path); err == nil { + t.Fatal("Load() error = nil, want a validation error for port 70000") + } +} From d58a0f2a9f22db81e7dc22de4b9b42076193111a Mon Sep 17 00:00:00 2001 From: Atharva0506 Date: Thu, 20 Aug 2026 14:36:25 +0530 Subject: [PATCH 2/2] test: cover env overrides on the missing-config-file path Load calls applyEnvOverrides twice: once in the missing-file branch and once after a successful parse. Only the second was covered, so removing the first left every test green -- verified by deleting the call, at which point nothing failed. That path is not a corner case either. It is what a container started with no config mounted actually runs, which is precisely where the environment is the only source of configuration. Also correct the precedence line in the README. It named config.yaml three lines below the text explaining that RELAY_CONFIG_PATH can point anywhere; "the selected YAML file" is what actually happens. Addresses CodeRabbit review feedback on #35. --- README.md | 2 +- internal/config/load_test.go | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 2bec44e..d63b9b0 100644 --- a/README.md +++ b/README.md @@ -198,7 +198,7 @@ docker run -v ./my-config.yaml:/etc/relay/config.yaml:ro ghcr.io/aossie-org/thru If no config file is found at the resolved path the server logs a warning at startup and runs on built-in defaults rather than failing. Precedence is -**built-in defaults → `config.yaml` → environment variables**, so an +**built-in defaults → the selected YAML file → environment variables**, so an environment variable always wins over the file. #### 5. Run Tests diff --git a/internal/config/load_test.go b/internal/config/load_test.go index 051aa3b..d080302 100644 --- a/internal/config/load_test.go +++ b/internal/config/load_test.go @@ -78,6 +78,33 @@ func TestLoad_MissingFileUsesDefaults(t *testing.T) { } } +// TestLoad_MissingFileStillAppliesEnvOverrides covers the second, easy-to-miss +// applyEnvOverrides call site. Load has one in the missing-file branch and one +// after a successful parse, so "no file, but env vars set" is its own path -- +// and it is the normal case for a container started with no config mounted. +func TestLoad_MissingFileStillAppliesEnvOverrides(t *testing.T) { + clearEnvOverrides(t) + + t.Setenv("RELAY_SERVER_PORT", "8123") + t.Setenv("RELAY_SECURITY_API_KEY", "from-env") + + path := filepath.Join(t.TempDir(), "does-not-exist.yaml") + + cfg, err := Load(path) + if err != nil { + t.Fatalf("Load() error = %v, want nil", err) + } + if cfg.Server.Port != 8123 { + t.Errorf("Server.Port = %d, want 8123 (env ignored on the missing-file path)", cfg.Server.Port) + } + if cfg.Security.APIKey != "from-env" { + t.Errorf("Security.APIKey = %q, want \"from-env\"", cfg.Security.APIKey) + } + if cfg.Source != "" { + t.Errorf("Source = %q, want empty string for a missing file", cfg.Source) + } +} + // TestLoad_EnvOverridesFile pins the precedence order: // defaults -> file -> environment. func TestLoad_EnvOverridesFile(t *testing.T) {