Skip to content

Commit ae7df2b

Browse files
authored
CCDB: improved support for CI Security Proxy (#15713)
Add ability to specify Gate Token also for DELETE / PUT / UPDATE operations.
1 parent 039b162 commit ae7df2b

2 files changed

Lines changed: 47 additions & 5 deletions

File tree

CCDB/src/CcdbApi.cxx

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1300,16 +1300,21 @@ void CcdbApi::deleteObject(std::string const& path, long timestamp) const
13001300
{
13011301
CURL* curl;
13021302
CURLcode res;
1303-
stringstream fullUrl;
13041303
long timestampLocal = timestamp == -1 ? getCurrentTimestamp() : timestamp;
13051304

13061305
curl = curl_easy_init();
13071306
if (curl != nullptr) {
13081307
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "DELETE");
13091308
curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
1309+
// A DELETE is a write, so it needs the gate token as storing does.
1310+
struct curl_slist* list = appendGateToken(nullptr);
1311+
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
13101312
curlSetSSLOptions(curl);
13111313

13121314
for (size_t hostIndex = 0; hostIndex < hostsPool.size(); hostIndex++) {
1315+
// Inside the loop: hoisted out, the stream accumulates and the second
1316+
// host's URL is the first with the second appended.
1317+
stringstream fullUrl;
13131318
fullUrl << getHostUrl(hostIndex) << "/" << path << "/" << timestampLocal;
13141319
curl_easy_setopt(curl, CURLOPT_URL, fullUrl.str().c_str());
13151320

@@ -1318,17 +1323,23 @@ void CcdbApi::deleteObject(std::string const& path, long timestamp) const
13181323
if (res != CURLE_OK) {
13191324
LOGP(alarm, "CURL_perform() failed: {}", curl_easy_strerror(res));
13201325
}
1321-
curl_easy_cleanup(curl);
13221326
}
1327+
// After the loop, not inside it: cleaning up per host left every later
1328+
// iteration using a freed handle.
1329+
curl_easy_cleanup(curl);
1330+
curl_slist_free_all(list);
13231331
}
13241332
}
13251333

13261334
void CcdbApi::truncate(std::string const& path) const
13271335
{
13281336
CURL* curl;
13291337
CURLcode res;
1330-
stringstream fullUrl;
13311338
for (size_t i = 0; i < hostsPool.size(); i++) {
1339+
// Declared inside the loop: a stringstream hoisted out of it accumulates,
1340+
// so the second host's URL would be the first one with the second appended
1341+
// to it. Latent until now -- every caller used a single-host pool.
1342+
stringstream fullUrl;
13321343
std::string url = getHostUrl(i);
13331344
fullUrl << url << "/truncate/" << path;
13341345

@@ -1337,6 +1348,13 @@ void CcdbApi::truncate(std::string const& path) const
13371348
if (curl != nullptr) {
13381349
curl_easy_setopt(curl, CURLOPT_URL, fullUrl.str().c_str());
13391350

1351+
// Truncating is a write, so it needs the gate token exactly as storing
1352+
// does. This was the one write path left without it, which a broker
1353+
// answers 401 -- failing every CCDB suite in their teardown, since each
1354+
// one truncates the path it just wrote.
1355+
struct curl_slist* list = appendGateToken(nullptr);
1356+
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
1357+
13401358
curlSetSSLOptions(curl);
13411359

13421360
// Perform the request, res will get the return code
@@ -1345,6 +1363,7 @@ void CcdbApi::truncate(std::string const& path) const
13451363
LOGP(alarm, "CURL_perform() failed: {}", curl_easy_strerror(res));
13461364
}
13471365
curl_easy_cleanup(curl);
1366+
curl_slist_free_all(list);
13481367
}
13491368
}
13501369
}
@@ -1363,6 +1382,17 @@ bool CcdbApi::isHostReachable() const
13631382
curl = curl_easy_init();
13641383
curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
13651384
if (curl) {
1385+
// NOTE: mUrl, not getHostUrl(hostIndex), even though hostIndex is unused.
1386+
// For a failover setup mUrl is the whole comma-separated list, which curl
1387+
// rejects as malformed, so every multi-host instance reports itself
1388+
// unreachable however healthy its hosts are -- and testCcdbApiMultipleUrls,
1389+
// whose cases are gated on this, is skipped rather than run.
1390+
//
1391+
// Fixing it is a separate change: the suite then runs for the first time
1392+
// and its storeAndRetrieve fails, so the multi-host store/retrieve path
1393+
// needs looking at before this can be corrected. Callers outside the tests
1394+
// are affected too -- HMPID/PedestalsCalculationSpec sets mWriteToDB from
1395+
// this, and TPC workflows branch on it.
13661396
for (size_t hostIndex = 0; hostIndex < hostsPool.size() && res != CURLE_OK; hostIndex++) {
13671397
curl_easy_setopt(curl, CURLOPT_URL, mUrl.data());
13681398
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_data);
@@ -1661,8 +1691,12 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
16611691
curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
16621692
if (curl != nullptr) {
16631693
CURLcode res;
1664-
stringstream fullUrl;
1694+
// A PUT is a write, so it needs the gate token as storing does.
1695+
struct curl_slist* list = appendGateToken(nullptr);
16651696
for (size_t hostIndex = 0; hostIndex < hostsPool.size(); hostIndex++) {
1697+
// Inside the loop: hoisted out, the stream accumulates and the second
1698+
// host's URL is the first with the second appended.
1699+
stringstream fullUrl;
16661700
fullUrl << getHostUrl(hostIndex) << "/" << path << "/" << timestamp;
16671701
if (newEOV > 0) {
16681702
fullUrl << "/" << newEOV;
@@ -1689,6 +1723,7 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
16891723
curl_easy_setopt(curl, CURLOPT_CUSTOMREQUEST, "PUT"); // make sure we use PUT
16901724
curl_easy_setopt(curl, CURLOPT_USERAGENT, mUniqueAgentID.c_str());
16911725
curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
1726+
curl_easy_setopt(curl, CURLOPT_HTTPHEADER, list);
16921727
curlSetSSLOptions(curl);
16931728

16941729
// Perform the request, res will get the return code
@@ -1699,9 +1734,12 @@ int CcdbApi::updateMetadata(std::string const& path, std::map<std::string, std::
16991734
} else {
17001735
ret = 0;
17011736
}
1702-
curl_easy_cleanup(curl);
17031737
}
17041738
}
1739+
// After the loop, not inside it: cleaning up per host left every later
1740+
// iteration using a freed handle.
1741+
curl_easy_cleanup(curl);
1742+
curl_slist_free_all(list);
17051743
}
17061744
return ret;
17071745
}

CCDB/test/testCcdbApiMultipleUrls.cxx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@ struct Fixture {
3535
Fixture()
3636
{
3737
CcdbApi api;
38+
// Deliberately NOT reading ALICEO2_CCDB_HOST like the other suites: this
39+
// one is skipped in practice, because isHostReachable() cannot report a
40+
// multi-host pool reachable (see CcdbApi::isHostReachable). Making it
41+
// configurable only matters once that is fixed and the suite runs.
3842
ccdbUrl = "https://localhost:22,https://localhost:8080,http://ccdb-test.cern.ch:8080";
3943
api.init(ccdbUrl);
4044
cout << "ccdb url: " << ccdbUrl << endl;

0 commit comments

Comments
 (0)