From 512e9304a3ce652b0aab9e77a524b4cbb2eda7aa Mon Sep 17 00:00:00 2001 From: Mark Kogan Date: Wed, 30 Jul 2025 12:54:19 +0000 Subject: [PATCH 01/20] rgw: add rate limit for LIST & DELETE ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add rate limiting specific to LIST ops, similar to the current rate-limiting (https://docs.ceph.com/en/latest/radosgw/admin/#rate-limit-management) Example usage: ``` ./bin/radosgw-admin ratelimit set --ratelimit-scope=user --uid= --max_list_ops=2 ./bin/radosgw-admin ratelimit set --ratelimit-scope=user --uid= --max_delete_ops=2 ./bin/radosgw-admin ratelimit enable --ratelimit-scope=user --uid= ./bin/radosgw-admin ratelimit get --ratelimit-scope=user --uid= { "user_ratelimit": { "max_read_ops": 0, "max_write_ops": 0, "max_list_ops": 2, "max_delete_ops": 2, "max_read_bytes": 0, "max_write_bytes": 0, "enabled": true } } pkill -9 radosgw ./bin/radosgw -c ./ceph.conf ... aws --endpoint-url 'http://0:8000' s3 mb s3://bkt aws --endpoint-url 'http://0:8000' s3 cp ./ceph.conf s3://bkt aws --endpoint-url http://0:8000 s3api list-objects-v2 --bucket bkt --prefix 'ceph.conf' --delimiter '/' { "Contents": [ { "Key": "ceph.conf", "LastModified": "2025-07-30T13:59:38+00:00", "ETag": "\"13d11d431ae290134562c019d9e40c0e\"", "Size": 32346, "StorageClass": "STANDARD" } ], "RequestCharged": null } aws --endpoint-url http://0:8000 s3api list-objects-v2 --bucket bkt --prefix 'ceph.conf' --delimiter '/' { "Contents": [ { "Key": "ceph.conf", "LastModified": "2025-07-30T13:59:38+00:00", "ETag": "\"13d11d431ae290134562c019d9e40c0e\"", "Size": 32346, "StorageClass": "STANDARD" } ], "RequestCharged": null } aws --endpoint-url http://0:8000 s3api list-objects-v2 --bucket bkt --prefix 'ceph.conf' --delimiter '/' argument of type 'NoneType' is not iterable tail -F ./out/radosgw.8000.log | grep beast ... beast: 0x7fffbbe09780: [30/Jul/2025:15:44:50.359 +0000] " GET /bkt?list-type=2&delimiter=%2F&prefix=ceph.conf&encoding-type=url HTTP/1.1" 200 535 - "aws-cli/2.15.31 Python/3.9.21 Linux/5.14.0-570.28.1.el9_6.x86_64 source/x86_64.rhel.9 prompt/off command/s3api.list-objects-v2" - latency=0.000999995s beast: 0x7fffbbe09780: [30/Jul/2025:15:44:53.904 +0000] " GET /bkt?list-type=2&delimiter=%2F&prefix=ceph.conf&encoding-type=url HTTP/1.1" 200 535 - "aws-cli/2.15.31 Python/3.9.21 Linux/5.14.0-570.28.1.el9_6.x86_64 source/x86_64.rhel.9 prompt/off command/s3api.list-objects-v2" - latency=0.000999995s vvv beast: 0x7fffbbe09780: [30/Jul/2025:15:44:58.192 +0000] " GET /bkt?list-type=2&delimiter=%2F&prefix=ceph.conf&encoding-type=url HTTP/1.1" 503 228 - "aws-cli/2.15.31 Python/3.9.21 Linux/5.14.0-570.28.1.el9_6.x86_64 source/x86_64.rhel.9 prompt/off command/s3api.list-objects-v2" - latency=0.000000000s beast: 0x7fffbbe09780: [30/Jul/2025:15:44:58.798 +0000] " GET /bkt?list-type=2&delimiter=%2F&prefix=ceph.conf&encoding-type=url HTTP/1.1" 503 228 - "aws-cli/2.15.31 Python/3.9.21 Linux/5.14.0-570.28.1.el9_6.x86_64 source/x86_64.rhel.9 prompt/off command/s3api.list-objects-v2" - latency=0.000999994s beast: 0x7fffbbe09780: [30/Jul/2025:15:44:59.807 +0000] " GET /bkt?list-type=2&delimiter=%2F&prefix=ceph.conf&encoding-type=url HTTP/1.1" 503 228 - "aws-cli/2.15.31 Python/3.9.21 Linux/5.14.0-570.28.1.el9_6.x86_64 source/x86_64.rhel.9 prompt/off command/s3api.list-objects-v2" - latency=0.000000000s s3cmd put ./ceph.conf s3://bkt/1 s3cmd put ./ceph.conf s3://bkt/2 s3cmd put ./ceph.conf s3://bkt/3 s3cmd rm s3://bkt/1 s3cmd rm s3://bkt/2 s3cmd rm s3://bkt/3 delete: 's3://bkt/1' delete: 's3://bkt/2' WARNING: Retrying failed request: /3 (503 (SlowDown)) WARNING: Waiting 3 sec... WARNING: Retrying failed request: /3 (503 (SlowDown)) ^^^ ``` Fixes: https://tracker.ceph.com/issues/72894 Signed-off-by: Mark Kogan Update PendingReleaseNotes Co-authored-by: Yuval Lifshitz Signed-off-by: Mark Kogan <31659604+mkogan1@users.noreply.github.com> Update PendingReleaseNotes Co-authored-by: Yuval Lifshitz Signed-off-by: Mark Kogan <31659604+mkogan1@users.noreply.github.com> (cherry picked from commit dae572d50080609c77d7131cfc99b1fb3f16d31b) Conflicts: src/common/options/rgw.yaml.in - Remove “rgw_redis_connection_pool_size” because tentacle doesn’t have 1ac08d30d1913fd99722689fa3cc1a0a5b73bd13 src/rgw/rgw_rest_ratelimit.cc - Removed "rgw_sal.h”, “rgw_sal_config.h” and imports because tentacle doesn’t have de9073218eef31aea29698e510c946e41e8b2c3a and 09383ab42ba0986bb1d942845115b4ef6f9184d6 --- PendingReleaseNotes | 7 + doc/radosgw/admin.rst | 52 +- doc/radosgw/adminops.rst | 29 +- qa/tasks/radosgw_admin_rest.py | 71 +++ src/common/options/rgw.yaml.in | 15 + src/rgw/radosgw-admin/radosgw-admin.cc | 116 ++-- src/rgw/rgw_common.cc | 2 + src/rgw/rgw_common.h | 24 +- src/rgw/rgw_process.cc | 16 +- src/rgw/rgw_ratelimit.h | 135 ++++- src/rgw/rgw_rest_ratelimit.cc | 38 ++ src/test/cli/radosgw-admin/help.t | 10 +- src/test/rgw/bench_rgw_ratelimit.cc | 2 +- src/test/rgw/bench_rgw_ratelimit_gc.cc | 2 +- src/test/rgw/test_rgw_ratelimit.cc | 747 +++++++++++++++++++++++-- 15 files changed, 1103 insertions(+), 163 deletions(-) diff --git a/PendingReleaseNotes b/PendingReleaseNotes index 6ad7159e9798..92cb4298c2ef 100644 --- a/PendingReleaseNotes +++ b/PendingReleaseNotes @@ -879,6 +879,13 @@ CephFS: Disallow delegating preallocated inode ranges to clients. Config * RGW: The serialized format of notification and topics has changed, so that new/updated topics will be unreadable by old RGWs. We recommend completing the RGW upgrades before creating or modifying any notification topics. +* RGW: New rate limiting options have been added to control the number of + bucket listing requests (`radosgw-admin ratelimit set ... --max-list-ops=`) + and delete operations (`radosgw-admin ratelimit set ... --max-delete-ops=`) + per accumulation interval, along with the ability to configure the + accumulation interval duration (`rgw_ratelimit_interval`). + Their default value is `0`, which keeps backward compatibility to the + previous behavior of READ and WRITE operations rate limiting. * RBD: Trailing newline in passphrase files (`` argument in `rbd encryption format` command and `--encryption-passphrase-file` option in other commands) is no longer stripped. diff --git a/doc/radosgw/admin.rst b/doc/radosgw/admin.rst index 1dd411fdcb07..007cacbdcb22 100644 --- a/doc/radosgw/admin.rst +++ b/doc/radosgw/admin.rst @@ -629,9 +629,21 @@ Rate Limit Management ===================== Quotas can be set for The Ceph Object Gateway on users and buckets. The "rate -limit" includes the maximum number of read operations (read ops) and write -operations (write ops) per minute as well as the number of bytes per minute -that can be written or read per user or per bucket. +limit" includes the maximum number of read operations and write operations +per accumulation interval as well as the number of bytes per accumulation interval +that can be written or read per user or per bucket. It also includes the maximum +number of list requests and delete operations per accumulation interval. +The accumulation interval is configured by the :confval:`rgw_ratelimit_interval` option. +The default value is 60 seconds. +(Note: S3 Multi-Object Delete operation are currently not supported by rate limiting) + +The configured limits should be divided by the number of active object gateways. For example, +if "user A" is to be be limited to 10 ops per minute and there are two object gateways in the cluster, +then the limit on "user A" should be 5 (10 ops per minute / 2 RGWs). +If the requests are not balanced between RGWs, the rate limit might be underutilized. +For example: if the ops limit is 5 and there are two RGWs, +but the Load Balancer sends load to only one of those RGWs, +the effective limit is 5 ops, because this limit is enforced per RGW. Read Requests and Write Requests -------------------------------- @@ -643,8 +655,8 @@ How Metrics Work Each object gateway tracks per-user metrics separately from bucket metrics. These metrics are not shared with other gateways. The configured limits should be divided by the number of active object gateways. For example, if "user A" is -to be be limited to 10 ops per minute and there are two object gateways in the -cluster, then the limit on "user A" should be ``5`` (10 ops per minute / 2 +to be be limited to 10 ops per accumulation interval and there are two object gateways in the +cluster, then the limit on "user A" should be ``5`` (10 ops per accumulation interval / 2 RGWs). If the requests are **not** balanced between RGWs, the rate limit might be underutilized. For example: if the ops limit is ``5`` and there are two RGWs, **but** the Load Balancer sends load to only one of those RGWs, the @@ -661,10 +673,10 @@ rate limit is reached during the execution of the request. The RGW keeps track of a "debt" consisting of bytes used in excess of the configured value; users or buckets that incur this kind of debt are prevented from sending more requests until the "debt" has been repaid. The maximum size of the "debt" is -twice the max-read/write-bytes per minute. If "user A" is subject to a 1-byte -read limit per minute and they attempt to ``GET`` an object that is 1 GB in size, +twice the max-read/write-bytes per accumulation interval. If "user A" is subject to a 1-byte +read limit per accumulation interval and they attempt to ``GET`` an object that is 1 GB in size, then the ``GET`` action will fail. After "user A" has completed this 1 GB -operation, RGW blocks the user's requests for up to two minutes. After this +operation, RGW blocks the user's requests for up to two accumulation intervals. After this time has elapsed, "user A" will be able to send ``GET`` requests again. @@ -675,19 +687,27 @@ time has elapsed, "user A" will be able to send ``GET`` requests again. user. - **Maximum Read Ops:** The ``--max-read-ops`` setting allows you to limit read - bytes per minute per RGW instance. A ``0`` value disables throttling. + bytes per accumulation interval per RGW instance. A ``0`` value disables throttling. - **Maximum Read Bytes:** The ``--max-read-bytes`` setting allows you to limit - read bytes per minute per RGW instance. A ``0`` value disables throttling. + read bytes per accumulation interval per RGW instance. A ``0`` value disables throttling. - **Maximum Write Ops:** The ``--max-write-ops`` setting allows you to specify - the maximum number of write ops per minute per RGW instance. A ``0`` value + the maximum number of write ops per accumulation interval per RGW instance. A ``0`` value disables throttling. - **Maximum Write Bytes:** The ``--max-write-bytes`` setting allows you to - specify the maximum number of write bytes per minute per RGW instance. A + specify the maximum number of write bytes per accumulation interval per RGW instance. A ``0`` value disables throttling. - + +- **Maximum List Ops:** The ``--max-list-ops`` setting allows you to + specify the maximum number of bucket listing requests per accumulation interval per RGW instance. + A ``0`` value disables throttling. + +- **Maximum Delete Ops:** The ``--max-delete-ops`` setting allows you to + specify the maximum number of delete operations per accumulation interval per RGW instance. + A ``0`` value disables throttling. + - **Rate Limit Scope:** The ``--ratelimit-scope`` option sets the scope for the rate limit. The options are ``bucket`` , ``user`` and ``anonymous``. Bucket rate limit apply to buckets. The user rate limit applies to a user. The @@ -706,7 +726,8 @@ parameters: radosgw-admin ratelimit set --ratelimit-scope=user --uid= \ <[--max-read-ops=] [--max-read-bytes=] \ - [--max-write-ops=] [--max-write-bytes=]> + [--max-write-ops=] [--max-write-bytes=] \ + [--max-list-ops=] [--max-delete-ops=]> An example of using ``radosgw-admin ratelimit set`` to set a rate limit might look like this: @@ -771,7 +792,8 @@ The following is the general form of commands that set rate limit parameters: radosgw-admin ratelimit set --ratelimit-scope=bucket --bucket= \ <[--max-read-ops=] [--max-read-bytes=] \ - [--max-write-ops=] [--max-write-bytes=]> + [--max-write-ops=] [--max-write-bytes=] \ + [--max-list-ops=] [--max-delete-ops=]> An example of using ``radosgw-admin ratelimit set`` to set a rate limit for a bucket might look like this: diff --git a/doc/radosgw/adminops.rst b/doc/radosgw/adminops.rst index 0433b2bb104c..569591586d4d 100644 --- a/doc/radosgw/adminops.rst +++ b/doc/radosgw/adminops.rst @@ -2788,8 +2788,9 @@ permission. :: Rate Limit ========== -The Admin Operations API enables you to set and get ratelimit configurations on users and on bucket and global rate limit configurations. See `Rate Limit Management`_ for additional details. -Rate Limit includes the maximum number of operations and/or bytes per minute, separated by read and/or write, to a bucket and/or by a user and the maximum storage size in megabytes. +The Admin Operations API enables you to set and get ratelimit configurations on users and on bucket and global rate limit configurations. See `Rate Limit Management`_ for additional details. +Rate Limit includes the maximum number of operations and/or bytes per accumulation interval, separated by read and/or write (Additionally list and get operations), +to a bucket and/or by a user and the maximum storage size in megabytes. To view rate limit, the user must have a ``ratelimit=read`` capability. To set, modify or disable a ratelimit, the user must have ``ratelimit=write`` capability. @@ -2803,16 +2804,22 @@ Valid parameters for quotas include: - **User:** The ``uid`` option allows you to specify a rate limit for a user. - **Maximum Read Bytes:** The ``max-read-bytes`` setting allows you to specify - the maximum number of read bytes per minute. A 0 value disables this setting. + the maximum number of read bytes per accumulation interval. A 0 value disables this setting. - **Maximum Write Bytes:** The ``max-write-bytes`` setting allows you to specify - the maximum number of write bytes per minute. A 0 value disables this setting. + the maximum number of write bytes per accumulation interval. A 0 value disables this setting. - **Maximum Read Ops:** The ``max-read-ops`` setting allows you to specify - the maximum number of read ops per minute. A 0 value disables this setting. + the maximum number of read ops per accumulation interval. A 0 value disables this setting. - **Maximum Write Ops:** The ``max-write-ops`` setting allows you to specify - the maximum number of write ops per minute. A 0 value disables this setting. + the maximum number of write ops per accumulation interval. A 0 value disables this setting. + +- **Maximum List Ops:** The ``max-list-ops`` setting allows you to specify + the maximum number of bucket listing requests per accumulation interval. A 0 value disables this setting. + +- **Maximum Delete Ops:** The ``max-delete-ops`` setting allows you to specify + the maximum number of delete operations per accumulation interval. A 0 value disables throttling. - **Global:** The ``global`` option allows you to specify a global rate limit. The value should be either 'True' or 'False'. @@ -2839,7 +2846,7 @@ Set User Rate Limit To set a rate limit, the user must have ``ratelimit`` capability set with ``write`` permission. :: - POST /{admin}/ratelimit?ratelimit-scope=user&uid=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][enabled=]> + POST /{admin}/ratelimit?ratelimit-scope=user&uid=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][&max-list-ops=][&max-delete-ops=][&enabled=]> @@ -2859,7 +2866,7 @@ Set Rate Limit for an Individual Bucket To set a rate limit, the user must have ``ratelimit`` capability set with ``write`` permission. :: - POST /{admin}/ratelimit?bucket=&ratelimit-scope=bucket<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=]> + POST /{admin}/ratelimit?bucket=&ratelimit-scope=bucket<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][&max-list-ops=][&max-delete-ops=][&enabled=]> @@ -2879,7 +2886,7 @@ Set Global User Rate Limit To set a rate limit, the user must have ``ratelimit`` capability set with ``write`` permission. :: - POST /{admin}/ratelimit?ratelimit-scope=user&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][enabled=]> + POST /{admin}/ratelimit?ratelimit-scope=user&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][&max-list-ops=][&max-delete-ops=][&enabled=]> @@ -2889,7 +2896,7 @@ Set Global Rate Limit Bucket To set a rate limit, the user must have ``ratelimit`` capability set with ``write`` permission. :: - POST /{admin}/ratelimit?ratelimit-scope=bucket&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=]> + POST /{admin}/ratelimit?ratelimit-scope=bucket&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][&max-list-ops=][&max-delete-ops=][&enabled=]> @@ -2899,7 +2906,7 @@ Set Global Anonymous User Rate Limit To set a rate limit, the user must have ``ratelimit`` capability set with ``write`` permission. :: - POST /{admin}/ratelimit?ratelimit-scope=anon&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][enabled=]> + POST /{admin}/ratelimit?ratelimit-scope=anon&global=<[&max-read-bytes=][&max-write-bytes=][&max-read-ops=][&max-write-ops=][&max-list-ops=][&max-delete-ops=][&enabled=]> diff --git a/qa/tasks/radosgw_admin_rest.py b/qa/tasks/radosgw_admin_rest.py index 3a64604d5f5d..d5447289127c 100644 --- a/qa/tasks/radosgw_admin_rest.py +++ b/qa/tasks/radosgw_admin_rest.py @@ -1072,3 +1072,74 @@ def task(ctx, config): # checking remove account access (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['account', 'rm'], {'id' : account_id}) assert ret == 200 + + # TESTCASE 'ratelimit' 'user' 'modify' 'max-list-ops = 100' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-list-ops' : '100'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + assert ret == 200 + user_ratelimit = out['user_ratelimit'] + assert user_ratelimit['max_list_ops'] == 100 + + # TESTCASE 'ratelimit' 'user' 'modify' 'max-delete-ops = 50' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-delete-ops' : '50'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + assert ret == 200 + user_ratelimit = out['user_ratelimit'] + assert user_ratelimit['max_delete_ops'] == 50 + + # TESTCASE 'ratelimit' 'bucket' 'modify' 'max-list-ops = 200' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-list-ops' : '200'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) + assert ret == 200 + bucket_ratelimit = out['bucket_ratelimit'] + assert bucket_ratelimit['max_list_ops'] == 200 + + # TESTCASE 'ratelimit' 'bucket' 'modify' 'max-delete-ops = 75' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-delete-ops' : '75'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) + assert ret == 200 + bucket_ratelimit = out['bucket_ratelimit'] + assert bucket_ratelimit['max_delete_ops'] == 75 + + # TESTCASE 'ratelimit' 'global' 'modify' 'bucket' 'max-list-ops = 500' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-list-ops' : '500'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'global' : 'true'}) + assert ret == 200 + # Check that global bucket ratelimit has the list ops set + assert 'bucket_ratelimit' in out + assert out['bucket_ratelimit']['max_list_ops'] == 500 + + # TESTCASE 'ratelimit' 'global' 'modify' 'bucket' 'max-delete-ops = 300' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-delete-ops' : '300'}) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'global' : 'true'}) + assert ret == 200 + # Check that global bucket ratelimit has the delete ops set + assert 'bucket_ratelimit' in out + assert out['bucket_ratelimit']['max_delete_ops'] == 300 + + # TESTCASE 'ratelimit' 'user' 'modify' 'multiple ops at once' 'succeeds' + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], { + 'ratelimit-scope' : 'user', + 'uid' : ratelimit_user, + 'max-read-ops' : '1000', + 'max-write-ops' : '800', + 'max-list-ops' : '600', + 'max-delete-ops' : '400', + 'enabled' : 'true' + }) + assert ret == 200 + (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + assert ret == 200 + user_ratelimit = out['user_ratelimit'] + assert user_ratelimit['enabled'] == True + assert user_ratelimit['max_read_ops'] == 1000 + assert user_ratelimit['max_write_ops'] == 800 + assert user_ratelimit['max_list_ops'] == 600 + assert user_ratelimit['max_delete_ops'] == 400 + diff --git a/src/common/options/rgw.yaml.in b/src/common/options/rgw.yaml.in index c43de92d06e6..34d1519812ef 100644 --- a/src/common/options/rgw.yaml.in +++ b/src/common/options/rgw.yaml.in @@ -4472,3 +4472,18 @@ options: flags: - runtime with_legacy: true + +- name: rgw_ratelimit_interval + type: uint + level: advanced + desc: Time window for rate limiting in seconds + long_desc: This option sets the time window for rate limiting accumulation in seconds. + Requests that exceed the configured rate limits within this time window will be rejected. + The default is a 60 second token bucket. + default: 60 + min: 1 + services: + - rgw + flags: + - startup + with_legacy: true diff --git a/src/rgw/radosgw-admin/radosgw-admin.cc b/src/rgw/radosgw-admin/radosgw-admin.cc index fae5885bcc37..fc39785976c2 100644 --- a/src/rgw/radosgw-admin/radosgw-admin.cc +++ b/src/rgw/radosgw-admin/radosgw-admin.cc @@ -495,10 +495,12 @@ void usage() cout << " --max-size specify max size (in B/K/M/G/T, negative value to disable)\n"; cout << " --quota-scope scope of quota (bucket, user, account)\n"; cout << "\nRate limiting options:\n"; - cout << " --max-read-ops specify max requests per minute for READ ops per RGW (GET and HEAD request methods), 0 means unlimited\n"; - cout << " --max-read-bytes specify max bytes per minute for READ ops per RGW (GET and HEAD request methods), 0 means unlimited\n"; - cout << " --max-write-ops specify max requests per minute for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited\n"; - cout << " --max-write-bytes specify max bytes per minute for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited\n"; + cout << " --max-read-ops specify max requests per accumulation interval for READ ops per RGW (GET and HEAD request methods), 0 means unlimited\n"; + cout << " --max-read-bytes specify max bytes per accumulation interval for READ ops per RGW (GET and HEAD request methods), 0 means unlimited\n"; + cout << " --max-write-ops specify max requests per accumulation interval for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited\n"; + cout << " --max-write-bytes specify max bytes per accumulation interval for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited\n"; + cout << " --max-list-ops specify max requests per accumulation interval for bucket listing requests per RGW, 0 means unlimited\n"; + cout << " --max-delete-ops specify max requests per accumulation interval for DELETE ops per RGW (DELETE request methods), 0 means unlimited\n"; cout << " --ratelimit-scope scope of rate limiting: bucket, user, anonymous\n"; cout << " anonymous can be configured only with global rate limit\n"; cout << "\nOrphans search options:\n"; @@ -1415,9 +1417,9 @@ static bool dump_string(const char *field_name, bufferlist& bl, Formatter *f) } bool set_ratelimit_info(RGWRateLimitInfo& ratelimit, OPT opt_cmd, int64_t max_read_ops, int64_t max_write_ops, - int64_t max_read_bytes, int64_t max_write_bytes, - bool have_max_read_ops, bool have_max_write_ops, - bool have_max_read_bytes, bool have_max_write_bytes) + int64_t max_list_ops, int64_t max_delete_ops, int64_t max_read_bytes, int64_t max_write_bytes, + bool have_max_read_ops, bool have_max_write_ops, bool have_max_list_ops, + bool have_max_delete_ops, bool have_max_read_bytes, bool have_max_write_bytes) { bool ratelimit_configured = true; switch (opt_cmd) { @@ -1441,6 +1443,18 @@ bool set_ratelimit_info(RGWRateLimitInfo& ratelimit, OPT opt_cmd, int64_t max_re ratelimit_configured = true; } } + if (have_max_list_ops) { + if (max_list_ops >= 0) { + ratelimit.max_list_ops = max_list_ops; + ratelimit_configured = true; + } + } + if (have_max_delete_ops) { + if (max_delete_ops >= 0) { + ratelimit.max_delete_ops = max_delete_ops; + ratelimit_configured = true; + } + } if (have_max_read_bytes) { if (max_read_bytes >= 0) { ratelimit.max_read_bytes = max_read_bytes; @@ -1525,10 +1539,10 @@ int set_bucket_quota(rgw::sal::Driver* driver, OPT opt_cmd, int set_bucket_ratelimit(rgw::sal::Driver* driver, OPT opt_cmd, const string& tenant_name, const string& bucket_name, - int64_t max_read_ops, int64_t max_write_ops, - int64_t max_read_bytes, int64_t max_write_bytes, - bool have_max_read_ops, bool have_max_write_ops, - bool have_max_read_bytes, bool have_max_write_bytes) + int64_t max_read_ops, int64_t max_write_ops, int64_t max_list_ops, + int64_t max_delete_ops, int64_t max_read_bytes, int64_t max_write_bytes, + bool have_max_read_ops, bool have_max_write_ops, bool have_max_list_ops, + bool have_max_delete_ops, bool have_max_read_bytes, bool have_max_write_bytes) { std::unique_ptr bucket; int r = driver->load_bucket(dpp(), rgw_bucket(tenant_name, bucket_name), @@ -1549,10 +1563,10 @@ int set_bucket_ratelimit(rgw::sal::Driver* driver, OPT opt_cmd, return -EIO; } } - bool ratelimit_configured = set_ratelimit_info(ratelimit_info, opt_cmd, max_read_ops, max_write_ops, - max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, - have_max_read_bytes, have_max_write_bytes); + bool ratelimit_configured = set_ratelimit_info(ratelimit_info, opt_cmd, max_read_ops, max_write_ops, max_list_ops, + max_delete_ops, max_read_bytes, max_write_bytes, + have_max_read_ops, have_max_write_ops, have_max_list_ops, + have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); if (!ratelimit_configured) { ldpp_dout(dpp(), 0) << "ERROR: no rate limit values have been specified" << dendl; return -EINVAL; @@ -1570,10 +1584,10 @@ int set_bucket_ratelimit(rgw::sal::Driver* driver, OPT opt_cmd, } int set_user_ratelimit(OPT opt_cmd, std::unique_ptr& user, - int64_t max_read_ops, int64_t max_write_ops, - int64_t max_read_bytes, int64_t max_write_bytes, - bool have_max_read_ops, bool have_max_write_ops, - bool have_max_read_bytes, bool have_max_write_bytes) + int64_t max_read_ops, int64_t max_write_ops, int64_t max_list_ops, + int64_t max_delete_ops, int64_t max_read_bytes, int64_t max_write_bytes, + bool have_max_read_ops, bool have_max_write_ops, bool have_max_list_ops, + bool have_max_delete_ops, bool have_max_read_bytes, bool have_max_write_bytes) { RGWRateLimitInfo ratelimit_info; user->load_user(dpp(), null_yield); @@ -1588,10 +1602,10 @@ int set_user_ratelimit(OPT opt_cmd, std::unique_ptr& user, return -EIO; } } - bool ratelimit_configured = set_ratelimit_info(ratelimit_info, opt_cmd, max_read_ops, max_write_ops, - max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, - have_max_read_bytes, have_max_write_bytes); + bool ratelimit_configured = set_ratelimit_info(ratelimit_info, opt_cmd, max_read_ops, max_write_ops, max_list_ops, + max_delete_ops, max_read_bytes, max_write_bytes, + have_max_read_ops, have_max_write_ops, have_max_list_ops, + have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); if (!ratelimit_configured) { ldpp_dout(dpp(), 0) << "ERROR: no rate limit values have been specified" << dendl; return -EINVAL; @@ -1622,7 +1636,10 @@ int show_user_ratelimit(std::unique_ptr& user, Formatter *format ldpp_dout(dpp(), 0) << "ERROR: failed to decode rate limit" << dendl; return -EIO; } + } else { + return -ENOENT; } + formatter->open_object_section("user_ratelimit"); encode_json("user_ratelimit", ratelimit_info, formatter); formatter->close_section(); @@ -3689,12 +3706,16 @@ int main(int argc, const char **argv) int64_t max_size = -1; int64_t max_read_ops = 0; int64_t max_write_ops = 0; + int64_t max_list_ops = 0; + int64_t max_delete_ops = 0; int64_t max_read_bytes = 0; int64_t max_write_bytes = 0; bool have_max_objects = false; bool have_max_size = false; bool have_max_write_ops = false; bool have_max_read_ops = false; + bool have_max_list_ops = false; + bool have_max_delete_ops = false; bool have_max_write_bytes = false; bool have_max_read_bytes = false; int include_all = false; @@ -3984,6 +4005,20 @@ int main(int argc, const char **argv) return EINVAL; } have_max_read_ops = true; + } else if (ceph_argparse_witharg(args, i, &val, "--max-list-ops", (char*)NULL)) { + max_list_ops = (int64_t)strict_strtoll(val.c_str(), 10, &err); + if (!err.empty()) { + cerr << "ERROR: failed to parse max list requests: " << err << std::endl; + return EINVAL; + } + have_max_list_ops = true; + } else if (ceph_argparse_witharg(args, i, &val, "--max-delete-ops", (char*)NULL)) { + max_delete_ops = (int64_t)strict_strtoll(val.c_str(), 10, &err); + if (!err.empty()) { + cerr << "ERROR: failed to parse max delete requests: " << err << std::endl; + return EINVAL; + } + have_max_delete_ops = true; } else if (ceph_argparse_witharg(args, i, &val, "--max-write-ops", (char*)NULL)) { max_write_ops = (int64_t)strict_strtoll(val.c_str(), 10, &err); if (!err.empty()) { @@ -4972,24 +5007,24 @@ int main(int argc, const char **argv) formatter->open_object_section("period_config"); if (ratelimit_scope == "bucket") { ratelimit_configured = set_ratelimit_info(period_config.bucket_ratelimit, opt_cmd, - max_read_ops, max_write_ops, + max_read_ops, max_write_ops, max_list_ops, max_delete_ops, max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, - have_max_read_bytes, have_max_write_bytes); + have_max_read_ops, have_max_write_ops, have_max_list_ops, + have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); encode_json("bucket_ratelimit", period_config.bucket_ratelimit, formatter.get()); } else if (ratelimit_scope == "user") { ratelimit_configured = set_ratelimit_info(period_config.user_ratelimit, opt_cmd, - max_read_ops, max_write_ops, + max_read_ops, max_write_ops, max_list_ops, max_delete_ops, max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, - have_max_read_bytes, have_max_write_bytes); + have_max_read_ops, have_max_write_ops, have_max_list_ops, + have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); encode_json("user_ratelimit", period_config.user_ratelimit, formatter.get()); } else if (ratelimit_scope == "anonymous") { ratelimit_configured = set_ratelimit_info(period_config.anon_ratelimit, opt_cmd, - max_read_ops, max_write_ops, + max_read_ops, max_write_ops, max_list_ops,max_delete_ops, max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, - have_max_read_bytes, have_max_write_bytes); + have_max_read_ops, have_max_write_ops, have_max_list_ops, + have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); encode_json("anonymous_ratelimit", period_config.anon_ratelimit, formatter.get()); } else if (ratelimit_scope.empty() && opt_cmd == OPT::GLOBAL_RATELIMIT_GET) { // if no scope is given for GET, print both @@ -6726,7 +6761,8 @@ int main(int argc, const char **argv) OPT::ROLE_CREATE, OPT::ROLE_DELETE, OPT::ROLE_POLICY_PUT, OPT::ROLE_POLICY_DELETE, OPT::ROLE_POLICY_ATTACH, OPT::ROLE_POLICY_DETACH, - OPT::USER_POLICY_ATTACH, OPT::USER_POLICY_DETACH}; + OPT::USER_POLICY_ATTACH, OPT::USER_POLICY_DETACH, + OPT::RATELIMIT_SET, OPT::RATELIMIT_ENABLE, OPT::RATELIMIT_DISABLE}; bool print_warning_message = (non_master_ops_list.find(opt_cmd) != non_master_ops_list.end() && non_master_cmd); @@ -11340,15 +11376,15 @@ int main(int argc, const char **argv) return EINVAL; } return set_bucket_ratelimit(driver, opt_cmd, tenant, bucket_name, - max_read_ops, max_write_ops, + max_read_ops, max_write_ops, max_list_ops, max_delete_ops, max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, + have_max_read_ops, have_max_write_ops, have_max_list_ops, have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); } else if (!rgw::sal::User::empty(user)) { if (ratelimit_scope == "user") { - return set_user_ratelimit(opt_cmd, user, max_read_ops, max_write_ops, + return set_user_ratelimit(opt_cmd, user, max_read_ops, max_write_ops, max_list_ops, max_delete_ops, max_read_bytes, max_write_bytes, - have_max_read_ops, have_max_write_ops, + have_max_read_ops, have_max_write_ops, have_max_list_ops, have_max_delete_ops, have_max_read_bytes, have_max_write_bytes); } else { cerr << "ERROR: invalid ratelimit scope specification. Please specify either --ratelimit-scope=bucket, or --ratelimit-scope=user" << std::endl; @@ -11371,7 +11407,11 @@ int main(int argc, const char **argv) return show_bucket_ratelimit(driver, tenant, bucket_name, formatter.get()); } else if (!rgw::sal::User::empty(user)) { if (ratelimit_scope == "user") { - return show_user_ratelimit(user, formatter.get()); + int ret = show_user_ratelimit(user, formatter.get()); + if (ret < 0) { + std::cerr << "ERROR: failed to get a ratelimit for user id: '" << user->get_id() << "', errno: " << cpp_strerror(-ret) << std::endl; + } + return ret; } else { cerr << "ERROR: invalid ratelimit scope specification. Please specify either --ratelimit-scope=bucket, or --ratelimit-scope=user" << std::endl; return EINVAL; diff --git a/src/rgw/rgw_common.cc b/src/rgw/rgw_common.cc index e4c9dda3a5cf..4599d45f3426 100644 --- a/src/rgw/rgw_common.cc +++ b/src/rgw/rgw_common.cc @@ -2829,6 +2829,8 @@ void RGWRateLimitInfo::dump(Formatter *f) const { f->dump_int("max_read_ops", max_read_ops); f->dump_int("max_write_ops", max_write_ops); + f->dump_int("max_list_ops", max_list_ops); + f->dump_int("max_delete_ops", max_delete_ops); f->dump_int("max_read_bytes", max_read_bytes); f->dump_int("max_write_bytes", max_write_bytes); f->dump_bool("enabled", enabled); diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index b213260013d9..f6771a7cb8d2 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -568,26 +568,40 @@ class RateLimiter; struct RGWRateLimitInfo { int64_t max_write_ops; int64_t max_read_ops; + int64_t max_list_ops; + int64_t max_delete_ops; int64_t max_write_bytes; int64_t max_read_bytes; bool enabled = false; RGWRateLimitInfo() - : max_write_ops(0), max_read_ops(0), max_write_bytes(0), max_read_bytes(0) {} + : max_write_ops(0), max_read_ops(0), max_list_ops(0), max_delete_ops(0), max_write_bytes(0), max_read_bytes(0) {} void encode(bufferlist& bl) const { - ENCODE_START(1, 1, bl); + ENCODE_START(2, 1, bl); encode(max_write_ops, bl); encode(max_read_ops, bl); + encode(max_list_ops, bl); + encode(max_delete_ops, bl); encode(max_write_bytes, bl); encode(max_read_bytes, bl); encode(enabled, bl); ENCODE_FINISH(bl); } void decode(bufferlist::const_iterator& bl) { - DECODE_START(1, bl); - decode(max_write_ops,bl); + DECODE_START(2, bl); + decode(max_write_ops, bl); decode(max_read_ops, bl); - decode(max_write_bytes,bl); + if (struct_v >= 2) { + decode(max_list_ops, bl); + } else { + max_list_ops = 0; + } + if (struct_v >= 2) { + decode(max_delete_ops, bl); + } else { + max_delete_ops = 0; + } + decode(max_write_bytes, bl); decode(max_read_bytes, bl); decode(enabled, bl); DECODE_FINISH(bl); diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc index 74ef704b541c..53c9f2a407bf 100644 --- a/src/rgw/rgw_process.cc +++ b/src/rgw/rgw_process.cc @@ -112,6 +112,14 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { s->ratelimit_bucket_marker = bucketfind; const char *method = s->info.method; + bool is_sts_user = (s->auth.identity && s->auth.identity->get_identity_type() == TYPE_ROLE); + if (is_sts_user) { + ldpp_dout(s, 21) << "STS user detected: uid=" << std::quoted(s->user->get_id().to_str()) << dendl; + auto op_ret = s->user->read_attrs(s, s->yield); + if (op_ret < 0) { + ldpp_dout(s, 0) << "checking rate_limit: uid=" << std::quoted(s->user->get_id().to_str()) << " failed to read user attrs" << dendl; + } + } auto iter = s->user->get_attrs().find(RGW_ATTR_RATELIMIT); if(iter != s->user->get_attrs().end()) { try { @@ -126,12 +134,14 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { ldpp_dout(s, 0) << "ERROR: failed to decode rate limit" << dendl; return -EIO; } + } else { + ldpp_dout(s, 21) << "checking rate_limit: uid=" << std::quoted(s->user->get_id().to_str()) << " does not have a rate limit attribute" << dendl; } if (s->user->get_id().id == RGW_USER_ANON_ID && global_anon.enabled) { *user_ratelimit = global_anon; } bool limit_bucket = false; - bool limit_user = s->ratelimit_data->should_rate_limit(method, s->ratelimit_user_name, s->time, user_ratelimit); + bool limit_user = s->ratelimit_data->should_rate_limit(method, s->ratelimit_user_name, s->time, user_ratelimit, s->info.request_params); if(!rgw::sal::Bucket::empty(s->bucket.get())) { @@ -151,11 +161,11 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { } } if (!limit_user) { - limit_bucket = s->ratelimit_data->should_rate_limit(method, s->ratelimit_bucket_marker, s->time, bucket_ratelimit); + limit_bucket = s->ratelimit_data->should_rate_limit(method, s->ratelimit_bucket_marker, s->time, bucket_ratelimit, s->info.request_params); } } if(limit_bucket && !limit_user) { - s->ratelimit_data->giveback_tokens(method, s->ratelimit_user_name); + s->ratelimit_data->giveback_tokens(method, s->ratelimit_user_name, s->info.request_params, user_ratelimit); } s->user_ratelimit = *user_ratelimit; s->bucket_ratelimit = *bucket_ratelimit; diff --git a/src/rgw/rgw_ratelimit.h b/src/rgw/rgw_ratelimit.h index 224312bb0d78..3da243063073 100644 --- a/src/rgw/rgw_ratelimit.h +++ b/src/rgw/rgw_ratelimit.h @@ -5,6 +5,8 @@ #include #include "rgw_common.h" +enum class OpType { Read, Write, List, Delete }; + class RateLimiterEntry { /* @@ -21,6 +23,8 @@ class RateLimiterEntry { }; counters read; counters write; + counters list; + counters del; ceph::timespan ts; bool first_run = true; std::mutex ts_lock; @@ -33,6 +37,14 @@ class RateLimiterEntry { { return write.ops / fixed_point_rgw_ratelimit; } + int64_t list_ops() const + { + return list.ops / fixed_point_rgw_ratelimit; + } + int64_t delete_ops() const + { + return del.ops / fixed_point_rgw_ratelimit; + } int64_t read_bytes() const { return read.bytes / fixed_point_rgw_ratelimit; @@ -52,6 +64,22 @@ class RateLimiterEntry { read.ops -= fixed_point_rgw_ratelimit; return false; } + bool should_rate_limit_list(int64_t ops_limit) + { + if ((list_ops() - 1 < 0) && (ops_limit > 0)) { + return true; + } + list.ops -= fixed_point_rgw_ratelimit; + return false; + } + bool should_rate_limit_delete(int64_t ops_limit) + { + if ((delete_ops() - 1 < 0) && (ops_limit > 0)) { + return true; + } + del.ops -= fixed_point_rgw_ratelimit; + return false; + } bool should_rate_limit_write(int64_t ops_limit, int64_t bw_limit) { //check if tenants did not reach their bw or ops limits and that the limits are not 0 (which is unlimited) @@ -72,7 +100,8 @@ class RateLimiterEntry { bool minimum_time_reached(ceph::timespan curr_timestamp) const { using namespace std::chrono; - constexpr auto min_duration = duration_cast(seconds(60)) / fixed_point_rgw_ratelimit; + const size_t accumulation_interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const auto min_duration = duration_cast(seconds(accumulation_interval)) / fixed_point_rgw_ratelimit; const auto delta = curr_timestamp - ts; if (delta < min_duration) { @@ -91,35 +120,48 @@ class RateLimiterEntry { write.bytes = info->max_write_bytes * fixed_point; read.ops = info->max_read_ops * fixed_point; read.bytes = info->max_read_bytes * fixed_point; + list.ops = info->max_list_ops * fixed_point; + del.ops = info->max_delete_ops * fixed_point; ts = curr_timestamp; first_run = false; return; } else if(curr_timestamp > ts && minimum_time_reached(curr_timestamp)) { - const int64_t time_in_ms = std::chrono::duration_cast(curr_timestamp - ts).count() / 60.0 / std::milli::den * fixed_point; // / 60 to make it work with 1 min token bucket + const size_t accumulation_interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const int64_t time_in_ms = std::chrono::duration_cast(curr_timestamp - ts).count() / static_cast(accumulation_interval) / std::milli::den * fixed_point; ts = curr_timestamp; const int64_t write_ops = info->max_write_ops * time_in_ms; const int64_t write_bw = info->max_write_bytes * time_in_ms; const int64_t read_ops = info->max_read_ops * time_in_ms; const int64_t read_bw = info->max_read_bytes * time_in_ms; + const int64_t list_ops = info->max_list_ops * time_in_ms; + const int64_t delete_ops = info->max_delete_ops * time_in_ms; read.ops = std::min(info->max_read_ops * fixed_point, read_ops + read.ops); read.bytes = std::min(info->max_read_bytes * fixed_point, read_bw + read.bytes); write.ops = std::min(info->max_write_ops * fixed_point, write_ops + write.ops); write.bytes = std::min(info->max_write_bytes * fixed_point, write_bw + write.bytes); + list.ops = std::min(info->max_list_ops * fixed_point, list_ops + list.ops); + del.ops = std::min(info->max_delete_ops * fixed_point, delete_ops + del.ops); } } public: - bool should_rate_limit(bool is_read, const RGWRateLimitInfo* ratelimit_info, ceph::timespan curr_timestamp) + bool should_rate_limit(OpType op_type, const RGWRateLimitInfo* ratelimit_info, ceph::timespan curr_timestamp) { std::unique_lock lock(ts_lock); increase_tokens(curr_timestamp, ratelimit_info); - if (is_read) - { - return should_rate_limit_read(ratelimit_info->max_read_ops, ratelimit_info->max_read_bytes); + switch (op_type) { + case OpType::Read: + return should_rate_limit_read(ratelimit_info->max_read_ops, ratelimit_info->max_read_bytes); + case OpType::Write: + return should_rate_limit_write(ratelimit_info->max_write_ops, ratelimit_info->max_write_bytes); + case OpType::List: + return should_rate_limit_list(ratelimit_info->max_list_ops); + case OpType::Delete: + return should_rate_limit_delete(ratelimit_info->max_delete_ops); } - return should_rate_limit_write(ratelimit_info->max_write_ops, ratelimit_info->max_write_bytes); + return false; } void decrease_bytes(bool is_read, int64_t amount, const RGWRateLimitInfo* info) { std::unique_lock lock(ts_lock); @@ -131,19 +173,27 @@ class RateLimiterEntry { write.bytes = std::max(write.bytes - amount * fixed_point_rgw_ratelimit,info->max_write_bytes * fixed_point_rgw_ratelimit * -2); } } - void giveback_tokens(bool is_read) + void giveback_tokens(OpType op_type) { std::unique_lock lock(ts_lock); - if (is_read) - { - read.ops += fixed_point_rgw_ratelimit; - } else { - write.ops += fixed_point_rgw_ratelimit; + switch (op_type) { + case OpType::Read: + read.ops += fixed_point_rgw_ratelimit; + break; + case OpType::Write: + write.ops += fixed_point_rgw_ratelimit; + break; + case OpType::List: + list.ops += fixed_point_rgw_ratelimit; + break; + case OpType::Delete: + del.ops += fixed_point_rgw_ratelimit; + break; } } }; -class RateLimiter { +class RateLimiter : public DoutPrefix { static constexpr size_t map_size = 2000000; // will create it with the closest upper prime number std::shared_mutex insert_lock; @@ -151,12 +201,31 @@ class RateLimiter { std::condition_variable& cv; typedef std::unordered_map hash_map; hash_map ratelimit_entries{map_size}; - static bool is_read_op(const std::string_view method) { + + static inline constexpr std::string_view RESOURCE_PATTERN_LIST_TYPE = "list-type="; + static inline constexpr std::string_view RESOURCE_PATTERN_PREFIX = "prefix="; + static inline constexpr std::string_view RESOURCE_PATTERN_DELIMITER = "delimiter="; + + + static OpType op_type(const std::string_view method, const std::string_view resource, const RGWRateLimitInfo* ratelimit_info) { + const bool ratelimit_list = ratelimit_info && (ratelimit_info->max_list_ops > 0); + const bool ratelimit_delete = ratelimit_info && (ratelimit_info->max_delete_ops > 0); + + auto contains_any = [](std::string_view s, auto&&... patterns) { + return ((s.find(patterns) != std::string::npos) || ...); + }; + if (ratelimit_list && method == "GET" && + !resource.empty() && contains_any(resource, RESOURCE_PATTERN_LIST_TYPE, RESOURCE_PATTERN_PREFIX, RESOURCE_PATTERN_DELIMITER)) { + return OpType::List; + } if (method == "GET" || method == "HEAD") { - return true; + return OpType::Read; } - return false; + if (ratelimit_delete && method == "DELETE") { + return OpType::Delete; + } + return OpType::Write; } // find or create an entry, and return its iterator @@ -187,41 +256,47 @@ class RateLimiter { RateLimiter(RateLimiter&&) = delete; RateLimiter& operator =(RateLimiter&&) = delete; RateLimiter() = delete; - RateLimiter(std::atomic_bool& replacing, std::condition_variable& cv) - : replacing(replacing), cv(cv) + RateLimiter(CephContext* cct, std::atomic_bool& replacing, std::condition_variable& cv) + : DoutPrefix(cct, ceph_subsys_rgw, "rate limiter: "), replacing(replacing), cv(cv) { // prevents rehash, so no iterators invalidation ratelimit_entries.max_load_factor(1000); }; - bool should_rate_limit(const char *method, const std::string& key, ceph::coarse_real_time curr_timestamp, const RGWRateLimitInfo* ratelimit_info) { + bool should_rate_limit(const char *method, const std::string& key, ceph::coarse_real_time curr_timestamp, const RGWRateLimitInfo* ratelimit_info, const std::string& resource) { + ldpp_dout(this, 21) << "checking should_rate_limit: key=" << std::quoted(key) << " enabled=" << ratelimit_info->enabled << dendl; if (key.empty() || key.length() == 1 || !ratelimit_info->enabled) { return false; } - bool is_read = is_read_op(method); + OpType type = op_type(method, resource, ratelimit_info); auto& it = find_or_create(key); auto curr_ts = curr_timestamp.time_since_epoch(); - return it.should_rate_limit(is_read ,ratelimit_info, curr_ts); + return it.should_rate_limit(type, ratelimit_info, curr_ts); } - void giveback_tokens(const char *method, const std::string& key) + void giveback_tokens(const char *method, const std::string& key, const std::string& resource, const RGWRateLimitInfo* ratelimit_info) { - bool is_read = is_read_op(method); + OpType type = op_type(method, resource, ratelimit_info); auto& it = find_or_create(key); - it.giveback_tokens(is_read); + it.giveback_tokens(type); } void decrease_bytes(const char *method, const std::string& key, const int64_t amount, const RGWRateLimitInfo* info) { if (key.empty() || key.length() == 1 || !info->enabled) { return; } - bool is_read = is_read_op(method); - if ((is_read && !info->max_read_bytes) || (!is_read && !info->max_write_bytes)) + OpType type = op_type(method, "", info); + // Only read and write ops affect bytes + if ((type == OpType::Read && !info->max_read_bytes) || (type == OpType::Write && !info->max_write_bytes)) { return; } auto& it = find_or_create(key); - it.decrease_bytes(is_read, amount, info); + if (type == OpType::Read) + it.decrease_bytes(true, amount, info); + else if (type == OpType::Write) + it.decrease_bytes(false, amount, info); + // OpType::List does not affect bytes } void clear() { ratelimit_entries.clear(); @@ -271,8 +346,8 @@ class ActiveRateLimiter : public DoutPrefix { ActiveRateLimiter(CephContext* cct) : DoutPrefix(cct, ceph_subsys_rgw, "rate limiter: ") { - ratelimit[0] = std::make_shared(replacing, cv); - ratelimit[1] = std::make_shared(replacing, cv); + ratelimit[0] = std::make_shared(cct, replacing, cv); + ratelimit[1] = std::make_shared(cct, replacing, cv); } ~ActiveRateLimiter() { ldpp_dout(this, 20) << "stopping ratelimit_gc thread" << dendl; diff --git a/src/rgw/rgw_rest_ratelimit.cc b/src/rgw/rgw_rest_ratelimit.cc index 128e44d9a5ae..32011212f48d 100644 --- a/src/rgw/rgw_rest_ratelimit.cc +++ b/src/rgw/rgw_rest_ratelimit.cc @@ -1,6 +1,9 @@ // -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- // vim: ts=8 sw=2 smarttab ft=cpp #include "rgw_rest_ratelimit.h" +#include "rgw_process_env.h" +#include "rgw_op.h" + class RGWOp_Ratelimit_Info : public RGWRESTOp { int check_caps(const RGWUserCaps& caps) override { return caps.check_cap("ratelimit", RGW_CAP_READ); @@ -130,12 +133,14 @@ class RGWOp_Ratelimit_Set : public RGWRESTOp { void set_ratelimit_info(bool have_max_read_ops, int64_t max_read_ops, bool have_max_write_ops, int64_t max_write_ops, bool have_max_read_bytes, int64_t max_read_bytes, bool have_max_write_bytes, int64_t max_write_bytes, + bool have_max_list_ops, int64_t max_list_ops, bool have_max_delete_ops, int64_t max_delete_ops, bool have_enabled, bool enabled, bool& ratelimit_configured, RGWRateLimitInfo& ratelimit_info); }; void RGWOp_Ratelimit_Set::set_ratelimit_info(bool have_max_read_ops, int64_t max_read_ops, bool have_max_write_ops, int64_t max_write_ops, bool have_max_read_bytes, int64_t max_read_bytes, bool have_max_write_bytes, int64_t max_write_bytes, + bool have_max_list_ops, int64_t max_list_ops, bool have_max_delete_ops, int64_t max_delete_ops, bool have_enabled, bool enabled, bool& ratelimit_configured, RGWRateLimitInfo& ratelimit_info) { if (have_max_read_ops) { @@ -162,6 +167,18 @@ class RGWOp_Ratelimit_Set : public RGWRESTOp { ratelimit_configured = true; } } + if (have_max_list_ops) { + if (max_list_ops >= 0) { + ratelimit_info.max_list_ops = max_list_ops; + ratelimit_configured = true; + } + } + if (have_max_delete_ops) { + if (max_delete_ops >= 0) { + ratelimit_info.max_delete_ops = max_delete_ops; + ratelimit_configured = true; + } + } if (have_enabled) { ratelimit_info.enabled = enabled; ratelimit_configured = true; @@ -194,6 +211,10 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) bool have_max_read_bytes = false; int64_t max_write_bytes = 0; bool have_max_write_bytes = false; + int64_t max_list_ops = 0; + bool have_max_list_ops = false; + int64_t max_delete_ops = 0; + bool have_max_delete_ops = false; RESTArgs::get_string(s, "uid", uid_str, &uid_str); RESTArgs::get_string(s, "ratelimit-scope", ratelimit_scope, &ratelimit_scope); RESTArgs::get_string(s, "bucket", bucket_name, &bucket_name); @@ -203,6 +224,8 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) op_ret |= RESTArgs::get_int64(s, "max-write-ops", 0, &max_write_ops, &have_max_write_ops); op_ret |= RESTArgs::get_int64(s, "max-read-bytes", 0, &max_read_bytes, &have_max_read_bytes); op_ret |= RESTArgs::get_int64(s, "max-write-bytes", 0, &max_write_bytes, &have_max_write_bytes); + op_ret |= RESTArgs::get_int64(s, "max-list-ops", 0, &max_list_ops, &have_max_list_ops); + op_ret |= RESTArgs::get_int64(s, "max-delete-ops", 0, &max_delete_ops, &have_max_delete_ops); if (op_ret) { ldpp_dout(this, 0) << "one of the maximum arguments could not be parsed" << dendl; return; @@ -227,8 +250,18 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) } } RESTArgs::get_bool(s, "global", false, &global, nullptr); + + // forward to master zonegroup + op_ret = rgw_forward_request_to_master(this, *s->penv.site, s->user->get_id(), + nullptr, nullptr, s->info, s->err, y); + if (op_ret < 0) { + ldpp_dout(this, 0) << "ERROR: forward_request_to_master returned ret=" << op_ret << dendl; + return; + } + set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); if (op_ret) { return; @@ -262,6 +295,7 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) } set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); bufferlist bl; ratelimit_info.encode(bl); @@ -295,6 +329,7 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) bufferlist bl; set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); ratelimit_info.encode(bl); rgw::sal::Attrs attr; @@ -314,6 +349,7 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) ratelimit_info = period_config.bucket_ratelimit; set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); period_config.bucket_ratelimit = ratelimit_info; op_ret = period_config.write(s, static_cast(driver)->svc()->sysobj, realm_id, y); @@ -323,6 +359,7 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) ratelimit_info = period_config.anon_ratelimit; set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); period_config.anon_ratelimit = ratelimit_info; op_ret = period_config.write(s, static_cast(driver)->svc()->sysobj, realm_id, y); @@ -332,6 +369,7 @@ void RGWOp_Ratelimit_Set::execute(optional_yield y) ratelimit_info = period_config.user_ratelimit; set_ratelimit_info(have_max_read_ops, max_read_ops, have_max_write_ops, max_write_ops, have_max_read_bytes, max_read_bytes, have_max_write_bytes, max_write_bytes, + have_max_list_ops, max_list_ops, have_max_delete_ops, max_delete_ops, have_enabled, enabled, ratelimit_configured, ratelimit_info); period_config.user_ratelimit = ratelimit_info; op_ret = period_config.write(s, static_cast(driver)->svc()->sysobj, realm_id, y); diff --git a/src/test/cli/radosgw-admin/help.t b/src/test/cli/radosgw-admin/help.t index 0786868d78cc..1f1c1ad5ec4f 100644 --- a/src/test/cli/radosgw-admin/help.t +++ b/src/test/cli/radosgw-admin/help.t @@ -358,10 +358,12 @@ --quota-scope scope of quota (bucket, user, account) Rate limiting options: - --max-read-ops specify max requests per minute for READ ops per RGW (GET and HEAD request methods), 0 means unlimited - --max-read-bytes specify max bytes per minute for READ ops per RGW (GET and HEAD request methods), 0 means unlimited - --max-write-ops specify max requests per minute for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited - --max-write-bytes specify max bytes per minute for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited + --max-read-ops specify max requests per accumulation interval for READ ops per RGW (GET and HEAD request methods), 0 means unlimited + --max-read-bytes specify max bytes per accumulation interval for READ ops per RGW (GET and HEAD request methods), 0 means unlimited + --max-write-ops specify max requests per accumulation interval for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited + --max-write-bytes specify max bytes per accumulation interval for WRITE ops per RGW (Not GET or HEAD request methods), 0 means unlimited + --max-list-ops specify max requests per accumulation interval for bucket listing requests per RGW, 0 means unlimited + --max-delete-ops specify max requests per accumulation interval for DELETE ops per RGW (DELETE request methods), 0 means unlimited --ratelimit-scope scope of rate limiting: bucket, user, anonymous anonymous can be configured only with global rate limit diff --git a/src/test/rgw/bench_rgw_ratelimit.cc b/src/test/rgw/bench_rgw_ratelimit.cc index 529d8f739fd9..21ed60f07b55 100644 --- a/src/test/rgw/bench_rgw_ratelimit.cc +++ b/src/test/rgw/bench_rgw_ratelimit.cc @@ -91,7 +91,7 @@ bool simulate_request(client_info& it, const RGWRateLimitInfo& info, std::shared int rw = 0; // will always use PUT method as there is no different std::string methodop = method[rw]; auto dout = DoutPrefix(g_ceph_context, ceph_subsys_rgw, "rate limiter: "); - bool to_fail = ratelimit->should_rate_limit(methodop.c_str(), it.tenant, time, &info); + bool to_fail = ratelimit->should_rate_limit(methodop.c_str(), it.tenant, time, &info, ""); if(to_fail) { it.rejected++; diff --git a/src/test/rgw/bench_rgw_ratelimit_gc.cc b/src/test/rgw/bench_rgw_ratelimit_gc.cc index ae422e1dada2..3b837b98f74e 100644 --- a/src/test/rgw/bench_rgw_ratelimit_gc.cc +++ b/src/test/rgw/bench_rgw_ratelimit_gc.cc @@ -46,7 +46,7 @@ int main(int argc, char **argv) { std::string tenant = "uuser" + std::to_string(i); auto time = ceph::coarse_real_clock::now(); - ratelimit->get_active()->should_rate_limit("PUT", tenant, time, &info); + ratelimit->get_active()->should_rate_limit("PUT", tenant, time, &info, ""); } } diff --git a/src/test/rgw/test_rgw_ratelimit.cc b/src/test/rgw/test_rgw_ratelimit.cc index 01be4df48a02..fd305fdfad0a 100644 --- a/src/test/rgw/test_rgw_ratelimit.cc +++ b/src/test/rgw/test_rgw_ratelimit.cc @@ -7,16 +7,17 @@ using namespace std::chrono_literals; + TEST(RGWRateLimit, op_limit_not_enabled) { // info.enabled = false, so no limit std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info); + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, reject_op_over_limit) @@ -24,15 +25,15 @@ TEST(RGWRateLimit, reject_op_over_limit) // check that request is being rejected because there are not enough tokens std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(true, success); } TEST(RGWRateLimit, accept_op_after_giveback) @@ -40,16 +41,16 @@ TEST(RGWRateLimit, accept_op_after_giveback) // check that giveback is working fine std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); - ratelimit.giveback_tokens("GET", key); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + ratelimit.giveback_tokens("GET", key, "", &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, accept_op_after_refill) @@ -57,15 +58,15 @@ TEST(RGWRateLimit, accept_op_after_refill) // check that tokens are being filled properly std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); time += 61s; - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, reject_bw_over_limit) @@ -73,16 +74,16 @@ TEST(RGWRateLimit, reject_bw_over_limit) // check that a newer request is rejected if there is no enough tokens (bw) std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_bytes = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 2, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(true, success); } TEST(RGWRateLimit, accept_bw) @@ -90,16 +91,16 @@ TEST(RGWRateLimit, accept_bw) // check that when there are enough tokens (bw) the request is still being served std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 1, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, check_bw_debt_at_max_120secs) @@ -107,16 +108,16 @@ TEST(RGWRateLimit, check_bw_debt_at_max_120secs) // check that the bandwidth debt is not larger than 120 seconds std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 100, &info); time += 121s; - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, check_that_bw_limit_not_affect_ops) @@ -124,17 +125,17 @@ TEST(RGWRateLimit, check_that_bw_limit_not_affect_ops) // check that high read bytes limit, does not affect ops limit std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(true, success); } TEST(RGWRateLimit, read_limit_does_not_affect_writes) @@ -142,17 +143,17 @@ TEST(RGWRateLimit, read_limit_does_not_affect_writes) // read limit does not affect writes std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info); + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); ratelimit.decrease_bytes("PUT",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("PUT", key, time, &info); + success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); EXPECT_EQ(false, success); } TEST(RGWRateLimit, write_limit_does_not_affect_reads) @@ -160,17 +161,17 @@ TEST(RGWRateLimit, write_limit_does_not_affect_reads) // write limit does not affect reads std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; info.max_write_ops = 1; info.max_write_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info); + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } @@ -179,12 +180,12 @@ TEST(RGWRateLimit, allow_unlimited_access) // 0 values in RGWRateLimitInfo should allow unlimited access std::atomic_bool replacing; std::condition_variable cv; - RateLimiter ratelimit(replacing, cv); + RateLimiter ratelimit(g_ceph_context, replacing, cv); RGWRateLimitInfo info; info.enabled = true; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info); + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); EXPECT_EQ(false, success); } @@ -199,7 +200,7 @@ TEST(RGWRateLimitGC, NO_GC_AHEAD_OF_TIME) RGWRateLimitInfo info; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - active->should_rate_limit("GET", key, time, &info); + active->should_rate_limit("GET", key, time, &info, ""); auto activegc = ratelimit->get_active(); EXPECT_EQ(activegc, active); } @@ -217,7 +218,7 @@ TEST(RGWRateLimiterGC, GC_IS_WORKING) std::string key = "-1"; for(int i = 0; i < 2000000; i++) { - active->should_rate_limit("GET", key, time, &info); + active->should_rate_limit("GET", key, time, &info, ""); key = std::to_string(i); } auto activegc = ratelimit->get_active(); @@ -231,7 +232,7 @@ TEST(RGWRateLimitEntry, op_limit_not_enabled) RateLimiterEntry entry; RGWRateLimitInfo info; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(false, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, reject_op_over_limit) @@ -243,9 +244,9 @@ TEST(RGWRateLimitEntry, reject_op_over_limit) info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(true, success); } TEST(RGWRateLimitEntry, accept_op_after_giveback) @@ -256,10 +257,10 @@ TEST(RGWRateLimitEntry, accept_op_after_giveback) info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); - entry.giveback_tokens(true); + bool success = entry.should_rate_limit(OpType::Read, &info, time); + entry.giveback_tokens(OpType::Read); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, accept_op_after_refill) @@ -270,9 +271,9 @@ TEST(RGWRateLimitEntry, accept_op_after_refill) info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); time += 61s; - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, reject_bw_over_limit) @@ -283,10 +284,10 @@ TEST(RGWRateLimitEntry, reject_bw_over_limit) info.enabled = true; info.max_read_bytes = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 2, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(true, success); } TEST(RGWRateLimitEntry, accept_bw) @@ -297,10 +298,10 @@ TEST(RGWRateLimitEntry, accept_bw) info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 1, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, check_bw_debt_at_max_120secs) @@ -311,10 +312,10 @@ TEST(RGWRateLimitEntry, check_bw_debt_at_max_120secs) info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 100, &info); time += 121s; - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, check_that_bw_limit_not_affect_ops) @@ -326,10 +327,10 @@ TEST(RGWRateLimitEntry, check_that_bw_limit_not_affect_ops) info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(true, success); } TEST(RGWRateLimitEntry, read_limit_does_not_affect_writes) @@ -341,10 +342,10 @@ TEST(RGWRateLimitEntry, read_limit_does_not_affect_writes) info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(false, &info, time); + bool success = entry.should_rate_limit(OpType::Write, &info, time); entry.decrease_bytes(false, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(false, &info, time); + success = entry.should_rate_limit(OpType::Write, &info, time); EXPECT_EQ(false, success); } TEST(RGWRateLimitEntry, write_limit_does_not_affect_reads) @@ -357,10 +358,10 @@ TEST(RGWRateLimitEntry, write_limit_does_not_affect_reads) info.max_write_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); std::string key = "uuser123"; - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(true, &info, time); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } @@ -371,6 +372,642 @@ TEST(RGWRateLimitEntry, allow_unlimited_access) RGWRateLimitInfo info; info.enabled = true; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(true, &info, time); + bool success = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(false, success); +} + + +// Helpers for LIST op resource string +// ref uri: "/aaa-gonzo-staging-bbb-checkpoint1-us-west-0000?list-type=2&delimiter=%2F&max-keys=2&prefix=spark%2Fgonzo-avro%2Fsplunk_hec_test%2Fchunk-commits%2F%2F00000007999&encoding-type=url"; +const std::string RES_LIST_TYPE_2 = "?list-type=2"; +const std::string RES_DELIMITER = "&delimiter=%2F"; +const std::string RES_PREFIX = "&prefix=spark%2Fgonzo-avro%2Fsplunk_hec_test%2Fchunk-commits%2F%2F00000007999"; + +TEST(RGWRateLimit, reject_list_op_over_limit) +{ + // check that LIST op is being rejected because there are not enough tokens + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimit, accept_list_op_after_giveback) +{ + // check that giveback is working for LIST ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + ratelimit.giveback_tokens("GET", key, RES_LIST_TYPE_2, &info); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, accept_list_op_after_refill) +{ + // check that tokens are being filled properly for LIST ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + time += 61s; + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, list_limit_does_not_affect_reads) +{ + // list limit does not affect reads + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + // Should still be able to do a normal GET (read) + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, read_limit_does_not_affect_lists) +{ + // read limit does not affect lists + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + // Should still be able to do a LIST op + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, list_limit_does_not_affect_writes) +{ + // list limit does not affect writes + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + // Should still be able to do a PUT (write) + success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, write_limit_does_not_affect_lists) +{ + // write limit does not affect lists + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + // Should still be able to do a LIST op + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, list_limit_does_not_affect_deletes) +{ + // list limit does not affect deletes + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + // Should still be able to do a DELETE op + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, delete_limit_does_not_affect_lists) +{ + // delete limit does not affect lists + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + // Should still be able to do a LIST op + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + +// LIST RES_DELIMITER minimal tests +TEST(RGWRateLimit, reject_delimiter_op_over_limit) +{ + // check that DELIMITER op is being rejected because there are not enough tokens + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimit, accept_delimiter_op_after_giveback) +{ + // check that giveback is working for DELIMITER ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + ratelimit.giveback_tokens("GET", key, RES_DELIMITER, &info); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, accept_delimiter_op_after_refill) +{ + // check that tokens are being filled properly for DELIMITER ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + time += 61s; + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_EQ(false, success); +} + +// LIST RES_PREFIX minimal tests +TEST(RGWRateLimit, reject_prefix_op_over_limit) +{ + // check that PREFIX op is being rejected because there are not enough tokens + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimit, accept_prefix_op_after_giveback) +{ + // check that giveback is working for PREFIX ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + ratelimit.giveback_tokens("GET", key, RES_PREFIX, &info); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, accept_prefix_op_after_refill) +{ + // check that tokens are being filled properly for PREFIX ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + time += 61s; + success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_EQ(false, success); +} + + +TEST(RGWRateLimitEntry, reject_list_op_over_limit) +{ + // check that LIST request is being rejected because there are not enough tokens + RGWRateLimitInfo info; + RateLimiterEntry entry; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimitEntry, accept_list_op_after_giveback) +{ + // check that giveback is working fine for LIST ops + RGWRateLimitInfo info; + RateLimiterEntry entry; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + entry.giveback_tokens(OpType::List); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, accept_list_op_after_refill) +{ + // check that tokens are being filled properly for LIST ops + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + time += 61s; + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, list_limit_does_not_affect_reads) +{ + // list limit does not affect reads + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, read_limit_does_not_affect_lists) +{ + // read limit does not affect lists + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Read, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, list_limit_does_not_affect_writes) +{ + // list limit does not affect writes + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Write, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, write_limit_does_not_affect_lists) +{ + // write limit does not affect lists + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Write, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, list_limit_does_not_affect_deletes) +{ + // list limit does not affect deletes + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::List, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, delete_limit_does_not_affect_lists) +{ + // delete limit does not affect lists + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_list_ops = 1; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(false, success); +} + + +TEST(RGWRateLimit, reject_delete_op_over_limit) +{ + // check that DELETE op is being rejected because there are not enough tokens + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimit, accept_delete_op_after_giveback) +{ + // check that giveback is working for DELETE ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + ratelimit.giveback_tokens("DELETE", key, "", &info); + time = ceph::coarse_real_clock::now(); + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, accept_delete_op_after_refill) +{ + // check that tokens are being filled properly for DELETE ops + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + time += 61s; + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, delete_limit_does_not_affect_reads) +{ + // delete limit does not affect reads + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + // Should still be able to do a normal GET (read) + success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, read_limit_does_not_affect_deletes) +{ + // read limit does not affect deletes + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + // Should still be able to do a DELETE op + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, write_limit_does_not_affect_deletes) +{ + // write limit does not affect deletes + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + // Should still be able to do a DELETE op + success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, delete_limit_does_not_affect_writes) +{ + // delete limit does not affect writes + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + // Should still be able to do a PUT (write) + success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, reject_delete_op_over_limit) +{ + // check that DELETE request is being rejected because there are not enough tokens + RGWRateLimitInfo info; + RateLimiterEntry entry; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(true, success); +} + +TEST(RGWRateLimitEntry, accept_delete_op_after_giveback) +{ + // check that giveback is working fine for DELETE ops + RGWRateLimitInfo info; + RateLimiterEntry entry; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + entry.giveback_tokens(OpType::Delete); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, accept_delete_op_after_refill) +{ + // check that tokens are being filled properly for DELETE ops + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + time += 61s; + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, delete_limit_does_not_affect_reads) +{ + // delete limit does not affect reads + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Read, &info, time); EXPECT_EQ(false, success); } + +TEST(RGWRateLimitEntry, read_limit_does_not_affect_deletes) +{ + // read limit does not affect deletes + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_read_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Read, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, write_limit_does_not_affect_deletes) +{ + // write limit does not affect deletes + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Write, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimitEntry, delete_limit_does_not_affect_writes) +{ + // delete limit does not affect writes + RateLimiterEntry entry; + RGWRateLimitInfo info; + info.enabled = true; + info.max_delete_ops = 1; + info.max_write_ops = 1; + auto time = ceph::coarse_real_clock::now().time_since_epoch(); + bool success = entry.should_rate_limit(OpType::Delete, &info, time); + time = ceph::coarse_real_clock::now().time_since_epoch(); + success = entry.should_rate_limit(OpType::Write, &info, time); + EXPECT_EQ(false, success); +} + From 137a8e332e44b377c475cf9fe9fda2f24bc07f83 Mon Sep 17 00:00:00 2001 From: morphes1995 Date: Thu, 4 Dec 2025 10:15:19 +0000 Subject: [PATCH 02/20] rgw: ops large budget caused by rate limiter was enabled and max_ops set to 0 Signed-off-by: morphes1995 (cherry picked from commit 91e9f00a7543c4651a983c2d785696a069d8ab28) --- src/rgw/rgw_ratelimit.h | 28 ++++++++-- src/test/rgw/test_rgw_ratelimit.cc | 88 ++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 4 deletions(-) diff --git a/src/rgw/rgw_ratelimit.h b/src/rgw/rgw_ratelimit.h index 3da243063073..6410d197758a 100644 --- a/src/rgw/rgw_ratelimit.h +++ b/src/rgw/rgw_ratelimit.h @@ -61,7 +61,12 @@ class RateLimiterEntry { return true; } // we don't want to reduce ops' tokens if we've rejected it. - read.ops -= fixed_point_rgw_ratelimit; + if (read_ops() > 0) + { + // make sure read.ops always greater or equal than 0 + // to avoid large budget caused by the case that rate limiter was enabled and read.ops set to 0 + read.ops -= fixed_point_rgw_ratelimit; + } return false; } bool should_rate_limit_list(int64_t ops_limit) @@ -69,7 +74,12 @@ class RateLimiterEntry { if ((list_ops() - 1 < 0) && (ops_limit > 0)) { return true; } - list.ops -= fixed_point_rgw_ratelimit; + if (list_ops() > 0) + { + // make sure list.ops always greater or equal than 0 + // to avoid large budget caused by the case that rate limiter was enabled and list.ops set to 0 + list.ops -= fixed_point_rgw_ratelimit; + } return false; } bool should_rate_limit_delete(int64_t ops_limit) @@ -77,7 +87,12 @@ class RateLimiterEntry { if ((delete_ops() - 1 < 0) && (ops_limit > 0)) { return true; } - del.ops -= fixed_point_rgw_ratelimit; + if (delete_ops() > 0) + { + // make sure del.ops always greater or equal than 0 + // to avoid large budget caused by the case that rate limiter was enabled and del.ops set to 0 + del.ops -= fixed_point_rgw_ratelimit; + } return false; } bool should_rate_limit_write(int64_t ops_limit, int64_t bw_limit) @@ -90,7 +105,12 @@ class RateLimiterEntry { } // we don't want to reduce ops' tokens if we've rejected it. - write.ops -= fixed_point_rgw_ratelimit; + if (write_ops() > 0) + { + // make sure write.ops always greater or equal than 0 + // to avoid large budget caused by the case that rate limiter was enabled and write.ops set to 0 + write.ops -= fixed_point_rgw_ratelimit; + } return false; } /* The purpose of this function is to minimum time before overriding the stored timestamp diff --git a/src/test/rgw/test_rgw_ratelimit.cc b/src/test/rgw/test_rgw_ratelimit.cc index fd305fdfad0a..67a69c8ce4f9 100644 --- a/src/test/rgw/test_rgw_ratelimit.cc +++ b/src/test/rgw/test_rgw_ratelimit.cc @@ -189,6 +189,50 @@ TEST(RGWRateLimit, allow_unlimited_access) EXPECT_EQ(false, success); } +TEST(RGWRateLimit, unlimited_access_not_left_large_read_ops_budget) +{ + // 0 values in RGWRateLimitInfo should allow unlimited access + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser123"; + + for (int i = 0; i < 10; i++) + { + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(false, success); + } + time += 61s; + info.max_read_ops = 1; // make read ops limited + bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(false, success); +} + +TEST(RGWRateLimit, unlimited_access_not_left_large_write_ops_budget) +{ + // 0 values in RGWRateLimitInfo should allow unlimited access + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser123"; + + for (int i = 0; i < 10; i++) + { + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(false, success); + } + time += 61s; + info.max_write_ops = 1; // make write ops limited + bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(false, success); +} + TEST(RGWRateLimitGC, NO_GC_AHEAD_OF_TIME) { // Test if GC is not starting the replace before getting to map_size * 0.9 @@ -489,6 +533,28 @@ TEST(RGWRateLimit, list_limit_does_not_affect_writes) EXPECT_EQ(false, success); } +TEST(RGWRateLimit, unlimited_access_not_left_large_list_ops_budget) +{ + // 0 values in RGWRateLimitInfo should allow unlimited access + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_list"; + + for (int i = 0; i < 10; i++) + { + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); + } + time += 61s; + info.max_list_ops = 1; // make list ops limited + bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(false, success); +} + TEST(RGWRateLimit, write_limit_does_not_affect_lists) { // write limit does not affect lists @@ -908,6 +974,28 @@ TEST(RGWRateLimit, delete_limit_does_not_affect_writes) EXPECT_EQ(false, success); } +TEST(RGWRateLimit, unlimited_access_not_left_large_delete_ops_budget) +{ + // 0 values in RGWRateLimitInfo should allow unlimited access + std::atomic_bool replacing; + std::condition_variable cv; + RateLimiter ratelimit(g_ceph_context, replacing, cv); + RGWRateLimitInfo info; + info.enabled = true; + auto time = ceph::coarse_real_clock::now(); + std::string key = "uuser_delete"; + + for (int i = 0; i < 10; i++) + { + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); + } + time += 61s; + info.max_delete_ops = 1; // make delete ops limited + bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(false, success); +} + TEST(RGWRateLimitEntry, reject_delete_op_over_limit) { // check that DELETE request is being rejected because there are not enough tokens From b9b783a8c27436ff766d340533cd37559c1f54f9 Mon Sep 17 00:00:00 2001 From: Lumir Sliva Date: Fri, 3 Apr 2026 23:56:41 +0200 Subject: [PATCH 03/20] rgw: add accurate Retry-After header to rate-limit responses When RGW rejects an S3 request with -ERR_RATE_LIMITED, return an HTTP Retry-After header that tells the client how many seconds to wait before retrying. The value is computed from the current token-bucket deficit and replenishment rate, so a polite client that honors the header will retry right around when its tokens recover. The delay is threaded out of the rate limiter through req_state and onto the response in abort_early(). The rate-limit check now returns int64_t seconds (0 = allowed) instead of bool, with a 1-second floor on the rejected path. The seconds-to-recover calculation is factored into a small static helper RateLimiterEntry::compute_delay(limit, needed, interval) so it can be unit-tested directly. Each dimension (ops, bytes) calls it independently and the caller takes the max, leaving room for additional limit dimensions in the future. The user-facing documentation is updated to describe the new header and to suggest proxy-layer remapping for operators who need 429 instead of the existing 503. Tracker: https://tracker.ceph.com/issues/64083 Tracker: https://tracker.ceph.com/issues/67655 Signed-off-by: Lumir Sliva <61183145+lumir-sliva@users.noreply.github.com> (cherry picked from commit 3dae903c9a31fe5161058a4bb1cfe4092839eb29) --- doc/radosgw/admin.rst | 6 + src/rgw/rgw_common.h | 1 + src/rgw/rgw_process.cc | 8 +- src/rgw/rgw_ratelimit.h | 118 ++++---- src/rgw/rgw_rest.cc | 4 + src/test/rgw/test_rgw_ratelimit.cc | 423 ++++++++++++++++------------- 6 files changed, 317 insertions(+), 243 deletions(-) diff --git a/doc/radosgw/admin.rst b/doc/radosgw/admin.rst index 007cacbdcb22..10ef6cb61a41 100644 --- a/doc/radosgw/admin.rst +++ b/doc/radosgw/admin.rst @@ -679,6 +679,12 @@ then the ``GET`` action will fail. After "user A" has completed this 1 GB operation, RGW blocks the user's requests for up to two accumulation intervals. After this time has elapsed, "user A" will be able to send ``GET`` requests again. +Rate-limited requests receive an HTTP ``503`` response with the S3 error code +``SlowDown``, matching the AWS S3 behavior. The response includes a +``Retry-After`` header with the estimated number of seconds until the rate-limit +window allows the next request through, computed from the current token-bucket +state. Load balancers or reverse proxies can be configured to map the ``503`` +with ``Retry-After`` to ``429 Too Many Requests`` if clients require it. - **Bucket:** The ``--bucket`` option allows you to specify a rate limit for a bucket. diff --git a/src/rgw/rgw_common.h b/src/rgw/rgw_common.h index f6771a7cb8d2..1f3e537b003d 100644 --- a/src/rgw/rgw_common.h +++ b/src/rgw/rgw_common.h @@ -1303,6 +1303,7 @@ struct req_state : DoutPrefixProvider { std::shared_ptr ratelimit_data; RGWRateLimitInfo user_ratelimit; RGWRateLimitInfo bucket_ratelimit; + int64_t ratelimit_retry_after{0}; std::string ratelimit_bucket_marker; std::string ratelimit_user_name; bool content_started{false}; diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc index 53c9f2a407bf..755b50dc1a26 100644 --- a/src/rgw/rgw_process.cc +++ b/src/rgw/rgw_process.cc @@ -140,8 +140,8 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { if (s->user->get_id().id == RGW_USER_ANON_ID && global_anon.enabled) { *user_ratelimit = global_anon; } - bool limit_bucket = false; - bool limit_user = s->ratelimit_data->should_rate_limit(method, s->ratelimit_user_name, s->time, user_ratelimit, s->info.request_params); + int64_t limit_bucket = 0; + int64_t limit_user = s->ratelimit_data->should_rate_limit(method, s->ratelimit_user_name, s->time, user_ratelimit, s->info.request_params); if(!rgw::sal::Bucket::empty(s->bucket.get())) { @@ -169,6 +169,10 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { } s->user_ratelimit = *user_ratelimit; s->bucket_ratelimit = *bucket_ratelimit; + int64_t delay = limit_user ? limit_user : limit_bucket; + if (delay > 0) { + s->ratelimit_retry_after = delay; + } return (limit_user || limit_bucket); } diff --git a/src/rgw/rgw_ratelimit.h b/src/rgw/rgw_ratelimit.h index 6410d197758a..04c366500089 100644 --- a/src/rgw/rgw_ratelimit.h +++ b/src/rgw/rgw_ratelimit.h @@ -9,13 +9,29 @@ enum class OpType { Read, Write, List, Delete }; class RateLimiterEntry { - /* +public: + /* fixed_point_rgw_ratelimit is important to preserve the precision of the token calculation for example: a user have a limit of single op per minute, the user will consume its single token and then will send another request, 1s after it. in that case, without this method, the user will get 0 tokens although it should get 0.016 tokens. using this method it will add 16 tokens to the user, and the user will have 16 tokens, each time rgw will do comparison rgw will divide by fixed_point_rgw_ratelimit, so the user will be blocked anyway until it has enough tokens. */ static constexpr int64_t fixed_point_rgw_ratelimit = 1000; + + // Compute the seconds needed to replenish `needed` tokens when tokens + // replenish at `limit` units per `interval` seconds. All three parameters + // are in fixed-point-scaled units (multiply user-visible token counts by + // fixed_point_rgw_ratelimit before passing). The result is in seconds, + // rounded up. Returns 0 when `limit` or `needed` is non-positive, which + // callers treat as "no delay required for this dimension". + static int64_t compute_delay(int64_t limit, int64_t needed, int64_t interval) { + if (limit <= 0 || needed <= 0) { + return 0; + } + return (needed * interval + limit - 1) / limit; + } + +private: // counters are tracked in multiples of fixed_point_rgw_ratelimit struct counters { int64_t ops = 0; @@ -53,66 +69,74 @@ class RateLimiterEntry { { return write.bytes / fixed_point_rgw_ratelimit; } - bool should_rate_limit_read(int64_t ops_limit, int64_t bw_limit) { - //check if tenants did not reach their bw or ops limits and that the limits are not 0 (which is unlimited) - if(((read_ops() - 1 < 0) && (ops_limit > 0)) || - (read_bytes() < 0 && bw_limit > 0)) - { - return true; - } + // Returns 0 if the request is allowed, or the estimated retry delay + // in seconds if rate-limited. compute_delay() is the sole decision + // maker: delay > 0 means rate-limited, 0 means allowed. + int64_t should_rate_limit_read(int64_t ops_limit, int64_t bw_limit) { + const int64_t interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const int64_t ops_delay = compute_delay(ops_limit * fixed_point_rgw_ratelimit, + fixed_point_rgw_ratelimit - read.ops, + interval); + const int64_t bw_delay = compute_delay(bw_limit * fixed_point_rgw_ratelimit, + -read.bytes, + interval); + const int64_t delay = std::max(ops_delay, bw_delay); + if (delay > 0) { + return delay; + } // we don't want to reduce ops' tokens if we've rejected it. - if (read_ops() > 0) - { - // make sure read.ops always greater or equal than 0 - // to avoid large budget caused by the case that rate limiter was enabled and read.ops set to 0 + if (read_ops() > 0) { read.ops -= fixed_point_rgw_ratelimit; } - return false; + return 0; } - bool should_rate_limit_list(int64_t ops_limit) + int64_t should_rate_limit_list(int64_t ops_limit) { - if ((list_ops() - 1 < 0) && (ops_limit > 0)) { - return true; + const int64_t interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const int64_t delay = compute_delay(ops_limit * fixed_point_rgw_ratelimit, + fixed_point_rgw_ratelimit - list.ops, + interval); + if (delay > 0) { + return delay; } - if (list_ops() > 0) - { - // make sure list.ops always greater or equal than 0 - // to avoid large budget caused by the case that rate limiter was enabled and list.ops set to 0 + if (list_ops() > 0) { list.ops -= fixed_point_rgw_ratelimit; } - return false; + return 0; } - bool should_rate_limit_delete(int64_t ops_limit) + int64_t should_rate_limit_delete(int64_t ops_limit) { - if ((delete_ops() - 1 < 0) && (ops_limit > 0)) { - return true; + const int64_t interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const int64_t delay = compute_delay(ops_limit * fixed_point_rgw_ratelimit, + fixed_point_rgw_ratelimit - del.ops, + interval); + if (delay > 0) { + return delay; } - if (delete_ops() > 0) - { - // make sure del.ops always greater or equal than 0 - // to avoid large budget caused by the case that rate limiter was enabled and del.ops set to 0 + if (delete_ops() > 0) { del.ops -= fixed_point_rgw_ratelimit; } - return false; + return 0; } - bool should_rate_limit_write(int64_t ops_limit, int64_t bw_limit) + int64_t should_rate_limit_write(int64_t ops_limit, int64_t bw_limit) { - //check if tenants did not reach their bw or ops limits and that the limits are not 0 (which is unlimited) - if(((write_ops() - 1 < 0) && (ops_limit > 0)) || - (write_bytes() < 0 && bw_limit > 0)) - { - return true; + const int64_t interval = g_ceph_context->_conf->rgw_ratelimit_interval; + const int64_t ops_delay = compute_delay(ops_limit * fixed_point_rgw_ratelimit, + fixed_point_rgw_ratelimit - write.ops, + interval); + const int64_t bw_delay = compute_delay(bw_limit * fixed_point_rgw_ratelimit, + -write.bytes, + interval); + const int64_t delay = std::max(ops_delay, bw_delay); + if (delay > 0) { + return delay; } - - // we don't want to reduce ops' tokens if we've rejected it. - if (write_ops() > 0) - { - // make sure write.ops always greater or equal than 0 - // to avoid large budget caused by the case that rate limiter was enabled and write.ops set to 0 + if (write_ops() > 0) { write.ops -= fixed_point_rgw_ratelimit; } - return false; + return 0; } + /* The purpose of this function is to minimum time before overriding the stored timestamp This function is necessary to force the increase tokens add at least 1 token when it updates the last stored timestamp. That way the user/bucket will not lose tokens because of rounding @@ -167,7 +191,8 @@ class RateLimiterEntry { } public: - bool should_rate_limit(OpType op_type, const RGWRateLimitInfo* ratelimit_info, ceph::timespan curr_timestamp) + // Returns 0 if allowed, or the retry delay in seconds if rate-limited. + int64_t should_rate_limit(OpType op_type, const RGWRateLimitInfo* ratelimit_info, ceph::timespan curr_timestamp) { std::unique_lock lock(ts_lock); increase_tokens(curr_timestamp, ratelimit_info); @@ -181,7 +206,7 @@ class RateLimiterEntry { case OpType::Delete: return should_rate_limit_delete(ratelimit_info->max_delete_ops); } - return false; + return 0; } void decrease_bytes(bool is_read, int64_t amount, const RGWRateLimitInfo* info) { std::unique_lock lock(ts_lock); @@ -283,11 +308,12 @@ class RateLimiter : public DoutPrefix { ratelimit_entries.max_load_factor(1000); }; - bool should_rate_limit(const char *method, const std::string& key, ceph::coarse_real_time curr_timestamp, const RGWRateLimitInfo* ratelimit_info, const std::string& resource) { + // Returns 0 if allowed, or the retry delay in seconds if rate-limited. + int64_t should_rate_limit(const char *method, const std::string& key, ceph::coarse_real_time curr_timestamp, const RGWRateLimitInfo* ratelimit_info, const std::string& resource) { ldpp_dout(this, 21) << "checking should_rate_limit: key=" << std::quoted(key) << " enabled=" << ratelimit_info->enabled << dendl; if (key.empty() || key.length() == 1 || !ratelimit_info->enabled) { - return false; + return 0; } OpType type = op_type(method, resource, ratelimit_info); auto& it = find_or_create(key); diff --git a/src/rgw/rgw_rest.cc b/src/rgw/rgw_rest.cc index 4152feee951d..813a1ff12581 100644 --- a/src/rgw/rgw_rest.cc +++ b/src/rgw/rgw_rest.cc @@ -717,6 +717,10 @@ void abort_early(req_state *s, RGWOp* op, int err_no, } dump_errno(s); + if (err_no == -ERR_RATE_LIMITED && s->ratelimit_retry_after > 0) { + dump_header(s, "Retry-After", + static_cast(s->ratelimit_retry_after)); + } dump_bucket_from_state(s); if (err_no == -ERR_PERMANENT_REDIRECT || err_no == -ERR_WEBSITE_REDIRECT) { string dest_uri; diff --git a/src/test/rgw/test_rgw_ratelimit.cc b/src/test/rgw/test_rgw_ratelimit.cc index 67a69c8ce4f9..3c1eeff983be 100644 --- a/src/test/rgw/test_rgw_ratelimit.cc +++ b/src/test/rgw/test_rgw_ratelimit.cc @@ -8,6 +8,36 @@ using namespace std::chrono_literals; +TEST(RGWRateLimitEntry, compute_delay) +{ + // Disabled limit returns 0 regardless of the deficit + EXPECT_EQ(0, RateLimiterEntry::compute_delay(0, 0, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(0, 1, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(0, 1000, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(0, 1001, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(-1, 1000, 1)); + + // Zero or negative deficit returns 0 (caller treats as "no delay needed") + EXPECT_EQ(0, RateLimiterEntry::compute_delay(1000, 0, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(1000, -1, 1)); + EXPECT_EQ(0, RateLimiterEntry::compute_delay(1000, -1000, 1)); + + // Ceil division at interval=1: tokens replenish at `limit` per second + EXPECT_EQ(1, RateLimiterEntry::compute_delay(1000, 1, 1)); + EXPECT_EQ(1, RateLimiterEntry::compute_delay(1000, 1000, 1)); + EXPECT_EQ(2, RateLimiterEntry::compute_delay(1000, 1001, 1)); + EXPECT_EQ(2, RateLimiterEntry::compute_delay(1000, 2000, 1)); + EXPECT_EQ(3, RateLimiterEntry::compute_delay(1000, 2001, 1)); + + // Ceil division at interval=100: tokens replenish at `limit` per 100s + EXPECT_EQ(0, RateLimiterEntry::compute_delay(10000, 0, 100)); + EXPECT_EQ(1, RateLimiterEntry::compute_delay(10000, 1, 100)); + EXPECT_EQ(1, RateLimiterEntry::compute_delay(10000, 100, 100)); + EXPECT_EQ(2, RateLimiterEntry::compute_delay(10000, 101, 100)); + EXPECT_EQ(2, RateLimiterEntry::compute_delay(10000, 200, 100)); + EXPECT_EQ(3, RateLimiterEntry::compute_delay(10000, 201, 100)); +} + TEST(RGWRateLimit, op_limit_not_enabled) { // info.enabled = false, so no limit @@ -17,12 +47,14 @@ TEST(RGWRateLimit, op_limit_not_enabled) RGWRateLimitInfo info; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, reject_op_over_limit) { - // check that request is being rejected because there are not enough tokens + // check that request is being rejected because there are not enough tokens, + // and that the returned delay matches the configured interval (default 60s) + // when the user is exactly one token short. std::atomic_bool replacing; std::condition_variable cv; RateLimiter ratelimit(g_ceph_context, replacing, cv); @@ -31,10 +63,10 @@ TEST(RGWRateLimit, reject_op_over_limit) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(60, delay); } TEST(RGWRateLimit, accept_op_after_giveback) { @@ -47,11 +79,11 @@ TEST(RGWRateLimit, accept_op_after_giveback) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.giveback_tokens("GET", key, "", &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, accept_op_after_refill) { @@ -64,10 +96,10 @@ TEST(RGWRateLimit, accept_op_after_refill) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); time += 61s; - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, reject_bw_over_limit) { @@ -80,11 +112,11 @@ TEST(RGWRateLimit, reject_bw_over_limit) info.max_read_bytes = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 2, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, accept_bw) { @@ -97,11 +129,11 @@ TEST(RGWRateLimit, accept_bw) info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 1, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, check_bw_debt_at_max_120secs) { @@ -114,11 +146,11 @@ TEST(RGWRateLimit, check_bw_debt_at_max_120secs) info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 100, &info); time += 121s; - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, check_that_bw_limit_not_affect_ops) { @@ -132,11 +164,11 @@ TEST(RGWRateLimit, check_that_bw_limit_not_affect_ops) info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, read_limit_does_not_affect_writes) { @@ -150,11 +182,11 @@ TEST(RGWRateLimit, read_limit_does_not_affect_writes) info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); ratelimit.decrease_bytes("PUT",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, write_limit_does_not_affect_reads) { @@ -168,11 +200,11 @@ TEST(RGWRateLimit, write_limit_does_not_affect_reads) info.max_write_bytes = 100000000; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); ratelimit.decrease_bytes("GET",key, 10000, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, allow_unlimited_access) @@ -185,8 +217,8 @@ TEST(RGWRateLimit, allow_unlimited_access) info.enabled = true; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser123"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, unlimited_access_not_left_large_read_ops_budget) @@ -202,13 +234,13 @@ TEST(RGWRateLimit, unlimited_access_not_left_large_read_ops_budget) for (int i = 0; i < 10; i++) { - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } time += 61s; info.max_read_ops = 1; // make read ops limited - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, unlimited_access_not_left_large_write_ops_budget) @@ -224,13 +256,13 @@ TEST(RGWRateLimit, unlimited_access_not_left_large_write_ops_budget) for (int i = 0; i < 10; i++) { - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } time += 61s; info.max_write_ops = 1; // make write ops limited - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitGC, NO_GC_AHEAD_OF_TIME) @@ -276,22 +308,23 @@ TEST(RGWRateLimitEntry, op_limit_not_enabled) RateLimiterEntry entry; RGWRateLimitInfo info; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, reject_op_over_limit) { - // check that request is being rejected because there are not enough tokens - + // check that request is being rejected because there are not enough tokens, + // and that the returned delay matches the configured interval (default 60s) + // when the user is exactly one token short. RGWRateLimitInfo info; RateLimiterEntry entry; info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(true, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(60, delay); } TEST(RGWRateLimitEntry, accept_op_after_giveback) { @@ -301,11 +334,11 @@ TEST(RGWRateLimitEntry, accept_op_after_giveback) info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.giveback_tokens(OpType::Read); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, accept_op_after_refill) { @@ -315,10 +348,10 @@ TEST(RGWRateLimitEntry, accept_op_after_refill) info.enabled = true; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); time += 61s; - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, reject_bw_over_limit) { @@ -328,11 +361,11 @@ TEST(RGWRateLimitEntry, reject_bw_over_limit) info.enabled = true; info.max_read_bytes = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 2, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(true, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_GT(delay, 0); } TEST(RGWRateLimitEntry, accept_bw) { @@ -342,11 +375,11 @@ TEST(RGWRateLimitEntry, accept_bw) info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 1, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, check_bw_debt_at_max_120secs) { @@ -356,11 +389,11 @@ TEST(RGWRateLimitEntry, check_bw_debt_at_max_120secs) info.enabled = true; info.max_read_bytes = 2; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 100, &info); time += 121s; - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, check_that_bw_limit_not_affect_ops) { @@ -371,11 +404,11 @@ TEST(RGWRateLimitEntry, check_that_bw_limit_not_affect_ops) info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(true, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_GT(delay, 0); } TEST(RGWRateLimitEntry, read_limit_does_not_affect_writes) { @@ -386,11 +419,11 @@ TEST(RGWRateLimitEntry, read_limit_does_not_affect_writes) info.max_read_ops = 1; info.max_read_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Write, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Write, &info, time); entry.decrease_bytes(false, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Write, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Write, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, write_limit_does_not_affect_reads) { @@ -402,11 +435,11 @@ TEST(RGWRateLimitEntry, write_limit_does_not_affect_reads) info.max_write_bytes = 100000000; auto time = ceph::coarse_real_clock::now().time_since_epoch(); std::string key = "uuser123"; - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); entry.decrease_bytes(true, 10000, &info); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, allow_unlimited_access) @@ -416,8 +449,8 @@ TEST(RGWRateLimitEntry, allow_unlimited_access) RGWRateLimitInfo info; info.enabled = true; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } @@ -438,10 +471,10 @@ TEST(RGWRateLimit, reject_list_op_over_limit) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, accept_list_op_after_giveback) @@ -455,11 +488,11 @@ TEST(RGWRateLimit, accept_list_op_after_giveback) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); ratelimit.giveback_tokens("GET", key, RES_LIST_TYPE_2, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, accept_list_op_after_refill) @@ -473,10 +506,10 @@ TEST(RGWRateLimit, accept_list_op_after_refill) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); time += 61s; - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, list_limit_does_not_affect_reads) @@ -491,10 +524,10 @@ TEST(RGWRateLimit, list_limit_does_not_affect_reads) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); // Should still be able to do a normal GET (read) - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, read_limit_does_not_affect_lists) @@ -509,10 +542,10 @@ TEST(RGWRateLimit, read_limit_does_not_affect_lists) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); // Should still be able to do a LIST op - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, list_limit_does_not_affect_writes) @@ -527,10 +560,10 @@ TEST(RGWRateLimit, list_limit_does_not_affect_writes) info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); // Should still be able to do a PUT (write) - success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, unlimited_access_not_left_large_list_ops_budget) @@ -546,13 +579,13 @@ TEST(RGWRateLimit, unlimited_access_not_left_large_list_ops_budget) for (int i = 0; i < 10; i++) { - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } time += 61s; info.max_list_ops = 1; // make list ops limited - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, write_limit_does_not_affect_lists) @@ -567,10 +600,10 @@ TEST(RGWRateLimit, write_limit_does_not_affect_lists) info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); // Should still be able to do a LIST op - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, list_limit_does_not_affect_deletes) @@ -585,10 +618,10 @@ TEST(RGWRateLimit, list_limit_does_not_affect_deletes) info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); // Should still be able to do a DELETE op - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, delete_limit_does_not_affect_lists) @@ -603,10 +636,10 @@ TEST(RGWRateLimit, delete_limit_does_not_affect_lists) info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); // Should still be able to do a LIST op - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_LIST_TYPE_2); + EXPECT_EQ(0, delay); } // LIST RES_DELIMITER minimal tests @@ -621,10 +654,10 @@ TEST(RGWRateLimit, reject_delimiter_op_over_limit) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, accept_delimiter_op_after_giveback) @@ -638,11 +671,11 @@ TEST(RGWRateLimit, accept_delimiter_op_after_giveback) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); ratelimit.giveback_tokens("GET", key, RES_DELIMITER, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, accept_delimiter_op_after_refill) @@ -656,10 +689,10 @@ TEST(RGWRateLimit, accept_delimiter_op_after_refill) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); time += 61s; - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_DELIMITER); + EXPECT_EQ(0, delay); } // LIST RES_PREFIX minimal tests @@ -674,10 +707,10 @@ TEST(RGWRateLimit, reject_prefix_op_over_limit) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, accept_prefix_op_after_giveback) @@ -691,11 +724,11 @@ TEST(RGWRateLimit, accept_prefix_op_after_giveback) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); ratelimit.giveback_tokens("GET", key, RES_PREFIX, &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, accept_prefix_op_after_refill) @@ -709,10 +742,10 @@ TEST(RGWRateLimit, accept_prefix_op_after_refill) info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_list"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); time += 61s; - success = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, RES_PREFIX); + EXPECT_EQ(0, delay); } @@ -724,10 +757,10 @@ TEST(RGWRateLimitEntry, reject_list_op_over_limit) info.enabled = true; info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(true, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_GT(delay, 0); } TEST(RGWRateLimitEntry, accept_list_op_after_giveback) @@ -738,11 +771,11 @@ TEST(RGWRateLimitEntry, accept_list_op_after_giveback) info.enabled = true; info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); entry.giveback_tokens(OpType::List); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, accept_list_op_after_refill) @@ -753,10 +786,10 @@ TEST(RGWRateLimitEntry, accept_list_op_after_refill) info.enabled = true; info.max_list_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); time += 61s; - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, list_limit_does_not_affect_reads) @@ -768,10 +801,10 @@ TEST(RGWRateLimitEntry, list_limit_does_not_affect_reads) info.max_list_ops = 1; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, read_limit_does_not_affect_lists) @@ -783,10 +816,10 @@ TEST(RGWRateLimitEntry, read_limit_does_not_affect_lists) info.max_list_ops = 1; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, list_limit_does_not_affect_writes) @@ -798,10 +831,10 @@ TEST(RGWRateLimitEntry, list_limit_does_not_affect_writes) info.max_list_ops = 1; info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Write, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Write, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, write_limit_does_not_affect_lists) @@ -813,10 +846,10 @@ TEST(RGWRateLimitEntry, write_limit_does_not_affect_lists) info.max_list_ops = 1; info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Write, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Write, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, list_limit_does_not_affect_deletes) @@ -828,10 +861,10 @@ TEST(RGWRateLimitEntry, list_limit_does_not_affect_deletes) info.max_list_ops = 1; info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::List, &info, time); + int64_t delay = entry.should_rate_limit(OpType::List, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, delete_limit_does_not_affect_lists) @@ -843,10 +876,10 @@ TEST(RGWRateLimitEntry, delete_limit_does_not_affect_lists) info.max_list_ops = 1; info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::List, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::List, &info, time); + EXPECT_EQ(0, delay); } @@ -861,10 +894,10 @@ TEST(RGWRateLimit, reject_delete_op_over_limit) info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(true, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_GT(delay, 0); } TEST(RGWRateLimit, accept_delete_op_after_giveback) @@ -878,11 +911,11 @@ TEST(RGWRateLimit, accept_delete_op_after_giveback) info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); ratelimit.giveback_tokens("DELETE", key, "", &info); time = ceph::coarse_real_clock::now(); - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, accept_delete_op_after_refill) @@ -896,10 +929,10 @@ TEST(RGWRateLimit, accept_delete_op_after_refill) info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); time += 61s; - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, delete_limit_does_not_affect_reads) @@ -914,10 +947,10 @@ TEST(RGWRateLimit, delete_limit_does_not_affect_reads) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); // Should still be able to do a normal GET (read) - success = ratelimit.should_rate_limit("GET", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, read_limit_does_not_affect_deletes) @@ -932,10 +965,10 @@ TEST(RGWRateLimit, read_limit_does_not_affect_deletes) info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("GET", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("GET", key, time, &info, ""); // Should still be able to do a DELETE op - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, write_limit_does_not_affect_deletes) @@ -950,10 +983,10 @@ TEST(RGWRateLimit, write_limit_does_not_affect_deletes) info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); // Should still be able to do a DELETE op - success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, delete_limit_does_not_affect_writes) @@ -968,10 +1001,10 @@ TEST(RGWRateLimit, delete_limit_does_not_affect_writes) info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now(); std::string key = "uuser_delete"; - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); // Should still be able to do a PUT (write) - success = ratelimit.should_rate_limit("PUT", key, time, &info, ""); - EXPECT_EQ(false, success); + delay = ratelimit.should_rate_limit("PUT", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimit, unlimited_access_not_left_large_delete_ops_budget) @@ -987,13 +1020,13 @@ TEST(RGWRateLimit, unlimited_access_not_left_large_delete_ops_budget) for (int i = 0; i < 10; i++) { - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } time += 61s; info.max_delete_ops = 1; // make delete ops limited - bool success = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); - EXPECT_EQ(false, success); + int64_t delay = ratelimit.should_rate_limit("DELETE", key, time, &info, ""); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, reject_delete_op_over_limit) @@ -1004,10 +1037,10 @@ TEST(RGWRateLimitEntry, reject_delete_op_over_limit) info.enabled = true; info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(true, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_GT(delay, 0); } TEST(RGWRateLimitEntry, accept_delete_op_after_giveback) @@ -1018,11 +1051,11 @@ TEST(RGWRateLimitEntry, accept_delete_op_after_giveback) info.enabled = true; info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); entry.giveback_tokens(OpType::Delete); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, accept_delete_op_after_refill) @@ -1033,10 +1066,10 @@ TEST(RGWRateLimitEntry, accept_delete_op_after_refill) info.enabled = true; info.max_delete_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); time += 61s; - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, delete_limit_does_not_affect_reads) @@ -1048,10 +1081,10 @@ TEST(RGWRateLimitEntry, delete_limit_does_not_affect_reads) info.max_delete_ops = 1; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Read, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Read, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, read_limit_does_not_affect_deletes) @@ -1063,10 +1096,10 @@ TEST(RGWRateLimitEntry, read_limit_does_not_affect_deletes) info.max_delete_ops = 1; info.max_read_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Read, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Read, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, write_limit_does_not_affect_deletes) @@ -1078,10 +1111,10 @@ TEST(RGWRateLimitEntry, write_limit_does_not_affect_deletes) info.max_delete_ops = 1; info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Write, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Write, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Delete, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Delete, &info, time); + EXPECT_EQ(0, delay); } TEST(RGWRateLimitEntry, delete_limit_does_not_affect_writes) @@ -1093,9 +1126,9 @@ TEST(RGWRateLimitEntry, delete_limit_does_not_affect_writes) info.max_delete_ops = 1; info.max_write_ops = 1; auto time = ceph::coarse_real_clock::now().time_since_epoch(); - bool success = entry.should_rate_limit(OpType::Delete, &info, time); + int64_t delay = entry.should_rate_limit(OpType::Delete, &info, time); time = ceph::coarse_real_clock::now().time_since_epoch(); - success = entry.should_rate_limit(OpType::Write, &info, time); - EXPECT_EQ(false, success); + delay = entry.should_rate_limit(OpType::Write, &info, time); + EXPECT_EQ(0, delay); } From 4a41a9b4ec7b412cc40490a995c21371e7046404 Mon Sep 17 00:00:00 2001 From: Tatjana Dehler Date: Thu, 8 Oct 2026 16:24:33 +0200 Subject: [PATCH 04/20] release: add backport metadata Signed-off-by: Tatjana Dehler --- .../01a11a76-84c1-7371-840f-b82c5fbf1a72.md | 144 ++++++++++++++++++ 1 file changed, 144 insertions(+) create mode 100644 release-management/backports/01a11a76-84c1-7371-840f-b82c5fbf1a72.md diff --git a/release-management/backports/01a11a76-84c1-7371-840f-b82c5fbf1a72.md b/release-management/backports/01a11a76-84c1-7371-840f-b82c5fbf1a72.md new file mode 100644 index 000000000000..19baad17f0fa --- /dev/null +++ b/release-management/backports/01a11a76-84c1-7371-840f-b82c5fbf1a72.md @@ -0,0 +1,144 @@ +--- +id: 01a11a76-84c1-7371-840f-b82c5fbf1a72 +title: rate limit "retry after" header + "rgw_ratelimit_interval" change +component: rgw +category: feature +provenance: + type: upstream_pr + upstream_prs: + - https://github.com/ceph/ceph/pull/64762 + - https://github.com/ceph/ceph/pull/66509 + - https://github.com/ceph/ceph/pull/68210 + upstream_pr_state: merged-main + upstream_commits: + - dae572d50080609c77d7131cfc99b1fb3f16d31b + - 91e9f00a7543c4651a983c2d785696a069d8ab28 + - 3dae903c9a31fe5161058a4bb1cfe4092839eb29 + upstream_trackers: + - https://tracker.ceph.com/issues/72894 + - https://tracker.ceph.com/issues/74088 + - https://tracker.ceph.com/issues/64083 + - https://tracker.ceph.com/issues/67655 +justification: + type: customer + refs: + - cobaltcore-dev/cloud-storage#588 + - todo + - '```' + - https://github.com/ceph/ceph/commit/3dae903c9a31fe5161058a4bb1cfe4092839eb29 + - https://tracker.ceph.com/issues/64083 + - https://tracker.ceph.com/issues/67655 + - 'plus:' + - ceph/ceph#64762 + - See also the squid backport effort for retry after that includes the + - ratelimit_interval change + - https://github.com/cobaltcore-dev/ceph/pull/19/commits + +risk: + # Stage-B — fill these during PR prep. See release-management/backports/RISK-RUBRIC.md. + blast: availability + conflict: trivial + coverage: partial +data_structure_change: + struct_v_bump: RGWRateLimitInfo 1 -> 2 + upstream_coordinated: true +--- + +## Justification (public) + +Clients rejected by RGW rate limiting receive no guidance on when to retry, so +they either back off far longer than necessary or hammer the endpoint until +they succeed. This backport adds a `Retry-After` header to rate-limited +responses, computed from the client's actual token deficit and replenishment +rate. + +It also adds rate limiting for LIST and DELETE operations, which previously +could not be limited independently of general read and write traffic, and makes +the token bucket accumulation window configurable via `rgw_ratelimit_interval`. + +dditionally, the backport fixes a bug. With rate limiting enabled but an +individual limit left at 0 (unlimited), ops tokens are still deducted per +request while never being refilled. That is harmless on its own, because the +credit isn't checked while the limit is 0. But if the operator later sets that +limit, the credit may already be far negative and recovers only at the newly +onfigured rate, so the user can be blocked for a long time. + +## Justification (internal) + +Three upstream changes are part of the backport: + +1. **ceph/ceph#64762** — LIST/DELETE rate limiting, `rgw_ratelimit_interval`. +2. **ceph/ceph#66509** — fixes unbounded negative ops budget. +3. **ceph/ceph#68210** — adds the `Retry-After` header itself. + +PR 1. and 3. are required by the customer. 2. is needed addition to 1. because +it resolves a bug and omits merge conflicts. + +## Risk notes + +**Blast: availability.** +- The backport changes the request admission. A defect is more likely to change + how traffic is accepted or rejected than to corrupt data. +- `dae572` bumps `RGWRateLimitInfo` from struct_v 1 to 2 to add `max_list_ops` + and `max_delete_ops`. Reading old records with the new build is safe: the + `struct_v >= 2` guards default both new fields to `0`, so v1 records decode + and behave as before. + The other direction is not. `compat_v` stays at `1` while the new fields are + inserted in the middle of the field order. So an old decoder doesn't reject a + v2 record. It reads it positionally and shifts the fields: `max_list_ops` + ends up in `max_write_bytes`, `max_delete_ops` in `max_read_bytes`, and + `enabled` gets the low byte of the real `max_write_bytes`. An RGW on the old + build reading a record written with `max_list_ops = 100` applies a 100 byte + write bandwidth limit. There is no decode error, just wrong values. + v2 records only appear once a rate limit is written by the new build + (`radosgw-admin ratelimit set` or the admin REST API). The upgrade itself + does not rewrite stored records, so an upgrade without a config change + produces no v2 data. +- Additionally, there is a minor issue the backport (`91e9f00`) introduces. If + the user is charged for a request and the bucket rejects the request, the + user gets the token back. The problem: it also happens if the user has + unlimited access. + So the amount of credits for a user can grow over time. However, it doesn't + matter much because it's corrected by increase_tokens, min(max * 1000, …) on + the next refill. Compared to the issue the commit is fixing, it's a minor one. + +**Conflict: trivial.** `91e9f00` and `3dae903` both cherry-pick onto the branch +with no conflicts at all. Only `dae572` was cherry-picked with minor conflicts +that are documented as part of the commit message. + +**Coverage: partial.** +- `91e9f00` adds four regression tests + (`unlimited_access_not_left_large_*_ops_budget`) +- `3dae903` adds + `TEST(RGWRateLimitEntry, compute_delay)` and updates the existing suite to + the new `int64_t` return +- `dae572` contributed both unit and integration tests for the new admin + options. + +No test covers the refund asymmetry described above. + +## Behavior change + +- Rate-limited S3 responses now carry a `Retry-After` header giving the seconds + until the client's credit recovers. The status code is unchanged, still + `503` (Swift: `498`). Operators who need `429` must remap at the proxy layer. +- New rate limit dimensions `max_list_ops` and `max_delete_ops` can be set via + `radosgw-admin ratelimit set` and the admin REST API. Both default to `0` + (unlimited). +- New config option `rgw_ratelimit_interval` (default `60` seconds) controls + the token bucket accumulation window. Previously hardcoded at 60, so it stays + the same in case of the default value. +- Ops counters no longer accumulate unbounded debt while a limit is `0`. A user + who would previously have been rejected for hours or days on first enabling a + limit now starts from a clean budget. + +## Upgrade notes + +`rgw_ratelimit_interval` is flagged `startup`, so changing it from the default +requires an RGW restart. + +Don't change any rate limit configuration until all RGWs run the new build.`RGW_ATTR_RATELIMIT` is shared state read by every RGW, radosgw-admin and the +admin REST API. A `ratelimit set` during the rollout rewrites the record as v2 +and any RGW still on the old build misreads it (see Risk notes). Reads are +unaffected, so the rolling restart itself is safe, only writes create v2 +records. From 5c3e965a174140e7be985d4eaa98096880f7a1a9 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Sat, 26 Sep 2026 16:22:29 -0400 Subject: [PATCH 05/20] mgr/dashboard: tolerate nodeenv leaving no src directory behind nodeenv 1.11.0 (released 2026-09-26) no longer leaves the node-env/src directory behind after installing a prebuilt node. The build step runs an unpinned `pip install nodeenv`, so as of today every build with WITH_MGR_DASHBOARD_FRONTEND fails at chown: cannot access '.../node-env/src': No such file or directory ninja: build stopped: subcommand failed. (e.g. ceph-pr-pipeline build 2504). The chown is a workaround for tarball uid mapping in containers and is only needed when the src directory exists, so skip it when there is nothing to fix up. Assisted-by: Claude Fable 5 Signed-off-by: David Galloway (cherry picked from commit 5aff2216101316107dbaa66e6b26a9afc59d5853) Conflicts: src/pybind/mgr/dashboard/frontend/CMakeLists.txt squid still creates the npm cache directory with "mkdir -p" and has no DEPENDS on setup-virtualenv.sh; kept those lines as they are and took only the guarded chown and its comment --- src/pybind/mgr/dashboard/frontend/CMakeLists.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/CMakeLists.txt b/src/pybind/mgr/dashboard/frontend/CMakeLists.txt index bb8feb7648d1..af28daeb114f 100644 --- a/src/pybind/mgr/dashboard/frontend/CMakeLists.txt +++ b/src/pybind/mgr/dashboard/frontend/CMakeLists.txt @@ -73,9 +73,12 @@ else(WITH_SYSTEM_NPM) # ensure that the files that nodeenv unpacks from tarballs are owned by # the current user. This can be an issue due to the node tarball using # uid 1000 and running the unpack in a id-mapped namespace (container) - # that lets tar set the uid to a "bad" uid outside the namespace - COMMAND bash -c 'chown -R `id -u`:`id -g` ${mgr-dashboard-nodeenv-dir}/src' - COMMAND mkdir -p ${npm-cache-dir} + # that lets tar set the uid to a "bad" uid outside the namespace. + # nodeenv >= 1.11.0 no longer leaves the src directory behind after a + # prebuilt node install, so tolerate it being absent + COMMAND bash -c 'test ! -e ${mgr-dashboard-nodeenv-dir}/src || chown -R `id -u`:`id -g` ${mgr-dashboard-nodeenv-dir}/src' + COMMAND ${CMAKE_COMMAND} -E make_directory ${npm-cache-dir} + DEPENDS ${CMAKE_SOURCE_DIR}/src/tools/setup-virtualenv.sh WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} COMMENT "dashboard nodeenv is being installed") if(DEFINED ENV{NPM_REGISTRY}) From 39b8ce40eb6e8069d56b8ea19fb41442c1f8fdd7 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 12:50:24 -0400 Subject: [PATCH 06/20] mon/MonClient: delete MonConnection move assignment 3e885abc2ae ("mon/MonClient: add assertions for monc lock in MonConnection") added a `ceph::mutex& monc_lock` reference member to MonConnection, which makes the explicitly defaulted move assignment operator implicitly deleted. clang (-Wdefaulted-function-deleted, promoted by -Werror) rejects that, so every make check / API build on main fails since that merge: src/mon/MonClient.h:70:18: error: explicitly defaulted move assignment operator is implicitly deleted [-Werror,-Wdefaulted-function-deleted] Nothing move-assigns a MonConnection (pending_cons/active_con only ever move-construct), so spell out what the compiler already decided. Fixes: 3e885abc2ae3ae50fe443046594a01ece386657b Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 79296fc969e68bcc0656fd61411c28d318232a40) (cherry picked from commit 3c86b2d880bba56f762540b4c1f152358e031405) --- src/mon/MonClient.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index 2e0c739f4b42..27ebc0e873f9 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -64,7 +64,7 @@ class MonConnection { ceph::mutex& m); ~MonConnection(); MonConnection(MonConnection&& rhs) = default; - MonConnection& operator=(MonConnection&&) = default; + MonConnection& operator=(MonConnection&&) = delete; MonConnection(const MonConnection& rhs) = delete; MonConnection& operator=(const MonConnection&) = delete; int handle_auth(MAuthReply *m, From 9763e6af16cfbc435db014b9fe6ee4c6f0402c40 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 12:50:24 -0400 Subject: [PATCH 07/20] qa/suites/rados/upgrade/cephx: add missing .qa symlinks The cephx upgrade suites added with the CVE-2025-30156 merge are missing the .qa links in X/ms_bind, X/tasks/07-client-auth-keys, X/tasks/08-allowed-ciphers and the release-X counterparts, so the "Check for missing .qa links" workflow (src/script/verify-qa) fails on every PR based on current main. Add the links verify-qa asks for. Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 62c4d50f6ae3dd8ab61be2c774e6ca72ef0b18b1) (cherry picked from commit 89e648232399135f1e214076344fad9ef6311c53) --- qa/suites/rados/upgrade/cephx/X/ms_bind/.qa | 1 + qa/suites/rados/upgrade/cephx/X/tasks/07-client-auth-keys/.qa | 1 + qa/suites/rados/upgrade/cephx/X/tasks/08-allowed-ciphers/.qa | 1 + qa/suites/rados/upgrade/cephx/release-X/ms_bind/.qa | 1 + .../rados/upgrade/cephx/release-X/tasks/07-client-auth-keys/.qa | 1 + .../rados/upgrade/cephx/release-X/tasks/08-allowed-ciphers/.qa | 1 + 6 files changed, 6 insertions(+) create mode 120000 qa/suites/rados/upgrade/cephx/X/ms_bind/.qa create mode 120000 qa/suites/rados/upgrade/cephx/X/tasks/07-client-auth-keys/.qa create mode 120000 qa/suites/rados/upgrade/cephx/X/tasks/08-allowed-ciphers/.qa create mode 120000 qa/suites/rados/upgrade/cephx/release-X/ms_bind/.qa create mode 120000 qa/suites/rados/upgrade/cephx/release-X/tasks/07-client-auth-keys/.qa create mode 120000 qa/suites/rados/upgrade/cephx/release-X/tasks/08-allowed-ciphers/.qa diff --git a/qa/suites/rados/upgrade/cephx/X/ms_bind/.qa b/qa/suites/rados/upgrade/cephx/X/ms_bind/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/X/ms_bind/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file diff --git a/qa/suites/rados/upgrade/cephx/X/tasks/07-client-auth-keys/.qa b/qa/suites/rados/upgrade/cephx/X/tasks/07-client-auth-keys/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/X/tasks/07-client-auth-keys/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file diff --git a/qa/suites/rados/upgrade/cephx/X/tasks/08-allowed-ciphers/.qa b/qa/suites/rados/upgrade/cephx/X/tasks/08-allowed-ciphers/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/X/tasks/08-allowed-ciphers/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file diff --git a/qa/suites/rados/upgrade/cephx/release-X/ms_bind/.qa b/qa/suites/rados/upgrade/cephx/release-X/ms_bind/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/release-X/ms_bind/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file diff --git a/qa/suites/rados/upgrade/cephx/release-X/tasks/07-client-auth-keys/.qa b/qa/suites/rados/upgrade/cephx/release-X/tasks/07-client-auth-keys/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/release-X/tasks/07-client-auth-keys/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file diff --git a/qa/suites/rados/upgrade/cephx/release-X/tasks/08-allowed-ciphers/.qa b/qa/suites/rados/upgrade/cephx/release-X/tasks/08-allowed-ciphers/.qa new file mode 120000 index 000000000000..a602a0353e75 --- /dev/null +++ b/qa/suites/rados/upgrade/cephx/release-X/tasks/08-allowed-ciphers/.qa @@ -0,0 +1 @@ +../.qa/ \ No newline at end of file From 8a639d150de8ac9f3da9409722fabb9f11fb2d98 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 13:33:21 -0400 Subject: [PATCH 08/20] mgr/cephadm: skip mgr/mon key rotation when no mon daemons are known fae0d0db8db ("mgr/cephadm: rotate keyring for core ceph daemons during upgrade") made _upgrade_daemons() call _rotate_mgr_mon_auth_keys() on every pass. That function indexes mon_daemons[0] unconditionally, and with an empty mon list _detect_need_upgrade() reports no mons needing upgrade, which is taken as "all mons upgraded" -- so it goes on to set auth_allowed_ciphers and rotate mgr keys before crashing with IndexError: list index out of range cephadm/upgrade.py:1795 Since that merge four cephadm unit tests fail deterministically in run_tox_mgr on every PR (test_upgrade_daemons_offline_hosts, test_upgrade_run[False/True], test_do_upgrade_limit_exhausted_marks_complete_without_scope_check); they exercise the upgrade machinery without any mon daemons in the cache. Without any known mon daemons we cannot tell whether the mons have been upgraded and there is no mon keyring to rotate, so skip the rotation for this pass and let the next upgrade pass retry once the cache has mons. Fixes: fae0d0db8dbc9f883f43f36e4735f2b456e79a9d Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit fa100f02071bd1dffe2ca3f5e9afdb1b06d8c85e) (cherry picked from commit d742babe59886393a10c9f6531f3b5ad00ebe2d6) --- src/pybind/mgr/cephadm/upgrade.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pybind/mgr/cephadm/upgrade.py b/src/pybind/mgr/cephadm/upgrade.py index 090c95fbf08e..6beb0f5ea99f 100644 --- a/src/pybind/mgr/cephadm/upgrade.py +++ b/src/pybind/mgr/cephadm/upgrade.py @@ -1081,6 +1081,12 @@ def _rotate_mgr_mon_auth_keys(self, target_image: str, target_digests: Optional[ self.upgrade_state.rotated_mgr_mon_auth_key_daemons = [] # do mgr and mon keyrings as one off after mons have been upgraded mon_daemons = self.mgr.cache.get_daemons_by_service('mon') + if not mon_daemons: + # Without any mon daemons in the cache we cannot tell whether + # the mons have been upgraded, and there is no mon keyring to + # rotate either. Try again on the next upgrade pass. + self.mgr.log.debug('Skipping mgr/mon key rotation, no mon daemons known') + return _, mons_needing_upgrade, __, ___ = self._detect_need_upgrade(mon_daemons, target_digests, target_image) need_rotate_self = False if not mons_needing_upgrade: From 42991c7d040a576ebf38f85cf368110f16b4cc4a Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 13:37:00 -0400 Subject: [PATCH 09/20] common/entity_name: print type names in get_valid_types_as_str() 0753b1d5326 ("common/entity_name: dump type name as string") turned STR_TO_ENTITY_TYPE into a vector of (entity_type_t, std::string) pairs but left get_valid_types_as_str() streaming .first, i.e. the uint8_t type value, so e.g. `ceph-conf --name total.garbage` now says valid types are: , \x01, \x04, \x02, \x10, \x08 instead of "auth, mon, osd, mds, mgr, client", and the src/test/cli/ceph-conf/invalid-args.t cram test fails in run-cli-tests. Stream the name (.second). Fixes: 0753b1d53262212fbc8e8ebc32cf6b695bf1ddf4 Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 18e32faf20ad2cfde6ef74e8a505816365767618) (cherry picked from commit 36982bbe0f4f58e9ac0d3fbf5155a764e028975a) --- src/common/entity_name.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/entity_name.cc b/src/common/entity_name.cc index 895f32f2b6fe..1cc46fa4dbb2 100644 --- a/src/common/entity_name.cc +++ b/src/common/entity_name.cc @@ -149,7 +149,7 @@ std::string EntityName::get_valid_types_as_str() { if (i > 0) { out << ", "; } - out << STR_TO_ENTITY_TYPE[i].first; + out << STR_TO_ENTITY_TYPE[i].second; } return out.str(); } From a9c6f3c9fc61db664d419b9c31e9ad8b0481efe8 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 13:37:15 -0400 Subject: [PATCH 10/20] test/cli: update cram expectations for cephx auth changes The CVE-2025-30156 cephx merge (96335615c1b) added a -t/--key-type option to ceph-authtool, --auth-service-cipher/--auth-allowed-ciphers/ --auth-preferred-cipher options and auth_epoch/auth_*_cipher fields to monmaptool, and the cephx_auth_aes256k mon feature bit, without updating the cram expectations under src/test/cli, so run-cli-tests fails on every PR. Update the ten affected .t files with the output the tools now produce. Fixes: 96335615c1bfdd598412d8768be09323d299ba33 Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) Conflicts: src/test/cli/monmaptool/feature-set-unset-list.t The mon feature lists in the expected output are release-specific, so the expectations were rebuilt from this branch's src/mon/mon_types.h (inserting cephx_auth_aes256k(2147483648) before nvmeof_beacon_diff(4294967296) in tentacle's supported/persistent feature sets) instead of taking main's lists. (cherry picked from commit a9351397d512c386c4a6582c7cd5f5beb7024653) (cherry picked from commit 7c4745bad91af9ebfb47a1c2c90a96b047bf24e0) --- src/test/cli/ceph-authtool/help.t | 1 + src/test/cli/monmaptool/add-exists.t | 4 +++ src/test/cli/monmaptool/add-many.t | 4 +++ src/test/cli/monmaptool/clobber.t | 8 ++++++ src/test/cli/monmaptool/create-print.t | 8 ++++++ src/test/cli/monmaptool/create-with-add.t | 4 +++ .../cli/monmaptool/feature-set-unset-list.t | 28 +++++++++---------- src/test/cli/monmaptool/help.t | 3 ++ src/test/cli/monmaptool/rm-nonexistent.t | 4 +++ src/test/cli/monmaptool/rm.t | 4 +++ 10 files changed, 54 insertions(+), 14 deletions(-) diff --git a/src/test/cli/ceph-authtool/help.t b/src/test/cli/ceph-authtool/help.t index 68f4a9699587..9d8b0cf67bc1 100644 --- a/src/test/cli/ceph-authtool/help.t +++ b/src/test/cli/ceph-authtool/help.t @@ -11,6 +11,7 @@ existing keyringfile -g, --gen-key will generate a new secret key for the specified entityname + -t, --key-type KEY_TYPE set the key-type --gen-print-key will generate a new secret key without set it to the keyringfile, prints the secret to stdout --import-keyring FILE will import the content of a given keyring diff --git a/src/test/cli/monmaptool/add-exists.t b/src/test/cli/monmaptool/add-exists.t index d223f7bec8d6..4220a4b75855 100644 --- a/src/test/cli/monmaptool/add-exists.t +++ b/src/test/cli/monmaptool/add-exists.t @@ -24,6 +24,10 @@ min_mon_release 20 (tentacle) election_strategy: 1 0: v1:2.3.4.5:6789/0 mon.foo + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ NEW_FSID="$(monmaptool --print mymonmap|grep ^fsid)" $ [ "$ORIG_FSID" = "$NEW_FSID" ] diff --git a/src/test/cli/monmaptool/add-many.t b/src/test/cli/monmaptool/add-many.t index 2f61888c9e9d..1d5de755846c 100644 --- a/src/test/cli/monmaptool/add-many.t +++ b/src/test/cli/monmaptool/add-many.t @@ -30,6 +30,10 @@ 1: [v2:172.21.15.68:6791/0,v1:172.21.15.68:6792/0] mon.fiz 2: v2:3.4.5.6:7890/0 mon.bar 3: v2:4.5.6.7:8901/0 mon.baz + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ NEW_FSID="$(monmaptool --print mymonmap|grep ^fsid)" $ [ "$ORIG_FSID" = "$NEW_FSID" ] diff --git a/src/test/cli/monmaptool/clobber.t b/src/test/cli/monmaptool/clobber.t index 963e1ebeda1c..779ac895166c 100644 --- a/src/test/cli/monmaptool/clobber.t +++ b/src/test/cli/monmaptool/clobber.t @@ -21,6 +21,10 @@ min_mon_release 20 (tentacle) election_strategy: 1 0: v1:2.3.4.5:6789/0 mon.foo + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ NEW_FSID="$(monmaptool --print mymonmap|grep ^fsid)" $ [ "$ORIG_FSID" = "$NEW_FSID" ] @@ -42,3 +46,7 @@ created \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.\d\d\d\d (re) min_mon_release 20 (tentacle) election_strategy: 1 + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k diff --git a/src/test/cli/monmaptool/create-print.t b/src/test/cli/monmaptool/create-print.t index bf99d2589930..542cab69eb37 100644 --- a/src/test/cli/monmaptool/create-print.t +++ b/src/test/cli/monmaptool/create-print.t @@ -12,6 +12,10 @@ created \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.\d\d\d\d (re) min_mon_release 20 (tentacle) election_strategy: 1 + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ monmaptool --print -- mymonmap monmaptool: monmap file mymonmap @@ -21,3 +25,7 @@ created \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.\d\d\d\d (re) min_mon_release 20 (tentacle) election_strategy: 1 + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k diff --git a/src/test/cli/monmaptool/create-with-add.t b/src/test/cli/monmaptool/create-with-add.t index 9aaaa7e6c8d3..8350c1c876f0 100644 --- a/src/test/cli/monmaptool/create-with-add.t +++ b/src/test/cli/monmaptool/create-with-add.t @@ -13,3 +13,7 @@ min_mon_release 20 (tentacle) election_strategy: 1 0: v1:2.3.4.5:6789/0 mon.foo + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k diff --git a/src/test/cli/monmaptool/feature-set-unset-list.t b/src/test/cli/monmaptool/feature-set-unset-list.t index 65841b7600ef..ca303ef39dc0 100644 --- a/src/test/cli/monmaptool/feature-set-unset-list.t +++ b/src/test/cli/monmaptool/feature-set-unset-list.t @@ -12,21 +12,21 @@ required: [none] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] MONMAP FEATURES: persistent: [none] optional: [none] required: [none] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] monmap:persistent:[none] monmap:optional:[none] monmap:required:[none] - available:supported:[kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - available:persistent:[kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + available:supported:[kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + available:persistent:[kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] $ monmaptool --feature-set foo /tmp/test.monmap.1234 unknown features name 'foo' or unable to parse value: Expected option value to be integer, got 'foo' @@ -45,8 +45,8 @@ required: [kraken(1),octopus(32),unknown(4096)] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] $ monmaptool --feature-unset 32 --optional --feature-list /tmp/test.monmap.1234 monmaptool: monmap file /tmp/test.monmap.1234 @@ -56,8 +56,8 @@ required: [kraken(1),octopus(32),unknown(4096)] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] monmaptool: writing epoch 0 to /tmp/test.monmap.1234 (1 monitors) $ monmaptool --feature-unset 32 --persistent --feature-unset 4096 --optional --feature-list /tmp/test.monmap.1234 @@ -68,8 +68,8 @@ required: [kraken(1)] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] monmaptool: writing epoch 0 to /tmp/test.monmap.1234 (1 monitors) $ monmaptool --feature-unset kraken --feature-list /tmp/test.monmap.1234 @@ -80,8 +80,8 @@ required: [none] AVAILABLE FEATURES: - supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] - persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),nvmeof_beacon_diff(4294967296)] + supported: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] + persistent: [kraken(1),luminous(2),mimic(4),osdmap-prune(8),nautilus(16),octopus(32),pacific(64),elector-pinging(128),quincy(256),reef(512),squid(1024),tentacle(2048),cephx_auth_aes256k(2147483648),nvmeof_beacon_diff(4294967296)] monmaptool: writing epoch 0 to /tmp/test.monmap.1234 (1 monitors) $ rm /tmp/test.monmap.1234 diff --git a/src/test/cli/monmaptool/help.t b/src/test/cli/monmaptool/help.t index f12c1a07d003..e8c2223fe1f8 100644 --- a/src/test/cli/monmaptool/help.t +++ b/src/test/cli/monmaptool/help.t @@ -8,4 +8,7 @@ [--feature-set [--optional|--persistent]] [--feature-unset [--optional|--persistent]] [--set-min-mon-release ] + [--auth-service-cipher ] + [--auth-allowed-ciphers ] + [--auth-preferred-cipher ] diff --git a/src/test/cli/monmaptool/rm-nonexistent.t b/src/test/cli/monmaptool/rm-nonexistent.t index c6709f934d0b..dfb56a2a8e64 100644 --- a/src/test/cli/monmaptool/rm-nonexistent.t +++ b/src/test/cli/monmaptool/rm-nonexistent.t @@ -22,6 +22,10 @@ min_mon_release 20 (tentacle) election_strategy: 1 0: v1:2.3.4.5:6789/0 mon.foo + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ NEW_FSID="$(monmaptool --print mymonmap|grep ^fsid)" $ [ "$ORIG_FSID" = "$NEW_FSID" ] diff --git a/src/test/cli/monmaptool/rm.t b/src/test/cli/monmaptool/rm.t index b84d15ea23f1..6194a86ea41c 100644 --- a/src/test/cli/monmaptool/rm.t +++ b/src/test/cli/monmaptool/rm.t @@ -19,6 +19,10 @@ created \d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d+.\d\d\d\d (re) min_mon_release 20 (tentacle) election_strategy: 1 + auth_epoch 0 + auth_service_cipher aes256k + auth_allowed_ciphers aes256k + auth_preferred_cipher aes256k $ NEW_FSID="$(monmaptool --print mymonmap|grep ^fsid)" $ [ "$ORIG_FSID" = "$NEW_FSID" ] From 52c577e85fac4ada8ddab32e99edca232f664634 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 13:37:15 -0400 Subject: [PATCH 11/20] qa/tasks: fix flake8 findings in ceph.py and radosbench.py The cephx merge (96335615c1b) left four unused local variables in qa/tasks/ceph.py (key_prune/key_rotate/task) and an f-string without placeholders plus an unused exception binding in qa/tasks/radosbench.py, so run-tox-qa (flake8 --select=F,E9) fails on every PR. No functional change. Fixes: 96335615c1bfdd598412d8768be09323d299ba33 Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 297c44b7e3ef9affc05027f88ced69149ec67184) (cherry picked from commit 4d90e678de13494547b6e1c6fde665b86d1f816f) --- qa/tasks/ceph.py | 5 ----- qa/tasks/radosbench.py | 4 ++-- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/qa/tasks/ceph.py b/qa/tasks/ceph.py index 81dbdca2d498..84bdbeaaf27f 100644 --- a/qa/tasks/ceph.py +++ b/qa/tasks/ceph.py @@ -1725,8 +1725,6 @@ def key_prune(ctx, config): elif isinstance(config, list): config = {'keys': config} - testdir = teuthology.get_testdir(ctx) - cluster_name = config.setdefault('cluster', 'ceph') manager = ctx.managers[cluster_name] @@ -1789,7 +1787,6 @@ def key_rotate(ctx, config): ] daemons = ctx.daemons.resolve_role_list(config.get('daemons', None), CEPH_ROLE_TYPES, True) - clusters = set() new_mon_key = None for role in daemons: @@ -1831,7 +1828,6 @@ def key_rotate(ctx, config): daemon.restart() clients = config.get('clients', []) - firstmon = teuthology.get_first_mon(ctx, config, cluster_name) keyring = ctx.ceph[cluster_name].keyring for client in clients: if client == 'all': @@ -2272,7 +2268,6 @@ def task(ctx, config): # stop logging health to clog during shutdown, or else we generate # a bunch of scary messages unrelated to our actual run. - firstmon = teuthology.get_first_mon(ctx, config, cluster_name) ctx.ceph[cluster_name].admin.run( args=[ 'sudo', diff --git a/qa/tasks/radosbench.py b/qa/tasks/radosbench.py index 825b5c9c76b5..7bd97ea82fcd 100644 --- a/qa/tasks/radosbench.py +++ b/qa/tasks/radosbench.py @@ -110,7 +110,7 @@ def task(ctx, config): extra_args.append(f'--auth_exit_on_failure={auth_exit_on_failure}') bench_args = [ - f'bench', + 'bench', f'--concurrent-ios={concurrency}', ] @@ -163,7 +163,7 @@ def task(ctx, config): log.info('joining radosbench (timing out after %ss)', timeout) try: run.wait(radosbench.values(), timeout=timeout) - except CommandFailedError as e: + except CommandFailedError: for p in radosbench.values(): if p.exitstatus == expected_rc: pass From bfabf7ecd7b7e8f32bba07a670d9da74b34d9599 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Wed, 19 Aug 2026 15:54:01 -0400 Subject: [PATCH 12/20] cephadm: black-format OSD key rotation code in daemons/ceph.py The OSD keyring rotation changes from the CVE-2025-30156 cephx merge (c9bc85adec3 "cephadm: restart OSD systemd unit after key rotation", 24dc65ef20d "cephadm: Add retries to OSD key rotation") added lines to cephadmlib/daemons/ceph.py that are not formatted the way `tox -e check-black` (black>=23,<24, -l78) expects, so run-tox-cephadm fails on every PR now that the py3 env passes again: would reformat /ceph/src/cephadm/cephadmlib/daemons/ceph.py 1 file would be reformatted, 50 files would be left unchanged. This is the output of `tox -e format-black` (black 23.12.1) on that file; no functional change. Fixes: 24dc65ef20defe9761036f10527e7c5ccbe057bd Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 6b206bc281c05b773b3e24c11b248a45c86ffed4) (cherry picked from commit 7fb3c927e31e430aa5795fd177e63134a06f7120) --- src/cephadm/cephadmlib/daemons/ceph.py | 27 +++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/cephadm/cephadmlib/daemons/ceph.py b/src/cephadm/cephadmlib/daemons/ceph.py index 8d4bed0e9f24..b05bfaac198c 100644 --- a/src/cephadm/cephadmlib/daemons/ceph.py +++ b/src/cephadm/cephadmlib/daemons/ceph.py @@ -339,7 +339,8 @@ def rotate_osd_lv_keyring( osd_lv_data = osd_bluestore_data.get(self.identity.daemon_id, []) if not osd_lv_data: logger.info( - 'No lv data found for OSD.%s, checking for raw OSD', self.identity.daemon_id + 'No lv data found for OSD.%s, checking for raw OSD', + self.identity.daemon_id, ) c_v_container = CephContainer( ctx, @@ -363,8 +364,13 @@ def rotate_osd_lv_keyring( ) raw_list_data = json.loads(out) for osd_data in raw_list_data.values(): - if str(osd_data.get('osd_id', -1)) == str(self.identity.daemon_id): - logger.info('Found data for osd.%s in raw list', str(self.identity.daemon_id)) + if str(osd_data.get('osd_id', -1)) == str( + self.identity.daemon_id + ): + logger.info( + 'Found data for osd.%s in raw list', + str(self.identity.daemon_id), + ) # misnomer since this isn't an LV, but it is the path # we need to pass to the bluestore tool lv_path = osd_data.get('device', None) @@ -476,10 +482,14 @@ def rotate_osd_lv_keyring( f'Out:{out}\n' f'Err:{err}' ) - logger.info(f'Got issue rotating osd keyring using ceph-bluestore-tool:\n{out}\n{err}\nRetrying in {i} seconds') + logger.info( + f'Got issue rotating osd keyring using ceph-bluestore-tool:\n{out}\n{err}\nRetrying in {i} seconds' + ) time.sleep(i) else: - logger.info(f'Successfully rotated osd.{self.identity.daemon_id} keyring') + logger.info( + f'Successfully rotated osd.{self.identity.daemon_id} keyring' + ) break if encrypted: @@ -508,8 +518,11 @@ def rotate_osd_lv_keyring( f'Err:{err}' ) - call(ctx, ['systemctl', 'reset-failed', self.identity.unit_name], - verbosity=CallVerbosity.QUIET_UNLESS_ERROR) + call( + ctx, + ['systemctl', 'reset-failed', self.identity.unit_name], + verbosity=CallVerbosity.QUIET_UNLESS_ERROR, + ) call( ctx, ['systemctl', 'start', self.identity.unit_name], From b57c661c7244e257e60e74f1c998d6634c3a5a1d Mon Sep 17 00:00:00 2001 From: David Galloway Date: Thu, 20 Aug 2026 13:53:46 -0400 Subject: [PATCH 13/20] tentacle: test/osd: add missing stringify.h include for TestOSDMap.cc TestOSDMap.cc uses stringify() but never includes include/stringify.h; until now it compiled via a transitive include that the CVE-2025-30156 cephx merge removed, so once the MonClient.h build break earlier in the compile is fixed, make check dies here instead: src/test/osd/TestOSDMap.cc: error: use of undeclared identifier 'stringify' main got the include from 073584877f5fee617645ec9845de90a38777161d ("test: add missing includes", part of a 22-commit include-hygiene PR that is not worth backporting wholesale), hence this branch-local commit rather than a cherry-pick. Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit 01a873fc3970c6c18821df7d4b0bdf2708357731) --- src/test/osd/TestOSDMap.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/osd/TestOSDMap.cc b/src/test/osd/TestOSDMap.cc index 6d480c9c9a77..878c1e945d9f 100644 --- a/src/test/osd/TestOSDMap.cc +++ b/src/test/osd/TestOSDMap.cc @@ -10,6 +10,7 @@ #include "common/common_init.h" #include "common/ceph_argparse.h" #include "common/ceph_json.h" +#include "include/stringify.h" #include #include From 78090b2397f83f90722a6fffc37fb4282fbd65db Mon Sep 17 00:00:00 2001 From: Uwe Schwaeke Date: Mon, 12 Oct 2026 09:53:03 +0000 Subject: [PATCH 14/20] github: run custom test workflow on tentacle and clear stale container Retarget test-ceph-custom.yml from the squid branches to the tentacle ones: the pull_request trigger now matches 'tentacle-*-cobaltcore-storage', and the workflow_dispatch example points at a tentacle branch. Also remove any leftover ceph_build container before building. A job that dies mid-build, for example when the runner is lost, leaves its container behind. build-with-container.py always names the container ceph_build, so every later run fails right away with "container name ceph_build already in use". Running podman rm -f ceph_build first clears it, and the podman ps line logs what was found. Signed-off-by: Uwe Schwaeke --- .github/workflows/test-ceph-custom.yml | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-ceph-custom.yml b/.github/workflows/test-ceph-custom.yml index b9aa65f6c971..7e465f8c3ed6 100644 --- a/.github/workflows/test-ceph-custom.yml +++ b/.github/workflows/test-ceph-custom.yml @@ -4,12 +4,12 @@ on: workflow_dispatch: inputs: base_branch: - description: 'Base branch to test (e.g. squid-19.2.4-cobaltcore-storage)' + description: 'Base branch to test (e.g. tentacle-20.2.4-cobaltcore-storage)' required: true type: string pull_request: branches: - - 'squid-*-cobaltcore-storage' + - 'tentacle-*-cobaltcore-storage' jobs: test-ceph: @@ -49,6 +49,13 @@ jobs: git submodule foreach --recursive git clean -fdx git submodule update --init --recursive + # A job that dies mid-build (e.g. lost runner) leaves its container + # behind, and build-with-container.py always names it ceph_build. + - name: Remove leftover build container + run: | + podman ps -a --filter name=ceph_build + podman rm -f ceph_build + # --- 2. COMPUTE PARALLELISM --- - name: Compute build + test parallelism run: | From c150a8128c2facc245812e0f6d1a5c65476bf6d1 Mon Sep 17 00:00:00 2001 From: Uwe Schwaeke Date: Mon, 12 Oct 2026 10:11:48 +0000 Subject: [PATCH 15/20] release: record backport metadata Signed-off-by: Uwe Schwaeke --- .../01a12f91-ee54-7edc-80b3-610fd82571ba.md | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) create mode 100644 release-management/backports/01a12f91-ee54-7edc-80b3-610fd82571ba.md diff --git a/release-management/backports/01a12f91-ee54-7edc-80b3-610fd82571ba.md b/release-management/backports/01a12f91-ee54-7edc-80b3-610fd82571ba.md new file mode 100644 index 000000000000..62d24b6659be --- /dev/null +++ b/release-management/backports/01a12f91-ee54-7edc-80b3-610fd82571ba.md @@ -0,0 +1,46 @@ +--- +id: 01a12f91-ee54-7edc-80b3-610fd82571ba +title: Fix Existing Build Errors +component: build +category: fix +provenance: + type: upstream_pr + upstream_prs: + - https://github.com/ceph/ceph/pull/71165 + - https://github.com/ceph/ceph/pull/71190 + upstream_commits: + - 6b206bc281c05b773b3e24c11b248a45c86ffed4 + - a8eddc15b6a7a6ddad7bc4cac27af2022cb3999b + upstream_pr_state: merged-stable + previous_backport: 01a0c326-e041-718b-bf29-f10dc5260620 +justification: + type: engineering + refs: + - cobaltcore-dev/cloud-storage#601 +risk: + # blast + coverage inherited from 01a0c326-e041-718b-bf29-f10dc5260620 (prior effort). + blast: availability + # conflict is a property of (change x base) — re-score against + # the new base; the schema rejects REPLACE_ME until you do. + conflict: clean + coverage: strong +--- + +## Justification (public) + +custom github build process uses -Werror and is failing. This errors are fixed in upstream after release 20.2.4 + +## Justification (internal) + +custom github build process uses -Werror and is failing. This errors are fixed in upstream after release 20.2.4 + +## Risk notes + +fixes warnings that will be treaten as errors during build for unit tests + +## Behavior change + + +## Upgrade notes + +No configuration, data migration, restart sequencing, or ACL rewrite is needed. From fea0f401221ae2403c45755ea76015a332b7bff9 Mon Sep 17 00:00:00 2001 From: David Galloway Date: Thu, 20 Aug 2026 16:43:00 -0400 Subject: [PATCH 16/20] tentacle: mgr/cephadm: drop orphaned noautoscale cleanup in _mark_upgrade_complete 5eaf4db5a88 ("mgr/cephadm: mute key rotation related health warnings during upgrade") brought a hunk of main's _mark_upgrade_complete() to tentacle that belongs to the PG autoscaler feature added on main by da4b361e63c ("mgr: Control PG autoscaler during upgrades with pg_autoscale_during_upgrade"), which was never backported. So tentacle calls self._unset_noautoscale() and assigns self.upgrade_state.noautoscale_set, neither of which exists here, and run-tox-mgr fails for every tentacle PR: cephadm/upgrade.py: error: "CephadmUpgrade" has no attribute "_unset_noautoscale" [attr-defined] cephadm/upgrade.py: error: "UpgradeState" has no attribute "noautoscale_set" [attr-defined] Nothing on tentacle ever sets noautoscale_set, so the getattr() guard is always False and the block is dead code; drop it rather than backporting the feature to a stable branch. Signed-off-by: David Galloway Assisted-by: Claude Fable 5 (claude-fable-5) (cherry picked from commit a3f44c1beb1befa86099d1a48ae9efda4d2a63f0) --- src/pybind/mgr/cephadm/upgrade.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/pybind/mgr/cephadm/upgrade.py b/src/pybind/mgr/cephadm/upgrade.py index 6beb0f5ea99f..7454242e3eab 100644 --- a/src/pybind/mgr/cephadm/upgrade.py +++ b/src/pybind/mgr/cephadm/upgrade.py @@ -1414,10 +1414,6 @@ def _mark_upgrade_complete(self, target_digests: Optional[List[str]], target_ima if self.upgrade_state.health_warnings_muted: self._unmute_upgrade_related_health_warnings() - if getattr(self.upgrade_state, 'noautoscale_set', False): - self._unset_noautoscale() - self.upgrade_state.noautoscale_set = False - logger.info('Upgrade: Complete!') if self.upgrade_state.progress_id: self.mgr.remote('progress', 'complete', From a9c30276568a301539cb604f126856eefa8760ab Mon Sep 17 00:00:00 2001 From: Uwe Schwaeke Date: Tue, 13 Oct 2026 06:42:57 +0000 Subject: [PATCH 17/20] tentacle: qa/tasks/radosgw_admin_rest: fix undefined admin_conn in ratelimit tests The backport of "rgw: add rate limit for LIST & DELETE ops" (512e9304a3c, cherry picked from dae572d5008) carried the qa test hunk unchanged from main, where it still used the old rgwadmin_rest(admin_conn, ...) call form. Tentacle's rgwadmin_rest() takes (endpoint, creds, cmd, ...) and no admin_conn exists in task(), so flake8 reports F821 undefined name 'admin_conn' and run-tox-qa fails. The test cases would also raise NameError under teuthology. Adapt the calls to rgwadmin_rest(endpoint, admin_creds, ...), as main did in a7f557c3fb3 ("qa/radosgw_admin_rest: replace boto2 with boto3"). The resulting ratelimit test block is identical to main's. Signed-off-by: Uwe Schwaeke Co-Authored-By: Claude Opus 5.5 (1M context) --- qa/tasks/radosgw_admin_rest.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/qa/tasks/radosgw_admin_rest.py b/qa/tasks/radosgw_admin_rest.py index d5447289127c..3cc12fd568e7 100644 --- a/qa/tasks/radosgw_admin_rest.py +++ b/qa/tasks/radosgw_admin_rest.py @@ -1074,57 +1074,57 @@ def task(ctx, config): assert ret == 200 # TESTCASE 'ratelimit' 'user' 'modify' 'max-list-ops = 100' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-list-ops' : '100'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-list-ops' : '100'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) assert ret == 200 user_ratelimit = out['user_ratelimit'] assert user_ratelimit['max_list_ops'] == 100 # TESTCASE 'ratelimit' 'user' 'modify' 'max-delete-ops = 50' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-delete-ops' : '50'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-delete-ops' : '50'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) assert ret == 200 user_ratelimit = out['user_ratelimit'] assert user_ratelimit['max_delete_ops'] == 50 # TESTCASE 'ratelimit' 'bucket' 'modify' 'max-list-ops = 200' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-list-ops' : '200'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-list-ops' : '200'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) assert ret == 200 bucket_ratelimit = out['bucket_ratelimit'] assert bucket_ratelimit['max_list_ops'] == 200 # TESTCASE 'ratelimit' 'bucket' 'modify' 'max-delete-ops = 75' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-delete-ops' : '75'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket, 'max-delete-ops' : '75'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'ratelimit-scope' : 'bucket', 'bucket' : ratelimit_bucket}) assert ret == 200 bucket_ratelimit = out['bucket_ratelimit'] assert bucket_ratelimit['max_delete_ops'] == 75 # TESTCASE 'ratelimit' 'global' 'modify' 'bucket' 'max-list-ops = 500' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-list-ops' : '500'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-list-ops' : '500'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'global' : 'true'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'global' : 'true'}) assert ret == 200 # Check that global bucket ratelimit has the list ops set assert 'bucket_ratelimit' in out assert out['bucket_ratelimit']['max_list_ops'] == 500 # TESTCASE 'ratelimit' 'global' 'modify' 'bucket' 'max-delete-ops = 300' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-delete-ops' : '300'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], {'ratelimit-scope' : 'bucket', 'global': 'true', 'max-delete-ops' : '300'}) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'global' : 'true'}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'global' : 'true'}) assert ret == 200 # Check that global bucket ratelimit has the delete ops set assert 'bucket_ratelimit' in out assert out['bucket_ratelimit']['max_delete_ops'] == 300 # TESTCASE 'ratelimit' 'user' 'modify' 'multiple ops at once' 'succeeds' - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'modify'], { + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'modify'], { 'ratelimit-scope' : 'user', 'uid' : ratelimit_user, 'max-read-ops' : '1000', @@ -1134,7 +1134,7 @@ def task(ctx, config): 'enabled' : 'true' }) assert ret == 200 - (ret, out) = rgwadmin_rest(admin_conn, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) + (ret, out) = rgwadmin_rest(endpoint, admin_creds, ['ratelimit', 'info'], {'ratelimit-scope' : 'user', 'uid' : ratelimit_user}) assert ret == 200 user_ratelimit = out['user_ratelimit'] assert user_ratelimit['enabled'] == True From a739f6604531d42895f40dc7b046b956e294593c Mon Sep 17 00:00:00 2001 From: Supriti Singh Date: Thu, 26 Feb 2026 09:59:52 +0100 Subject: [PATCH 18/20] rgw: Inject keystone roles into IAM policy Keystone roles are injected into the IAM policy evaluation environment as "keystone:role" condition keys. This enables bucket policies to grant or deny access based on Keystone roles. It bridges the gap between Openstack RBAC and S3 IAM policy. Document keystone:role in bucket policy and Keystone S3 integration Signed-off-by: Supriti Singh Signed-off-by: Marcel Lauhoff On-behalf-of: SAP On-behalf-of: SAP (cherry picked from commit 1e6e39346667f82591644760311093729db8a915) --- doc/radosgw/bucketpolicy.rst | 4 + doc/radosgw/keystone.rst | 6 ++ src/rgw/rgw_auth.cc | 9 +++ src/rgw/rgw_auth.h | 7 +- src/rgw/rgw_auth_keystone.cc | 16 +++- src/test/rgw/test_rgw_iam_policy.cc | 116 ++++++++++++++++++++++++++++ 6 files changed, 152 insertions(+), 6 deletions(-) diff --git a/doc/radosgw/bucketpolicy.rst b/doc/radosgw/bucketpolicy.rst index 23356624a46a..6c27f1828e08 100644 --- a/doc/radosgw/bucketpolicy.rst +++ b/doc/radosgw/bucketpolicy.rst @@ -122,6 +122,10 @@ For all requests, condition keys we support are: - aws:UserAgent - aws:username +Request that authenticate with Keystone also include: + +- keystone:role + We support certain s3 condition keys for bucket and object requests. *Support for the following bucket-related operations was added in the Mimic diff --git a/doc/radosgw/keystone.rst b/doc/radosgw/keystone.rst index 068eac24ff2c..c817cfca4f3c 100644 --- a/doc/radosgw/keystone.rst +++ b/doc/radosgw/keystone.rst @@ -159,6 +159,12 @@ S3 API (with AWS-like access and secret keys), if the ``rgw s3 auth use keystone`` option is set. For details, see :doc:`s3/authentication`. +Requests authenticated via Keystone expose the Keystone role names in the +IAM policy environment as the condition key ``keystone:role``. It can be +used in bucket policies and idenitity policies to allow or deny access by +role (e.g. ``StringEquals`` on ``keystone:role``). +See :doc:`bucketpolicy` for list of supported condition keys. + Service token support --------------------- diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index 74805fbb186b..a40f94cd796b 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -1033,6 +1033,15 @@ void rgw::auth::RemoteApplier::modify_request_state(const DoutPrefixProvider* dp // copy our identity policies into req_state s->iam_identity_policies.insert(s->iam_identity_policies.end(), policies.begin(), policies.end()); + + for (auto role : this->info.keystone_roles) { + // Keystone roles are case-insensitive. Normalize the roles to + // lowercase before placing them into the environment. + std::transform(role.begin(), role.end(), role.begin(), + [](unsigned char c){ return std::tolower(c); }); + s->env.emplace("keystone:role", std::move(role)); + } + } /* rgw::auth::LocalApplier */ diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index b29f863d030f..ef2def743754 100644 --- a/src/rgw/rgw_auth.h +++ b/src/rgw/rgw_auth.h @@ -592,6 +592,7 @@ class RemoteApplier : public IdentityApplier { const std::string access_key_id; const std::string subuser; const std::optional keystone_scope; + const std::vector keystone_roles; public: enum class acct_privilege_t { @@ -609,7 +610,8 @@ class RemoteApplier : public IdentityApplier { const std::string access_key_id, const std::string subuser, const uint32_t acct_type=TYPE_NONE, - std::optional keystone_scope=std::nullopt) + std::optional keystone_scope=std::nullopt, + std::vector keystone_roles = {}) : acct_user(acct_user), acct_name(acct_name), perm_mask(perm_mask), @@ -617,7 +619,8 @@ class RemoteApplier : public IdentityApplier { acct_type(acct_type), access_key_id(access_key_id), subuser(subuser), - keystone_scope(std::move(keystone_scope)) { + keystone_scope(std::move(keystone_scope)), + keystone_roles(std::move(keystone_roles)) { } }; diff --git a/src/rgw/rgw_auth_keystone.cc b/src/rgw/rgw_auth_keystone.cc index e0678e6fb9f5..1717b957aa2e 100644 --- a/src/rgw/rgw_auth_keystone.cc +++ b/src/rgw/rgw_auth_keystone.cc @@ -144,13 +144,14 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token ) const noexcept { using acct_privilege_t = rgw::auth::RemoteApplier::AuthInfo::acct_privilege_t; + std::vector role_names; /* Check whether the user has an admin status. */ acct_privilege_t level = acct_privilege_t::IS_PLAIN_ACCT; for (const auto& role : token.roles) { + role_names.push_back(role.name); if (role.is_admin && !role.is_reader) { level = acct_privilege_t::IS_ADMIN_ACCT; - break; } } @@ -169,8 +170,9 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token rgw::auth::RemoteApplier::AuthInfo::NO_ACCESS_KEY, rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER, TYPE_KEYSTONE, - std::move(keystone_scope) -}; + std::move(keystone_scope), + std::move(role_names) + }; } static inline const std::string @@ -673,6 +675,11 @@ EC2Engine::get_creds_info(const EC2Engine::token_envelope_t& token, /* Build keystone scope info if ops logging is enabled */ auto keystone_scope = rgw::keystone::build_scope_info(cct, token); + std::vector role_names; + for (const auto& role : token.roles) { + role_names.push_back(role.name); + } + return auth_info_t { /* Suggested account name for the authenticated user. */ rgw_user(token.get_project_id()), @@ -685,7 +692,8 @@ EC2Engine::get_creds_info(const EC2Engine::token_envelope_t& token, access_key_id, rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER, TYPE_KEYSTONE, - std::move(keystone_scope) + std::move(keystone_scope), + std::move(role_names) }; } diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index b7c9eb229d4c..be794cae8a24 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/src/test/rgw/test_rgw_iam_policy.cc @@ -1779,3 +1779,119 @@ TEST_F(ConditionTest, Null) EXPECT_TRUE(notNull.eval({{key, "admin/config.txt"}})); } } + +TEST_F(ConditionTest, KeyStoneRoleStringEquals) +{ + std::string key = "keystone:role"; + + Condition cond{TokenID::StringEquals, key.data(), key.size(), false}; + cond.vals.push_back("testrole"); + + // No roles + EXPECT_FALSE(cond.eval({})); + + // Single matching role + EXPECT_TRUE(cond.eval({{key, "testrole"}})); + + // Single non-matching role + EXPECT_FALSE(cond.eval({{key, "member"}})); + + //Multiple roles in env,one matches + Environment multi_env; + multi_env.emplace(key, "member"); + multi_env.emplace(key, "testrole"); + multi_env.emplace(key, "reader"); + EXPECT_TRUE(cond.eval(multi_env)); + + //Multiple roles in env, no match + Environment no_match_env; + no_match_env.emplace(key, "member"); + no_match_env.emplace(key, "reader"); + EXPECT_FALSE(cond.eval(no_match_env)); + + // Multiple identical roles (redundancy check) + Environment duplicate_env; + duplicate_env.emplace(key, "testrole"); + duplicate_env.emplace(key, "testrole"); + EXPECT_TRUE(cond.eval(duplicate_env)); +} + +TEST_F(ConditionTest, KeyStoneRoleNotStringEquals) +{ + std::string key = "keystone:role"; + + Condition cond{TokenID::StringNotEquals, key.data(), key.size(), false}; + cond.vals.push_back("admin"); + + // No roles + EXPECT_FALSE(cond.eval({})); + + // Role matches + EXPECT_FALSE(cond.eval({{key, "admin"}})); + + // Role doesn't match + EXPECT_TRUE(cond.eval({{key, "member"}})); + + // Multiple roles in env, one matches -> false + Environment multi_env; + multi_env.emplace(key, "member"); + multi_env.emplace(key, "admin"); + EXPECT_FALSE(multi_env.count(key) == 0); + EXPECT_FALSE(cond.eval(multi_env)); + + // Multiple roles, none match -> true + Environment no_match_env; + no_match_env.emplace(key, "member"); + no_match_env.emplace(key, "reader"); + EXPECT_TRUE(cond.eval(no_match_env)); +} + +TEST_F(ConditionTest, KeyStoneRolePolicyParsing) +{ + string keystone_role_policy = R"( + { + "Version": "2012-10-17", + "Statement": [{ + "Effect" : "Allow", + "Principal": "*", + "Action": "s3:GetObject", + "Resource": [ + "arn:aws:s3:::example_bucket/*" + ], + "Condition": { + "StringEquals": { + "keystone:role": "testrole" + } + } + }] + } + )"; + + string tenant = "arbitrary_tenant"; + boost::optional p; + + ASSERT_NO_THROW( + p = Policy(cct.get(), &tenant, keystone_role_policy, true)); + ASSERT_TRUE(p); + EXPECT_EQ(p->statements.size(), 1U); + EXPECT_EQ(p->statements[0].conditions.size(), 1U); + EXPECT_EQ(p->statements[0].conditions[0].key, "keystone:role"); + EXPECT_EQ(p->statements[0].conditions[0].vals.size(), 1U); + EXPECT_EQ(p->statements[0].conditions[0].vals[0], "testrole"); + + // Eval with matching role in environment + Environment match_env; + match_env.emplace("keystone:role", "testrole"); + EXPECT_TRUE(p->statements[0].conditions[0].eval(match_env)); + + // Eval with non-matching role + Environment nomatch_env; + nomatch_env.emplace("keystone:role", "member"); + EXPECT_FALSE(p->statements[0].conditions[0].eval(nomatch_env)); + + // Eval with multiple roles, one matching + Environment multi_env; + multi_env.emplace("keystone:role", "member"); + multi_env.emplace("keystone:role", "testrole"); + EXPECT_TRUE(p->statements[0].conditions[0].eval(multi_env)); +} \ No newline at end of file From 626b232ed2ffceb96babceaa180df9144b48fbfe Mon Sep 17 00:00:00 2001 From: Supriti Singh Date: Thu, 19 Mar 2026 10:26:32 +0100 Subject: [PATCH 19/20] rgw: Inject keystone userid into IAM policy Expose the Keystone token user UUID as condition key "keystone:userid" so IAM and bucket policies can restrict access by user (e.g. per-user buckets or user-specific allow/deny). On-behalf-of: SAP Signed-off-by: Supriti Singh (cherry picked from commit b06b7a82ed1a5c57dce8b9ce2fa0931fe0f55ff7) --- doc/radosgw/bucketpolicy.rst | 1 + doc/radosgw/keystone.rst | 15 +++++++++++---- src/rgw/rgw_auth.cc | 3 +++ src/rgw/rgw_auth.h | 8 ++++++-- src/rgw/rgw_auth_keystone.cc | 6 ++++-- src/test/rgw/test_rgw_iam_policy.cc | 11 +++++++++++ 6 files changed, 36 insertions(+), 8 deletions(-) diff --git a/doc/radosgw/bucketpolicy.rst b/doc/radosgw/bucketpolicy.rst index 6c27f1828e08..0ade1b080f1c 100644 --- a/doc/radosgw/bucketpolicy.rst +++ b/doc/radosgw/bucketpolicy.rst @@ -125,6 +125,7 @@ For all requests, condition keys we support are: Request that authenticate with Keystone also include: - keystone:role +- keystone:userid We support certain s3 condition keys for bucket and object requests. diff --git a/doc/radosgw/keystone.rst b/doc/radosgw/keystone.rst index c817cfca4f3c..21f79e490599 100644 --- a/doc/radosgw/keystone.rst +++ b/doc/radosgw/keystone.rst @@ -159,10 +159,17 @@ S3 API (with AWS-like access and secret keys), if the ``rgw s3 auth use keystone`` option is set. For details, see :doc:`s3/authentication`. -Requests authenticated via Keystone expose the Keystone role names in the -IAM policy environment as the condition key ``keystone:role``. It can be -used in bucket policies and idenitity policies to allow or deny access by -role (e.g. ``StringEquals`` on ``keystone:role``). +Requests authenticated via Keystone (Swift tokens or S3 with Keystone-managed +credentials) expose the Keystone idenitity in the IAM policy environment +as the condition keys: ``keystone:role`` (role names) and +``keystone:userid`` (user UUID). These conditions can be used in bucket +policies and idenitity policies with ``StringEquals``, ``StringNotEquals`` etc. + +- **keystone:role** - Allow or deny by *role* (e.g only users with role + ``reader`` get read access). Use for RBAC. +- **keystone:userid** - Restrict to specific *user*. Use when policy + depends on a specific user, not just their role. + See :doc:`bucketpolicy` for list of supported condition keys. Service token support diff --git a/src/rgw/rgw_auth.cc b/src/rgw/rgw_auth.cc index a40f94cd796b..ab7cc143a030 100644 --- a/src/rgw/rgw_auth.cc +++ b/src/rgw/rgw_auth.cc @@ -1042,6 +1042,9 @@ void rgw::auth::RemoteApplier::modify_request_state(const DoutPrefixProvider* dp s->env.emplace("keystone:role", std::move(role)); } + if (!info.keystone_user_id.empty()) { + s->env.emplace("keystone:userid", info.keystone_user_id); + } } /* rgw::auth::LocalApplier */ diff --git a/src/rgw/rgw_auth.h b/src/rgw/rgw_auth.h index ef2def743754..10009c5d8f6e 100644 --- a/src/rgw/rgw_auth.h +++ b/src/rgw/rgw_auth.h @@ -593,6 +593,7 @@ class RemoteApplier : public IdentityApplier { const std::string subuser; const std::optional keystone_scope; const std::vector keystone_roles; + const std::string keystone_user_id; public: enum class acct_privilege_t { @@ -611,7 +612,9 @@ class RemoteApplier : public IdentityApplier { const std::string subuser, const uint32_t acct_type=TYPE_NONE, std::optional keystone_scope=std::nullopt, - std::vector keystone_roles = {}) + std::vector keystone_roles = {}, + const std::string keystone_user_id = {} + ) : acct_user(acct_user), acct_name(acct_name), perm_mask(perm_mask), @@ -620,7 +623,8 @@ class RemoteApplier : public IdentityApplier { access_key_id(access_key_id), subuser(subuser), keystone_scope(std::move(keystone_scope)), - keystone_roles(std::move(keystone_roles)) { + keystone_roles(std::move(keystone_roles)), + keystone_user_id(keystone_user_id) { } }; diff --git a/src/rgw/rgw_auth_keystone.cc b/src/rgw/rgw_auth_keystone.cc index 1717b957aa2e..2185efd81082 100644 --- a/src/rgw/rgw_auth_keystone.cc +++ b/src/rgw/rgw_auth_keystone.cc @@ -171,7 +171,8 @@ TokenEngine::get_creds_info(const TokenEngine::token_envelope_t& token rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER, TYPE_KEYSTONE, std::move(keystone_scope), - std::move(role_names) + std::move(role_names), + token.get_user_id() }; } @@ -693,7 +694,8 @@ EC2Engine::get_creds_info(const EC2Engine::token_envelope_t& token, rgw::auth::RemoteApplier::AuthInfo::NO_SUBUSER, TYPE_KEYSTONE, std::move(keystone_scope), - std::move(role_names) + std::move(role_names), + token.get_user_id() }; } diff --git a/src/test/rgw/test_rgw_iam_policy.cc b/src/test/rgw/test_rgw_iam_policy.cc index be794cae8a24..ec6228df7c47 100644 --- a/src/test/rgw/test_rgw_iam_policy.cc +++ b/src/test/rgw/test_rgw_iam_policy.cc @@ -1894,4 +1894,15 @@ TEST_F(ConditionTest, KeyStoneRolePolicyParsing) multi_env.emplace("keystone:role", "member"); multi_env.emplace("keystone:role", "testrole"); EXPECT_TRUE(p->statements[0].conditions[0].eval(multi_env)); +} + +TEST_F(ConditionTest, KeystoneUserIdStringEquals) +{ + const std::string key = "keystone:userid"; + Condition cond{TokenID::StringEquals, key.data(), key.size(), false}; + cond.vals.push_back("user-123"); + + EXPECT_FALSE(cond.eval({})); + EXPECT_TRUE(cond.eval({{key, "user-123"}})); + EXPECT_FALSE(cond.eval({{key, "user-456"}})); } \ No newline at end of file From 467672104f18ea7f5beb611d06846456e9caf9cb Mon Sep 17 00:00:00 2001 From: Supriti Singh Date: Fri, 23 Oct 2026 10:23:05 +0200 Subject: [PATCH 20/20] backport: rgw keystone roles and userid in IAM policy Signed-off-by: Supriti Singh --- .../01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md diff --git a/release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md b/release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md new file mode 100644 index 000000000000..2b36cd87152c --- /dev/null +++ b/release-management/backports/01a16411-8df8-7e1f-baaf-8aa40a09ecdd.md @@ -0,0 +1,80 @@ +--- +id: 01a16411-8df8-7e1f-baaf-8aa40a09ecdd +title: 'IAM / Bucket Policy: Keystone role and user ID support' +component: rgw +category: feature +provenance: + type: upstream_pr + upstream_prs: + - https://github.com/ceph/ceph/pull/67752 + upstream_pr_state: merged-main + upstream_commits: + - b06b7a82ed1a5c57dce8b9ce2fa0931fe0f55ff7 + previous_backport: 01a0cdb3-3f84-74ab-9403-c7638a039697 +justification: + type: customer + refs: + - cobaltcore-dev/cloud-storage#566 + - cobaltcore-dev/cloud-storage#565 + - https://github.com/cobaltcore-dev/cloud-storage/issues/380 + - https://github.com/cobaltcore-dev/cloud-storage/issues/372 +risk: + blast: cosmetic + conflict: trivial + coverage: strong +--- + +## Justification (public) + +Allows Keystone role names and user IDs to be injected as `keystone:role` and +`keystone:userid` condition keys in S3 bucket policies, enabling role-based and +per-user access control via standard IAM policy conditions. + +## Justification (internal) + +Improves keystone authorization features. + +## Risk notes + +**blast: cosmetic** — purely additive. Two new condition keys are populated for +Keystone-authenticated requests. No existing condition key, policy or auth path +changes meaning, and non-Keystone requests are untouched. + +**conflict: trivial** + +**coverage: strong** — +47/47 unit tests pass in `unittest_rgw_iam_policy`. +14/14 live checks pass against vstart RGW and a real Keystone, with two +multi-role users in one project: `keystone:role` matching (single value, +multi-value, shared role, absent role), case handling (`StringEquals` is +case-sensitive, `StringEqualsIgnoreCase` is not), `keystone:userid` matching per +user, `StringLike` wildcards, and three `StringNotEquals` cases that confirm the +AND semantics with multiple roles. +7/7 live grant checks pass with non-owner users from a second project: `Allow` +on `keystone:role` or `keystone:userid` grants exactly the listed read, list, +write and delete actions, users without the role or ID get nothing, and `Deny` +overrides `Allow`. + +## Behavior change + +Requests authenticated via Keystone now expose two additional IAM policy +condition keys: `keystone:role` (one entry per role, lowercased) and +`keystone:userid` (the Keystone user UUID). Requests not authenticated via +Keystone are unchanged, and no existing policy changes behaviour. + +### Two points for policy authors + +Role names in policies are case-sensitive. Roles are lowercased on the way into +the policy environment, so how the role is spelled in Keystone does not matter, +but the value typed in the policy is compared exactly. A policy containing +`"TestRole"` never matches and gives no warning. Write role names in lower case, +or use `StringEqualsIgnoreCase`. + +Rules that distinguish between users of the same project must be written as +`Deny`. All users in an OpenStack project share a single RGW account, so each is +an owner of buckets created there, and owners are not restricted by `Allow` +conditions. `Deny` statements do apply to owners and discriminate correctly. + +## Upgrade notes + +None.