SERVER-68769 If the shard key index cannot be dropped, it cannot be hidden.

9 files changed, 225 insertions, 30 deletions

diff --git a/jstests/libs/index_catalog_helpers.js b/jstests/libs/index_catalog_helpers.js
new file mode 100644
index 00000000000..a61267fbbbd
--- /dev/null
+++ b/jstests/libs/index_catalog_helpers.js
@@ -0,0 +1,85 @@
+"use strict";
+
+/**
+ * Helper functions that help test things to do with the index catalog.
+ */
+var IndexCatalogHelpers = (function() {
+    /**
+     * Returns the index specification with the name 'indexName' if it is present in the
+     * 'indexSpecs' array, and returns null otherwise.
+     */
+    function getIndexSpecByName(indexSpecs, indexName) {
+        if (typeof indexName !== "string") {
+            throw new Error("'indexName' parameter must be a string, but got " + tojson(indexName));
+        }
+
+        const found = indexSpecs.filter(spec => spec.name === indexName);
+
+        if (found.length > 1) {
+            throw new Error("Found multiple indexes with name '" + indexName +
+                            "': " + tojson(indexSpecs));
+        }
+        return (found.length === 1) ? found[0] : null;
+    }
+
+    /**
+     * Returns the index specification with the key pattern 'keyPattern' and the collation
+     * 'collation' if it is present in the 'indexSpecs' array, and returns null otherwise.
+     *
+     * The 'collation' parameter is optional and is only required to be specified when multiple
+     * indexes with the same key pattern exist.
+     */
+    function getIndexSpecByKeyPattern(indexSpecs, keyPattern, collation) {
+        const collationWasSpecified = arguments.length >= 3;
+        const foundByKeyPattern = indexSpecs.filter(spec => {
+            return bsonWoCompare(spec.key, keyPattern) === 0;
+        });
+
+        if (!collationWasSpecified) {
+            if (foundByKeyPattern.length > 1) {
+                throw new Error(
+                    "Found multiple indexes with key pattern " + tojson(keyPattern) +
+                    " and 'collation' parameter was not specified: " + tojson(indexSpecs));
+            }
+            return (foundByKeyPattern.length === 1) ? foundByKeyPattern[0] : null;
+        }
+
+        const foundByKeyPatternAndCollation = foundByKeyPattern.filter(spec => {
+            if (collation.locale === "simple") {
+                // The simple collation is not explicitly stored in the index catalog, so we expect
+                // the "collation" field to be absent.
+                return !spec.hasOwnProperty("collation");
+            }
+            return bsonWoCompare(spec.collation, collation) === 0;
+        });
+
+        if (foundByKeyPatternAndCollation.length > 1) {
+            throw new Error("Found multiple indexes with key pattern" + tojson(keyPattern) +
+                            " and collation " + tojson(collation) + ": " + tojson(indexSpecs));
+        }
+        return (foundByKeyPatternAndCollation.length === 1) ? foundByKeyPatternAndCollation[0]
+                                                            : null;
+    }
+
+    function createSingleIndex(coll, key, parameters) {
+        return coll.getDB().runCommand(
+            {createIndexes: coll.getName(), indexes: [Object.assign({key: key}, parameters)]});
+    }
+
+    function createIndexAndVerifyWithDrop(coll, key, parameters) {
+        coll.dropIndexes();
+        assert.commandWorked(createSingleIndex(coll, key, parameters));
+        assert.neq(
+            null,
+            getIndexSpecByName(coll.getIndexes(), parameters.name),
+            () =>
+                `Could not find index with name ${parameters.name}: ${tojson(coll.getIndexes())}`);
+    }
+
+    return {
+        findByName: getIndexSpecByName,
+        findByKeyPattern: getIndexSpecByKeyPattern,
+        createSingleIndex: createSingleIndex,
+        createIndexAndVerifyWithDrop: createIndexAndVerifyWithDrop,
+    };
+})();
diff --git a/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js b/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js
index 12c0f4bf6ef..ae9b843e445 100644
--- a/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js
+++ b/jstests/multiVersion/targetedTestsLastLtsFeatures/sharded_timeseries_collmod_mixed_version.js
@@ -32,11 +32,12 @@ assert.commandWorked(mongos.adminCommand({
 assert.commandWorked(mongos.adminCommand({setFeatureCompatibilityVersion: '5.0'}));
 
 const oldDb = st.s1.getDB(dbName);
+
+assert.commandWorked(db[collName].createIndex({[timeField]: 1}));
 // Assert that collMod works with matching versions of mongos and mongod.
-assert.commandWorked(db.runCommand({collMod: collName, index: {name: indexName, hidden: true}}));
+assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'tm_1', hidden: true}}));
 // Assert that collMod still works with old version of mongos.
