Skip to content

Commit a23f559

Browse files
committed
fix: gcp bugs
1. Invalid release up of the pool client handle. 2. add a missing header for gcp metadata request. Signed-off-by: Roman Gershman <romange@gmail.com>
1 parent 875cedd commit a23f559

File tree

2 files changed

+17
-8
lines changed

2 files changed

+17
-8
lines changed

util/cloud/gcp/gcs.cc

+6-1
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,11 @@ io::Result<TokenTtl> ParseTokenResponse(std::string&& response) {
144144
return result;
145145
}
146146

147+
static constexpr string_view kMetaDataHost = "metadata.google.internal"sv;
148+
147149
error_code ConfigureMetadataClient(fb2::ProactorBase* pb, http::Client* client) {
148150
client->set_connect_timeout_ms(1000);
149-
static const char kMetaDataHost[] = "metadata.google.internal";
151+
150152
return client->Connect(kMetaDataHost, "80");
151153
}
152154

@@ -158,6 +160,7 @@ error_code ReadGCPConfigFromMetadata(fb2::ProactorBase* pb, string* account_id,
158160
const char kEmailUrl[] = "/computeMetadata/v1/instance/service-accounts/default/email";
159161
h2::request<h2::empty_body> req{h2::verb::get, kEmailUrl, 11};
160162
req.set("Metadata-Flavor", "Google");
163+
req.set(h2::field::host, kMetaDataHost.data());
161164

162165
h2::response<h2::string_body> resp;
163166
RETURN_ERROR(client.Send(req, &resp));
@@ -270,6 +273,8 @@ error_code GCPCredsProvider::RefreshToken() {
270273

271274
h2::request<h2::empty_body> req{h2::verb::get, kInstanceTokenUrl, 11};
272275
req.set("Metadata-Flavor", "Google");
276+
req.set(h2::field::host, kMetaDataHost.data());
277+
273278
RETURN_ERROR(client.Send(req, &resp));
274279
} else {
275280
constexpr char kDomain[] = "oauth2.googleapis.com";

util/cloud/gcp/gcs_file.cc

+11-7
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
#include "util/cloud/gcp/gcs_file.h"
55

6+
#include <absl/cleanup/cleanup.h>
67
#include <absl/strings/str_cat.h>
78
#include <absl/strings/strip.h>
89

@@ -93,7 +94,7 @@ class GcsWriteFile : public detail::AbstractStorageFile {
9394
GcsWriteFileOptions opts_;
9495
};
9596

96-
class GcsReadFile final : public io::ReadonlyFile {
97+
class GcsReadFile final : public io::ReadonlyFile {
9798
public:
9899
// does not own gcs object, only wraps it with ReadonlyFile interface.
99100
GcsReadFile(string read_obj_url, const GcsReadFileOptions& opts)
@@ -193,7 +194,6 @@ error_code GcsWriteFile::Close() {
193194
return {};
194195
}
195196

196-
197197
error_code GcsWriteFile::Upload() {
198198
size_t body_size = body_mb_.size();
199199
CHECK_GT(body_size, 0u);
@@ -317,11 +317,17 @@ io::Result<io::WriteFile*> OpenWriteGcsFile(const string& bucket, const string&
317317

318318
RobustSender sender(opts.pool, opts.creds_provider);
319319
RobustSender::SenderResult send_res;
320-
error_code ec = sender.Send(3, &empty_req, &send_res);
321-
if (ec) {
320+
absl::Cleanup cleanup([&] {
322321
if (opts.pool_owned) {
322+
// We must reset the client handle before deleting the pool that owns it.
323+
send_res.client_handle.reset();
323324
delete opts.pool;
324325
}
326+
});
327+
328+
error_code ec = sender.Send(3, &empty_req, &send_res);
329+
330+
if (ec) {
325331
return nonstd::make_unexpected(ec);
326332
}
327333

@@ -330,11 +336,9 @@ io::Result<io::WriteFile*> OpenWriteGcsFile(const string& bucket, const string&
330336
auto it = headers.find(h2::field::location);
331337
if (it == headers.end()) {
332338
LOG(ERROR) << "Could not find location in " << headers;
333-
if (opts.pool_owned) {
334-
delete opts.pool;
335-
}
336339
return nonstd::make_unexpected(make_error_code(errc::connection_refused));
337340
}
341+
std::move(cleanup).Cancel();
338342

339343
return new GcsWriteFile(key, detail::FromBoostSV(it->value()), opts);
340344
}

0 commit comments

Comments
 (0)