Fix bugs in remote config

Author: cataphract

src/datadog/remote_config/remote_config.cpp

@@ -154,12 +154,12 @@ nlohmann::json Manager::make_request_payload() {
       nlohmann::json cached_file = {
           {"path", config.path},
           {"length", config.content.size()},
-          {"hashes", {{"algorithm", "sha256"}, {"hash", config.hash}}}};
+          {"hashes", {{{"algorithm", "sha256"}, {"hash", config.hash}}}}};
 
       cached_target_files.emplace_back(std::move(cached_file));
     }
 
-    j["cached_target"] = cached_target_files;
+    j["cached_target_files"] = cached_target_files;
     j["client"]["state"]["config_states"] = config_states;
   }
 
@@ -168,12 +168,26 @@ nlohmann::json Manager::make_request_payload() {
 
 void Manager::process_response(const nlohmann::json& json) {
   state_.error_message = nullopt;
+  struct UpdateTargetsVersion {
+    decltype(state_)& state;
+    std::uint64_t new_targets_version;
+
+    ~UpdateTargetsVersion() noexcept {
+      // if there was a global error, the old targets_version should be sent in
+      // the next request (at least this is what the system-tests expect)
+      if ((!state.error_message || state.error_message->empty()) &&
+          new_targets_version != std::uint64_t(-1)) {
+        state.targets_version = new_targets_version;
+      }
+    }
+  } update_targets_version{state_, std::uint64_t(-1)};
 
   try {
     const auto targets = nlohmann::json::parse(
         base64_decode(json.at("targets").get<StringView>()));
 
-    state_.targets_version = targets.at("/signed/version"_json_pointer);
+    update_targets_version.new_targets_version =
+        targets.at("/signed/version"_json_pointer).get<std::uint64_t>();
     state_.opaque_backend_state =
         targets.at("/signed/custom/opaque_backend_state"_json_pointer);
 

test/remote_config/test_remote_config.cpp

@@ -1,3 +1,5 @@
+#include <algorithm>
+
 #include "catch.hpp"
 #include "datadog/json.hpp"
 #include "datadog/remote_config/remote_config.h"
@@ -184,10 +186,13 @@ REMOTE_CONFIG_TEST("response processing") {
 
     rc.process_response(response_json);
 
-    // Next payload should contains an error.
+    // Next payload should contain an error.
     const auto payload = rc.make_request_payload();
     CHECK(payload.contains("/client/state/has_error"_json_pointer) == true);
     CHECK(payload.contains("/client/state/error"_json_pointer) == true);
+
+    // targets_version should not have been updated.
+    CHECK(payload["client"]["state"]["targets_version"] == 0);
   }
 
   SECTION("update dispatch") {
@@ -270,6 +275,36 @@ REMOTE_CONFIG_TEST("response processing") {
       }
     }
 
+    SECTION("cached_target_files is correctly populated") {
+      auto payload = rc.make_request_payload();
+
+      auto cached_target_files = payload.find("cached_target_files");
+      REQUIRE(cached_target_files != payload.end());
+
+      REQUIRE(cached_target_files->is_array());
+      REQUIRE(cached_target_files->size() == 3);
+
+      std::sort(cached_target_files->begin(), cached_target_files->end(),
+                [](const auto& a, const auto& b) {
+                  return a.at("path").template get<std::string_view>() <
+                         b.at("path").template get<std::string_view>();
+                });
+
+      const auto ctf = cached_target_files->at(0);
+      REQUIRE(ctf.at("path").get<std::string_view>() ==
+              "employee/AGENT_CONFIG/test_rc_update/flare_conf");
+      REQUIRE(ctf.at("length").get<std::uint64_t>() == 381UL);
+
+      auto hashes = ctf.at("hashes");
+
+      REQUIRE(hashes.is_array());
+      REQUIRE(hashes.size() == 1);
+
+      const auto h = hashes.at(0);
+      REQUIRE(h.at("algorithm").get<std::string_view>() == "sha256");
+      REQUIRE(h.at("hash").get<std::string_view>().size() == 64U);
+    }
+
     SECTION("same config update should not trigger listeners") {
       rc.process_response(response_json);
       CHECK(tracing_listener->count_on_update == 1);