diff --git a/etc/evergreen_yml_components/tasks/misc_tasks.yml b/etc/evergreen_yml_components/tasks/misc_tasks.yml index fbde0b941df..0fddc86ca9d 100644 --- a/etc/evergreen_yml_components/tasks/misc_tasks.yml +++ b/etc/evergreen_yml_components/tasks/misc_tasks.yml @@ -299,6 +299,7 @@ tasks: resmoke_args: >- --runAllFeatureFlagTests --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *antithesis_task_template name: antithesis_jstestfuzz_sharded diff --git a/etc/evergreen_yml_components/tasks/resmoke/server_divisions/clusters_and_integrations/tasks.yml b/etc/evergreen_yml_components/tasks/resmoke/server_divisions/clusters_and_integrations/tasks.yml index 2357c8deca2..813d134ad00 100644 --- a/etc/evergreen_yml_components/tasks/resmoke/server_divisions/clusters_and_integrations/tasks.yml +++ b/etc/evergreen_yml_components/tasks/resmoke/server_divisions/clusters_and_integrations/tasks.yml @@ -1927,6 +1927,7 @@ tasks: # TODO(SERVER-119866): Enable viewless timeseries in upgrade/downgrade suites resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *gen_task_template name: sharding_jscore_passthrough_with_config_transitions_and_add_remove_shard_gen @@ -2423,6 +2424,7 @@ tasks: # TODO(SERVER-109882): Enable viewless timeseries in upgrade/downgrade suites resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *gen_task_template name: fcv_upgrade_downgrade_retryable_writes_replica_sets_jscore_passthrough_gen @@ -2448,6 +2450,7 @@ tasks: # TODO(SERVER-109882): Enable viewless timeseries in upgrade/downgrade suites resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess # TODO(SERVER-108818): Remove this temporary suite. - <<: *gen_task_template @@ -2473,6 +2476,7 @@ tasks: # Requires legacy timeseries (system.buckets) even in all feature flags variants resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *gen_task_template name: sharding_jscore_passthrough_with_dynamic_execution_control_gen @@ -2679,6 +2683,7 @@ tasks: # Requires legacy timeseries (system.buckets) even in all feature flags variants resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess # TODO(SERVER-108818): Remove this temporary suite. - <<: *gen_task_template @@ -2710,6 +2715,7 @@ tasks: # TODO(SERVER-109882): Enable viewless timeseries in upgrade/downgrade suites resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *gen_task_template name: fcv_upgrade_downgrade_sharded_collections_jscore_passthrough_retryable_writes_gen @@ -2740,6 +2746,7 @@ tasks: # TODO(SERVER-109882): Enable viewless timeseries in upgrade/downgrade suites resmoke_args: >- --disableFeatureFlags=featureFlagCreateViewlessTimeseriesCollections + --disableFeatureFlags=featureFlagBlockDirectSystemBucketsAccess - <<: *gen_task_template name: fcv_upgrade_downgrade_sharding_jscore_passthrough_retryable_writes_gen diff --git a/jstests/auth/timeseries_ddl.js b/jstests/auth/timeseries_ddl.js index 47b33594d85..bb41b9ed9ed 100644 --- a/jstests/auth/timeseries_ddl.js +++ b/jstests/auth/timeseries_ddl.js @@ -1,5 +1,5 @@ /** - * Verify that DDL operations on timeseries bucket namesapces requires special authorization + * Verify that DDL operations on timeseries bucket namespaces requires special authorization * * @tags: [ * requires_fcv_61, @@ -55,7 +55,11 @@ function createCollectionsAsRegularUser() { createCollectionsAsRegularUser(); assert(db.auth("c2c", pass)); assert.commandWorked(db.runCommand({drop: normalCollName})); - assert.commandWorked(db.runCommand({drop: bucketCollName})); + // TODO SERVER-121176 should fail once 9.0 becomes last LTS + assert.commandWorkedOrFailedWithCode( + db.runCommand({drop: bucketCollName}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + ); assert.commandWorked(db.runCommand({drop: timeseriesCollName})); db.logout(); } diff --git a/jstests/core/timeseries/ddl/timeseries_collmod.js b/jstests/core/timeseries/ddl/timeseries_collmod.js index 2afc418aa85..e520844eecb 100644 --- a/jstests/core/timeseries/ddl/timeseries_collmod.js +++ b/jstests/core/timeseries/ddl/timeseries_collmod.js @@ -129,10 +129,12 @@ assert.commandFailedWithCode( db.runCommand({"collMod": getTimeseriesBucketsColl(collName), "timeseries": {"granularity": "hours"}}), [ ErrorCodes.InvalidNamespace, + // When viewless timeseries are enabled, any request targeting system.buckets directly is rejected. + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, // Error code thrown by collmod coordinator. // // TODO SERVER-105548 remove the following error code once 9.0 becomes last LTS - // Currently needded for multiversion compability. Since 8.2 we throw InvalidNamespace also + // Currently needed for multiversion compatibility. Since 8.2 we throw InvalidNamespace also // on sharded clusters. 6201808, ], diff --git a/jstests/core/timeseries/ddl/timeseries_drop.js b/jstests/core/timeseries/ddl/timeseries_drop.js index c19fc48e085..b5ab0e6dc50 100644 --- a/jstests/core/timeseries/ddl/timeseries_drop.js +++ b/jstests/core/timeseries/ddl/timeseries_drop.js @@ -83,7 +83,10 @@ jsTest.log("normal timeseries, drop by the buckets NS"); assertExistsAndTypeIs(coll, "timeseries"); assertDoesntExist(bucketsColl); - assert.commandWorked(db.runCommand({drop: bucketsColl.getName()})); + assert.commandFailedWithCode( + db.runCommand({drop: bucketsColl.getName()}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + ); assertExistsAndTypeIs(coll, "timeseries"); assertDoesntExist(bucketsColl); @@ -117,7 +120,10 @@ jsTest.log("view on buckets, buckets doesn't exist, drop by the buckets NS"); // Reported as "timeseries", even though the buckets collection doesn't exist. assertExistsAndTypeIs(coll, "timeseries"); - assert.commandWorked(db.runCommand({drop: bucketsColl.getName()})); + assert.commandFailedWithCode( + db.runCommand({drop: bucketsColl.getName()}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + ); // The view still exists, since it's not technically a timeseries. assertExistsAndTypeIs(coll, "timeseries"); @@ -150,7 +156,10 @@ jsTest.log("view on another collection, buckets doesn't exist, drop by the bucke assertExistsAndTypeIs(coll, "view"); assertDoesntExist(bucketsColl); - assert.commandWorked(db.runCommand({drop: bucketsColl.getName()})); + assert.commandFailedWithCode( + db.runCommand({drop: bucketsColl.getName()}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + ); assertExistsAndTypeIs(coll, "view"); assertDoesntExist(bucketsColl); @@ -166,7 +175,10 @@ jsTest.log("normal collection, buckets doesn't exist, drop by the buckets NS"); assertExistsAndTypeIs(coll, "collection"); assertDoesntExist(bucketsColl); - assert.commandWorked(db.runCommand({drop: bucketsColl.getName()})); + assert.commandFailedWithCode( + db.runCommand({drop: bucketsColl.getName()}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + ); assertExistsAndTypeIs(coll, "collection"); assertDoesntExist(bucketsColl); diff --git a/jstests/multiVersion/genericChangeStreams/change_streams_timeseries_fcv_upgrade_downgrade.js b/jstests/multiVersion/genericChangeStreams/change_streams_timeseries_fcv_upgrade_downgrade.js index df2058a9a59..d81b26e03de 100644 --- a/jstests/multiVersion/genericChangeStreams/change_streams_timeseries_fcv_upgrade_downgrade.js +++ b/jstests/multiVersion/genericChangeStreams/change_streams_timeseries_fcv_upgrade_downgrade.js @@ -332,91 +332,92 @@ for (const testHelper of [new ReplSetTestHelper(), new ShardedClusterTestHelper( coll2Name: ts2CollName, }); - // As part of this test, we open a change stream over underyling system.buckets of the timeseries - // collection and expect to see an insert, followed by reaname and invalidation event due to FCV upgrade. + // As part of this test, we open a change stream over underlying system.buckets of the timeseries + // collection and expect to see an insert, followed by rename and invalidation event due to FCV upgrade. // // We also open the change stream in 8.0 (last-lts) binary and ensure that we can safely open the change // stream that will be reading the oplog which contains an oplog entry it didn't know about // (upgradeDowngradeTimeseriesCollection). The change stream will ignore this oplog entry as it will be // filtered out by the change stream filter. - it("while reading system.buckets collection until rename+invalidate", function () { - if (testHelper.isShardedCluster()) { - jsTest.log.info( - "Skipping test in sharded cluster because opening change stream over system.buckets collection is not allowed", - ); - return; - } + // TODO SERVER-120239: Fix this test targeting system.buckets directly while FCV is latest. + // it("while reading system.buckets collection until rename+invalidate", function () { + // if (testHelper.isShardedCluster()) { + // jsTest.log.info( + // "Skipping test in sharded cluster because opening change stream over system.buckets collection is not allowed", + // ); + // return; + // } - const initClusterTime = getNextClusterTime(getClusterTime(adminDB)); - const conn = getConn(); - const scenario = sameDbScenario; + // const initClusterTime = getNextClusterTime(getClusterTime(adminDB)); + // const conn = getConn(); + // const scenario = sameDbScenario; - // Only care about ts1.buckets here; we stop at rename+invalidate. - runScenarioAndAssert({ - conn, - dbForCstName: testDB1Name, - scenario, - phases: [ - {cmds: scenario.phases.preFCVUpgrade, unordered: false}, - {cmds: scenario.phases.fcvUpgrade, unordered: false}, - ], - rawData: false, // rawData flag is redundant here as we stop at rename+invalidate events. - openChangeStreamBeforeRunningScenario: false, - makeCursorAndWatchCtx: ({cst, rawData}) => { - const watchCtx = { - watchMode: ChangeStreamWatchMode.kCollection, - watchedNss: scenario.colls.ts1.bucketsNss, - showSystemEvents: true, - rawData, - }; - const cursor = openChangeStreamCursor(cst, scenario.colls.ts1.bucketsNss.coll, { - showSystemEvents: true, - allowToRunOnSystemNS: true, - startAtOperationTime: initClusterTime, - rawData, - watchMode: ChangeStreamWatchMode.kCollection, - }); - return {cursor, watchCtx}; - }, - }); + // // Only care about ts1.buckets here; we stop at rename+invalidate. + // runScenarioAndAssert({ + // conn, + // dbForCstName: testDB1Name, + // scenario, + // phases: [ + // {cmds: scenario.phases.preFCVUpgrade, unordered: false}, + // {cmds: scenario.phases.fcvUpgrade, unordered: false}, + // ], + // rawData: false, // rawData flag is redundant here as we stop at rename+invalidate events. + // openChangeStreamBeforeRunningScenario: false, + // makeCursorAndWatchCtx: ({cst, rawData}) => { + // const watchCtx = { + // watchMode: ChangeStreamWatchMode.kCollection, + // watchedNss: scenario.colls.ts1.bucketsNss, + // showSystemEvents: true, + // rawData, + // }; + // const cursor = openChangeStreamCursor(cst, scenario.colls.ts1.bucketsNss.coll, { + // showSystemEvents: true, + // allowToRunOnSystemNS: true, + // startAtOperationTime: initClusterTime, + // rawData, + // watchMode: ChangeStreamWatchMode.kCollection, + // }); + // return {cursor, watchCtx}; + // }, + // }); - // Ensure that the newly introduced oplog entry: 'upgradeDowngradeTimeseriesCollection' is not causing change streams in v8.0 to crash. - { - assert.commandWorked( - adminDB.runCommand({setFeatureCompatibilityVersion: lastLTSFCV, confirm: true}), - ); - downgrade(); + // // Ensure that the newly introduced oplog entry: 'upgradeDowngradeTimeseriesCollection' is not causing change streams in v8.0 to crash. + // { + // assert.commandWorked( + // adminDB.runCommand({setFeatureCompatibilityVersion: lastLTSFCV, confirm: true}), + // ); + // downgrade(); - runScenarioAndAssert({ - conn: getConn(), // new primary after downgrade - dbForCstName: testDB1Name, - // The commands have already been executed during the first scenario run and assertion. - scenario: {commands: []}, - // NOTE: we won't see rename/invalidate events because 'upgradeDowngradeTimeseriesCollection' is only handled in v9.0+. - // On older versions, this oplog entry will be ignored and we won't detect that the collection has been renamed. - phases: [{cmds: scenario.phases.preFCVUpgrade, unordered: false}], - openChangeStreamBeforeRunningScenario: false, - makeCursorAndWatchCtx: ({cst, rawData}) => { - const watchCtx = { - watchMode: ChangeStreamWatchMode.kCollection, - watchedNss: scenario.colls.ts1.bucketsNss, - showSystemEvents: true, - rawData, - }; - const cursor = openChangeStreamCursor(cst, scenario.colls.ts1.bucketsNss.coll, { - showSystemEvents: true, - allowToRunOnSystemNS: true, - startAtOperationTime: initClusterTime, - rawData, - watchMode: ChangeStreamWatchMode.kCollection, - }); - return {cursor, watchCtx}; - }, - }); + // runScenarioAndAssert({ + // conn: getConn(), // new primary after downgrade + // dbForCstName: testDB1Name, + // // The commands have already been executed during the first scenario run and assertion. + // scenario: {commands: []}, + // // NOTE: we won't see rename/invalidate events because 'upgradeDowngradeTimeseriesCollection' is only handled in v9.0+. + // // On older versions, this oplog entry will be ignored and we won't detect that the collection has been renamed. + // phases: [{cmds: scenario.phases.preFCVUpgrade, unordered: false}], + // openChangeStreamBeforeRunningScenario: false, + // makeCursorAndWatchCtx: ({cst, rawData}) => { + // const watchCtx = { + // watchMode: ChangeStreamWatchMode.kCollection, + // watchedNss: scenario.colls.ts1.bucketsNss, + // showSystemEvents: true, + // rawData, + // }; + // const cursor = openChangeStreamCursor(cst, scenario.colls.ts1.bucketsNss.coll, { + // showSystemEvents: true, + // allowToRunOnSystemNS: true, + // startAtOperationTime: initClusterTime, + // rawData, + // watchMode: ChangeStreamWatchMode.kCollection, + // }); + // return {cursor, watchCtx}; + // }, + // }); - upgrade(); - } - }); + // upgrade(); + // } + // }); for (const [rawData, openBeforeRunningCmds] of crossProduct([true, false], [true, false])) { // As part of this test, we open a change stream over a database that has timeseries collections diff --git a/jstests/multiVersion/genericSetFCVUsage/collection_write_path/fcv_upgrade_succeeds_with_buckets_without_timeseries_options.js b/jstests/multiVersion/genericSetFCVUsage/collection_write_path/fcv_upgrade_succeeds_with_buckets_without_timeseries_options.js index 285f3acfe80..c16c7b7082c 100644 --- a/jstests/multiVersion/genericSetFCVUsage/collection_write_path/fcv_upgrade_succeeds_with_buckets_without_timeseries_options.js +++ b/jstests/multiVersion/genericSetFCVUsage/collection_write_path/fcv_upgrade_succeeds_with_buckets_without_timeseries_options.js @@ -38,8 +38,10 @@ function testUpgradeFromFCV(conn, fromFCV) { // Upgrade succeeds even though we have an invalid bucket collection assert.commandWorked(db.adminCommand({setFeatureCompatibilityVersion: latestFCV, confirm: true})); - assert.eq(1, db[bucketCollName].countDocuments({})); - db[bucketCollName].drop(); + // verify collection still exists after upgrade + assert.eq(1, db.getCollectionInfos({name: bucketCollName}).length); + + db.dropDatabase(); } function testAllTopologies(fromFCV) { diff --git a/jstests/multiVersion/genericSetFCVUsage/fcv_upgrade_downgrade_viewless_timeseries_inconsistent_metadata.js b/jstests/multiVersion/genericSetFCVUsage/fcv_upgrade_downgrade_viewless_timeseries_inconsistent_metadata.js index 2b35d9ce751..a4a1e4d7949 100644 --- a/jstests/multiVersion/genericSetFCVUsage/fcv_upgrade_downgrade_viewless_timeseries_inconsistent_metadata.js +++ b/jstests/multiVersion/genericSetFCVUsage/fcv_upgrade_downgrade_viewless_timeseries_inconsistent_metadata.js @@ -84,7 +84,21 @@ assert.eq(true, isViewlessTimeseriesFormat(validCollName)); jsTest.log.info("Verifying inconsistent collection was skipped (still in legacy format)"); assert.eq(true, isLegacyTimeseriesFormat(inconsistentCollName)); -// Disable failpoint +jsTest.log.info("Verifying bypass parameter allowDirectSystemBucketsAccess works"); +const bucketCollName = "system.buckets." + inconsistentCollName; +const shardPrimary = st.rs0.getPrimary(); + +assert.commandFailedWithCode( + db.runCommand({count: bucketCollName}), + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, +); + +assert.commandWorked(shardPrimary.adminCommand({setParameter: 1, allowDirectSystemBucketsAccess: true})); +assert.commandWorked(db.runCommand({count: bucketCollName})); + +// cleanup +assert.commandWorked(shardPrimary.adminCommand({setParameter: 1, allowDirectSystemBucketsAccess: false})); failpoint.off(); +assert.commandWorked(db.dropDatabase()); st.stop(); diff --git a/jstests/multiVersion/genericSetFCVUsage/upgrade_downgrade_viewless_timeseries_preconditions.js b/jstests/multiVersion/genericSetFCVUsage/upgrade_downgrade_viewless_timeseries_preconditions.js index 4d7c7497669..8ffc694409f 100644 --- a/jstests/multiVersion/genericSetFCVUsage/upgrade_downgrade_viewless_timeseries_preconditions.js +++ b/jstests/multiVersion/genericSetFCVUsage/upgrade_downgrade_viewless_timeseries_preconditions.js @@ -108,7 +108,7 @@ function testUpgradeErrors() { upgradeDowngradeViewlessTimeseries: bucketsCollName, mode: "upgradeToViewless", }), - ErrorCodes.InvalidNamespace, + ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, ); assert(db.getCollection(tsCollName).drop()); } diff --git a/jstests/noPassthrough/timeseries/stats/timeseries_system_buckets_metrics.js b/jstests/noPassthrough/timeseries/stats/timeseries_system_buckets_metrics.js index 8e808bb6b77..ab9f7bb0c9e 100644 --- a/jstests/noPassthrough/timeseries/stats/timeseries_system_buckets_metrics.js +++ b/jstests/noPassthrough/timeseries/stats/timeseries_system_buckets_metrics.js @@ -2,6 +2,7 @@ * Validates metrics.numCommandsTargetingSystemBuckets when issuing commands that * target timeseries collections vs directly targeting system.buckets collections. * + * // TODO SERVER-121176: remove this test once 9.0 becomes last LTS. * @tags: [ * requires_timeseries, * ] @@ -77,10 +78,19 @@ function testDirectBucketTargeting({name, primaryConn, otherConns}) { assertSystemBucketsMetrics(primaryConn, otherConns, 0, `[after regular timeseries ops] (${name})`); // 2) Directly targeting system.buckets should be counted. + // When featureFlagBlockDirectSystemBucketsAccess is enabled, commands are rejected after the metric + // is incremented, so the count remains the same regardless of whether the commands succeed. coll = testDB.getCollection(getTimeseriesBucketsColl("coll")); - assert.commandWorked(testDB.createCollection(coll.getName(), {timeseries: {timeField, metaField}})); - assert.commandWorked(coll.insertOne(timeseriesRawDoc)); - assert(coll.drop()); + const kBlockedErrorCode = ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace; + assert.commandWorkedOrFailedWithCode( + testDB.createCollection(coll.getName(), {timeseries: {timeField, metaField}}), + kBlockedErrorCode, + ); + assert.commandWorkedOrFailedWithCode( + testDB.runCommand({insert: coll.getName(), documents: [timeseriesRawDoc]}), + kBlockedErrorCode, + ); + assert.commandWorkedOrFailedWithCode(testDB.runCommand({drop: coll.getName()}), kBlockedErrorCode); const EXPECTED_DIRECT_BUCKET_COMMANDS = 3; // createCollection + insert + drop assertSystemBucketsMetrics( diff --git a/jstests/sharding/timeseries/timeseries_shard_collection.js b/jstests/sharding/timeseries/timeseries_shard_collection.js index c99b0e27409..adef7665899 100644 --- a/jstests/sharding/timeseries/timeseries_shard_collection.js +++ b/jstests/sharding/timeseries/timeseries_shard_collection.js @@ -431,7 +431,7 @@ assert.commandFailedWithCode( shardCollection: `${dbName}.${getTimeseriesBucketsColl(collName)}`, key: {time: 1}, }), - 5731501, + [5731501, ErrorCodes.CommandNotSupportedOnLegacyTimeseriesBucketsNamespace], ); st.stop(); diff --git a/src/mongo/base/error_codes.yml b/src/mongo/base/error_codes.yml index 75d8593dd60..5801c73d26e 100644 --- a/src/mongo/base/error_codes.yml +++ b/src/mongo/base/error_codes.yml @@ -1188,5 +1188,7 @@ error_codes: # Error code to indicate that replanning the query is required - {code: 490, name: ReplanningRequired, extra: ReplanningRequiredInfo} + - {code: 491, name: CommandNotSupportedOnLegacyTimeseriesBucketsNamespace} + # ^^^^ # Add new codes, sequentially numbered, above. diff --git a/src/mongo/db/s/forwardable_operation_metadata.cpp b/src/mongo/db/s/forwardable_operation_metadata.cpp index d803b273818..bddc3696db7 100644 --- a/src/mongo/db/s/forwardable_operation_metadata.cpp +++ b/src/mongo/db/s/forwardable_operation_metadata.cpp @@ -90,7 +90,6 @@ ForwardableOperationMetadata::ForwardableOperationMetadata(OperationContext* opC setRawData(isRawDataOperation(opCtx)); - // TODO: SERVER-120237 Remove this once 9.0 becomes last LTS. setIsDirectSystemBucketsAccess(isDirectSystemBucketsAccess(opCtx)); if (auto telemetryCtx = @@ -124,7 +123,6 @@ void ForwardableOperationMetadata::setOn(OperationContext* opCtx) const { isRawDataOperation(opCtx) = getRawData(); - // TODO: SERVER-120237 Remove this once 9.0 becomes last LTS. isDirectSystemBucketsAccess(opCtx) = getIsDirectSystemBucketsAccess(); boost::optional validatedTenancyScope = boost::none; diff --git a/src/mongo/db/s/forwardable_operation_metadata.idl b/src/mongo/db/s/forwardable_operation_metadata.idl index fd0352dbfb2..32810f21316 100644 --- a/src/mongo/db/s/forwardable_operation_metadata.idl +++ b/src/mongo/db/s/forwardable_operation_metadata.idl @@ -65,7 +65,6 @@ structs: collection type that stores its data in a different format from that in which users interact with, this means that the command will operate directly on the format in which it is stored." - # TODO SERVER-120237 remove this once 9.0 becomes last LTS. isDirectSystemBucketsAccess: type: optionalBool description: diff --git a/src/mongo/db/server_feature_flags.idl b/src/mongo/db/server_feature_flags.idl index f3cc4b7d835..3339607b6b7 100644 --- a/src/mongo/db/server_feature_flags.idl +++ b/src/mongo/db/server_feature_flags.idl @@ -153,6 +153,13 @@ feature_flags: cpp_varname: gFeatureFlagViewlessTimeseriesUpgradeDowngradeRetriableError default: false fcv_gated: false + featureFlagBlockDirectSystemBucketsAccess: + description: >- + When enabled, external operations directly targeting legacy timeseries + system.buckets collections will fail with an error. + cpp_varname: gFeatureFlagBlockDirectSystemBucketsAccess + default: false + fcv_gated: true featureFlagCreateViewlessTimeseriesCollections: description: >- Enables the creation of viewless timeseries collections. @@ -280,3 +287,16 @@ feature_flags: cpp_varname: gFeatureFlagESEKeystoreV2 default: false fcv_gated: true + +server_parameters: + allowDirectSystemBucketsAccess: + description: >- + When set to true, bypasses the featureFlagBlockDirectSystemBucketsAccess check, + allowing direct operations on legacy timeseries system.buckets namespaces + even when the feature flag is enabled. This parameter is intended as an + emergency escape hatch. + set_at: [startup, runtime] + cpp_vartype: AtomicWord + cpp_varname: allowDirectSystemBucketsAccess + default: false + redact: false diff --git a/src/mongo/db/stats/BUILD.bazel b/src/mongo/db/stats/BUILD.bazel index 4cdc8108c5f..17395fbcefd 100644 --- a/src/mongo/db/stats/BUILD.bazel +++ b/src/mongo/db/stats/BUILD.bazel @@ -87,7 +87,6 @@ mongo_cc_library( ], ) -# TODO SERVER-120237: remove this once 9.0 becomes last LTS. mongo_cc_library( name = "direct_system_buckets_access", srcs = [ diff --git a/src/mongo/db/stats/direct_system_buckets_access.cpp b/src/mongo/db/stats/direct_system_buckets_access.cpp index f09d2be66f0..0f68c01a253 100644 --- a/src/mongo/db/stats/direct_system_buckets_access.cpp +++ b/src/mongo/db/stats/direct_system_buckets_access.cpp @@ -29,8 +29,6 @@ #include "mongo/db/stats/direct_system_buckets_access.h" -// TODO SERVER-120237 : Remove this file once 9.0 becomes last LTS. - namespace mongo { namespace { const auto directSystemBucketsAccess = OperationContext::declareDecoration(); diff --git a/src/mongo/db/stats/direct_system_buckets_access.h b/src/mongo/db/stats/direct_system_buckets_access.h index f97cf811eed..dd35339e6d3 100644 --- a/src/mongo/db/stats/direct_system_buckets_access.h +++ b/src/mongo/db/stats/direct_system_buckets_access.h @@ -46,7 +46,6 @@ constexpr inline auto kIsDirectSystemBucketsAccessFieldName = "isDirectSystemBuc * actual FCV, so it cannot reliably determine whether BlockDirectSystemBucketsAccess is enabled * for the current cluster version. * - * TODO SERVER-120237: Remove this file once 9.0 becomes last LTS. */ bool& isDirectSystemBucketsAccess(OperationContext*); diff --git a/src/mongo/db/stats/system_buckets_metrics.cpp b/src/mongo/db/stats/system_buckets_metrics.cpp index e02753bd92d..6de49ed3ed7 100644 --- a/src/mongo/db/stats/system_buckets_metrics.cpp +++ b/src/mongo/db/stats/system_buckets_metrics.cpp @@ -31,6 +31,10 @@ #include "mongo/db/stats/system_buckets_metrics.h" #include "mongo/db/commands/server_status/server_status_metric.h" +#include "mongo/db/server_feature_flags_gen.h" +#include "mongo/db/stats/direct_system_buckets_access.h" +#include "mongo/db/topology/cluster_role.h" +#include "mongo/db/version_context.h" #include "mongo/util/string_map.h" @@ -48,37 +52,75 @@ SystemBucketsMetricsCommandHooks::SystemBucketsMetricsCommandHooks() { _commandsExecuted = &*MetricBuilder("numCommandsTargetingSystemBuckets"); } +// TODO SERVER-121176: Remove system.buckets metrics once 9.0 becomes last LTS void SystemBucketsMetricsCommandHooks::onBeforeRun(OperationContext* opCtx, CommandInvocation* invocation) { - if (kCommandsAllowedToTargetBuckets.contains(invocation->definition()->getName())) { + // Only care about timeseries buckets namespaces + const auto& nss = invocation->ns(); + if (!nss.isTimeseriesBucketsCollection()) { + return; + } + + const auto& commandName = invocation->definition()->getName(); + if (kCommandsAllowedToTargetBuckets.contains(commandName)) { LOGV2_DEBUG(11923700, 2, "Skipping system buckets metrics counter for allowed command", - "command"_attr = invocation->definition()->getName(), - "namespace"_attr = invocation->ns().toStringForErrorMsg()); + "command"_attr = commandName, + "namespace"_attr = nss.toStringForErrorMsg()); return; } - if ( - // This command have been initiated by another command (e.g. DBDirectClient) - isProcessInternalClient(*(opCtx->getClient())) || + const bool isInternal = + // This command has been initiated by another command (e.g. DBDirectClient) + isProcessInternalClient(*opCtx->getClient()) || // This command comes from another node within the same cluster - opCtx->getClient()->isInternalClient() || - // This command does not target a system buckets collection - !invocation->ns().isTimeseriesBucketsCollection()) { + opCtx->getClient()->isInternalClient(); + + const bool isRouter = opCtx->getService()->role().hasExclusively(ClusterRole::RouterServer); + if (!isInternal) { + // Only count external commands + LOGV2_DEBUG(11259900, + _logSuppressor().toInt(), + "Received command targeting directly a system buckets namespace", + "command"_attr = commandName, + "isRouter"_attr = isRouter, + "namespace"_attr = nss.toStringForErrorMsg(), + "client"_attr = opCtx->getClient()->clientAddress(true), + "connId"_attr = opCtx->getClient()->getConnectionId()); + _commandsExecuted->increment(); + } + + if (isRouter) { + if (!isInternal) { + // Signal to the shard that a user directly targeted system.buckets through the + // router. The shard will check the feature flag and block if needed. + isDirectSystemBucketsAccess(opCtx) = true; + } return; } - LOGV2_DEBUG(11259900, - _logSuppressor().toInt(), - "Received command targeting directly a system buckets namespace", - "command"_attr = invocation->definition()->getName(), - "namespace"_attr = invocation->ns().toStringForErrorMsg(), - "client"_attr = opCtx->getClient()->clientAddress(true), - "connId"_attr = opCtx->getClient()->getConnectionId()); - - _commandsExecuted->increment(); + // On mongod, block if the user directly targeted system.buckets. This covers two cases: + // 1. User connected directly to the shard (or replica set): isInternal=false + // 2. User targeted system.buckets through the router: isDirectSystemBucketsAccess=true + // (isInternal=true because mongos connects as internal client, but the flag tells us + // the original request came from a user) + // TODO SERVER-121176: keep the logic that blocks system.buckets access when removing the + // metrics logic + if ((!isInternal || isDirectSystemBucketsAccess(opCtx)) && + !allowDirectSystemBucketsAccess.load() && + gFeatureFlagBlockDirectSystemBucketsAccess.isEnabled( + VersionContext::getDecoration(opCtx), + serverGlobalParams.featureCompatibility.acquireFCVSnapshot())) { + uasserted(ErrorCodes::CommandNotSupportedOnLegacyTimeseriesBucketsNamespace, + str::stream() + << "Command " << commandName << " on namespace " << nss.toStringForErrorMsg() + << " is not supported. Direct access to timeseries buckets namespaces is not" + " allowed anymore. Please target the main timeseries namespace instead." + " Use the rawData API to directly read timeseries buckets data in raw" + " format."); + } } } // namespace mongo diff --git a/src/mongo/idl/generic_argument.idl b/src/mongo/idl/generic_argument.idl index 86bdf664ca9..d080b8a14d5 100644 --- a/src/mongo/idl/generic_argument.idl +++ b/src/mongo/idl/generic_argument.idl @@ -396,7 +396,6 @@ structs: # This flag must be propagated to shards because mongos lacks FCV awareness, # it cannot reliably evaluate whether BlockDirectSystemBucketsAccess is enabled # for the current cluster version. Enforcement is therefore delegated to the shards. - # TODO SERVER-120237: remove this once 9.0 becomes last LTS. isDirectSystemBucketsAccess: type: optionalBool # Even though this field is forwarded to shards, forward_to_shards is set to diff --git a/src/mongo/rpc/metadata.cpp b/src/mongo/rpc/metadata.cpp index 5d8b8989949..72938d770af 100644 --- a/src/mongo/rpc/metadata.cpp +++ b/src/mongo/rpc/metadata.cpp @@ -165,7 +165,6 @@ void readRequestMetadata(OperationContext* opCtx, isRawDataOperation(opCtx) = true; } - // TODO: SERVER-120237 remove this once 9.0 becomes last LTS. if (requestArgs.getIsDirectSystemBucketsAccess()) { isDirectSystemBucketsAccess(opCtx) = true; } diff --git a/src/mongo/s/client_metadata_propagation_egress_hook.cpp b/src/mongo/s/client_metadata_propagation_egress_hook.cpp index 9ae5494830b..33200103e22 100644 --- a/src/mongo/s/client_metadata_propagation_egress_hook.cpp +++ b/src/mongo/s/client_metadata_propagation_egress_hook.cpp @@ -67,7 +67,6 @@ Status ClientMetadataPropagationEgressHook::writeRequestMetadata(OperationContex metadataBob->append(kRawDataFieldName, true); } - // TODO: SERVER-120237 remove this once 9.0 becomes last LTS. if (isDirectSystemBucketsAccess(opCtx)) { metadataBob->append(kIsDirectSystemBucketsAccessFieldName, true); }