Skip to content

Commit 4af819d

Browse files
committed
fix: return error with conflicting credential opts
This commit ensures conflicting credential options result in a user-facing error. This commit moves all the dialer config initialization into a single function in options.go. This will help the migration to supporting auth.Credentials and makes the relationship between all the options clear. Previously, there was a mix of option functions setting various fields, possibly returning errors, and the dialer initializer doing additional configuration work. This made it hard to understand how the different credential options related to one another and hid the bug of allowing callers to set conflicting options. In addition, this commit adds end to end tests to verify the credential-related options work as intended. This is a belated port of GoogleCloudPlatform/cloud-sql-go-connector#460. Related to #646
1 parent c42792b commit 4af819d

File tree

4 files changed

+271
-84
lines changed

4 files changed

+271
-84
lines changed

credentials_test.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// Copyright 2025 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package alloydbconn_test
16+
17+
import (
18+
"context"
19+
"fmt"
20+
"net"
21+
"os"
22+
"testing"
23+
"time"
24+
25+
"cloud.google.com/go/alloydbconn"
26+
"github.com/jackc/pgx/v5/pgxpool"
27+
"golang.org/x/oauth2"
28+
"golang.org/x/oauth2/google"
29+
)
30+
31+
// removeAuthEnvVar retrieves an OAuth2 token and a path to a service account key
32+
// and then unsets GOOGLE_APPLICATION_CREDENTIALS. It returns a cleanup function
33+
// that restores the original setup.
34+
func removeAuthEnvVar(t *testing.T) (*oauth2.Token, string, func()) {
35+
ts, err := google.DefaultTokenSource(context.Background(),
36+
"https://www.googleapis.com/auth/cloud-platform",
37+
)
38+
if err != nil {
39+
t.Errorf("failed to resolve token source: %v", err)
40+
}
41+
42+
tok, err := ts.Token()
43+
if err != nil {
44+
t.Errorf("failed to get token: %v", err)
45+
}
46+
path, ok := os.LookupEnv("GOOGLE_APPLICATION_CREDENTIALS")
47+
if !ok {
48+
t.Fatalf("GOOGLE_APPLICATION_CREDENTIALS was not set in the environment")
49+
}
50+
if err := os.Unsetenv("GOOGLE_APPLICATION_CREDENTIALS"); err != nil {
51+
t.Fatalf("failed to unset GOOGLE_APPLICATION_CREDENTIALS")
52+
}
53+
return tok, path, func() {
54+
os.Setenv("GOOGLE_APPLICATION_CREDENTIALS", path)
55+
}
56+
}
57+
58+
func keyfile(t *testing.T) string {
59+
path := os.Getenv("GOOGLE_APPLICATION_CREDENTIALS")
60+
if path == "" {
61+
t.Fatal("GOOGLE_APPLICATION_CREDENTIALS not set")
62+
}
63+
creds, err := os.ReadFile(path)
64+
if err != nil {
65+
t.Fatalf("io.ReadAll(): %v", err)
66+
}
67+
return string(creds)
68+
}
69+
70+
func TestAuthenticationOptions(t *testing.T) {
71+
if testing.Short() {
72+
t.Skip("skipping credential integration tests")
73+
}
74+
requireAlloyDBVars(t)
75+
creds := keyfile(t)
76+
77+
tok, path, cleanup := removeAuthEnvVar(t)
78+
defer cleanup()
79+
80+
tcs := []struct {
81+
desc string
82+
opt alloydbconn.Option
83+
}{
84+
{
85+
desc: "Application Default Credentials (ADC)",
86+
opt: nil, // allow ADC to find the credentials
87+
},
88+
{
89+
desc: "with token",
90+
opt: alloydbconn.WithTokenSource(oauth2.StaticTokenSource(tok)),
91+
},
92+
{
93+
desc: "with credentials file",
94+
opt: alloydbconn.WithCredentialsFile(path),
95+
},
96+
{
97+
desc: "with credentials JSON",
98+
opt: alloydbconn.WithCredentialsJSON([]byte(creds)),
99+
},
100+
}
101+
for _, tc := range tcs {
102+
t.Run(tc.desc, func(t *testing.T) {
103+
ctx := context.Background()
104+
105+
d, err := alloydbconn.NewDialer(ctx, tc.opt, alloydbconn.WithIAMAuthN())
106+
if err != nil {
107+
t.Fatalf("failed to init Dialer: %v", err)
108+
}
109+
defer d.Close()
110+
111+
dsn := fmt.Sprintf("user=%s dbname=%s sslmode=disable", alloydbIAMUser, alloydbDB)
112+
config, err := pgxpool.ParseConfig(dsn)
113+
if err != nil {
114+
t.Fatalf("failed to parse pgx config: %v", err)
115+
}
116+
117+
config.ConnConfig.DialFunc = func(ctx context.Context, _ string, _ string) (net.Conn, error) {
118+
return d.Dial(ctx, alloydbInstanceName)
119+
}
120+
121+
pool, err := pgxpool.NewWithConfig(ctx, config)
122+
if err != nil {
123+
t.Fatalf("failed to create pool: %s", err)
124+
}
125+
defer pool.Close()
126+
127+
var now time.Time
128+
err = pool.QueryRow(context.Background(), "SELECT NOW()").Scan(&now)
129+
if err != nil {
130+
t.Fatalf("QueryRow failed: %s", err)
131+
}
132+
t.Log(now)
133+
})
134+
}
135+
}

