From 70d63dac461ee3d31d8420ed0628b8a94851f85f Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Mon, 15 Jan 2024 12:35:58 -0500 Subject: [PATCH] rgw/iam: add pagination to ListRoles rename sal::Driver::get_roles() to list_roles() and add pagination support for the RGWListRoles op and 'radosgw-admin role list' Signed-off-by: Casey Bodley --- src/rgw/driver/daos/rgw_sal_daos.cc | 11 +++-- src/rgw/driver/daos/rgw_sal_daos.h | 11 +++-- src/rgw/driver/motr/rgw_sal_motr.cc | 12 +++-- src/rgw/driver/motr/rgw_sal_motr.h | 12 +++-- src/rgw/driver/rados/rgw_sal_rados.cc | 69 ++++++++++++++++----------- src/rgw/driver/rados/rgw_sal_rados.h | 12 +++-- src/rgw/rgw_admin.cc | 65 ++++++++++++++----------- src/rgw/rgw_rest_role.cc | 39 ++++++++++----- src/rgw/rgw_rest_role.h | 3 ++ src/rgw/rgw_sal.h | 12 +++-- src/rgw/rgw_sal_dbstore.cc | 12 +++-- src/rgw/rgw_sal_dbstore.h | 12 +++-- src/rgw/rgw_sal_filter.cc | 17 ++++--- src/rgw/rgw_sal_filter.h | 12 +++-- 14 files changed, 180 insertions(+), 119 deletions(-) diff --git a/src/rgw/driver/daos/rgw_sal_daos.cc b/src/rgw/driver/daos/rgw_sal_daos.cc index e62495c3ac94c..75e85f2074976 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.cc +++ b/src/rgw/driver/daos/rgw_sal_daos.cc @@ -2095,10 +2095,13 @@ std::unique_ptr DaosStore::get_role(std::string id) { return std::unique_ptr(p); } -int DaosStore::get_roles(const DoutPrefixProvider* dpp, optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - vector>& roles) { +int DaosStore::list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) { return DAOS_NOT_IMPLEMENTED_LOG(dpp); } diff --git a/src/rgw/driver/daos/rgw_sal_daos.h b/src/rgw/driver/daos/rgw_sal_daos.h index 04263e50dd805..3d5446734477b 100644 --- a/src/rgw/driver/daos/rgw_sal_daos.h +++ b/src/rgw/driver/daos/rgw_sal_daos.h @@ -999,10 +999,13 @@ class DaosStore : public StoreDriver { std::multimap tags = {}) override; virtual std::unique_ptr get_role(const RGWRoleInfo& info) override; virtual std::unique_ptr get_role(std::string id) override; - virtual int get_roles(const DoutPrefixProvider* dpp, optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) override; + int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) override; virtual std::unique_ptr get_oidc_provider() override; virtual int get_oidc_providers( const DoutPrefixProvider* dpp, const std::string& tenant, diff --git a/src/rgw/driver/motr/rgw_sal_motr.cc b/src/rgw/driver/motr/rgw_sal_motr.cc index ee90ba42a38a6..07ccab041e8f7 100644 --- a/src/rgw/driver/motr/rgw_sal_motr.cc +++ b/src/rgw/driver/motr/rgw_sal_motr.cc @@ -3050,11 +3050,13 @@ std::unique_ptr MotrStore::get_role(std::string id) return std::unique_ptr(p); } -int MotrStore::get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - vector>& roles) +int MotrStore::list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) { return 0; } diff --git a/src/rgw/driver/motr/rgw_sal_motr.h b/src/rgw/driver/motr/rgw_sal_motr.h index c9ffcffc2d40c..e856c34e0b2e9 100644 --- a/src/rgw/driver/motr/rgw_sal_motr.h +++ b/src/rgw/driver/motr/rgw_sal_motr.h @@ -1064,11 +1064,13 @@ class MotrStore : public StoreDriver { std::multimap tags={}) override; virtual std::unique_ptr get_role(const RGWRoleInfo& info) override; virtual std::unique_ptr get_role(std::string id) override; - virtual int get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) override; + int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) override; virtual std::unique_ptr get_oidc_provider() override; virtual int get_oidc_providers(const DoutPrefixProvider *dpp, const std::string& tenant, diff --git a/src/rgw/driver/rados/rgw_sal_rados.cc b/src/rgw/driver/rados/rgw_sal_rados.cc index 79431b3521e0d..61b1159104ae3 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.cc +++ b/src/rgw/driver/rados/rgw_sal_rados.cc @@ -1804,13 +1804,17 @@ std::unique_ptr RadosStore::get_role(const RGWRoleInfo& info) return std::make_unique(this, info); } -int RadosStore::get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - vector>& roles) +int RadosStore::list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) { - auto pool = svc()->zone->get_zone_params().roles_pool; + listing.roles.clear(); + + const auto& pool = svc()->zone->get_zone_params().roles_pool; std::string prefix; // List all roles if path prefix is empty @@ -1821,46 +1825,53 @@ int RadosStore::get_roles(const DoutPrefixProvider *dpp, } //Get the filtered objects - list result; - bool is_truncated; RGWListRawObjsCtx ctx; - do { - list oids; - int r = rados->list_raw_objects(dpp, pool, prefix, 1000, ctx, oids, &is_truncated); - if (r < 0) { - ldpp_dout(dpp, 0) << "ERROR: listing filtered objects failed: " - << prefix << ": " << cpp_strerror(-r) << dendl; - return r; - } - for (const auto& iter : oids) { - result.push_back(iter.substr(RGWRole::role_path_oid_prefix.size())); - } - } while (is_truncated); + int r = rados->list_raw_objects_init(dpp, pool, marker, &ctx); + if (r < 0) { + return r; + } + + bool is_truncated = false; + list oids; + r = rados->list_raw_objects(dpp, pool, prefix, max_items, + ctx, oids, &is_truncated); + if (r == -ENOENT) { + r = 0; + } else if (r < 0) { + return r; + } + + for (const auto& oid : oids) { + const std::string key = oid.substr(RGWRole::role_path_oid_prefix.size()); - for (const auto& it : result) { //Find the role oid prefix from the end - size_t pos = it.rfind(RGWRole::role_oid_prefix); + size_t pos = key.rfind(RGWRole::role_oid_prefix); if (pos == std::string::npos) { - continue; + continue; } // Split the result into path and info_oid + id - std::string path = it.substr(0, pos); + std::string path = key.substr(0, pos); /*Make sure that prefix is part of path (False results could've been returned) because of the role info oid + id appended to the path)*/ if(path_prefix.empty() || path.find(path_prefix) != std::string::npos) { //Get id from info oid prefix + id - std::string id = it.substr(pos + RGWRole::role_oid_prefix.length()); + std::string id = key.substr(pos + RGWRole::role_oid_prefix.length()); std::unique_ptr role = get_role(id); - int ret = role->read_info(dpp, y); - if (ret < 0) { - return ret; + r = role->read_info(dpp, y); + if (r < 0) { + return r; } - roles.push_back(std::move(role)); + listing.roles.push_back(std::move(role->get_info())); } } + if (is_truncated) { + listing.next_marker = rados->list_raw_objs_get_cursor(ctx); + } else { + listing.next_marker.clear(); + } return 0; } diff --git a/src/rgw/driver/rados/rgw_sal_rados.h b/src/rgw/driver/rados/rgw_sal_rados.h index 7702fd3a8f2ec..cfee6b872662c 100644 --- a/src/rgw/driver/rados/rgw_sal_rados.h +++ b/src/rgw/driver/rados/rgw_sal_rados.h @@ -337,11 +337,13 @@ class RadosStore : public StoreDriver { std::multimap tags={}) override; virtual std::unique_ptr get_role(std::string id) override; virtual std::unique_ptr get_role(const RGWRoleInfo& info) override; - virtual int get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) override; + int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) override; virtual std::unique_ptr get_oidc_provider() override; virtual int get_oidc_providers(const DoutPrefixProvider *dpp, const std::string& tenant, diff --git a/src/rgw/rgw_admin.cc b/src/rgw/rgw_admin.cc index ddedb4f22de15..badb43b3c1904 100644 --- a/src/rgw/rgw_admin.cc +++ b/src/rgw/rgw_admin.cc @@ -1163,26 +1163,6 @@ static void show_policy_names(std::vector policy_names, Formatter* forma formatter->flush(cout); } -static void show_role_info(rgw::sal::RGWRole* role, Formatter* formatter) -{ - formatter->open_object_section("role"); - role->dump(formatter); - formatter->close_section(); - formatter->flush(cout); -} - -static void show_roles_info(vector>& roles, Formatter* formatter) -{ - formatter->open_array_section("Roles"); - for (const auto& it : roles) { - formatter->open_object_section("role"); - it->dump(formatter); - formatter->close_section(); - } - formatter->close_section(); - formatter->flush(cout); -} - static void show_reshard_status( const list& status, Formatter *formatter) { @@ -6812,7 +6792,8 @@ int main(int argc, const char **argv) if (ret < 0) { return -ret; } - show_role_info(role.get(), formatter.get()); + encode_json("role", *role, formatter.get()); + formatter->flush(cout); return 0; } case OPT::ROLE_DELETE: @@ -6840,7 +6821,8 @@ int main(int argc, const char **argv) if (ret < 0) { return -ret; } - show_role_info(role.get(), formatter.get()); + encode_json("role", *role, formatter.get()); + formatter->flush(cout); return 0; } case OPT::ROLE_TRUST_POLICY_MODIFY: @@ -6880,12 +6862,41 @@ int main(int argc, const char **argv) } case OPT::ROLE_LIST: { - vector> result; - ret = driver->get_roles(dpp(), null_yield, path_prefix, tenant, result); - if (ret < 0) { - return -ret; + rgw::sal::RoleList listing; + listing.next_marker = marker; + + int32_t remaining = std::numeric_limits::max(); + if (max_entries_specified) { + remaining = max_entries; + formatter->open_object_section("result"); } - show_roles_info(result, formatter.get()); + formatter->open_array_section("Roles"); + + do { + constexpr int32_t max_chunk = 100; + int32_t count = std::min(max_chunk, remaining); + + ret = driver->list_roles(dpp(), null_yield, tenant, path_prefix, + listing.next_marker, count, listing); + if (ret < 0) { + return -ret; + } + for (const auto& info : listing.roles) { + encode_json("member", info, formatter.get()); + } + formatter->flush(cout); + remaining -= listing.roles.size(); + } while (!listing.next_marker.empty() && remaining > 0); + + formatter->close_section(); // Roles + + if (max_entries_specified) { + if (!listing.next_marker.empty()) { + encode_json("next-marker", listing.next_marker, formatter.get()); + } + formatter->close_section(); // result + } + formatter->flush(cout); return 0; } case OPT::ROLE_POLICY_PUT: diff --git a/src/rgw/rgw_rest_role.cc b/src/rgw/rgw_rest_role.cc index 4ee5707e0b05a..05450491f440e 100644 --- a/src/rgw/rgw_rest_role.cc +++ b/src/rgw/rgw_rest_role.cc @@ -434,6 +434,13 @@ void RGWModifyRoleTrustPolicy::execute(optional_yield y) int RGWListRoles::init_processing(optional_yield y) { path_prefix = s->info.args.get("PathPrefix"); + marker = s->info.args.get("Marker"); + + int r = s->info.args.get_int("MaxItems", &max_items, max_items); + if (r < 0 || max_items > 1000) { + s->err.message = "Invalid value for MaxItems"; + return -EINVAL; + } return 0; } @@ -441,24 +448,30 @@ int RGWListRoles::init_processing(optional_yield y) void RGWListRoles::execute(optional_yield y) { // TODO: list_account_roles() for account owner - vector> result; - op_ret = driver->get_roles(s, y, path_prefix, s->user->get_tenant(), result); + rgw::sal::RoleList listing; + op_ret = driver->list_roles(s, y, s->user->get_tenant(), path_prefix, + marker, max_items, listing); if (op_ret == 0) { - s->formatter->open_array_section("ListRolesResponse"); - s->formatter->open_array_section("ListRolesResult"); - s->formatter->open_object_section("Roles"); - for (const auto& it : result) { - s->formatter->open_object_section("member"); - it->dump(s->formatter); - s->formatter->close_section(); + s->formatter->open_object_section("ListRolesResponse"); + s->formatter->open_object_section("ListRolesResult"); + s->formatter->open_array_section("Roles"); + for (const auto& info : listing.roles) { + encode_json("member", info, s->formatter); } - s->formatter->close_section(); - s->formatter->close_section(); + s->formatter->close_section(); // Roles + + const bool truncated = !listing.next_marker.empty(); + encode_json("IsTruncated", truncated, s->formatter); + if (truncated) { + encode_json("Marker", listing.next_marker, s->formatter); + } + + s->formatter->close_section(); // ListRolesResult s->formatter->open_object_section("ResponseMetadata"); s->formatter->dump_string("RequestId", s->trans_id); - s->formatter->close_section(); - s->formatter->close_section(); + s->formatter->close_section(); // ResponseMetadata + s->formatter->close_section(); // ListRolesResponse } } diff --git a/src/rgw/rgw_rest_role.h b/src/rgw/rgw_rest_role.h index fff9406227b32..be8f80b53ae43 100644 --- a/src/rgw/rgw_rest_role.h +++ b/src/rgw/rgw_rest_role.h @@ -81,6 +81,9 @@ public: class RGWListRoles : public RGWRestRole { std::string path_prefix; + std::string marker; + int max_items = 100; + std::string next_marker; public: RGWListRoles() : RGWRestRole(rgw::IAM::iamListRoles, RGW_CAP_READ) {} int init_processing(optional_yield y) override; diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h index d2e0e0f03d2ad..910a4e5b142cb 100644 --- a/src/rgw/rgw_sal.h +++ b/src/rgw/rgw_sal.h @@ -573,11 +573,13 @@ class Driver { virtual std::unique_ptr get_role(std::string id) = 0; virtual std::unique_ptr get_role(const RGWRoleInfo& info) = 0; /** Get all IAM Roles optionally filtered by path */ - virtual int get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) = 0; + virtual int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) = 0; /** Get an empty Open ID Connector provider */ virtual std::unique_ptr get_oidc_provider() = 0; /** Get all Open ID Connector providers, optionally filtered by tenant */ diff --git a/src/rgw/rgw_sal_dbstore.cc b/src/rgw/rgw_sal_dbstore.cc index bfd05e5d51112..4a0015e046d51 100644 --- a/src/rgw/rgw_sal_dbstore.cc +++ b/src/rgw/rgw_sal_dbstore.cc @@ -1411,11 +1411,13 @@ namespace rgw::sal { return std::unique_ptr(p); } - int DBStore::get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - vector>& roles) + int DBStore::list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) { return 0; } diff --git a/src/rgw/rgw_sal_dbstore.h b/src/rgw/rgw_sal_dbstore.h index 7a28bcc1a4f64..9a0f638e3da5b 100644 --- a/src/rgw/rgw_sal_dbstore.h +++ b/src/rgw/rgw_sal_dbstore.h @@ -910,11 +910,13 @@ public: std::multimap tags={}) override; virtual std::unique_ptr get_role(std::string id) override; virtual std::unique_ptr get_role(const RGWRoleInfo& info) override; - virtual int get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) override; + int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) override; virtual std::unique_ptr get_oidc_provider() override; virtual int get_oidc_providers(const DoutPrefixProvider *dpp, const std::string& tenant, diff --git a/src/rgw/rgw_sal_filter.cc b/src/rgw/rgw_sal_filter.cc index 4fe26e1c7e27f..6b17cd36d5349 100644 --- a/src/rgw/rgw_sal_filter.cc +++ b/src/rgw/rgw_sal_filter.cc @@ -570,13 +570,16 @@ std::unique_ptr FilterDriver::get_role(const RGWRoleInfo& info) return next->get_role(info); } -int FilterDriver::get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) -{ - return next->get_roles(dpp, y, path_prefix, tenant, roles); +int FilterDriver::list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) +{ + return next->list_roles(dpp, y, tenant, path_prefix, + marker, max_items, listing); } std::unique_ptr FilterDriver::get_oidc_provider() diff --git a/src/rgw/rgw_sal_filter.h b/src/rgw/rgw_sal_filter.h index 5dfa6c209a933..7b31f9c751fdc 100644 --- a/src/rgw/rgw_sal_filter.h +++ b/src/rgw/rgw_sal_filter.h @@ -396,11 +396,13 @@ public: std::multimap tags={}) override; virtual std::unique_ptr get_role(std::string id) override; virtual std::unique_ptr get_role(const RGWRoleInfo& info) override; - virtual int get_roles(const DoutPrefixProvider *dpp, - optional_yield y, - const std::string& path_prefix, - const std::string& tenant, - std::vector>& roles) override; + virtual int list_roles(const DoutPrefixProvider *dpp, + optional_yield y, + const std::string& tenant, + const std::string& path_prefix, + const std::string& marker, + uint32_t max_items, + RoleList& listing) override; virtual std::unique_ptr get_oidc_provider() override; virtual int get_oidc_providers(const DoutPrefixProvider *dpp, const std::string& tenant, -- 2.39.5