Skip to content

Commit

Permalink
SERVER-72083 Pass in tenant information when parsing ns in struct ite…
Browse files Browse the repository at this point in the history
…ms of IDL command
  • Loading branch information
sophiatll authored and Evergreen Agent committed Jan 15, 2023
1 parent 08744cf commit f5444a2
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 22 deletions.
23 changes: 12 additions & 11 deletions buildscripts/idl/idl/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,8 @@ def _gen_field_deserializer_expression(self, element_name, field, ast_type, tena
"""

if ast_type.is_struct:
self._writer.write_line(
'IDLParserContext tempContext(%s, &ctxt);' % (_get_field_constant_name(field)))
self._writer.write_line('IDLParserContext tempContext(%s, &ctxt, %s);' %
(_get_field_constant_name(field), tenant))
self._writer.write_line('const auto localObject = %s.Obj();' % (element_name))
return '%s::parse(tempContext, localObject)' % (ast_type.cpp_type, )
elif ast_type.deserializer and 'BSONElement::' in ast_type.deserializer:
Expand All @@ -1252,8 +1252,8 @@ def _gen_field_deserializer_expression(self, element_name, field, ast_type, tena

# For fields which are enums, pass a IDLParserContext
if ast_type.is_enum:
self._writer.write_line('IDLParserContext tempContext(%s, &ctxt);' %
(_get_field_constant_name(field)))
self._writer.write_line('IDLParserContext tempContext(%s, &ctxt, %s);' %
(_get_field_constant_name(field), tenant))
return common.template_args("${method_name}(tempContext, ${expression})",
method_name=method_name, expression=expression)

Expand Down Expand Up @@ -1286,8 +1286,8 @@ def _gen_array_deserializer(self, field, bson_element, ast_type, tenant):
cpp_type = cpp_type_info.get_type_name()

self._writer.write_line('std::uint32_t expectedFieldNumber{0};')
self._writer.write_line(
'const IDLParserContext arrayCtxt(%s, &ctxt);' % (_get_field_constant_name(field)))
self._writer.write_line('const IDLParserContext arrayCtxt(%s, &ctxt, %s);' %
(_get_field_constant_name(field), tenant))
self._writer.write_line('std::vector<%s> values;' % (cpp_type))
self._writer.write_empty_line()

Expand Down Expand Up @@ -1505,8 +1505,8 @@ def validate_and_assign_or_uassert(field, expression):
self._writer.write_line(
'ctxt.throwMissingField(%s);' % (_get_field_constant_name(field)))

def gen_doc_sequence_deserializer(self, field):
# type: (ast.Field) -> None
def gen_doc_sequence_deserializer(self, field, tenant):
# type: (ast.Field, str) -> None
"""Generate the C++ deserializer piece for a C++ mongo::OpMsg::DocumentSequence."""
cpp_type_info = cpp_types.get_cpp_type(field)
cpp_type = cpp_type_info.get_type_name()
Expand All @@ -1522,8 +1522,8 @@ def gen_doc_sequence_deserializer(self, field):

# Either we are deserializing BSON Objects or IDL structs
if field.type.is_struct:
self._writer.write_line(
'IDLParserContext tempContext(%s, &ctxt);' % (_get_field_constant_name(field)))
self._writer.write_line('IDLParserContext tempContext(%s, &ctxt, %s);' %
(_get_field_constant_name(field), tenant))
array_value = '%s::parse(tempContext, sequenceObject)' % (field.type.cpp_type, )
else:
assert field.type.bson_serialization_type == ['object']
Expand Down Expand Up @@ -1920,7 +1920,8 @@ def gen_op_msg_request_deserializer_methods(self, struct):
self._writer.write_line(
'%s = true;' % (_get_has_field_member_name(field)))

self.gen_doc_sequence_deserializer(field)
self.gen_doc_sequence_deserializer(field,
"request.getValidatedTenantId()")

if first_field:
first_field = False
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,6 @@ selector:
- jstests/core/txns/write_conflicts_with_non_txns.js
# TODO SERVER-72291: rolesInfo commmand is failed when querying relative roles by the tenant.
- jstests/core/**/roles_info.js
# TODO SERVER-72083: tenant information of bulkWrite command cannot be parsed correctly.
- jstests/core/**/bulk_write.js
# TODO SERVER-72357: cannot get the expected error due to an authorization contract issue.
- jstests/core/txns/multi_statement_transaction_command_args.js

Expand Down
2 changes: 1 addition & 1 deletion src/mongo/idl/idl_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ void IDLParserContext::appendGenericCommandArguments(const BSONObj& commandPasst
}

const boost::optional<TenantId>& IDLParserContext::getTenantId() const {
if (_predecessor == nullptr)
if (_tenantId || _predecessor == nullptr)
return _tenantId;

return _predecessor->getTenantId();
Expand Down
22 changes: 21 additions & 1 deletion src/mongo/idl/idl_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,14 @@ class IDLParserContext {
_predecessor(nullptr) {}

IDLParserContext(StringData fieldName, const IDLParserContext* predecessor)
: _currentField(fieldName), _predecessor(predecessor) {}
: IDLParserContext(fieldName, predecessor, boost::none) {}

IDLParserContext(StringData fieldName,
const IDLParserContext* predecessor,
boost::optional<TenantId> tenantId)
: _currentField(fieldName), _tenantId(tenantId), _predecessor(predecessor) {
assertTenantIdMatchesPredecessor(predecessor);
}

/**
* Check that BSON element is a given type or whether the field should be skipped.
Expand Down Expand Up @@ -390,6 +397,19 @@ class IDLParserContext {
*/
bool checkAndAssertBinDataTypeSlowPath(const BSONElement& element, BinDataType type) const;

void assertTenantIdMatchesPredecessor(const IDLParserContext* predecessor) {
if (!_tenantId || predecessor == nullptr) {
return;
}

auto& parentTenantId = predecessor->getTenantId();
iassert(8423379,
str::stream() << "The IDLParserContext tenantId " << _tenantId->toString()
<< " must match the predecessor's tenantId "
<< parentTenantId->toString(),
!parentTenantId || parentTenantId == _tenantId);
}

private:
// Name of the current field that is being parsed.
const StringData _currentField;
Expand Down
86 changes: 79 additions & 7 deletions src/mongo/idl/idl_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4059,13 +4059,14 @@ TEST(IDLTypeCommand, TestCommandWithNamespaceMember_WithTenant) {
RAIIServerParameterControllerForTest multitenanyController("multitenancySupport", true);
RAIIServerParameterControllerForTest featureFlagController("featureFlagRequireTenantID", true);
IDLParserContext ctxt("root");
const char* ns1 = "db.coll1";
const char* ns2 = "a.b";
const char* ns3 = "c.d";

auto testDoc = BSONObjBuilder{}
.append("CommandWithNamespaceMember", 1)
.append("field1", "db.coll1")
.append("field2",
BSON_ARRAY("a.b"
<< "c.d"))
.append("field1", ns1)
.append("field2", BSON_ARRAY(ns2 << ns3))
.append("$db", "admin")
.obj();

Expand All @@ -4076,15 +4077,86 @@ TEST(IDLTypeCommand, TestCommandWithNamespaceMember_WithTenant) {
assert_same_types<decltype(testStruct.getField2()),
const std::vector<mongo::NamespaceString>&>();

ASSERT_EQUALS(testStruct.getField1(), NamespaceString(tenantId, "db.coll1"));
std::vector<NamespaceString> field2{NamespaceString(tenantId, "a.b"),
NamespaceString(tenantId, "c.d")};
ASSERT_EQUALS(testStruct.getField1(), NamespaceString(tenantId, ns1));
std::vector<NamespaceString> field2{NamespaceString(tenantId, ns2),
NamespaceString(tenantId, ns3)};
ASSERT_TRUE(field2 == testStruct.getField2());

// Positive: Test we can roundtrip from the just parsed document
ASSERT_BSONOBJ_EQ(testDoc, serializeCmd(testStruct));
}

TEST(IDLTypeCommand, TestCommandWithNamespaceStruct_WithTenant) {
RAIIServerParameterControllerForTest multitenanyController("multitenancySupport", true);
RAIIServerParameterControllerForTest featureFlagController("featureFlagRequireTenantID", true);
IDLParserContext ctxt("root");
const char* ns1 = "db.coll1";
const char* ns2 = "a.b";
const char* ns3 = "c.d";

auto nsInfoStructBSON = [&](const char* ns) {
BSONObjBuilder builder;
builder.append("ns", ns);
return builder.obj();
};
auto testDoc = BSONObjBuilder{}
.append("CommandWithNamespaceStruct", 1)
.append("field1", nsInfoStructBSON(ns1))
.append("$db", "admin")
.append("field2", BSON_ARRAY(nsInfoStructBSON(ns2) << nsInfoStructBSON(ns3)))
.obj();

const auto tenantId = TenantId(OID::gen());
auto testStruct = CommandWithNamespaceStruct::parse(ctxt, makeOMRWithTenant(testDoc, tenantId));

assert_same_types<decltype(testStruct.getField1()), NamespaceInfoStruct&>();
assert_same_types<decltype(testStruct.getField2()), std::vector<NamespaceInfoStruct>&>();

ASSERT_EQUALS(testStruct.getField1().getNs(), NamespaceString(tenantId, ns1));
std::vector<NamespaceString> field2Nss{NamespaceString(tenantId, ns2),
NamespaceString(tenantId, ns3)};
std::vector<NamespaceInfoStruct>& field2 = testStruct.getField2();
ASSERT_TRUE(field2Nss[0] == field2[0].getNs());
ASSERT_TRUE(field2Nss[1] == field2[1].getNs());

// Positive: Test we can round trip to a document sequence from the just parsed document
{
OpMsgRequest loopbackRequest = testStruct.serialize(BSONObj());
OpMsgRequest request = makeOMR(testDoc);

assertOpMsgEquals(request, loopbackRequest);
ASSERT_EQUALS(loopbackRequest.sequences.size(), 1UL);
ASSERT_EQUALS(loopbackRequest.sequences[0].objs.size(), 2UL);
}
}

TEST(IDLParserContext, TestConstructorWithPredecessorAndDifferentTenant) {
// Negative: Test the child IDLParserContext cannot has different tenant id from its
// predecessor.
RAIIServerParameterControllerForTest multitenanyController("multitenancySupport", true);

const auto tenantId = TenantId(OID::gen());
const auto otherTenantId = TenantId(OID::gen());
IDLParserContext ctxt("root", false, tenantId);

auto nsInfoStructBSON = [&](const char* ns) {
BSONObjBuilder builder;
builder.append("ns", ns);
return builder.obj();
};
auto testDoc =
BSONObjBuilder{}
.append("CommandWithNamespaceStruct", 1)
.append("field1", nsInfoStructBSON("db.coll1"))
.append("$db", "admin")
.append("field2", BSON_ARRAY(nsInfoStructBSON("a.b") << nsInfoStructBSON("c.d")))
.obj();
ASSERT_THROWS_CODE(
CommandWithNamespaceStruct::parse(ctxt, makeOMRWithTenant(testDoc, otherTenantId)),
DBException,
8423379);
}

void verifyContract(const AuthorizationContract& left, const AuthorizationContract& right) {
ASSERT_TRUE(left.contains(right));
ASSERT_TRUE(right.contains(left));
Expand Down
24 changes: 24 additions & 0 deletions src/mongo/idl/unittest.idl
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,18 @@ structs:
always_serialize: false
optional: true

##################################################################################################
#
# Test struct with namespace string field
#
##################################################################################################
NamespaceInfoStruct:
description: "Test struct with namespace string field."
strict: true
fields:
ns:
type: namespacestring

##################################################################################################
#
# Test array of simple types
Expand Down Expand Up @@ -1171,6 +1183,18 @@ commands:
field2:
type: array<namespacestring>

CommandWithNamespaceStruct:
description: UnitTest for a command whose struct field has namespace string data
command_name: CommandWithNamespaceStruct
namespace: ignored
api_version: ""
fields:
field1:
type: NamespaceInfoStruct
field2:
type: array<NamespaceInfoStruct>
supports_doc_sequence: true

AccessCheckNone:
description: A Stable API command with access_check and none
command_name: AccessCheckNoneCommandName
Expand Down

0 comments on commit f5444a2

Please sign in to comment.