From 708e0ada1aa52f0627f0554ad01b8d22e099850c Mon Sep 17 00:00:00 2001
From: Paul Cristian Sarbu <paul.cristian.sarbu@huawei.com>
Date: Fri, 8 Dec 2023 09:50:57 +0100
Subject: serve configuration: Fix remote execution check to allow unset value

In the scenario when 'just serve' acts as 'just execute', the
remote execution endpoint returned by the serve service should be
allowed to be empty.
In this case, from the server's perspective, there is nothing to be
checked, however a client might still want to ensure that its own
configured serve and execution endpoints match.
---
 .../serve_api/remote/configuration_client.cpp      | 59 ++++++++++++++++++----
 1 file changed, 48 insertions(+), 11 deletions(-)

(limited to 'src/buildtool/serve_api/remote/configuration_client.cpp')

diff --git a/src/buildtool/serve_api/remote/configuration_client.cpp b/src/buildtool/serve_api/remote/configuration_client.cpp
index b4f74230..acb43886 100644
--- a/src/buildtool/serve_api/remote/configuration_client.cpp
+++ b/src/buildtool/serve_api/remote/configuration_client.cpp
@@ -14,8 +14,11 @@
 
 #include "src/buildtool/serve_api/remote/configuration_client.hpp"
 
+#include <optional>
+
 #include "nlohmann/json.hpp"
 #include "src/buildtool/execution_api/remote/config.hpp"
+#include "src/buildtool/serve_api/remote/config.hpp"
 
 auto ConfigurationClient::CheckServeRemoteExecution() -> bool {
     auto client_remote_address = RemoteExecutionConfig::RemoteAddress();
@@ -25,6 +28,13 @@ auto ConfigurationClient::CheckServeRemoteExecution() -> bool {
                      "have been set.");
         return false;
     }
+    auto client_serve_address = RemoteServeConfig::RemoteAddress();
+    if (!client_serve_address) {
+        logger_.Emit(
+            LogLevel::Error,
+            "Internal error: the serve endpoint should have been set.");
+        return false;
+    }
 
     grpc::ClientContext context;
     justbuild::just_serve::RemoteExecutionEndpointRequest request{};
@@ -36,16 +46,43 @@ auto ConfigurationClient::CheckServeRemoteExecution() -> bool {
         LogStatus(&logger_, LogLevel::Error, status);
         return false;
     }
-    auto serve_remote_endpoint = nlohmann::json::parse(response.address());
-    if (serve_remote_endpoint != client_remote_address->ToJson()) {
-        Logger::Log(LogLevel::Error,
-                    "Different execution endpoint detected.\nIn order to "
-                    "correctly use just serve, its remote execution endpoint "
-                    "must be the same used by the client.\nserve remote "
-                    "endpoint:  {}\nclient remote endpoint: {}",
-                    serve_remote_endpoint.dump(),
-                    client_remote_address->ToJson().dump());
-        return false;
+    auto client_msg = client_remote_address->ToJson().dump();
+    std::string serve_msg{};
+
+    if (response.address().empty()) {
+        // just serve acts as just execute, so from the server's perspective
+        // there is nothing to be checked and it's the client's job to ensure
+        // that its remote execution and serve endpoints match
+        //
+        // NOTE: This check might make sense to be removed altogether in the
+        // future, or updated to (at most) a warning.
+        if (client_remote_address->ToJson() == client_serve_address->ToJson()) {
+            return true;
+        }
+        serve_msg = client_serve_address->ToJson().dump();
+    }
+    else {
+        nlohmann::json serve_remote_endpoint{};
+        try {
+            serve_remote_endpoint = nlohmann::json::parse(response.address());
+        } catch (std::exception const& ex) {
+            logger_.Emit(
+                LogLevel::Error,
+                "Parsing configured address from response failed with:\n{}",
+                ex.what());
+        }
+        if (serve_remote_endpoint == client_remote_address->ToJson()) {
+            return true;
+        }
+        serve_msg = serve_remote_endpoint.dump();
     }
-    return true;
+    // log any mismatch found
+    logger_.Emit(
+        LogLevel::Error,
+        "Different execution endpoint detected.\nIn order to correctly use "
+        "just serve, its remote execution endpoint must be the same used by "
+        "the client.\nserve remote endpoint:  {}\nclient remote endpoint: {}",
+        serve_msg,
+        client_msg);
+    return false;
 }
-- 
cgit v1.2.3