rgw/lua: allow access to object data #46550

yuvalif · 2022-06-07T11:47:45Z

Signed-off-by: Yuval Lifshitz ylifshit@redhat.com

testing

using vstart cluster: MON=1 OSD=1 MDS=0 MGR=0 RGW=1 ../src/vstart.sh -n -d
sample script (data.lua):

RGWDebugLog("Data[0]="..tostring(Data[0]))
RGWDebugLog("Data[-1]="..tostring(Data[-1]))
RGWDebugLog("Data[99]="..tostring(Data[99]))
RGWDebugLog("Data[1]="..tostring(Data[1]))
RGWDebugLog("Data[8]="..tostring(Data[8]))
RGWDebugLog("Data[9]="..tostring(Data[9]))
RGWDebugLog("#Data"..tostring(#Data))

RGWDebugLog("ipairs");
for i, v in ipairs(Data) do
  RGWDebugLog("Data["..tostring(i).."]="..v)
end

RGWDebugLog("pairs");
for i, v in pairs(Data) do
  RGWDebugLog("Data["..tostring(i).."]="..v)
end

upload script: bin/radosgw-admin script put --infile=./data.lua --context=putData
local file to upload (data.txt):

ABCDEFG

create bucket, then upload and download an object:

aws --endpoint-url http://localhost:8000 s3 mb s3://kaboom
aws --endpoint-url http://localhost:8000 s3 cp data.txt s3://kaboom
aws --endpoint-url http://localhost:8000 s3 cp s3://kaboom/data.txt .

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows

src/rgw/rgw_lua_filter.h

src/rgw/rgw_op.h

src/rgw/CMakeLists.txt

src/rgw/rgw_lua_filter.cc

github-actions · 2022-06-08T06:19:55Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

yuvalif · 2022-06-08T15:52:09Z

comments from RGW refactoring call:

split to 2 contexts (putData and getData)
add a parameter to block the request from the lua script
no need to pass offset and length into lua

doc/radosgw/lua-scripting.rst

anthonyeleven · 2022-07-10T19:54:33Z

doc/radosgw/lua-scripting.rst

+Tracing functions can be used only in `postRequest` context.
+
+- ``Request.Trace.SetAttribute()`` - sets the attribute for the request's trace.
+  Takes two arguments. The first is the `key`, which should be a string. The second is the value, which can either be a string or a number.


First bit of line 322 isn’t a sentence. Suggest

“the request’s trace, and takes two arguments.”

Also if `key` is in backticks, shouldn’t `value` be as well?

Maybe clarify if “number” means an integer, a real, or either?

Hate to quibble, but I think you added "of" instead of "or"

doc/radosgw/lua-scripting.rst

(will squash before merge) Signed-off-by: yuval Lifshitz <ylifshit@redhat.com>

yuvalif · 2022-07-11T10:15:57Z

jenkins test make check

doc/radosgw/lua-scripting.rst

cbodley · 2022-07-12T14:36:30Z

src/rgw/rgw_op.cc

+    }
+    if (run_lua) {
+      filter = &*run_lua;
+    }


don't we want to add this lua filter last so its data isn't transformed by compression or encryption? have you tested these combinations?

running lua on compressed and encrypted data won't be useful.
this is why i added it first in the "put" case.
if someone wants to encrypt or compress the after lua read it, there should not be any limitation.

anyway, will test to see that it works, and update the documentation accordingly

running lua on compressed and encrypted data won't be useful.
this is why i added it first in the "put" case.

right, i just think you have it backwards. this code is adding wrapper filters around the object processor, so the filter added last is the one on the outside, so it sees the data first

what about the "get" case:
https://github.com/ceph/ceph/pull/46550/files#diff-319900fb50bdb02677039fb81949243b9c0e7d98690f6fddf35e6ca5d5d52b2cR2308 ?
here i added the lua filter last, should i switch it to be first (so it runs last)?

the "get" case looks correct as-is. there we're wrapping filters around send_response_data(). you're adding the lua filter before decompress/decrypt filters, so your lua filter should see the data in plain-text. it's still worth testing to confirm

the "get" case was reading the compressed data.
i moved it to be first in 3ca1a48
so it is executed last

cbodley · 2022-07-12T14:38:12Z

doc/radosgw/lua-scripting.rst

-Global ``RGW`` Table
+Data Context
+--------------------
+Both ``getdata`` and ``putdata`` contexts has a single field named ``Data`` which is read only, optional and iterable (byte by byte).


it's probably worth noting that these scripts only see one chunk of data per execution, right?

src/rgw/rgw_lua_data_filter.cc

anthonyeleven · 2022-07-12T14:43:00Z

doc/radosgw/lua-scripting.rst

+
+- ``Request.Trace.SetAttribute(<key>, <value>)`` - sets the attribute for the request's trace.
+  The function takes two arguments: the first is the ``key``, which should be a string, and the second is the ``value``, which can either be a string or a number (integer or double).
+  Using the attribute, you can locate specific traces.


You may then locate specific traces by using this attribute.

anthonyeleven · 2022-07-12T14:43:53Z

doc/radosgw/lua-scripting.rst

+
+- ``Request.Trace.AddEvent(<name>, <attributes>)`` - adds an event to the first span of the request's trace
+  An event is defined by event name, event time, and zero or more event attributes.
+  Therefore, the function accepts one or two arguments. A string containing the event ``name`` should be the first argument, followed by the event ``attributes``, which is optional for events without attributes.


The function accepts one or two arguments: A string

anthonyeleven · 2022-07-12T14:44:18Z

doc/radosgw/lua-scripting.rst

-Global ``RGW`` Table
+Data Context
+--------------------
+Both ``getdata`` and ``putdata`` contexts has a single field named ``Data`` which is read only, optional and iterable (byte by byte).


s/has/have/
s/read only/read-only/

anthonyeleven

Docs lgtm, a couple of small requests but not enough to block approval.

anthonyeleven · 2022-07-17T05:59:32Z

doc/radosgw/lua-scripting.rst

+A request (pre or post) or data (get or put) context script may be constrained to operations belonging to a specific tenant's users.
+The request context script can also access fields in the request and modify certain fields, as well as the `Global RGW Table`_.
+The data context script can access the content of the object as well as the request fields and the `Global RGW Table`_. 
+All Lua language features can be used in all contexts.

 By default, all lua standard libraries are available in the script, however, in order to allow for other lua modules to be used in the script, we support adding packages to an allowlist:


Lua capitalized?

anthonyeleven · 2022-07-17T06:00:42Z

doc/radosgw/lua-scripting.rst

-    By default, this directory would be `/tmp/luarocks/<entity name>`. Its prefix part (`/tmp/luarocks/`) could be set to a different location via the `rgw_luarocks_location` configuration parameter. 
-    Note that this parameter should not be set to one of the default locations where luarocks install packages (e.g. `$HOME/.luarocks`, `/usr/lib64/lua`, `/usr/share/lua`)
+    By default, this directory would be ``/tmp/luarocks/<entity name>``. Its prefix part (``/tmp/luarocks/``) could be set to a different location via the ``rgw_luarocks_location`` configuration parameter. 
+    Note that this parameter should not be set to one of the default locations where luarocks install packages (e.g. ``$HOME/.luarocks``, ``/usr/lib64/lua``, ``/usr/share/lua``)


Period at the end?

anthonyeleven · 2022-07-17T06:02:15Z

doc/radosgw/lua-scripting.rst

@@ -511,3 +526,39 @@ in `postRequest` context, we can add attributes and events to the request's trac

  Request.Trace.AddEvent("second event", event_attrs)

+- Calculate the entropy and size of uploaded objects and print to debug log


I'm curious, how is this entropy value useful?

could be used to detect encryption of objects. added this to the doc

anthonyeleven · 2022-07-18T08:29:55Z

doc/radosgw/lua-scripting.rst

+The data context script can access the content of the object as well as the request fields and the `Global RGW Table`_. 
+All Lua language features can be used in all contexts.
+
+By default, all Lua standard libraries are available in the script, however, in order to allow for other lua modules to be used in the script, we support adding packages to an allowlist:


There's a second s/lua/Lua/ on this line :)

fixed (together with some more lower case "lau") - hope this is the last round :-)

