use ServiceAddress in curl implementation

Summary: This diff makes `MononokeCurlBackingStore` uses `ServiceAddress` for getting address of Mononoke service.

Reviewed By: chadaustin

Differential Revision: D14864675

fbshipit-source-id: 5c3d5e82a3bd6b55ff722427b3f44d20a3544a1d

eden/fs/store/hg/HgBackingStore.cpp

@@ -383,6 +383,21 @@ void HgBackingStore::initializeTreeManifestImport(
 #endif // EDEN_HAVE_HG_TREEMANIFEST
 }
 
+std::unique_ptr<ServiceAddress> HgBackingStore::getMononokeServiceAddress() {
+  auto edenConfig = config_->getEdenConfig();
+  auto hostname = edenConfig->getMononokeHostName();
+
+  if (hostname) {
+    auto port = edenConfig->getMononokePort();
+    XLOG(DBG2) << "Using " << *hostname << ":" << port << " for Mononoke";
+    return std::make_unique<ServiceAddress>(*hostname, port);
+  }
+
+  const auto& tier = edenConfig->getMononokeTierName();
+  XLOG(DBG2) << "Using SMC tier " << tier << " for Mononoke";
+  return std::make_unique<ServiceAddress>(tier);
+}
+
 #ifndef EDEN_WIN_NOMONONOKE
 std::unique_ptr<MononokeHttpBackingStore>
 HgBackingStore::initializeHttpMononokeBackingStore() {
@@ -399,27 +414,11 @@ HgBackingStore::initializeHttpMononokeBackingStore() {
     return nullptr;
   }
 
-  auto executor = folly::getIOExecutor();
-  auto hostname = edenConfig->getMononokeHostName();
-
-  std::unique_ptr<ServiceAddress> service;
-  if (hostname) {
-    auto port = edenConfig->getMononokePort();
-    service = std::make_unique<ServiceAddress>(*hostname, port);
-    XLOG(DBG2) << "HTTP Mononoke enabled for repository " << repoName_
-               << ", using host " << *hostname << ":" << port;
-  } else {
-    const auto& tier = edenConfig->getMononokeTierName();
-    service = std::make_unique<ServiceAddress>(tier);
-    XLOG(DBG2) << "HTTP Mononoke enabled for repository " << repoName_
-               << ", using tier " << tier;
-  }
-
   return std::make_unique<MononokeHttpBackingStore>(
-      std::move(service),
+      getMononokeServiceAddress(),
       repoName_,
       std::chrono::milliseconds(FLAGS_mononoke_timeout),
-      executor.get(),
+      folly::getIOExecutor().get(),
       sslContext);
 }
 
