envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
25.01k stars 4.81k forks source link

Add support for https uri in extensions.wasm.v3.VmConfig #29824

Open boeboe opened 1 year ago

boeboe commented 1 year ago

Title: Add support to fetch wasm binary from https base clusters

Description: Currently, using https urls to fetch wasm binaries, does not support https, as it will always set the wrong :scheme:http header, despite configuring an https url.

Bootstrapping wasm service:

        bootstrap_extensions:
        - name: envoy.bootstrap.wasm
          typed_config:
            "@type": type.googleapis.com/envoy.extensions.wasm.v3.WasmService
            singleton: true
            config:
              name: wasm-plugin
              root_id: wasm_root_id
              vm_config:
                runtime: "envoy.wasm.runtime.v8"
                code:
                  remote:
                    http_uri:
                      uri: "https://github.com/XXX/YYY/releases/download/ZZZ/envoy-wasm-plugin.wasm"
                      timeout: 600s
                      cluster: "github-com"
                    sha256: "aab45ba4aab5c313b5206fa57b3843cd6a09c633eddef08371a83699c312aaf2"

Creating static cluster:

          - name: "github-com"
            connect_timeout: "60s"
            type: STRICT_DNS
            load_assignment:
              cluster_name: "github-com"
              endpoints:
              - lb_endpoints:
                - endpoint:
                    address:
                      socket_address:
                        address: github.com
                        port_value: 443
            transport_socket:
              name: envoy.transport_sockets.tls
              typed_config:
                "@type": type.googleapis.com/envoy.extensions.transport_sockets.tls.v3.UpstreamTlsContext
                sni: github.com

This results in the following error traces:

2023-09-26T19:30:48.587732Z debug   envoy config external/envoy/source/common/config/remote_data_fetcher.cc:35  fetch remote data from [uri = https://github.com/XXX/YYY/releases/download/ZZZ/envoy-wasm-plugin.wasm]: start   thread=27
2023-09-26T19:30:48.587780Z debug envoy router external/envoy/source/common/router/router.cc:478  [C0][S17801876496647140807] cluster 'github-com' match for URL '/XXX/YYY/releases/download/ZZZ/envoy-wasm-plugin.wasm' thread=27
2023-09-26T19:30:48.587866Z debug envoy router external/envoy/source/common/router/router.cc:686  [C0][S17801876496647140807] router decoding headers:
':path', '/XXX/YYY/releases/download/ZZZ/envoy-wasm-plugin.wasm'
':authority', 'github.com'
':method', 'GET'
':scheme', 'http'
'x-envoy-internal', 'true'
'x-forwarded-for', '10.244.0.17'
'x-envoy-expected-rq-timeout-ms', '600000'
  thread=27

...

2023-09-26T19:30:48.588508Z debug envoy router external/envoy/source/common/router/router.cc:1278 [C0][S17801876496647140807] upstream reset: reset reason: connection failure, transport failure reason: immediate connect error: Network is unreachable thread=27
2023-09-26T19:30:48.588576Z debug envoy http external/envoy/source/common/http/async_client_impl.cc:123 async http request response headers (end_stream=false):
':status', '503'
'content-length', '166'
'content-type', 'text/plain'
  thread=27

As one can see, the headers are not taking into account the fact that my uri is of scheme https.

Source code:

Digging into the code, I can see the following.

https://github.com/envoyproxy/envoy/blob/f1369ae370a1062744980b0c0a4a991c8acf68a2/source/common/config/remote_data_fetcher.cc#L33C52-L33C66

void RemoteDataFetcher::fetch() {
  Http::RequestMessagePtr message = Http::Utility::prepareHeaders(uri_);
  message->headers().setReferenceMethod(Http::Headers::get().MethodValues.Get);
  ENVOY_LOG(debug, "fetch remote data from [uri = {}]: start", uri_.uri());
  const auto thread_local_cluster = cm_.getThreadLocalCluster(uri_.cluster());
  if (thread_local_cluster != nullptr) {
    request_ = thread_local_cluster->httpAsyncClient().send(
        std::move(message), *this,
        Http::AsyncClient::RequestOptions().setTimeout(
            std::chrono::milliseconds(DurationUtil::durationToMilliseconds(uri_.timeout()))));
  } else {
    ENVOY_LOG(debug, "fetch remote data [uri = {}]: no cluster {}", uri_.uri(), uri_.cluster());
    callback_.onFailure(FailureReason::Network);
  }
}

https://github.com/envoyproxy/envoy/blob/f1369ae370a1062744980b0c0a4a991c8acf68a2/source/common/http/utility.cc#L1083

RequestMessagePtr Utility::prepareHeaders(const envoy::config::core::v3::HttpUri& http_uri) {
  absl::string_view host, path;
  extractHostPathFromUri(http_uri.uri(), host, path);

  RequestMessagePtr message(new RequestMessageImpl());
  message->headers().setPath(path);
  message->headers().setHost(host);

  return message;
}

As you can see, they just do not take into account the scheme at all.

Also, there is no logic to deal with retries, or with 302 HTTP redirect (eg http > https) from the upstream web server containing the wasm binary.

kyessenov commented 1 year ago

This is correct, URL fetching of wasm is very rudimentary and should only be used with care in production. Envoy should be issuing a warning if you look into the logs. This is the reason why Istio built a whole separate sidecar to pre-fetch the modules.

github-actions[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] commented 1 year ago

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.