From a690088193711447aa4d526f2257027f9a459efa Mon Sep 17 00:00:00 2001 From: wayne Date: Tue, 20 Feb 2024 08:38:06 -0700 Subject: [PATCH] GH-40097: [Go][FlightRPC] Enable disabling TLS (#40098) See https://github.com/apache/arrow/issues/40097 for more in-depth description about the problem that led me to file this PR. ### Rationale for this change Because it's annoying to not be able to connect to a non-TLS flightsql endpoint in my development environment just because my development environment happens to still use token authentication. ### What changes are included in this PR? Thread the flightsql `DriverConfig.TLSEnabled` parameter into the `grpcCredentials` type so that `grpcCredentials.RequireTransportSecurity` can return false if TLS is not enabled on the driver config. One thing that occurred to me about the `DriverConfig.TLSEnabled` field is that its semantics seem very mildly dangerous since golang `bool` types are `false` by default and golang doesn't require fields on structs to be explicitly initialized. It seems to me that `DriverConfig.TLSDisabled` would be better (semantically speaking) because then the API user doesn't have to explicitly enable TLS. But I suppose it's probably undesirable to change the name of a public field on a public type. ### Are these changes tested? I haven't written any tests, mostly because there weren't already any tests for the `grpcCredentials` type but I have manually verified this fixes the problem I described in https://github.com/apache/arrow/issues/40097 by rebuilding my tool and running it against the non-TLS listening thing in my development environment. ### Are there any user-facing changes? * Closes: #40097 Authored-by: wayne warren Signed-off-by: Matt Topol --- go/arrow/flight/flightsql/driver/driver.go | 9 +++++---- go/arrow/flight/flightsql/driver/utils.go | 11 ++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/go/arrow/flight/flightsql/driver/driver.go b/go/arrow/flight/flightsql/driver/driver.go index 852a97fb4d3ca..65068048ab3d8 100644 --- a/go/arrow/flight/flightsql/driver/driver.go +++ b/go/arrow/flight/flightsql/driver/driver.go @@ -364,10 +364,11 @@ func (c *Connector) Configure(config *DriverConfig) error { // Set authentication credentials rpcCreds := grpcCredentials{ - username: config.Username, - password: config.Password, - token: config.Token, - params: config.Params, + username: config.Username, + password: config.Password, + token: config.Token, + params: config.Params, + tlsEnabled: config.TLSEnabled, } c.options = append(c.options, grpc.WithPerRPCCredentials(rpcCreds)) diff --git a/go/arrow/flight/flightsql/driver/utils.go b/go/arrow/flight/flightsql/driver/utils.go index f7bd2a2e02113..a99c045e2ed02 100644 --- a/go/arrow/flight/flightsql/driver/utils.go +++ b/go/arrow/flight/flightsql/driver/utils.go @@ -27,10 +27,11 @@ import ( // *** GRPC helpers *** type grpcCredentials struct { - username string - password string - token string - params map[string]string + username string + password string + token string + params map[string]string + tlsEnabled bool } func (g grpcCredentials) GetRequestMetadata(ctx context.Context, uri ...string) (map[string]string, error) { @@ -53,7 +54,7 @@ func (g grpcCredentials) GetRequestMetadata(ctx context.Context, uri ...string) } func (g grpcCredentials) RequireTransportSecurity() bool { - return g.token != "" || g.username != "" + return g.tlsEnabled && (g.token != "" || g.username != "") } // *** Type conversions ***