Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement postgres support for TLS #3488

Merged
merged 39 commits into from
Oct 14, 2020
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
4f517f4
implement postgres support for TLS
nadilas Aug 29, 2020
94c8cef
Merge branch 'master' into patch-1
anish531213 Aug 31, 2020
482f3d8
Merge branch 'master' into patch-1
nadilas Sep 1, 2020
6484cbf
Merge branch 'master' into patch-1
meiliang86 Sep 10, 2020
55e5da9
Syncrhonize with Temporal postgres plugin
nadilas Sep 11, 2020
de0fde0
Merge branch 'master' into patch-1
meiliang86 Sep 11, 2020
717e016
Merge branch 'master' into patch-1
meiliang86 Sep 14, 2020
41b52b6
implement postgres support for TLS
nadilas Aug 29, 2020
674eb00
Enforce re-replication context timeout for standby tasks (#3473)
yycptt Aug 31, 2020
995493f
Enable processing queue split policy by domainID (#3486)
yycptt Aug 31, 2020
a43df67
Allow DLQ cli use a range of shard ids (#3481)
yux0 Aug 31, 2020
ed904c2
Integrate current execution check with replication resender (#3487)
yux0 Sep 1, 2020
c65e4c9
Do not extend activity expiration time (#3489)
yycptt Sep 1, 2020
f383e1e
Start queue processor before failover callback registration (#3494)
yycptt Sep 2, 2020
c73d42d
Clean up kafka replicagtion in worker (#3493)
yux0 Sep 8, 2020
936ba28
[SQL]Fix upsert SQL template for Postgres plugin (#3498)
longquanzheng Sep 8, 2020
3d778ed
Ignore reapplication if the domain is pending active (#3502)
yux0 Sep 9, 2020
19956f1
Add domain tag to history query metrics (#3504)
andrewjdawson2016 Sep 9, 2020
efb92b1
Remove kafka replication from history and cli (#3503)
yux0 Sep 9, 2020
f16ee6b
move retryer to persistance package (#3497)
mantas-sidlauskas Sep 10, 2020
43c4136
Update shard info when adding failover marker (#3507)
yux0 Sep 10, 2020
b19844b
Syncrhonize with Temporal postgres plugin
nadilas Sep 11, 2020
298c843
k8s: fix cassandra-tool env key conflict with k8s (#3505)
pip1998 Sep 11, 2020
42d8b2d
Improve developer contribution guide for Postgres development (#3495)
longquanzheng Sep 11, 2020
739578c
Fix NDC resetter persistence bugs (#3500)
longquanzheng Sep 11, 2020
23849cb
Multicursor Queue Processor Improvements (#3509)
yycptt Sep 12, 2020
504ce17
Add multicursor processing queue related metrics (#3510)
yycptt Sep 14, 2020
0919b63
fixes postgres_server_test.go wrongly setting db password, hence plug…
Oct 3, 2020
d9d7832
Merge remote-tracking branch 'origin/patch-1' into patch-1
Oct 3, 2020
220a335
Merge branch 'master' into patch-1
longquanzheng Oct 11, 2020
3d18b6b
Merge branch 'master' into patch-1
meiliang86 Oct 12, 2020
dbc483f
Merge branch 'master' into patch-1
nadilas Oct 13, 2020
275ff1b
Merge branch 'master' into patch-1
longquanzheng Oct 13, 2020
f03a71b
fixes testPassword issue blocking postgres_server_test.go in postgres…
Oct 14, 2020
36da622
Merge branch 'master' into patch-1
nadilas Oct 14, 2020
c55e87a
fixes go.sum updates needed issue
Oct 14, 2020
2aaa1d8
Merge branch 'master' into patch-1
longquanzheng Oct 14, 2020
019b1b1
tidy go.sum
Oct 14, 2020
0439167
Merge branch 'master' into patch-1
longquanzheng Oct 14, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 50 additions & 26 deletions common/persistence/sql/sqlplugin/postgres/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,32 +21,28 @@
package postgres

import (
"errors"
"fmt"
"net"
"net/url"
"os"
"runtime"

pt "github.com/uber/cadence/common/persistence/persistence-tests"
"github.com/uber/cadence/environment"

"github.com/iancoleman/strcase"
"github.com/jmoiron/sqlx"

"github.com/uber/cadence/common/persistence/sql"
"github.com/uber/cadence/common/persistence/sql/sqlplugin"
"github.com/uber/cadence/common/service/config"

"github.com/iancoleman/strcase"
"github.com/jmoiron/sqlx"
)

const (
// PluginName is the name of the plugin
PluginName = "postgres"
dataSourceNamePostgres = "user=%v host=%v port=%v dbname=%v sslmode=disable %v "
dataSourceNamePostgresPassword = "password=%v"
PluginName = "postgres"
dsnFmt = "postgres://%s@%s:%s/%s"
)

var errTLSNotImplemented = errors.New("tls for postgres has not been implemented")

type plugin struct{}

var _ sqlplugin.Plugin = (*plugin)(nil)
Expand Down Expand Up @@ -80,7 +76,7 @@ func (d *plugin) CreateAdminDB(cfg *config.SQL) (sqlplugin.AdminDB, error) {
// SQL database and the object can be used to perform CRUD operations on
// the tables in the database
func (d *plugin) createDBConnection(cfg *config.SQL) (*sqlx.DB, error) {
err := registerTLSConfig(cfg)
sslParams, err := registerTLSConfig(cfg)
if err != nil {
return nil, err
}
Expand All @@ -90,32 +86,60 @@ func (d *plugin) createDBConnection(cfg *config.SQL) (*sqlx.DB, error) {
return nil, fmt.Errorf("invalid connect address, it must be in host:port format, %v, err: %v", cfg.ConnectAddr, err)
}

db, err := sqlx.Connect(PluginName, buildDSN(cfg, host, port, sslParams))
if err != nil {
return nil, err
}
if cfg.MaxConns > 0 {
db.SetMaxOpenConns(cfg.MaxConns)
}
if cfg.MaxIdleConns > 0 {
db.SetMaxIdleConns(cfg.MaxIdleConns)
}
if cfg.MaxConnLifetime > 0 {
db.SetConnMaxLifetime(cfg.MaxConnLifetime)
}

// Maps struct names in CamelCase to snake without need for db struct tags.
db.MapperFunc(strcase.ToSnake)
return db, nil
}

func buildDSN(cfg *config.SQL, host string, port string, sslParams url.Values) string {
dbName := cfg.DatabaseName
//NOTE: postgres doesn't allow to connect with empty dbName, the admin dbName is "postgres"
if dbName == "" {
dbName = "postgres"
}
password := ""
if cfg.Password != "" {
password = fmt.Sprintf(dataSourceNamePostgresPassword, cfg.Password)
}
db, err := sqlx.Connect(PluginName, fmt.Sprintf(dataSourceNamePostgres, cfg.User, host, port, dbName, password))

if err != nil {
return nil, err
credentialString := generateCredentialString(cfg.User, cfg.Password)
dsn := fmt.Sprintf(dsnFmt, credentialString, host, port, dbName)
if attrs := sslParams.Encode(); attrs != "" {
dsn += "?" + attrs
}
return dsn
}

// Maps struct names in CamelCase to snake without need for db struct tags.
db.MapperFunc(strcase.ToSnake)
return db, nil
func generateCredentialString(user string, password string) string {
userPass := user
if password != "" {
userPass += ":" + password
}
return userPass
}

// TODO: implement postgres specific support for TLS
func registerTLSConfig(cfg *config.SQL) error {
if cfg.TLS == nil || !cfg.TLS.Enabled {
return nil
func registerTLSConfig(cfg *config.SQL) (sslParams url.Values, err error) {
sslParams = url.Values{}
if cfg.TLS != nil && cfg.TLS.Enabled {
sslParams.Set("ssl", "true")
sslParams.Set("sslmode", "require")
Copy link

@JosefWN JosefWN Nov 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I'm late to the party, but is it wise to hard code this? How would I connect using the more secure modes verify-ca or verify-full for example?

Could as well let the user choose?

sslParams.Set("sslmode", cfg.TLS.SSLMode)

Just looking at the config it's hard to know what cfg.TLS.Enabled means; is it allow, prefer, require, verify-ca or verify-full? All of those modes are enabling TLS support to various degrees.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @JosefWN you are not late! Apparently I don't have enough expertise/experience about security so I didn't think too much about that.
Is value of sslmode the only thing needed to change to support all other modes like you mention? If so then I could make a PR out for this.

Or it would be nice if you can make a PR for this. Because I am not sure how to test it.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think making the sslmode configurable is sufficient (which would also make cfg.TLS.Enabled redundant).

Perhaps something like this:

	sslParams.Set("sslmode", cfg.SSLMode)
	if cfg.SSLMode != "disable" && cfg.TLS != nil {
		sslParams.Set("sslrootcert", cfg.TLS.CaFile)
		sslParams.Set("sslkey", cfg.TLS.KeyFile)
		sslParams.Set("sslcert", cfg.TLS.CertFile)
	}

The default is prefer, so it should be backwards compatible even if SSLMode is not set (but would need to test that it actually works).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! PR out here: https://github.com/uber/cadence/pull/3787/files
But I think we have to make default as require now...since people started to use it. I believe it's okay, lmk if I am wrong.

sslParams.Set("sslrootcert", cfg.TLS.CaFile)
sslParams.Set("sslkey", cfg.TLS.KeyFile)
sslParams.Set("sslcert", cfg.TLS.CertFile)
} else {
sslParams.Set("sslmode", "disable")
}
return errTLSNotImplemented
return
}

const (
Expand Down
32 changes: 32 additions & 0 deletions common/persistence/sql/sqlplugin/postgres/postgres_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,38 @@ import (
pt "github.com/uber/cadence/common/persistence/persistence-tests"
)


const (
testSchemaDir = "schema/postgres"
)

func getTestClusterOption() *pt.TestBaseOptions {
nadilas marked this conversation as resolved.
Show resolved Hide resolved
testUser := "postgres"
testPassword := "cadence"

if runtime.GOOS == "darwin" {
testUser = os.Getenv("USER")
testPassword = ""
}

if os.Getenv("POSTGRES_USER") != "" {
testUser = os.Getenv("POSTGRES_USER")
}

if os.Getenv("POSTGRES_PASSWORD") != "" {
testPassword = os.Getenv("POSTGRES_PASSWORD")
}

return &pt.TestBaseOptions{
SQLDBPluginName: PluginName,
DBUsername: testUser,
DBPassword: testPassword,
DBHost: environment.GetPostgresAddress(),
DBPort: environment.GetPostgresPort(),
SchemaDir: testSchemaDir,
}
}

func TestSQLHistoryV2PersistenceSuite(t *testing.T) {
s := new(pt.HistoryV2PersistenceSuite)
s.TestBase = pt.NewTestBaseWithSQL(GetTestClusterOption())
Expand Down
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ require (
go.uber.org/yarpc v1.42.0
go.uber.org/zap v1.12.0
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
golang.org/x/sys v0.0.0-20191029155521-f43be2a4598c // indirect
golang.org/x/time v0.0.0-20190308202827-9d24e82272b4
golang.org/x/tools v0.0.0-20200917221617-d56e4e40bc9d
gonum.org/v1/gonum v0.7.0
Expand Down