SERVER-53737 Check for sharded collections in renameCollectionLegacy

6 files changed, 34 insertions, 27 deletions

diff --git a/src/mongo/db/catalog/capped_utils.cpp b/src/mongo/db/catalog/capped_utils.cpp
index 0baba04783f..6fdd90d1de2 100644
--- a/src/mongo/db/catalog/capped_utils.cpp
+++ b/src/mongo/db/catalog/capped_utils.cpp
@@ -296,7 +296,6 @@ void convertToCapped(OperationContext* opCtx, const NamespaceString& ns, long lo
     RenameCollectionOptions options;
     options.dropTarget = true;
     options.stayTemp = false;
-    options.skipSourceCollectionShardedCheck = true;
     uassertStatusOK(renameCollection(opCtx, tempNs, ns, options));
 }
 
diff --git a/src/mongo/db/catalog/rename_collection.cpp b/src/mongo/db/catalog/rename_collection.cpp
index 8ee6c5c9ca6..e7f3e8f5c1e 100644
--- a/src/mongo/db/catalog/rename_collection.cpp
+++ b/src/mongo/db/catalog/rename_collection.cpp
@@ -52,7 +52,6 @@
 #include "mongo/db/ops/insert.h"
 #include "mongo/db/query/query_knobs_gen.h"
 #include "mongo/db/repl/replication_coordinator.h"
-#include "mongo/db/s/collection_sharding_state.h"
 #include "mongo/db/s/database_sharding_state.h"
 #include "mongo/db/server_options.h"
 #include "mongo/db/service_context.h"
@@ -71,11 +70,6 @@ boost::optional<NamespaceString> getNamespaceFromUUID(OperationContext* opCtx, c
     return CollectionCatalog::get(opCtx)->lookupNSSByUUID(opCtx, uuid);
 }
 
-bool isCollectionSharded(OperationContext* opCtx, const NamespaceString& nss) {
-    return opCtx->writesAreReplicated() &&
-        CollectionShardingState::get(opCtx, nss)->getCollectionDescription(opCtx).isSharded();
-}
-
 // From a replicated to an unreplicated collection or vice versa.
 bool isReplicatedChanged(OperationContext* opCtx,
                          const NamespaceString& source,
@@ -98,9 +92,6 @@ Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
                       str::stream() << "Not primary while renaming collection " << source << " to "
                                     << target);
 
-    if (!options.skipSourceCollectionShardedCheck && isCollectionSharded(opCtx, source))
-        return {ErrorCodes::IllegalOperation, "source namespace cannot be sharded"};
-
     if (isReplicatedChanged(opCtx, source, target))
         return {ErrorCodes::IllegalOperation,
                 "Cannot rename collections between a replicated and an unreplicated database"};
@@ -130,9 +121,6 @@ Status checkSourceAndTargetNamespaces(OperationContext* opCtx,
             return Status(ErrorCodes::NamespaceExists,
                           str::stream() << "a view already exists with that name: " << target);
     } else {
-        if (isCollectionSharded(opCtx, target))
-            return {ErrorCodes::IllegalOperation, "cannot rename to a sharded collection"};
-
         if (!targetExistsAllowed && !options.dropTarget)
             return Status(ErrorCodes::NamespaceExists, "target namespace exists");
     }
@@ -495,10 +483,6 @@ Status renameBetweenDBs(OperationContext* opCtx,
         return Status(ErrorCodes::NamespaceNotFound, "source namespace does not exist");
     }
 
-    // The source collection is not temporary, so we should check if is sharded or not.
-    if (isCollectionSharded(opCtx, source))
-        return {ErrorCodes::IllegalOperation, "source namespace cannot be sharded"};
-
     if (isReplicatedChanged(opCtx, source, target))
         return {ErrorCodes::IllegalOperation,
                 "Cannot rename collections between a replicated and an unreplicated database"};
@@ -518,9 +502,6 @@ Status renameBetweenDBs(OperationContext* opCtx,
             return Status::OK();
         }
 
-        if (isCollectionSharded(opCtx, target))
-            return {ErrorCodes::IllegalOperation, "cannot rename to a sharded collection"};
-
         if (!options.dropTarget) {
             return Status(ErrorCodes::NamespaceExists, "target namespace exists");
         }
