Discharge7443/fleet
1
0
Fork 0
Code Issues Pull Requests Actions 5 Packages Projects Releases Wiki Activity

Extract geoIP and mail service initialization out of runServeCmd (#47151)

Browse Source 
Extracts the geoIP provider and mail service setup out of `runServeCmd`
and into new `cmd/fleet/geoip.go` and `cmd/fleet/mail.go`. Same pattern
as the prior extractions on this issue (#44929, #45343, #45583, #46166,
#46421, #46517, #46742, #46830, #46893). Both are best-effort startup
providers — they log and fall back rather than aborting boot — so they
group naturally.

Functions:

- `initGeoIP` — returns the GeoIP provider. When no database path is
configured, or the MaxMind database fails to load, it returns a no-op
provider and logs rather than aborting startup.
- `initMailService` — configures the mail service; a construction
failure is logged and the (possibly nil) service is returned, matching
the prior best-effort behavior.
- `shouldForceSMTPBackend` — the SMTP-vs-custom-backend rule, pulled out
so the decision is its own testable unit: SMTP and a custom email
backend are mutually exclusive, and an already-enabled SMTP
configuration wins.

Behavior is preserved — `runServeCmd` calls these in the same place with
the same arguments, and the full `cmd/fleet` suite passes against MySQL
+ Redis. The mail block's `config.Email.EmailBackend` reset is local to
mail construction (nothing downstream reads it), so moving it into
`initMailService` is behavior-identical.

On test scope: `TestInitGeoIP` pins the not-fatal fallback for both the
missing-path and invalid-path cases — GeoIP being best-effort is a real
guarantee worth locking. `TestShouldForceSMTPBackend` covers the backend
mutual-exclusion decision, including the nil app config / nil SMTP
settings edges. I didn't add a full `initMailService` happy-path unit
test: `mail.NewService` builds real SMTP/SES backends, so that path is
exercised by booting the server.

**Related issue:** Refs #33370

# Checklist for submitter

- [x] Added/updated automated tests
- Changes file: not applicable — internal refactor with no user-visible
behavior change

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

## Release Notes

* **Refactor**
* Improved GeoIP initialization with automatic fallback when database
configuration is unavailable
* Enhanced mail service initialization with better error handling during
startup
  * Refined SMTP backend precedence logic

* **Tests**
* Added comprehensive unit tests for GeoIP and mail service
initialization scenarios

<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rajendra kadam
2026-06-09 10:19:42 +02:00
committed by GitHub
parent 446b46e029
commit 2def0f22f1
5 changed files with 123 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/fleet/geoip.go
+27
View File
@@ -0,0 +1,27 @@
package main
import (
"context"
"log/slog"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/fleet"
)
// initGeoIP returns the GeoIP provider for the server. GeoIP is best-effort:
// when no database path is configured, or the MaxMind database fails to load,
// it falls back to a no-op provider and logs the problem rather than aborting
// startup.
func initGeoIP(ctx context.Context, cfg config.FleetConfig, logger *slog.Logger) fleet.GeoIP {
if cfg.GeoIP.DatabasePath == "" {
return &fleet.NoOpGeoIP{}
}
maxmind, err := fleet.NewMaxMindGeoIP(logger, cfg.GeoIP.DatabasePath)
if err != nil {
logger.ErrorContext(ctx, "failed to initialize maxmind geoip, check database path", "database_path",
cfg.GeoIP.DatabasePath, "error", err)
return &fleet.NoOpGeoIP{}
}
return maxmind
}
cmd/fleet/geoip_test.go
+26
View File
@@ -0,0 +1,26 @@
package main
import (
"log/slog"
"testing"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/stretchr/testify/assert"
)
func TestInitGeoIP(t *testing.T) {
logger := slog.New(slog.DiscardHandler)
t.Run("no database path returns no-op provider", func(t *testing.T) {
got := initGeoIP(t.Context(), config.FleetConfig{}, logger)
assert.IsType(t, &fleet.NoOpGeoIP{}, got)
})
t.Run("invalid database path falls back to no-op, not fatal", func(t *testing.T) {
cfg := config.FleetConfig{}
cfg.GeoIP.DatabasePath = "/nonexistent/geoip.mmdb"
got := initGeoIP(t.Context(), cfg, logger)
assert.IsType(t, &fleet.NoOpGeoIP{}, got)
})
}
cmd/fleet/mail.go
+38
View File
@@ -0,0 +1,38 @@
package main
import (
"context"
"log/slog"
"github.com/fleetdm/fleet/v4/server/config"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/fleetdm/fleet/v4/server/mail"
)
// shouldForceSMTPBackend reports whether a configured (non-SMTP) email backend
// must be cleared because SMTP is already enabled in the app config. SMTP and a
// custom email backend are mutually exclusive, and an already-enabled SMTP
// configuration takes precedence.
func shouldForceSMTPBackend(appCfg *fleet.AppConfig, emailBackend string) bool {
return appCfg != nil &&
appCfg.SMTPSettings != nil &&
appCfg.SMTPSettings.SMTPEnabled &&
emailBackend != ""
}
// initMailService configures the mail service. Mail is best-effort at startup:
// a construction failure is logged and the (possibly nil) service is returned
// rather than aborting boot.
func initMailService(ctx context.Context, cfg config.FleetConfig, appCfg *fleet.AppConfig, logger *slog.Logger) fleet.MailService {
if shouldForceSMTPBackend(appCfg, cfg.Email.EmailBackend) {
// Force-load the SMTP implementation by clearing the configured backend.
cfg.Email.EmailBackend = ""
logger.WarnContext(ctx, "SMTP is already enabled, first disable SMTP to utilize a different email backend")
}
mailService, err := mail.NewService(cfg)
if err != nil {
logger.ErrorContext(ctx, "failed to configure mailing service", "err", err)
}
return mailService
}
cmd/fleet/mail_test.go
+30
View File
@@ -0,0 +1,30 @@
package main
import (
"testing"
"github.com/fleetdm/fleet/v4/server/fleet"
"github.com/stretchr/testify/assert"
)
func TestShouldForceSMTPBackend(t *testing.T) {
smtpOn := &fleet.AppConfig{SMTPSettings: &fleet.SMTPSettings{SMTPEnabled: true}}
smtpOff := &fleet.AppConfig{SMTPSettings: &fleet.SMTPSettings{SMTPEnabled: false}}
for _, tc := range []struct {
name string
appCfg *fleet.AppConfig
backend string
want bool
}{
{name: "smtp enabled and custom backend set forces smtp", appCfg: smtpOn, backend: "ses", want: true},
{name: "smtp enabled but no custom backend", appCfg: smtpOn, backend: "", want: false},
{name: "smtp disabled with custom backend", appCfg: smtpOff, backend: "ses", want: false},
{name: "nil smtp settings", appCfg: &fleet.AppConfig{}, backend: "ses", want: false},
{name: "nil app config", appCfg: nil, backend: "ses", want: false},
} {
t.Run(tc.name, func(t *testing.T) {
assert.Equal(t, tc.want, shouldForceSMTPBackend(tc.appCfg, tc.backend))
})
}
}
cmd/fleet/serve.go
+2 -24
View File
@@ -64,7 +64,6 @@ import (
"github.com/fleetdm/fleet/v4/server/health"
"github.com/fleetdm/fleet/v4/server/launcher"
"github.com/fleetdm/fleet/v4/server/live_query"
"github.com/fleetdm/fleet/v4/server/mail"
"github.com/fleetdm/fleet/v4/server/mdm/acme"
acme_api "github.com/fleetdm/fleet/v4/server/mdm/acme/api"
acme_bootstrap "github.com/fleetdm/fleet/v4/server/mdm/acme/bootstrap"
@@ -313,17 +312,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
defer sentry.Flush(2 * time.Second)
}
var geoIP fleet.GeoIP
geoIP = &fleet.NoOpGeoIP{}
if config.GeoIP.DatabasePath != "" {
maxmind, err := fleet.NewMaxMindGeoIP(logger, config.GeoIP.DatabasePath)
if err != nil {
logger.ErrorContext(cmd.Context(), "failed to initialize maxmind geoip, check database path", "database_path",
config.GeoIP.DatabasePath, "error", err)
} else {
geoIP = maxmind
}
}
geoIP := initGeoIP(cmd.Context(), config, logger)
if config.MDM.EnableCustomOSUpdatesAndFileVault && !license.IsPremium() {
config.MDM.EnableCustomOSUpdatesAndFileVault = false
@@ -409,18 +398,7 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev
initFatal(err, "saving app config")
}
// setup mail service
if appCfg.SMTPSettings != nil && appCfg.SMTPSettings.SMTPEnabled {
// if SMTP is already enabled then default the backend to empty string, which fill force load the SMTP implementation
if config.Email.EmailBackend != "" {
config.Email.EmailBackend = ""
logger.WarnContext(cmd.Context(), "SMTP is already enabled, first disable SMTP to utilize a different email backend")
}
}
mailService, err := mail.NewService(config)
if err != nil {
logger.ErrorContext(cmd.Context(), "failed to configure mailing service", "err", err)
}
mailService := initMailService(cmd.Context(), config, appCfg, logger)
cronSchedules := fleet.NewCronSchedules()