-assert.commandWorked(
-    oldDb.runCommand({collMod: collName, index: {name: indexName, hidden: false}}));
+assert.commandWorked(oldDb.runCommand({collMod: collName, index: {name: 'tm_1', hidden: false}}));
 
 // Assert that collMod with granularity update fails with matching versions of mongos and mongod.
 assert.commandFailedWithCode(db.runCommand({collMod: collName, timeseries: {granularity: 'hours'}}),
diff --git a/jstests/sharding/hidden_index.js b/jstests/sharding/hidden_index.js
new file mode 100644
index 00000000000..6bf2c00cdf5
--- /dev/null
+++ b/jstests/sharding/hidden_index.js
@@ -0,0 +1,80 @@
+/**
+ * Test to validate that a shard key index cannot be hidden if it cannot be dropped.
+ *  @tags: [
+ *   requires_fcv_60,
+ * ]
+ */
+
+(function() {
+'use strict';
+load("jstests/libs/index_catalog_helpers.js");  // For IndexCatalogHelpers.findByName.
+
+// Test to validate the correct behaviour of hiding an index in a sharded cluster with a shard key.
+function validateHiddenIndexBehaviour() {
+    let index_type = 1;
+    let index_name = "a_" + index_type;
+    assert.commandWorked(coll.createIndex({"a": index_type}));
+
+    let idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name);
+    assert.eq(idxSpec.hidden, undefined);
+
+    assert.commandWorked(coll.hideIndex(index_name));
+    idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name);
+    assert(idxSpec.hidden);
+
+    assert.commandWorked(coll.unhideIndex(index_name));
+    idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name);
+    assert.eq(idxSpec.hidden, undefined);
+
+    assert.commandWorked(coll.dropIndex(index_name));
+    assert.commandWorked(coll.createIndex({"a": index_type}, {hidden: true}));
+
+    idxSpec = IndexCatalogHelpers.findByName(coll.getIndexes(), index_name);
+    assert(idxSpec.hidden);
+    assert.commandWorked(coll.dropIndexes());
+}
+
+// Check that command will fail when we try to hide the only shard key index of the collection
+function validateOneShardKeyHiddenIndexBehaviour() {
+    assert.commandFailedWithCode(coll.hideIndex("skey_1"), ErrorCodes.InvalidOptions);
+    assert.commandFailedWithCode(
+        testDb.runCommand({"collMod": coll.getName(), "index": {"name": "skey_1", "hidden": true}}),
+        ErrorCodes.InvalidOptions);
+}
+
+// Check that command will fail when we try to hide or drop an index that is the last shard key
+// index of the collection
+function validateDifferentHiddenIndexesBehaviour() {
+    // Create index on skey
+    assert.commandWorked(coll.createIndex({skey: 1, anotherkey: 1}));
+
+    assert.commandWorked(testDb.runCommand(
+        {"collMod": coll.getName(), "index": {"name": "skey_1", "hidden": true}}));
+
+    assert.commandFailedWithCode(
+        testDb.runCommand(
+            {"collMod": coll.getName(), "index": {"name": "skey_1_anotherkey_1", "hidden": true}}),
+        ErrorCodes.InvalidOptions);
+
+    assert.commandFailed(coll.dropIndex("skey_1_anotherkey_1"));
+}
+
+// Configure initial sharded cluster
+const st = new ShardingTest({shards: 2});
+const mongos = st.s;
+const testDb = mongos.getDB("test");
+const coll = testDb.getCollection("foo");
+
+// Enable sharding at collection 'foo' and create a new shard key
+assert.commandWorked(st.s.adminCommand({enableSharding: testDb.getName()}));
+
+// Crate a new shard key
+assert.commandWorked(
+    st.s.adminCommand({shardcollection: testDb.getName() + '.' + coll.getName(), key: {skey: 1}}));
+
+validateHiddenIndexBehaviour();
+validateOneShardKeyHiddenIndexBehaviour();
+validateDifferentHiddenIndexesBehaviour();
+
+st.stop();
+})();
diff --git a/jstests/sharding/timeseries_coll_mod.js b/jstests/sharding/timeseries_coll_mod.js
index 2627fa21cf4..36b33cb3e04 100644
--- a/jstests/sharding/timeseries_coll_mod.js
+++ b/jstests/sharding/timeseries_coll_mod.js
@@ -68,11 +68,10 @@ function runBasicTest(failPoint) {
         key: {[metaField]: 1},
     }));
 
