Skip to content

cls/user: add interfaces to index user account resources#54563

Closed
cbodley wants to merge 3 commits intoceph:mainfrom
cbodley:wip-cls-user-account
Closed

cls/user: add interfaces to index user account resources#54563
cbodley wants to merge 3 commits intoceph:mainfrom
cbodley:wip-cls-user-account

Conversation

@cbodley
Copy link
Contributor

@cbodley cbodley commented Nov 19, 2023 •
edited
Loading

a small building block for the rados user account implementation. this allows each account to maintain omap indices of its resources in rados objects like {account}.users, {account}.roles, and {account}.groups. these indices will only be used to serve the iam rest apis like ListUsers/ListRoles/ListGroups. the existing cls_user apis like cls_user_set_buckets() will be used to track {account}.buckets for ListBuckets and account stats

adds a generic struct cls_user_account_resource which can represent a user, role, or group. cls_user only needs to know about the resource's name and path. all other type-specific information needed by the iam apis can be encoded in the opaque bufferlist metadata field

the cls_user_client interface is very simple, providing methods to add a cls_user_account_resource entry to the index, remove an entry by name, and to list entries with pagination and filtering by path prefix

the cls_user server implementation just manages two omap indices:

// Account resource index design:
//
// The resource_add/rm ops require entries to be indexed by name, while the
// resource_list op requires them to be indexed by path for filtering by
// path_prefix. To avoid indirection on listing, the path is used as the
// primary index.
//
// The omap entries for the name index use keys of the form "n:{name}", and
// their value stores the (possibly empty) path string.
//
// The omap entries for the path index use keys of the form "p:{path}{name}"
// so they can be sorted/filtered by path prefix, and their value encodes a
// cls_user_account_resource.
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
  • jenkins test rook e2e

@cbodley cbodley requested a review from a team as a code owner November 19, 2023 18:42
@cbodley cbodley marked this pull request as draft November 20, 2023 15:10
@cbodley
Copy link
Contributor Author

cbodley commented Nov 20, 2023 •
edited
Loading

i had planned to only index the resource IDs, and require the ops like ListRoles to look up the rest of the metadata like Path/Name/Arn/CreateDate/etc. but each of these iam List* apis also support a PathPrefix request param to filter by path, so cls will need to know paths and maintain an index by-path. so the omap entries should probably contain all of the other metadata required for listing too

edit: updated to index by-path and by-name, and to store additional metadata needed by the apis

@cbodley cbodley force-pushed the wip-cls-user-account branch 2 times, most recently from 30d3e1a to acd2017 Compare November 20, 2023 22:47
@cbodley cbodley marked this pull request as ready for review November 20, 2023 22:54
@cbodley cbodley mentioned this pull request Nov 21, 2023
Merged
52 tasks
@cbodley
cmake: move WITH_RADOSGW checks up to test/CMakeLists.txt
b0289ff 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley
cls/user: add interfaces to index user account resources
e57554d 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley force-pushed the wip-cls-user-account branch from acd2017 to 0adbda9 Compare November 21, 2023 14:20
@cbodley
Copy link
Contributor Author

cbodley commented Nov 21, 2023 

2 cls_user_account_resource_list_op
/tmp/typ-lOPQNzmmm /tmp/typ-3JctRy4d2 differ: byte 15, line 1
**** cls_user_account_resource_list_op test 1 binary reencode check failed ****
   ceph-dencoder type cls_user_account_resource_list_op select_test 1 encode export /tmp/typ-lOPQNzmmm
   ceph-dencoder type cls_user_account_resource_list_op select_test 1 encode decode encode export /tmp/typ-3JctRy4d2
1,2c1,2

fixed - uint32_t max_entries was uninitialized, so differed between tests

@cbodley
test/cls: add ceph_test_cls_user
5f66625 
Signed-off-by: Casey Bodley <cbodley@redhat.com>
@cbodley cbodley force-pushed the wip-cls-user-account branch from 0adbda9 to 5f66625 Compare November 22, 2023 20:14
@cbodley
Copy link
Contributor Author

