Skip to content

Commit

Permalink
Bugfix: Allow enabling TLS for sql tool without requiring key/cert. (#…
Browse files Browse the repository at this point in the history
…1252)

* Bugfix: Allow enabling TLS for sql tool without requiring key/cert.

This allows using TLS for transport but not authentication, consistent
with temporal server. Prior to this fix it was not possible to use the
SQL tool to connect to Postgres which required TLS for transport
security but was not setup to use TLS for authentication.

* Don't validate TLS config inside sql tool.

The config will be validated during connection anyway.

Remove related test.

* Remove tests that are no longer relevant.

Also rename file according to golang conventions.
  • Loading branch information
robholland authored Feb 3, 2021
1 parent 9d9d8bf commit 69da561
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"

"go.temporal.io/server/common/auth"
"go.temporal.io/server/common/service/config"
"go.temporal.io/server/environment"
"go.temporal.io/server/tools/common/schema"
Expand Down Expand Up @@ -85,28 +84,4 @@ func (s *HandlerTestSuite) TestValidateConnectConfig() {
cfg.DatabaseName = "foobar"
s.Nil(sql.ValidateConnectConfig(cfg, false))
s.Nil(sql.ValidateConnectConfig(cfg, true))

cfg.TLS = &auth.TLS{}
cfg.TLS.Enabled = true
s.NotNil(sql.ValidateConnectConfig(cfg, false))
s.NotNil(sql.ValidateConnectConfig(cfg, true))

cfg.TLS.CaFile = "ca.pem"
s.Nil(sql.ValidateConnectConfig(cfg, false))
s.Nil(sql.ValidateConnectConfig(cfg, true))

cfg.TLS.KeyFile = "key_file"
cfg.TLS.CertFile = ""
s.NotNil(sql.ValidateConnectConfig(cfg, false))
s.NotNil(sql.ValidateConnectConfig(cfg, true))

cfg.TLS.KeyFile = ""
cfg.TLS.CertFile = "cert_file"
s.NotNil(sql.ValidateConnectConfig(cfg, false))
s.NotNil(sql.ValidateConnectConfig(cfg, true))

cfg.TLS.KeyFile = "key_file"
cfg.TLS.CertFile = "cert_file"
s.Nil(sql.ValidateConnectConfig(cfg, false))
s.Nil(sql.ValidateConnectConfig(cfg, true))
}
12 changes: 0 additions & 12 deletions tools/sql/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,19 +265,7 @@ func ValidateConnectConfig(cfg *config.SQL, isDryRun bool) error {
}
cfg.DatabaseName = schema.DryrunDBName
}
if cfg.TLS != nil && cfg.TLS.Enabled {
enabledCaFile := cfg.TLS.CaFile != ""
enabledCertFile := cfg.TLS.CertFile != ""
enabledKeyFile := cfg.TLS.KeyFile != ""

if (enabledCertFile && !enabledKeyFile) || (!enabledCertFile && enabledKeyFile) {
return schema.NewConfigError("must have both CertFile and KeyFile set")
}

if !enabledCaFile && !enabledCertFile && !enabledKeyFile {
return schema.NewConfigError("must provide tls certs to use")
}
}
return nil
}

Expand Down

0 comments on commit 69da561

Please sign in to comment.