[rid/store] Fail ISA search instead of silently truncating the result list

[hdimer]

Fixes the RID identification service area case of #1120.

SearchISAs capped its result list to dssmodels.MaxResultLimit and returned it with err == nil, so a client querying a dense area received a non-exhaustive list with no indication that anything had been dropped. Both backends now return dsserr.BadRequest once more than MaxResultLimit areas match, which the v1 and v2 ISA handlers already map to a 400 with the message, so no handler change was needed.

This follows the approach @barroco recorded from the 2024-09-17 contributors sync call in #1120 (comment): fail with a 400 and ask clients to scope their request more tightly, rather than paginate.

What changed

• pkg/rid/store/memstore/identification_service_area.go — the break (and its // This mimics sqlstore behaviour, but it's not very good. TODO) becomes a BadRequest.
• pkg/rid/store/sqlstore/identification_service_area.go — LIMIT MaxResultLimit + 1, then the same BadRequest when that extra row comes back. Selecting one row past the limit is what lets an over-limit result set be distinguished from one that exactly fills it.
• The boundary is unchanged for legitimate queries: exactly MaxResultLimit matches still return the full list with no error.

Incidentally this removes a real divergence between the two backends. memstore appended before checking len(out) > MaxResultLimit, so it returned 10001 items where sqlstore's LIMIT 10000 returned 10000; I measured both before changing anything. The old TODO claimed memstore mimicked sqlstore, and it didn't quite. Both now agree.

ListExpiredISAs is deliberately left alone: it is the operator-run evict sweep, where processing at most MaxResultLimit entries per run and picking up the rest next time is the desired behaviour, not a lie to a client.

One thing I would like your call on

App.SearchISAs has six call sites and only two are the search endpoint. The other four are subscription create/update (pkg/rid/server/v{1,2}/subscription_handler.go), which call it to populate service_areas in the response — and the subscription is already committed by then. So in an area with more than MaxResultLimit ISAs, PUT/PATCH subscription now returns 400 after the write, and a client that retries the same ID gets 409. Previously that branch was unreachable, since SearchISAs could only fail there on empty cells.

That truncated service_areas list is arguably the same bug as #1120 — a client bootstrapping its notification state gets a silently incomplete picture — so failing seems consistent with the recorded decision. But the write-then-400 ordering is not great. Moving the SearchISAs call above the insert/update at those four sites would fix it cleanly (inserting a subscription cannot change which ISAs match; different tables), and I am happy to do that here or in a follow-up. I did not want to touch four handlers uninvited on a PR this size. The release note covers the behaviour either way.

Testing

• TestStoreSearchISAsResultLimit in the memstore pins both halves of the boundary: exactly MaxResultLimit matches succeed with a full list, one more errors with BadRequest. I ran it against the unpatched store first and confirmed it fails on the over-limit assertion for the right reason. It also fails if > is loosened to >=, if the truncated list is returned alongside the error, or if the error is raised without the BadRequest code (which would surface as a 500 rather than a 400).
• go test ./pkg/... ./cmds/... green, gofmt -s -l . clean, golangci-lint run (v2.12.2, per the Makefile) reports 0 issues.
• I could not run make test-go-units-crdb locally, so the sqlstore half is not covered by a test I executed — the existing TestStoreSearchISAs in that package will confirm in CI that the query still works with the new limit, but nothing reaches the new branch. I left it that way on purpose rather than adding a crdb test that inserts 10001 rows into the shared CI suite for logic already pinned in memstore. Say the word and I will add one.

Follow-ups (not in this PR)

The identical pattern is still in pkg/rid/store/{memstore,sqlstore}/subscriptions.go and pkg/scd/store/{memstore,sqlstore}/{subscriptions,operational_intents,constraints}.go — memstore/subscriptions.go even carries the same TODO comment. Worth noting for whoever picks those up: the SCD sites break on >=, so they need the same limit + 1 treatment rather than a copy of the predicate from here. Happy to take them one at a time if this shape is what you want.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: hdimer / name: Haim Dimer (38cdff4)