feat: Add diagnostics messages and identity to storage helper greeting

Storage helpers may need to report startup problems back to ccache, e.g.
unknown or malformed attributes. This can now be done in a new greeting
format where such diagnostic messages can be included. It also adds a
server identity field so that ccache can log the storage helper version.

diff --git a/doc/remote_storage_helper_spec.md b/doc/remote_storage_helper_spec.md
index 66f3c4b37910e132063788d557978481df6984c0..be29e17d96654312aa8e786e28ba09a27f1fe28f 100644 (file)
--- a/doc/remote_storage_helper_spec.md
+++ b/doc/remote_storage_helper_spec.md
@@ -37,14 +37,18 @@ is no authentication or authorization mechanism within the protocol.
 
 ## Storage helper startup
 
-The helper program will get needed information as environment variables:
+The helper program will get startup parameters as environment variables:
 
 - `CRSH_IPC_ENDPOINT`: Unix socket path or Windows named pipe name (without
-  `\\.\pipe\` prefix).
+  `\\.\pipe\` prefix). The client is responsible for choosing an endpoint name
+  that is unique for the startup parameters that affect helper behavior.
 - `CRSH_URL`: URL to the remote storage server.
 - `CRSH_IDLE_TIMEOUT`: Timeout (in seconds) for the storage helper to wait
   before exiting after client inactivity. If set to 0, the helper will stay up
   indefinitely and never exit due to inactivity.
+- `CRSH_FORMAT_MAX`: Maximum version of the protocol greeting message format
+  supported by the client as a positive integer. If not set, the storage helper
+  must use the default value `1`.
 
 Custom attributes from ccache's `remote_storage` configuration will also
 provided as environment variables:
@@ -101,22 +105,37 @@ This is a specification of the custom binary IPC protocol between ccache
                                     ;   put: key/value not stored
                                     ;   remove: key/value not removed
 <err>             ::= 0x02 <msg>    ; e.g. bad parameters, network/server errors
+<capabilities>    ::= <cap_len> <cap_data>
+<cap_len>         ::= <u8>
+<cap_data>        ::= <cap>*        ; <cap_len> entries
+<cap>             ::= <cap0>        ; currently only one capability is defined
+<cap0>            ::= 0x00          ; get/put/remove/stop operations
 ```
 
 ### Server greeting (server to client)
 
-The server sends a greeting to the client when the client has connected. The
-client must verify protocol version and server capabilities before sending any
-request.
+In response to the client's startup parameters, the server sends a greeting to
+the client when the client has connected. The server must use the highest
+greeting message format that is supported by both sides. If the client's maximum
+greeting message format (`CRSH_FORMAT_MAX`) is lower than the lowest format the
+server supports, the server must close the connection.
+
+The client must verify server capabilities before sending any request.
+
+#### Greeting 1
 
 ```
-<greeting>        ::= <protocol_ver> <capabilities>
-<protocol_ver>    ::= 0x01          ; protocol version 1
-<capabilities>    ::= <cap_len> <cap_data>
-<cap_len>         ::= <u8>
-<cap_data>        ::= <cap>*        ; <cap_len> entries
-<cap>             ::= <cap0>        ; currently only one capability is defined
-<cap0>            ::= 0x00          ; get/put/remove/stop operations
+<greeting_1>      ::= 0x01 <capabilities>
+```
+
+#### Greeting 2
+
+```
+<greeting_2>      ::= 0x02 <capabilities> <server_identity> <diagnostics>
+<server_identity> ::= <msg>              ; software name and version, etc.
+<diagnostics>     ::= <diag_num> <diag>* ; <diag_num> diagnostics messages
+<diag_num>        ::= <u8>
+<diag>            ::= <msg>              ; message to be logged by the client
 ```
 
 ### Requests (client to server with response from server)

diff --git a/src/ccache/storage/remote/client.cpp b/src/ccache/storage/remote/client.cpp
index a478ffcc9197b3b8c59ce36fd5bfe94590442052..8fb8d90e9ea156100a496e714552a10e56647c55 100644 (file)
--- a/src/ccache/storage/remote/client.cpp
+++ b/src/ccache/storage/remote/client.cpp
@@ -95,9 +95,21 @@ Client::connect(const std::string& path)
 }
 
 uint8_t
-Client::protocol_version() const
+Client::greeting_format() const
 {
-  return m_protocol_version;
+  return m_greeting_format;
+}
+
+const std::string&
+Client::server_identity() const
+{
+  return m_server_identity;
+}
+
+const std::vector<std::string>&
+Client::diagnostics() const
+{
+  return m_diagnostics;
 }
 
 const std::vector<Client::Capability>&
