Skip to content

Commit

Permalink
GH-40097: [Go][FlightRPC] Enable disabling TLS (#40098)
Browse files Browse the repository at this point in the history
See #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 #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 <wayne.warren.s@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
  • Loading branch information
waynr authored Feb 20, 2024
1 parent a8b9537 commit a690088
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
9 changes: 5 additions & 4 deletions go/arrow/flight/flightsql/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))

Expand Down
11 changes: 6 additions & 5 deletions go/arrow/flight/flightsql/driver/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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 ***
Expand Down

0 comments on commit a690088

Please sign in to comment.