diff --git a/cmd/server/config.go b/cmd/server/config.go index c461eaf1..cd767c54 100644 --- a/cmd/server/config.go +++ b/cmd/server/config.go @@ -142,6 +142,14 @@ type Config struct { // GOMEMLIMIT environment variable, when set, takes precedence. Runtime *RuntimeConfig `json:"runtime,omitempty"` GeoFilter *GeoFilterConfig `json:"geo_filter,omitempty"` + // GeoFilterAppliesToNodeList restores the pre-opt-in behavior where + // configuring geo_filter alone made GET /api/nodes exclude out-of- + // polygon nodes by default (no ?geoFilter=1 needed). Off by default — + // deployments that already relied on the old always-on filtering + // (added under #730) can set this to keep it; new deployments get the + // non-surprising default of geo_filter not silently changing what the + // node list/map returns. + GeoFilterAppliesToNodeList bool `json:"geoFilterAppliesToNodeList,omitempty"` Areas map[string]AreaEntry `json:"areas,omitempty"` diff --git a/cmd/server/nodes_geofilter_optin_test.go b/cmd/server/nodes_geofilter_optin_test.go index c96f29a2..cbdd310b 100644 --- a/cmd/server/nodes_geofilter_optin_test.go +++ b/cmd/server/nodes_geofilter_optin_test.go @@ -74,6 +74,28 @@ func TestHandleNodes_GeoFilterExcludedByDefault(t *testing.T) { t.Error("expected OutsideUntagged (outside polygon, not yet foreign-tagged) to be excluded when geoFilter=1") } }) + + t.Run("GeoFilterAppliesToNodeList=true restores the pre-opt-in always-on behavior without needing ?geoFilter=1", func(t *testing.T) { + srv.cfg.GeoFilterAppliesToNodeList = true + defer func() { srv.cfg.GeoFilterAppliesToNodeList = false }() + + req := httptest.NewRequest("GET", "/api/nodes?limit=50", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + if w.Code != 200 { + t.Fatalf("expected 200, got %d", w.Code) + } + got := names(w) + if !got["InsideNode"] { + t.Error("expected InsideNode (within polygon) to be present") + } + if !got["OutsideTagged"] { + t.Error("expected OutsideTagged (foreign_advert=1) to be present even though it's outside the polygon — #730") + } + if got["OutsideUntagged"] { + t.Error("expected OutsideUntagged to be excluded by default when GeoFilterAppliesToNodeList=true, without needing ?geoFilter=1") + } + }) } func floatPtr(f float64) *float64 { return &f } diff --git a/cmd/server/openapi.go b/cmd/server/openapi.go index 46464ecb..36c12843 100644 --- a/cmd/server/openapi.go +++ b/cmd/server/openapi.go @@ -80,7 +80,7 @@ func routeDescriptions() map[string]routeMeta { QueryParams: []paramMeta{ {Name: "role", Description: "Filter by node role", Type: "string"}, {Name: "status", Description: "Filter by status (active/stale/offline)", Type: "string"}, - {Name: "geoFilter", Description: "Set to \"1\" to exclude nodes outside the configured geo_filter (unless foreign_advert-tagged). Opt-in — omitting it returns every node regardless of geo_filter.", Type: "string"}, + {Name: "geoFilter", Description: "Set to \"1\" to exclude nodes outside the configured geo_filter (unless foreign_advert-tagged). Opt-in — omitting it returns every node regardless of geo_filter, unless the deployment's config.json sets geoFilterAppliesToNodeList=true to restore the pre-opt-in always-on behavior.", Type: "string"}, }}, "GET /api/nodes/search": {Summary: "Search nodes", Description: "Search nodes by name or public key prefix.", Tag: "nodes", QueryParams: []paramMeta{{Name: "q", Description: "Search query", Type: "string", Required: true}}}, "GET /api/nodes/bulk-health": {Summary: "Bulk node health", Description: "Returns health status for all nodes in one call.", Tag: "nodes"}, diff --git a/cmd/server/routes.go b/cmd/server/routes.go index d56f0207..513a7773 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -1426,16 +1426,21 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) { } } } - // Opt-in only (?geoFilter=1): configuring geo_filter alone must not - // change what /api/nodes (and therefore the live map, which lists - // straight off this endpoint) returns by default. It used to — any node - // outside the polygon and not YET foreign_advert-tagged (which only - // happens on that node's next ADVERT after geo_filter was configured) - // would silently vanish from every view the moment geo_filter was set, - // with no per-request way to see them anyway. geo_filter's own - // ingestor-side tagging (and the explicit prune-geo-filter admin flow) - // are unaffected by this — this only gates the passive declutter view. - if s.cfg.GeoFilter != nil && q.Get("geoFilter") == "1" { + // Opt-in by default (?geoFilter=1), unless the deployment has + // GeoFilterAppliesToNodeList set to restore the pre-existing always-on + // behavior. Configuring geo_filter alone must not, by default, change + // what /api/nodes (and therefore the live map, which lists straight off + // this endpoint) returns — any node outside the polygon and not YET + // foreign_advert-tagged (which only happens on that node's next ADVERT + // after geo_filter was configured) would otherwise silently vanish from + // every view the moment geo_filter was set, with no per-request way to + // see them anyway. A deployment that intentionally relies on the old + // #730 declutter behavior can set GeoFilterAppliesToNodeList to keep + // it. geo_filter's own ingestor-side tagging (and the explicit + // prune-geo-filter admin flow) are unaffected either way — this only + // gates the passive declutter view. + applyGeoFilter := s.cfg.GeoFilterAppliesToNodeList || q.Get("geoFilter") == "1" + if s.cfg.GeoFilter != nil && applyGeoFilter { filtered := nodes[:0] for _, node := range nodes { // Foreign-flagged nodes (#730) are kept even when their GPS lies