@@ -212,19 +224,22 @@ Client::close()
   if (m_connected) {
     m_channel.close();
     m_connected = false;
-    m_protocol_version = 0;
+    m_greeting_format = 0;
     m_capabilities.clear();
+    m_server_identity.clear();
+    m_diagnostics.clear();
   }
 }
 
 tl::expected<void, Client::Error>
 Client::read_greeting()
 {
-  TRY_ASSIGN(m_protocol_version, receive_u8());
-  if (m_protocol_version != k_protocol_version) {
+  TRY_ASSIGN(m_greeting_format, receive_u8());
+  if (m_greeting_format != k_greeting_format_1
+      && m_greeting_format != k_greeting_format_2) {
     return tl::unexpected(
       Error(Failure::error,
-            FMT("Unsupported protocol version: {}", m_protocol_version)));
+            FMT("Unsupported greeting format: {}", m_greeting_format)));
   }
 
   TRY_ASSIGN(uint8_t cap_len, receive_u8());
@@ -235,6 +250,23 @@ Client::read_greeting()
     m_capabilities.push_back(static_cast<Capability>(cap_byte));
   }
 
+  if (m_greeting_format == k_greeting_format_2) {
+    TRY_ASSIGN(uint8_t identity_len, receive_u8());
+    TRY_ASSIGN(auto identity_bytes, receive_bytes(identity_len));
+    m_server_identity.assign(
+      reinterpret_cast<const char*>(identity_bytes.data()), identity_len);
+
+    TRY_ASSIGN(uint8_t diag_num, receive_u8());
+    m_diagnostics.clear();
+    m_diagnostics.reserve(diag_num);
+    for (uint8_t i = 0; i < diag_num; ++i) {
+      TRY_ASSIGN(uint8_t msg_len, receive_u8());
+      TRY_ASSIGN(auto msg_bytes, receive_bytes(msg_len));
+      m_diagnostics.emplace_back(
+        reinterpret_cast<const char*>(msg_bytes.data()), msg_len);
+    }
+  }
+
   return {};
 }
 

diff --git a/src/ccache/storage/remote/client.hpp b/src/ccache/storage/remote/client.hpp
index bb3359e260ed1137c62d0ad7a53228c63d04d9c8..5935067c400e3f971aa0073d4629ce52fe1de414 100644 (file)
--- a/src/ccache/storage/remote/client.hpp
+++ b/src/ccache/storage/remote/client.hpp
@@ -45,7 +45,9 @@ namespace storage::remote {
 class Client : util::NonCopyable
 {
 public:
-  static constexpr uint8_t k_protocol_version = 0x01;
+  static constexpr uint8_t k_greeting_format_1 = 0x01;
+  static constexpr uint8_t k_greeting_format_2 = 0x02;
+  static constexpr uint8_t k_max_greeting_format = k_greeting_format_2;
 
   enum class Capability : uint8_t {
     get_put_remove_stop = 0x00, // get/put/remove/stop operations
@@ -84,8 +86,10 @@ public:
   ~Client();
 
   tl::expected<void, Error> connect(const std::string& path);
-  uint8_t protocol_version() const;
+  uint8_t greeting_format() const;
   const std::vector<Capability>& capabilities() const;
+  const std::string& server_identity() const;
+  const std::vector<std::string>& diagnostics() const;
   bool has_capability(Capability cap) const;
 
   tl::expected<std::optional<util::Bytes>, Error>
@@ -104,8 +108,10 @@ private:
 #else
   util::BufferedIpcChannelClient<util::UnixSocketClient> m_channel;
 #endif
-  uint8_t m_protocol_version = 0;
+  uint8_t m_greeting_format;
   std::vector<Capability> m_capabilities;
+  std::string m_server_identity;
+  std::vector<std::string> m_diagnostics;
   bool m_connected = false;
   std::chrono::milliseconds m_data_timeout;
   std::chrono::milliseconds m_request_timeout;
@@ -123,4 +129,14 @@ private:
   tl::expected<void, Error> receive_response_void();
 };
 
+inline std::string
+to_string(Client::Capability capability)
+{
+  switch (capability) {
+  case Client::Capability::get_put_remove_stop:
+    return "get_put_remove_stop";
+  }
+  return FMT("{}", static_cast<int>(capability));
+}
+
 } // namespace storage::remote

diff --git a/src/ccache/storage/remote/helper.cpp b/src/ccache/storage/remote/helper.cpp
index f33f0edce5cc3baa3f85fd28f6ca5d37dbc324a3..01bcd020d22fcfce011e765ab72f5d668570ca53 100644 (file)
--- a/src/ccache/storage/remote/helper.cpp
+++ b/src/ccache/storage/remote/helper.cpp
@@ -76,7 +76,8 @@ constexpr std::string_view k_named_pipe_prefix = "\\\\.\\pipe\\";
 std::string
 generate_endpoint_name(
   const Url& url,
-  const std::vector<RemoteStorage::Backend::Attribute>& attributes)
+  const std::vector<RemoteStorage::Backend::Attribute>& attributes,
+  uint8_t max_greeting_format)
 {
   static const uint8_t delimiter[1] = {0};
 
@@ -98,6 +99,8 @@ generate_endpoint_name(
     hash.hash(delimiter);
     hash.hash(attr.value);
   }
+  hash.hash(delimiter);
+  hash.hash(static_cast<int64_t>(max_greeting_format));
   return FMT("storage-{}-{}", url.scheme(), util::format_base16(hash.digest()));
 }
 
