From 42782ff176e5c3d8f59cfcf5e19ace1d40e924eb Mon Sep 17 00:00:00 2001 From: Gavin Halliday Date: Wed, 22 Nov 2023 12:21:56 +0000 Subject: [PATCH] HPCC-30299 Update secrets in the background to avoid roxie stalls Signed-off-by: Gavin Halliday --- helm/hpcc/values.schema.json | 4 + roxie/ccd/ccdmain.cpp | 3 + system/jlib/jsecrets.cpp | 284 ++++++++++++++++++++++++++------ system/jlib/jsecrets.hpp | 13 +- testing/unittests/jlibtests.cpp | 151 +++++++++++++++-- 5 files changed, 391 insertions(+), 64 deletions(-) diff --git a/helm/hpcc/values.schema.json b/helm/hpcc/values.schema.json index d73687e2039..0bb896e12d4 100644 --- a/helm/hpcc/values.schema.json +++ b/helm/hpcc/values.schema.json @@ -2322,6 +2322,10 @@ "default": false, "description": "Require SOAPCALL and HTTPCALL URLs are secrets or mapped to secrets" }, + "updateSecretsInBackground": { + "type": "boolean", + "description": "Preemptively update secrets that are active and about to expire in the background" + }, "expert": { "description": "Custom internal options usually reserved for internal testing", "type": "object" diff --git a/roxie/ccd/ccdmain.cpp b/roxie/ccd/ccdmain.cpp index 159d6f99642..0b0358b6c90 100644 --- a/roxie/ccd/ccdmain.cpp +++ b/roxie/ccd/ccdmain.cpp @@ -945,6 +945,8 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml) #endif roxieMetrics.setown(createRoxieMetricsManager()); + if (topology->getPropBool("@updateSecretsInBackground", !runOnce)) + startSecretUpdateThread(0); Owned userMetrics = topology->getElements("./UserMetric"); ForEach(*userMetrics) @@ -1686,6 +1688,7 @@ int CCD_API roxie_main(int argc, const char *argv[], const char * defaultYaml) E->Release(); } + stopSecretUpdateThread(); roxieMetrics.clear(); #ifndef _CONTAINERIZED stopPerformanceMonitor(); diff --git a/system/jlib/jsecrets.cpp b/system/jlib/jsecrets.cpp index 322c6c12f73..6f71f983f59 100644 --- a/system/jlib/jsecrets.cpp +++ b/system/jlib/jsecrets.cpp @@ -24,6 +24,7 @@ #include "jptree.hpp" #include "jerror.hpp" #include "jsecrets.hpp" +#include "jthread.hpp" //including cpp-httplib single header file REST client // doesn't work with format-nonliteral as an error @@ -368,8 +369,52 @@ static bool isEmptyTimeval(const timeval &tv) //--------------------------------------------------------------------------------------------------------------------- +//The secret key has the form category/name[@vaultId][#version] + +static std::string buildSecretKey(const char * category, const char * name, const char * optVaultId, const char * optVersion) +{ + std::string key; + key.append(category).append("/").append(name); + if (optVaultId) + key.append("@").append(optVaultId); + if (optVersion) + key.append("#").append(optVersion); + return key; +} + +static void expandSecretKey(std::string & category, std::string & name, std::string & optVaultId, std::string & optVersion, const char * key) +{ + const char * slash = strchr(key, '/'); + assertex(slash); + const char * at = strchr(slash, '@'); + const char * hash = strchr(slash, '#'); + + const char * end = nullptr; + if (hash) + { + optVersion.assign(hash+1); + end = hash; + } + if (at) + { + if (end) + optVaultId.assign(at+1, end-at-1); + else + optVaultId.assign(at+1); + end = at; + } + if (end) + name.assign(slash+1, end-slash-1); + else + name.assign(slash+1); + category.assign(key, slash-key); +} + +//--------------------------------------------------------------------------------------------------------------------- + //Represents an entry in the secret cache. Once created it is always used for the secret. using cache_timestamp = unsigned; +using cache_timestamp_diff = signed; class SecretCacheEntry : public CInterface { friend class SecretCache; @@ -377,8 +422,8 @@ class SecretCacheEntry : public CInterface public: //A cache entry is initally created that has a create and access time of now, but the checkTimestamp //is set so that needsRefresh() will return true. - SecretCacheEntry(cache_timestamp _now) - : contentTimestamp(_now), accessedTimestamp(_now), checkedTimestamp(_now - 2 * secretTimeoutMs) + SecretCacheEntry(cache_timestamp _now, const char * _secretKey) + : secretKey(_secretKey), contentTimestamp(_now), accessedTimestamp(_now), checkedTimestamp(_now - 2 * secretTimeoutMs) { } @@ -387,6 +432,11 @@ class SecretCacheEntry : public CInterface return contentHash; } + void getSecretOptions(std::string & category, std::string & name, std::string & optVaultId, std::string & optVersion) + { + expandSecretKey(category, name, optVaultId, optVersion, secretKey.c_str()); + } + // We should never replace known contents for unknown contents // so once this returns true it should always return true bool hasContents() const @@ -394,6 +444,12 @@ class SecretCacheEntry : public CInterface return contents != nullptr; } + //Has the secret value been used since it was last checked for an update? + bool isActive() const + { + return (cache_timestamp_diff)(accessedTimestamp - checkedTimestamp) >= 0; + } + //Is the secret potentially out of date? bool isStale() const { @@ -409,28 +465,33 @@ class SecretCacheEntry : public CInterface return (elapsed > secretTimeoutMs); } - bool needsRefresh() const + void noteAccessed(cache_timestamp now) { - return needsRefresh(msTick()); + accessedTimestamp = now; } - void noteFailedUpdate(cache_timestamp now) + void noteFailedUpdate(cache_timestamp now, bool accessed) { //Update the checked timestamp - so that we do not continually check for updates to secrets which //are stale because the vault or other source of values in inaccessible. //Keep using the last good value checkedTimestamp = now; + if (accessed) + accessedTimestamp = now; } + const char * queryTraceName() const { return secretKey.c_str(); } + //The following functions can only be called from member functions of SecretCache private: - void updateContents(IPropertyTree * _contents, cache_timestamp now) + void updateContents(IPropertyTree * _contents, cache_timestamp now, bool accessed) { contents.set(_contents); updateHash(); contentTimestamp = now; - accessedTimestamp = now; checkedTimestamp = now; + if (accessed) + accessedTimestamp = now; } void updateHash() @@ -441,6 +502,7 @@ class SecretCacheEntry : public CInterface contentHash = 0; } private: + const std::string secretKey; // Duplicate of the key used to find this entry in the cache Linked contents;// Can only be accessed when SecretCache::cs is held cache_timestamp contentTimestamp = 0; // When was this secret read from disk/vault cache_timestamp accessedTimestamp = 0; // When was this secret last accessed? @@ -448,6 +510,7 @@ class SecretCacheEntry : public CInterface unsigned contentHash = 0; }; + // A cache of (secret[:version] to a secret cache entry) // Once a hash table entry has been created for a secret it is never removed and the associated // value is never replaced. This means it is safe to keep a pointer to the entry in another class. @@ -462,7 +525,7 @@ class SecretCache } //Check to see if a secret exists, and if not add a null entry that has expired. - SecretCacheEntry * resolveSecret(const std::string & secretKey, cache_timestamp now) + SecretCacheEntry * getSecret(const std::string & secretKey, cache_timestamp now) { SecretCacheEntry * result; CriticalBlock block(cs); @@ -475,16 +538,28 @@ class SecretCache else { //Insert an entry with a null value that is marked as out of date - result = new SecretCacheEntry(now); + result = new SecretCacheEntry(now, secretKey.c_str()); secrets.emplace(secretKey, result); } return result; } - void updateSecret(SecretCacheEntry * match, IPropertyTree * value, cache_timestamp now) + + void gatherPendingRefresh(std::vector & pending, cache_timestamp when) + { + CriticalBlock block(cs); + for (auto & entry : secrets) + { + SecretCacheEntry * secret = entry.second.get(); + if (secret->isActive() && secret->needsRefresh(when)) + pending.push_back(secret); + } + } + + void updateSecret(SecretCacheEntry * match, IPropertyTree * value, cache_timestamp now, bool accessed) { CriticalBlock block(cs); - match->updateContents(value, now); + match->updateContents(value, now, accessed); } private: @@ -805,7 +880,7 @@ class CVault else if (authType == VaultAuthType::clientcert) clientCertLogin(permissionDenied); else if (permissionDenied && authType == VaultAuthType::token) - vaultAuthError("token permission denied"); //don't permenently invalidate token. Try again next time because it could be permissions for a particular secret rather than invalid token + vaultAuthError("token permission denied"); //don't permanently invalidate token. Try again next time because it could be permissions for a particular secret rather than invalid token if (clientToken.isEmpty()) vaultAuthError("no vault access token"); } @@ -1061,46 +1136,64 @@ static IPropertyTree *resolveVaultSecret(const char *category, const char * name return createPTreeFromVaultSecret(json.str(), kind); } - -static SecretCacheEntry * getSecretEntry(const char *category, const char * name, const char * optVaultId, const char * optVersion) +static IPropertyTree * resolveSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion) { - cache_timestamp now = msTick(); - - std::string key; - key.append(category).append("/").append(name); - if (optVaultId) - key.append("@").append(optVaultId); - if (optVersion) - key.append("#").append(optVersion); - - SecretCacheEntry * match = globalSecretCache.resolveSecret(key, now); - if (!match->needsRefresh(now)) - return match; - - Owned resolved; if (!isEmptyString(optVaultId)) { if (strieq(optVaultId, "k8s")) - resolved.setown(resolveLocalSecret(category, name)); + return resolveLocalSecret(category, name); else - resolved.setown(resolveVaultSecret(category, name, optVaultId, optVersion)); + return resolveVaultSecret(category, name, optVaultId, optVersion); } else { - resolved.setown(resolveLocalSecret(category, name)); + Owned resolved(resolveLocalSecret(category, name)); if (!resolved) resolved.setown(resolveVaultSecret(category, name, nullptr, optVersion)); + return resolved.getClear(); } +} + +static SecretCacheEntry * getSecretEntry(const char * category, const char * name, const char * optVaultId, const char * optVersion) +{ + cache_timestamp now = msTick(); + + std::string key(buildSecretKey(category, name, optVaultId, optVersion)); + + SecretCacheEntry * match = globalSecretCache.getSecret(key, now); + if (!match->needsRefresh(now)) + return match; + + Owned resolved(resolveSecret(category, name, optVaultId, optVersion)); //If the secret could no longer be resolved (e.g. a vault has gone down) then keep the old one if (resolved) - globalSecretCache.updateSecret(match, resolved, now); + globalSecretCache.updateSecret(match, resolved, now, true); else - match->noteFailedUpdate(now); + match->noteFailedUpdate(now, true); return match; } +static void refreshSecret(SecretCacheEntry * secret, bool accessed) +{ + cache_timestamp now = msTick(); + + std::string category; + std::string name; + std::string optVaultId; + std::string optVersion; + secret->getSecretOptions(category, name, optVaultId, optVersion); + + Owned resolved(resolveSecret(category.c_str(), name.c_str(), optVaultId.c_str(), optVersion.c_str())); + + //If the secret could no longer be resolved (e.g. a vault has gone down) then keep the old one + if (resolved) + globalSecretCache.updateSecret(secret, resolved, now, accessed); + else + secret->noteFailedUpdate(now, accessed); +} + static const IPropertyTree *getSecretTree(const char *category, const char * name, const char * optVaultId, const char * optVersion) { SecretCacheEntry * secret = getSecretEntry(category, name, optVaultId, optVersion); @@ -1153,8 +1246,8 @@ extern jlib_decl bool getSecretValue(StringBuffer & result, const char *category class CSecret final : public CInterfaceOf { public: - CSecret(const char *_category, const char * _name, const char * _vaultId, const char * _version, SecretCacheEntry * _secret) - : category(_category), name(_name), vaultId(_vaultId), version(_version), secret(_secret) + CSecret(SecretCacheEntry * _secret) + : secret(_secret) { } @@ -1193,10 +1286,6 @@ class CSecret final : public CInterfaceOf void checkUptoDate() const; protected: - StringAttr category; - StringAttr name; - StringAttr vaultId; - StringAttr version; mutable CriticalSection secretCs; mutable SecretCacheEntry * secret; }; @@ -1211,35 +1300,117 @@ const IPropertyTree * CSecret::getTree() const void CSecret::checkUptoDate() const { - if (secret->needsRefresh()) + cycle_t now = msTick(); + if (secret->needsRefresh(now)) { #ifdef TRACE_SECRETS - DBGLOG("Secret %s/%s is stale updating from %u...", category.str(), name.str(), secretHash); + DBGLOG("Secret %s is stale updating from %u...", secret->queryTraceName(), secretHash); #endif //MORE: This could block or fail - in roxie especially it would be better to return the old value try { - SecretCacheEntry * newSecret = getSecretEntry(category, name, vaultId, version); - //Check the secret is always returned consistently. It would be possible to call a slightly - //more optimal function to refresh a secret, but this is simplest. - assertex(secret == newSecret); + refreshSecret(secret, true); } catch (IException * e) { - VStringBuffer msg("Failed to update secret %s.%s", category.str(), name.str()); + VStringBuffer msg("Failed to update secret %s", secret->queryTraceName()); EXCLOG(e, msg.str()); e->Release(); } } + else + secret->noteAccessed(now); } -ISyncedPropertyTree * resolveSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion) +ISyncedPropertyTree * getSyncedSecret(const char *category, const char * name, const char * optVaultId, const char * optVersion) { validateCategoryName(category); validateSecretName(name); SecretCacheEntry * resolved = getSecretEntry(category, name, optVaultId, optVersion); - return new CSecret(category, name, optVaultId, optVersion, resolved); + return new CSecret(resolved); +} + +//--------------------------------------------------------------------------------------------------------------------- + +//Manage a background thread, that checks which of the secrets have been accessed recently and refreshes them if they +//are going to go out of date soon. + +static cache_timestamp refreshLookaheadMs = 0; +class SecretRefreshThread : public Thread +{ +public: + virtual int run() override + { + std::vector pending; + while (!abort) + { +#ifdef TRACE_SECRETS + DBGLOG("Check for expired secrets..."); +#endif + cache_timestamp now = msTick(); + globalSecretCache.gatherPendingRefresh(pending, now + refreshLookaheadMs); + for (auto secret : pending) + { +#ifdef TRACE_SECRETS + DBGLOG("Refreshing secret %s", secret->queryTraceName()); +#endif + refreshSecret(secret, false); + } + pending.clear(); + + unsigned interval = refreshLookaheadMs/4; + if (sem.wait(interval)) + break; + } + return 0; + } + + void stop() + { + abort = true; + sem.signal(); + join(); + } + +public: + std::atomic abort{false}; + Semaphore sem; +}; +static Owned refreshThread; + +void startSecretUpdateThread(unsigned lookaheadMs) +{ + if (lookaheadMs == 0) + lookaheadMs = secretTimeoutMs / 5; + if (lookaheadMs > secretTimeoutMs / 2) + lookaheadMs = secretTimeoutMs / 2; + refreshLookaheadMs = lookaheadMs; + if (!refreshThread) + { + refreshThread.setown(new SecretRefreshThread()); + refreshThread->start(); + } +} + +void stopSecretUpdateThread() +{ + if (refreshThread) + { + refreshThread->stop(); + refreshThread->join(); + refreshThread.clear(); + } +} + +MODULE_INIT(INIT_PRIORITY_SYSTEM) +{ + return true; +} +MODULE_EXIT() +{ + //This should have been called already, but for safety ensure the thread is terminated. + stopSecretUpdateThread(); } //--------------------------------------------------------------------------------------------------------------------- @@ -1423,7 +1594,7 @@ class CIssuerConfig final : public CSyncedCertificateBase CIssuerConfig(const char *_issuer, const char * _trustedPeers, bool _isClientConnection, bool _acceptSelfSigned, bool _addCACert, bool _disableMTLS) : CSyncedCertificateBase(_issuer), trustedPeers(_trustedPeers), isClientConnection(_isClientConnection), acceptSelfSigned(_acceptSelfSigned), addCACert(_addCACert), disableMTLS(_disableMTLS) { - secret.setown(resolveSecret("certificates", issuer, nullptr, nullptr)); + secret.setown(getSyncedSecret("certificates", issuer, nullptr, nullptr)); createConfig(); } @@ -1478,7 +1649,7 @@ class CCertificateConfig final : public CSyncedCertificateBase CCertificateConfig(const char * _category, const char * _secretName, bool _addCACert) : CSyncedCertificateBase(nullptr), addCACert(_addCACert) { - secret.setown(resolveSecret(_category, _secretName, nullptr, nullptr)); + secret.setown(getSyncedSecret(_category, _secretName, nullptr, nullptr)); if (!secret->isValid()) throw makeStringExceptionV(-1, "secret %s.%s not found", _category, _secretName); createConfig(); @@ -1592,3 +1763,16 @@ jlib_decl bool queryMtls() else return false; } + + +#ifdef _USE_CPPUNIT +std::string testBuildSecretKey(const char * category, const char * name, const char * optVaultId, const char * optVersion) +{ + return buildSecretKey(category, name, optVaultId, optVersion); +} + +void testExpandSecretKey(std::string & category, std::string & name, std::string & optVaultId, std::string & optVersion, const char * key) +{ + expandSecretKey(category, name, optVaultId, optVersion, key); +} +#endif diff --git a/system/jlib/jsecrets.hpp b/system/jlib/jsecrets.hpp index 2d7f1c286be..baf9462c907 100644 --- a/system/jlib/jsecrets.hpp +++ b/system/jlib/jsecrets.hpp @@ -29,10 +29,10 @@ extern jlib_decl void setSecretTimeout(unsigned timeoutMs); //Return the current (cached) value of a secret. If the secret is not defined, return nullptr. extern jlib_decl const IPropertyTree *getSecret(const char *category, const char * name, const char * optVaultId = nullptr, const char * optVersion = nullptr); -// resolveSecret() always returns an object, which will potentially be updated behind the scenes. If no secret is originally +// getSyncedSecret() always returns an object, which will potentially be updated behind the scenes. If no secret is originally // defined, but it then added to a vault or Kubernetes secret, it will then be picked up when the cache entry is // refreshed - allowing missing configuration to be updated for a live system. -extern jlib_decl ISyncedPropertyTree * resolveSecret(const char *category, const char * name, const char * optRequiredVault, const char* optVersion); +extern jlib_decl ISyncedPropertyTree * getSyncedSecret(const char *category, const char * name, const char * optRequiredVault, const char* optVersion); extern jlib_decl bool getSecretKeyValue(MemoryBuffer & result, const IPropertyTree *secret, const char * key); extern jlib_decl bool getSecretKeyValue(StringBuffer & result, const IPropertyTree *secret, const char * key); @@ -58,6 +58,15 @@ extern jlib_decl void splitUrlIsolateScheme(const char *url, StringBuffer &user, extern jlib_decl StringBuffer &generateDynamicUrlSecretName(StringBuffer &secretName, const char *scheme, const char *userPasswordPair, const char *host, unsigned port, const char *path); extern jlib_decl StringBuffer &generateDynamicUrlSecretName(StringBuffer &secretName, const char *url, const char *username); +//Start a background thread that updates active secrets that are due to need refeshing within lookaheadMs +extern jlib_decl void startSecretUpdateThread(unsigned lookaheadMs); +extern jlib_decl void stopSecretUpdateThread(); + extern jlib_decl bool queryMtls(); +#ifdef _USE_CPPUNIT +extern jlib_decl std::string testBuildSecretKey(const char * category, const char * name, const char * optVaultId, const char * optVersion); +extern jlib_decl void testExpandSecretKey(std::string & category, std::string & name, std::string & optVaultId, std::string & optVersion, const char * key); +#endif + #endif diff --git a/testing/unittests/jlibtests.cpp b/testing/unittests/jlibtests.cpp index 9918b77c5ef..0736c201ccd 100644 --- a/testing/unittests/jlibtests.cpp +++ b/testing/unittests/jlibtests.cpp @@ -3637,6 +3637,8 @@ class JLibSecretsTest : public CppUnit::TestFixture CPPUNIT_TEST(setup); CPPUNIT_TEST(testUpdate1); CPPUNIT_TEST(testUpdate2); + CPPUNIT_TEST(testBackgroundUpdate); + CPPUNIT_TEST(testKeyEncoding); CPPUNIT_TEST_SUITE_END(); //Each test creates a different instance of the class(!) so member values cannot be used to pass items @@ -3644,21 +3646,35 @@ class JLibSecretsTest : public CppUnit::TestFixture StringBuffer secretRoot; protected: - void checkSecret(const char * secret, const char * key, const char * expectedValue) + void checkSecret(const IPropertyTree * match, const char * key, const char * expectedValue) { - Owned match = getSecret("testing", secret); - CPPUNIT_ASSERT(match); - const char * secretValue = match->queryProp(key); - if (secretValue) + if (match) { - CPPUNIT_ASSERT_EQUAL_STR(secretValue, expectedValue); + const char * secretValue = match->queryProp(key); + if (secretValue) + { + CPPUNIT_ASSERT_EQUAL_STR(secretValue, expectedValue); + } + else + { + //IPropertyTree doesn't allow blank values, so a missing value is the same as a blank value + //We should probably revisit some day, but it is likely to break existing code if we do. + CPPUNIT_ASSERT_EQUAL_STR("", expectedValue); + } } else - { - //IPropertyTree doesn't allow blank values, so a missing value is the same as a blank value - //We should probably revisit some day, but it is likely to break existing code if we do. CPPUNIT_ASSERT_EQUAL_STR("", expectedValue); - } + } + void checkSecret(const char * secret, const char * key, const char * expectedValue) + { + Owned match = getSecret("testing", secret); + checkSecret(match, key, expectedValue); + } + + void checkSecret(ISyncedPropertyTree * secret, const char * key, const char * expectedValue) + { + Owned match = secret->getTree(); + checkSecret(match, key, expectedValue); } bool hasSecret(const char * name) @@ -3699,7 +3715,7 @@ class JLibSecretsTest : public CppUnit::TestFixture //Secret should not appear yet - null should be cached. CPPUNIT_ASSERT(!hasSecret("secret1")); - Owned secret2 = resolveSecret("testing", "secret2", nullptr, nullptr); + Owned secret2 = getSyncedSecret("testing", "secret2", nullptr, nullptr); CPPUNIT_ASSERT(!secret2->isValid()); CPPUNIT_ASSERT(!secret2->isStale()); @@ -3723,7 +3739,7 @@ class JLibSecretsTest : public CppUnit::TestFixture { initPath(); // secretRoot needs to be called for each test - Owned secret3 = resolveSecret("testing", "secret3", nullptr, nullptr); + Owned secret3 = getSyncedSecret("testing", "secret3", nullptr, nullptr); unsigned version = secret3->getVersion(); CPPUNIT_ASSERT(!secret3->isValid()); CPPUNIT_ASSERT(!secret3->isStale()); @@ -3806,6 +3822,117 @@ class JLibSecretsTest : public CppUnit::TestFixture writeTestingSecret("secret3", "value", nullptr); } + void testBackgroundUpdate() + { + initPath(); // secretRoot needs to be called for each test + startSecretUpdateThread(20); // 100ms expiry, check every 5ms for items expiring in 20ms time. + + //--------- First check that a missed secret is checked in the background --------- + Owned secret4 = getSyncedSecret("testing", "secret4", nullptr, nullptr); + CPPUNIT_ASSERT(!secret4->isValid()); + CPPUNIT_ASSERT(!secret4->isStale()); + + //Sleep for less than the update interval + MilliSleep(50); + CPPUNIT_ASSERT(!secret4->isValid()); + CPPUNIT_ASSERT(!secret4->isStale()); + + //Sleep so the cache entry should have expired, and no data around to make it not stale. + MilliSleep(60); + CPPUNIT_ASSERT(!secret4->isValid()); + CPPUNIT_ASSERT(secret4->isStale()); + + //--------- Now update the value in the background --------- + //First check that a missed secret is checked in the background + Owned secret5 = getSyncedSecret("testing", "secret5", nullptr, nullptr); + CPPUNIT_ASSERT(!secret5->isValid()); + CPPUNIT_ASSERT(!secret5->isStale()); + //And write a value so it is picked up on the next refresh + writeTestingSecret("secret5", "value", "secret5Value"); + + //Sleep for less than the update interval + MilliSleep(50); // elapsed=50 + CPPUNIT_ASSERT(!secret5->isValid()); + CPPUNIT_ASSERT(!secret5->isStale()); + + //Sleep so the cache entry should have expired and the value reread since reading ahead + MilliSleep(60); // elapsed=110 = 80 + 30 + CPPUNIT_ASSERT(secret5->isValid()); + CPPUNIT_ASSERT(!secret5->isStale()); + + //Sleep again so it is not accessed within the timeout period - it should now be marked as stale but valid + MilliSleep(100); // elapsed=210 = 80 + 80 + 50 + CPPUNIT_ASSERT(secret5->isValid()); + CPPUNIT_ASSERT(secret5->isStale()); + + //--------- Check that accessing the function marks the value so it is refreshed --------- + Owned secret6 = getSyncedSecret("testing", "secret6", nullptr, nullptr); + CPPUNIT_ASSERT(!secret6->isValid()); + CPPUNIT_ASSERT(!secret6->isStale()); + //And write a value so it is picked up on the next refresh + writeTestingSecret("secret6", "value", "secret6Value"); + + //Sleep for less than the update interval + MilliSleep(50); // elapsed=50 + CPPUNIT_ASSERT(!secret6->isValid()); + CPPUNIT_ASSERT(!secret6->isStale()); + + //Sleep so the cache entry should have expired and the value reread since reading ahead + MilliSleep(60); // elapsed=110 = 80 + 30 + CPPUNIT_ASSERT(secret6->isValid()); + CPPUNIT_ASSERT(!secret6->isStale()); + unsigned version1 = secret6->getVersion(); // Mark the value as accessed, but too early to be refreshed + writeTestingSecret("secret6", "value", "secret6Value2"); + + MilliSleep(40); // elapsed=150 = 80 + 70 + CPPUNIT_ASSERT(secret6->isValid()); + CPPUNIT_ASSERT(!secret6->isStale()); + unsigned version2 = secret6->getVersion(); // Mark the value as accessed, but too early to be refreshed + CPPUNIT_ASSERT(version2 == version1); + + MilliSleep(30); // elapsed=180 = 80 + 80 + 20 + CPPUNIT_ASSERT(secret6->isValid()); + CPPUNIT_ASSERT(!secret6->isStale()); + unsigned version3 = secret6->getVersion(); // Mark the value as accessed, but will now have been refreshed + CPPUNIT_ASSERT(version3 != version1); + checkSecret(secret4, "value", ""); + checkSecret(secret5, "value", "secret5Value"); + checkSecret(secret6, "value", "secret6Value2"); + + //Cleanup + writeTestingSecret("secret5", "value", nullptr); + writeTestingSecret("secret6", "value", nullptr); + stopSecretUpdateThread(); + } + + void testKeyEncoding() + { + for (auto category : { "abc", "def" }) + { + for (auto name : { "x", "y" }) + { + for (auto vault : { "vaultx", "" }) + { + for (auto version : { "", "v1" }) + { + std::string encoded = testBuildSecretKey(category, name, vault, version); + + std::string readCategory; + std::string readName; + std::string readVaultId; + std::string readVersion; + testExpandSecretKey(readCategory, readName, readVaultId, readVersion, encoded.c_str()); + + CPPUNIT_ASSERT_EQUAL_STR(category, readCategory.c_str()); + CPPUNIT_ASSERT_EQUAL_STR(name, readName.c_str()); + CPPUNIT_ASSERT_EQUAL_STR(vault, readVaultId.c_str()); + CPPUNIT_ASSERT_EQUAL_STR(version, readVersion.c_str()); + } + } + } + } + } + void writeTestingSecret(const char * secret, const char * key, const char * value) { StringBuffer filename;