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

Add pgx Cloud SQL Proxy in-process example #433

Closed
StevenACoffman opened this issue Aug 20, 2020 · 14 comments
Closed

Add pgx Cloud SQL Proxy in-process example #433

StevenACoffman opened this issue Aug 20, 2020 · 14 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@StevenACoffman
Copy link

The authors of the lib/pq library are encouraging new projects to use pgx instead:

This package is effectively in maintenance mode and is not actively developed. Small patches and features are only rarely reviewed and merged. We recommend using pgx which is actively maintained.

I am connecting to cloudsql from GKE, but I would prefer not to run a sidecar. However, I am using pgx, rather than pg. I have modified the hook_test.go file to use pgx as follows:

package main

import (
	"context"
	"fmt"
	"log"
	"net"
	"os"
	"regexp"
	"time"

	"github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy"
	"github.com/jackc/pgx/v4"
)

var (
	host = os.Getenv("DBHOST") // DBHOST for GCP cloudsql is in format project:region:instance
	name = os.Getenv("DBNAME")
	user = os.Getenv("DBUSER")
	pass = os.Getenv("DBPASS")
)

// parses an addr like "[project:region:instance]:port"
var instanceRegexp = regexp.MustCompile(`^\[(.+)\]:[0-9]+$`)

// Example shows how to use cloudsqlpostgres dialer
func main() {

	// In-process Proxy
	// Note that sslmode=disable is required it does not mean that the connection
	// is unencrypted. All connections via the proxy are completely encrypted.
	dsn := fmt.Sprintf("host=%s user=%s password=%s dbname=%s sslmode=disable", host, user, pass, name)

	config, configErr := pgx.ParseConfig(dsn)
	if configErr != nil {
		log.Fatal(configErr)
	}

	config.DialFunc = func(ctx context.Context, network string, addr string) (net.Conn, error) {
		matches := instanceRegexp.FindStringSubmatch(addr)
		if len(matches) != 2 {
			return nil, fmt.Errorf("failed to parse addr: %q. It should conform to the regular expression %q", addr, instanceRegexp)
		}
		instance := matches[1]
		return proxy.Dial(instance)
	}

	config.LookupFunc = func(ctx context.Context, host string) ([]string, error) {
		return []string{host}, nil
	}

	conn, connErr := pgx.ConnectConfig(context.Background(), config)
	if connErr != nil {
		log.Fatal(connErr)
	}

	defer conn.Close(context.Background())

	var now time.Time
	err := conn.QueryRow(context.Background(), "SELECT NOW()").Scan(&now)
	if err != nil {
		fmt.Fprintf(os.Stderr, "QueryRow failed: %v\n", err)
		os.Exit(1)
	}
	fmt.Println(now)
}

I think an example like this might be helpful to include in this project. If there is a better mechanism (perhaps using the pgx/stdlib ?), I would be very interested to know that.

@StevenACoffman StevenACoffman added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Aug 20, 2020
@shubha-rajan shubha-rajan added the priority: p2 Moderately-important priority. Fix may not be included in next release. label Aug 20, 2020
@shubha-rajan
Copy link
Contributor

Hi @StevenACoffman. I agree that the cloudsqlpostgres dialer example should demonstrate connecting with pgx, since lib/pq is no longer being actively developed. Our Go connectivity sample demonstrates connecting using pgx/stdlib and database/sql, but assumes that you're running the proxy in the background which won't work if you prefer not to run a sidecar.
It looks like the example you posted are actually rewriting the code in hook.go to use pgx instead of using the existing postgres dialer to connect. I think the best way to approach this would be to either to write a new postgres dialer that uses pgx, or to update the existing one and replace lib/pq with pgx.

@kurtisvg thoughts?

@kurtisvg
Copy link
Contributor

Replacing lib/pg with pgx is probably a breaking change - I'm not sure what level of compatibility there is between dsn. So probably would need to be a new hook.

I think using DialFunc looks like a reasonable method, although it would be nice if we provided a more convenient dial func than the one currently offered.

@StevenACoffman
Copy link
Author

StevenACoffman commented Aug 20, 2020

Yeah, it is probably better to add a new hook. I'm not sure how you would replicate the current hook's driver's Open function using pgx:

func (d *Driver) Open(name string) (driver.Conn, error) {
	return pq.DialOpen(dialer{}, name)
}

If you have any better idea for a more convenient dial func (that uses context?), I'd love to see that too.

