Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage): retry SignBlob call for URL signing #7862

Merged
merged 32 commits into from
Dec 11, 2024
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
809530a
feat: add integration test for universe domain
thiyaguk09 Sep 27, 2024
b250912
Merge branch 'googleapis:main' into universe-domain-add-integration-t…
thiyaguk09 Sep 27, 2024
2bcb0cd
Merge pull request #1 from thiyaguk09/universe-domain-add-integration…
thiyaguk09 Sep 30, 2024
a315588
style fix
thiyaguk09 Sep 30, 2024
f701a10
style fix
thiyaguk09 Sep 30, 2024
a31e6ec
Merge branch 'googleapis:main' into main
thiyaguk09 Oct 1, 2024
a93d0ec
Merge branch 'googleapis:main' into main
thiyaguk09 Oct 23, 2024
f7d79c3
Adds support for the restore token feature
thiyaguk09 Oct 29, 2024
0d55821
Merge branch 'googleapis:main' into main
thiyaguk09 Oct 29, 2024
3f5079e
lint fix
thiyaguk09 Oct 29, 2024
9720b9b
Merge pull request #2 from thiyaguk09/support-for-restore-token
thiyaguk09 Oct 29, 2024
2c935f4
Merge branch 'main' into main
bshaffer Oct 31, 2024
8dc9eb1
Merge branch 'googleapis:main' into main
thiyaguk09 Nov 12, 2024
bec1876
Merge branch 'googleapis:main' into main
thiyaguk09 Nov 25, 2024
42526e7
Merge branch 'googleapis:main' into correct_retries-signBlob_api
thiyaguk09 Nov 25, 2024
d1f4b32
Merge branch 'googleapis:main' into main
thiyaguk09 Nov 26, 2024
f1fe1e3
signblob retries
thiyaguk09 Nov 27, 2024
93a16be
Merge branch 'googleapis:main' into correct_retries-signBlob_api
thiyaguk09 Nov 27, 2024
18171d5
Merge branch 'correct_retries-signBlob_api' of https://github.com/thi…
thiyaguk09 Nov 27, 2024
7048bbf
lint fix
thiyaguk09 Nov 27, 2024
6f64bed
signblob retry test cases
thiyaguk09 Nov 27, 2024
1ab9c96
Merge pull request #3 from thiyaguk09/correct_retries-signBlob_api
thiyaguk09 Nov 27, 2024
4607e65
using existing retryTrait
thiyaguk09 Dec 3, 2024
b8247f2
Merge branch 'googleapis:main' into main
thiyaguk09 Dec 3, 2024
173d8bb
Merge pull request #4 from thiyaguk09/correct_retries-signBlob_api
thiyaguk09 Dec 3, 2024
5efdf4b
lint issues fix
thiyaguk09 Dec 3, 2024
031c0a7
review corrections and lint fix
thiyaguk09 Dec 3, 2024
69eaeb1
Test case error fix
thiyaguk09 Dec 3, 2024
cf9fd55
add retry using idempontentOps list
thiyaguk09 Dec 9, 2024
e9c05ce
remove bug
thiyaguk09 Dec 9, 2024
a0fb38d
code standard
thiyaguk09 Dec 10, 2024
9b67a34
Retry the test case for maximum attempts.
thiyaguk09 Dec 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@ keys/
.testing
.split
__pycache__
.vscode/
1 change: 1 addition & 0 deletions Storage/src/Connection/RetryTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ trait RetryTrait
'objects.get',
'objects.list',
'serviceaccount.get',
'signBlob.execute'
];

