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..d63b9b0 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 → the selected YAML file → 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..d080302 --- /dev/null +++ b/internal/config/load_test.go @@ -0,0 +1,171 @@ +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_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) { + 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") + } +}