Skip to content

Commit

Permalink
feat(storage): per-operation options / service accounts (#9199)
Browse files Browse the repository at this point in the history 
This introduces support for per-operation `google::cloud::Options`,
starting with the operations related to service accounts.

This change also introduced some changes to the test fixtures that I
expect will be used in all tests.

Also fixed an unrelated typo.
  • Loading branch information
@coryan
coryan authored Jun 8, 2022
1 parent 8742011 commit e6c39c1
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 9 deletions.
14 changes: 14 additions & 0 deletions google/cloud/storage/client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include "google/cloud/storage/hmac_key_metadata.h"
#include "google/cloud/storage/internal/logging_client.h"
#include "google/cloud/storage/internal/make_options_span.h"
#include "google/cloud/storage/internal/parameter_pack_validation.h"
#include "google/cloud/storage/internal/policy_document_request.h"
#include "google/cloud/storage/internal/retry_client.h"
Expand Down Expand Up @@ -2493,6 +2494,7 @@ class Client {
template <typename... Options>
StatusOr<ServiceAccount> GetServiceAccountForProject(
std::string const& project_id, Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
internal::GetProjectServiceAccountRequest request(project_id);
request.set_multiple_options(std::forward<Options>(options)...);
return raw_client_->GetServiceAccount(request);
Expand Down Expand Up @@ -2527,6 +2529,7 @@ class Client {
*/
template <typename... Options>
StatusOr<ServiceAccount> GetServiceAccount(Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
return GetServiceAccountForProject(project_id,
std::forward<Options>(options)...);
Expand Down Expand Up @@ -2563,6 +2566,7 @@ class Client {
*/
template <typename... Options>
ListHmacKeysReader ListHmacKeys(Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
internal::ListHmacKeysRequest request(project_id);
request.set_multiple_options(std::forward<Options>(options)...);
Expand Down Expand Up @@ -2611,6 +2615,7 @@ class Client {
template <typename... Options>
StatusOr<std::pair<HmacKeyMetadata, std::string>> CreateHmacKey(
std::string service_account, Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
internal::CreateHmacKeyRequest request(project_id,
std::move(service_account));
Expand Down Expand Up @@ -2653,6 +2658,7 @@ class Client {
*/
template <typename... Options>
Status DeleteHmacKey(std::string access_id, Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
internal::DeleteHmacKeyRequest request(project_id, std::move(access_id));
request.set_multiple_options(std::forward<Options>(options)...);
Expand Down Expand Up @@ -2689,6 +2695,7 @@ class Client {
template <typename... Options>
StatusOr<HmacKeyMetadata> GetHmacKey(std::string access_id,
Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
internal::GetHmacKeyRequest request(project_id, std::move(access_id));
request.set_multiple_options(std::forward<Options>(options)...);
Expand Down Expand Up @@ -2730,6 +2737,7 @@ class Client {
StatusOr<HmacKeyMetadata> UpdateHmacKey(std::string access_id,
HmacKeyMetadata resource,
Options&&... options) {
auto const span = MakeSpan(std::forward<Options>(options)...);
auto const& project_id = raw_client_->client_options().project_id();
internal::UpdateHmacKeyRequest request(project_id, std::move(access_id),
std::move(resource));
Expand Down Expand Up @@ -3164,6 +3172,12 @@ class Client {
ObjectWriteStream WriteObjectImpl(
internal::ResumableUploadRequest const& request);

template <typename... RequestOptions>
google::cloud::internal::OptionsSpan MakeSpan(RequestOptions&&... o) const {
return internal::MakeOptionsSpan(raw_client_->options(),
std::forward<RequestOptions>(o)...);
}

// The version of UploadFile() where UseResumableUploadSession is one of the
// options. Note how this does not use InsertObjectMedia at all.
template <typename... Options>
Expand Down
29 changes: 22 additions & 7 deletions google/cloud/storage/client_service_account_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ namespace storage {
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
namespace {

using ::google::cloud::internal::CurrentOptions;
using ::google::cloud::storage::testing::canonical_errors::TransientError;
using ::testing::Return;
using ms = std::chrono::milliseconds;
Expand All @@ -49,13 +50,15 @@ TEST_F(ServiceAccountTest, GetProjectServiceAccount) {
.WillOnce(Return(StatusOr<ServiceAccount>(TransientError())))
.WillOnce(
[&expected](internal::GetProjectServiceAccountRequest const& r) {
EXPECT_EQ(CurrentOptions().get<AuthorityOption>(), "a-default");
EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
EXPECT_EQ("test-project", r.project_id());

return make_status_or(expected);
});
auto client = ClientForMock();
StatusOr<ServiceAccount> actual =
client.GetServiceAccountForProject("test-project");
StatusOr<ServiceAccount> actual = client.GetServiceAccountForProject(
"test-project", Options{}.set<UserProjectOption>("u-p-test"));
ASSERT_STATUS_OK(actual);
EXPECT_EQ(expected, *actual);
}
Expand Down Expand Up @@ -89,6 +92,8 @@ TEST_F(ServiceAccountTest, CreateHmacKey) {
.WillOnce(
Return(StatusOr<internal::CreateHmacKeyResponse>(TransientError())))
.WillOnce([&expected](internal::CreateHmacKeyRequest const& r) {
EXPECT_EQ(CurrentOptions().get<AuthorityOption>(), "a-default");
EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
EXPECT_EQ("test-project", r.project_id());
EXPECT_EQ("test-service-account", r.service_account());

Expand All @@ -97,7 +102,8 @@ TEST_F(ServiceAccountTest, CreateHmacKey) {
auto client = ClientForMock();
StatusOr<std::pair<HmacKeyMetadata, std::string>> actual =
client.CreateHmacKey("test-service-account",
OverrideDefaultProject("test-project"));
OverrideDefaultProject("test-project"),
Options{}.set<UserProjectOption>("u-p-test"));
ASSERT_STATUS_OK(actual);
EXPECT_EQ(expected.metadata, actual->first);
EXPECT_EQ(expected.secret, actual->second);
Expand Down Expand Up @@ -126,14 +132,17 @@ TEST_F(ServiceAccountTest, DeleteHmacKey) {
EXPECT_CALL(*mock_, DeleteHmacKey)
.WillOnce(Return(StatusOr<internal::EmptyResponse>(TransientError())))
.WillOnce([](internal::DeleteHmacKeyRequest const& r) {
EXPECT_EQ(CurrentOptions().get<AuthorityOption>(), "a-default");
EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
EXPECT_EQ("test-project", r.project_id());
EXPECT_EQ("test-access-id-1", r.access_id());

return make_status_or(internal::EmptyResponse{});
});
auto client = ClientForMock();
Status actual = client.DeleteHmacKey("test-access-id-1",
OverrideDefaultProject("test-project"));
Status actual = client.DeleteHmacKey(
"test-access-id-1", OverrideDefaultProject("test-project"),
Options{}.set<UserProjectOption>("u-p-test"));
ASSERT_STATUS_OK(actual);
}

Expand Down Expand Up @@ -161,14 +170,17 @@ TEST_F(ServiceAccountTest, GetHmacKey) {
EXPECT_CALL(*mock_, GetHmacKey)
.WillOnce(Return(StatusOr<HmacKeyMetadata>(TransientError())))
.WillOnce([&expected](internal::GetHmacKeyRequest const& r) {
EXPECT_EQ(CurrentOptions().get<AuthorityOption>(), "a-default");
EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
EXPECT_EQ("test-project", r.project_id());
EXPECT_EQ("test-access-id-1", r.access_id());

return make_status_or(expected);
});
auto client = ClientForMock();
StatusOr<HmacKeyMetadata> actual = client.GetHmacKey(
"test-access-id-1", OverrideDefaultProject("test-project"));
"test-access-id-1", OverrideDefaultProject("test-project"),
Options{}.set<UserProjectOption>("u-p-test"));
ASSERT_STATUS_OK(actual);
EXPECT_EQ(expected.access_id(), actual->access_id());
EXPECT_EQ(expected.state(), actual->state());
Expand Down Expand Up @@ -202,6 +214,8 @@ TEST_F(ServiceAccountTest, UpdateHmacKey) {
EXPECT_CALL(*mock_, UpdateHmacKey)
.WillOnce(Return(StatusOr<HmacKeyMetadata>(TransientError())))
.WillOnce([&expected](internal::UpdateHmacKeyRequest const& r) {
EXPECT_EQ(CurrentOptions().get<AuthorityOption>(), "a-default");
EXPECT_EQ(CurrentOptions().get<UserProjectOption>(), "u-p-test");
EXPECT_EQ("test-project", r.project_id());
EXPECT_EQ("test-access-id-1", r.access_id());

Expand All @@ -210,7 +224,8 @@ TEST_F(ServiceAccountTest, UpdateHmacKey) {
auto client = ClientForMock();
StatusOr<HmacKeyMetadata> actual = client.UpdateHmacKey(
"test-access-id-1", HmacKeyMetadata().set_state("ACTIVE"),
OverrideDefaultProject("test-project"));
OverrideDefaultProject("test-project"),
Options{}.set<UserProjectOption>("u-p-test"));
ASSERT_STATUS_OK(actual);
EXPECT_EQ(expected.access_id(), actual->access_id());
EXPECT_EQ(expected.state(), actual->state());
Expand Down
16 changes: 16 additions & 0 deletions google/cloud/storage/internal/hmac_key_requests.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,22 @@ class GenericHmacKeyRequest
}

Derived& set_multiple_options() { return *static_cast<Derived*>(this); }
template <typename... T>
Derived& set_multiple_options(google::cloud::Options const&&, T&&... tail) {
return set_multiple_options(std::forward<T>(tail)...);
}
template <typename... T>
Derived& set_multiple_options(google::cloud::Options const&, T&&... tail) {
return set_multiple_options(std::forward<T>(tail)...);
}
template <typename... T>
Derived& set_multiple_options(google::cloud::Options&&, T&&... tail) {
return set_multiple_options(std::forward<T>(tail)...);
}
template <typename... T>
Derived& set_multiple_options(google::cloud::Options&, T&&... tail) {
return set_multiple_options(std::forward<T>(tail)...);
}

using GenericRequest<Derived, UserProject, Options...>::set_option;
void set_option(OverrideDefaultProject const& o) {
Expand Down
2 changes: 1 addition & 1 deletion google/cloud/storage/internal/make_options_span.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace internal {
* values are preferred (i.e. they override previous values) as defined by
* `google::cloud::internal::MergeOptions()`.
*
* @note This does not support `volative`-qualified references.
* @note This does not support `volatile`-qualified references.
*/
inline google::cloud::Options GroupOptions() {
return google::cloud::Options{};
Expand Down
9 changes: 8 additions & 1 deletion google/cloud/storage/testing/client_unit_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,18 @@ namespace cloud {
namespace storage {
namespace testing {

using ::testing::Return;
using ::testing::ReturnRef;

ClientUnitTest::ClientUnitTest()
: mock_(std::make_shared<testing::MockClient>()),
client_options_(ClientOptions(oauth2::CreateAnonymousCredentials())) {
EXPECT_CALL(*mock_, client_options())
.WillRepeatedly(::testing::ReturnRef(client_options_));
.WillRepeatedly(ReturnRef(client_options_));
EXPECT_CALL(*mock_, options)
.WillRepeatedly(Return(Options{}
.set<AuthorityOption>("a-default")
.set<UserProjectOption>("u-p-default")));
}

Client ClientUnitTest::ClientForMock() {
Expand Down

0 comments on commit e6c39c1

Please sign in to comment.