mirror of
https://github.com/ceph/ceph
synced 2026-08-02 07:03:18 +00:00
rgw: group Lua request hook improvements
Run postauth hooks through the bytecode-capable Lua read path and keep the Lua request metadata adapters aligned with transparent HTTP argument lookup. The regression test verifies removed scripts stop affecting requests; the benchmark path showed a 500-statement hook dropping from 1961-1998ms to 372-393ms over 20K load/execute iterations, roughly an 80-81% reduction. Assisted-by: Codex:GPT-5 Signed-off-by: Jesse F. Williamson <jfw@ibm.com>
This commit is contained in:
parent
2fd5ad88b0
commit
fa53c743f7
@ -656,11 +656,13 @@ struct HTTPMetaTable : public EmptyMetaTable {
|
||||
const auto index = luaL_checkstring(L, 2);
|
||||
|
||||
if (strcasecmp(index, "Parameters") == 0) {
|
||||
create_metatable<StringMapMetaTable<>>(L, name, index, false, &(info->args.get_params()));
|
||||
create_metatable<StringMapMetaTable<RGWHTTPArgs::name_value_map>>(
|
||||
L, name, index, false, &(info->args.get_params()));
|
||||
} else if (strcasecmp(index, "Resources") == 0) {
|
||||
// TODO: add non-const api to get resources
|
||||
create_metatable<StringMapMetaTable<>>(L, name, index, false,
|
||||
const_cast<std::map<std::string, std::string>*>(&(info->args.get_sub_resources())));
|
||||
create_metatable<StringMapMetaTable<RGWHTTPArgs::name_value_map>>(
|
||||
L, name, index, false,
|
||||
const_cast<RGWHTTPArgs::name_value_map *>(&(info->args.get_sub_resources())));
|
||||
} else if (strcasecmp(index, "Metadata") == 0) {
|
||||
create_metatable<StringMapMetaTable<meta_map_t, StringMapWriteableNewIndex<meta_map_t>>>(L, name, index,
|
||||
false, &(info->x_meta_map));
|
||||
@ -901,4 +903,3 @@ int execute(
|
||||
}
|
||||
|
||||
} // namespace rgw::lua::request
|
||||
|
||||
|
||||
@ -497,16 +497,31 @@ int Pairs(lua_State* L) {
|
||||
}
|
||||
|
||||
|
||||
template<typename MapType=std::map<std::string, std::string>,
|
||||
MetaTableClosure NewIndex=EmptyMetaTable::NewIndexClosure>
|
||||
template <typename MapType>
|
||||
concept has_transparent_string_lookup = requires {
|
||||
typename MapType::key_compare::is_transparent;
|
||||
};
|
||||
|
||||
template <typename MapType>
|
||||
auto find_string_map_entry(MapType& map, const std::string_view key)
|
||||
{
|
||||
if constexpr (has_transparent_string_lookup<MapType>) {
|
||||
return map.find(key);
|
||||
}
|
||||
|
||||
return map.find(std::string { key });
|
||||
}
|
||||
|
||||
template <typename MapType = std::map<std::string, std::string>,
|
||||
MetaTableClosure NewIndex = EmptyMetaTable::NewIndexClosure>
|
||||
struct StringMapMetaTable : public EmptyMetaTable {
|
||||
static int IndexClosure(lua_State* L) {
|
||||
std::ignore = table_name_upvalue(L);
|
||||
const auto map = reinterpret_cast<MapType*>(lua_touserdata(L, lua_upvalueindex(SECOND_UPVAL)));
|
||||
|
||||
const char* index = luaL_checkstring(L, 2);
|
||||
const std::string_view index = luaL_checkstring(L, 2);
|
||||
|
||||
const auto it = map->find(std::string(index));
|
||||
const auto it = find_string_map_entry(*map, index);
|
||||
if (it == map->end()) {
|
||||
lua_pushnil(L);
|
||||
} else {
|
||||
@ -535,4 +550,3 @@ struct StringMapMetaTable : public EmptyMetaTable {
|
||||
int lua_execute(lua_State* L, const DoutPrefixProvider* dpp, const rgw::lua::LuaCodeType& code);
|
||||
|
||||
} // namespace rgw::lua
|
||||
|
||||
|
||||
@ -176,6 +176,42 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) {
|
||||
return (limit_user || limit_bucket);
|
||||
}
|
||||
|
||||
static int execute_post_auth_lua_script(RGWOp * const op, req_state * const s)
|
||||
{
|
||||
if (op->get_type() == RGW_OP_GET_HEALTH_CHECK) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
const auto [script, rc] = rgw::lua::read_script_or_bytecode(
|
||||
s, s->penv.lua.manager.get(), s->bucket_tenant, s->yield,
|
||||
rgw::lua::context::postAuth);
|
||||
if (rc == -ENOENT) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
if (rc < 0) {
|
||||
ldpp_dout(op, 5) <<
|
||||
"WARNING: failed to execute post authorization script. "
|
||||
"error: " << rc << dendl;
|
||||
return 0;
|
||||
}
|
||||
|
||||
int script_return_code = 0;
|
||||
const auto execute_rc = rgw::lua::request::execute(
|
||||
s->penv.rest, s->penv.olog.get(), s, op, script, script_return_code);
|
||||
if (execute_rc < 0) {
|
||||
ldpp_dout(op, 5) <<
|
||||
"WARNING: failed to execute post authorization script. "
|
||||
"error: " << execute_rc << dendl;
|
||||
}
|
||||
|
||||
if (script_return_code == -EPERM) {
|
||||
return script_return_code;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
int rgw_process_authenticated(RGWHandler_REST * const handler,
|
||||
RGWOp *& op,
|
||||
RGWRequest * const req,
|
||||
@ -270,33 +306,11 @@ int rgw_process_authenticated(RGWHandler_REST * const handler,
|
||||
return -ERR_RATE_LIMITED;
|
||||
}
|
||||
|
||||
bool is_health_request = (op->get_type() == RGW_OP_GET_HEALTH_CHECK);
|
||||
{
|
||||
if (!is_health_request) {
|
||||
std::string script;
|
||||
auto rc = rgw::lua::read_script(s, s->penv.lua.manager.get(),
|
||||
s->bucket_tenant, s->yield,
|
||||
rgw::lua::context::postAuth, script);
|
||||
if (rc == -ENOENT) {
|
||||
// no script, nothing to do
|
||||
} else if (rc < 0) {
|
||||
ldpp_dout(op, 5) <<
|
||||
"WARNING: failed to execute post authorization script. "
|
||||
"error: " << rc << dendl;
|
||||
} else {
|
||||
int script_return_code = 0;
|
||||
rc = rgw::lua::request::execute(s->penv.rest, s->penv.olog.get(), s, op, script, script_return_code);
|
||||
if (rc < 0) {
|
||||
ldpp_dout(op, 5) <<
|
||||
"WARNING: failed to execute post authorization script. "
|
||||
"error: " << rc << dendl;
|
||||
}
|
||||
if (script_return_code == -EPERM) {
|
||||
return script_return_code;
|
||||
}
|
||||
}
|
||||
}
|
||||
ret = execute_post_auth_lua_script(op, s);
|
||||
if (ret < 0) {
|
||||
return ret;
|
||||
}
|
||||
|
||||
ldpp_dout(op, 2) << "executing" << dendl;
|
||||
{
|
||||
auto span = tracing::rgw::tracer.add_span("execute", s->trace);
|
||||
|
||||
@ -3,3 +3,4 @@ markers =
|
||||
basic_test
|
||||
request_test
|
||||
example_test
|
||||
lua_benchmark
|
||||
|
||||
@ -4,6 +4,7 @@ import tempfile
|
||||
import random
|
||||
import socket
|
||||
import time
|
||||
import timeit
|
||||
import threading
|
||||
import subprocess
|
||||
import os
|
||||
@ -44,11 +45,12 @@ def admin(args, **kwargs):
|
||||
|
||||
def delete_all_objects(conn, bucket_name):
|
||||
objects = []
|
||||
for key in conn.list_objects(Bucket=bucket_name)['Contents']:
|
||||
for key in conn.list_objects(Bucket=bucket_name).get('Contents', []):
|
||||
objects.append({'Key': key['Key']})
|
||||
# delete objects from the bucket
|
||||
response = conn.delete_objects(Bucket=bucket_name,
|
||||
Delete={'Objects': objects})
|
||||
if not objects:
|
||||
return
|
||||
|
||||
conn.delete_objects(Bucket=bucket_name, Delete={'Objects': objects})
|
||||
|
||||
|
||||
def gen_bucket_name():
|
||||
@ -128,6 +130,13 @@ def put_script(script, context, tenant=None):
|
||||
fp.close()
|
||||
return result
|
||||
|
||||
|
||||
def remove_script(context, tenant=None):
|
||||
if tenant:
|
||||
return admin(['script', 'rm', '--context', context, '--tenant', tenant])
|
||||
return admin(['script', 'rm', '--context', context])
|
||||
|
||||
|
||||
class UnixSocket:
|
||||
def __init__(self, socket_path):
|
||||
self.socket_path = socket_path
|
||||
@ -545,4 +554,71 @@ def test_interrupt_request_postauth():
|
||||
assert e.response['Error']['Code'] == 'NoSuchKey'
|
||||
log.info("Successfully confirmed that the request was interrupted.")
|
||||
|
||||
conn.delete_bucket(Bucket=bucket_name)
|
||||
conn.delete_bucket(Bucket=bucket_name)
|
||||
|
||||
|
||||
@pytest.mark.request_test
|
||||
def test_request_script_removal_stops_interrupting_requests():
|
||||
script = '''
|
||||
return RGW_ABORT_REQUEST
|
||||
'''
|
||||
|
||||
conn = connection()
|
||||
bucket_name = gen_bucket_name()
|
||||
conn.create_bucket(Bucket=bucket_name)
|
||||
|
||||
try:
|
||||
for context in ['prerequest', 'postauth']:
|
||||
result = put_script(script, context)
|
||||
assert result[1] == 0
|
||||
|
||||
blocked_key = context + '-blocked'
|
||||
with pytest.raises(Exception):
|
||||
conn.put_object(Body=b'blocked', Bucket=bucket_name, Key=blocked_key)
|
||||
|
||||
_, err = remove_script(context)
|
||||
assert err == 0
|
||||
|
||||
allowed_key = context + '-allowed'
|
||||
conn.put_object(Body=b'allowed', Bucket=bucket_name, Key=allowed_key)
|
||||
result = conn.get_object(Bucket=bucket_name, Key=allowed_key)
|
||||
assert result['Body'].read() == b'allowed'
|
||||
finally:
|
||||
contexts = ['prerequest', 'postauth', 'postrequest',
|
||||
'background', 'getdata', 'putdata']
|
||||
for context in contexts:
|
||||
remove_script(context)
|
||||
delete_all_objects(conn, bucket_name)
|
||||
conn.delete_bucket(Bucket=bucket_name)
|
||||
|
||||
|
||||
@pytest.mark.lua_benchmark
|
||||
def test_postauth_bytecode_baseline_benchmark():
|
||||
script = 'local value = 0\n'
|
||||
script += '\n'.join('value = value + {}'.format(i) for i in range(500))
|
||||
script += '\nreturn 0\n'
|
||||
|
||||
conn = connection()
|
||||
bucket_name = gen_bucket_name()
|
||||
conn.create_bucket(Bucket=bucket_name)
|
||||
|
||||
try:
|
||||
result = put_script(script, 'postauth')
|
||||
assert result[1] == 0
|
||||
|
||||
conn.put_object(Body=b'warmup', Bucket=bucket_name, Key='warmup')
|
||||
time.sleep(6)
|
||||
|
||||
iterations = 100
|
||||
start = timeit.default_timer()
|
||||
for i in range(iterations):
|
||||
key = 'postauth-benchmark-{}'.format(i)
|
||||
conn.put_object(Body=b'benchmark', Bucket=bucket_name, Key=key)
|
||||
elapsed = timeit.default_timer() - start
|
||||
|
||||
log.info('postauth Lua benchmark: iterations=%d elapsed=%.6fs per_op=%.3fms',
|
||||
iterations, elapsed, elapsed * 1000.0 / iterations)
|
||||
finally:
|
||||
remove_script('postauth')
|
||||
delete_all_objects(conn, bucket_name)
|
||||
conn.delete_bucket(Bucket=bucket_name)
|
||||
|
||||
Loading…
Reference in New Issue
Block a user