-    // Normal collMod commands works for the sharded time-series collection.
-    assert.commandWorked(
-        db.runCommand({collMod: collName, index: {name: indexName, hidden: true}}));
-    assert.commandWorked(
-        db.runCommand({collMod: collName, index: {name: indexName, hidden: false}}));
+    // Check that collMod commands works for the sharded time-series collection.
+    assert.commandWorked(db[collName].createIndex({'a': 1}));
+    assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'a_1', hidden: true}}));
+    assert.commandWorked(db.runCommand({collMod: collName, index: {name: 'a_1', hidden: false}}));
 
     if (failPoint) {
         // Granularity update disabled for sharded time-series collection, when we're using primary
diff --git a/src/mongo/db/catalog/coll_mod.cpp b/src/mongo/db/catalog/coll_mod.cpp
index cedf7a39cef..4134c9bcf57 100644
--- a/src/mongo/db/catalog/coll_mod.cpp
+++ b/src/mongo/db/catalog/coll_mod.cpp
@@ -53,6 +53,7 @@
 #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/s/shard_key_index_util.h"
 #include "mongo/db/server_options.h"
 #include "mongo/db/service_context.h"
 #include "mongo/db/storage/recovery_unit.h"
@@ -303,6 +304,27 @@ StatusWith<std::pair<ParsedCollModRequest, BSONObj>> parseCollModRequest(Operati
             cmrIndex->idx = indexes[0];
         }
 
+        if (cmdIndex.getHidden()) {
+            // Checks if it is possible to drop the shard key, so it could be possible to hide it
+            if (auto catalogClient = Grid::get(opCtx)->catalogClient()) {
+                try {
+                    auto shardedColl = catalogClient->getCollection(opCtx, nss);
+
+                    if (isLastNonHiddenShardKeyIndex(opCtx,
+                                                     coll,
+                                                     coll->getIndexCatalog(),
+                                                     indexName.toString(),
+                                                     shardedColl.getKeyPattern().toBSON())) {
+                        return {ErrorCodes::InvalidOptions,
+                                "Cannot hide the only compatible index for this collection's shard "
+                                "key"};
+                    }
+                } catch (ExceptionFor<ErrorCodes::NamespaceNotFound>&) {
+                    // The collection is unsharded or doesn't exist.
+                }
+            }
+        }
+
         if (cmdIndex.getExpireAfterSeconds()) {
             parsed.numModifications++;
             BSONElement oldExpireSecs = cmrIndex->idx->infoObj().getField("expireAfterSeconds");
diff --git a/src/mongo/db/catalog/drop_indexes.cpp b/src/mongo/db/catalog/drop_indexes.cpp
index 2c10eb005c5..4114c3e9aae 100644
--- a/src/mongo/db/catalog/drop_indexes.cpp
+++ b/src/mongo/db/catalog/drop_indexes.cpp
@@ -344,7 +344,7 @@ void dropReadyIndexes(OperationContext* opCtx,
             uassert(
                 ErrorCodes::CannotDropShardKeyIndex,
                 "Cannot drop the only compatible index for this collection's shard key",
-                !isLastShardKeyIndex(
+                !isLastNonHiddenShardKeyIndex(
                     opCtx, collection, indexCatalog, indexName, collDescription.getKeyPattern()));
         }
 
@@ -515,11 +515,11 @@ DropIndexesReply dropIndexes(OperationContext* opCtx,
                 if (collDescription.isSharded()) {
                     uassert(ErrorCodes::CannotDropShardKeyIndex,
                             "Cannot drop the only compatible index for this collection's shard key",
-                            !isLastShardKeyIndex(opCtx,
-                                                 collection->getCollection(),
-                                                 indexCatalog,
-                                                 indexName,
-                                                 collDescription.getKeyPattern()));
+                            !isLastNonHiddenShardKeyIndex(opCtx,
+                                                          collection->getCollection(),
+                                                          indexCatalog,
+                                                          indexName,
+                                                          collDescription.getKeyPattern()));
                 }
 
                 auto desc = indexCatalog->findIndexByName(opCtx, indexName, includeUnfinished);
diff --git a/src/mongo/db/s/shard_key_index_util.cpp b/src/mongo/db/s/shard_key_index_util.cpp
index 56d6ca08656..542da1e129c 100644
--- a/src/mongo/db/s/shard_key_index_util.cpp
+++ b/src/mongo/db/s/shard_key_index_util.cpp
@@ -64,6 +64,10 @@ const boost::optional<ShardKeyIndex> _findShardKeyPrefixedIndex(
             continue;
         }
 
+        if (indexDescriptor->hidden()) {
+            continue;
+        }
+
         if (isCompatibleWithShardKey(opCtx, collection, indexEntry, shardKey, requireSingleKey)) {
             if (!indexEntry->isMultikey(opCtx, collection)) {
                 return ShardKeyIndex(indexDescriptor);
@@ -128,11 +132,11 @@ bool isCompatibleWithShardKey(OperationContext* opCtx,
     return false;
 }
 
-bool isLastShardKeyIndex(OperationContext* opCtx,
-                         const CollectionPtr& collection,
-                         const IndexCatalog* indexCatalog,
-                         const std::string& indexName,
-                         const BSONObj& shardKey) {
+bool isLastNonHiddenShardKeyIndex(OperationContext* opCtx,
+                                  const CollectionPtr& collection,
+                                  const IndexCatalog* indexCatalog,
+                                  const std::string& indexName,
+                                  const BSONObj& shardKey) {
     return !_findShardKeyPrefixedIndex(
                 opCtx, collection, indexCatalog, indexName, shardKey, false /* requireSingleKey */)
                 .is_initialized();
diff --git a/src/mongo/db/s/shard_key_index_util.h b/src/mongo/db/s/shard_key_index_util.h
index db59c57ceca..1fefa4af87e 100644
--- a/src/mongo/db/s/shard_key_index_util.h
+++ b/src/mongo/db/s/shard_key_index_util.h
@@ -81,6 +81,7 @@ bool isCompatibleWithShardKey(OperationContext* opCtx,
  * - must be prefixed by 'shardKey', and
  * - must not be a partial index.
  * - must have the simple collation.
+ * - must not be hidden.
  *
  * If the parameter 'requireSingleKey' is true, then this index additionally must not be
  * multi-key.
@@ -92,13 +93,13 @@ const boost::optional<ShardKeyIndex> findShardKeyPrefixedIndex(OperationContext*
                                                                bool requireSingleKey);
 
 /**
- * Returns true if the given index name is the last remaining index that is compatible with the
- * shard key index.
+ * Returns true if the given index name is the last remaining not hidden index that is compatible
+ * with the shard key index.
  */
-bool isLastShardKeyIndex(OperationContext* opCtx,
-                         const CollectionPtr& collection,
-                         const IndexCatalog* indexCatalog,
-                         const std::string& indexName,
-                         const BSONObj& shardKey);
+bool isLastNonHiddenShardKeyIndex(OperationContext* opCtx,
+                                  const CollectionPtr& collection,
+                                  const IndexCatalog* indexCatalog,
+                                  const std::string& indexName,
+                                  const BSONObj& shardKey);
 
 }  // namespace mongo
diff --git a/src/mongo/db/s/shard_key_index_util_test.cpp b/src/mongo/db/s/shard_key_index_util_test.cpp
index c9c1b4fdc23..bf1502b1ff3 100644
--- a/src/mongo/db/s/shard_key_index_util_test.cpp
+++ b/src/mongo/db/s/shard_key_index_util_test.cpp
@@ -203,9 +203,12 @@ TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithSingleCandidate) {
     createIndex(BSON("key" << BSON("x" << 1) << "name"
                            << "x"
                            << "v" << kIndexVersion));
+    createIndex(BSON("key" << BSON("x" << 1 << "y" << 1) << "name"
+                           << "xy"
+                           << "v" << kIndexVersion << "hidden" << true));
 
-    ASSERT_TRUE(
-        isLastShardKeyIndex(opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1)));
+    ASSERT_TRUE(isLastNonHiddenShardKeyIndex(
+        opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1)));
 }
 
 TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithMultipleCandidates) {
@@ -219,8 +222,8 @@ TEST_F(ShardKeyIndexUtilTest, LastShardIndexWithMultipleCandidates) {
                            << "xy"
                            << "v" << kIndexVersion));
 
-    ASSERT_FALSE(
-        isLastShardKeyIndex(opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1)));
+    ASSERT_FALSE(isLastNonHiddenShardKeyIndex(
+        opCtx(), coll(), coll()->getIndexCatalog(), "x", BSON("x" << 1)));
 }
 
 }  // namespace