cbodley commented Nov 23, 2023

jenkins test this please

@cbodley
Copy link
Contributor Author

cbodley commented Nov 29, 2023

passed qa in https://pulpito.ceph.com/cbodley-2023-11-27_20:42:48-rgw-wip-cbodley-testing-distro-default-smithi/ after rerun https://pulpito.ceph.com/cbodley-2023-11-29_13:34:32-rgw-wip-cbodley-testing-distro-default-smithi/

ceph_test_cls_user passing:

2023-11-29T14:05:57.530 INFO:tasks.workunit.client.0.smithi070.stdout:[==========] Running 3 tests from 1 test suite.
2023-11-29T14:05:57.530 INFO:tasks.workunit.client.0.smithi070.stdout:[----------] Global test environment set-up.
2023-11-29T14:05:59.362 INFO:tasks.workunit.client.0.smithi070.stdout:[----------] 3 tests from ClsAccount
2023-11-29T14:05:59.362 INFO:tasks.workunit.client.0.smithi070.stdout:[ RUN      ] ClsAccount.add
2023-11-29T14:05:59.397 INFO:tasks.workunit.client.0.smithi070.stdout:[       OK ] ClsAccount.add (34 ms)
2023-11-29T14:05:59.397 INFO:tasks.workunit.client.0.smithi070.stdout:[ RUN      ] ClsAccount.rm
2023-11-29T14:05:59.488 INFO:tasks.workunit.client.0.smithi070.stdout:[       OK ] ClsAccount.rm (91 ms)
2023-11-29T14:05:59.488 INFO:tasks.workunit.client.0.smithi070.stdout:[ RUN      ] ClsAccount.list
2023-11-29T14:05:59.528 INFO:tasks.workunit.client.0.smithi070.stdout:[       OK ] ClsAccount.list (41 ms)
2023-11-29T14:05:59.528 INFO:tasks.workunit.client.0.smithi070.stdout:[----------] 3 tests from ClsAccount (166 ms total)
2023-11-29T14:05:59.528 INFO:tasks.workunit.client.0.smithi070.stdout:
2023-11-29T14:05:59.528 INFO:tasks.workunit.client.0.smithi070.stdout:[----------] Global test environment tear-down
2023-11-29T14:06:00.397 INFO:tasks.workunit.client.0.smithi070.stdout:[==========] 3 tests from 1 test suite ran. (2921 ms total)
2023-11-29T14:06:00.397 INFO:tasks.workunit.client.0.smithi070.stdout:[  PASSED  ] 3 tests.

@cbodley cbodley mentioned this pull request Dec 5, 2023
Merged
@cbodley cbodley mentioned this pull request Dec 13, 2023
Closed
14 tasks
@cbodley
Copy link
Contributor Author

cbodley commented Dec 19, 2023

// Account resource index design:
//
// The resource_add/rm ops require entries to be indexed by name, while the
// resource_list op requires them to be indexed by path for filtering by
// path_prefix. To avoid indirection on listing, the path is used as the
// primary index.

i guess i was mistaken about this part after testing against aws in ceph/s3-tests#537. aws is returning users sorted by name, regardless of their paths

that implies that we shouldn't use PathPrefix to limit the range of keys we visit (by passing that prefix to cls_cxx_map_get_vals()). instead we need to visit the entire range of names, and filter entries by path after decoding their value

that certainly makes the cls_user logic simpler, but it means we'll have different behavior when listing roles in accounts vs. the per-tenant roles outside of accounts

@cbodley
Copy link
Contributor Author

cbodley commented Dec 19, 2023

i pushed updated versions of these commits to #54333 and see it passing these tests now

@cbodley cbodley closed this Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pritha-srivastava pritha-srivastava Awaiting requested review from pritha-srivastava

Assignees

No one assigned

Labels

build/ops needs-review rgw TESTED tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@cbodley