Skip to content

Commit

Permalink
fix(storage): migrate oauth2/google usages to cloud.google.com/go/auth
Browse files Browse the repository at this point in the history
  • Loading branch information
quartzmo committed Nov 26, 2024
1 parent ebf3657 commit 2f8457f
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 40 deletions.
12 changes: 6 additions & 6 deletions storage/bucket.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,11 +200,11 @@ func (b *BucketHandle) SignedURL(object string, opts *SignedURLOptions) (string,
newopts.GoogleAccessID = id
}
if newopts.SignBytes == nil && len(newopts.PrivateKey) == 0 {
if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
if j := b.c.credsJSON(); len(j) > 0 {
var sa struct {
PrivateKey string `json:"private_key"`
}
err := json.Unmarshal(b.c.creds.JSON, &sa)
err := json.Unmarshal(j, &sa)
if err == nil && sa.PrivateKey != "" {
newopts.PrivateKey = []byte(sa.PrivateKey)
}
Expand Down Expand Up @@ -248,11 +248,11 @@ func (b *BucketHandle) GenerateSignedPostPolicyV4(object string, opts *PostPolic
newopts.GoogleAccessID = id
}
if newopts.SignBytes == nil && newopts.SignRawBytes == nil && len(newopts.PrivateKey) == 0 {
if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
if j := b.c.credsJSON(); len(j) > 0 {
var sa struct {
PrivateKey string `json:"private_key"`
}
err := json.Unmarshal(b.c.creds.JSON, &sa)
err := json.Unmarshal(j, &sa)
if err == nil && sa.PrivateKey != "" {
newopts.PrivateKey = []byte(sa.PrivateKey)
}
Expand All @@ -270,14 +270,14 @@ func (b *BucketHandle) GenerateSignedPostPolicyV4(object string, opts *PostPolic
func (b *BucketHandle) detectDefaultGoogleAccessID() (string, error) {
returnErr := errors.New("no credentials found on client and not on GCE (Google Compute Engine)")

if b.c.creds != nil && len(b.c.creds.JSON) > 0 {
if j := b.c.credsJSON(); len(j) > 0 {
var sa struct {
ClientEmail string `json:"client_email"`
SAImpersonationURL string `json:"service_account_impersonation_url"`
CredType string `json:"type"`
}

err := json.Unmarshal(b.c.creds.JSON, &sa)
err := json.Unmarshal(j, &sa)
if err != nil {
returnErr = err
} else {
Expand Down
13 changes: 9 additions & 4 deletions storage/bucket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@ import (
"testing"
"time"

"cloud.google.com/go/auth/credentials"
"cloud.google.com/go/compute/metadata"
"cloud.google.com/go/internal/testutil"
"cloud.google.com/go/storage/internal/apiv2/storagepb"
"github.com/google/go-cmp/cmp"
gax "github.com/googleapis/gax-go/v2"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/option"
raw "google.golang.org/api/storage/v1"
Expand Down Expand Up @@ -1291,11 +1291,16 @@ func TestDetectDefaultGoogleAccessID(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
jsonBytes := []byte(tc.creds(tc.serviceAccount))
creds, err := credentials.DetectDefault(&credentials.DetectOptions{
CredentialsJSON: jsonBytes,
})
if tc.expectSuccess && err != nil {
t.Fatal(err)
}
bucket := BucketHandle{
c: &Client{
creds: &google.Credentials{
JSON: []byte(tc.creds(tc.serviceAccount)),
},
creds: creds,
},
name: "my-bucket",
}
Expand Down
10 changes: 5 additions & 5 deletions storage/http_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ import (
"strings"
"time"

"cloud.google.com/go/auth"
"cloud.google.com/go/iam/apiv1/iampb"
"cloud.google.com/go/internal/optional"
"cloud.google.com/go/internal/trace"
"github.com/googleapis/gax-go/v2/callctx"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/iterator"
"google.golang.org/api/option"
Expand All @@ -48,7 +48,7 @@ import (
// httpStorageClient is the HTTP-JSON API implementation of the transport-agnostic
// storageClient interface.
type httpStorageClient struct {
creds *google.Credentials
creds *auth.Credentials
hc *http.Client
xmlHost string
raw *raw.Service
Expand All @@ -65,7 +65,7 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
o := s.clientOption
config := newStorageConfig(o...)

var creds *google.Credentials
var creds *auth.Credentials
// In general, it is recommended to use raw.NewService instead of htransport.NewClient
// since raw.NewService configures the correct default endpoints when initializing the
// internal http client. However, in our case, "NewRangeReader" in reader.go needs to
Expand All @@ -83,10 +83,10 @@ func newHTTPStorageClient(ctx context.Context, opts ...storageOption) (storageCl
)
// Don't error out here. The user may have passed in their own HTTP
// client which does not auth with ADC or other common conventions.
c, err := transport.Creds(ctx, o...)
c, err := transport.AuthCreds(ctx, o...)
if err == nil {
creds = c
o = append(o, internaloption.WithCredentials(creds))
o = append(o, option.WithAuthCredentials(creds))
}
} else {
var hostURL *url.URL
Expand Down
58 changes: 38 additions & 20 deletions storage/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"testing"
"time"

"cloud.google.com/go/auth"
"cloud.google.com/go/auth/credentials"
"cloud.google.com/go/httpreplay"
"cloud.google.com/go/iam"
"cloud.google.com/go/iam/apiv1/iampb"
Expand All @@ -55,13 +57,11 @@ import (
"github.com/googleapis/gax-go/v2/apierror"
"go.opentelemetry.io/contrib/detectors/gcp"
"go.opentelemetry.io/otel/sdk/resource"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/iterator"
itesting "google.golang.org/api/iterator/testing"
"google.golang.org/api/option"
"google.golang.org/api/option/internaloption"
"google.golang.org/api/transport"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)
Expand Down Expand Up @@ -94,6 +94,13 @@ var (
controlClient *control.StorageControlClient
)

var (
testScopes = []string{
ScopeFullControl,
"https://www.googleapis.com/auth/cloud-platform",
}
)

func TestMain(m *testing.M) {
cleanup := initIntegrationTest()
cleanupEmulatorClients := initEmulatorClients()
Expand Down Expand Up @@ -5348,6 +5355,24 @@ func TestIntegration_NewReaderWithContentEncodingGzip(t *testing.T) {
})
}

type credentialsFile struct {
Type string `json:"type"`

// Service Account email
ClientEmail string `json:"client_email"`
}

func jwtConfigFromJSON(jsonKey []byte) (*credentialsFile, error) {
var f credentialsFile
if err := json.Unmarshal(jsonKey, &f); err != nil {
return nil, err
}
if f.Type != "service_account" {
return nil, fmt.Errorf("read JWT from JSON credentials: 'type' field is %q (expected service_account)", f.Type)
}
return &f, nil
}

func TestIntegration_HMACKey(t *testing.T) {
ctx := skipJSONReads(skipGRPC("hmac not implemented"), "no reads in test")
multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, _, _ string, client *Client) {
Expand All @@ -5367,13 +5392,12 @@ func TestIntegration_HMACKey(t *testing.T) {
if credentials.JSON == nil {
t.Fatal("could not read the JSON key file, is GCLOUD_TESTS_GOLANG_KEY set correctly?")
}
conf, err := google.JWTConfigFromJSON(credentials.JSON)
conf, err := jwtConfigFromJSON(credentials.JSON)
if err != nil {
t.Fatal(err)
}
serviceAccountEmail := conf.Email

hmacKey, err := client.CreateHMACKey(ctx, projectID, serviceAccountEmail)
hmacKey, err := client.CreateHMACKey(ctx, projectID, conf.ClientEmail)
if err != nil {
t.Fatalf("Failed to create HMACKey: %v", err)
}
Expand Down Expand Up @@ -5600,8 +5624,7 @@ func TestIntegration_SignedURL_WithCreds(t *testing.T) {
}

ctx := context.Background()

creds, err := findTestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", ScopeFullControl, "https://www.googleapis.com/auth/cloud-platform")
creds, err := findTestCredentials(ctx, "GCLOUD_TESTS_GOLANG_KEY", testScopes)
if err != nil {
t.Fatalf("unable to find test credentials: %v", err)
}
Expand All @@ -5626,7 +5649,7 @@ func TestIntegration_SignedURL_WithCreds(t *testing.T) {
if err := verifySignedURL(url, nil, contents); err != nil {
t.Fatalf("problem with the signed URL: %v", err)
}
}, option.WithCredentials(creds))
}, option.WithAuthCredentials(creds))
}

func TestIntegration_SignedURL_DefaultSignBytes(t *testing.T) {
Expand Down Expand Up @@ -5683,7 +5706,7 @@ func TestIntegration_PostPolicyV4_WithCreds(t *testing.T) {
// By default we are authed with a token source, so don't have the context to
// read some of the fields from the keyfile.
// Here we explictly send the key to the client.
creds, err := findTestCredentials(context.Background(), "GCLOUD_TESTS_GOLANG_KEY", ScopeFullControl, "https://www.googleapis.com/auth/cloud-platform")
creds, err := findTestCredentials(context.Background(), "GCLOUD_TESTS_GOLANG_KEY", testScopes)
if err != nil {
t.Fatalf("unable to find test credentials: %v", err)
}
Expand Down Expand Up @@ -5728,7 +5751,7 @@ func TestIntegration_PostPolicyV4_WithCreds(t *testing.T) {
}
})
}
}, option.WithCredentials(creds))
}, option.WithAuthCredentials(creds))

}

Expand Down Expand Up @@ -5969,16 +5992,11 @@ func verifyPostPolicy(pv4 *PostPolicyV4, obj *ObjectHandle, bytesToWrite []byte,
})
}

func findTestCredentials(ctx context.Context, envVar string, scopes ...string) (*google.Credentials, error) {
key := os.Getenv(envVar)
var opts []option.ClientOption
if len(scopes) > 0 {
opts = append(opts, option.WithScopes(scopes...))
}
if key != "" {
opts = append(opts, option.WithCredentialsFile(key))
}
return transport.Creds(ctx, opts...)
func findTestCredentials(ctx context.Context, envVar string, scopes []string) (*auth.Credentials, error) {
return credentials.DetectDefault(&credentials.DetectOptions{
CredentialsFile: os.Getenv(envVar),
Scopes: scopes,
})
}

type testHelper struct {
Expand Down
19 changes: 14 additions & 5 deletions storage/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"time"
"unicode/utf8"

"cloud.google.com/go/auth"
"cloud.google.com/go/internal/optional"
"cloud.google.com/go/internal/trace"
"cloud.google.com/go/storage/internal"
Expand All @@ -46,7 +47,6 @@ import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/metric/metricdata"
"golang.org/x/oauth2/google"
"google.golang.org/api/googleapi"
"google.golang.org/api/option"
"google.golang.org/api/option/internaloption"
Expand Down Expand Up @@ -117,13 +117,22 @@ type Client struct {
// xmlHost is the default host used for XML requests.
xmlHost string
// May be nil.
creds *google.Credentials
creds *auth.Credentials
retry *retryConfig

// tc is the transport-agnostic client implemented with either gRPC or HTTP.
tc storageClient
}

// credsJSON returns the raw JSON of the Client's creds, or an empty slice
// if no credentials JSON is available.
func (c Client) credsJSON() []byte {
if c.creds != nil && len(c.creds.JSON()) > 0 {
return c.creds.JSON()
}
return []byte{}
}

// NewClient creates a new Google Cloud Storage client using the HTTP transport.
// The default scope is ScopeFullControl. To use a different scope, like
// ScopeReadOnly, use option.WithScopes.
Expand All @@ -134,7 +143,7 @@ type Client struct {
// You may configure the client by passing in options from the [google.golang.org/api/option]
// package. You may also use options defined in this package, such as [WithJSONReads].
func NewClient(ctx context.Context, opts ...option.ClientOption) (*Client, error) {
var creds *google.Credentials
var creds *auth.Credentials

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

// Don't error out here. The user may have passed in their own HTTP
// client which does not auth with ADC or other common conventions.
c, err := transport.Creds(ctx, opts...)
c, err := transport.AuthCreds(ctx, opts...)
if err == nil {
creds = c
opts = append(opts, internaloption.WithCredentials(creds))
opts = append(opts, option.WithAuthCredentials(creds))
}
} else {
var hostURL *url.URL
Expand Down

0 comments on commit 2f8457f

Please sign in to comment.