mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(geo): read the Nominatim override the way the schema validates it
Follow-up to #2379. NOMINATIM_URL arrives from a compose file or a ConfigMap with whatever whitespace was typed around it. The schema validates the trimmed string and calls a blank one unset, the derivation did neither, so " " passed startup as "use the default" and then became the base URL that cannot be fetched, and a padded value lost every geocoding call the same way. It trims now, which is also what the places client next door does with its own override. The chart could not pass the variable at all: the ConfigMap is an allowlist and an unknown key is dropped without a word, which is exactly the locked-down cluster the override exists for. The docs said what to set and not what it costs. They now say that it replaces the public service with no fallback, that the endpoint has to speak the Nominatim API, and that the one-per-second spacing stays either way, so nobody stands up their own instance expecting it to run faster. Test IDs follow the file again.
This commit is contained in:
Maurice
@@ -73,6 +73,9 @@ data:
|
||||
{{- if .Values.env.MCP_RATE_LIMIT }}
|
||||
MCP_RATE_LIMIT: {{ .Values.env.MCP_RATE_LIMIT | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.NOMINATIM_URL }}
|
||||
NOMINATIM_URL: {{ .Values.env.NOMINATIM_URL | quote }}
|
||||
{{- end }}
|
||||
{{- if .Values.env.OVERPASS_URL }}
|
||||
OVERPASS_URL: {{ .Values.env.OVERPASS_URL | quote }}
|
||||
{{- end }}
|
||||
|
||||
@@ -78,6 +78,11 @@ env:
|
||||
# Max MCP API requests per user per minute. Defaults to 300.
|
||||
# MCP_MAX_SESSION_PER_USER: "20"
|
||||
# Max concurrent MCP sessions per user. Defaults to 20.
|
||||
# NOMINATIM_URL: ""
|
||||
# Custom Nominatim base URL for every geocoding call the server makes (place search, reverse lookup, Atlas region
|
||||
# fill). When set it REPLACES the public OpenStreetMap service, with no fallback, so the instance behind it needs
|
||||
# the coverage you expect and has to speak the Nominatim API. TREK keeps its ~1 request per second spacing either
|
||||
# way. Defaults to https://nominatim.openstreetmap.org.
|
||||
# OVERPASS_URL: ""
|
||||
# Custom Overpass endpoint(s) for the map POI "explore" search, comma-separated. When set, REPLACES the bundled
|
||||
# public mirrors — point it at an internal/self-hosted Overpass instance when the public mirrors are unreachable
|
||||
|
||||
+1
-1
@@ -49,7 +49,7 @@ DEMO_MODE=false # Demo mode - resets data hourly
|
||||
# MCP_RATE_LIMIT=300 # Max MCP API requests per user per minute (default: 300)
|
||||
# MCP_MAX_SESSION_PER_USER=20 # Max concurrent MCP sessions per user (default: 20)
|
||||
|
||||
# NOMINATIM_URL=https://nominatim.openstreetmap.org # Custom HTTP(S) Nominatim base URL for maps and Atlas geocoding. Unset/blank uses the public default.
|
||||
# NOMINATIM_URL=https://nominatim.openstreetmap.org # Custom HTTP(S) Nominatim base URL for every geocoding call the server makes: place search, reverse lookup, place details and the Atlas region fill. When set it REPLACES the public service with no fallback, so whatever answers has to speak the Nominatim API (/search, /reverse, /lookup) and hold the coverage you expect. TREK keeps its ~1 request per second spacing whichever instance answers. Unset/blank uses the public default.
|
||||
|
||||
# OVERPASS_URL= # Custom Overpass endpoint(s) for the map POI "explore" search, comma-separated. When set, REPLACES the bundled public mirrors — point it at an internal/self-hosted Overpass instance when the public ones are unreachable from your network. Non-http(s) entries are ignored. If you don't self-host Overpass but the public mirrors throttle you, setting APP_URL also gives outbound requests a unique User-Agent the mirrors rate-limit less.
|
||||
# OVERPASS_TIMEOUT_MS=12000 # Per-endpoint timeout (ms) for Overpass POI requests; slower endpoints are abandoned so a faster mirror wins. Raise it for a slow self-hosted instance. (default: 12000)
|
||||
|
||||
@@ -265,7 +265,10 @@ export function deriveIntegrations(raw: RawEnv) {
|
||||
return {
|
||||
unsplashAccessKey: raw.UNSPLASH_ACCESS_KEY?.trim(),
|
||||
transitApiBase: stripTrailingSlashes(raw.TRANSIT_API_URL || 'https://api.transitous.org'),
|
||||
nominatimUrl: stripTrailingSlashes(raw.NOMINATIM_URL || 'https://nominatim.openstreetmap.org'),
|
||||
// Trimmed before the default fires: the schema validates the trimmed value and
|
||||
// treats a blank one as unset, so a padded or whitespace-only value would
|
||||
// otherwise pass startup and then be the string that cannot be fetched.
|
||||
nominatimUrl: stripTrailingSlashes(raw.NOMINATIM_URL?.trim() || 'https://nominatim.openstreetmap.org'),
|
||||
overpassUrl: raw.OVERPASS_URL,
|
||||
// Longer than the `[timeout:20]` the query itself carries, or we abort an answer the
|
||||
// mirror was still allowed to be working on. See OVERPASS_QUERY_TIMEOUT_S.
|
||||
|
||||
@@ -163,6 +163,12 @@ export interface NominatimOptions {
|
||||
* The abort signal is constructed after the throttle wait on purpose: building it
|
||||
* first would spend up to 1.1s of the caller's budget queueing rather than
|
||||
* fetching, which for the 2.5s identity lookup is nearly half of it.
|
||||
*
|
||||
* The base is `NOMINATIM_URL` where an operator set one and the public service
|
||||
* otherwise, read live rather than frozen at import so a test (and a restart-free
|
||||
* change) is seen on the next call. Neither the cursor nor the User-Agent asks
|
||||
* which one it got: the spacing is what keeps an operator's installs inside the
|
||||
* policy, and that holds for a gateway in front of the public service too.
|
||||
*/
|
||||
export async function nominatimFetch(
|
||||
path: 'search' | 'reverse' | 'lookup',
|
||||
|
||||
@@ -214,10 +214,17 @@ describe('derivePlugins', () => {
|
||||
});
|
||||
|
||||
describe('deriveIntegrations', () => {
|
||||
it('pins unsplash trim, transit base strip + default, overpass timeout', () => {
|
||||
it('pins unsplash trim, transit base strip + default, nominatim trim + default, overpass timeout', () => {
|
||||
expect(deriveIntegrations({ UNSPLASH_ACCESS_KEY: ' key ' }).unsplashAccessKey).toBe('key');
|
||||
expect(deriveIntegrations({}).transitApiBase).toBe('https://api.transitous.org');
|
||||
expect(deriveIntegrations({ TRANSIT_API_URL: 'https://t.example//' }).transitApiBase).toBe('https://t.example');
|
||||
expect(deriveIntegrations({}).nominatimUrl).toBe('https://nominatim.openstreetmap.org');
|
||||
// Padded and whitespace-only both come back from a compose file or a ConfigMap,
|
||||
// and the schema already called the second one unset.
|
||||
expect(deriveIntegrations({ NOMINATIM_URL: ' http://nominatim:8080/geo// ' }).nominatimUrl).toBe(
|
||||
'http://nominatim:8080/geo',
|
||||
);
|
||||
expect(deriveIntegrations({ NOMINATIM_URL: ' ' }).nominatimUrl).toBe('https://nominatim.openstreetmap.org');
|
||||
// Against the constant, not a literal: the client budget is derived from the timeout
|
||||
// the query itself carries, and pinning the number here is how the two drifted apart.
|
||||
expect(deriveIntegrations({}).overpassTimeoutMs).toBe(OVERPASS_TIMEOUT_DEFAULT_MS);
|
||||
|
||||
@@ -55,26 +55,6 @@ async function drain<T>(fn: () => Promise<T>): Promise<T> {
|
||||
}
|
||||
|
||||
describe('nominatimFetch', () => {
|
||||
it('uses the public endpoint for an empty override', async () => {
|
||||
vi.stubEnv('NOMINATIM_URL', '');
|
||||
await drain(() => nominatimFetch('search', new URLSearchParams({ q: 'Berlin' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://nominatim.openstreetmap.org/search?q=Berlin');
|
||||
});
|
||||
|
||||
it.each(['search', 'reverse', 'lookup'] as const)('routes %s to the configured path prefix', async (path) => {
|
||||
vi.stubEnv('NOMINATIM_URL', 'http://nominatim:8080/geocoding///');
|
||||
const params = new URLSearchParams({ q: 'New York & Berlin' });
|
||||
await drain(() => nominatimFetch(path, params));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe(`http://nominatim:8080/geocoding/${path}?q=New+York+%26+Berlin`);
|
||||
expect(fetchMock.mock.calls[0][1].headers['User-Agent']).toBe(UA);
|
||||
});
|
||||
|
||||
it('supports an HTTPS override without a trailing slash', async () => {
|
||||
vi.stubEnv('NOMINATIM_URL', 'https://geo.example.com');
|
||||
await drain(() => nominatimFetch('lookup', new URLSearchParams({ osm_ids: 'N1' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://geo.example.com/lookup?osm_ids=N1');
|
||||
});
|
||||
|
||||
it('GEO-001: identifies the instance on every request', async () => {
|
||||
await drain(() => nominatimFetch('search', new URLSearchParams({ q: 'Berlin' })));
|
||||
|
||||
@@ -156,6 +136,43 @@ describe('nominatimFetch', () => {
|
||||
const order = fetchMock.mock.calls.slice(1).map(([url]) => (url as string).includes('q=ui') ? 'ui' : 'bg');
|
||||
expect(order[0]).toBe('ui');
|
||||
});
|
||||
|
||||
it('GEO-012: an unset or blank override keeps the public endpoint', async () => {
|
||||
vi.stubEnv('NOMINATIM_URL', '');
|
||||
await drain(() => nominatimFetch('search', new URLSearchParams({ q: 'Berlin' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://nominatim.openstreetmap.org/search?q=Berlin');
|
||||
});
|
||||
|
||||
it.each(['search', 'reverse', 'lookup'] as const)(
|
||||
'GEO-013: routes %s under the configured path prefix',
|
||||
async (path) => {
|
||||
vi.stubEnv('NOMINATIM_URL', 'http://nominatim:8080/geocoding///');
|
||||
const params = new URLSearchParams({ q: 'New York & Berlin' });
|
||||
await drain(() => nominatimFetch(path, params));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe(`http://nominatim:8080/geocoding/${path}?q=New+York+%26+Berlin`);
|
||||
},
|
||||
);
|
||||
|
||||
it('GEO-014: takes an override with no trailing slash as it is', async () => {
|
||||
vi.stubEnv('NOMINATIM_URL', 'https://geo.example.com');
|
||||
await drain(() => nominatimFetch('lookup', new URLSearchParams({ osm_ids: 'N1' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://geo.example.com/lookup?osm_ids=N1');
|
||||
});
|
||||
|
||||
// A compose file or a ConfigMap hands the value over with whatever whitespace
|
||||
// was typed around it. The schema validates the trimmed string and calls a
|
||||
// blank one unset, so the derivation has to read it the same way or the value
|
||||
// that passed startup is the one that cannot be fetched.
|
||||
it('GEO-015: trims the override, so a padded value still resolves', async () => {
|
||||
vi.stubEnv('NOMINATIM_URL', ' https://geo.example.com/geocoding/ ');
|
||||
await drain(() => nominatimFetch('search', new URLSearchParams({ q: 'Berlin' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://geo.example.com/geocoding/search?q=Berlin');
|
||||
|
||||
fetchMock.mockClear();
|
||||
vi.stubEnv('NOMINATIM_URL', ' ');
|
||||
await drain(() => nominatimFetch('search', new URLSearchParams({ q: 'Berlin' })));
|
||||
expect(fetchMock.mock.calls[0][0]).toBe('https://nominatim.openstreetmap.org/search?q=Berlin');
|
||||
});
|
||||
});
|
||||
|
||||
describe('geo cache', () => {
|
||||
|
||||
@@ -396,7 +396,7 @@ next key rotation or admin reset would quietly switch the file back to WAL.
|
||||
| Variable | Description | Default |
|
||||
|---------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------------|
|
||||
| `IDEMPOTENCY_TTL_SECONDS` | How long (in seconds) stored idempotency keys are kept before garbage collection. The offline client replays queued mutations with their `X-Idempotency-Key` on reconnect, so this must exceed the longest expected offline window or a replay could create a duplicate. Invalid values abort startup. | `2592000` (30 days) |
|
||||
| `NOMINATIM_URL` | Custom HTTP(S) Nominatim base URL for maps and Atlas geocoding. Unset or blank uses the default. | `https://nominatim.openstreetmap.org` |
|
||||
| `NOMINATIM_URL` | Custom HTTP(S) Nominatim base URL for every geocoding call the server makes: place search, reverse lookup, place details and the Atlas region fill. When set it **replaces** the public OpenStreetMap service for all of them, with no fallback, so point it at an instance whose import covers the places your users plan and that speaks the Nominatim API (`/search`, `/reverse`, `/lookup`). Photon, Geoapify and LocationIQ are not drop-in replacements. TREK keeps its client-side spacing of roughly one request a second whichever instance answers, so this changes where the requests go, not how fast they are sent. Unset or blank uses the default. | `https://nominatim.openstreetmap.org` |
|
||||
| `OVERPASS_URL` | Custom [Overpass API](https://wiki.openstreetmap.org/wiki/Overpass_API) endpoint(s) used by the map's POI "explore" search, comma-separated. When set it **replaces** the bundled public mirrors — point it at an internal or self-hosted Overpass instance when the public mirrors are unreachable from your network (e.g. firewalled/locked-down egress in a Kubernetes cluster). Entries that aren't valid `http(s)` URLs are ignored. If you don't run your own Overpass but the public mirrors throttle TREK, first make sure `APP_URL` (or `ALLOWED_ORIGINS`) is set: that alone gives outbound Overpass/Nominatim requests a unique User-Agent, which the public mirrors rate-limit far less. | bundled public mirrors |
|
||||
| `OVERPASS_TIMEOUT_MS` | Per-endpoint timeout (in milliseconds) for Overpass POI requests. Endpoints race in parallel and one that hasn't answered within this window is abandoned so a faster mirror can win. Raise it if you run a slow self-hosted Overpass instance. Invalid values abort startup. | `12000` |
|
||||
| `LLM_TIMEOUT_MS` | How long (in milliseconds) one AI-parsing call may take before it is abandoned. One ceiling for every provider, applied to the abort signal and to the underlying HTTP client alike. The default is generous so heavier parsing work fits without a code change; lower it if you use a cloud provider and would rather fail fast. Invalid values abort startup. | `900000` (15 min) |
|
||||
|
||||
Reference in New Issue
Block a user