Providing additional TLS certificates for subgraph requests

[abernix]

Currently, it seems possible to use the SSL_CERT_FILE environment variable to set a path to a (for example, self-signed, intermediary) certificate file and have that work for non-subgraph requests, however, that doesn't appear to work for subgraph requests themselves.

It will become necessary to provide a way to do this for subgraphs, perhaps even on a subgraph-by-subgraph basis. In the near term, it would be nice if SSL_CERT_FILE worked for all outbound requests, including subgraphs, but in the future configuration for all of these things might become necessary.

Short term ask is:

• SSL_CERT_FILE support if possible
• And documentation, because it took me a minute to figure out even SSL_CERT_FILE.

[o0Ignition0o]

There are several underlying issues we might want to derive from this one.

SSL_CERT_FILE is supported, however a self signed certificate generated with CA:TRUE is still not valid. This causes most of self signed certificates to not work.

This can be fixed by adding support for an other environment variable that, given a certificate, would only check for the domain name and expiration to be valid (which we would definitely document and flag as not safe / dangerous).

There's also an other, kind of related issue: Proxy support.

the reqwest library supports SSL_CERT_FILE and also has a danger_accept_invalid_certs parameter.

When it comes to proxies, reqwest is also able to use the systems proxy settings, and supports the HTTP_PROXY and HTTPS_PROXY environment variables.

However hyper::Client (which we are using for our subgraphs) doesn't support all of this out of the box.

It is worth mentioning we moved away from reqwest for performance, and telemetry related reasons, which may or may not still apply.

I'd say supporting a danger_accept_invalid_certs equivalent in the configuration would be pretty straightforward, but adding proxy support to hyper::Client, and doing it right would be non trivial.

I d be in favor of using reqwest again, if anything it would be more symmetrical to rover, and uplink. But I think @Geal had reasons to move away from it, so it's a tuff call.

[Geal]

SSL_CERT_FILE is supported, however a self signed certificate generated with CA:TRUE is still not valid. This causes most of self signed certificates to not work.

an optional verifier that accepts known self signed certificates with the CA:true flag looks like the way to go

There's also an other, kind of related issue: Proxy support.

I don't think the router should support proxies, that is better handled at the infra layer (service meshes, etc)

It is worth mentioning we moved away from reqwest for performance, and telemetry related reasons, which may or may not still apply.

so, some context on this, the subgraph service was moved from reqwest to tower+hyper in #769. That was due to multiple issues.

The first one is performance. We can see in the PR that it resulted in a 20% gain in requests per second. That's because internally, reqwest takes the URL we give, converts it to a string, then reparses it, on every request:

https://github.com/seanmonstar/reqwest/blob/c6eb2c4fcbc3934b119772e0997d91671ff68dc7/src/into_url.rs#L67-L72

pub(crate) fn expect_uri(url: &Url) -> http::Uri {
        url.as_str()
            .parse()
            .expect("a parsed Url should always be a valid Uri")
    }

That is unlikely to be fixed, because reqwest and hyper do not use the same URL type.

The other reason is that we got issues with header propagation and log level filters: https://github.com/TrueLayer/reqwest-middleware/issues/37 Since the log level filter can be set per crate or module, the spans created in reqwest-tracing were not affected by the log level of the router, which in turn could cause opentelemetry header propagation to fail (#656)