From 9e2f8eb074db402f3d25264b7c0b299a95b81acc Mon Sep 17 00:00:00 2001 From: Shreyas Kalyan Date: Fri, 15 Jan 2021 19:48:03 -0800 Subject: SERVER-52555 Convert listIndexes command implementation to inherit from IDL-generated base class --- src/mongo/db/auth/authorization_session.h | 4 + src/mongo/db/auth/authorization_session_impl.cpp | 9 + src/mongo/db/auth/authorization_session_impl.h | 2 + src/mongo/db/catalog/list_indexes.cpp | 8 +- src/mongo/db/catalog/list_indexes.h | 6 +- src/mongo/db/commands/list_indexes.cpp | 271 +++++++++++----------- src/mongo/db/list_indexes.idl | 2 +- src/mongo/embedded/embedded_auth_session.cpp | 4 + src/mongo/s/commands/cluster_list_indexes_cmd.cpp | 117 ++++------ 9 files changed, 209 insertions(+), 214 deletions(-) (limited to 'src/mongo') diff --git a/src/mongo/db/auth/authorization_session.h b/src/mongo/db/auth/authorization_session.h index f5a4917cd35..88474551dea 100644 --- a/src/mongo/db/auth/authorization_session.h +++ b/src/mongo/db/auth/authorization_session.h @@ -252,6 +252,10 @@ public: // given BSONElement. virtual bool isAuthorizedToParseNamespaceElement(const BSONElement& elem) = 0; + // Checks if this connection has the privileges necessary to parse a namespace from a + // given NamespaceOrUUID object. + virtual bool isAuthorizedToParseNamespaceElement(const NamespaceStringOrUUID& nss) = 0; + // Checks if this connection has the privileges necessary to create a new role virtual bool isAuthorizedToCreateRole(const RoleName& roleName) = 0; diff --git a/src/mongo/db/auth/authorization_session_impl.cpp b/src/mongo/db/auth/authorization_session_impl.cpp index 001ab4692c0..51f638ebc4a 100644 --- a/src/mongo/db/auth/authorization_session_impl.cpp +++ b/src/mongo/db/auth/authorization_session_impl.cpp @@ -551,6 +551,15 @@ bool AuthorizationSessionImpl::isAuthorizedToParseNamespaceElement(const BSONEle return true; } +bool AuthorizationSessionImpl::isAuthorizedToParseNamespaceElement( + const NamespaceStringOrUUID& nss) { + if (nss.uuid()) { + return isAuthorizedForActionsOnResource(ResourcePattern::forClusterResource(), + ActionType::useUUID); + } + return true; +} + bool AuthorizationSessionImpl::isAuthorizedToCreateRole(const RoleName& roleName) { // A user is allowed to create a role under either of two conditions. diff --git a/src/mongo/db/auth/authorization_session_impl.h b/src/mongo/db/auth/authorization_session_impl.h index 08b8506aacf..cc9cda143fe 100644 --- a/src/mongo/db/auth/authorization_session_impl.h +++ b/src/mongo/db/auth/authorization_session_impl.h @@ -138,6 +138,8 @@ public: bool isAuthorizedToParseNamespaceElement(const BSONElement& elem) override; + bool isAuthorizedToParseNamespaceElement(const NamespaceStringOrUUID& nss) override; + bool isAuthorizedToCreateRole(const RoleName& roleName) override; bool isAuthorizedToGrantRole(const RoleName& role) override; diff --git a/src/mongo/db/catalog/list_indexes.cpp b/src/mongo/db/catalog/list_indexes.cpp index 027fb790243..8312e0f4998 100644 --- a/src/mongo/db/catalog/list_indexes.cpp +++ b/src/mongo/db/catalog/list_indexes.cpp @@ -51,7 +51,7 @@ namespace mongo { StatusWith> listIndexes(OperationContext* opCtx, const NamespaceStringOrUUID& ns, - bool includeBuildUUIDs) { + boost::optional includeBuildUUIDs) { AutoGetCollectionForReadCommandMaybeLockFree collection(opCtx, ns); auto nss = collection.getNss(); if (!collection) { @@ -66,7 +66,7 @@ StatusWith> listIndexes(OperationContext* opCtx, std::list listIndexesInLock(OperationContext* opCtx, const CollectionPtr& collection, const NamespaceString& nss, - bool includeBuildUUIDs) { + boost::optional includeBuildUUIDs) { invariant(opCtx->lockState()->isCollectionLockedForMode(nss, MODE_IS)); auto durableCatalog = DurableCatalog::get(opCtx); @@ -84,7 +84,7 @@ std::list listIndexesInLock(OperationContext* opCtx, for (size_t i = 0; i < indexNames.size(); i++) { auto indexSpec = writeConflictRetry(opCtx, "listIndexes", nss.ns(), [&] { - if (includeBuildUUIDs && + if (includeBuildUUIDs.value_or(false) && !durableCatalog->isIndexReady(opCtx, collection->getCatalogId(), indexNames[i])) { // The durable catalog will not have a build UUID for the given index name if it was // not being built with two-phase. @@ -110,7 +110,7 @@ std::list listIndexesInLock(OperationContext* opCtx, } std::list listIndexesEmptyListIfMissing(OperationContext* opCtx, const NamespaceStringOrUUID& nss, - bool includeBuildUUIDs) { + boost::optional includeBuildUUIDs) { auto listStatus = listIndexes(opCtx, nss, includeBuildUUIDs); return listStatus.isOK() ? listStatus.getValue() : std::list(); } diff --git a/src/mongo/db/catalog/list_indexes.h b/src/mongo/db/catalog/list_indexes.h index 1dd298c1b15..fd3053fa75f 100644 --- a/src/mongo/db/catalog/list_indexes.h +++ b/src/mongo/db/catalog/list_indexes.h @@ -42,13 +42,13 @@ namespace mongo { */ StatusWith> listIndexes(OperationContext* opCtx, const NamespaceStringOrUUID& ns, - bool includeBuildUUIDs); + boost::optional includeBuildUUIDs); std::list listIndexesInLock(OperationContext* opCtx, const CollectionPtr& collection, const NamespaceString& nss, - bool includeBuildUUIDs); + boost::optional includeBuildUUIDs); std::list listIndexesEmptyListIfMissing(OperationContext* opCtx, const NamespaceStringOrUUID& nss, - bool includeBuildUUIDs); + boost::optional includeBuildUUIDs); } // namespace mongo diff --git a/src/mongo/db/commands/list_indexes.cpp b/src/mongo/db/commands/list_indexes.cpp index d08b649f43e..7cd9d857a55 100644 --- a/src/mongo/db/commands/list_indexes.cpp +++ b/src/mongo/db/commands/list_indexes.cpp @@ -66,15 +66,6 @@ using std::vector; namespace { -void appendListIndexesCursorReply(CursorId cursorId, - const NamespaceString& cursorNss, - std::vector&& firstBatch, - BSONObjBuilder& result) { - auto reply = - ListIndexesReply(ListIndexesReplyCursor(cursorId, cursorNss, std::move(firstBatch))); - reply.serialize(&result); -} - /** * Lists the indexes for a given collection. * If 'includeBuildUUIDs' is true, then the index build uuid is also returned alongside the index @@ -102,166 +93,168 @@ void appendListIndexesCursorReply(CursorId cursorId, * buildUUID: * } */ -class CmdListIndexes : public BasicCommand { -public: - CmdListIndexes() : BasicCommand("listIndexes") {} - - const std::set& apiVersions() const { - return kApiVersions1; - } - AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { +class CmdListIndexes final : public ListIndexesCmdVersion1Gen { +public: + AllowedOnSecondary secondaryAllowed(ServiceContext*) const final { return AllowedOnSecondary::kOptIn; } - - bool maintenanceOk() const override { - return false; - } - virtual bool adminOnly() const { + bool maintenanceOk() const final { return false; } - virtual bool supportsWriteConcern(const BSONObj& cmd) const override { + bool adminOnly() const final { return false; } - bool collectsResourceConsumptionMetrics() const override { + bool collectsResourceConsumptionMetrics() const final { return true; } - std::string help() const override { + std::string help() const final { return "list indexes for a collection"; } - Status checkAuthForOperation(OperationContext* opCtx, - const std::string& dbname, - const BSONObj& cmdObj) const override { - AuthorizationSession* authzSession = AuthorizationSession::get(opCtx->getClient()); + class Invocation final : public InvocationBaseGen { + public: + using InvocationBaseGen::InvocationBaseGen; - if (!authzSession->isAuthorizedToParseNamespaceElement(cmdObj.firstElement())) { - return Status(ErrorCodes::Unauthorized, "Unauthorized"); + bool supportsWriteConcern() const final { + return false; } - // Check for the listIndexes ActionType on the database. - const auto nss = CollectionCatalog::get(opCtx)->resolveNamespaceStringOrUUID( - opCtx, CommandHelpers::parseNsOrUUID(dbname, cmdObj)); - if (authzSession->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(nss), - ActionType::listIndexes)) { - return Status::OK(); + NamespaceString ns() const final { + auto nss = request().getNamespaceOrUUID(); + if (nss.uuid()) { + // UUID requires opCtx to resolve, settle on just the dbname. + return NamespaceString(request().getDbName(), ""); + } + invariant(nss.nss()); + return nss.nss().get(); } - return Status(ErrorCodes::Unauthorized, - str::stream() - << "Not authorized to list indexes on collection: " << nss.ns()); - } + void doCheckAuthorization(OperationContext* opCtx) const final { + AuthorizationSession* authzSession = AuthorizationSession::get(opCtx->getClient()); + + auto& cmd = request(); + uassert( + ErrorCodes::Unauthorized, + "Unauthorized", + authzSession->isAuthorizedToParseNamespaceElement(request().getNamespaceOrUUID())); + + const auto nss = CollectionCatalog::get(opCtx)->resolveNamespaceStringOrUUID( + opCtx, cmd.getNamespaceOrUUID()); - bool run(OperationContext* opCtx, - const string& dbname, - const BSONObj& cmdObj, - BSONObjBuilder& result) { - CommandHelpers::handleMarkKillOnClientDisconnect(opCtx); - const auto parsed = ListIndexes::parse({"listIndexes"}, cmdObj); - long long batchSize = std::numeric_limits::max(); - if (parsed.getCursor() && parsed.getCursor()->getBatchSize()) { - batchSize = *parsed.getCursor()->getBatchSize(); + uassert(ErrorCodes::Unauthorized, + str::stream() << "Not authorized to list indexes on collection:" << nss.ns(), + authzSession->isAuthorizedForActionsOnResource( + ResourcePattern::forExactNamespace(nss), ActionType::listIndexes)); } - NamespaceString nss; - std::unique_ptr exec; - std::vector firstBatch; - { - AutoGetCollectionForReadCommandMaybeLockFree collection(opCtx, - parsed.getNamespaceOrUUID()); - uassert(ErrorCodes::NamespaceNotFound, - str::stream() << "ns does not exist: " << collection.getNss().ns(), - collection); - nss = collection.getNss(); - - auto expCtx = make_intrusive( - opCtx, std::unique_ptr(nullptr), nss); - - auto indexList = listIndexesInLock( - opCtx, collection.getCollection(), nss, parsed.getIncludeBuildUUIDs()); - auto ws = std::make_unique(); - auto root = std::make_unique(expCtx.get(), ws.get()); - - for (auto&& indexSpec : indexList) { - WorkingSetID id = ws->allocate(); - WorkingSetMember* member = ws->get(id); - member->keyData.clear(); - member->recordId = RecordId(); - member->resetDocument(SnapshotId(), indexSpec.getOwned()); - member->transitionToOwnedObj(); - root->pushBack(id); + ListIndexesReply typedRun(OperationContext* opCtx) final { + CommandHelpers::handleMarkKillOnClientDisconnect(opCtx); + auto& cmd = request(); + + long long batchSize = std::numeric_limits::max(); + if (cmd.getCursor() && cmd.getCursor()->getBatchSize()) { + batchSize = *cmd.getCursor()->getBatchSize(); } - exec = uassertStatusOK( - plan_executor_factory::make(expCtx, - std::move(ws), - std::move(root), - &CollectionPtr::null, - PlanYieldPolicy::YieldPolicy::NO_YIELD, - false, /* whether returned BSON must be owned */ - nss)); - - int bytesBuffered = 0; - for (long long objCount = 0; objCount < batchSize; objCount++) { - BSONObj nextDoc; - PlanExecutor::ExecState state = exec->getNext(&nextDoc, nullptr); - if (state == PlanExecutor::IS_EOF) { - break; + NamespaceString nss; + std::unique_ptr exec; + std::vector firstBatch; + { + AutoGetCollectionForReadCommandMaybeLockFree collection(opCtx, + cmd.getNamespaceOrUUID()); + uassert(ErrorCodes::NamespaceNotFound, + str::stream() << "ns does not exist: " << collection.getNss().ns(), + collection); + nss = collection.getNss(); + + auto expCtx = make_intrusive( + opCtx, std::unique_ptr(nullptr), nss); + + auto indexList = listIndexesInLock( + opCtx, collection.getCollection(), nss, cmd.getIncludeBuildUUIDs()); + auto ws = std::make_unique(); + auto root = std::make_unique(expCtx.get(), ws.get()); + + for (auto&& indexSpec : indexList) { + WorkingSetID id = ws->allocate(); + WorkingSetMember* member = ws->get(id); + member->keyData.clear(); + member->recordId = RecordId(); + member->resetDocument(SnapshotId(), indexSpec.getOwned()); + member->transitionToOwnedObj(); + root->pushBack(id); } - invariant(state == PlanExecutor::ADVANCED); - // If we can't fit this result inside the current batch, then we stash it for later. - if (!FindCommon::haveSpaceForNext(nextDoc, objCount, bytesBuffered)) { - exec->enqueue(nextDoc); - break; + exec = uassertStatusOK( + plan_executor_factory::make(expCtx, + std::move(ws), + std::move(root), + &CollectionPtr::null, + PlanYieldPolicy::YieldPolicy::NO_YIELD, + false, /* whether returned BSON must be owned */ + nss)); + + int bytesBuffered = 0; + for (long long objCount = 0; objCount < batchSize; objCount++) { + BSONObj nextDoc; + PlanExecutor::ExecState state = exec->getNext(&nextDoc, nullptr); + if (state == PlanExecutor::IS_EOF) { + break; + } + invariant(state == PlanExecutor::ADVANCED); + + // If we can't fit this result inside the current batch, then we stash it for + // later. + if (!FindCommon::haveSpaceForNext(nextDoc, objCount, bytesBuffered)) { + exec->enqueue(nextDoc); + break; + } + + try { + firstBatch.push_back(ListIndexesReplyItem::parse( + IDLParserErrorContext("ListIndexesReplyItem"), nextDoc)); + } catch (const DBException& exc) { + LOGV2_ERROR(5254500, + "Could not parse catalog entry while replying to listIndexes", + "entry"_attr = nextDoc, + "error"_attr = exc); + uasserted(5254501, + "Could not parse catalog entry while replying to listIndexes"); + } + bytesBuffered += nextDoc.objsize(); } - try { - firstBatch.push_back(ListIndexesReplyItem::parse( - IDLParserErrorContext("ListIndexesReplyItem"), nextDoc)); - } catch (const DBException& exc) { - LOGV2_ERROR(5254500, - "Could not parse catalog entry while replying to listIndexes", - "entry"_attr = nextDoc, - "error"_attr = exc); - uasserted(5254501, - "Could not parse catalog entry while replying to listIndexes"); + if (exec->isEOF()) { + return ListIndexesReply( + ListIndexesReplyCursor(0 /* cursorId */, nss, std::move(firstBatch))); } - bytesBuffered += nextDoc.objsize(); - } - - if (exec->isEOF()) { - appendListIndexesCursorReply(0 /* cursorId */, nss, std::move(firstBatch), result); - return true; - } - - exec->saveState(); - exec->detachFromOperationContext(); - } // Drop collection lock. Global cursor registration must be done without holding any - // locks. - - auto pinnedCursor = CursorManager::get(opCtx)->registerCursor( - opCtx, - {std::move(exec), - nss, - AuthorizationSession::get(opCtx->getClient())->getAuthenticatedUserNames(), - APIParameters::get(opCtx), - opCtx->getWriteConcern(), - repl::ReadConcernArgs::get(opCtx), - cmdObj, - {Privilege(ResourcePattern::forExactNamespace(nss), ActionType::listIndexes)}}); - - pinnedCursor->incNBatches(); - pinnedCursor->incNReturnedSoFar(firstBatch.size()); - - appendListIndexesCursorReply( - pinnedCursor.getCursor()->cursorid(), nss, std::move(firstBatch), result); - - return true; - } + exec->saveState(); + exec->detachFromOperationContext(); + } // Drop collection lock. Global cursor registration must be done without holding any + // locks. + + auto pinnedCursor = CursorManager::get(opCtx)->registerCursor( + opCtx, + {std::move(exec), + nss, + AuthorizationSession::get(opCtx->getClient())->getAuthenticatedUserNames(), + APIParameters::get(opCtx), + opCtx->getWriteConcern(), + repl::ReadConcernArgs::get(opCtx), + cmd.toBSON({}), + {Privilege(ResourcePattern::forExactNamespace(nss), ActionType::listIndexes)}}); + + pinnedCursor->incNBatches(); + pinnedCursor->incNReturnedSoFar(firstBatch.size()); + + return ListIndexesReply(ListIndexesReplyCursor( + pinnedCursor.getCursor()->cursorid(), nss, std::move(firstBatch))); + } + }; } cmdListIndexes; } // namespace diff --git a/src/mongo/db/list_indexes.idl b/src/mongo/db/list_indexes.idl index 81b592c9a10..e53c8947da5 100644 --- a/src/mongo/db/list_indexes.idl +++ b/src/mongo/db/list_indexes.idl @@ -162,6 +162,6 @@ commands: optional: true includeBuildUUIDs: type: safeBool + optional: true unstable: true - default: false reply_type: ListIndexesReply diff --git a/src/mongo/embedded/embedded_auth_session.cpp b/src/mongo/embedded/embedded_auth_session.cpp index 4203ec8ca78..60030d1b783 100644 --- a/src/mongo/embedded/embedded_auth_session.cpp +++ b/src/mongo/embedded/embedded_auth_session.cpp @@ -173,6 +173,10 @@ public: return true; } + bool isAuthorizedToParseNamespaceElement(const NamespaceStringOrUUID&) override { + return true; + } + bool isAuthorizedToCreateRole(const RoleName&) override { return true; } diff --git a/src/mongo/s/commands/cluster_list_indexes_cmd.cpp b/src/mongo/s/commands/cluster_list_indexes_cmd.cpp index d0be564ab86..7fca87f809c 100644 --- a/src/mongo/s/commands/cluster_list_indexes_cmd.cpp +++ b/src/mongo/s/commands/cluster_list_indexes_cmd.cpp @@ -34,18 +34,18 @@ #include "mongo/db/auth/authorization_session.h" #include "mongo/db/commands.h" #include "mongo/db/list_indexes_gen.h" +#include "mongo/rpc/get_status_from_command_result.h" #include "mongo/s/cluster_commands_helpers.h" #include "mongo/s/query/store_possible_cursor.h" namespace mongo { namespace { -bool cursorCommandPassthroughShardWithMinKeyChunk(OperationContext* opCtx, - const NamespaceString& nss, - const ChunkManager& cm, - const BSONObj& cmdObj, - BSONObjBuilder* out, - const PrivilegeVector& privileges) { +ListIndexesReply cursorCommandPassthroughShardWithMinKeyChunk(OperationContext* opCtx, + const NamespaceString& nss, + const ChunkManager& cm, + const BSONObj& cmdObj, + const PrivilegeVector& privileges) { auto response = executeCommandAgainstShardWithMinKeyChunk( opCtx, nss, @@ -65,82 +65,65 @@ bool cursorCommandPassthroughShardWithMinKeyChunk(OperationContext* opCtx, Grid::get(opCtx)->getCursorManager(), privileges)); - CommandHelpers::filterCommandReplyForPassthrough(transformedResponse, out); - if (out->asTempObj()["ok"].trueValue()) { - // The reply syntax must conform to its IDL definition. - ListIndexesReply::parse({"listIndexes"}, out->asTempObj()); - } - return true; + BSONObjBuilder out; + CommandHelpers::filterCommandReplyForPassthrough(transformedResponse, &out); + const auto& resultObj = out.obj(); + uassertStatusOK(getStatusFromCommandResult(resultObj)); + // The reply syntax must conform to its IDL definition. + return ListIndexesReply::parse({"listIndexes"}, resultObj); } -class CmdListIndexes : public BasicCommand { +class CmdListIndexes final : public ListIndexesCmdVersion1Gen { public: - CmdListIndexes() : BasicCommand("listIndexes") {} - - const std::set& apiVersions() const { - return kApiVersions1; - } - - std::string parseNs(const std::string& dbname, const BSONObj& cmdObj) const override { - return CommandHelpers::parseNsCollectionRequired(dbname, cmdObj).ns(); - } - - bool supportsWriteConcern(const BSONObj& cmd) const override { - return false; - } - - AllowedOnSecondary secondaryAllowed(ServiceContext*) const override { + AllowedOnSecondary secondaryAllowed(ServiceContext*) const final { return AllowedOnSecondary::kAlways; } - - bool maintenanceOk() const override { + bool maintenanceOk() const final { return false; } - - bool adminOnly() const override { + bool adminOnly() const final { return false; } - Status checkAuthForCommand(Client* client, - const std::string& dbname, - const BSONObj& cmdObj) const override { - AuthorizationSession* authzSession = AuthorizationSession::get(client); - - // Check for the listIndexes ActionType on the database. - const NamespaceString ns(parseNs(dbname, cmdObj)); + class Invocation final : public InvocationBaseGen { + public: + using InvocationBaseGen::InvocationBaseGen; - if (authzSession->isAuthorizedForActionsOnResource(ResourcePattern::forExactNamespace(ns), - ActionType::listIndexes)) { - return Status::OK(); + bool supportsWriteConcern() const final { + return false; } - return Status(ErrorCodes::Unauthorized, - str::stream() - << "Not authorized to list indexes on collection: " << ns.coll()); - } - - bool run(OperationContext* opCtx, - const std::string& dbName, - const BSONObj& cmdObj, - BSONObjBuilder& result) override { - CommandHelpers::handleMarkKillOnClientDisconnect(opCtx); - // Check the command syntax before passing to shard. - const auto parsed = ListIndexes::parse({"listIndexes"}, cmdObj); - - // The command's IDL definition permits namespace or UUID, but mongos requires a namespace. - const NamespaceString nss(parseNs(dbName, cmdObj)); - const auto cm = - uassertStatusOK(Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfo(opCtx, nss)); + NamespaceString ns() const final { + const auto& nss = request().getNamespaceOrUUID().nss(); + uassert( + ErrorCodes::BadValue, "Mongos requires a namespace for listIndexes command", nss); + return nss.get(); + } - return cursorCommandPassthroughShardWithMinKeyChunk( - opCtx, - nss, - cm, - applyReadWriteConcern(opCtx, this, cmdObj), - &result, - {Privilege(ResourcePattern::forExactNamespace(nss), ActionType::listIndexes)}); - } + void doCheckAuthorization(OperationContext* opCtx) const final { + AuthorizationSession* authzSession = AuthorizationSession::get(opCtx->getClient()); + uassert(ErrorCodes::Unauthorized, + str::stream() << "Not authorized to list indexes on collection:" + << ns().toString(), + authzSession->isAuthorizedForActionsOnResource( + ResourcePattern::forExactNamespace(ns()), ActionType::listIndexes)); + } + ListIndexesReply typedRun(OperationContext* opCtx) final { + CommandHelpers::handleMarkKillOnClientDisconnect(opCtx); + // The command's IDL definition permits namespace or UUID, but mongos requires a + // namespace. + const auto cm = uassertStatusOK( + Grid::get(opCtx)->catalogCache()->getCollectionRoutingInfo(opCtx, ns())); + + return cursorCommandPassthroughShardWithMinKeyChunk( + opCtx, + ns(), + cm, + applyReadWriteConcern(opCtx, this, request().toBSON({})), + {Privilege(ResourcePattern::forExactNamespace(ns()), ActionType::listIndexes)}); + } + }; } cmdListIndexes; } // namespace -- cgit v1.2.1