From 4c96930ccb001d91fd188380f80756c19f1ad3c1 Mon Sep 17 00:00:00 2001 From: Gabriel Marks Date: Tue, 18 Jan 2022 15:12:06 +0000 Subject: SERVER-60817 Check write concern string for validity --- src/mongo/db/commands/rwc_defaults_commands.cpp | 8 ++++++++ src/mongo/db/repl/repl_set_config.cpp | 9 ++++++++- src/mongo/db/repl/repl_set_config.h | 6 ++++++ src/mongo/db/repl/replication_coordinator.h | 5 +++++ src/mongo/db/repl/replication_coordinator_impl.cpp | 6 ++++++ src/mongo/db/repl/replication_coordinator_impl.h | 2 ++ src/mongo/db/repl/replication_coordinator_mock.cpp | 6 ++++++ src/mongo/db/repl/replication_coordinator_mock.h | 2 ++ src/mongo/db/repl/replication_coordinator_noop.cpp | 5 +++++ src/mongo/db/repl/replication_coordinator_noop.h | 2 ++ src/mongo/db/write_concern.cpp | 5 ++--- src/mongo/db/write_concern_options.h | 4 ++++ src/mongo/embedded/replication_coordinator_embedded.cpp | 5 +++++ src/mongo/embedded/replication_coordinator_embedded.h | 2 ++ src/mongo/s/commands/cluster_rwc_defaults_commands.cpp | 11 +++++++++++ 15 files changed, 74 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/mongo/db/commands/rwc_defaults_commands.cpp b/src/mongo/db/commands/rwc_defaults_commands.cpp index bb8266929bc..f02cd506186 100644 --- a/src/mongo/db/commands/rwc_defaults_commands.cpp +++ b/src/mongo/db/commands/rwc_defaults_commands.cpp @@ -116,6 +116,14 @@ public: auto& rwcDefaults = ReadWriteConcernDefaults::get(opCtx->getServiceContext()); auto newDefaults = rwcDefaults.generateNewCWRWCToBeSavedOnDisk( opCtx, request().getDefaultReadConcern(), request().getDefaultWriteConcern()); + // We don't want to check if the custom write concern exists on the config servers + // because it only has to exist on the actual shards in order to be valid. + if (serverGlobalParams.clusterRole != ClusterRole::ConfigServer) { + if (auto optWC = newDefaults.getDefaultWriteConcern()) { + uassertStatusOK( + repl::ReplicationCoordinator::get(opCtx)->validateWriteConcern(*optWC)); + } + } updatePersistedDefaultRWConcernDocument(opCtx, newDefaults); LOGV2(20498, diff --git a/src/mongo/db/repl/repl_set_config.cpp b/src/mongo/db/repl/repl_set_config.cpp index a626634af15..ecae0048ee3 100644 --- a/src/mongo/db/repl/repl_set_config.cpp +++ b/src/mongo/db/repl/repl_set_config.cpp @@ -427,7 +427,7 @@ Status ReplSetConfig::_validate(bool allowSplitHorizonIP) const { Status ReplSetConfig::checkIfWriteConcernCanBeSatisfied( const WriteConcernOptions& writeConcern) const { - if (!writeConcern.wMode.empty() && writeConcern.wMode != WriteConcernOptions::kMajority) { + if (writeConcern.writeModeIsCustom()) { StatusWith tagPatternStatus = findCustomWriteMode(writeConcern.wMode); if (!tagPatternStatus.isOK()) { return tagPatternStatus.getStatus(); @@ -738,6 +738,13 @@ bool ReplSetConfig::containsCustomizedGetLastErrorDefaults() const { getLastErrorDefaults.syncMode == WriteConcernOptions::SyncMode::UNSET); } +Status ReplSetConfig::validateWriteConcern(const WriteConcernOptions& writeConcern) const { + if (writeConcern.writeModeIsCustom()) { + return findCustomWriteMode(writeConcern.wMode).getStatus(); + } + return Status::OK(); +} + bool ReplSetConfig::isSplitConfig() const { return !!_recipientConfig; } diff --git a/src/mongo/db/repl/repl_set_config.h b/src/mongo/db/repl/repl_set_config.h index 0de35a94b9c..0ff4711f14d 100644 --- a/src/mongo/db/repl/repl_set_config.h +++ b/src/mongo/db/repl/repl_set_config.h @@ -535,6 +535,12 @@ public: */ bool containsCustomizedGetLastErrorDefaults() const; + /** + * Returns Status::OK if write concern is valid for this config, or appropriate status + * otherwise. + */ + Status validateWriteConcern(const WriteConcernOptions& writeConcern) const; + /** * Returns true if this config is a split config, which is determined by checking if it contains * a recipient config for a shard split operation. diff --git a/src/mongo/db/repl/replication_coordinator.h b/src/mongo/db/repl/replication_coordinator.h index db983ff46ce..12dd736fdd0 100644 --- a/src/mongo/db/repl/replication_coordinator.h +++ b/src/mongo/db/repl/replication_coordinator.h @@ -672,6 +672,11 @@ public: */ virtual BSONObj getConfigBSON() const = 0; + /** + * Validates the given WriteConcernOptions on the current ReplSetConfig. + */ + virtual Status validateWriteConcern(const WriteConcernOptions& writeConcern) const = 0; + /** * Returns a pointer to the MemberConfig corresponding to the member with the given * HostAndPort in the current ReplSetConfig, or NULL if there is no member with that address. diff --git a/src/mongo/db/repl/replication_coordinator_impl.cpp b/src/mongo/db/repl/replication_coordinator_impl.cpp index cb64681b369..55b52fcf4c0 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.cpp +++ b/src/mongo/db/repl/replication_coordinator_impl.cpp @@ -3223,6 +3223,12 @@ Milliseconds ReplicationCoordinatorImpl::getConfigHeartbeatInterval() const { return _rsConfig.getHeartbeatInterval(); } +Status ReplicationCoordinatorImpl::validateWriteConcern( + const WriteConcernOptions& writeConcern) const { + stdx::lock_guard lock(_mutex); + return _rsConfig.validateWriteConcern(writeConcern); +} + WriteConcernOptions ReplicationCoordinatorImpl::_getOplogCommitmentWriteConcern(WithLock lk) { auto syncMode = getWriteConcernMajorityShouldJournal_inlock() ? WriteConcernOptions::SyncMode::JOURNAL diff --git a/src/mongo/db/repl/replication_coordinator_impl.h b/src/mongo/db/repl/replication_coordinator_impl.h index 73af8253f16..e2403064ea3 100644 --- a/src/mongo/db/repl/replication_coordinator_impl.h +++ b/src/mongo/db/repl/replication_coordinator_impl.h @@ -257,6 +257,8 @@ public: virtual Milliseconds getConfigHeartbeatInterval() const override; + virtual Status validateWriteConcern(const WriteConcernOptions& writeConcern) const override; + virtual void processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus = false, bool includeNewlyAdded = false) override; diff --git a/src/mongo/db/repl/replication_coordinator_mock.cpp b/src/mongo/db/repl/replication_coordinator_mock.cpp index 9a01fdf5c88..f88130cf440 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.cpp +++ b/src/mongo/db/repl/replication_coordinator_mock.cpp @@ -418,6 +418,12 @@ BSONObj ReplicationCoordinatorMock::getConfigBSON() const { return _getConfigReturnValue.toBSON(); } +Status ReplicationCoordinatorMock::validateWriteConcern( + const WriteConcernOptions& writeConcern) const { + stdx::lock_guard lock(_mutex); + return _getConfigReturnValue.validateWriteConcern(writeConcern); +} + const MemberConfig* ReplicationCoordinatorMock::findConfigMemberByHostAndPort( const HostAndPort& hap) const { stdx::lock_guard lock(_mutex); diff --git a/src/mongo/db/repl/replication_coordinator_mock.h b/src/mongo/db/repl/replication_coordinator_mock.h index cb17793befe..d927664194a 100644 --- a/src/mongo/db/repl/replication_coordinator_mock.h +++ b/src/mongo/db/repl/replication_coordinator_mock.h @@ -220,6 +220,8 @@ public: virtual const MemberConfig* findConfigMemberByHostAndPort( const HostAndPort& hap) const override; + virtual Status validateWriteConcern(const WriteConcernOptions& writeConcern) const override; + virtual bool isConfigLocalHostAllowed() const override; virtual Milliseconds getConfigHeartbeatInterval() const override; diff --git a/src/mongo/db/repl/replication_coordinator_noop.cpp b/src/mongo/db/repl/replication_coordinator_noop.cpp index 38639d12dab..e2d0994e89d 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.cpp +++ b/src/mongo/db/repl/replication_coordinator_noop.cpp @@ -346,6 +346,11 @@ Milliseconds ReplicationCoordinatorNoOp::getConfigHeartbeatInterval() const { MONGO_UNREACHABLE; } +Status ReplicationCoordinatorNoOp::validateWriteConcern( + const WriteConcernOptions& writeConcern) const { + MONGO_UNREACHABLE; +} + void ReplicationCoordinatorNoOp::processReplSetGetConfig(BSONObjBuilder* result, bool commitmentStatus, bool includeNewlyAdded) { diff --git a/src/mongo/db/repl/replication_coordinator_noop.h b/src/mongo/db/repl/replication_coordinator_noop.h index 6764deafacd..c355ce73e37 100644 --- a/src/mongo/db/repl/replication_coordinator_noop.h +++ b/src/mongo/db/repl/replication_coordinator_noop.h @@ -197,6 +197,8 @@ public: Milliseconds getConfigHeartbeatInterval() const final; + Status validateWriteConcern(const WriteConcernOptions& writeConcern) const final; + void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false, bool includeNewlyAdded = false) final; diff --git a/src/mongo/db/write_concern.cpp b/src/mongo/db/write_concern.cpp index 2cdd02e6913..f574d63ef72 100644 --- a/src/mongo/db/write_concern.cpp +++ b/src/mongo/db/write_concern.cpp @@ -189,10 +189,9 @@ Status validateWriteConcern(OperationContext* opCtx, const WriteConcernOptions& return Status(ErrorCodes::BadValue, "cannot use 'w' > 1 when a host is not replicated"); } - if (replMode != repl::ReplicationCoordinator::modeReplSet && !writeConcern.wMode.empty() && - writeConcern.wMode != WriteConcernOptions::kMajority) { + if (replMode != repl::ReplicationCoordinator::modeReplSet && writeConcern.writeModeIsCustom()) { return Status(ErrorCodes::BadValue, - string("cannot use non-majority 'w' mode ") + writeConcern.wMode + + string("cannot use custom 'w' mode ") + writeConcern.wMode + " when a host is not a member of a replica set"); } diff --git a/src/mongo/db/write_concern_options.h b/src/mongo/db/write_concern_options.h index 2eb0468b804..039a2c3a4a2 100644 --- a/src/mongo/db/write_concern_options.h +++ b/src/mongo/db/write_concern_options.h @@ -130,6 +130,10 @@ public: return usedDefaultConstructedWC || _provenance.isImplicitDefault(); } + bool writeModeIsCustom() const { + return !wMode.empty() && wMode != WriteConcernOptions::kMajority; + } + SyncMode syncMode; // The w parameter for this write concern. The wMode represents the string format and diff --git a/src/mongo/embedded/replication_coordinator_embedded.cpp b/src/mongo/embedded/replication_coordinator_embedded.cpp index 784de9edccf..b178e9d1972 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.cpp +++ b/src/mongo/embedded/replication_coordinator_embedded.cpp @@ -372,6 +372,11 @@ Milliseconds ReplicationCoordinatorEmbedded::getConfigHeartbeatInterval() const UASSERT_NOT_IMPLEMENTED; } +Status ReplicationCoordinatorEmbedded::validateWriteConcern( + const WriteConcernOptions& writeConcern) const { + UASSERT_NOT_IMPLEMENTED; +} + void ReplicationCoordinatorEmbedded::processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus, bool includeNewlyAdded) { diff --git a/src/mongo/embedded/replication_coordinator_embedded.h b/src/mongo/embedded/replication_coordinator_embedded.h index b41f8430096..9a6bad99f28 100644 --- a/src/mongo/embedded/replication_coordinator_embedded.h +++ b/src/mongo/embedded/replication_coordinator_embedded.h @@ -206,6 +206,8 @@ public: Milliseconds getConfigHeartbeatInterval() const override; + Status validateWriteConcern(const WriteConcernOptions& writeConcern) const override; + void processReplSetGetConfig(BSONObjBuilder*, bool commitmentStatus = false, bool includeNewlyAdded = false) override; diff --git a/src/mongo/s/commands/cluster_rwc_defaults_commands.cpp b/src/mongo/s/commands/cluster_rwc_defaults_commands.cpp index abb36e4ab89..6bf1755941d 100644 --- a/src/mongo/s/commands/cluster_rwc_defaults_commands.cpp +++ b/src/mongo/s/commands/cluster_rwc_defaults_commands.cpp @@ -36,6 +36,7 @@ #include "mongo/db/commands/rwc_defaults_commands_gen.h" #include "mongo/db/namespace_string.h" #include "mongo/db/read_write_concern_defaults.h" +#include "mongo/logv2/log.h" #include "mongo/s/cluster_commands_helpers.h" #include "mongo/s/grid.h" @@ -70,6 +71,16 @@ public: // Quickly pick up the new defaults by setting them in the cache. auto newDefaults = RWConcernDefault::parse( IDLParserErrorContext("ClusterSetDefaultRWConcern"), cmdResponse.response); + if (auto optWC = newDefaults.getDefaultWriteConcern()) { + if (optWC->writeModeIsCustom()) { + LOGV2_WARNING( + 6081700, + "A custom write concern is being set as the default write concern in a sharded " + "cluster. This set is unchecked, but if the custom write concern does not " + "exist on all shards in the cluster, errors will occur upon writes", + "customWriteConcern"_attr = optWC->wMode); + } + } ReadWriteConcernDefaults::get(opCtx).setDefault(opCtx, std::move(newDefaults)); CommandHelpers::filterCommandReplyForPassthrough(cmdResponse.response, &result); -- cgit v1.2.1