Skip to content

Commit

Permalink
postgres: replace the package lib/pq with pgx (hashicorp#15343)
Browse files Browse the repository at this point in the history
* WIP replacing lib/pq

* change timezome param to be URI format

* add changelog

* add changelog for redshift

* update changelog

* add test for DSN style connection string

* more parseurl and quoteidentify to sdk; include copyright and license

* call dbutil.ParseURL instead, fix import ordering

Co-authored-by: Calvin Leung Huang <[email protected]>
  • Loading branch information
Jim Kalafut and calvn authored May 23, 2022
1 parent 4f21baa commit c5a88aa
Show file tree
Hide file tree
Showing 23 changed files with 350 additions and 110 deletions.
13 changes: 3 additions & 10 deletions builtin/logical/database/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ import (

"github.com/go-test/deep"
mongodbatlas "github.com/hashicorp/vault-plugin-database-mongodbatlas"
"github.com/lib/pq"
"github.com/mitchellh/mapstructure"

"github.com/hashicorp/vault/helper/namespace"
postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql"
vaulthttp "github.com/hashicorp/vault/http"
Expand All @@ -30,6 +27,8 @@ import (
"github.com/hashicorp/vault/sdk/helper/pluginutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/vault"
_ "github.com/jackc/pgx/v4"
"github.com/mitchellh/mapstructure"
)

func getCluster(t *testing.T) (*vault.TestCluster, logical.SystemView) {
Expand Down Expand Up @@ -1472,14 +1471,8 @@ func testCredsExist(t *testing.T, resp *logical.Response, connURL string) bool {
t.Fatal(err)
}
log.Printf("[TRACE] Generated credentials: %v", d)
conn, err := pq.ParseURL(connURL)
if err != nil {
t.Fatal(err)
}

conn += " timezone=utc"

db, err := sql.Open("postgres", conn)
db, err := sql.Open("pgx", connURL+"&timezone=utc")
if err != nil {
t.Fatal(err)
}
Expand Down
10 changes: 3 additions & 7 deletions builtin/logical/database/rotation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import (
"github.com/hashicorp/vault/sdk/helper/pluginutil"
"github.com/hashicorp/vault/sdk/logical"
"github.com/hashicorp/vault/sdk/queue"
"github.com/lib/pq"
_ "github.com/jackc/pgx/v4/stdlib"
"github.com/stretchr/testify/mock"
mongodbatlasapi "go.mongodb.org/atlas/mongodbatlas"
"go.mongodb.org/mongo-driver/mongo"
Expand Down Expand Up @@ -419,12 +419,8 @@ func TestBackend_StaticRole_Revoke_user(t *testing.T) {
func createTestPGUser(t *testing.T, connURL string, username, password, query string) {
t.Helper()
log.Printf("[TRACE] Creating test user")
conn, err := pq.ParseURL(connURL)
if err != nil {
t.Fatal(err)
}

db, err := sql.Open("postgres", conn)
db, err := sql.Open("pgx", connURL)
defer db.Close()
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -456,7 +452,7 @@ func createTestPGUser(t *testing.T, connURL string, username, password, query st
func verifyPgConn(t *testing.T, username, password, connURL string) {
t.Helper()
cURL := strings.Replace(connURL, "postgres:secret", username+":"+password, 1)
db, err := sql.Open("postgres", cURL)
db, err := sql.Open("pgx", cURL)
if err != nil {
t.Fatal(err)
}
Expand Down
1 change: 0 additions & 1 deletion builtin/logical/mysql/path_role_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/dbtxn"
"github.com/hashicorp/vault/sdk/logical"
_ "github.com/lib/pq"
)

func pathRoleCreate(b *backend) *framework.Path {
Expand Down
5 changes: 2 additions & 3 deletions builtin/logical/postgresql/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"sync"

log "github.com/hashicorp/go-hclog"

"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
)
Expand Down Expand Up @@ -108,10 +107,10 @@ func (b *backend) DB(ctx context.Context, s logical.Storage) (*sql.DB, error) {
conn += "?timezone=utc"
}
} else {
conn += " timezone=utc"
conn += "&timezone=utc"
}

b.db, err = sql.Open("postgres", conn)
b.db, err = sql.Open("pgx", conn)
if err != nil {
return nil, err
}
Expand Down
25 changes: 3 additions & 22 deletions builtin/logical/postgresql/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (
logicaltest "github.com/hashicorp/vault/helper/testhelpers/logical"
postgreshelper "github.com/hashicorp/vault/helper/testhelpers/postgresql"
"github.com/hashicorp/vault/sdk/logical"
"github.com/lib/pq"
"github.com/mitchellh/mapstructure"
)

Expand Down Expand Up @@ -272,14 +271,8 @@ func testAccStepReadCreds(t *testing.T, b logical.Backend, s logical.Storage, na
return err
}
log.Printf("[TRACE] Generated credentials: %v", d)
conn, err := pq.ParseURL(connURL)
if err != nil {
t.Fatal(err)
}

conn += " timezone=utc"

db, err := sql.Open("postgres", conn)
db, err := sql.Open("pgx", connURL+"&timezone=utc")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -356,14 +349,8 @@ func testAccStepCreateTable(t *testing.T, b logical.Backend, s logical.Storage,
return err
}
log.Printf("[TRACE] Generated credentials: %v", d)
conn, err := pq.ParseURL(connURL)
if err != nil {
t.Fatal(err)
}

conn += " timezone=utc"

db, err := sql.Open("postgres", conn)
db, err := sql.Open("pgx", connURL+"&timezone=utc")
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -410,14 +397,8 @@ func testAccStepDropTable(t *testing.T, b logical.Backend, s logical.Storage, na
return err
}
log.Printf("[TRACE] Generated credentials: %v", d)
conn, err := pq.ParseURL(connURL)
if err != nil {
t.Fatal(err)
}

conn += " timezone=utc"

db, err := sql.Open("postgres", conn)
db, err := sql.Open("pgx", connURL+"&timezone=utc")
if err != nil {
t.Fatal(err)
}
Expand Down
4 changes: 2 additions & 2 deletions builtin/logical/postgresql/path_config_connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (

"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/logical"
_ "github.com/lib/pq"
_ "github.com/jackc/pgx/v4/stdlib"
)

func pathConfigConnection(b *backend) *framework.Path {
Expand Down Expand Up @@ -109,7 +109,7 @@ func (b *backend) pathConnectionWrite(ctx context.Context, req *logical.Request,
verifyConnection := data.Get("verify_connection").(bool)
if verifyConnection {
// Verify the string
db, err := sql.Open("postgres", connURL)
db, err := sql.Open("pgx", connURL)
if err != nil {
return logical.ErrorResponse(fmt.Sprintf(
"Error validating connection info: %s", err)), nil
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/postgresql/path_role_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/dbtxn"
"github.com/hashicorp/vault/sdk/logical"
_ "github.com/lib/pq"
_ "github.com/jackc/pgx/v4/stdlib"
)

func pathRoleCreate(b *backend) *framework.Path {
Expand Down
25 changes: 13 additions & 12 deletions builtin/logical/postgresql/secret_creds.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,12 @@ import (
"strings"
"time"

"github.com/hashicorp/vault/sdk/database/helper/dbutil"

"github.com/hashicorp/go-secure-stdlib/strutil"
"github.com/hashicorp/vault/sdk/framework"
"github.com/hashicorp/vault/sdk/helper/dbtxn"
"github.com/hashicorp/vault/sdk/logical"
"github.com/lib/pq"
)

const SecretCredsType = "creds"
Expand Down Expand Up @@ -75,7 +76,7 @@ func (b *backend) secretCredsRenew(ctx context.Context, req *logical.Request, d

query := fmt.Sprintf(
"ALTER ROLE %s VALID UNTIL '%s';",
pq.QuoteIdentifier(username),
dbutil.QuoteIdentifier(username),
expiration)
stmt, err := db.Prepare(query)
if err != nil {
Expand Down Expand Up @@ -171,27 +172,27 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d
}
revocationStmts = append(revocationStmts, fmt.Sprintf(
`REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA %s FROM %s;`,
pq.QuoteIdentifier(schema),
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(schema),
dbutil.QuoteIdentifier(username)))

revocationStmts = append(revocationStmts, fmt.Sprintf(
`REVOKE USAGE ON SCHEMA %s FROM %s;`,
pq.QuoteIdentifier(schema),
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(schema),
dbutil.QuoteIdentifier(username)))
}

// for good measure, revoke all privileges and usage on schema public
revocationStmts = append(revocationStmts, fmt.Sprintf(
`REVOKE ALL PRIVILEGES ON ALL TABLES IN SCHEMA public FROM %s;`,
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(username)))

revocationStmts = append(revocationStmts, fmt.Sprintf(
"REVOKE ALL PRIVILEGES ON ALL SEQUENCES IN SCHEMA public FROM %s;",
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(username)))

revocationStmts = append(revocationStmts, fmt.Sprintf(
"REVOKE USAGE ON SCHEMA public FROM %s;",
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(username)))

// get the current database name so we can issue a REVOKE CONNECT for
// this username
Expand All @@ -203,8 +204,8 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d
if dbname.Valid {
revocationStmts = append(revocationStmts, fmt.Sprintf(
`REVOKE CONNECT ON DATABASE %s FROM %s;`,
pq.QuoteIdentifier(dbname.String),
pq.QuoteIdentifier(username)))
dbutil.QuoteIdentifier(dbname.String),
dbutil.QuoteIdentifier(username)))
}

// again, here, we do not stop on error, as we want to remove as
Expand All @@ -226,7 +227,7 @@ func (b *backend) secretCredsRevoke(ctx context.Context, req *logical.Request, d

// Drop this user
stmt, err = db.Prepare(fmt.Sprintf(
`DROP ROLE IF EXISTS %s;`, pq.QuoteIdentifier(username)))
`DROP ROLE IF EXISTS %s;`, dbutil.QuoteIdentifier(username)))
if err != nil {
return nil, err
}
Expand Down
15 changes: 15 additions & 0 deletions changelog/15343.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
```release-note:change
physical/cockroachdb: Change underlying driver library from [lib/pq](https://github.com/lib/pq) to [pgx](https://github.com/jackc/pgx)
```

```release-note:change
physical/postgres: Change underlying driver library from [lib/pq](https://github.com/lib/pq) to [pgx](https://github.com/jackc/pgx)
```

```release-note:change
database/postgres: Change underlying driver library from [lib/pq](https://github.com/lib/pq) to [pgx](https://github.com/jackc/pgx)
```

```release-note:change
database/redshift: Change underlying driver library from [lib/pq](https://github.com/lib/pq) to [pgx](https://github.com/jackc/pgx)
```
10 changes: 9 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -122,14 +122,14 @@ require (
github.com/hashicorp/vault/api/auth/userpass v0.1.0
github.com/hashicorp/vault/sdk v0.4.2-0.20220511000023-fa93782110bf
github.com/influxdata/influxdb1-client v0.0.0-20200827194710-b269163b24ab
github.com/jackc/pgx/v4 v4.15.0
github.com/jcmturner/gokrb5/v8 v8.4.2
github.com/jefferai/isbadcipher v0.0.0-20190226160619-51d2077c035f
github.com/jefferai/jsonx v1.0.0
github.com/joyent/triton-go v1.7.1-0.20200416154420-6801d15b779f
github.com/keybase/go-crypto v0.0.0-20190403132359-d65b6b94177f
github.com/kr/pretty v0.3.0
github.com/kr/text v0.2.0
github.com/lib/pq v1.10.3
github.com/mattn/go-colorable v0.1.12
github.com/mattn/go-isatty v0.0.14
github.com/mholt/archiver/v3 v3.5.1
Expand Down Expand Up @@ -296,6 +296,13 @@ require (
github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87 // indirect
github.com/huandu/xstrings v1.3.2 // indirect
github.com/imdario/mergo v0.3.12 // indirect
github.com/jackc/chunkreader/v2 v2.0.1 // indirect
github.com/jackc/pgconn v1.11.0 // indirect
github.com/jackc/pgio v1.0.0 // indirect
github.com/jackc/pgpassfile v1.0.0 // indirect
github.com/jackc/pgproto3/v2 v2.2.0 // indirect
github.com/jackc/pgservicefile v0.0.0-20200714003250-2b9c44734f2b // indirect
github.com/jackc/pgtype v1.10.0 // indirect
github.com/jackc/pgx v3.3.0+incompatible // indirect
github.com/jcmturner/aescts/v2 v2.0.0 // indirect
github.com/jcmturner/dnsutils/v2 v2.0.0 // indirect
Expand All @@ -309,6 +316,7 @@ require (
github.com/kelseyhightower/envconfig v1.4.0 // indirect
github.com/klauspost/compress v1.13.6 // indirect
github.com/klauspost/pgzip v1.2.5 // indirect
github.com/lib/pq v1.10.2 // indirect
github.com/linode/linodego v0.7.1 // indirect
github.com/mattn/go-ieproxy v0.0.1 // indirect
github.com/matttproud/golang_protobuf_extensions v1.0.2-0.20181231171920-c182affec369 // indirect
Expand Down
Loading

0 comments on commit c5a88aa

Please sign in to comment.