@@ -164,6 +167,8 @@ build_helper_env(
   env_vars.emplace_back(FMT("CRSH_URL={}", url.str()));
   env_vars.emplace_back(
     FMT("CRSH_IDLE_TIMEOUT={}", idle_timeout.count() / 1000));
+  env_vars.emplace_back(
+    FMT("CRSH_FORMAT_MAX={}", Client::k_max_greeting_format));
   env_vars.emplace_back(FMT("CRSH_NUM_ATTR={}", attributes.size()));
 
   for (size_t i = 0; i < attributes.size(); ++i) {
@@ -183,8 +188,8 @@ is_ccache_crsh_var(std::string_view entry)
   }
 
   return name == "CRSH_IPC_ENDPOINT" || name == "CRSH_URL"
-         || name == "CRSH_IDLE_TIMEOUT" || name == "CRSH_NUM_ATTR"
-         || name.starts_with("CRSH_ATTR_KEY_")
+         || name == "CRSH_IDLE_TIMEOUT" || name == "CRSH_FORMAT_MAX"
+         || name == "CRSH_NUM_ATTR" || name.starts_with("CRSH_ATTR_KEY_")
          || name.starts_with("CRSH_ATTR_VALUE_");
 }
 
@@ -420,7 +425,8 @@ HelperBackend::HelperBackend(const fs::path& helper_path,
     // No m_endpoint_lock_path needed since we won't spawn a helper.
   } else {
     // The common case:
-    auto endpoint_name = generate_endpoint_name(url, attributes);
+    auto endpoint_name =
+      generate_endpoint_name(url, attributes, Client::k_max_greeting_format);
 #ifdef _WIN32
     m_endpoint = FMT("{}ccache-{}", k_named_pipe_prefix, endpoint_name);
     m_endpoint_lock_path = FMT("{}/{}", temp_dir, endpoint_name);
@@ -439,15 +445,21 @@ HelperBackend::HelperBackend(const fs::path& helper_path,
 tl::expected<void, RemoteStorage::Backend::Failure>
 HelperBackend::finalize_connection()
 {
-  if (m_client.protocol_version() != Client::k_protocol_version) {
-    LOG("Unexpected remote storage helper protocol version: {} (!= {})",
-        m_client.protocol_version(),
-        Client::k_protocol_version);
-    return tl::unexpected(Failure::error);
+  if (util::logging::enabled()) {
+    auto capabilities = util::join(m_client.capabilities(), " ");
+    LOG("Storage helper: {} (format: {}, capabilities: {})",
+        !m_client.server_identity().empty() ? m_client.server_identity()
+                                            : "[unknown]",
+        m_client.greeting_format(),
+        capabilities);
+    for (const auto& msg : m_client.diagnostics()) {
+      LOG("Storage helper diagnostic: {}", msg);
+    }
   }
 
   if (!m_client.has_capability(Client::Capability::get_put_remove_stop)) {
-    LOG("Remote storage helper does not support capability 0");
+    LOG("Storage helper does not support capability {}",
+        static_cast<int>(Client::Capability::get_put_remove_stop));
     return tl::unexpected(Failure::error);
   }
 

diff --git a/test/storage/client/main.cpp b/test/storage/client/main.cpp
index 633d6676aa079bb45c8e7684a6770acffcffc5c7..ca5cb6bc44bf88b7bbb75a4c822c7354ca3c6b7e 100644 (file)
--- a/test/storage/client/main.cpp
+++ b/test/storage/client/main.cpp
@@ -282,13 +282,6 @@ main(int argc, char* argv[])
     return 1;
   }
 
-  if (client.protocol_version()
-      != storage::remote::Client::k_protocol_version) {
-    PRINT(
-      stderr, "Unsupported protocol version: {}\n", client.protocol_version());
-    return 1;
-  }
-
   if (!client.has_capability(
         storage::remote::Client::Capability::get_put_remove_stop)) {
     PRINT(stderr, "Helper does not support get/put/remove/stop operations\n");

diff --git a/test/storage/helper/main.cpp b/test/storage/helper/main.cpp
index 46c08c3a0e958f2b2dee88ff9a2a41ac1eea7215..011e880b505704677e984c9219fc2f5d4052a22d 100644 (file)
--- a/test/storage/helper/main.cpp
+++ b/test/storage/helper/main.cpp
@@ -56,7 +56,6 @@
 
 namespace fs = util::filesystem;
 
-constexpr uint8_t PROTOCOL_VERSION = 0x01;
 constexpr uint8_t CAP_GET_PUT_REMOVE_STOP = 0x00;
 
 constexpr uint8_t STATUS_OK = 0x00;
@@ -105,10 +104,13 @@ fail(const std::string& message)
 class IpcServer
 {
 public:
-  IpcServer(const std::string& endpoint, std::chrono::seconds idle_timeout)
+  IpcServer(const std::string& endpoint,
+            std::chrono::seconds idle_timeout,
+            uint8_t greeting_format)
     : m_endpoint(endpoint),
       m_idle_timeout(idle_timeout),
-      m_last_activity(util::now())
+      m_last_activity(util::now()),
+      m_greeting_format(greeting_format)
   {
   }
 
@@ -118,6 +120,7 @@ private:
   std::string m_endpoint;
   std::chrono::seconds m_idle_timeout;
   util::TimePoint m_last_activity;
+  uint8_t m_greeting_format;
   std::unordered_map<std::string, std::vector<uint8_t>> m_storage;
 
   bool recv_exact(ConnHandle conn, uint8_t* buf, size_t count);
@@ -355,8 +358,21 @@ IpcServer::handle_stop(ConnHandle conn)
 void
 IpcServer::handle_client(ConnHandle conn)
 {
-  uint8_t greeting[3] = {PROTOCOL_VERSION, 1, CAP_GET_PUT_REMOVE_STOP};
-  send_data(conn, greeting, sizeof(greeting));
+  if (m_greeting_format == 2) {
+    const std::string_view identity = "ccache-storage-test";
+    std::vector<uint8_t> greeting;
+    greeting.push_back(0x02); // greeting_2
+    greeting.push_back(1);    // 1 capability
+    greeting.push_back(CAP_GET_PUT_REMOVE_STOP);
+    greeting.push_back(static_cast<uint8_t>(identity.size()));
+    auto id_data = reinterpret_cast<const uint8_t*>(identity.data());
+    greeting.insert(greeting.end(), id_data, id_data + identity.size());
+    greeting.push_back(0); // 0 diagnostics
+    send_data(conn, greeting.data(), greeting.size());
+  } else {
+    const uint8_t greeting[] = {0x01, 1, CAP_GET_PUT_REMOVE_STOP};
+    send_data(conn, greeting, sizeof(greeting));
+  }
 
   while (true) {
     uint8_t request_type;
@@ -569,12 +585,25 @@ main()
     idle_timeout = *value;
   }
 
+  uint8_t greeting_format = 1;
+  const char* format_max_env = std::getenv("CRSH_FORMAT_MAX");
+  if (format_max_env) {
+    auto client_format_max = util::parse_unsigned(format_max_env, 1, UINT8_MAX);
+    if (!client_format_max) {
+      fail(FMT("Invalid CRSH_FORMAT_MAX: {}", client_format_max.error()));
+    }
+    constexpr uint8_t server_format_max = 2;
+    greeting_format = std::min<uint8_t>(*client_format_max, server_format_max);
+  }
+
   log_msg("Starting");
   log_msg(FMT("IPC endpoint: {}", endpoint));
   log_msg(FMT("URL: {}", url));
   log_msg(FMT("Idle timeout: {}", idle_timeout));
+  log_msg(FMT("Greeting format: {}", greeting_format));
 
-  IpcServer helper(endpoint, std::chrono::seconds{idle_timeout});
+  IpcServer helper(
+    endpoint, std::chrono::seconds{idle_timeout}, greeting_format);
   helper.run();
 
   log_msg("Shutdown complete");