dialer.go

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@ import (
3636
"cloud.google.com/go/alloydbconn/internal/alloydb"
3737
"cloud.google.com/go/alloydbconn/internal/tel"
3838
"github.com/google/uuid"
39-
"golang.org/x/net/proxy"
4039
"golang.org/x/oauth2"
41-
"golang.org/x/oauth2/google"
4240
"google.golang.org/api/option"
4341
"google.golang.org/protobuf/proto"
4442

@@ -189,34 +187,9 @@ func (nullLogger) Debugf(context.Context, string, ...any) {}
189187
// RSA keypair is performed. Calls with a WithRSAKeyPair DialOption or after a default
190188
// RSA keypair is generated will be faster.
191189
func NewDialer(ctx context.Context, opts ...Option) (*Dialer, error) {
192-
cfg := &dialerConfig{
193-
refreshTimeout: alloydb.RefreshTimeout,
194-
dialFunc: proxy.Dial,
195-
logger: nullLogger{},
196-
userAgents: []string{userAgent},
197-
}
198-
for _, opt := range opts {
199-
opt(cfg)
200-
if cfg.err != nil {
201-
return nil, cfg.err
202-
}
203-
}
204-
if cfg.disableMetadataExchange && cfg.useIAMAuthN {
205-
return nil, errors.New("incompatible options: WithOptOutOfAdvancedConnection " +
206-
"check cannot be used with WithIAMAuthN")
207-
}
208-
userAgent := strings.Join(cfg.userAgents, " ")
209-
// Add user agent to the end to make sure it's not overridden.
210-
cfg.clientOpts = append(cfg.clientOpts, option.WithUserAgent(userAgent))
211-
212-
// If no token source is configured, use ADC's token source.
213-
ts := cfg.tokenSource
214-
if ts == nil {
215-
var err error
216-
ts, err = google.DefaultTokenSource(ctx, CloudPlatformScope)
217-
if err != nil {
218-
return nil, err
219-
}
190+
cfg, err := newDialerConfig(ctx, opts...)
191+
if err != nil {
192+
return nil, err
220193
}
221194

222195
cOpts := append(cfg.alloydbClientOpts, cfg.clientOpts...)
@@ -261,7 +234,7 @@ func NewDialer(ctx context.Context, opts ...Option) (*Dialer, error) {
261234
metricRecorders: map[alloydb.InstanceURI]telv2.MetricRecorder{},
262235
dialFunc: cfg.dialFunc,
263236
useIAMAuthN: cfg.useIAMAuthN,
264-
iamTokenSource: ts,
237+
iamTokenSource: cfg.tokenSource,
265238
userAgent: userAgent,
266239
buffer: newBuffer(),
267240
}

0 commit comments

Comments
 (0)