From 47c26773bdf2b49ab2425443a8848cf77cfde084 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Tue, 19 Nov 2024 20:44:05 +0000 Subject: [PATCH 1/3] fix(storage): retry SignBlob call for URL signing Adds a retry to the SignBlob call made by the default SignBytes function from BucketHandle.SignedURL(). This is an idempotent call so fully safe to retry. Signed URL integration tests pass locally. --- storage/bucket.go | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/storage/bucket.go b/storage/bucket.go index 3eded017831e..6d262b56d780 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -326,12 +326,16 @@ func (b *BucketHandle) defaultSignBytesFunc(email string) func([]byte) ([]byte, if err != nil { return nil, fmt.Errorf("unable to create iamcredentials client: %w", err) } - - resp, err := svc.Projects.ServiceAccounts.SignBlob(fmt.Sprintf("projects/-/serviceAccounts/%s", email), &iamcredentials.SignBlobRequest{ - Payload: base64.StdEncoding.EncodeToString(in), - }).Do() - if err != nil { + // Do the SignBlob call with a retry for transient errors. + var resp *iamcredentials.SignBlobResponse + if err := run(ctx, func(ctx context.Context) error { + resp, err = svc.Projects.ServiceAccounts.SignBlob(fmt.Sprintf("projects/-/serviceAccounts/%s", email), &iamcredentials.SignBlobRequest{ + Payload: base64.StdEncoding.EncodeToString(in), + }).Do() + return err + }, b.retry, true); err != nil { return nil, fmt.Errorf("unable to sign bytes: %w", err) + } out, err := base64.StdEncoding.DecodeString(resp.SignedBlob) if err != nil { From 4664963dc4031125468d511744ca11084fa5a221 Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Tue, 19 Nov 2024 20:49:39 +0000 Subject: [PATCH 2/3] fmt --- storage/bucket.go | 1 - 1 file changed, 1 deletion(-) diff --git a/storage/bucket.go b/storage/bucket.go index 6d262b56d780..5601357c9b2b 100644 --- a/storage/bucket.go +++ b/storage/bucket.go @@ -335,7 +335,6 @@ func (b *BucketHandle) defaultSignBytesFunc(email string) func([]byte) ([]byte, return err }, b.retry, true); err != nil { return nil, fmt.Errorf("unable to sign bytes: %w", err) - } out, err := base64.StdEncoding.DecodeString(resp.SignedBlob) if err != nil { From 5d0b9dac50e6999f8a0953882656cbfc4a032ffd Mon Sep 17 00:00:00 2001 From: Chris Cotter Date: Wed, 20 Nov 2024 05:38:50 +0000 Subject: [PATCH 3/3] add test with mock transport --- storage/bucket_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/storage/bucket_test.go b/storage/bucket_test.go index 7ae7b06e7e8d..abd117d6e45a 100644 --- a/storage/bucket_test.go +++ b/storage/bucket_test.go @@ -17,6 +17,7 @@ package storage import ( "context" "fmt" + "net/http" "testing" "time" @@ -1640,3 +1641,30 @@ func TestBucketSignedURL_Endpoint_Emulator_Host(t *testing.T) { }) } } + +// Test retry logic for default SignBlob function used by BucketHandle.SignedURL. +// This cannot be tested via the emulator so we use a mock. +func TestDefaultSignBlobRetry(t *testing.T) { + ctx := context.Background() + + // Use mock transport. Return 2 503 responses before succeeding. + mt := mockTransport{} + mt.addResult(&http.Response{StatusCode: 503, Body: bodyReader("")}, nil) + mt.addResult(&http.Response{StatusCode: 503, Body: bodyReader("")}, nil) + mt.addResult(&http.Response{StatusCode: 200, Body: bodyReader("{}")}, nil) + + client, err := NewClient(ctx, option.WithHTTPClient(&http.Client{Transport: &mt})) + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + b := client.Bucket("fakebucket") + + if _, err := b.SignedURL("fakeobj", &SignedURLOptions{ + Method: "GET", + Expires: time.Now().Add(time.Hour), + SignBytes: b.defaultSignBytesFunc("example@example.com"), + }); err != nil { + t.Fatalf("BucketHandle.SignedURL: %v", err) + } +}