From 4281bc80e169e454945d8855c34d9f00a43481d5 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 01:56:57 +0000 Subject: [PATCH 01/15] chore: first pass --- dialer.go | 4 ++-- dialer_test.go | 2 +- internal/alloydb/instance.go | 2 +- internal/alloydb/instance_test.go | 2 +- internal/alloydb/refresh.go | 9 ++++++--- internal/alloydb/refresh_test.go | 2 +- internal/mock/alloydb.go | 2 +- internal/mock/alloydbadmin.go | 2 +- metrics_test.go | 2 +- options.go | 1 + 10 files changed, 16 insertions(+), 12 deletions(-) diff --git a/dialer.go b/dialer.go index 643e5f22..76ba8e6c 100644 --- a/dialer.go +++ b/dialer.go @@ -30,8 +30,8 @@ import ( "sync/atomic" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" - "cloud.google.com/go/alloydb/connectors/apiv1beta/connectorspb" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" + "cloud.google.com/go/alloydb/connectors/apiv1alpha/connectorspb" "cloud.google.com/go/alloydbconn/errtype" "cloud.google.com/go/alloydbconn/internal/alloydb" "cloud.google.com/go/alloydbconn/internal/trace" diff --git a/dialer_test.go b/dialer_test.go index 1f64e88d..37d0097b 100644 --- a/dialer_test.go +++ b/dialer_test.go @@ -28,7 +28,7 @@ import ( "testing" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" "cloud.google.com/go/alloydbconn/errtype" "cloud.google.com/go/alloydbconn/internal/alloydb" "cloud.google.com/go/alloydbconn/internal/mock" diff --git a/internal/alloydb/instance.go b/internal/alloydb/instance.go index d86b5640..6bf49c45 100644 --- a/internal/alloydb/instance.go +++ b/internal/alloydb/instance.go @@ -23,7 +23,7 @@ import ( "sync" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" "cloud.google.com/go/alloydbconn/errtype" "golang.org/x/time/rate" ) diff --git a/internal/alloydb/instance_test.go b/internal/alloydb/instance_test.go index f3a9c149..b2f9eef4 100644 --- a/internal/alloydb/instance_test.go +++ b/internal/alloydb/instance_test.go @@ -23,7 +23,7 @@ import ( "testing" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" "cloud.google.com/go/alloydbconn/errtype" "cloud.google.com/go/alloydbconn/internal/mock" "golang.org/x/oauth2" diff --git a/internal/alloydb/refresh.go b/internal/alloydb/refresh.go index 1c131ac8..36e81dbb 100644 --- a/internal/alloydb/refresh.go +++ b/internal/alloydb/refresh.go @@ -26,8 +26,8 @@ import ( "strings" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" - "cloud.google.com/go/alloydb/apiv1beta/alloydbpb" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" + "cloud.google.com/go/alloydb/apiv1alpha/alloydbpb" "cloud.google.com/go/alloydbconn/errtype" "cloud.google.com/go/alloydbconn/internal/trace" "google.golang.org/protobuf/types/known/durationpb" @@ -36,6 +36,9 @@ import ( type connectInfo struct { // ipAddr is the instance's IP addresses ipAddr string + // publicIpAddress is the instance's public IP address. Will be an empty + // string if instance does not have public IP configured. + publicIpAddress string // uid is the instance UID uid string } @@ -56,7 +59,7 @@ func fetchMetadata(ctx context.Context, cl *alloydbadmin.AlloyDBAdminClient, ins if err != nil { return connectInfo{}, errtype.NewRefreshError("failed to get instance metadata", inst.String(), err) } - return connectInfo{ipAddr: resp.IpAddress, uid: resp.InstanceUid}, nil + return connectInfo{ipAddr: resp.IpAddress, publicIpAddress: resp.GetPublicIpAddress(), uid: resp.InstanceUid}, nil } var errInvalidPEM = errors.New("certificate is not a valid PEM") diff --git a/internal/alloydb/refresh_test.go b/internal/alloydb/refresh_test.go index 0481e076..b4f716e3 100644 --- a/internal/alloydb/refresh_test.go +++ b/internal/alloydb/refresh_test.go @@ -20,7 +20,7 @@ import ( "testing" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" "cloud.google.com/go/alloydbconn/internal/mock" "google.golang.org/api/option" ) diff --git a/internal/mock/alloydb.go b/internal/mock/alloydb.go index b3295d94..31f2647c 100644 --- a/internal/mock/alloydb.go +++ b/internal/mock/alloydb.go @@ -28,7 +28,7 @@ import ( "testing" "time" - "cloud.google.com/go/alloydb/connectors/apiv1beta/connectorspb" + "cloud.google.com/go/alloydb/connectors/apiv1alpha/connectorspb" "google.golang.org/protobuf/proto" ) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 9bc29d8e..e502979e 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -28,7 +28,7 @@ import ( "sync" "time" - "cloud.google.com/go/alloydb/apiv1beta/alloydbpb" + "cloud.google.com/go/alloydb/apiv1alpha/alloydbpb" "google.golang.org/protobuf/encoding/protojson" ) diff --git a/metrics_test.go b/metrics_test.go index 890bf669..f4dcb5bd 100644 --- a/metrics_test.go +++ b/metrics_test.go @@ -19,7 +19,7 @@ import ( "testing" "time" - alloydbadmin "cloud.google.com/go/alloydb/apiv1beta" + alloydbadmin "cloud.google.com/go/alloydb/apiv1alpha" "cloud.google.com/go/alloydbconn/internal/mock" "go.opencensus.io/stats/view" "google.golang.org/api/option" diff --git a/options.go b/options.go index fc653370..bdcd64b8 100644 --- a/options.go +++ b/options.go @@ -167,6 +167,7 @@ type DialOption func(d *dialCfg) type dialCfg struct { dialFunc func(ctx context.Context, network, addr string) (net.Conn, error) + ipType string tcpKeepAlive time.Duration } From 9a9b84d2c1fc59afbe8d65b3cade6c887937375f Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 13:31:46 +0000 Subject: [PATCH 02/15] chore: use map for ipAddrs --- dialer.go | 7 +++--- internal/alloydb/instance.go | 15 ++++++++++--- internal/alloydb/refresh.go | 42 ++++++++++++++++++++++++++---------- options.go | 15 +++++++++++++ 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/dialer.go b/dialer.go index 76ba8e6c..e82ffb63 100644 --- a/dialer.go +++ b/dialer.go @@ -75,7 +75,7 @@ func getDefaultKeys() (*rsa.PrivateKey, error) { type connectionInfoCache interface { OpenConns() *uint64 - ConnectInfo(context.Context) (string, *tls.Config, error) + ConnectInfo(context.Context, string) (string, *tls.Config, error) ForceRefresh() io.Closer } @@ -156,6 +156,7 @@ func NewDialer(ctx context.Context, opts ...Option) (*Dialer, error) { } dialCfg := dialCfg{ + ipType: alloydb.PrivateIP, tcpKeepAlive: defaultTCPKeepAlive, } for _, opt := range cfg.dialOpts { @@ -211,7 +212,7 @@ func (d *Dialer) Dial(ctx context.Context, instance string, opts ...DialOption) endInfo(err) return nil, err } - addr, tlsCfg, err := i.ConnectInfo(ctx) + addr, tlsCfg, err := i.ConnectInfo(ctx, cfg.ipType) if err != nil { d.lock.Lock() defer d.lock.Unlock() @@ -231,7 +232,7 @@ func (d *Dialer) Dial(ctx context.Context, instance string, opts ...DialOption) if invalidClientCert(tlsCfg) { i.ForceRefresh() // Block on refreshed connection info - addr, tlsCfg, err = i.ConnectInfo(ctx) + addr, tlsCfg, err = i.ConnectInfo(ctx, cfg.ipType) if err != nil { d.lock.Lock() defer d.lock.Unlock() diff --git a/internal/alloydb/instance.go b/internal/alloydb/instance.go index 6bf49c45..264c7c9f 100644 --- a/internal/alloydb/instance.go +++ b/internal/alloydb/instance.go @@ -193,13 +193,22 @@ func (i *Instance) Close() error { return nil } -// ConnectInfo returns an IP address of the AlloyDB instance. -func (i *Instance) ConnectInfo(ctx context.Context) (string, *tls.Config, error) { +// ConnectInfo returns an IP address specified by ipType (i.e., public or +// private) of the AlloyDB instance. +func (i *Instance) ConnectInfo(ctx context.Context, ipType string) (string, *tls.Config, error) { res, err := i.result(ctx) if err != nil { return "", nil, err } - return res.result.instanceIPAddr, res.result.conf, nil + addr, ok = res.result.ipAddrs[ipType] + if !ok { + err := errtype.NewConfigError( + fmt.Sprintf("instance does not have IP of type %q", ipType), + i.instanceURI.String(), + ) + return "", nil, err + } + return addr, res.result.conf, nil } // ForceRefresh triggers an immediate refresh operation to be scheduled and diff --git a/internal/alloydb/refresh.go b/internal/alloydb/refresh.go index 36e81dbb..d9762ac7 100644 --- a/internal/alloydb/refresh.go +++ b/internal/alloydb/refresh.go @@ -33,12 +33,16 @@ import ( "google.golang.org/protobuf/types/known/durationpb" ) +const ( + // PublicIP is the value for public IP connections. + PublicIP = "PUBLIC" + // PrivateIP is the value for private IP connections. + PrivateIP = "PRIVATE" +) + type connectInfo struct { - // ipAddr is the instance's IP addresses - ipAddr string - // publicIpAddress is the instance's public IP address. Will be an empty - // string if instance does not have public IP configured. - publicIpAddress string + // ipAddrs is the instance's IP addresses + ipAddrs map[string]string // uid is the instance UID uid string } @@ -59,7 +63,23 @@ func fetchMetadata(ctx context.Context, cl *alloydbadmin.AlloyDBAdminClient, ins if err != nil { return connectInfo{}, errtype.NewRefreshError("failed to get instance metadata", inst.String(), err) } - return connectInfo{ipAddr: resp.IpAddress, publicIpAddress: resp.GetPublicIpAddress(), uid: resp.InstanceUid}, nil + + // parse any ip addresses that might be used to connect + ipAddrs := make(map[string]string) + if resp.GetIpAddress() != "" { + ipAddrs[PrivateIP] = resp.GetIpAddress() + } + if resp.GetPublicIpAddress() != "" { + ipAddrs[PublicIP] = resp.GetPublicIpAddress() + } + + if len(ipAddrs) == 0 { + return connectInfo{}, errtype.NewConfigError( + "cannot connect to instance - it has no supported IP addresses", + inst.String(), + ) + } + return connectInfo{ipAddrs: ipAddrs, uid: resp.InstanceUid}, nil } var errInvalidPEM = errors.New("certificate is not a valid PEM") @@ -187,9 +207,9 @@ type refresher struct { } type refreshResult struct { - instanceIPAddr string - conf *tls.Config - expiry time.Time + ipAddrs map[string]string + conf *tls.Config + expiry time.Time } type certs struct { @@ -257,9 +277,9 @@ func (r refresher) performRefresh(ctx context.Context, cn InstanceURI, k *rsa.Pr c := &tls.Config{ Certificates: []tls.Certificate{cc.certChain}, RootCAs: caCerts, - ServerName: info.ipAddr, + ServerName: info.ipAddrs[PrivateIP], MinVersion: tls.VersionTLS13, } - return refreshResult{instanceIPAddr: info.ipAddr, conf: c, expiry: cc.expiry}, nil + return refreshResult{ipAddrs: info.ipAddrs, conf: c, expiry: cc.expiry}, nil } diff --git a/options.go b/options.go index bdcd64b8..b77acb84 100644 --- a/options.go +++ b/options.go @@ -23,6 +23,7 @@ import ( "time" "cloud.google.com/go/alloydbconn/errtype" + "cloud.google.com/go/alloydbconn/internal/alloydb" "golang.org/x/oauth2" "golang.org/x/oauth2/google" apiopt "google.golang.org/api/option" @@ -195,3 +196,17 @@ func WithTCPKeepAlive(d time.Duration) DialOption { cfg.tcpKeepAlive = d } } + +// WithPublicIP returns a DialOption that specifies a public IP will be used to connect. +func WithPublicIP() DialOption { + return func(cfg *dialCfg) { + cfg.ipType = alloydb.PublicIP + } +} + +// WithPrivateIP returns a DialOption that specifies a private IP (VPC) will be used to connect. +func WithPrivateIP() DialOption { + return func(cfg *dialCfg) { + cfg.ipType = alloydb.PrivateIP + } +} From 2ad7367087dbb9c7ba2e0b40d74ec2f74e4a063c Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 13:39:54 +0000 Subject: [PATCH 03/15] chore: add type for addr --- internal/alloydb/instance.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/alloydb/instance.go b/internal/alloydb/instance.go index 264c7c9f..3a35557c 100644 --- a/internal/alloydb/instance.go +++ b/internal/alloydb/instance.go @@ -200,6 +200,10 @@ func (i *Instance) ConnectInfo(ctx context.Context, ipType string) (string, *tls if err != nil { return "", nil, err } + var ( + addr string + ok bool + ) addr, ok = res.result.ipAddrs[ipType] if !ok { err := errtype.NewConfigError( From 8be46266f06952522741e605e009002418019f97 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 15:30:53 +0000 Subject: [PATCH 04/15] chore: update existing tests --- dialer.go | 2 +- dialer_test.go | 2 +- internal/alloydb/instance_test.go | 14 ++++++++++---- internal/alloydb/refresh_test.go | 21 +++++++++++++++++---- internal/mock/alloydb.go | 18 ++++++++++++------ internal/mock/alloydbadmin.go | 18 +++++++++++++++++- 6 files changed, 58 insertions(+), 17 deletions(-) diff --git a/dialer.go b/dialer.go index e82ffb63..64d88017 100644 --- a/dialer.go +++ b/dialer.go @@ -156,7 +156,7 @@ func NewDialer(ctx context.Context, opts ...Option) (*Dialer, error) { } dialCfg := dialCfg{ - ipType: alloydb.PrivateIP, + ipType: alloydb.PrivateIP, tcpKeepAlive: defaultTCPKeepAlive, } for _, opt := range cfg.dialOpts { diff --git a/dialer_test.go b/dialer_test.go index 37d0097b..4f934d83 100644 --- a/dialer_test.go +++ b/dialer_test.go @@ -341,7 +341,7 @@ type spyConnectionInfoCache struct { connectionInfoCache } -func (s *spyConnectionInfoCache) ConnectInfo(_ context.Context) (string, *tls.Config, error) { +func (s *spyConnectionInfoCache) ConnectInfo(_ context.Context, _ string) (string, *tls.Config, error) { s.mu.Lock() defer s.mu.Unlock() res := s.connectInfoCalls[s.connectInfoIndex] diff --git a/internal/alloydb/instance_test.go b/internal/alloydb/instance_test.go index b2f9eef4..d4bf0480 100644 --- a/internal/alloydb/instance_test.go +++ b/internal/alloydb/instance_test.go @@ -128,7 +128,7 @@ func TestConnectInfo(t *testing.T) { wantAddr := "0.0.0.0" inst := mock.NewFakeInstance( "my-project", "my-region", "my-cluster", "my-instance", - mock.WithIPAddr(wantAddr), + mock.WithPrivateIP(wantAddr), ) mc, url, cleanup := mock.HTTPClient( mock.InstanceGetSuccess(inst, 1), @@ -157,7 +157,7 @@ func TestConnectInfo(t *testing.T) { t.Fatalf("failed to create mock instance: %v", err) } - gotAddr, _, err := i.ConnectInfo(ctx) + gotAddr, _, err := i.ConnectInfo(ctx, PrivateIP) if err != nil { t.Fatalf("failed to retrieve connect info: %v", err) } @@ -191,11 +191,17 @@ func TestConnectInfoErrors(t *testing.T) { t.Fatalf("failed to initialize Instance: %v", err) } - _, _, err = i.ConnectInfo(ctx) + _, _, err = i.ConnectInfo(ctx, PrivateIP) var wantErr *errtype.DialError if !errors.As(err, &wantErr) { t.Fatalf("when connect info fails, want = %T, got = %v", wantErr, err) } + + // when client asks for wrong IP address type + gotAddr, _, err := i.ConnectInfo(ctx, PublicIP) + if err == nil { + t.Fatalf("expected ConnectInfo to fail but returned IP address = %v", gotAddr) + } } func TestClose(t *testing.T) { @@ -215,7 +221,7 @@ func TestClose(t *testing.T) { } i.Close() - _, _, err = i.ConnectInfo(ctx) + _, _, err = i.ConnectInfo(ctx, PrivateIP) if !strings.Contains(err.Error(), "context was canceled or expired") { t.Fatalf("failed to retrieve connect info: %v", err) } diff --git a/internal/alloydb/refresh_test.go b/internal/alloydb/refresh_test.go index b4f716e3..37c458a9 100644 --- a/internal/alloydb/refresh_test.go +++ b/internal/alloydb/refresh_test.go @@ -28,7 +28,8 @@ import ( const testDialerID = "some-dialer-id" func TestRefresh(t *testing.T) { - wantIP := "10.0.0.1" + wantPrivateIP := "10.0.0.1" + wantPublicIP := "127.0.0.1" wantExpiry := time.Now().Add(time.Hour).UTC().Round(time.Second) wantInstURI := "/projects/my-project/locations/my-region/clusters/my-cluster/instances/my-instance" cn, err := ParseInstURI(wantInstURI) @@ -37,7 +38,8 @@ func TestRefresh(t *testing.T) { } inst := mock.NewFakeInstance( "my-project", "my-region", "my-cluster", "my-instance", - mock.WithIPAddr(wantIP), + mock.WithPrivateIP(wantPrivateIP), + mock.WithPublicIP(wantPublicIP), mock.WithCertExpiry(wantExpiry), ) mc, url, cleanup := mock.HTTPClient( @@ -64,8 +66,19 @@ func TestRefresh(t *testing.T) { t.Fatalf("performRefresh unexpectedly failed with error: %v", err) } - if got := res.instanceIPAddr; wantIP != got { - t.Fatalf("metadata IP mismatch, want = %v, got = %v", wantIP, got) + gotIP, ok := res.ipAddrs[PrivateIP] + if !ok { + t.Fatal("metadata IP addresses did not include private address") + } + if wantPrivateIP != gotIP { + t.Fatalf("metadata IP mismatch, want = %v, got = %v", wantPrivateIP, gotIP) + } + gotIP, ok = res.ipAddrs[PublicIP] + if !ok { + t.Fatal("metadata IP addresses did not include public address") + } + if wantPublicIP != gotIP { + t.Fatalf("metadata IP mismatch, want = %v, got = %v", wantPublicIP, gotIP) } if got := res.expiry; wantExpiry != got { t.Fatalf("expiry mismatch, want = %v, got = %v", wantExpiry, got) diff --git a/internal/mock/alloydb.go b/internal/mock/alloydb.go index 31f2647c..2abd45ed 100644 --- a/internal/mock/alloydb.go +++ b/internal/mock/alloydb.go @@ -35,13 +35,19 @@ import ( // Option configures a FakeAlloyDBInstance type Option func(*FakeAlloyDBInstance) -// WithIPAddr sets the IP address of the instance. -func WithIPAddr(addr string) Option { +// WithPublicIP sets the public IP address to addr. +func WithPublicIP(addr string) Option { return func(f *FakeAlloyDBInstance) { - f.ipAddr = addr + f.ipAddrs["PUBLIC"] = addr } } +// WithPrivateIP sets the private IP address to addr. +func WithPrivateIP(addr string) Option { + return func(f *FakeAlloyDBInstance) { + f.ipAddrs["PRIVATE"] = addr + } +} // WithServerName sets the name that server uses to identify itself in the TLS // handshake. func WithServerName(name string) Option { @@ -63,8 +69,8 @@ type FakeAlloyDBInstance struct { region string cluster string name string - - ipAddr string + // ipAddrs is a map of IP type (PUBLIC or PRIVATE) to IP address. + ipAddrs map[string]string uid string serverName string certExpiry time.Time @@ -100,7 +106,7 @@ func NewFakeInstance(proj, reg, clust, name string, opts ...Option) FakeAlloyDBI region: reg, cluster: clust, name: name, - ipAddr: "127.0.0.1", + ipAddrs: map[string]string{"PRIVATE": "127.0.0.1"}, uid: "00000000-0000-0000-0000-000000000000", serverName: "00000000-0000-0000-0000-000000000000.server.alloydb", certExpiry: time.Now().Add(24 * time.Hour), diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index e502979e..8e5c12d2 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -67,13 +67,29 @@ func (r *Request) matches(hR *http.Request) bool { func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { p := fmt.Sprintf("/v1beta/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", i.project, i.region, i.cluster, i.name) + + res := map[string]string{} + for ipType, addr := range i.ipAddrs { + if ipType == "PRIVATE" { + res["ipAddress"] = addr + continue + } + if ipType == "PUBLIC" { + res["publicIpAddress"] = addr + } + } + res["instanceUid"] = i.uid + jsonString, err := json.Marshal(res) + if err != nil { + panic(err) + } return &Request{ reqMethod: http.MethodGet, reqPath: p, reqCt: ct, handle: func(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(http.StatusOK) - resp.Write([]byte(fmt.Sprintf(`{"ipAddress":"%s","instanceUid":"%s"}`, i.ipAddr, i.uid))) + resp.Write(jsonString) }, } } From 02a9d34e4f3271569a17fc19763989b455161dc7 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 15:45:19 +0000 Subject: [PATCH 05/15] chore: lint --- internal/alloydb/instance.go | 2 +- internal/mock/alloydbadmin.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/alloydb/instance.go b/internal/alloydb/instance.go index 3a35557c..5c4b35b1 100644 --- a/internal/alloydb/instance.go +++ b/internal/alloydb/instance.go @@ -193,7 +193,7 @@ func (i *Instance) Close() error { return nil } -// ConnectInfo returns an IP address specified by ipType (i.e., public or +// ConnectInfo returns an IP address specified by ipType (i.e., public or // private) of the AlloyDB instance. func (i *Instance) ConnectInfo(ctx context.Context, ipType string) (string, *tls.Config, error) { res, err := i.result(ctx) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 8e5c12d2..2b1fbedc 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -67,7 +67,7 @@ func (r *Request) matches(hR *http.Request) bool { func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { p := fmt.Sprintf("/v1beta/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", i.project, i.region, i.cluster, i.name) - + res := map[string]string{} for ipType, addr := range i.ipAddrs { if ipType == "PRIVATE" { From 791a946745215245becb2cdd8802a08c93d3dd39 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 15:59:00 +0000 Subject: [PATCH 06/15] chore: lint --- internal/mock/alloydb.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/mock/alloydb.go b/internal/mock/alloydb.go index 2abd45ed..dd5dadf5 100644 --- a/internal/mock/alloydb.go +++ b/internal/mock/alloydb.go @@ -48,6 +48,7 @@ func WithPrivateIP(addr string) Option { f.ipAddrs["PRIVATE"] = addr } } + // WithServerName sets the name that server uses to identify itself in the TLS // handshake. func WithServerName(name string) Option { From 346ecffcb737fc4c484e0db8cddee28a7bb873af Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 16:05:19 +0000 Subject: [PATCH 07/15] chore: fix response attribute --- internal/mock/alloydbadmin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 2b1fbedc..f6c51325 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -75,7 +75,7 @@ func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { continue } if ipType == "PUBLIC" { - res["publicIpAddress"] = addr + res["PublicIpAddress"] = addr } } res["instanceUid"] = i.uid From cfe2e3e996ba48a080ee85034c182fbe992a3993 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Wed, 17 Jan 2024 18:39:04 +0000 Subject: [PATCH 08/15] chore: fix json --- internal/mock/alloydbadmin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index f6c51325..2b1fbedc 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -75,7 +75,7 @@ func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { continue } if ipType == "PUBLIC" { - res["PublicIpAddress"] = addr + res["publicIpAddress"] = addr } } res["instanceUid"] = i.uid From ac44f62acd21bb1321703d55aedabd65ae502732 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 02:08:48 +0000 Subject: [PATCH 09/15] chore: add e2e test --- e2e_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/e2e_test.go b/e2e_test.go index c5931273..69acbdfb 100644 --- a/e2e_test.go +++ b/e2e_test.go @@ -227,3 +227,41 @@ func TestAutoIAMAuthN(t *testing.T) { } t.Log(tt) } + + +func TestPublicIP(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + ctx := context.Background() + + d, err := alloydbconn.NewDialer(ctx) + if err != nil { + t.Fatalf("failed to init Dialer: %v", err) + } + + dsn := fmt.Sprintf( + "user=%s password=%s dbname=%s sslmode=disable", + alloydbUser, alloydbPass, alloydbDB, + ) + config, err := pgx.ParseConfig(dsn) + if err != nil { + t.Fatalf("failed to parse pgx config: %v", err) + } + + config.DialFunc = func(ctx context.Context, network string, instance string) (net.Conn, error) { + return d.Dial(ctx, alloydbInstanceName, alloydbconn.WithPublicIP()) + } + + conn, connErr := pgx.ConnectConfig(ctx, config) + if connErr != nil { + t.Fatalf("failed to connect: %s", connErr) + } + defer conn.Close(ctx) + + var tt time.Time + if err := conn.QueryRow(context.Background(), "SELECT NOW()").Scan(&tt); err != nil { + t.Fatal(err) + } + t.Log(tt) +} From 2b18059943522fadc2e5fbe6c4891ea707ddcb23 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 02:33:28 +0000 Subject: [PATCH 10/15] chore: lint --- e2e_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/e2e_test.go b/e2e_test.go index 69acbdfb..dea1e0c0 100644 --- a/e2e_test.go +++ b/e2e_test.go @@ -228,7 +228,6 @@ func TestAutoIAMAuthN(t *testing.T) { t.Log(tt) } - func TestPublicIP(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") From 4c8fec7ec60f37bc5ac3cc67ef5fa2c1520e0e52 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 14:48:20 +0000 Subject: [PATCH 11/15] chore: revert alloydbadmin changes to fake --- internal/mock/alloydbadmin.go | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 2b1fbedc..0df49f01 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -67,29 +67,13 @@ func (r *Request) matches(hR *http.Request) bool { func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { p := fmt.Sprintf("/v1beta/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", i.project, i.region, i.cluster, i.name) - - res := map[string]string{} - for ipType, addr := range i.ipAddrs { - if ipType == "PRIVATE" { - res["ipAddress"] = addr - continue - } - if ipType == "PUBLIC" { - res["publicIpAddress"] = addr - } - } - res["instanceUid"] = i.uid - jsonString, err := json.Marshal(res) - if err != nil { - panic(err) - } return &Request{ reqMethod: http.MethodGet, reqPath: p, reqCt: ct, handle: func(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(http.StatusOK) - resp.Write(jsonString) + resp.Write([]byte(fmt.Sprintf(`{"ipAddress":"%s","instanceUid":"%s"}`, i.ipAddrs["PRIVATE"], i.uid))) }, } } From 67fdde6c6ab8947750aefebd54ed4cdbd8a9b311 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 14:55:51 +0000 Subject: [PATCH 12/15] chore: re-add fake metadata success --- internal/mock/alloydbadmin.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 0df49f01..2b1fbedc 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -67,13 +67,29 @@ func (r *Request) matches(hR *http.Request) bool { func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { p := fmt.Sprintf("/v1beta/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", i.project, i.region, i.cluster, i.name) + + res := map[string]string{} + for ipType, addr := range i.ipAddrs { + if ipType == "PRIVATE" { + res["ipAddress"] = addr + continue + } + if ipType == "PUBLIC" { + res["publicIpAddress"] = addr + } + } + res["instanceUid"] = i.uid + jsonString, err := json.Marshal(res) + if err != nil { + panic(err) + } return &Request{ reqMethod: http.MethodGet, reqPath: p, reqCt: ct, handle: func(resp http.ResponseWriter, req *http.Request) { resp.WriteHeader(http.StatusOK) - resp.Write([]byte(fmt.Sprintf(`{"ipAddress":"%s","instanceUid":"%s"}`, i.ipAddrs["PRIVATE"], i.uid))) + resp.Write(jsonString) }, } } From 5ed0a66257e76228917db1cce75676816ac50142 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 18:48:11 +0000 Subject: [PATCH 13/15] chore: update fake to v1alpha --- internal/mock/alloydbadmin.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/mock/alloydbadmin.go b/internal/mock/alloydbadmin.go index 2b1fbedc..4af660dd 100644 --- a/internal/mock/alloydbadmin.go +++ b/internal/mock/alloydbadmin.go @@ -65,7 +65,7 @@ func (r *Request) matches(hR *http.Request) bool { // InstanceGetSuccess returns a Request that responds to the `instance.get` // AlloyDB Admin API endpoint. func InstanceGetSuccess(i FakeAlloyDBInstance, ct int) *Request { - p := fmt.Sprintf("/v1beta/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", + p := fmt.Sprintf("/v1alpha/projects/%s/locations/%s/clusters/%s/instances/%s/connectionInfo", i.project, i.region, i.cluster, i.name) res := map[string]string{} @@ -100,7 +100,7 @@ func CreateEphemeralSuccess(i FakeAlloyDBInstance, ct int) *Request { return &Request{ reqMethod: http.MethodPost, reqPath: fmt.Sprintf( - "/v1beta/projects/%s/locations/%s/clusters/%s:generateClientCertificate", + "/v1alpha/projects/%s/locations/%s/clusters/%s:generateClientCertificate", i.project, i.region, i.cluster), reqCt: ct, handle: func(resp http.ResponseWriter, req *http.Request) { From 38d32379be99b20ff27a9ecb95e1285fb06d0789 Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Thu, 18 Jan 2024 19:36:19 +0000 Subject: [PATCH 14/15] chore: add public IP to README --- README.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index f3a16235..c0321f4e 100644 --- a/README.md +++ b/README.md @@ -29,6 +29,7 @@ language. Using an AlloyDB connector provides the following benefits: ## Installation You can install this repo with `go get`: + ```sh go get cloud.google.com/go/alloydbconn ``` @@ -39,9 +40,11 @@ This package provides several functions for authorizing and encrypting connections. These functions can be used with your database driver to connect to your AlloyDB instance. -AlloyDB supports network connectivity through private, internal IP addresses only. -This package must be run in an environment that is connected to the -[VPC Network][vpc] that hosts your AlloyDB private IP address. +AlloyDB supports network connectivity through public IP addresses and private, +internal IP addresses. By default this package will attempt to connect over a +private IP connection. When doing so, this package must be run in an +environment that is connected to the [VPC Network][vpc] that hosts your +AlloyDB private IP address. Please see [Configuring AlloyDB Connectivity][alloydb-connectivity] for more details. @@ -52,12 +55,12 @@ Please see [Configuring AlloyDB Connectivity][alloydb-connectivity] for more det This package requires the following to connect successfully: -- IAM principal (user, service account, etc.) with the [AlloyDB +* IAM principal (user, service account, etc.) with the [AlloyDB Client and Service Usage Consumer][client-role] roles or equivalent permissions. [Credentials](#credentials) for the IAM principal are used to authorize connections to an AlloyDB instance. -- The [AlloyDB Admin API][admin-api] to be enabled within your Google Cloud +* The [AlloyDB Admin API][admin-api] to be enabled within your Google Cloud Project. By default, the API will be called in the project associated with the IAM principal. @@ -136,14 +139,14 @@ For a full list of customizable behavior, see alloydbconn.Option. ### Using DialOptions -If you want to customize things about how the connection is created, use -`DialOption`: +If you want to customize things about how the connection is created, such as +connecting to AlloyDB over a public IP, use a `DialOption`: ```go conn, err := d.Dial( ctx, "projects//locations//clusters//instances/", - alloydbconn.WithTCPKeepAlive(30*time.Second), + alloydbconn.WithPublicIP(), ) ``` @@ -154,7 +157,7 @@ be used by default: d, err := alloydbconn.NewDialer( ctx, alloydbconn.WithDefaultDialOptions( - alloydbconn.WithTCPKeepAlive(30*time.Second), + alloydbconn.WithPublicIP(), ), ) ``` From af2b200525c1c59bb938591a6fdc896689e698ff Mon Sep 17 00:00:00 2001 From: jackwotherspoon Date: Mon, 29 Jan 2024 18:20:17 +0000 Subject: [PATCH 15/15] chore: merge main --- internal/alloydb/instance.go | 20 ++++++++++++++------ internal/alloydb/instance_test.go | 3 +-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/internal/alloydb/instance.go b/internal/alloydb/instance.go index 5c4b35b1..3279009f 100644 --- a/internal/alloydb/instance.go +++ b/internal/alloydb/instance.go @@ -189,7 +189,11 @@ func (i *Instance) OpenConns() *uint64 { // Close closes the instance; it stops the refresh cycle and prevents it from // making additional calls to the AlloyDB Admin API. func (i *Instance) Close() error { + i.resultGuard.Lock() + defer i.resultGuard.Unlock() i.cancel() + i.cur.cancel() + i.next.cancel() return nil } @@ -243,6 +247,8 @@ func (i *Instance) result(ctx context.Context) (*refreshOperation, error) { err = res.err case <-ctx.Done(): err = ctx.Err() + case <-i.ctx.Done(): + err = i.ctx.Err() } if err != nil { return nil, err @@ -273,6 +279,13 @@ func (i *Instance) scheduleRefresh(d time.Duration) *refreshOperation { r := &refreshOperation{} r.ready = make(chan struct{}) r.timer = time.AfterFunc(d, func() { + // instance has been closed, don't schedule anything + if err := i.ctx.Err(); err != nil { + r.err = err + close(r.ready) + return + } + ctx, cancel := context.WithTimeout(i.ctx, i.refreshTimeout) defer cancel() @@ -293,6 +306,7 @@ func (i *Instance) scheduleRefresh(d time.Duration) *refreshOperation { // result and schedule a new refresh i.resultGuard.Lock() defer i.resultGuard.Unlock() + // if failed, scheduled the next refresh immediately if r.err != nil { i.next = i.scheduleRefresh(0) @@ -310,12 +324,6 @@ func (i *Instance) scheduleRefresh(d time.Duration) *refreshOperation { // Update the current results, and schedule the next refresh in // the future i.cur = r - select { - case <-i.ctx.Done(): - // instance has been closed, don't schedule anything - return - default: - } t := refreshDuration(time.Now(), i.cur.result.expiry) i.next = i.scheduleRefresh(t) }) diff --git a/internal/alloydb/instance_test.go b/internal/alloydb/instance_test.go index d4bf0480..d8d4084c 100644 --- a/internal/alloydb/instance_test.go +++ b/internal/alloydb/instance_test.go @@ -19,7 +19,6 @@ import ( "crypto/rand" "crypto/rsa" "errors" - "strings" "testing" "time" @@ -222,7 +221,7 @@ func TestClose(t *testing.T) { i.Close() _, _, err = i.ConnectInfo(ctx, PrivateIP) - if !strings.Contains(err.Error(), "context was canceled or expired") { + if !errors.Is(err, context.Canceled) { t.Fatalf("failed to retrieve connect info: %v", err) } }