From: Arran Cudbard-Bell Date: Wed, 20 Mar 2024 16:35:56 +0000 (-0400) Subject: Add distinct store/update methods to rlm_cache X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4cbcac55e7e1e83e6db31a7a4d53f47eb4b7deca;p=thirdparty%2Ffreeradius-server.git Add distinct store/update methods to rlm_cache --- diff --git a/raddb/mods-available/cache b/raddb/mods-available/cache index a5314d938d3..1d5a8b64cd9 100644 --- a/raddb/mods-available/cache +++ b/raddb/mods-available/cache @@ -314,6 +314,7 @@ cache { # | Return | Description # | `ok` | if a cache entry was found. # | `notfound` | if no cache entry was found. + # | `fail` | if the cache was unavailable. # |=== # # cache.load:: Load an existing cache entry and merge it into the request. @@ -323,16 +324,27 @@ cache { # | Return | Description # | `updated` | if a cache entry was found and loaded. # | `notfound` | if no cache entry was found. + # | `fail` | if the cache was unavailable. # |=== # - # cache.store:: Perform an upset against the data store. (Not affect the existing - # request). + # cache.update:: Perform an upsert against the data store, updating the entry TTL # # [options="header,autowidth"] # |=== # | Return | Description # | `updated` | if we added cache entry. - # | `noop` | if a cache entry ready exists. + # | `fail` | if the cache was unavailable. + # |=== + # + # cache.store:: Inserts data into the cache if, and only if, it is not already present + # Will not update the entry TTL. + # + # [options="header,autowidth"] + # |=== + # | Return | Description + # | `updated` | we created or updated a cache entry. + # | `noop` | if a cache entry aready existed. + # | `fail` | if the cache was unavailable. # |=== # # cache.clear:: Delete cache entry from the data store without checking if the entry @@ -341,8 +353,9 @@ cache { # [options="header,autowidth"] # |=== # | Return | Description - # | `ok` | if we found and remove a entry. + # | `ok` | if we found and removed a entry. # | `notfound` | if no cache entry was found. + # | `fail` | if the cache was unavailable. # |=== # # cache.ttl:: Change the TTL on an existing entry. @@ -352,6 +365,7 @@ cache { # | Return | Description # | `updated` | if we found entry and updated the ttl. # | `notfound` | if no cache entry was found. + # | `fail` | if the cache was unavailable. # |=== # # ### Examples diff --git a/raddb/sites-available/default b/raddb/sites-available/default index b9916b4eaa5..1bf0b37aaf7 100644 --- a/raddb/sites-available/default +++ b/raddb/sites-available/default @@ -759,9 +759,6 @@ recv Access-Request { # We also recommend doing user lookups in the `inner-tunnel` # virtual server. # - eap { - ok = return - } # # The `unix` module will obtain passwords from `/etc/passwd` @@ -969,7 +966,6 @@ authenticate ldap { # For EAP-MD5, EAP-MSCHAP, EAP-TLS, EAP-TTLS, EAP-PEAP, EAP-PWD, etc. # authenticate eap { - eap } @@ -1072,7 +1068,6 @@ send Access-Accept { # For EAP, ensure that the Access-Accept contains a User-Name # attribute. # - eap # # Get an address from the IP Pool. @@ -1231,7 +1226,6 @@ send Access-Reject { # Insert an EAP-Failure message if the request was rejected by # policy, instead of from an authentication failure. # - eap # # Call an instance of `linelog` to log the authentication failure diff --git a/src/modules/rlm_cache/rlm_cache.c b/src/modules/rlm_cache/rlm_cache.c index 2b0f8e834ee..58755bef390 100644 --- a/src/modules/rlm_cache/rlm_cache.c +++ b/src/modules/rlm_cache/rlm_cache.c @@ -934,8 +934,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_status(rlm_rcode_t *p_result, rlm_cache_entry_t *entry = NULL; rlm_cache_handle_t *handle = NULL; - DEBUG3("Calling %s.status", mctx->inst->name); - if (env->key->vb_length == 0) { REDEBUG("Zero length key string is invalid"); RETURN_MODULE_FAIL; @@ -974,8 +972,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_load(rlm_rcode_t *p_result, m rlm_cache_entry_t *entry = NULL; rlm_cache_handle_t *handle = NULL; - DEBUG3("Calling %s.load", mctx->inst->name); - if (env->key->vb_length == 0) { REDEBUG("Zero length key string is invalid"); RETURN_MODULE_FAIL; @@ -1003,14 +999,14 @@ finish: RETURN_MODULE_RCODE(rcode); } -/** Create and insert a cache entry +/** Create, or update a cache entry * * @return * - #RLM_MODULE_OK on success. * - #RLM_MODULE_UPDATED if we merged the cache entry. * - #RLM_MODULE_FAIL on failure. */ -static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request) +static unlang_action_t CC_HINT(nonnull) mod_method_update(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request) { rlm_cache_t const *inst = talloc_get_type_abort(mctx->inst->data, rlm_cache_t); cache_call_env_t *env = talloc_get_type_abort(mctx->env_data, cache_call_env_t); @@ -1021,8 +1017,6 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, rlm_cache_handle_t *handle = NULL; fr_pair_t *vp; - DEBUG3("Calling %s.store", mctx->inst->name); - if (env->key->vb_length == 0) { REDEBUG("Zero length key string is invalid"); RETURN_MODULE_FAIL; @@ -1074,12 +1068,77 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, * should perform upserts. */ if (expire) { - DEBUG4("Set the cache expire"); + DEBUG3("Expiring cache entry"); cache_expire(&rcode, inst, request, &handle, env->key); if (rcode == RLM_MODULE_FAIL) goto finish; } + /* + * Inserts are upserts, so we don't care about the + * entry state. + */ + cache_insert(&rcode, inst, request, &handle, env->key, env->maps, ttl); + if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED; + +finish: + cache_unref(request, inst, entry, handle); + + RETURN_MODULE_RCODE(rcode); +} + +/** Create, or update a cache entry + * + * @return + * - #RLM_MODULE_NOOP if an entry already existed. + * - #RLM_MODULE_UPDATED if we inserted a cache entry. + * - #RLM_MODULE_FAIL on failure. + */ +static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, module_ctx_t const *mctx, request_t *request) +{ + rlm_cache_t const *inst = talloc_get_type_abort(mctx->inst->data, rlm_cache_t); + cache_call_env_t *env = talloc_get_type_abort(mctx->env_data, cache_call_env_t); + rlm_rcode_t rcode = RLM_MODULE_NOOP; + fr_time_delta_t ttl; + rlm_cache_entry_t *entry = NULL; + rlm_cache_handle_t *handle = NULL; + fr_pair_t *vp; + + if (env->key->vb_length == 0) { + REDEBUG("Zero length key string is invalid"); + RETURN_MODULE_FAIL; + } + + if (cache_acquire(&handle, inst, request) < 0) { + RETURN_MODULE_FAIL; + } + + /* Process the TTL */ + ttl = inst->config.ttl; /* Set the default value from cache { ttl=... } */ + vp = fr_pair_find_by_da(&request->control_pairs, NULL, attr_cache_ttl); + if (vp && (vp->vp_int32 > 0)) { + ttl = fr_time_delta_from_sec(vp->vp_int32); + + DEBUG3("Overriding default TTL %pV -> %d", fr_box_time_delta(ttl), vp->vp_int32); + } + + /* + * We can only alter the TTL on an entry if it exists. + */ + cache_find(&rcode, &entry, inst, request, &handle, env->key); + switch (rcode) { + default: + case RLM_MODULE_OK: + rcode = RLM_MODULE_NOOP; + goto finish; + + case RLM_MODULE_FAIL: + goto finish; + + case RLM_MODULE_NOTFOUND: + break; + } + /* * Inserts are upserts, so we don't care about the * entry state, just that we're not meant to be @@ -1087,10 +1146,10 @@ static unlang_action_t CC_HINT(nonnull) mod_method_store(rlm_rcode_t *p_result, * insert. */ cache_insert(&rcode, inst, request, &handle, env->key, env->maps, ttl); - if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED; finish: cache_unref(request, inst, entry, handle); + if (rcode == RLM_MODULE_OK) rcode = RLM_MODULE_UPDATED; RETURN_MODULE_RCODE(rcode); } @@ -1342,6 +1401,7 @@ module_rlm_t rlm_cache = { .method_names = (module_method_name_t[]){ { .name1 = "status", .name2 = CF_IDENT_ANY, .method = mod_method_status, .method_env = &cache_method_env }, { .name1 = "load", .name2 = CF_IDENT_ANY, .method = mod_method_load, .method_env = &cache_method_env }, + { .name1 = "update", .name2 = CF_IDENT_ANY, .method = mod_method_update, .method_env = &cache_method_env }, { .name1 = "store", .name2 = CF_IDENT_ANY, .method = mod_method_store, .method_env = &cache_method_env }, { .name1 = "clear", .name2 = CF_IDENT_ANY, .method = mod_method_clear, .method_env = &cache_method_env }, { .name1 = "ttl", .name2 = CF_IDENT_ANY, .method = mod_method_ttl, .method_env = &cache_method_env }, diff --git a/src/tests/modules/cache_rbtree/cache-method-logic.unlang b/src/tests/modules/cache_rbtree/cache-method-logic.unlang index a526452782e..f39efbbeb3b 100644 --- a/src/tests/modules/cache_rbtree/cache-method-logic.unlang +++ b/src/tests/modules/cache_rbtree/cache-method-logic.unlang @@ -4,11 +4,11 @@ &Filter-Id := 'testkey1' # -# 0. Basic store and retrieve +# 0. Basic update and retrieve # &control.Callback-Id := 'cache me' -cache.store +cache.update if (!updated) { test_fail } @@ -63,13 +63,13 @@ if (!notfound) { # 14. This should still allow the creation of a new entry &control.Cache-TTL := -2 -cache.store +cache.update if (!updated) { test_fail } # 12. We have nothing to do if it is ready added. -cache.store +cache.update if (!updated) { test_fail } @@ -102,7 +102,7 @@ if (&Callback-Id == &control.Callback-Id) { # 20. Check that a new entry is created &control.Cache-TTL := -2 -cache.store +cache.update if (!updated) { test_fail } @@ -128,7 +128,7 @@ if (&Callback-Id != &control.Callback-Id) { &control.Cache-TTL := -2 &control.Cache-Merge-New := yes -cache.store +cache.update if (!updated) { test_fail } @@ -148,4 +148,21 @@ if (&Cache-Entry-Hits != 1) { test_fail } +# 27. Try and store an existing entry, should do nothing +cache.store +if (!noop) { + test_fail +} + +# 28. But with the entry removed, we can now create a new entry +cache.clear +if (!ok) { + test_fail +} + +cache.store +if (!updated) { + test_fail +} + test_pass diff --git a/src/tests/modules/cache_rbtree/cache-method-update.unlang b/src/tests/modules/cache_rbtree/cache-method-update.unlang index a5cc65398d9..6242ee10b07 100644 --- a/src/tests/modules/cache_rbtree/cache-method-update.unlang +++ b/src/tests/modules/cache_rbtree/cache-method-update.unlang @@ -17,17 +17,17 @@ } # -# Basic store and retrieve +# Basic update and retrieve # &control.Callback-Id := 'cache me' -cache_update.store +cache_update.update if (!updated) { test_fail } # Merge -cache_update.store +cache_update.update if (!updated) { test_fail } diff --git a/src/tests/modules/cache_rbtree/cache-update.unlang b/src/tests/modules/cache_rbtree/cache-update.unlang index 962fcec86d3..d09bac4020e 100644 --- a/src/tests/modules/cache_rbtree/cache-update.unlang +++ b/src/tests/modules/cache_rbtree/cache-update.unlang @@ -17,7 +17,7 @@ } # -# Basic store and retrieve +# Basic update and retrieve # &control.Callback-Id := 'cache me'