Skip to content

Commit 3a22349

Browse files
quartzmoquartzmo
authored
fix(storage): migrate oauth2/google usages to cloud.google.com/go/auth (#11191)
1 parent e54c439 commit 3a22349

6 files changed

Lines changed: 91 additions & 47 deletions

File tree

‎storage/bucket.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -199,11 +199,11 @@ func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string,
199199
newopts.GoogleAccessID = id
200200
}
201201
if newopts.SignBytes == nil && len(newopts.PrivateKey) == 0 {
202-
if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
202+
if j, ok := b.c.credsJSON(); ok {
203203
var sa struct {
204204
PrivateKey string `json:"private_key"`
205205
}
206-
err := json.Unmarshal(b.c.creds.JSON, &sa)
206+
err := json.Unmarshal(j, &sa)
207207
if err == nil && sa.PrivateKey != "" {
208208
newopts.PrivateKey = []byte(sa.PrivateKey)
209209
}
@@ -247,11 +247,11 @@ func (b *BucketHandle) GenerateSignedPostPolicyV4(object string, opts *PostPolic
247247
newopts.GoogleAccessID = id
248248
}
249249
if newopts.SignBytes == nil && newopts.SignRawBytes == nil && len(newopts.PrivateKey) == 0 {
250-
if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
250+
if j, ok := b.c.credsJSON(); ok {
251251
var sa struct {
252252
PrivateKey string `json:"private_key"`
253253
}
254-
err := json.Unmarshal(b.c.creds.JSON, &sa)
254+
err := json.Unmarshal(j, &sa)
255255
if err == nil && sa.PrivateKey != "" {
256256
newopts.PrivateKey = []byte(sa.PrivateKey)
257257
}
@@ -269,14 +269,14 @@ func (b *BucketHandle) GenerateSignedPostPolicyV4(object string, opts *PostPolic
269269
func (b *BucketHandle) detectDefaultGoogleAccessID() (string, error) {
270270
returnErr := errors.New("no credentials found on client and not on GCE (Google Compute Engine)")
271271

272-
if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
272+
if j, ok := b.c.credsJSON(); ok {
273273
var sa struct {
274274
ClientEmail string `json:"client_email"`
275275
SAImpersonationURL string `json:"service_account_impersonation_url"`
276276
CredType string `json:"type"`
277277
}
278278

279-
err := json.Unmarshal(b.c.creds.JSON, &sa)
279+
err := json.Unmarshal(j, &sa)
280280
if err != nil {
281281
returnErr = err
282282
} else {
@@ -322,7 +322,7 @@ func (b *BucketHandle) defaultSignBytesFunc(email string) func([]byte) ([]byte,
322322
opts := []option.ClientOption{option.WithHTTPClient(b.c.hc)}
323323

324324
if b.c.creds != nil {
325-
universeDomain, err := b.c.creds.GetUniverseDomain()
325+
universeDomain, err := b.c.creds.UniverseDomain(ctx)
326326
if err != nil {
327327
return nil, err
328328
}

‎storage/bucket_test.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ import (
2121
"testing"
2222
"time"
2323

24+
"cloud.google.com/go/auth"
2425
"cloud.google.com/go/compute/metadata"
2526
"cloud.google.com/go/internal/testutil"
2627
"cloud.google.com/go/storage/internal/apiv2/storagepb"
2728
"github.com/google/go-cmp/cmp"
2829
gax "github.com/googleapis/gax-go/v2"
29-
"golang.org/x/oauth2/google"
3030
"google.golang.org/api/googleapi"
3131
"google.golang.org/api/option"
3232
raw "google.golang.org/api/storage/v1"
@@ -1300,9 +1300,7 @@ func TestDetectDefaultGoogleAccessID(t *testing.T) {
13001300
t.Run(tc.name, func(t *testing.T) {
13011301
bucket := BucketHandle{
13021302
c: &Client{
1303-
creds: &google.Credentials{
1304-
JSON: []byte(tc.creds(tc.serviceAccount)),
1305-
},
1303+
creds: auth.NewCredentials(&auth.CredentialsOptions{JSON: []byte(tc.creds(tc.serviceAccount))}),
13061304
},
13071305
name: "my-bucket",
13081306
}

‎storage/go.mod‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ retract [v1.25.0, v1.27.0] // due to https://github.com/googleapis/google-cloud-
66

77
require (
88
cloud.google.com/go v0.121.0
9+
cloud.google.com/go/auth v0.16.1
910
cloud.google.com/go/compute/metadata v0.6.0
1011
cloud.google.com/go/iam v1.5.2
1112
cloud.google.com/go/longrunning v0.6.7
@@ -31,7 +32,6 @@ require (
3132

3233
require (
3334
cel.dev/expr v0.20.0 // indirect
34-
cloud.google.com/go/auth v0.16.1 // indirect
3535
cloud.google.com/go/auth/oauth2adapt v0.2.8 // indirect
3636
cloud.google.com/go/monitoring v1.24.0 // indirect
3737
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.27.0 // indirect

‎storage/http_client.go‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,24 @@ import (
3030
"strings"
3131
"time"
3232

33+
"cloud.google.com/go/auth"
3334
"cloud.google.com/go/iam/apiv1/iampb"
3435
"cloud.google.com/go/internal/optional"
3536
"cloud.google.com/go/internal/trace"
3637
"github.com/google/uuid"
3738
"github.com/googleapis/gax-go/v2/callctx"
38-
"golang.org/x/oauth2/google"
3939
"google.golang.org/api/googleapi"
4040
"google.golang.org/api/iterator"
4141
"google.golang.org/api/option"
4242
"google.golang.org/api/option/internaloption"
4343
raw "google.golang.org/api/storage/v1"
44-
"google.golang.org/api/transport"
4544
htransport "google.golang.org/api/transport/http"
4645
)
4746

4847
// httpStorageClient is the HTTP-JSON API implementation of the transport-agnostic
4948
// storageClient interface.
5049
type httpStorageClient struct {
51-
creds *google.Credentials
50+
creds *auth.Credentials
5251
hc *http.Client
5352
xmlHost string
5453
raw *raw.Service
@@ -65,7 +64,7 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
6564
o := s.clientOption
6665
config := newStorageConfig(o...)
6766

68-
var creds *google.Credentials
67+
var creds *auth.Credentials
6968
// In general, it is recommended to use raw.NewService instead of htransport.NewClient
7069
// since raw.NewService configures the correct default endpoints when initializing the
7170
// internal http client. However, in our case, "NewRangeReader" in reader.go needs to
@@ -83,10 +82,10 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
8382
)
8483
// Don't error out here. The user may have passed in their own HTTP
8584
// client which does not auth with ADC or other common conventions.
86-
c, err := transport.Creds(ctx, o...)
85+
c, err := internaloption.AuthCreds(ctx, o)
8786
if err == nil {
8887
creds = c
89-
o = append(o, internaloption.WithCredentials(creds))
88+
o = append(o, option.WithAuthCredentials(creds))
9089
}
9190
} else {
9291
var hostURL *url.URL

‎storage/integration_test.go‎

Lines changed: 62 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ import (
4444
"testing"
4545
"time"
4646

47+
"cloud.google.com/go/auth"
48+
"cloud.google.com/go/auth/credentials"
4749
"cloud.google.com/go/compute/metadata"
4850
"cloud.google.com/go/httpreplay"
4951
"cloud.google.com/go/iam"
@@ -107,6 +109,13 @@ var (
107109
controlClient *control.StorageControlClient
108110
)
109111

112+
var (
113+
testScopes = []string{
114+
ScopeFullControl,
115+
"https://www.googleapis.com/auth/cloud-platform",
116+
}
117+
)
118+
110119
func TestMain(m *testing.M) {
111120
cleanup := initIntegrationTest()
112121
cleanupEmulatorClients := initEmulatorClients()
@@ -6320,6 +6329,24 @@ func TestIntegration_NewReaderWithContentEncodingGzip(t *testing.T) {
63206329
})
63216330
}
63226331

6332+
type credentialsFile struct {
6333+
Type string `json:"type"`
6334+
6335+
// Service Account email
6336+
ClientEmail string `json:"client_email"`
6337+
}
6338+
6339+
func jwtConfigFromJSON(jsonKey []byte) (*credentialsFile, error) {
6340+
var f credentialsFile
6341+
if err := json.Unmarshal(jsonKey, &f); err != nil {
6342+
return nil, err
6343+
}
6344+
if f.Type != "service_account" {
6345+
return nil, fmt.Errorf("read JWT from JSON credentials: 'type' field is %q (expected service_account)", f.Type)
6346+
}
6347+
return &f, nil
6348+
}
6349+
63236350
func TestIntegration_HMACKey(t *testing.T) {
63246351
ctx := skipExtraReadAPIs(skipGRPC("hmac not implemented"), "no reads in test")
63256352
multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, _, _ string, client *Client) {
@@ -6339,13 +6366,12 @@ func TestIntegration_HMACKey(t *testing.T) {
63396366
if credentials.JSON == nil {
63406367
t.Fatal("could not read the JSON key file, is GCLOUD_TESTS_GOLANG_KEY set correctly?")
63416368
}
6342-
conf, err := google.JWTConfigFromJSON(credentials.JSON)
6369+
conf, err := jwtConfigFromJSON(credentials.JSON)
63436370
if err != nil {
63446371
t.Fatal(err)
63456372
}
6346-
serviceAccountEmail := conf.Email
63476373

6348-
hmacKey, err := client.CreateHMACKey(ctx, projectID, serviceAccountEmail)
6374+
hmacKey, err := client.CreateHMACKey(ctx, projectID, conf.ClientEmail)
63496375
if err != nil {
63506376
t.Fatalf("Failed to create HMACKey: %v", err)
63516377
}
@@ -6571,14 +6597,8 @@ func TestIntegration_SignedURL_WithCreds(t *testing.T) {
65716597
t.Skip("Integration tests skipped in short mode")
65726598
}
65736599

6574-
ctx := context.Background()
6575-
6576-
creds, err := findTestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", ScopeFullControl, "https://www.googleapis.com/auth/cloud-platform")
6577-
if err != nil {
6578-
t.Fatalf("unable to find test credentials: %v", err)
6579-
}
6580-
6581-
multiTransportTest(skipGRPC("creds capture logic must be implemented for gRPC constructor"), t, func(t *testing.T, ctx context.Context, bucket, _ string, client *Client) {
6600+
ctx := skipGRPC("creds capture logic must be implemented for gRPC constructor")
6601+
tFunc := func(t *testing.T, ctx context.Context, bucket, _ string, client *Client) {
65826602
// We can use any client to create the object
65836603
obj := "testBucketSignedURL"
65846604
contents := []byte("test")
@@ -6598,7 +6618,17 @@ func TestIntegration_SignedURL_WithCreds(t *testing.T) {
65986618
if err := verifySignedURL(url, nil, contents); err != nil {
65996619
t.Fatalf("problem with the signed URL: %v", err)
66006620
}
6601-
}, option.WithCredentials(creds))
6621+
}
6622+
creds, err := findLegacyOAuth2TestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", testScopes)
6623+
if err != nil {
6624+
t.Fatalf("unable to find test credentials: %v", err)
6625+
}
6626+
multiTransportTest(ctx, t, tFunc, option.WithCredentials(creds))
6627+
newAuthCreds, err := findNewAuthTestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", testScopes)
6628+
if err != nil {
6629+
t.Fatalf("unable to find test credentials: %v", err)
6630+
}
6631+
multiTransportTest(ctx, t, tFunc, option.WithAuthCredentials(newAuthCreds))
66026632
}
66036633

66046634
func TestIntegration_SignedURL_DefaultSignBytes(t *testing.T) {
@@ -6652,16 +6682,8 @@ func TestIntegration_PostPolicyV4_WithCreds(t *testing.T) {
66526682
t.Skip("Integration tests skipped in short mode")
66536683
}
66546684

6655-
// By default we are authed with a token source, so don't have the context to
6656-
// read some of the fields from the keyfile.
6657-
// Here we explictly send the key to the client.
6658-
creds, err := findTestCredentials(context.Background(), "GCLOUD_TESTS_GOLANG_KEY", ScopeFullControl, "https://www.googleapis.com/auth/cloud-platform")
6659-
if err != nil {
6660-
t.Fatalf("unable to find test credentials: %v", err)
6661-
}
6662-
66636685
ctx := skipExtraReadAPIs(skipGRPC("creds capture logic must be implemented for gRPC constructor"), "test is not testing the read behaviour")
6664-
multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, bucket, _ string, clientWithCredentials *Client) {
6686+
tFunc := func(t *testing.T, ctx context.Context, bucket, _ string, clientWithCredentials *Client) {
66656687
h := testHelper{t}
66666688

66676689
statusCodeToRespond := 200
@@ -6700,7 +6722,17 @@ func TestIntegration_PostPolicyV4_WithCreds(t *testing.T) {
67006722
}
67016723
})
67026724
}
6703-
}, option.WithCredentials(creds))
6725+
}
6726+
creds, err := findLegacyOAuth2TestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", testScopes)
6727+
if err != nil {
6728+
t.Fatalf("unable to find test credentials: %v", err)
6729+
}
6730+
multiTransportTest(ctx, t, tFunc, option.WithCredentials(creds))
6731+
newAuthCreds, err := findNewAuthTestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", testScopes)
6732+
if err != nil {
6733+
t.Fatalf("unable to find test credentials: %v", err)
6734+
}
6735+
multiTransportTest(ctx, t, tFunc, option.WithAuthCredentials(newAuthCreds))
67046736

67056737
}
67066738

@@ -7014,7 +7046,7 @@ func verifyPostPolicy(pv4 *PostPolicyV4, obj *ObjectHandle, bytesToWrite []byte,
70147046
})
70157047
}
70167048

7017-
func findTestCredentials(ctx context.Context, envVar string, scopes ...string) (*google.Credentials, error) {
7049+
func findLegacyOAuth2TestCredentials(ctx context.Context, envVar string, scopes []string) (*google.Credentials, error) {
70187050
key := os.Getenv(envVar)
70197051
var opts []option.ClientOption
70207052
if len(scopes) > 0 {
@@ -7026,6 +7058,13 @@ func findTestCredentials(ctx context.Context, envVar string, scopes ...string) (
70267058
return transport.Creds(ctx, opts...)
70277059
}
70287060

7061+
func findNewAuthTestCredentials(ctx context.Context, envVar string, scopes []string) (*auth.Credentials, error) {
7062+
return credentials.DetectDefault(&credentials.DetectOptions{
7063+
CredentialsFile: os.Getenv(envVar),
7064+
Scopes: scopes,
7065+
})
7066+
}
7067+
70297068
type testHelper struct {
70307069
t *testing.T
70317070
}

‎storage/storage.go‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import (
3838
"time"
3939
"unicode/utf8"
4040

41+
"cloud.google.com/go/auth"
4142
"cloud.google.com/go/internal/optional"
4243
"cloud.google.com/go/internal/trace"
4344
"cloud.google.com/go/storage/internal"
@@ -46,12 +47,10 @@ import (
4647
"go.opentelemetry.io/otel/attribute"
4748
"go.opentelemetry.io/otel/sdk/metric"
4849
"go.opentelemetry.io/otel/sdk/metric/metricdata"
49-
"golang.org/x/oauth2/google"
5050
"google.golang.org/api/googleapi"
5151
"google.golang.org/api/option"
5252
"google.golang.org/api/option/internaloption"
5353
raw "google.golang.org/api/storage/v1"
54-
"google.golang.org/api/transport"
5554
htransport "google.golang.org/api/transport/http"
5655
"google.golang.org/grpc/codes"
5756
"google.golang.org/grpc/experimental/stats"
@@ -121,7 +120,7 @@ type Client struct {
121120
// xmlHost is the default host used for XML requests.
122121
xmlHost string
123122
// May be nil.
124-
creds *google.Credentials
123+
creds *auth.Credentials
125124
retry *retryConfig
126125

127126
// tc is the transport-agnostic client implemented with either gRPC or HTTP.
@@ -131,6 +130,15 @@ type Client struct {
131130
grpcAppendableUploads bool
132131
}
133132

133+
// credsJSON returns the raw JSON of the Client's creds and true, or an empty slice
134+
// and false if no credentials JSON is available.
135+
func (c Client) credsJSON() ([]byte, bool) {
136+
if c.creds != nil && len(c.creds.JSON()) > 0 {
137+
return c.creds.JSON(), true
138+
}
139+
return []byte{}, false
140+
}
141+
134142
// NewClient creates a new Google Cloud Storage client using the HTTP transport.
135143
// The default scope is ScopeFullControl. To use a different scope, like
136144
// ScopeReadOnly, use option.WithScopes.
@@ -141,7 +149,7 @@ type Client struct {
141149
// You may configure the client by passing in options from the [google.golang.org/api/option]
142150
// package. You may also use options defined in this package, such as [WithJSONReads].
143151
func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error) {
144-
var creds *google.Credentials
152+
var creds *auth.Credentials
145153

146154
// In general, it is recommended to use raw.NewService instead of htransport.NewClient
147155
// since raw.NewService configures the correct default endpoints when initializing the
@@ -161,10 +169,10 @@ func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error
161169

162170
// Don't error out here. The user may have passed in their own HTTP
163171
// client which does not auth with ADC or other common conventions.
164-
c, err := transport.Creds(ctx, opts...)
172+
c, err := internaloption.AuthCreds(ctx, opts)
165173
if err == nil {
166174
creds = c
167-
opts = append(opts, internaloption.WithCredentials(creds))
175+
opts = append(opts, option.WithAuthCredentials(creds))
168176
}
169177
} else {
170178
var hostURL *url.URL

0 commit comments

Comments
 (0)