refactor(backend,infra): unify SMTP env schema on canonical SMTP_* names
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) <noreply@anthropic.com>
This commit is contained in:
parent
7974517c03
commit
9002e91d91
5 changed files with 171 additions and 38 deletions
|
|
@ -211,8 +211,8 @@ services:
|
||||||
- HLS_STORAGE_DIR=/data/hls
|
- HLS_STORAGE_DIR=/data/hls
|
||||||
- SMTP_HOST=mailhog
|
- SMTP_HOST=mailhog
|
||||||
- SMTP_PORT=1025
|
- SMTP_PORT=1025
|
||||||
- FROM_EMAIL=${FROM_EMAIL:-no-reply@veza.local}
|
- SMTP_FROM=${SMTP_FROM:-no-reply@veza.local}
|
||||||
- FROM_NAME=${FROM_NAME:-Veza (dev)}
|
- SMTP_FROM_NAME=${SMTP_FROM_NAME:-Veza (dev)}
|
||||||
volumes:
|
volumes:
|
||||||
- hls-data:/data/hls
|
- hls-data:/data/hls
|
||||||
ports:
|
ports:
|
||||||
|
|
|
||||||
|
|
@ -119,17 +119,13 @@ BASE_URL=http://veza.fr:8080
|
||||||
# to inspect captured emails.
|
# to inspect captured emails.
|
||||||
# Production: override with your SMTP provider (SendGrid, AWS SES, etc.).
|
# Production: override with your SMTP provider (SendGrid, AWS SES, etc.).
|
||||||
#
|
#
|
||||||
# TODO(v1.0.6): internal/services/email_service.go and internal/email/sender.go
|
# v1.0.6: unified on canonical SMTP_* names. The legacy SMTP_USER /
|
||||||
# read different variable names for the same fields (`SMTP_USER` vs
|
# FROM_EMAIL / FROM_NAME are still accepted as a deprecation fallback
|
||||||
# `SMTP_USERNAME`, `FROM_EMAIL` vs `SMTP_FROM`, `FROM_NAME` vs `SMTP_FROM_NAME`).
|
# (backend logs a warning on use; removed in v1.1.0).
|
||||||
# Both sets are exported here until the two services are reconciled.
|
|
||||||
SMTP_HOST=localhost
|
SMTP_HOST=localhost
|
||||||
SMTP_PORT=1025
|
SMTP_PORT=1025
|
||||||
SMTP_USER=
|
|
||||||
SMTP_USERNAME=
|
SMTP_USERNAME=
|
||||||
SMTP_PASSWORD=
|
SMTP_PASSWORD=
|
||||||
FROM_EMAIL=no-reply@veza.local
|
|
||||||
FROM_NAME=Veza (dev)
|
|
||||||
SMTP_FROM=no-reply@veza.local
|
SMTP_FROM=no-reply@veza.local
|
||||||
SMTP_FROM_NAME=Veza (dev)
|
SMTP_FROM_NAME=Veza (dev)
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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")
|
return fmt.Errorf("SendTemplate not implemented directly, use EmailJob instead")
|
||||||
}
|
}
|
||||||
|
|
||||||
// LoadSMTPConfigFromEnv charge la config SMTP depuis les variables d'environnement
|
// v1.0.6 — single SMTP env schema. Before this commit, two services read
|
||||||
func LoadSMTPConfigFromEnv() SMTPConfig {
|
// different env names for the same fields:
|
||||||
// En dev, fallback sur MailHog si pas de config
|
//
|
||||||
host := os.Getenv("SMTP_HOST")
|
// internal/email/sender.go internal/services/email_service.go
|
||||||
port := os.Getenv("SMTP_PORT")
|
// SMTP_USERNAME SMTP_USER
|
||||||
if host == "" {
|
// SMTP_FROM FROM_EMAIL
|
||||||
host = os.Getenv("MAILHOG_HOST")
|
// SMTP_FROM_NAME FROM_NAME
|
||||||
if host == "" {
|
//
|
||||||
host = "localhost"
|
// 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
|
||||||
if port == "" {
|
// be removed in v1.1.0.
|
||||||
port = os.Getenv("MAILHOG_PORT")
|
|
||||||
if port == "" {
|
|
||||||
port = "1025" // MailHog default
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
|
// 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{
|
return SMTPConfig{
|
||||||
Host: host,
|
Host: os.Getenv("SMTP_HOST"),
|
||||||
Port: port,
|
Port: os.Getenv("SMTP_PORT"),
|
||||||
Username: os.Getenv("SMTP_USERNAME"),
|
Username: resolveDeprecated(logger, "SMTP_USERNAME", "SMTP_USER"),
|
||||||
Password: os.Getenv("SMTP_PASSWORD"),
|
Password: os.Getenv("SMTP_PASSWORD"),
|
||||||
From: os.Getenv("SMTP_FROM"),
|
From: resolveDeprecated(logger, "SMTP_FROM", "FROM_EMAIL"),
|
||||||
FromName: os.Getenv("SMTP_FROM_NAME"),
|
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 ""
|
||||||
|
}
|
||||||
|
|
|
||||||
103
veza-backend-api/internal/email/smtp_env_test.go
Normal file
103
veza-backend-api/internal/email/smtp_env_test.go
Normal file
|
|
@ -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)
|
||||||
|
}
|
||||||
|
|
@ -15,6 +15,7 @@ import (
|
||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
|
|
||||||
"veza-backend-api/internal/database"
|
"veza-backend-api/internal/database"
|
||||||
|
"veza-backend-api/internal/email"
|
||||||
|
|
||||||
"go.uber.org/zap"
|
"go.uber.org/zap"
|
||||||
)
|
)
|
||||||
|
|
@ -31,17 +32,21 @@ type EmailService struct {
|
||||||
fromName string
|
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 {
|
func NewEmailService(db *database.Database, logger *zap.Logger) *EmailService {
|
||||||
|
cfg := email.LoadSMTPConfigFromEnvWithLogger(logger)
|
||||||
return &EmailService{
|
return &EmailService{
|
||||||
db: db,
|
db: db,
|
||||||
logger: logger,
|
logger: logger,
|
||||||
smtpHost: os.Getenv("SMTP_HOST"),
|
smtpHost: cfg.Host,
|
||||||
smtpPort: os.Getenv("SMTP_PORT"),
|
smtpPort: cfg.Port,
|
||||||
smtpUser: os.Getenv("SMTP_USER"),
|
smtpUser: cfg.Username,
|
||||||
smtpPass: os.Getenv("SMTP_PASSWORD"),
|
smtpPass: cfg.Password,
|
||||||
fromEmail: os.Getenv("FROM_EMAIL"),
|
fromEmail: cfg.From,
|
||||||
fromName: os.Getenv("FROM_NAME"),
|
fromName: cfg.FromName,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue