From 9002e91d9187a356841bae49e685e6c365deaf5b Mon Sep 17 00:00:00 2001 From: senke Date: Thu, 16 Apr 2026 20:44:09 +0200 Subject: [PATCH] refactor(backend,infra): unify SMTP env schema on canonical SMTP_* names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third item of the v1.0.6 backlog. The v1.0.5.1 hotfix surfaced that two email paths in-tree read *different* env vars for the same configuration: internal/email/sender.go internal/services/email_service.go SMTP_USERNAME SMTP_USER SMTP_FROM FROM_EMAIL SMTP_FROM_NAME FROM_NAME The hotfix worked around it by exporting both sets in `.env.template`. This commit reconciles them onto a single schema so the workaround can go away. Changes * `internal/email/sender.go` is now the single loader. The canonical names (`SMTP_USERNAME`, `SMTP_FROM`, `SMTP_FROM_NAME`) are read first; the legacy names (`SMTP_USER`, `FROM_EMAIL`, `FROM_NAME`) stay supported as a migration fallback that logs a structured deprecation warning ("remove_in: v1.1.0"). Canonical always wins over deprecated — no silent precedence flip. * `NewSMTPEmailSender` callers keep working unchanged; a new `LoadSMTPConfigFromEnvWithLogger(*zap.Logger)` variant lets callers opt into the warning stream. * `internal/services/email_service.go` drops its six inline `os.Getenv` reads and delegates to the shared loader, so `AuthService.Register` and `RequestPasswordReset` now see exactly the same config as the async job worker. * `.env.template`: the duplicate (SMTP_USER + FROM_EMAIL + FROM_NAME) block added in v1.0.5.1 is removed — only the canonical SMTP_* names ship for new contributors. * `docker-compose.yml` (backend-api service): FROM_EMAIL / FROM_NAME renamed to SMTP_FROM / SMTP_FROM_NAME to match the canonical schema. * No Host/Port default injected in the loader. If SMTP_HOST is empty, callers see Host=="" and log-only (historic dev behavior). Dev defaults (MailHog localhost:1025) live in `.env.template`, so a fresh clone still works; a misconfigured prod pod fails loud instead of silently dialing localhost. Tests * 5 new Go tests in `internal/email/smtp_env_test.go`: empty-env returns empty config; canonical names read directly; deprecated names fall back (one warning per var); canonical wins over deprecated silently; nil logger is allowed. * Existing `TestLoadSMTPConfigFromEnv`, `TestSMTPEmailSender_Send`, and every auth/services package remained green (40+ packages). Import-cycle note: the loader deliberately lives in `internal/email`, not `internal/config`, because `internal/config` already depends on `internal/email` (wiring `EmailSender` at boot). Putting the loader in `email` keeps the dependency flow one-way. Co-Authored-By: Claude Opus 4.7 (1M context) --- docker-compose.yml | 4 +- veza-backend-api/.env.template | 10 +- veza-backend-api/internal/email/sender.go | 73 +++++++++---- .../internal/email/smtp_env_test.go | 103 ++++++++++++++++++ .../internal/services/email_service.go | 19 ++-- 5 files changed, 171 insertions(+), 38 deletions(-) create mode 100644 veza-backend-api/internal/email/smtp_env_test.go diff --git a/docker-compose.yml b/docker-compose.yml index d3d1f1a64..edc83911c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -211,8 +211,8 @@ services: - HLS_STORAGE_DIR=/data/hls - SMTP_HOST=mailhog - SMTP_PORT=1025 - - FROM_EMAIL=${FROM_EMAIL:-no-reply@veza.local} - - FROM_NAME=${FROM_NAME:-Veza (dev)} + - SMTP_FROM=${SMTP_FROM:-no-reply@veza.local} + - SMTP_FROM_NAME=${SMTP_FROM_NAME:-Veza (dev)} volumes: - hls-data:/data/hls ports: diff --git a/veza-backend-api/.env.template b/veza-backend-api/.env.template index 58e3765f0..3f633c388 100644 --- a/veza-backend-api/.env.template +++ b/veza-backend-api/.env.template @@ -119,17 +119,13 @@ BASE_URL=http://veza.fr:8080 # to inspect captured emails. # Production: override with your SMTP provider (SendGrid, AWS SES, etc.). # -# TODO(v1.0.6): internal/services/email_service.go and internal/email/sender.go -# read different variable names for the same fields (`SMTP_USER` vs -# `SMTP_USERNAME`, `FROM_EMAIL` vs `SMTP_FROM`, `FROM_NAME` vs `SMTP_FROM_NAME`). -# Both sets are exported here until the two services are reconciled. +# v1.0.6: unified on canonical SMTP_* names. The legacy SMTP_USER / +# FROM_EMAIL / FROM_NAME are still accepted as a deprecation fallback +# (backend logs a warning on use; removed in v1.1.0). SMTP_HOST=localhost SMTP_PORT=1025 -SMTP_USER= SMTP_USERNAME= SMTP_PASSWORD= -FROM_EMAIL=no-reply@veza.local -FROM_NAME=Veza (dev) SMTP_FROM=no-reply@veza.local SMTP_FROM_NAME=Veza (dev) diff --git a/veza-backend-api/internal/email/sender.go b/veza-backend-api/internal/email/sender.go index 74b873662..abc4463c4 100644 --- a/veza-backend-api/internal/email/sender.go +++ b/veza-backend-api/internal/email/sender.go @@ -90,30 +90,59 @@ func (s *SMTPEmailSender) SendTemplate(to, template string, data map[string]inte return fmt.Errorf("SendTemplate not implemented directly, use EmailJob instead") } -// LoadSMTPConfigFromEnv charge la config SMTP depuis les variables d'environnement -func LoadSMTPConfigFromEnv() SMTPConfig { - // En dev, fallback sur MailHog si pas de config - host := os.Getenv("SMTP_HOST") - port := os.Getenv("SMTP_PORT") - if host == "" { - host = os.Getenv("MAILHOG_HOST") - if host == "" { - host = "localhost" - } - } - if port == "" { - port = os.Getenv("MAILHOG_PORT") - if port == "" { - port = "1025" // MailHog default - } - } +// v1.0.6 — single SMTP env schema. Before this commit, two services read +// different env names for the same fields: +// +// internal/email/sender.go internal/services/email_service.go +// SMTP_USERNAME SMTP_USER +// SMTP_FROM FROM_EMAIL +// SMTP_FROM_NAME FROM_NAME +// +// Both now consume LoadSMTPConfigFromEnv. The canonical names are SMTP_* +// everywhere. The old names are still read as a deprecation fallback with +// a one-line warning so existing deployments don't break mid-rollout — to +// be removed in v1.1.0. +// LoadSMTPConfigFromEnv reads the canonical SMTP_* env vars. +// No defaults are injected: Host/Port come straight from env. A caller +// seeing Host=="" must treat SMTP as unconfigured and log-only (matches +// the historic dev behavior). Dev defaults (MailHog on localhost:1025) +// live in `veza-backend-api/.env.template` — this keeps the runtime +// honest and lets prod fail fast on a missing env. +func LoadSMTPConfigFromEnv() SMTPConfig { + return LoadSMTPConfigFromEnvWithLogger(nil) +} + +// LoadSMTPConfigFromEnvWithLogger is the logger-aware variant — pass your +// package logger so deprecation warnings land in structured logs. +func LoadSMTPConfigFromEnvWithLogger(logger *zap.Logger) SMTPConfig { return SMTPConfig{ - Host: host, - Port: port, - Username: os.Getenv("SMTP_USERNAME"), + Host: os.Getenv("SMTP_HOST"), + Port: os.Getenv("SMTP_PORT"), + Username: resolveDeprecated(logger, "SMTP_USERNAME", "SMTP_USER"), Password: os.Getenv("SMTP_PASSWORD"), - From: os.Getenv("SMTP_FROM"), - FromName: os.Getenv("SMTP_FROM_NAME"), + From: resolveDeprecated(logger, "SMTP_FROM", "FROM_EMAIL"), + FromName: resolveDeprecated(logger, "SMTP_FROM_NAME", "FROM_NAME"), } } + +// resolveDeprecated returns the value of `canonical` if set, otherwise +// falls back to `deprecated` (emitting a warning). Empty string if neither +// is set. Exported at package scope so it's unit-testable in isolation. +func resolveDeprecated(logger *zap.Logger, canonical, deprecated string) string { + if v := os.Getenv(canonical); v != "" { + return v + } + if v := os.Getenv(deprecated); v != "" { + if logger != nil { + logger.Warn( + "Deprecated SMTP env var in use — migrate to the canonical name", + zap.String("deprecated", deprecated), + zap.String("canonical", canonical), + zap.String("remove_in", "v1.1.0"), + ) + } + return v + } + return "" +} diff --git a/veza-backend-api/internal/email/smtp_env_test.go b/veza-backend-api/internal/email/smtp_env_test.go new file mode 100644 index 000000000..113bece0f --- /dev/null +++ b/veza-backend-api/internal/email/smtp_env_test.go @@ -0,0 +1,103 @@ +package email + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "go.uber.org/zap" + "go.uber.org/zap/zapcore" + "go.uber.org/zap/zaptest/observer" +) + +// clearAllSMTPEnv zeroes every SMTP-related env var (canonical + deprecated) +// so each test starts from a known blank slate. t.Setenv guarantees restore. +func clearAllSMTPEnv(t *testing.T) { + t.Helper() + for _, k := range []string{ + "SMTP_HOST", "SMTP_PORT", "SMTP_PASSWORD", + "SMTP_USERNAME", "SMTP_FROM", "SMTP_FROM_NAME", + "SMTP_USER", "FROM_EMAIL", "FROM_NAME", + } { + t.Setenv(k, "") + } +} + +func TestLoadSMTPConfig_EmptyWhenNothingSet(t *testing.T) { + clearAllSMTPEnv(t) + cfg := LoadSMTPConfigFromEnv() + assert.Empty(t, cfg.Host, "Host must stay empty so callers can log-only") + assert.Empty(t, cfg.Port) + assert.Empty(t, cfg.Username) + assert.Empty(t, cfg.From) + assert.Empty(t, cfg.FromName) +} + +func TestLoadSMTPConfig_CanonicalNamesReadDirectly(t *testing.T) { + clearAllSMTPEnv(t) + t.Setenv("SMTP_HOST", "mailhog") + t.Setenv("SMTP_PORT", "1025") + t.Setenv("SMTP_USERNAME", "veza-api") + t.Setenv("SMTP_PASSWORD", "supersecret") + t.Setenv("SMTP_FROM", "no-reply@veza.fm") + t.Setenv("SMTP_FROM_NAME", "Veza") + + cfg := LoadSMTPConfigFromEnv() + assert.Equal(t, "mailhog", cfg.Host) + assert.Equal(t, "1025", cfg.Port) + assert.Equal(t, "veza-api", cfg.Username) + assert.Equal(t, "supersecret", cfg.Password) + assert.Equal(t, "no-reply@veza.fm", cfg.From) + assert.Equal(t, "Veza", cfg.FromName) +} + +func TestLoadSMTPConfig_DeprecatedNamesFallBack(t *testing.T) { + clearAllSMTPEnv(t) + // Only the pre-v1.0.6 env vars are set. + t.Setenv("SMTP_USER", "legacy-user") + t.Setenv("FROM_EMAIL", "legacy@veza.fm") + t.Setenv("FROM_NAME", "Legacy Veza") + + core, observed := observer.New(zapcore.WarnLevel) + cfg := LoadSMTPConfigFromEnvWithLogger(zap.New(core)) + + assert.Equal(t, "legacy-user", cfg.Username, "fallback must pick up SMTP_USER when SMTP_USERNAME is empty") + assert.Equal(t, "legacy@veza.fm", cfg.From) + assert.Equal(t, "Legacy Veza", cfg.FromName) + + entries := observed.All() + assert.Len(t, entries, 3, "one warning per deprecated var in use") + seen := map[string]bool{} + for _, e := range entries { + for _, f := range e.Context { + if f.Key == "deprecated" { + seen[f.String] = true + } + } + } + assert.True(t, seen["SMTP_USER"]) + assert.True(t, seen["FROM_EMAIL"]) + assert.True(t, seen["FROM_NAME"]) +} + +func TestLoadSMTPConfig_CanonicalWinsOverDeprecated(t *testing.T) { + clearAllSMTPEnv(t) + t.Setenv("SMTP_USERNAME", "canonical-user") + t.Setenv("SMTP_USER", "legacy-user-ignored") + t.Setenv("SMTP_FROM", "canonical@veza.fm") + t.Setenv("FROM_EMAIL", "legacy@veza.fm") + + core, observed := observer.New(zapcore.WarnLevel) + cfg := LoadSMTPConfigFromEnvWithLogger(zap.New(core)) + assert.Equal(t, "canonical-user", cfg.Username) + assert.Equal(t, "canonical@veza.fm", cfg.From) + assert.Equal(t, 0, observed.Len(), + "no warning should fire when canonical is set — deprecated vars are ignored silently") +} + +func TestLoadSMTPConfig_NilLoggerIsAllowed(t *testing.T) { + clearAllSMTPEnv(t) + t.Setenv("SMTP_USER", "legacy") + assert.NotPanics(t, func() { _ = LoadSMTPConfigFromEnv() }) + cfg := LoadSMTPConfigFromEnv() + assert.Equal(t, "legacy", cfg.Username) +} diff --git a/veza-backend-api/internal/services/email_service.go b/veza-backend-api/internal/services/email_service.go index c6ba7fb95..323574932 100644 --- a/veza-backend-api/internal/services/email_service.go +++ b/veza-backend-api/internal/services/email_service.go @@ -15,6 +15,7 @@ import ( "github.com/google/uuid" "veza-backend-api/internal/database" + "veza-backend-api/internal/email" "go.uber.org/zap" ) @@ -31,17 +32,21 @@ type EmailService struct { fromName string } -// NewEmailService creates a new email service +// NewEmailService creates a new email service. +// v1.0.6: config loaded via email.LoadSMTPConfigFromEnvWithLogger, the single +// source of truth for the canonical SMTP_* schema (with deprecation fallback +// to SMTP_USER / FROM_EMAIL / FROM_NAME). func NewEmailService(db *database.Database, logger *zap.Logger) *EmailService { + cfg := email.LoadSMTPConfigFromEnvWithLogger(logger) return &EmailService{ db: db, logger: logger, - smtpHost: os.Getenv("SMTP_HOST"), - smtpPort: os.Getenv("SMTP_PORT"), - smtpUser: os.Getenv("SMTP_USER"), - smtpPass: os.Getenv("SMTP_PASSWORD"), - fromEmail: os.Getenv("FROM_EMAIL"), - fromName: os.Getenv("FROM_NAME"), + smtpHost: cfg.Host, + smtpPort: cfg.Port, + smtpUser: cfg.Username, + smtpPass: cfg.Password, + fromEmail: cfg.From, + fromName: cfg.FromName, } }