yuvalif · 2022-07-19T18:07:50Z

jenkins test api

Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

yuvalif · 2022-08-07T06:58:03Z

jenkins test make check

yuvalif · 2022-08-07T18:38:53Z

http://pulpito.front.sepia.ceph.com/yuvalif-2022-08-07_13:01:20-rgw:verify-wip-yuval-lua-filter-distro-default-smithi/
teuthology is passing with valgrind issue

out of 24 failures, 22 are valgrind (the known boost/asio ones)
one failure is "reached maximum tries (90) after waiting for 540 seconds"
one failure is an selinux check: "SELinux denials found on ubuntu@smithi117.front.sepia.ceph.com"

cbodley · 2022-08-08T21:10:19Z

@yuvalif please run the full rgw suite with --subset ~1/3, there are important regression tests in the other subsuites!

yuvalif · 2022-08-09T09:14:32Z

@yuvalif please run the full rgw suite with --subset ~1/3, there are important regression tests in the other subsuites!

i ran with:
teuthology-suite --ceph-repo https://github.com/ceph/ceph-ci.git --subset 1/3 -p 75 -s rgw --ceph wip-yuval-lua-filter -m smithi
it is running 58 jobs, is this enough?

yuvalif · 2022-08-15T08:27:24Z

teuthology results: http://pulpito.front.sepia.ceph.com/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/

