amadan

carlos / platform Public

geoip: bounded bucket calls at startup, staging and off; alert reading unknown until state is read; alert mail on a geoip reason change; region guess credits the answering database (Astra code review R4)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 9cd8e78faecae33478df5a803e19243bb50603ad parent 64be2b1
11 files changed, +241 −56
  • internal/console/alertwatch.go +28 −4
  • internal/console/alertwatch_test.go +27 −0
  • internal/console/geoipfollower_test.go +14 −0
  • internal/console/geoippublish.go +12 −2
  • internal/console/geoippublish_test.go +31 −0
  • internal/console/regionpicker.go +33 −5
  • internal/console/server.go +15 −17
  • internal/edge/geoipmirror.go +26 −10
  • internal/edge/geoipmirror_test.go +36 −15
  • internal/geo/swappable.go +13 −0
  • internal/host/agent.go +6 −3

CI passed — run details

diff --git a/internal/console/alertwatch.go b/internal/console/alertwatch.go
index 3ec64c0a3..c7a643b3b 100644
--- a/internal/console/alertwatch.go
+++ b/internal/console/alertwatch.go
@@ -189,16 +189,16 @@ func (s *Server) alertRule(ctx context.Context, run *alertRun, box, rule string,
// Nobody the console can write as. Record the transition against the
// synthetic recipient so it stops replaying, and say so out loud —
// this is the one path where an alert is genuinely dropped.
- if s.alertDue(box, rule, noMailboxAddress, rs.Firing) {
+ if s.alertDueFor(box, rule, noMailboxAddress, rs) {
run.noteMissing()
log.Printf("console: alert watch: %s/%s transition dropped — no operator holds a keymail grant", box, rule)
- s.recordAlertSeen(box, rule, noMailboxAddress, rs.Firing)
+ s.recordAlertSeenFor(box, rule, noMailboxAddress, rs)
}
return
}
var due []operatorGrant
for _, op := range run.to {
- if s.alertDue(box, rule, op.address, rs.Firing) {
+ if s.alertDueFor(box, rule, op.address, rs) {
due = append(due, op)
}
}
@@ -215,7 +215,31 @@ func (s *Server) alertRule(ctx context.Context, run *alertRun, box, rule string,
log.Printf("console: alert watch: keymail %s/%s to %s: %v", box, rule, op.address, err)
continue
}
- s.recordAlertSeen(box, rule, op.address, rs.Firing)
+ s.recordAlertSeenFor(box, rule, op.address, rs)
+ }
+}
+
+// geoipReasonRule is the dedup key for one geoip reason (geoip spec
+// §10.4). The geoip rule's VALUE is a reason code, and a reason that
+// changes while firing (expiring becomes expired) is news the operator
+// must get, though firing never went false (code review R4-3). The plain
+// "geoip" row still carries firing/cleared; one extra row per reason marks
+// which reasons this address has been told about.
+func geoipReasonRule(code float64) string { return fmt.Sprintf("geoip@%d", int(code)) }
+
+// alertDueFor is alertDue plus the geoip reason rule.
+func (s *Server) alertDueFor(box, rule, address string, rs metrics.RuleState) bool {
+ due := s.alertDue(box, rule, address, rs.Firing)
+ if rule == "geoip" && rs.Firing && !due {
+ due = s.alertDue(box, geoipReasonRule(rs.Value), address, true)
+ }
+ return due
+}
+
+func (s *Server) recordAlertSeenFor(box, rule, address string, rs metrics.RuleState) {
+ s.recordAlertSeen(box, rule, address, rs.Firing)
+ if rule == "geoip" && rs.Firing {
+ s.recordAlertSeen(box, geoipReasonRule(rs.Value), address, true)
}
}
diff --git a/internal/console/alertwatch_test.go b/internal/console/alertwatch_test.go
index 34997cc5a..100406c86 100644
--- a/internal/console/alertwatch_test.go
+++ b/internal/console/alertwatch_test.go
@@ -317,3 +317,30 @@ func TestNewServerDefaultsSendApp(t *testing.T) {
}
var _ release.Store = fx.rel
}
+
+func geoipAlert(code float64, firing bool) metrics.AlertState {
+ rs := metrics.RuleState{Firing: firing, Value: code, FiredAt: time.Date(2026, 10, 6, 10, 0, 0, 0, time.UTC)}
+ return metrics.AlertState{V: 1, Box: "box-a", UpdatedAt: time.Date(2026, 10, 6, 10, 5, 0, 0, time.UTC),
+ Rules: map[string]metrics.RuleState{"geoip": rs}}
+}
+
+// Code review R4-3: a geoip reason that changes while firing is mailed.
+func TestAlertPassMailsAGeoIPReasonChange(t *testing.T) {
+ af := newAlertFixture(t)
+ step := func(st metrics.AlertState, wantMails int, wantBody string) {
+ t.Helper()
+ af.writeAlert(t, st)
+ af.pass(t) // resets af.sent
+ got := af.sent
+ if len(got) != wantMails {
+ t.Fatalf("mails = %d, want %d: %+v", len(got), wantMails, got)
+ }
+ if wantBody != "" && !strings.Contains(got[0].body, wantBody) {
+ t.Fatalf("body lacks %q: %s", wantBody, got[0].body)
+ }
+ }
+ step(geoipAlert(2, true), 1, "expires within 9 days")
+ step(geoipAlert(2, true), 0, "")
+ step(geoipAlert(3, true), 1, "expired and removed")
+ step(geoipAlert(0, false), 1, "")
+}
diff --git a/internal/console/geoipfollower_test.go b/internal/console/geoipfollower_test.go
index b30468ad1..9370e8fc9 100644
--- a/internal/console/geoipfollower_test.go
+++ b/internal/console/geoipfollower_test.go
@@ -116,3 +116,17 @@ func TestFollowerExpiresTheLoadedReaderWhateverCurrentSays(t *testing.T) {
}
}
}
+
+// Code review R4-4: the guess's credit names the reader that answered.
+func TestRecordingLocatorNamesTheAnsweringReader(t *testing.T) {
+ sw := geo.NewSwappable()
+ rec := &recordingLocator{sw: sw}
+ if _, ok := rec.Locate(net.ParseIP("81.2.69.142")); ok || rec.used {
+ t.Fatal("an empty reader answered")
+ }
+ db, _ := geo.OpenMMDB("../geo/testdata/GeoIP2-City-Test.mmdb")
+ sw.Swap(db)
+ if _, ok := rec.Locate(net.ParseIP("81.2.69.142")); !ok || !rec.used || rec.source != "GeoIP2-City" {
+ t.Fatalf("recorded = %+v", rec)
+ }
+}
diff --git a/internal/console/geoippublish.go b/internal/console/geoippublish.go
index 9f49f8dba..165a9b4dc 100644
--- a/internal/console/geoippublish.go
+++ b/internal/console/geoippublish.go
@@ -48,6 +48,9 @@ type geoipPublisher struct {
// expired is D1's rule, a field so a test can apply it to the GeoIP2
// fixtures (which never expire) without a GeoLite2 file in the repo.
expired func(dbType string, build, now time.Time) bool
+ // storeTimeout bounds a single small bucket read made while holding,
+ // or about to hold, the publish slot.
+ storeTimeout time.Duration
}
type geoipPublishOpts struct {
@@ -74,7 +77,7 @@ func newGeoipPublisher(store release.Store, tmpRoot string) *geoipPublisher {
}
}
return &geoipPublisher{store: store, tmpRoot: tmpRoot, maxBytes: geoip.MaxBlobBytes,
- deadline: 15 * time.Minute, now: time.Now, expired: geoip.Expired}
+ deadline: 15 * time.Minute, now: time.Now, expired: geoip.Expired, storeTimeout: 30 * time.Second}
}
func marshalPointer(p geoip.Pointer) ([]byte, error) { return json.MarshalIndent(p, "", " ") }
@@ -128,7 +131,12 @@ func (p *geoipPublisher) Stage(ctx context.Context, src io.Reader) (*geoipStaged
return nil, err
}
st := &geoipStaged{dir: dir, path: filepath.Join(dir, "db.mmdb")}
- st.cur, st.curEtag, st.curPresent, st.curErr = p.Current(ctx)
+ // The pointer read is bounded on its own: the caller's context may be
+ // an HTTP request whose only deadline is on reading its body (code
+ // review R4-1).
+ rctx, rcancel := context.WithTimeout(ctx, p.storeTimeout)
+ st.cur, st.curEtag, st.curPresent, st.curErr = p.Current(rctx)
+ rcancel()
if st.curErr != nil && !st.curPresent {
p.Abandon(st)
return nil, st.curErr // the store itself failed
@@ -348,6 +356,8 @@ func (p *geoipPublisher) Off(ctx context.Context) error {
return errGeoipBusy
}
defer p.mu.Unlock()
+ ctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+ defer cancel()
if err := p.store.Delete(ctx, geoip.PointerKey); err != nil && !errors.Is(err, os.ErrNotExist) {
return err
}
diff --git a/internal/console/geoippublish_test.go b/internal/console/geoippublish_test.go
index f258f6671..de2d26e05 100644
--- a/internal/console/geoippublish_test.go
+++ b/internal/console/geoippublish_test.go
@@ -7,6 +7,7 @@ import (
"encoding/hex"
"errors"
"fmt"
+ "io"
"os"
"path/filepath"
"strings"
@@ -420,3 +421,33 @@ func TestFeedPublishRules(t *testing.T) {
t.Fatalf("feed over a manual pointer = %v", err)
}
}
+
+type stallingStore struct{ release.Store }
+
+func (stallingStore) Open(ctx context.Context, key string) (io.ReadCloser, string, error) {
+ <-ctx.Done()
+ return nil, "", ctx.Err()
+}
+
+func (s stallingStore) Get(ctx context.Context, key string) ([]byte, string, error) {
+ <-ctx.Done()
+ return nil, "", ctx.Err()
+}
+
+// Code review R4-1: a stalled bucket cannot hold the publish slot forever.
+func TestStageGivesUpOnAStalledStoreAndFreesTheSlot(t *testing.T) {
+ p, s := newTestPublisher(t)
+ p.store = stallingStore{s}
+ p.storeTimeout = 50 * time.Millisecond
+ start := time.Now()
+ if _, err := p.Stage(context.Background(), bytes.NewReader([]byte("x"))); err == nil {
+ t.Fatal("Stage succeeded against a stalled store")
+ }
+ if time.Since(start) > 5*time.Second {
+ t.Fatal("Stage waited on the stalled store")
+ }
+ if !p.mu.TryLock() {
+ t.Fatal("the slot stayed held after giving up")
+ }
+ p.mu.Unlock()
+}
diff --git a/internal/console/regionpicker.go b/internal/console/regionpicker.go
index 722ab5fd8..599347e40 100644
--- a/internal/console/regionpicker.go
+++ b/internal/console/regionpicker.go
@@ -4,6 +4,8 @@ import (
"context"
"crypto/subtle"
"fmt"
+ "github.com/carlosframework/platform/internal/geo"
+ "github.com/carlosframework/platform/internal/geoip"
"net"
"net/http"
"strings"
@@ -42,12 +44,23 @@ func (s *Server) newAppPicker(ctx context.Context, ip net.IP) *regionPickerView
if eligibleCount(cands) < 2 {
return nil
}
- g, _ := guessRegion(cands, func(string) bool { return true }, guessInput{IP: ip}, s.cfg.GeoIP)
+ var loc geo.Locator = s.cfg.GeoIP
+ var rec *recordingLocator
+ if s.cfg.GeoIPReader != nil {
+ rec = &recordingLocator{sw: s.cfg.GeoIPReader}
+ loc = rec
+ }
+ g, _ := guessRegion(cands, func(string) bool { return true }, guessInput{IP: ip}, loc)
v := &regionPickerView{Auto: fmt.Sprintf(regionCopy.AutoOption, s.placeOf(g.Code, cands))}
- switch a, ok := s.geoipAttribution(); {
- case ok:
- v.Attribution, v.AttributionText, v.AttributionURL = true, a.Text, a.URL
- case s.cfg.GeoIPReader == nil && s.cfg.GeoIP != nil:
+ switch {
+ case rec != nil:
+ // The credit names the database that made THIS guess, read under
+ // the same lock as the lookup (code review R4-4); a guess that used
+ // no GeoIP data owes no credit.
+ if a, ok := geoip.AttributionFor(rec.source); ok && rec.used {
+ v.Attribution, v.AttributionText, v.AttributionURL = true, a.Text, a.URL
+ }
+ case s.cfg.GeoIP != nil:
// A bare Locator (tests, older wiring): the IP step is DB-IP's.
v.Attribution, v.AttributionText, v.AttributionURL = true, regionCopy.Attribution, regionCopy.AttributionURL
}
@@ -103,3 +116,18 @@ func (s *Server) handleNewAppRegion(w http.ResponseWriter, r *http.Request) {
"code": s.homeRegionOut(g.Code), "name": s.placeOf(g.Code, cands), "source": g.Source,
})
}
+
+// recordingLocator remembers which database answered the guess's lookup.
+type recordingLocator struct {
+ sw *geo.Swappable
+ source string
+ used bool
+}
+
+func (r *recordingLocator) Locate(ip net.IP) (geo.Point, bool) {
+ p, ok, src := r.sw.LocateFrom(ip)
+ if ok {
+ r.source, r.used = src, true
+ }
+ return p, ok
+}
diff --git a/internal/console/server.go b/internal/console/server.go
index cbc851ebf..da7d6a3b2 100644
--- a/internal/console/server.go
+++ b/internal/console/server.go
@@ -38,7 +38,6 @@ import (
"github.com/carlosframework/platform/internal/economics"
"github.com/carlosframework/platform/internal/edgesfile"
"github.com/carlosframework/platform/internal/geo"
- "github.com/carlosframework/platform/internal/geoip"
"github.com/carlosframework/platform/internal/infraview"
"github.com/carlosframework/platform/internal/mail"
"github.com/carlosframework/platform/internal/objstore"
@@ -1441,16 +1440,28 @@ func (s *Server) StartGeoipJobs(ctx context.Context) {
log.Printf("console: geoip feed %s armed (first check in 5 minutes, then daily)", f.spec.Name)
go f.run(ctx)
} else {
- clearFeedStatus(ctx, s.cfg.Releases)
+ // Off the startup path, and bounded: a stalled bucket must not
+ // hold the console's start (code review R4-1).
+ go func() {
+ cctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+ defer cancel()
+ clearFeedStatus(cctx, s.cfg.Releases)
+ }()
}
go func() {
for {
- if err := s.geoip.Sweep(ctx); err != nil {
+ // Each bounded: these hold the publish slot, and a stalled
+ // bucket must not leave publish and off answering 409 forever.
+ sctx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+ if err := s.geoip.Sweep(sctx); err != nil {
log.Printf("console: geoip sweep: %v", err)
}
- if _, err := s.geoip.ExpireIfDue(ctx, time.Now()); err != nil {
+ cancel()
+ ectx, cancel := context.WithTimeout(ctx, 2*time.Minute)
+ if _, err := s.geoip.ExpireIfDue(ectx, time.Now()); err != nil {
log.Printf("console: geoip expiry check: %v", err)
}
+ cancel()
select {
case <-ctx.Done():
return
@@ -4805,16 +4816,3 @@ func summarize(v AppView, acct string) appRow {
}
return row
}
-
-// geoipAttribution is the credit the loaded GeoIP database requires, or
-// false when none is loaded or its licence names none (geoip spec §13).
-func (s *Server) geoipAttribution() (geoip.Attribution, bool) {
- if s.cfg.GeoIPReader == nil {
- return geoip.Attribution{}, false
- }
- src, _, ok := s.cfg.GeoIPReader.Loaded()
- if !ok {
- return geoip.Attribution{}, false
- }
- return geoip.AttributionFor(src)
-}
diff --git a/internal/edge/geoipmirror.go b/internal/edge/geoipmirror.go
index e7b32c839..5a1b008cb 100644
--- a/internal/edge/geoipmirror.go
+++ b/internal/edge/geoipmirror.go
@@ -67,9 +67,15 @@ type geoMirror struct {
wantSince time.Time // a live pointer has named a sha not loaded here since then
feedFailing bool
feedBox string
- lastErr string // last poll's error category, "" when healthy
- failingAt time.Time
- lastFailLog time.Time
+ // pointerKnown and feedKnown are false until a read has succeeded or
+ // confirmed absence: until then the alert's reading is unknown (nil),
+ // so a restart cannot clear an alert the last process raised (code
+ // review R4-2).
+ pointerKnown bool
+ feedKnown bool
+ lastErr string // last poll's error category, "" when healthy
+ failingAt time.Time
+ lastFailLog time.Time
}
func newGeoMirror(store release.Store, dir string, sw *geo.Swappable) *geoMirror {
@@ -265,6 +271,7 @@ func (m *geoMirror) poll(ctx context.Context, recheck bool) error {
}
m.pointerSHA, m.skipSHA = "", map[string]string{}
m.pointerPresent, m.pointerType, m.pointerBuild, m.pointerMarked, m.wantSince = false, "", time.Time{}, false, time.Time{}
+ m.pointerKnown = true
m.clearLocked()
m.lastErr = ""
m.genMu.Unlock()
@@ -278,6 +285,7 @@ func (m *geoMirror) poll(ctx context.Context, recheck bool) error {
}
m.genMu.Lock()
m.pointerPresent, m.pointerType, m.pointerBuild, m.pointerMarked = true, p.DatabaseType, p.Build(), p.Expired
+ m.pointerKnown = true
switch {
case p.Expired || geoip.Expired(p.DatabaseType, p.Build(), m.now()):
m.wantSince = time.Time{}
@@ -511,7 +519,7 @@ func (m *geoMirror) readFeedStatus(ctx context.Context) {
raw, err := readCapped(ctx, m.store, geoip.FeedKey, geoip.MaxPointerBytes)
if errors.Is(err, os.ErrNotExist) {
m.genMu.Lock()
- m.feedFailing, m.feedBox = false, "" // no feed: confirmed
+ m.feedFailing, m.feedBox, m.feedKnown = false, "", true // no feed: confirmed
m.genMu.Unlock()
return
}
@@ -523,7 +531,7 @@ func (m *geoMirror) readFeedStatus(ctx context.Context) {
return
}
m.genMu.Lock()
- m.feedFailing, m.feedBox = st.Failing(), st.Box
+ m.feedFailing, m.feedBox, m.feedKnown = st.Failing(), st.Box, true
m.genMu.Unlock()
}
@@ -538,10 +546,18 @@ const (
)
// AlertCode is the geoip rule's reading for this box: the most severe
-// reason that holds (3, 4, 2, 1, then 5), or 0.
-func (m *geoMirror) AlertCode(now time.Time, box string) float64 {
+// reason that holds (3, 4, 2, 1, then 5), or 0. known=false until both the
+// pointer and the feed status have been read (or confirmed absent) once.
+func (m *geoMirror) AlertCode(now time.Time, box string) (code float64, known bool) {
m.genMu.Lock()
defer m.genMu.Unlock()
+ if !m.pointerKnown || !m.feedKnown {
+ return GeoAlertNone, false
+ }
+ return m.alertCodeLocked(now, box), true
+}
+
+func (m *geoMirror) alertCodeLocked(now time.Time, box string) float64 {
expired := m.pointerPresent && (m.pointerMarked || geoip.Expired(m.pointerType, m.pointerBuild, now))
switch {
case expired:
@@ -631,10 +647,10 @@ func (e *Edge) GeoIPState() GeoIPState {
}
// GeoIPAlertCode is the geoip rule's reading for this box (spec §10.4);
-// 0 on an edge with no mirror.
-func (e *Edge) GeoIPAlertCode(now time.Time) float64 {
+// 0 on an edge with no mirror. known=false means "no reading yet".
+func (e *Edge) GeoIPAlertCode(now time.Time) (float64, bool) {
if e.geoMirror == nil {
- return GeoAlertNone
+ return GeoAlertNone, true
}
return e.geoMirror.AlertCode(now, e.box)
}
diff --git a/internal/edge/geoipmirror_test.go b/internal/edge/geoipmirror_test.go
index 4abb18244..85cd35a72 100644
--- a/internal/edge/geoipmirror_test.go
+++ b/internal/edge/geoipmirror_test.go
@@ -346,23 +346,23 @@ func TestMirrorAlertCodes(t *testing.T) {
m.now = func() time.Time { return now }
ctx := context.Background()
- if c := m.AlertCode(now, "box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(now, "box"); c != GeoAlertNone {
t.Fatalf("nothing published = %v", c)
}
publishFixture(t, s, geoFixture(t, "GeoIP2-City-Test.mmdb"), "GeoIP2-City", now)
m.Poll(ctx)
// The fixture was built 2026-02-04: more than 45 days before now.
- if c := m.AlertCode(now, "box"); c != GeoAlertStale {
+ if c, _ := m.AlertCode(now, "box"); c != GeoAlertStale {
t.Fatalf("an old database = %v, want stale", c)
}
- if c := m.AlertCode(time.Date(2026, 2, 10, 0, 0, 0, 0, time.UTC), "box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(time.Date(2026, 2, 10, 0, 0, 0, 0, time.UTC), "box"); c != GeoAlertNone {
t.Fatalf("a fresh database = %v", c)
}
// GeoLite2 at 22 days: expiring wins over stale's absence.
m.genMu.Lock()
m.loadedType, m.loadedBuild = "GeoLite2-City", now.Add(-22*24*time.Hour)
m.genMu.Unlock()
- if c := m.AlertCode(now, "box"); c != GeoAlertExpiring {
+ if c, _ := m.AlertCode(now, "box"); c != GeoAlertExpiring {
t.Fatalf("GeoLite2 at 22 days = %v, want expiring", c)
}
}
@@ -376,7 +376,7 @@ func TestMirrorAlertSurvivesExpiryUntilOff(t *testing.T) {
ctx := context.Background()
publishFixture(t, s, geoFixture(t, "GeoIP2-City-Test.mmdb"), "GeoLite2-City", now.Add(-31*24*time.Hour)) // unmarked, past D1
m.Poll(ctx)
- if c := m.AlertCode(now, "box"); c != GeoAlertExpired {
+ if c, _ := m.AlertCode(now, "box"); c != GeoAlertExpired {
t.Fatalf("expired, unmarked = %v, want expired", c)
}
// A restart with the same unmarked pointer: still expired.
@@ -384,13 +384,13 @@ func TestMirrorAlertSurvivesExpiryUntilOff(t *testing.T) {
m2.now = m.now
m2.Boot()
m2.Poll(ctx)
- if c := m2.AlertCode(now, "box"); c != GeoAlertExpired {
+ if c, _ := m2.AlertCode(now, "box"); c != GeoAlertExpired {
t.Fatalf("after restart = %v, want expired", c)
}
// A fresh publish clears it.
publishFixture(t, s, geoFixture(t, "GeoIP2-Country-Test.mmdb"), "GeoIP2-Country", now)
m.Poll(ctx)
- if c := m.AlertCode(time.Date(2026, 2, 10, 0, 0, 0, 0, time.UTC), "box"); c == GeoAlertExpired {
+ if c, _ := m.AlertCode(time.Date(2026, 2, 10, 0, 0, 0, 0, time.UTC), "box"); c == GeoAlertExpired {
t.Fatal("a fresh publish left the expired reason")
}
// Expired again, then off: off clears every pointer reason.
@@ -398,7 +398,7 @@ func TestMirrorAlertSurvivesExpiryUntilOff(t *testing.T) {
m.Poll(ctx)
s.Delete(ctx, geoip.PointerKey)
m.Poll(ctx)
- if c := m.AlertCode(now, "box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(now, "box"); c != GeoAlertNone {
t.Fatalf("after off = %v, want none", c)
}
}
@@ -411,10 +411,10 @@ func TestMirrorAlertNotLoadedAfterAnHour(t *testing.T) {
p := publishFixture(t, s, geoFixture(t, "GeoIP2-City-Test.mmdb"), "GeoIP2-City", now)
os.WriteFile(filepath.Join(testMirrorStoreDir[s], p.Key), []byte("corrupt"), 0o644)
m.Poll(context.Background())
- if c := m.AlertCode(clock.Add(59*time.Minute), "box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(clock.Add(59*time.Minute), "box"); c != GeoAlertNone {
t.Fatalf("not loaded for 59 minutes = %v", c)
}
- if c := m.AlertCode(clock.Add(61*time.Minute), "box"); c != GeoAlertNotLoaded {
+ if c, _ := m.AlertCode(clock.Add(61*time.Minute), "box"); c != GeoAlertNotLoaded {
t.Fatalf("not loaded for 61 minutes = %v, want not loaded", c)
}
}
@@ -428,10 +428,10 @@ func TestMirrorAlertFeedReasonOnlyOnTheConsoleBox(t *testing.T) {
t.Fatal(err)
}
m.Poll(context.Background()) // no pointer at all: a fresh deployment
- if c := m.AlertCode(now, "console-box"); c != GeoAlertFeed {
+ if c, _ := m.AlertCode(now, "console-box"); c != GeoAlertFeed {
t.Fatalf("console box = %v, want feed", c)
}
- if c := m.AlertCode(now, "other-box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(now, "other-box"); c != GeoAlertNone {
t.Fatalf("another box = %v, want none", c)
}
}
@@ -469,12 +469,12 @@ func TestMirrorFeedStateSurvivesAReadBlip(t *testing.T) {
_, etag, _ := s.Get(context.Background(), geoip.FeedKey)
s.Put(context.Background(), geoip.FeedKey, []byte("garbled"), etag)
m.Poll(context.Background())
- if c := m.AlertCode(now, "console-box"); c != GeoAlertFeed {
+ if c, _ := m.AlertCode(now, "console-box"); c != GeoAlertFeed {
t.Fatalf("a garbled read cleared the feed reason: %v", c)
}
s.Delete(context.Background(), geoip.FeedKey)
m.Poll(context.Background())
- if c := m.AlertCode(now, "console-box"); c != GeoAlertNone {
+ if c, _ := m.AlertCode(now, "console-box"); c != GeoAlertNone {
t.Fatalf("a deleted feed.json must clear it: %v", c)
}
}
@@ -492,7 +492,7 @@ func TestMirrorUnusableDirectoryStillTracksThePointer(t *testing.T) {
if err := m.Poll(context.Background()); err == nil {
t.Fatal("an unusable directory reported success")
}
- if c := m.AlertCode(now.Add(61*time.Minute), "box"); c != GeoAlertNotLoaded {
+ if c, _ := m.AlertCode(now.Add(61*time.Minute), "box"); c != GeoAlertNotLoaded {
t.Fatalf("code = %v, want not loaded", c)
}
}
@@ -516,3 +516,24 @@ func TestRemoveGenerationKeepsRetainedOnes(t *testing.T) {
}
}
}
+
+// Code review R4-2: a cold mirror has no reading until it has read both
+// the pointer and the feed status.
+func TestMirrorAlertUnknownUntilStateIsRead(t *testing.T) {
+ m, s, _ := newTestMirror(t)
+ if _, known := m.AlertCode(time.Now(), "box"); known {
+ t.Fatal("a cold mirror claimed a reading")
+ }
+ s.Put(context.Background(), geoip.FeedKey, []byte("garbled"), "") // feed unreadable
+ m.Poll(context.Background())
+ if _, known := m.AlertCode(time.Now(), "box"); known {
+ t.Fatal("an unreadable feed status gave a reading")
+ }
+ _, etag, _ := s.Get(context.Background(), geoip.FeedKey)
+ _ = etag
+ s.Delete(context.Background(), geoip.FeedKey)
+ m.Poll(context.Background())
+ if c, known := m.AlertCode(time.Now(), "box"); !known || c != GeoAlertNone {
+ t.Fatalf("after both reads = (%v, %v)", c, known)
+ }
+}
diff --git a/internal/geo/swappable.go b/internal/geo/swappable.go
index c24948e2c..90f373f02 100644
--- a/internal/geo/swappable.go
+++ b/internal/geo/swappable.go
@@ -73,3 +73,16 @@ func (s *Swappable) Swap(next *MMDB) {
old.Close()
}
}
+
+// LocateFrom is Locate plus the answering reader's source, under one read
+// lock, so a credit shown beside a guess always names the database that
+// made it.
+func (s *Swappable) LocateFrom(ip net.IP) (Point, bool, string) {
+ s.mu.RLock()
+ defer s.mu.RUnlock()
+ if s.cur == nil {
+ return Point{}, false, ""
+ }
+ p, ok := s.cur.Locate(ip)
+ return p, ok, s.cur.Source()
+}
diff --git a/internal/host/agent.go b/internal/host/agent.go
index e6bf35fe5..d1baf5766 100644
--- a/internal/host/agent.go
+++ b/internal/host/agent.go
@@ -2794,16 +2794,19 @@ var geoIPSource atomic.Value // func() edge.GeoIPState
// geoIPAlertSource is the edge's geoip reason code for the box alert rule
// (geoip spec §10.4), published from the same ready callback.
-var geoIPAlertSource atomic.Value // func(time.Time) float64
+var geoIPAlertSource atomic.Value // func(time.Time) (float64, bool)
// geoIPAlertReading is the rule's reading, or nil before the edge is ready
// (no data never fires and never clears).
func geoIPAlertReading(now time.Time) *float64 {
- fn, _ := geoIPAlertSource.Load().(func(time.Time) float64)
+ fn, _ := geoIPAlertSource.Load().(func(time.Time) (float64, bool))
if fn == nil {
return nil
}
- v := fn(now)
+ v, known := fn(now)
+ if !known {
+ return nil // no data never fires and never clears
+ }
return &v
}