CCDB: improved support for CI Security Proxy

* Fix support for multiurl CCDB backends
* Add ability to specify Gate Token also for DELETE / PUT operations
* Fix hardcoded URL in test

Author: ktf

CCDB/src/CcdbApi.cxx

@@ -1300,16 +1300,21 @@ void CcdbApi::deleteObject(std::string const& path, long timestamp) const
 {
   CURL* curl;
   CURLcode res;
-  stringstream fullUrl;
   long timestampLocal = timestamp == -1 ? getCurrentTimestamp() : timestamp;
 
   curl = curl_easy_init();
   if (curl != nullptr) {
     curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "DELETE");
     curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
+    // A DELETE is a write, so it needs the gate token as storing does.
+    struct curl_slist* list = appendGateToken(nullptr);
+    curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
     curlSetSSLOptions(curl);
 
     for (size_t hostIndex = 0; hostIndex < hostsPool.size(); hostIndex++) {
+      // Inside the loop: hoisted out, the stream accumulates and the second
+      // host's URL is the first with the second appended.
+      stringstream fullUrl;
       fullUrl << getHostUrl(hostIndex) << "/" << path << "/" << timestampLocal;
       curl_easy_setopt(curl, CURLOPT_URL, fullUrl.str().c_str());
 
@@ -1318,17 +1323,23 @@ void CcdbApi::deleteObject(std::string const& path, long timestamp) const
       if (res != CURLE_OK) {
         LOGP(alarm, "CURL_perform() failed: {}", curl_easy_strerror(res));
       }
-      curl_easy_cleanup(curl);
     }
+    // After the loop, not inside it: cleaning up per host left every later
+    // iteration using a freed handle.
+    curl_easy_cleanup(curl);
+    curl_slist_free_all(list);
   }
 }
 
 void CcdbApi::truncate(std::string const& path) const
 {
   CURL* curl;
   CURLcode res;
-  stringstream fullUrl;
   for (size_t i = 0; i < hostsPool.size(); i++) {
+    // Declared inside the loop: a stringstream hoisted out of it accumulates,
+    // so the second host's URL would be the first one with the second appended
+    // to it. Latent until now -- every caller used a single-host pool.
+    stringstream fullUrl;
     std::string url = getHostUrl(i);
     fullUrl << url << "/truncate/" << path;
 
@@ -1337,6 +1348,13 @@ void CcdbApi::truncate(std::string const& path) const
     if (curl != nullptr) {
       curl_easy_setopt(curl, CURLOPT_URL, fullUrl.str().c_str());
 
+      // Truncating is a write, so it needs the gate token exactly as storing
+      // does. This was the one write path left without it, which a broker
+      // answers 401 -- failing every CCDB suite in their teardown, since each
+      // one truncates the path it just wrote.
+      struct curl_slist* list = appendGateToken(nullptr);
+      curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
+
       curlSetSSLOptions(curl);
 
       // Perform the request, res will get the return code
@@ -1345,6 +1363,7 @@ void CcdbApi::truncate(std::string const& path) const
         LOGP(alarm, "CURL_perform() failed: {}", curl_easy_strerror(res));
       }
       curl_easy_cleanup(curl);
+      curl_slist_free_all(list);
     }
   }
 }
@@ -1363,8 +1382,15 @@ bool CcdbApi::isHostReachable() const
   curl = curl_easy_init();
   curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
   if (curl) {
+    // Each host in turn, not mUrl: mUrl holds whatever was passed to init(),
+    // which for a failover setup is the whole comma-separated list. curl
+    // rejects that as malformed (CURLE_URL_MALFORMAT), so every multi-host
+    // instance reported itself unreachable no matter how healthy its hosts
+    // were -- and testCcdbApiMultipleUrls, whose cases are gated on this,
+    // silently skipped instead of running.
     for (size_t hostIndex = 0; hostIndex < hostsPool.size() && res != CURLE_OK; hostIndex++) {
-      curl_easy_setopt(curl, CURLOPT_URL, mUrl.data());
+      std::string url = getHostUrl(hostIndex);
+      curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
       curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_data);
       curlSetSSLOptions(curl);
       res = CURL_perform(curl);
@@ -1661,8 +1687,12 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
   curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
   if (curl != nullptr) {
     CURLcode res;
-    stringstream fullUrl;
+    // A PUT is a write, so it needs the gate token as storing does.
+    struct curl_slist* list = appendGateToken(nullptr);
     for (size_t hostIndex = 0; hostIndex < hostsPool.size(); hostIndex++) {
+      // Inside the loop: hoisted out, the stream accumulates and the second
+      // host's URL is the first with the second appended.
+      stringstream fullUrl;
       fullUrl << getHostUrl(hostIndex) << "/" << path << "/" << timestamp;
       if (newEOV > 0) {
         fullUrl << "/" << newEOV;
@@ -1689,6 +1719,7 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
         curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "PUT"); // make sure we use PUT
         curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
         curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
+        curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
         curlSetSSLOptions(curl);
 
         // Perform the request, res will get the return code
@@ -1699,9 +1730,12 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
         } else {
           ret = 0;
         }
-        curl_easy_cleanup(curl);
       }
     }
+    // After the loop, not inside it: cleaning up per host left every later
+    // iteration using a freed handle.
+    curl_easy_cleanup(curl);
+    curl_slist_free_all(list);
   }
   return ret;
 }

CCDB/test/testCcdbApiMultipleUrls.cxx

@@ -17,6 +17,7 @@
 #include "CCDB/CCDBTimeStampUtils.h"
 #include <boost/test/unit_test.hpp>
 #include <cstdio>
+#include <cstdlib>
 #include <TH1F.h>
 
 using namespace std;
@@ -35,7 +36,13 @@ struct Fixture {
   Fixture()
   {
     CcdbApi api;
-    ccdbUrl = "https://localhost:22,https://localhost:8080,http://ccdb-test.cern.ch:8080";
+    // This suite uploads, so it needs a WRITABLE instance -- ccdb-test by
+    // default, not the official CCDB. Only the LAST entry is configurable:
+    // what is under test here is the failover, so the two dead endpoints in
+    // front of it are part of the fixture and stay hardcoded.
+    const char* host = std::getenv("ALICEO2_CCDB_HOST");
+    ccdbUrl = "https://localhost:22,https://localhost:8080,";
+    ccdbUrl += host ? host : "http://ccdb-test.cern.ch:8080";
     api.init(ccdbUrl);
     cout << "ccdb url: " << ccdbUrl << endl;
     hostReachable = api.isHostReachable();