valgrind issues
"Cannot create secret" failure
"multisite" is failing mostly on AssertionError: failed meta checkpoint for zone=test-zone2
- one failure on rgw_multi.tests.test_bucket_cors
- see: http://qa-proxy.ceph.com/teuthology/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/6972376/teuthology.log
LC failure in: http://qa-proxy.ceph.com/teuthology/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/6972370/teuthology.log
LC failure in: http://qa-proxy.ceph.com/teuthology/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/6972374/teuthology.log
tempest failing in: http://qa-proxy.ceph.com/teuthology/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/6972380/teuthology.log
"max retries" failure in: http://qa-proxy.ceph.com/teuthology/yuvalif-2022-08-14_16:15:18-rgw-wip-yuval-lua-filter-distro-default-smithi/6972425/teuthology.log

cbodley · 2022-08-15T18:16:01Z

src/rgw/rgw_op.cc

@@ -2084,6 +2086,20 @@ int RGWGetObj::get_data_cb(bufferlist& bl, off_t bl_ofs, off_t bl_len)
  return send_response_data(bl, bl_ofs, bl_len);
 }

+int RGWGetObj::get_lua_filter(std::unique_ptr<RGWGetObj_Filter>* filter, RGWGetObj_Filter* cb) {
+  std::string script;
+  const auto rc = rgw::lua::read_script(s, store, s->bucket_tenant, s->yield, rgw::lua::context::getData, script);


this broke the build on main. it looks like a conflict with a1e21d0 which changed the signature of read_script()

it's not clear how we're supposed to get that LuaManager all the way into RGWGetObj/PutObj. in the meantime, i prepared a revert at #47612

yuvalif added rgw DNM labels Jun 7, 2022

yuvalif requested a review from cbodley June 7, 2022 11:47

github-actions bot added the build/ops label Jun 7, 2022

yuvalif added the needs-doc label Jun 7, 2022

cbodley reviewed Jun 7, 2022

View reviewed changes

src/rgw/rgw_lua_filter.h Outdated Show resolved Hide resolved

src/rgw/rgw_lua_filter.h Outdated Show resolved Hide resolved

src/rgw/rgw_op.h Show resolved Hide resolved

src/rgw/CMakeLists.txt Outdated Show resolved Hide resolved

src/rgw/rgw_lua_filter.cc Outdated Show resolved Hide resolved

github-actions bot added the needs-rebase label Jun 8, 2022

yuvalif force-pushed the wip-yuval-lua-filter branch from 79d3978 to 12ae262 Compare July 10, 2022 18:53

yuvalif requested a review from a team as a code owner July 10, 2022 18:53

github-actions bot added documentation tests and removed needs-rebase labels Jul 10, 2022

yuvalif removed needs-doc DNM labels Jul 10, 2022

yuvalif requested a review from mattbenjamin July 10, 2022 18:56

anthonyeleven requested changes Jul 10, 2022

View reviewed changes

yuvalif added a commit to yuvalif/ceph that referenced this pull request Jul 11, 2022

rgw/lua: fix doc comments from PR ceph#46550

17a7462

(will squash before merge) Signed-off-by: yuval Lifshitz <ylifshit@redhat.com>

yuvalif requested a review from anthonyeleven July 11, 2022 09:22

yuvalif changed the title ~~[WIP] rgw/lua: allow access to object data~~ rgw/lua: allow access to object data Jul 11, 2022

anthonyeleven requested changes Jul 11, 2022

View reviewed changes

doc/radosgw/lua-scripting.rst Outdated Show resolved Hide resolved

yuvalif force-pushed the wip-yuval-lua-filter branch from 17a7462 to 6d3448c Compare July 11, 2022 18:16

cbodley reviewed Jul 12, 2022

View reviewed changes

anthonyeleven requested changes Jul 12, 2022

View reviewed changes

yuvalif requested a review from anthonyeleven July 17, 2022 04:02

anthonyeleven approved these changes Jul 17, 2022

View reviewed changes

yuvalif added needs-qa wip-yuval-testing labels Jul 18, 2022

yuvalif force-pushed the wip-yuval-lua-filter branch from 3ca1a48 to f5f4ef5 Compare July 18, 2022 08:07

anthonyeleven reviewed Jul 18, 2022

View reviewed changes

yuvalif force-pushed the wip-yuval-lua-filter branch from f5f4ef5 to 95c22b8 Compare July 18, 2022 16:33

yuvalif requested a review from cbodley July 18, 2022 16:35

rgw/lua: allow read access to object data

f4501f6

Signed-off-by: Yuval Lifshitz <ylifshit@redhat.com>

yuvalif force-pushed the wip-yuval-lua-filter branch from 95c22b8 to f4501f6 Compare August 4, 2022 16:22

cbodley approved these changes Aug 7, 2022

View reviewed changes

yuvalif removed the needs-qa label Aug 7, 2022

yuvalif merged commit ad51b94 into ceph:main Aug 15, 2022

cbodley reviewed Aug 15, 2022

View reviewed changes

yuvalif mentioned this pull request Aug 21, 2022

rgw/lua: allow read access to object data #47719

Merged

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rgw/lua: allow access to object data #46550

rgw/lua: allow access to object data #46550

yuvalif commented Jun 7, 2022 •

edited

github-actions bot commented Jun 8, 2022

yuvalif commented Jun 8, 2022

anthonyeleven Jul 10, 2022

anthonyeleven Jul 11, 2022

yuvalif commented Jul 11, 2022

cbodley Jul 12, 2022

yuvalif Jul 13, 2022

cbodley Jul 13, 2022

yuvalif Jul 13, 2022

cbodley Jul 14, 2022

yuvalif Jul 14, 2022

cbodley Jul 12, 2022

anthonyeleven Jul 12, 2022

anthonyeleven Jul 12, 2022

anthonyeleven Jul 12, 2022

anthonyeleven left a comment

anthonyeleven Jul 17, 2022

anthonyeleven Jul 17, 2022

anthonyeleven Jul 17, 2022

yuvalif Jul 18, 2022

anthonyeleven Jul 18, 2022

yuvalif Jul 18, 2022

yuvalif commented Jul 19, 2022

yuvalif commented Aug 7, 2022

yuvalif commented Aug 7, 2022

cbodley commented Aug 8, 2022

yuvalif commented Aug 9, 2022

yuvalif commented Aug 15, 2022

cbodley Aug 15, 2022

cbodley Aug 15, 2022

		@@ -511,3 +526,39 @@ in `postRequest` context, we can add attributes and events to the request's trac

		Request.Trace.AddEvent("second event", event_attrs)

		- Calculate the entropy and size of uploaded objects and print to debug log

rgw/lua: allow access to object data #46550

rgw/lua: allow access to object data #46550

Conversation

yuvalif commented Jun 7, 2022 • edited

testing

Contribution Guidelines

Checklist

github-actions bot commented Jun 8, 2022

yuvalif commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuvalif commented Jul 11, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anthonyeleven left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuvalif commented Jul 19, 2022

yuvalif commented Aug 7, 2022

yuvalif commented Aug 7, 2022

cbodley commented Aug 8, 2022

yuvalif commented Aug 9, 2022

yuvalif commented Aug 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuvalif commented Jun 7, 2022 •

edited