/**
Expand Down
53 changes: 46 additions & 7 deletions Storage/src/SigningHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
use Google\Cloud\Core\JsonTrait;
use Google\Cloud\Core\Timestamp;
use Google\Cloud\Storage\Connection\ConnectionInterface;
use Google\Cloud\Core\Exception\ServiceException;
use Google\Cloud\Storage\Connection\RetryTrait;

/**
* Provides common methods for signing storage URLs.
Expand All @@ -33,6 +35,7 @@ class SigningHelper
{
use ArrayTrait;
use JsonTrait;
use RetryTrait;

const DEFAULT_URL_SIGNING_VERSION = 'v2';
const DEFAULT_DOWNLOAD_HOST = 'storage.googleapis.com';
Expand Down Expand Up @@ -169,7 +172,7 @@ public function v2Sign(ConnectionInterface $connection, $expires, $resource, $ge

$signedHeaders = [];
foreach ($headers as $name => $value) {
$signedHeaders[] = $name .':'. $value;
$signedHeaders[] = $name . ':' . $value;
}

// Push the headers onto the end of the signing string.
Expand All @@ -181,9 +184,12 @@ public function v2Sign(ConnectionInterface $connection, $expires, $resource, $ge

$stringToSign = $this->createV2CanonicalRequest($toSign);

$signature = $credentials->signBlob($stringToSign, [
'forceOpenssl' => $options['forceOpenssl']
]);
// Use exponential backOff
$signature = $this->retrySignBlob(function () use ($credentials, $stringToSign, $options) {
return $credentials->signBlob($stringToSign, [
'forceOpenssl' => $options['forceOpenssl']
]);
});
thiyaguk09 marked this conversation as resolved.
Show resolved Hide resolved

// Start with user-provided query params and add required parameters.
$params = $options['queryParams'];
Expand Down Expand Up @@ -337,9 +343,15 @@ public function v4Sign(ConnectionInterface $connection, $expires, $resource, $ge
$requestHash
]);

$signature = bin2hex(base64_decode($credentials->signBlob($stringToSign, [
'forceOpenssl' => $options['forceOpenssl']
])));
$signature = bin2hex(base64_decode($this->retrySignBlob(function () use (
$credentials,
$stringToSign,
$options
) {
return $credentials->signBlob($stringToSign, [
'forceOpenssl' => $options['forceOpenssl']
]);
})));
thiyaguk09 marked this conversation as resolved.
Show resolved Hide resolved

// Construct the modified resource name. If a custom hostname is provided,
// this will remove the bucket name from the resource.
Expand Down Expand Up @@ -882,4 +894,31 @@ private function buildQueryString(array $input)

return implode('&', $q);
}

/**
* Retry logic for signBlob
*
* @param callable $signBlobFn A callable that perform the actual signBlob operation.
* @param string $resourceName The resource name for logging or retry strategy determination.
* @param array $args Arguments for the operations, include preconditions
* @return string The signature genarated by signBlob.
* @throws ServiceException If non-retryable error occur.
* @throws \RuntimeException If retries are exhausted.
*/
private function retrySignBlob(callable $signBlobFn, string $resourceName = 'signBlob', array $args = [])
{
// Generate a retry decider function using the RetryTrait logic.
$retryDecider = $this->getRestRetryFunction($resourceName, 'execute', $args);
thiyaguk09 marked this conversation as resolved.
Show resolved Hide resolved
while (true) {
try {
// Attempt the operation
return $signBlobFn();
} catch (\Exception $exception) {
if (!$retryDecider($exception)) {
// Non-retryable error
throw $exception;
}
}
}
}
}
49 changes: 48 additions & 1 deletion Storage/tests/Unit/SigningHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use Google\Auth\Credentials\ServiceAccountCredentials;
use Google\Auth\SignBlobInterface;
use Google\Cloud\Core\Exception\ServiceException;
use Google\Cloud\Core\RequestWrapper;
use Google\Cloud\Core\Testing\TestHelpers;
use Google\Cloud\Core\Timestamp;
Expand Down Expand Up @@ -398,7 +399,7 @@ public function testV4SignCanonicalRequestSaveAsName()

$this->helper->createV4CanonicalRequest = function ($request) use ($saveAsName) {
parse_str($request[2], $query);
$expectedDisposition = 'attachment; filename="' . $saveAsName .'"';
$expectedDisposition = 'attachment; filename="' . $saveAsName . '"';
$this->assertEquals($expectedDisposition, $query['response-content-disposition']);
};

Expand Down Expand Up @@ -789,6 +790,52 @@ private function mockConnection($credentials = null, $rw = null)

return $conn->reveal();
}

public function testRetrySignBlobSuccessFirstAttempt()
{
$signBlobFn = function () {
return 'signature';
};

$res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
$signBlobFn
]);

$this->assertEquals('signature', $res);
}

public function testRetrySignBlobSuccessAfterRetries()
{
$attempt = 0;
$signBlobFn = function () use (&$attempt) {
if (++$attempt < 3) {
throw new ServiceException('Transient error', 503);
}
return 'signature';
};

$res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
$signBlobFn
]);

$this->assertEquals('signature', $res);
thiyaguk09 marked this conversation as resolved.
Show resolved Hide resolved
}

public function testRetrySignBlobNonRetryableError()
{
$this->expectException(ServiceException::class);
$this->expectExceptionMessage('Non-retryable error');

$signBlobFn = function () {
throw new ServiceException('Non-retryable error', 400);
};

$res = $this->helper->proxyPrivateMethodCall('retrySignBlob', [
$signBlobFn
]);

$this->assertEquals('Non-retryable error', $res);
thiyaguk09 marked this conversation as resolved.
Show resolved Hide resolved
}
}

//@codingStandardsIgnoreStart
Expand Down
Loading