@@ -440,25 +439,16 @@ HgBackingStore::initializeThriftMononokeBackingStore() {
 std::unique_ptr<MononokeCurlBackingStore>
 HgBackingStore::initializeCurlMononokeBackingStore() {
   auto edenConfig = config_->getEdenConfig();
-  auto hostname = edenConfig->getMononokeHostName();
   auto clientCertificate = edenConfig->getClientCertificate();
 
-  if (!hostname) {
-    XLOG(WARN) << "Mononoke is disabled because no Mononoke host is provided";
-    return nullptr;
-  }
-
   if (!clientCertificate) {
     XLOG(WARN)
         << "Mononoke is disabled because no client certificate is provided";
     return nullptr;
   }
 
-  XLOG(DBG2) << "Initializing cURL Mononoke backing store for repository "
-             << repoName_ << ", using host " << *hostname;
-
   return std::make_unique<MononokeCurlBackingStore>(
-      *hostname,
+      getMononokeServiceAddress(),
       AbsolutePath(folly::to<std::string>(*clientCertificate)),
       repoName_,
       std::chrono::milliseconds(FLAGS_mononoke_timeout),

eden/fs/store/hg/HgBackingStore.h

@@ -43,6 +43,7 @@ class MononokeThriftBackingStore;
 class MononokeCurlBackingStore;
 class UnboundedQueueExecutor;
 class ReloadableConfig;
+class ServiceAddress;
 
 /**
  * A BackingStore implementation that loads data out of a mercurial repository.
@@ -117,6 +118,13 @@ class HgBackingStore : public BackingStore {
    */
   std::shared_ptr<BackingStore> getMononoke();
 
+  /**
+   * Get an instance of `ServiceAddress` that points to Mononoke API Server
+   * based on user's configuration. It could be a pair of host and port or a smc
+   * tier name.
+   */
+  std::unique_ptr<ServiceAddress> getMononokeServiceAddress();
+
 #ifndef EDEN_WIN_NOMONONOKE
   /**
    * Create an instance of MononokeHttpBackingStore with values from config_

eden/fs/store/mononoke/CurlHttpClient.cpp

@@ -30,15 +30,16 @@ write_callback(char* contents, size_t size, size_t nmemb, void* out) {
 } // namespace
 
 CurlHttpClient::CurlHttpClient(
-    std::string host,
+    folly::SocketAddress address,
     AbsolutePath certificate,
     std::chrono::milliseconds timeout)
-    : host_(std::move(host)),
+    : address_(std::move(address)),
       certificate_(std::move(certificate)),
       timeout_(timeout) {
   handle_ = buildRequest();
 }
 
+/// Makes an HTTP GET request to the given path.
 std::unique_ptr<folly::IOBuf> CurlHttpClient::get(const std::string& path) {
   auto buffer = folly::IOBufQueue{};
 
@@ -46,10 +47,12 @@ std::unique_ptr<folly::IOBuf> CurlHttpClient::get(const std::string& path) {
     throw std::runtime_error("curl failed to set CURLOPT_WRITEDATA");
   }
 
-  if (curl_easy_setopt(handle_.get(), CURLOPT_URL, (host_ + path).c_str()) !=
-      CURLE_OK) {
+  auto url = folly::to<std::string>(
+      "https://", address_.getHostStr(), ":", address_.getPort(), path);
+
+  if (curl_easy_setopt(handle_.get(), CURLOPT_URL, url.c_str()) != CURLE_OK) {
     throw std::runtime_error(
-        folly::to<std::string>("curl failed to set url: ", host_, path));
+        folly::to<std::string>("curl failed to set url: ", url));
   }
 
   auto ret = curl_easy_perform(handle_.get());

eden/fs/store/mononoke/CurlHttpClient.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include <curl/curl.h>
+#include <folly/SocketAddress.h>
 #include <folly/futures/Future.h>
 #include <folly/io/IOBuf.h>
 #include <memory>
@@ -34,7 +35,7 @@ struct CurlDeleter {
 class CurlHttpClient {
  public:
   CurlHttpClient(
-      std::string host,
+      folly::SocketAddress address,
       AbsolutePath certificate,
       std::chrono::milliseconds timeout);
 
@@ -44,7 +45,7 @@ class CurlHttpClient {
   void initGlobal();
   std::unique_ptr<CURL, CurlDeleter> buildRequest();
 
-  std::string host_;
+  folly::SocketAddress address_;
   AbsolutePath certificate_;
 
   // cURL timeout for the request (see CURLOPT_TIMEOUT_MS for detail)

eden/fs/store/mononoke/MononokeCurlBackingStore.cpp

@@ -16,6 +16,7 @@
 #include <folly/executors/thread_factory/NamedThreadFactory.h>
 #include <folly/executors/thread_factory/ThreadFactory.h>
 #include <folly/json.h>
+#include <folly/logging/xlog.h>
 
 #include "eden/fs/model/Blob.h"
 #include "eden/fs/model/Hash.h"
@@ -23,6 +24,7 @@
 #include "eden/fs/store/mononoke/CurlHttpClient.h"
 #include "eden/fs/store/mononoke/MononokeAPIUtils.h"
 #include "eden/fs/utils/PathFuncs.h"
+#include "eden/fs/utils/ServiceAddress.h"
 
 DEFINE_int32(
     mononoke_curl_threads,
@@ -39,7 +41,8 @@ static folly::ThreadLocalPtr<CurlHttpClient> threadCurlClient;
 CurlHttpClient& getCurlHttpClient() {
   if (!threadCurlClient) {
     throw std::logic_error(
-        "Attempting to use curl client in a non-curl client thread");
+        "Attempting to use curl client in a non-curl client thread "
+        "or failed to resolve service address");
   }
   return *threadCurlClient;
 }
@@ -47,31 +50,43 @@ CurlHttpClient& getCurlHttpClient() {
 class MononokeCurlThreadFactory : public folly::ThreadFactory {
  public:
   MononokeCurlThreadFactory(
-      std::string host,
+      std::unique_ptr<ServiceAddress> service,
       AbsolutePath certificate,
       std::chrono::milliseconds timeout)
       : delegate_("CurlClient"),
-        host_(host),
+        service_(std::move(service)),
         certificate_(certificate),
         timeout_(timeout) {}
 
   std::thread newThread(folly::Func&& func) override {
     return delegate_.newThread([this, func = std::move(func)]() mutable {
-      threadCurlClient.reset(new CurlHttpClient(host_, certificate_, timeout_));
-      func();
+      try {
+        auto address = service_->getSocketAddressBlocking();
+        if (address) {
+          threadCurlClient.reset(
+              new CurlHttpClient(address->first, certificate_, timeout_));
+          func();
+        } else {
+          XLOG(WARN) << "failed to resolve address for Mononoke API Server";
+        }
+      } catch (const std::exception& ex) {
+        XLOG(WARN)
+            << "failed to resolve address for Mononoke API Server, reason: "
+            << ex.what();
+      }
     });
   }
 
  private:
   folly::NamedThreadFactory delegate_;
-  std::string host_;
+  std::unique_ptr<ServiceAddress> service_;
   AbsolutePath certificate_;
   const std::chrono::milliseconds timeout_;
-};
+}; // namespace
 } // namespace
 
 MononokeCurlBackingStore::MononokeCurlBackingStore(
-    std::string host,
+    std::unique_ptr<ServiceAddress> service,
     AbsolutePath certificate,
     std::string repo,
     std::chrono::milliseconds timeout,
@@ -82,7 +97,7 @@ MononokeCurlBackingStore::MononokeCurlBackingStore(
           std::make_unique<folly::UnboundedBlockingQueue<
               folly::CPUThreadPoolExecutor::CPUTask>>(),
           std::make_shared<MononokeCurlThreadFactory>(
-              host,
+              std::move(service),
               certificate,
               timeout))),
       serverExecutor_(std::move(executor)) {}

eden/fs/store/mononoke/MononokeCurlBackingStore.h

@@ -28,11 +28,12 @@ namespace eden {
 class Blob;
 class Tree;
 class Hash;
+class ServiceAddress;
 
 class MononokeCurlBackingStore : public BackingStore {
  public:
   MononokeCurlBackingStore(
-      std::string host,
+      std::unique_ptr<ServiceAddress> service,
       AbsolutePath certificate,
       std::string repo,
       std::chrono::milliseconds timeout,

eden/fs/store/mononoke/test/CurlTest.cpp

@@ -44,6 +44,7 @@ class TestServer {
   }
 };
 
+// @lint-ignore-every PRIVATEKEY1
 const std::string kClientCACertName = "client-ca-cert.pem";
 const std::string kClientCACertContent = folly::stripLeftMargin(R"(
   -----BEGIN CERTIFICATE-----
@@ -263,8 +264,7 @@ class CurlTest : public ::testing::Test {
         certs_->path() / kClientCACertName);
 
     const auto address = server_->getAddresses()[0].address;
-    host_ = folly::to<std::string>(
-        "https://[", address.getFullyQualified(), "]:", address.getPort());
+    address_ = folly::SocketAddress("::1", address.getPort());
   }
 
   static void TearDownTestCase() {
@@ -274,16 +274,16 @@ class CurlTest : public ::testing::Test {
 
   static std::unique_ptr<TemporaryDirectory> certs_;
   static std::unique_ptr<ScopedHTTPServer> server_;
-  static std::string host_;
+  static folly::SocketAddress address_;
 };
 
 std::unique_ptr<TemporaryDirectory> CurlTest::certs_ = nullptr;
 std::unique_ptr<ScopedHTTPServer> CurlTest::server_ = nullptr;
-std::string CurlTest::host_ = "";
+folly::SocketAddress CurlTest::address_;
 
 TEST_F(CurlTest, Success) {
   auto client = CurlHttpClient(
-      host_,
+      address_,
       AbsolutePath((certs_->path() / kClientChainName).native()),
       std::chrono::milliseconds(100000));
 
@@ -294,7 +294,7 @@ TEST_F(CurlTest, Success) {
 
 TEST_F(CurlTest, InvalidClientCertificate) {
   auto client = CurlHttpClient(
-      host_,
+      address_,
       AbsolutePath((certs_->path() / kInvalidChainName).native()),
       std::chrono::milliseconds(100000));
 
@@ -308,7 +308,7 @@ TEST_F(CurlTest, InvalidClientCertificate) {
 
 TEST_F(CurlTest, ThrowOn4XX) {
   auto client = CurlHttpClient(
-      host_,
+      address_,
       AbsolutePath((certs_->path() / kClientChainName).native()),
       std::chrono::milliseconds(100000));
 

eden/win/fs/edenfs.vcxproj

@@ -205,6 +205,7 @@
     <ClCompile Include="..\..\fs\utils\PathFuncs.cpp" />
     <ClCompile Include="..\..\fs\utils\TimeUtil.cpp" />
     <ClCompile Include="..\..\fs\utils\UnboundedQueueExecutor.cpp" />
+    <ClCompile Include="..\..\fs\utils\ServiceAddress.cpp" />
     <ClCompile Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookService.cpp" />
     <ClCompile Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookServiceAsyncClient.cpp" />
     <ClCompile Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookService_processmap_binary.cpp" />
@@ -289,6 +290,7 @@
     <ClInclude Include="..\..\fs\utils\PathFuncs.h" />
     <ClInclude Include="..\..\fs\utils\TimeUtil.h" />
     <ClInclude Include="..\..\fs\utils\UnboundedQueueExecutor.h" />
+    <ClInclude Include="..\..\fs\utils\ServiceAddress.h" />
     <ClInclude Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookService.h" />
     <ClInclude Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookServiceAsyncClient.h" />
     <ClInclude Include="..\build\thrift\common\fb303\if\gen-cpp2\FacebookService_custom_protocol.h" />
@@ -342,4 +344,4 @@
   <ImportGroup Label="ExtensionTargets">
     <Import Project="D:\edenwin64\vcpkg\scripts\buildsystems\msbuild\vcpkg.targets" Condition="Exists('D:\edenwin64\vcpkg\scripts\buildsystems\msbuild\vcpkg.targets')" />
   </ImportGroup>
-</Project>
+</Project>

eden/win/fs/edenfs.vcxproj.filters

@@ -320,6 +320,9 @@
     <ClCompile Include="..\..\fs\tracing\EdenStats.cpp">
       <Filter>fs\Tracing</Filter>
     </ClCompile>
+    <ClCompile Include="..\..\fs\utils\ServiceAddress.cpp">
+      <Filter>fs\Util</Filter>
+    </ClCompile>
   </ItemGroup>
   <ItemGroup>
     <ClInclude Include="..\..\fs\store\hg\HgBackingStore.h">
@@ -583,6 +586,9 @@
     <ClInclude Include="utils\UserInfo.h">
       <Filter>Win\fs\Utils</Filter>
     </ClInclude>
+    <ClInclude Include="..\..\fs\utils\ServiceAddress.h">
+      <Filter>fs\Util</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <None Include="..\build\thrift\eden\fs\service\gen-cpp2\eden_types.tcc">
@@ -610,4 +616,4 @@
       <Filter>fs\Service</Filter>
     </None>
   </ItemGroup>
-</Project>
+</Project>