@StevenACoffman
Copy link
Author

StevenACoffman commented Aug 24, 2020

Ok, if I was making a new pgxhook.go in a new package pgxhook? i think it would work like something this:

// Package pgxproxy adds a 'cloudsqlpostgres' driver to use when you want
// to access a Cloud SQL Database via the go database/sql library.
// It is a wrapper over the driver found at github.com/lib/pqx.
// To use this driver, you can look at an example in
// postgres_test package in the hook_test.go file
package pgxproxy

import (
	"context"
	"database/sql"
	"database/sql/driver"
	"fmt"
	"net"
	"regexp"
	"time"

	"github.com/jackc/pgx/v4"
	"github.com/jackc/pgx/v4/stdlib"

	"github.com/GoogleCloudPlatform/cloudsql-proxy/proxy/proxy"
)

func init() {
	sql.Register("cloudsqlpostgres", &Driver{})
}

type Driver struct {
	ctx context.Context
}

// instanceRegexp is used to parse the addr returned by lib/pq.
// lib/pq returns the format '[project:region:instance]:port'
var instanceRegexp = regexp.MustCompile(`^\[(.+)\]:[0-9]+$`)

func DialFunc(ctx context.Context, network string, addr string) (net.Conn, error) {
	matches := instanceRegexp.FindStringSubmatch(addr)
	if len(matches) != 2 {
		return nil, fmt.Errorf("failed to parse addr: %q. It should conform to the regular expression %q", addr, instanceRegexp)
	}
	instance := matches[1]
	return proxy.Dial(instance)
}

func LookupFunc(ctx context.Context, host string) ([]string, error) {
	return []string{host}, nil
}

func DialTimeout(ntw, addr string, timeout time.Duration) (net.Conn, error) {
	return nil, fmt.Errorf("timeout is not currently supported for cloudsqlpostgres dialer")
}

func (d *Driver) WithContext(ctx context.Context) {
	d.ctx = ctx
}

func (d *Driver) Open(dsn string) (driver.Conn, error) {
	config, configErr := pgx.ParseConfig(dsn)
	if configErr != nil {
		return nil, configErr
	}

	config.DialFunc = DialFunc

	config.LookupFunc = LookupFunc
	if d.ctx == nil {
		d.ctx = context.Background()
	}
	connStr := stdlib.RegisterConnConfig(config)

	// pgx? cloudsqlpostgres?
	db, dbErr := sql.Open("pgx", connStr)
	if dbErr != nil {
		return nil, dbErr
	}
	driver := db.Driver()
	// pgx? cloudsqlpostgres?
	return driver.Open("pgx")
}

Not sure about the name string argument for the two Open calls, since this isn't my normal use case.
Exposing the DialFunc publicly would allow people to just use it with a more nuanced config (which is my normal use case). However, I would appreciate exposing a more convenient dial func (with context cancellation and timeout support perhaps?).

@StevenACoffman
Copy link
Author

StevenACoffman commented Aug 24, 2020

The above Open(dsn string) does not work.
I also tried using pgx/stdlib like this:

