From 861c55edb151cb3d2e281ef73e78ad5b20d4af3b Mon Sep 17 00:00:00 2001 From: Thomas Orozco Date: Mon, 17 Apr 2023 19:01:54 +0200 Subject: [PATCH] buck2: don't add tls_config when URI does not request TLS Summary: Like it says in the title. Right now Tonic will try to connect using TLS if you pass `tls_config` even if the URI clearly does not call for it. See: https://github.com/facebook/buck2/issues/156 Test Plan: We have tests internally for the HTTPS case. The non-HTTPS scenario is unfortunately one for which we lack test coverage right now, but let's at least unbreak this to start. I did test that this works locally with non-TLS Build Barn. --- remote_execution/oss/re_grpc/src/client.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/remote_execution/oss/re_grpc/src/client.rs b/remote_execution/oss/re_grpc/src/client.rs index a34b006d71e4..040179e13a61 100644 --- a/remote_execution/oss/re_grpc/src/client.rs +++ b/remote_execution/oss/re_grpc/src/client.rs @@ -55,6 +55,7 @@ use tonic::transport::channel::ClientTlsConfig; use tonic::transport::Certificate; use tonic::transport::Channel; use tonic::transport::Identity; +use tonic::transport::Uri; use crate::error::*; use crate::metadata::*; @@ -154,10 +155,21 @@ impl REClientBuilder { let create_channel = |address: Option| async move { let address = address.as_ref().context("No address")?; let address = substitute_env_vars(address).context("Invalid address")?; + let uri: Uri = address.parse().context("Invalid address")?; + + let uses_tls = match uri.scheme_str() { + Some("grpc") | Some("http") => false, + Some("grpcs") | Some("https") => true, + _ => return Err(anyhow::anyhow!("Invalid scheme")), + }; + + let mut channel = Channel::builder(uri); + if uses_tls { + channel = channel.tls_config(tls_config.clone())?; + } anyhow::Ok( - Channel::from_shared(address.clone())? - .tls_config(tls_config.clone())? + channel .connect() .await .with_context(|| format!("Error connecting to `{}`", address))?,