mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-27 01:51:30 +02:00
[management] Check the upstream a Bedrock listing never touches
Bedrock lists from the control plane and infers on the runtime host, so a successful listing says nothing about the URL on the record. An upstream matching no catalog template left no region to derive, which skipped the check entirely: a record pointed at a host that does not exist saved clean, and every request it later served went nowhere. Entries that declare a listing host of their own now have their configured upstream resolved on its own account. A host that will not resolve blocks the save; one that resolves privately does not, since Bedrock behind a proxy is a supported configuration and stays the unverifiable case it already was.
This commit is contained in:
@@ -124,6 +124,17 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
|
||||
return nil, ErrNoDiscovery
|
||||
}
|
||||
|
||||
// An entry with a listing host of its own answers from somewhere other
|
||||
// than the upstream on the record — Bedrock lists from the control plane
|
||||
// and infers on the runtime host. Reaching the listing therefore proves
|
||||
// nothing about the host requests will actually go to, so that one is
|
||||
// checked separately or not at all.
|
||||
if entry.Discovery.Host != "" {
|
||||
if err := c.checkUpstreamHost(entry, req.UpstreamURL); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
}
|
||||
|
||||
endpoint, err := c.discoveryURL(entry, req)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -201,20 +212,40 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
|
||||
}
|
||||
|
||||
target := &url.URL{Scheme: "https", Host: host, Path: entry.Discovery.Path, RawQuery: entry.Discovery.Query}
|
||||
if err := c.checkPublicHost(target.Hostname()); err != nil {
|
||||
// A host that refuses to resolve fails here, before any request is
|
||||
// built, and it is the commonest way for an upstream to be wrong. It
|
||||
// has to reach the caller as unreachable rather than as an
|
||||
// unclassified fault. ErrPrivateHost is the other outcome and means
|
||||
// something else entirely — not a bad host, one we decline to dial.
|
||||
if errors.Is(err, ErrPrivateHost) {
|
||||
return "", err
|
||||
}
|
||||
return "", &UnreachableError{Provider: entry.Name, Err: err}
|
||||
if err := c.classifyHost(entry, target.Hostname()); err != nil {
|
||||
return "", err
|
||||
}
|
||||
return target.String(), nil
|
||||
}
|
||||
|
||||
// checkUpstreamHost verifies the host the operator configured, for entries
|
||||
// whose listing lives elsewhere and so cannot vouch for it.
|
||||
//
|
||||
// A name that does not resolve is the record being wrong. One that resolves
|
||||
// privately is not: an upstream behind a proxy is a supported configuration,
|
||||
// and ErrPrivateHost carries that difference on to the caller, which treats it
|
||||
// as unverifiable rather than as a failure.
|
||||
func (c *Client) checkUpstreamHost(entry catalog.Provider, upstreamURL string) error {
|
||||
parsed, err := url.Parse(strings.TrimSpace(upstreamURL))
|
||||
if err != nil || parsed.Hostname() == "" {
|
||||
return fmt.Errorf("%w: provider upstream %q is not a usable URL", ErrInvalidRequest, upstreamURL)
|
||||
}
|
||||
return c.classifyHost(entry, parsed.Hostname())
|
||||
}
|
||||
|
||||
// classifyHost renders a failed host check as the two outcomes the caller
|
||||
// distinguishes. A host that refuses to resolve is the commonest way for an
|
||||
// upstream to be wrong and has to arrive as unreachable rather than as an
|
||||
// unclassified fault. ErrPrivateHost means something else entirely — not a bad
|
||||
// host, one we decline to dial.
|
||||
func (c *Client) classifyHost(entry catalog.Provider, host string) error {
|
||||
err := c.checkPublicHost(host)
|
||||
if err == nil || errors.Is(err, ErrPrivateHost) {
|
||||
return err
|
||||
}
|
||||
return &UnreachableError{Provider: entry.Name, Err: err}
|
||||
}
|
||||
|
||||
// RegionFromUpstream recovers the region an operator embedded in the provider
|
||||
// upstream, by matching it against the catalog's own host template. Bedrock's
|
||||
// template is "bedrock-runtime.<region>.amazonaws.com" and Vertex's is
|
||||
|
||||
@@ -597,3 +597,50 @@ func TestFetch_AProxyInThePathDoesNotSilentlyDisableTheCheck(t *testing.T) {
|
||||
type roundTripFunc func(*http.Request) (*http.Response, error)
|
||||
|
||||
func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
|
||||
|
||||
// TestFetch_TheUpstreamIsCheckedWhenTheListingCannotVouchForIt covers the hole
|
||||
// a separate listing host leaves. Bedrock lists from the control plane, so a
|
||||
// record whose runtime upstream does not exist reaches a perfectly good
|
||||
// listing and saves — the requests it then serves go nowhere.
|
||||
//
|
||||
// Both halves matter. A runtime host that cannot be resolved is the record
|
||||
// being wrong, and blocks. A proxied one resolves and only leaves the region
|
||||
// underivable, which stays the unverifiable outcome it already was.
|
||||
func TestFetch_TheUpstreamIsCheckedWhenTheListingCannotVouchForIt(t *testing.T) {
|
||||
refusing := &net.Resolver{
|
||||
PreferGo: true,
|
||||
Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
|
||||
return nil, errors.New("resolver unavailable")
|
||||
},
|
||||
}
|
||||
client := &Client{Resolver: refusing}
|
||||
|
||||
_, err := client.Fetch(context.Background(), Request{
|
||||
CatalogID: "bedrock_api",
|
||||
// Matches no catalog template, so nothing here reaches the control
|
||||
// plane the listing comes from: without its own check this upstream
|
||||
// was never contacted at all.
|
||||
UpstreamURL: "https://bedrock.typo.example.invalid",
|
||||
APIKey: "aws-bearer",
|
||||
})
|
||||
|
||||
require.Error(t, err)
|
||||
var unreachable *UnreachableError
|
||||
require.ErrorAs(t, err, &unreachable, "a runtime host that will not resolve must block the save")
|
||||
}
|
||||
|
||||
// TestFetch_AListingHostOfItsOwnDoesNotReachThroughTheUpstream keeps the check
|
||||
// above from reading the operator's upstream as the place to list from.
|
||||
func TestFetch_AListingHostOfItsOwnDoesNotReachThroughTheUpstream(t *testing.T) {
|
||||
cl, tr := newStubClient(http.StatusOK, bedrockListing)
|
||||
|
||||
_, err := cl.Fetch(context.Background(), Request{
|
||||
CatalogID: "bedrock_api",
|
||||
UpstreamURL: "https://bedrock-runtime.eu-central-1.amazonaws.com",
|
||||
APIKey: "aws-bearer",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, "bedrock.eu-central-1.amazonaws.com", tr.got.URL.Host,
|
||||
"checking the runtime host must not turn it into the listing host")
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user