@@ -708,7 +689,6 @@ Status renameBetweenDBs(OperationContext* opCtx,
     // in-place rename and remove the source collection.
     invariant(tmpName.db() == target.db());
     RenameCollectionOptions tempOptions(options);
-    tempOptions.skipSourceCollectionShardedCheck = true;
     Status status = renameCollectionWithinDB(opCtx, tmpName, target, tempOptions);
     if (!status.isOK())
         return status;
diff --git a/src/mongo/db/catalog/rename_collection.h b/src/mongo/db/catalog/rename_collection.h
index 2af16a8ddee..9b1c5ddf5ae 100644
--- a/src/mongo/db/catalog/rename_collection.h
+++ b/src/mongo/db/catalog/rename_collection.h
@@ -45,13 +45,11 @@ class OpTime;
 /**
  * Renames the collection from "source" to "target" and drops the existing collection if
  * "dropTarget" is true. "stayTemp" indicates whether a collection should maintain its
- * temporariness. "skipSourceCollectionShardedCheck" indicates the "source" collection sharding
- * state shouldn't be checked.
+ * temporariness.
  */
 struct RenameCollectionOptions {
     bool dropTarget = false;
     bool stayTemp = false;
-    bool skipSourceCollectionShardedCheck = false;
 };
 
 void doLocalRenameIfOptionsAndIndexesHaveNotChanged(OperationContext* opCtx,
diff --git a/src/mongo/db/commands/internal_rename_if_options_and_indexes_match_cmd.cpp b/src/mongo/db/commands/internal_rename_if_options_and_indexes_match_cmd.cpp
index 627e20b351a..f9384853ac4 100644
--- a/src/mongo/db/commands/internal_rename_if_options_and_indexes_match_cmd.cpp
+++ b/src/mongo/db/commands/internal_rename_if_options_and_indexes_match_cmd.cpp
@@ -35,8 +35,18 @@
 #include "mongo/db/catalog/rename_collection.h"
 #include "mongo/db/commands.h"
 #include "mongo/db/db_raii.h"
+#include "mongo/db/s/collection_sharding_state.h"
 
 namespace mongo {
+namespace {
+
+bool isCollectionSharded(OperationContext* opCtx, const NamespaceString& nss) {
+    AutoGetCollectionForRead lock(opCtx, nss);
+    return opCtx->writesAreReplicated() &&
+        CollectionShardingState::get(opCtx, nss)->getCollectionDescription(opCtx).isSharded();
+}
+
+}  // namespace
 
 /**
  * Rename a collection while checking collection option and indexes.
@@ -52,15 +62,20 @@ public:
 
         void typedRun(OperationContext* opCtx) {
             auto thisRequest = request();
+            auto toNss = thisRequest.getTo();
+
+            uassert(ErrorCodes::IllegalOperation,
+                    str::stream() << "cannot rename to sharded collection '" << toNss << "'",
+                    !isCollectionSharded(opCtx, toNss));
+
             auto originalIndexes = thisRequest.getIndexes();
             auto indexList = std::list<BSONObj>(originalIndexes.begin(), originalIndexes.end());
             RenameCollectionOptions options;
             options.dropTarget = true;
             options.stayTemp = false;
-            options.skipSourceCollectionShardedCheck = true;
             doLocalRenameIfOptionsAndIndexesHaveNotChanged(opCtx,
                                                            thisRequest.getFrom(),
-                                                           thisRequest.getTo(),
+                                                           toNss,
                                                            options,
                                                            std::move(indexList),
                                                            thisRequest.getCollectionOptions());
diff --git a/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp b/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
index 88971bc4b0c..26ae6cda451 100644
--- a/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
+++ b/src/mongo/db/pipeline/process_interface/non_shardsvr_process_interface.cpp
@@ -155,7 +155,6 @@ void NonShardServerProcessInterface::renameIfOptionsAndIndexesHaveNotChanged(
     options.dropTarget = renameCommandObj["dropTarget"].trueValue();
     options.stayTemp = renameCommandObj["stayTemp"].trueValue();
     // skip sharding validation on non sharded servers
-    options.skipSourceCollectionShardedCheck = true;
     doLocalRenameIfOptionsAndIndexesHaveNotChanged(
         opCtx, sourceNs, targetNs, options, originalIndexes, originalCollectionOptions);
 }
diff --git a/src/mongo/db/s/shardsvr_rename_collection.cpp b/src/mongo/db/s/shardsvr_rename_collection.cpp
index 57b4feb6291..b61de22b547 100644
--- a/src/mongo/db/s/shardsvr_rename_collection.cpp
+++ b/src/mongo/db/s/shardsvr_rename_collection.cpp
@@ -34,6 +34,8 @@
 #include "mongo/db/auth/authorization_session.h"
 #include "mongo/db/catalog/rename_collection.h"
 #include "mongo/db/commands.h"
+#include "mongo/db/db_raii.h"
+#include "mongo/db/s/collection_sharding_state.h"
 #include "mongo/db/s/sharding_state.h"
 #include "mongo/logv2/log.h"
 #include "mongo/s/grid.h"
@@ -42,6 +44,12 @@
 namespace mongo {
 namespace {
 
+bool isCollectionSharded(OperationContext* opCtx, const NamespaceString& nss) {
+    AutoGetCollectionForRead lock(opCtx, nss);
+    return opCtx->writesAreReplicated() &&
+        CollectionShardingState::get(opCtx, nss)->getCollectionDescription(opCtx).isSharded();
+}
+
 RenameCollectionResponse renameCollectionLegacy(OperationContext* opCtx,
                                                 const ShardsvrRenameCollection& request,
                                                 const NamespaceString& fromNss) {
@@ -57,6 +65,14 @@ RenameCollectionResponse renameCollectionLegacy(OperationContext* opCtx,
             "Source and destination collections must be on same shard",
             fromDB.primaryId() == toDB.primaryId());
 
+    // Make sure that source and target collection are not sharded
+    uassert(ErrorCodes::IllegalOperation,
+            str::stream() << "source namespace '" << fromNss << "' must not be sharded",
+            !isCollectionSharded(opCtx, fromNss));
+    uassert(ErrorCodes::IllegalOperation,
+            str::stream() << "cannot rename to sharded collection '" << toNss << "'",
+            !isCollectionSharded(opCtx, toNss));
+
     RenameCollectionOptions options;
     options.dropTarget = request.getDropTarget();
     options.stayTemp = request.getStayTemp();