Runs beside the other live discovery tests and takes its credential from
the same AWS_BEARER_TOKEN_BEDROCK the suite already sources, so it needs no
setup of its own.
Reads the listing three ways — one unparameterised GET as Fetch issues it,
the same call followed through nextToken, and Fetch itself — then breaks the
result down by status, type, geography and vendor, counts distinct models
after normalization, and separates the ones the catalog can price from the
ones it cannot.
Not for merge.
Two things the first pass could not do.
An environment may carry SigV4 credentials rather than a Bedrock API key,
and skipping there wastes the only account within reach. It now signs with
the default credential chain when no bearer token is set, and records which
mode it used. Fetch is bearer-only, so its step is skipped under SigV4 —
itself worth knowing, since it means a record holding an access key cannot
discover at all.
A non-200 printed nothing but its status. The body is the whole point of a
failure here: an IAM denial names the action it refused, which is a
different fix from a credential AWS does not recognise.
Not for merge. Discovery reports 100+ models for an account whose console
shows 38 in the same region, and the production path cannot explain it:
parseListing keeps an id, a name and a status and drops the rest of every
summary, so the type, the geography and whether a nextToken was present
never reach a log line.
Reads the listing three ways — one unparameterised GET as Fetch issues it,
the same call followed through nextToken, and Fetch itself — then breaks
the result down by status, type, geography and vendor, counts distinct
models after normalization, and lists which of those the catalog can
price.
Carries its own build tag, so no ordinary test run or CI job compiles it,
and needs only the token: no docker, no management server.
The cross-region inference-profile prefix was matched against a list of
four — us, eu, apac, global — so a profile issued under any other
geography kept its prefix through normalization. That form matches no
catalog key, which cost more than a blank price column:
- discovery returned those models unpriced, so a real account's listing
came back almost entirely at $0
- the cost meter keys its table by the same normalized id, and operators
are told to register a Bedrock id exactly as AWS issues it, region
prefix included — so the default entry never resolved and every cache
bucket, and any model priced only by catalog defaults, metered free
Identify the geography by what follows it instead: a leading segment is a
geography when a known Bedrock vendor namespace comes next. New
geographies then need no change at all, and a vendor missing from the map
fails safe by keeping the prefix — the behaviour of the list this
replaces. Over-stripping is the direction that must not happen, since the
normalized id also decides which route may claim a model.
Covered at all three seams the id passes through: the normalizer, the cost
meter config the proxy bills from, and the discovery listing the dashboard
renders. Each test fails against the old four-geography list.
A Bedrock provider could never answer a discovery request. The router
routed GET /inference-profiles to the record's upstream, which has to be
bedrock-runtime.<region> for InvokeModel to work, and that host does not
implement the operation — AWS answers <UnknownOperationException/>.
ListInferenceProfiles lives on the control plane at bedrock.<region>.
Give the route a discovery host, taken from the catalog's declaration
with the region read back out of the configured upstream, and send the
listing — and only the listing — there. Inference is untouched, and a
proxied or self-hosted Bedrock endpoint gets no discovery host at all
rather than a guessed one, since inventing a host would send the
operator's credential somewhere they never configured.
Two things had to follow for the listing to be usable once it arrives.
The response filter only understood OpenAI's {data:[{id:…}]}, so a
Bedrock listing was forwarded whole — offering every profile in the
account whatever the policy said. It now recognises the
inferenceProfileSummaries envelope, and matches a listing id against the
record's models after stripping the region prefix and version suffix, so
the two spellings of one model line up.
The policy bound had the same problem from the other side: it intersected
by exact string, so a record registering the raw profile id while a
guardrail names the catalog key intersected to nothing and would have
bounded a working provider's listing down to empty. routeClaimsModel
already normalises the candidate for this reason; the bound now agrees
with it.
The live discovery e2e flips from asserting the 404 to asserting a real
filtered listing. The mock upstream cannot cover any of this: it answers
/inference-profiles on the same listener as everything else, so a
mock-based test passes whichever host the request went to.
The endpoint reported pricing_known and then made the operator find the
price themselves. The dashboard has nothing to prefill a model row with, so
a discovered model either arrived at zero — silently metering every request
against it as free — or had to be priced by hand against a table NetBird
already ships.
Each model now carries the rates it would actually be billed at.
Rates come from the live default pricing table rather than the compiled-in
catalog, because that is the table the synthesiser ships to the proxy: an
operator running a defaults_llm_pricing.yaml would otherwise be shown one
price in the form and charged another. It is the same lookup the catalog
endpoint prefills from, so a model reached by either route prices
identically — pinned by TestDiscoveredRatesMatchTheCatalogEndpoint, since
the two are separate call paths that would otherwise drift.
pricing_known now derives from that same lookup instead of a second pass
over the compiled catalog, so "we can price this" and "here is the price"
can no longer disagree.
input_per_1k and output_per_1k are required and sent even at zero: an
unpriced model is offered at zero and flagged rather than withheld — the
vendor says the credential can reach it, and hiding it would hide a model
the operator genuinely has. The cache rates stay absent when unset, matching
the catalog response, because a zero there reads as "free" rather than "not
applicable".
Five smaller points from the same review as the four already fixed.
**The no-redirect policy had no test.** Every Fetch test injects an
HTTPClient, which bypasses httpClient() and therefore the policy entirely,
so nothing asserted that a 302 is refused — and the policy is a security
control: a redirect moves the request to a host checkPublicHost never
resolved. TestRedirectsAreNotFollowed drives the real constructor against an
httptest server that redirects to the cloud metadata address, and asserts
exactly one request leaves the client.
**An injected client silently lost that policy.** Production is safe today
only because NewManager passes a nil HTTPClient; any future non-test
injection would have dropped the guarantee with no signal. An injected
client that states no policy now inherits ours.
Implemented by copying the client rather than assigning into it. Writing
c.HTTPClient.CheckRedirect from httpClient() would mutate a struct shared by
every request goroutine for the process's lifetime — a data race, and the
exact hazard the same review's next point warns about. The copy shares the
Transport, which is safe for concurrent use by design.
**Two comments described things that were not true.** The manager's doc
claimed reading a stored credential "is permission-checked as one"; there is
no per-record Read check, just the single Create check, which covers it
because Create is stronger and the lookup is account-scoped. Said that
instead. The modelDiscovery field now records that it is shared across
requests and must stay read-only after construction.
**The handler test asserted neither the label contract nor the upstream.**
The response omits label entirely when a vendor supplies none, and the
dashboard falls back to the id on absence — an empty string would render a
blank row. The fixture had no label-less model to prove it with (the review
described one, but both existing entries carry labels), so this adds one.
The upstream assertion matters because Bedrock's region is read back out of
it. Also fixes the "blank catalog provide" subtest name.
Four points from the review of #7246, all confirmed against the code.
**regionFromUpstream panicked on Bedrock's regionless endpoint.** The
template is "bedrock-runtime.<region>.amazonaws.com", so the two fixed
halves are "bedrock-runtime." and ".amazonaws.com". The regionless host
"bedrock-runtime.amazonaws.com" carries both at once, with the halves
overlapping rather than sandwiching a region — it satisfied HasPrefix and
HasSuffix, then sliced host[16:15]:
panic: runtime error: slice bounds out of range [16:15]
That is reachable from any operator who types that host into upstream_url on
a Bedrock record. The length check makes the overlap read as "no region
here", which is what it is.
**A DNS-rebinding window sat between the guard and the dial.**
checkPublicHost resolved the host and the transport resolved it again to
dial, and the name's owner picks both answers. Public to the first lookup,
127.0.0.1 to the second, and the request reached loopback carrying the
operator's provider credential. The dialer now re-checks at the socket
through net.Dialer.Control, which runs post-resolution and pre-connect for
each address tried, so it sees what the second lookup actually returned.
The transport is cloned from http.DefaultTransport to keep its proxy and
TLS behaviour, and shared package-wide so the connection pool survives.
**Caller-input failures answered 500.** An unknown provider, an unusable
upstream, a region that cannot be read and a missing key are all reachable
from a well-formed request with a bad field value, and the OpenAPI document
already declares 400 for this endpoint. They now carry ErrInvalidRequest and
the handler branches on the sentinel rather than on message text. The
non-public-address refusal is included: that is the caller's own URL.
**The catalog id was trimmed for the emptiness test and then discarded.** A
padded " openai_api " cleared the check and reached catalog.Lookup with its
spaces, so the operator was told their provider was unknown.
Adds POST /api/agent-network/catalog/providers/models, so the provider
form can offer the models an operator's own credential can reach instead
of only the compiled-in catalog.
A caller names a catalog provider and supplies either the key they are
typing (the record does not exist yet) or the id of a saved record whose
stored credential should be reused — which lets the dashboard refresh a
list without ever holding the key. The two are mutually exclusive:
accepting both would run an arbitrary credential under the identity of a
record the caller may only be permitted to read. When a record id is
given, the catalog id and upstream come from the record too, so the
credential cannot be aimed at a different vendor's endpoint.
Gated on Create rather than Read. This spends the operator's credential
against a third party, which is not something a read-only role should be
able to make the server do.
A provider with no listing endpoint answers 422 rather than 500: the
caller falls back to the catalog's own models on that outcome, so it has
to be distinguishable from a failure.
The region is read back out of the configured upstream by matching it
against the catalog's host template, since a provider record has no
region field and the operator already encoded one when they set up
inference. An upstream matching no template is refused rather than
guessed at — a wrong region would dial another account's endpoint.
The catalog is the only source of models an operator can pick from, and
it cannot know two things that matter. It goes stale — its entries carry
comments recording which models a vendor retired on which date — and it
cannot see an account: which OpenAI models an org is entitled to, which
Bedrock inference profiles a given account and region hold, which Vertex
models a project has enabled.
Add a client that asks the vendor directly, with the endpoint, auth
header and response shape all declared by the catalog rather than
supplied by the caller. The four shapes come from probing the live APIs
(see the discovery e2e); each vendor invented its own envelope and none
can be guessed from the request.
Bedrock is the case that shaped the design. Its listing lives on the
control plane while inference must go to the runtime host, so Discovery
carries its own host. The ids it returns are region-prefixed and are
taken verbatim, because that prefix is what AWS requires and it cannot
be derived from the configured region — an eu-central-1 account holds
global.* profiles alongside its eu.* ones.
The vendor is authoritative for the id; the catalog stays authoritative
for pricing. A discovered model the shipped table cannot price is
reported as such, so it cannot be registered at a silent zero rate.
Management has not made outbound calls on an operator's behalf before,
and it holds a credential for every provider, so the host is checked
before dialing: every resolved address must be public, which covers the
cloud metadata address and NetBird's own overlay range, and redirects
are not followed since they would move the request to a host the check
never saw.