func (d *Driver) Open(dsn string) (driver.Conn, error) {
	config, configErr := pgx.ParseConfig(dsn)
	if configErr != nil {
		return nil, fmt.Errorf("couldn't parse DB DSN: %w", configErr)
	}

	config.DialFunc = DialFunc

	config.LookupFunc = LookupFunc
	if d.ctx == nil {
		d.ctx = context.Background()
	}

	_ = stdlib.RegisterConnConfig(config)
	return stdlib.GetDefaultDriver().Open(dsn)

So I'm not sure I can put a PR that will get accepted, despite the fact that my use case is a little simpler to solve. Can I get a hand?

@kurtisvg
Copy link
Contributor

Exposing the DialFunc publicly would allow people to just use it with a more nuanced config (which is my normal use case). However, I would appreciate exposing a more convenient dial func (with context cancellation and timeout support perhaps?).

I think right now my preference is exposing a better Dial - I don't know how wide spread support for passing a "DialFunc" is, but it seems common enough in other languages. Creating a wrapper driver feels a bit like a last resort, and doesn't make it easier for the user to switch drivers if they choose. I agree that this new Dial should support context's as well - if you are interested in attempting to do so, please feel free and I'm happy to review the PR. However, given that context's aren't currently used as consistently as they could be - please be aware it might involve a bit of work. I'm planning on making some larger changes related that should also accomplish this, but I can't give a concrete timeline on when something like this might happen except to mention it's nearing the top of my priority queue.

If you would like to directly address this issue and just document the current behavior with the current Dial, I would be open to a PR in the README in cloudsql-proxy/proxy and/or maybe something in examples/pgx.

If you'd prefer to work on a better API surface for Dial immediately, you are welcome to look into adding a DialContext func or similar as well.

Otherwise we can leave this issue open as is since it contains the current instructions for using pgx, and mark it closed when we have a better Dialer available with instructions for a generic driver.

@StevenACoffman
Copy link
Author

Yes, please go for that by all means. I'm having trouble making more time to do this properly, as we're able to limp by with my hacky solution until contexts are used more consistently.

@StevenACoffman
Copy link
Author

StevenACoffman commented Sep 30, 2020

With the DialContext added in #483 connecting to cloudsql instance using in-process cloudsql-proxy with jackc/pgx can be accomplished in a context aware way:

	var (
		host = os.Getenv("DBHOST") // DBHOST for GCP cloudsql is in format project:region:instance
		name = os.Getenv("DBNAME")
		user = os.Getenv("DBUSER")
		pass = os.Getenv("DBPASS")
	)
	var instanceRegexp = regexp.MustCompile(`^\[(.+)\]:[0-9]+$`)

	// In-process Proxy
	// Note that sslmode=disable is required it does not mean that the connection
	// is unencrypted. All connections via the proxy are completely encrypted.
	dsn := fmt.Sprintf("host=%s user=%s password=%s dbname=%s sslmode=disable", host, user, pass, name)

	config, configErr := pgx.ParseConfig(dsn)
	if configErr != nil {
		log.Fatal(configErr)
	}

	// e.g. net.Dialer.DialContext
	config.DialFunc = func(ctx context.Context, network string, addr string) (net.Conn, error) {
		matches := instanceRegexp.FindStringSubmatch(addr)
		if len(matches) != 2 {
			return nil, fmt.Errorf("failed to parse addr: %q. It should conform to the regular expression %q", addr, instanceRegexp)
		}
		instance := matches[1]
		return proxy.DialContext(ctx, instance)
	}
	ctx := context.Background()

	// e.g. net.Resolver.LookupHost
	config.LookupFunc = func(ctx context.Context, host string) ([]string, error) {
		return []string{host}, nil
	}

	conn, connErr := pgx.ConnectConfig(ctx, config)

@enocom enocom added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Feb 9, 2021
@enocom
Copy link
Member

enocom commented Feb 10, 2021

We're currently working on some major structural improvements to the proxy. Once those are done we'll circle back to address support for pgx.

@StevenACoffman
Copy link
Author

StevenACoffman commented Aug 16, 2021

@enocom @kurtisvg Just checking in after 6 months. Our specific use case for cloudsql-proxy is in conjunction with sqlc, which just added support for pgx/v4 connection pool with their respective API usage, which differs from the standard. The details of that are in database/sqlsqlc-dev/sqlc#1037). If this is on the near term roadmap, knowing it would help us with our internal planning.

@enocom
Copy link
Member

enocom commented Aug 16, 2021

Good timing. We've just shared a new dialer which is built with pgx support.

Here's the issue requesting feedback on the new design: #870.

Also, we're planning on incorporating that new dialer into the proxy. Details here: #872.

@enocom
Copy link
Member

enocom commented Sep 7, 2021

And if you're looking to just get something working, here's an example of how to use pgx with the new dialer: https://github.com/kurtisvg/cloud-sql-go-connector#pgx-for-postgres.

@enocom enocom added the v2 label Dec 2, 2021
@enocom
Copy link
Member

enocom commented Dec 2, 2021

Circling back to this. Right now I'd recommend trying out the v2 dialer which makes this a breeze. In addition, we're working on some updated code samples that will highlight how to use pgx with an in-process dialer.

@enocom
Copy link
Member

enocom commented Aug 29, 2022

In addition to the example linked above with the Go connector, we also have an example that hooks up pgx with the database/sql interface: https://github.com/GoogleCloudPlatform/golang-samples/blob/main/cloudsql/postgres/database-sql/connect_connector.go.

Also, v2 is finally available in public preview: https://github.com/GoogleCloudPlatform/cloud-sql-proxy/releases/tag/v2.0.0.preview.0.

@enocom enocom closed this as completed Aug 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

4 participants