Skip to content

Commit 38eaa56

Browse files
coryancoryan
authored
ci: enable GCS+gRPC production integration tests (#9060)
The integration tests against production were disabled while the protos stabilized. There are no anticipated breaking changes at this point. We cannot pass the options created for a `GrpcClient` to a `CurlClient`. The latter has a very subtle interpretation of `AuthorityOption` as it supports two separate services: `iamcredentials.googleapis.com` is used to sign blobs. A number of tests remain disabled because they use metadata operations in gRPC, which are not ready in production. Changed their TODO entries to point to the right bug.
1 parent d8de996 commit 38eaa56

17 files changed

Lines changed: 150 additions & 41 deletions

‎ci/cloudbuild/builds/gcs-grpc.sh‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ mapfile -t args < <(bazel::common_args)
3333
# Run as many of the integration tests as possible using production and gRPC:
3434
# "media" says to use the hybrid gRPC/REST client. For more details see
3535
# https://github.com/googleapis/google-cloud-cpp/issues/6268
36-
# TODO(#7257) - restore production tests
37-
# readonly GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG=media
36+
readonly GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG=media
3837
mapfile -t integration_args < <(integration::bazel_args)
3938
bazel test "${args[@]}" "${integration_args[@]}" -- //google/cloud/storage/... "${excluded_rules[@]}"

‎google/cloud/storage/google_cloud_cpp_storage_grpc.cmake‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ if (BUILD_TESTING AND GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
149149

150150
set(storage_client_grpc_unit_tests
151151
# cmake-format: sort
152+
grpc_plugin_test.cc
152153
internal/grpc_bucket_access_control_parser_test.cc
153154
internal/grpc_bucket_metadata_parser_test.cc
154155
internal/grpc_bucket_request_parser_test.cc

‎google/cloud/storage/grpc_plugin.cc‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,23 @@
1515
#include "google/cloud/storage/grpc_plugin.h"
1616
#include "google/cloud/storage/internal/grpc_client.h"
1717
#include "google/cloud/storage/internal/hybrid_client.h"
18+
#include "google/cloud/internal/getenv.h"
1819

1920
namespace google {
2021
namespace cloud {
2122
namespace storage_experimental {
2223
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
2324

2425
google::cloud::storage::Client DefaultGrpcClient(Options opts) {
25-
opts = google::cloud::storage::internal::DefaultOptionsGrpc(std::move(opts));
26-
auto config = opts.get<GrpcPluginOption>();
26+
using ::google::cloud::internal::GetEnv;
27+
auto const config = GetEnv("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG")
28+
.value_or(opts.get<GrpcPluginOption>());
2729
if (config == "none" || config.empty()) {
2830
return google::cloud::storage::Client(std::move(opts));
2931
}
3032
if (config == "metadata") {
33+
opts =
34+
google::cloud::storage::internal::DefaultOptionsGrpc(std::move(opts));
3135
return storage::internal::ClientImplDetails::CreateClient(
3236
storage::internal::GrpcClient::Create(opts));
3337
}

‎google/cloud/storage/grpc_plugin_test.cc‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
// Copyright 2022 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include "google/cloud/storage/grpc_plugin.h"
16+
#include "google/cloud/storage/internal/curl_client.h"
17+
#include "google/cloud/storage/internal/grpc_client.h"
18+
#include "google/cloud/storage/internal/hybrid_client.h"
19+
#include "google/cloud/storage/internal/retry_client.h"
20+
#include "google/cloud/testing_util/scoped_environment.h"
21+
#include <gmock/gmock.h>
22+
23+
namespace google {
24+
namespace cloud {
25+
namespace storage_experimental {
26+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN
27+
namespace {
28+
29+
using ::google::cloud::storage::internal::ClientImplDetails;
30+
using ::google::cloud::storage::internal::CurlClient;
31+
using ::google::cloud::storage::internal::GrpcClient;
32+
using ::google::cloud::storage::internal::HybridClient;
33+
using ::google::cloud::storage::internal::RetryClient;
34+
using ::google::cloud::testing_util::ScopedEnvironment;
35+
using ::testing::IsNull;
36+
using ::testing::NotNull;
37+
38+
TEST(GrpcPluginTest, MetadataConfigCreatesGrpc) {
39+
// Explicitly disable the RestClient, which may be enabled by our CI builds.
40+
auto rest = ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_HAVE_REST_CLIENT",
41+
absl::nullopt);
42+
// Explicitly disable logging, which may be enabled by our CI builds.
43+
auto logging =
44+
ScopedEnvironment("CLOUD_STORAGE_ENABLE_TRACING", absl::nullopt);
45+
auto config =
46+
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", absl::nullopt);
47+
auto client = DefaultGrpcClient(Options{}.set<GrpcPluginOption>("metadata"));
48+
auto const* const retry =
49+
dynamic_cast<RetryClient*>(ClientImplDetails::GetRawClient(client).get());
50+
ASSERT_THAT(retry, NotNull());
51+
auto const* const grpc =
52+
dynamic_cast<GrpcClient const*>(retry->client().get());
53+
ASSERT_THAT(grpc, NotNull());
54+
}
55+
56+
TEST(GrpcPluginTest, EnvironmentOverrides) {
57+
// Explicitly disable the RestClient, which may be enabled by our CI builds.
58+
auto rest = ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_HAVE_REST_CLIENT",
59+
absl::nullopt);
60+
// Explicitly disable logging, which may be enabled by our CI builds.
61+
auto logging =
62+
ScopedEnvironment("CLOUD_STORAGE_ENABLE_TRACING", absl::nullopt);
63+
auto config =
64+
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", "none");
65+
auto client = DefaultGrpcClient(Options{}.set<GrpcPluginOption>("metadata"));
66+
auto const* const retry =
67+
dynamic_cast<RetryClient*>(ClientImplDetails::GetRawClient(client).get());
68+
ASSERT_THAT(retry, NotNull());
69+
auto const* const grpc = dynamic_cast<GrpcClient*>(retry->client().get());
70+
ASSERT_THAT(grpc, IsNull());
71+
}
72+
73+
TEST(GrpcPluginTest, UnsetConfigCreatesCurl) {
74+
// Explicitly disable the RestClient, which may be enabled by our CI builds.
75+
auto rest = ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_HAVE_REST_CLIENT",
76+
absl::nullopt);
77+
// Explicitly disable logging, which may be enabled by our CI builds.
78+
auto logging =
79+
ScopedEnvironment("CLOUD_STORAGE_ENABLE_TRACING", absl::nullopt);
80+
auto config =
81+
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", absl::nullopt);
82+
auto client = DefaultGrpcClient(Options{});
83+
auto const* const retry =
84+
dynamic_cast<RetryClient*>(ClientImplDetails::GetRawClient(client).get());
85+
ASSERT_THAT(retry, NotNull());
86+
auto const* const curl = dynamic_cast<CurlClient*>(retry->client().get());
87+
ASSERT_THAT(curl, NotNull());
88+
}
89+
90+
TEST(GrpcPluginTest, NoneConfigCreatesCurl) {
91+
// Explicitly disable the RestClient, which may be enabled by our CI builds.
92+
auto rest = ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_HAVE_REST_CLIENT",
93+
absl::nullopt);
94+
// Explicitly disable logging, which may be enabled by our CI builds.
95+
auto logging =
96+
ScopedEnvironment("CLOUD_STORAGE_ENABLE_TRACING", absl::nullopt);
97+
auto config =
98+
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", absl::nullopt);
99+
auto client = DefaultGrpcClient(Options{}.set<GrpcPluginOption>("none"));
100+
auto const* const retry =
101+
dynamic_cast<RetryClient*>(ClientImplDetails::GetRawClient(client).get());
102+
ASSERT_THAT(retry, NotNull());
103+
auto const* const curl = dynamic_cast<CurlClient*>(retry->client().get());
104+
ASSERT_THAT(curl, NotNull());
105+
}
106+
107+
TEST(GrpcPluginTest, MediaConfigCreatesHybrid) {
108+
// Explicitly disable the RestClient, which may be enabled by our CI builds.
109+
auto rest = ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_HAVE_REST_CLIENT",
110+
absl::nullopt);
111+
// Explicitly disable logging, which may be enabled by our CI builds.
112+
auto logging =
113+
ScopedEnvironment("CLOUD_STORAGE_ENABLE_TRACING", absl::nullopt);
114+
auto config =
115+
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", absl::nullopt);
116+
auto client = DefaultGrpcClient(Options{}.set<GrpcPluginOption>("media"));
117+
auto const* const retry =
118+
dynamic_cast<RetryClient*>(ClientImplDetails::GetRawClient(client).get());
119+
ASSERT_THAT(retry, NotNull());
120+
auto const* const hybrid = dynamic_cast<HybridClient*>(retry->client().get());
121+
ASSERT_THAT(hybrid, NotNull());
122+
}
123+
124+
} // namespace
125+
GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END
126+
} // namespace storage_experimental
127+
} // namespace cloud
128+
} // namespace google

‎google/cloud/storage/internal/grpc_client.cc‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
// limitations under the License.
1414

1515
#include "google/cloud/storage/internal/grpc_client.h"
16-
#include "google/cloud/storage/grpc_plugin.h"
1716
#include "google/cloud/storage/internal/grpc_bucket_access_control_parser.h"
1817
#include "google/cloud/storage/internal/grpc_bucket_metadata_parser.h"
1918
#include "google/cloud/storage/internal/grpc_bucket_request_parser.h"
@@ -91,7 +90,6 @@ int DefaultGrpcNumChannels(std::string const& endpoint) {
9190

9291
Options DefaultOptionsGrpc(Options options) {
9392
using ::google::cloud::internal::GetEnv;
94-
using ::google::cloud::storage_experimental::GrpcPluginOption;
9593

9694
options = DefaultOptionsWithCredentials(std::move(options));
9795
if (!options.has<UnifiedCredentialsOption>() &&
@@ -106,10 +104,6 @@ Options DefaultOptionsGrpc(Options options) {
106104
if (!options.has<AuthorityOption>()) {
107105
options.set<AuthorityOption>("storage.googleapis.com");
108106
}
109-
if (!options.has<GrpcPluginOption>()) {
110-
options.set<GrpcPluginOption>(
111-
GetEnv("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG").value_or("none"));
112-
}
113107
if (GetEnv("CLOUD_STORAGE_EMULATOR_ENDPOINT")) {
114108
// The emulator does not support HTTPS or authentication, use insecure
115109
// (sometimes called "anonymous") credentials, which disable SSL.

‎google/cloud/storage/internal/grpc_client_test.cc‎

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
#include "google/cloud/grpc_options.h"
2020
#include "google/cloud/options.h"
2121
#include "google/cloud/testing_util/is_proto_equal.h"
22-
#include "google/cloud/testing_util/scoped_environment.h"
2322
#include "google/cloud/testing_util/status_matchers.h"
2423
#include "google/cloud/testing_util/validate_metadata.h"
2524
#include <google/protobuf/text_format.h>
@@ -34,9 +33,7 @@ namespace {
3433

3534
namespace v2 = ::google::storage::v2;
3635
using ::google::cloud::internal::CurrentOptions;
37-
using ::google::cloud::storage_experimental::GrpcPluginOption;
3836
using ::google::cloud::testing_util::IsProtoEqual;
39-
using ::google::cloud::testing_util::ScopedEnvironment;
4037
using ::google::cloud::testing_util::StatusIs;
4138
using ::google::cloud::testing_util::ValidateMetadataFixture;
4239
using ::google::protobuf::TextFormat;
@@ -182,24 +179,6 @@ std::shared_ptr<GrpcClient> CreateTestClient(
182179
return GrpcClient::CreateMock(std::move(stub), TestOptions());
183180
}
184181

185-
TEST_F(GrpcClientTest, DefaultOptionsGrpcNoEnv) {
186-
auto grpc_config =
187-
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", absl::nullopt);
188-
auto opts = DefaultOptionsGrpc(TestOptions());
189-
EXPECT_EQ(opts.get<GrpcPluginOption>(), "none");
190-
opts = DefaultOptionsGrpc(Options{}.set<GrpcPluginOption>("configured"));
191-
EXPECT_EQ(opts.get<GrpcPluginOption>(), "configured");
192-
}
193-
194-
TEST_F(GrpcClientTest, DefaultOptionsGrpcWithEnv) {
195-
auto grpc_config =
196-
ScopedEnvironment("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG", "env");
197-
auto opts = DefaultOptionsGrpc(TestOptions());
198-
EXPECT_EQ(opts.get<GrpcPluginOption>(), "env");
199-
opts = DefaultOptionsGrpc(Options{}.set<GrpcPluginOption>("configured"));
200-
EXPECT_EQ(opts.get<GrpcPluginOption>(), "configured");
201-
}
202-
203182
TEST_F(GrpcClientTest, DefaultOptionsGrpcChannelCount) {
204183
using ::google::cloud::GrpcNumChannelsOption;
205184
struct TestCase {

‎google/cloud/storage/internal/hybrid_client.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ std::shared_ptr<RawClient> HybridClient::Create(Options const& options) {
2626
}
2727

2828
HybridClient::HybridClient(Options const& options)
29-
: grpc_(GrpcClient::Create(options)), curl_(CurlClient::Create(options)) {}
29+
: grpc_(GrpcClient::Create(DefaultOptionsGrpc(std::move(options)))),
30+
curl_(CurlClient::Create(DefaultOptionsWithCredentials(options))) {}
3031

3132
ClientOptions const& HybridClient::client_options() const {
3233
return curl_->client_options();

‎google/cloud/storage/storage_client_grpc_unit_tests.bzl‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"""Automatically generated unit tests list - DO NOT EDIT."""
1818

1919
storage_client_grpc_unit_tests = [
20+
"grpc_plugin_test.cc",
2021
"internal/grpc_bucket_access_control_parser_test.cc",
2122
"internal/grpc_bucket_metadata_parser_test.cc",
2223
"internal/grpc_bucket_request_parser_test.cc",

‎google/cloud/storage/tests/grpc_bucket_acl_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class GrpcBucketAclIntegrationTest
4545
TEST_F(GrpcBucketAclIntegrationTest, AclCRUD) {
4646
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
4747
"metadata");
48-
// TODO(#7257) - restore gRPC integration tests against production
48+
// TODO(#5673) - restore gRPC integration tests against production
4949
if (!UsingEmulator()) GTEST_SKIP();
5050

5151
auto const project_id = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");

‎google/cloud/storage/tests/grpc_bucket_metadata_integration_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class GrpcBucketMetadataIntegrationTest
4848
TEST_F(GrpcBucketMetadataIntegrationTest, ObjectMetadataCRUD) {
4949
ScopedEnvironment grpc_config("GOOGLE_CLOUD_CPP_STORAGE_GRPC_CONFIG",
5050
"metadata");
51-
// TODO(#7257) - restore gRPC integration tests against production
51+
// TODO(#5673) - restore gRPC integration tests against production
5252
if (!UsingEmulator()) GTEST_SKIP();
5353

5454
auto const project_name = GetEnv("GOOGLE_CLOUD_PROJECT").value_or("");

0 commit comments

Comments
 (0)