summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHuayu Ouyang <huayu.ouyang@mongodb.com>2021-03-15 23:29:45 +0000
committerEvergreen Agent <no-reply@evergreen.mongodb.com>2021-03-18 23:34:01 +0000
commit37bc14bbf9e1241bf77684d3ab13ae36929a386b (patch)
tree0fe52d29ec840403a1bcaae1a301e1e277df55d7
parent138acafd27f145622506ef422a48c056fb4883df (diff)
downloadmongo-37bc14bbf9e1241bf77684d3ab13ae36929a386b.tar.gz
SERVER-54532 Extend buildscripts/idl/idl_check_compatibility.py to check for additions and changes in complex
-rw-r--r--buildscripts/idl/idl/syntax.py10
-rw-r--r--buildscripts/idl/idl_check_compatibility.py95
-rw-r--r--buildscripts/idl/idl_compatibility_errors.py48
-rw-r--r--buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl157
-rw-r--r--buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl154
-rw-r--r--buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl75
-rw-r--r--buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl78
-rw-r--r--buildscripts/idl/tests/test_compatibility.py65
8 files changed, 655 insertions, 27 deletions
diff --git a/buildscripts/idl/idl/syntax.py b/buildscripts/idl/idl/syntax.py
index aa0985fc193..2e0277751d7 100644
--- a/buildscripts/idl/idl/syntax.py
+++ b/buildscripts/idl/idl/syntax.py
@@ -566,6 +566,16 @@ class AccessChecks(common.SourceLocation):
super(AccessChecks, self).__init__(file_name, line, column)
+ def get_access_check_type(self) -> str:
+ """Get type of AccessChecks."""
+ if self.none:
+ return "none"
+ if self.simple:
+ return "simple"
+ if self.complex:
+ return "complex"
+ return "undefined"
+
class Command(Struct):
"""
diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py
index f071e7a839e..672bd45762b 100644
--- a/buildscripts/idl/idl_check_compatibility.py
+++ b/buildscripts/idl/idl_check_compatibility.py
@@ -711,28 +711,79 @@ def check_error_reply(old_basic_types_path: str, new_basic_types_path: str,
return ctxt.errors
-def check_security_access_check(
- ctxt: IDLCompatibilityContext, old_access_checks: syntax.AccessChecks,
- new_access_checks: syntax.AccessChecks, cmd_name: str, new_idl_file_path: str) -> None:
+def split_complex_checks(
+ complex_checks: List[syntax.AccessCheck]) -> Tuple[List[str], List[syntax.Privilege]]:
+ """Split a list of AccessCheck into checks and privileges."""
+ checks = [x.check for x in complex_checks if x.check is not None]
+ privileges = [x.privilege for x in complex_checks if x.privilege is not None]
+ # Sort the list of privileges by the length of the action_type list, in decreasing order
+ # so that two lists of privileges can be compared later.
+ return checks, sorted(privileges, key=lambda x: len(x.action_type), reverse=True)
+
+
+def check_security_access_checks(ctxt: IDLCompatibilityContext,
+ old_access_checks: syntax.AccessChecks,
+ new_access_checks: syntax.AccessChecks, cmd: syntax.Command,
+ new_idl_file_path: str) -> None:
"""Check the compatibility between security access checks of the old and new command."""
+ # pylint:disable=too-many-locals,too-many-branches,too-many-nested-blocks
+ cmd_name = cmd.command_name
if old_access_checks is not None and new_access_checks is not None:
- old_simple_check = old_access_checks.simple
- new_simple_check = new_access_checks.simple
- if old_simple_check is not None and new_simple_check is not None:
- if old_simple_check.check != new_simple_check.check:
- ctxt.add_check_not_equal_error(cmd_name, old_simple_check.check,
- new_simple_check.check, new_idl_file_path)
-
- else:
- old_privilege = old_simple_check.privilege
- new_privilege = new_simple_check.privilege
- if old_privilege is not None and new_privilege is not None:
- if old_privilege.resource_pattern != new_privilege.resource_pattern:
- ctxt.add_resource_pattern_not_equal_error(
- cmd_name, old_privilege.resource_pattern,
- new_privilege.resource_pattern, new_idl_file_path)
- if not set(new_privilege.action_type).issubset(old_privilege.action_type):
- ctxt.add_new_action_types_not_subset_error(cmd_name, new_idl_file_path)
+ old_access_check_type = old_access_checks.get_access_check_type()
+ new_access_check_type = new_access_checks.get_access_check_type()
+ if old_access_check_type != new_access_check_type:
+ ctxt.add_access_check_type_not_equal_error(cmd_name, old_access_check_type,
+ new_access_check_type, new_idl_file_path)
+ else:
+ old_simple_check = old_access_checks.simple
+ new_simple_check = new_access_checks.simple
+ if old_simple_check is not None and new_simple_check is not None:
+ if old_simple_check.check != new_simple_check.check:
+ ctxt.add_check_not_equal_error(cmd_name, old_simple_check.check,
+ new_simple_check.check, new_idl_file_path)
+ else:
+ old_privilege = old_simple_check.privilege
+ new_privilege = new_simple_check.privilege
+ if old_privilege is not None and new_privilege is not None:
+ if old_privilege.resource_pattern != new_privilege.resource_pattern:
+ ctxt.add_resource_pattern_not_equal_error(
+ cmd_name, old_privilege.resource_pattern,
+ new_privilege.resource_pattern, new_idl_file_path)
+ if not set(new_privilege.action_type).issubset(old_privilege.action_type):
+ ctxt.add_new_action_types_not_subset_error(cmd_name, new_idl_file_path)
+
+ old_complex_checks = old_access_checks.complex
+ new_complex_checks = new_access_checks.complex
+ if old_complex_checks is not None and new_complex_checks is not None:
+ if len(new_complex_checks) > len(old_complex_checks):
+ ctxt.add_new_additional_complex_access_check_error(cmd_name, new_idl_file_path)
+ else:
+ old_checks, old_privileges = split_complex_checks(old_complex_checks)
+ new_checks, new_privileges = split_complex_checks(new_complex_checks)
+ if not set(new_checks).issubset(old_checks):
+ ctxt.add_new_complex_checks_not_subset_error(cmd_name, new_idl_file_path)
+
+ if len(new_privileges) > len(old_privileges):
+ ctxt.add_new_complex_privileges_not_subset_error(
+ cmd_name, new_idl_file_path)
+ else:
+ # Check that each new_privilege matches an old_privilege (the resource_pattern is
+ # equal and the action_types are a subset of the old action_types).
+ for new_privilege in new_privileges:
+ for old_privilege in old_privileges:
+ if (new_privilege.resource_pattern == old_privilege.resource_pattern
+ and set(new_privilege.action_type).issubset(
+ old_privilege.action_type)):
+ old_privileges.remove(old_privilege)
+ break
+ else:
+ ctxt.add_new_complex_privileges_not_subset_error(
+ cmd_name, new_idl_file_path)
+
+ elif new_access_checks is None and old_access_checks is not None:
+ ctxt.add_removed_access_check_field_error(cmd_name, new_idl_file_path)
+ elif old_access_checks is None and new_access_checks is not None and cmd.api_version == '1':
+ ctxt.add_added_access_check_field_error(cmd_name, new_idl_file_path)
def check_compatibility(old_idl_dir: str, new_idl_dir: str,
@@ -803,8 +854,8 @@ def check_compatibility(old_idl_dir: str, new_idl_dir: str,
old_idl_file, new_idl_file, old_idl_file_path,
new_idl_file_path)
- check_security_access_check(ctxt, old_cmd.access_check, new_cmd.access_check,
- old_cmd.command_name, new_idl_file_path)
+ check_security_access_checks(ctxt, old_cmd.access_check, new_cmd.access_check,
+ old_cmd, new_idl_file_path)
# TODO (SERVER-55203): Remove error_skipped logic.
ctxt.errors.remove_skipped_errors_and_dump_all_errors("Commands", old_idl_dir, new_idl_dir)
diff --git a/buildscripts/idl/idl_compatibility_errors.py b/buildscripts/idl/idl_compatibility_errors.py
index 47f157f4f7c..4b1b9e79059 100644
--- a/buildscripts/idl/idl_compatibility_errors.py
+++ b/buildscripts/idl/idl_compatibility_errors.py
@@ -102,6 +102,12 @@ ERROR_ID_CHECK_NOT_EQUAL = "ID0058"
ERROR_ID_RESOURCE_PATTERN_NOT_EQUAL = "ID0059"
ERROR_ID_NEW_ACTION_TYPES_NOT_SUBSET = "ID0060"
ERROR_ID_TYPE_NOT_ARRAY = "ID0061"
+ERROR_ID_ACCESS_CHECK_TYPE_NOT_EQUAL = "ID0062"
+ERROR_ID_NEW_COMPLEX_CHECKS_NOT_SUBSET = "ID0063"
+ERROR_ID_NEW_COMPLEX_PRIVILEGES_NOT_SUBSET = "ID0064"
+ERROR_ID_NEW_ADDITIONAL_COMPLEX_ACCESS_CHECK = "ID0065"
+ERROR_ID_REMOVED_ACCESS_CHECK_FIELD = "ID0066"
+ERROR_ID_ADDED_ACCESS_CHECK_FIELD = "ID0067"
# TODO (SERVER-55203): Remove SKIPPED_COMMANDS logic.
# Any breaking changes added to API V1 before releasing 5.0 should be added to SKIPPED_COMMANDS to
@@ -870,7 +876,7 @@ class IDLCompatibilityContext(object):
) % (command_name, new_resource_pattern, old_resource_pattern), file)
def add_new_action_types_not_subset_error(self, command_name: str, file: str) -> None:
- """Add an error about the command access_check check not being equal."""
+ """Add an error about the new access_check action types not being a subset of the old ones."""
self._add_error(ERROR_ID_NEW_ACTION_TYPES_NOT_SUBSET, command_name,
("'%s' has new action types that are not a subset of the old action types")
% (command_name), file)
@@ -889,6 +895,46 @@ class IDLCompatibilityContext(object):
"The command '%s' has %s: '%s' with new type '%s' while the older type was '%s'." %
(command_name, symbol, symbol_name, new_type, old_type), file)
+ def add_access_check_type_not_equal_error(self, command_name: str, old_type: str, new_type: str,
+ file: str) -> None:
+ """Add an error about the command access_check types not being equal."""
+ self._add_error(
+ ERROR_ID_ACCESS_CHECK_TYPE_NOT_EQUAL, command_name,
+ ("'%s' has access_check of type %s in the old version and access_check of type '%s'"
+ "in the new version.") % (command_name, old_type, new_type), file)
+
+ def add_new_complex_checks_not_subset_error(self, command_name: str, file: str) -> None:
+ """Add an error about the complex access_check checks not being a subset of the old ones."""
+ self._add_error(
+ ERROR_ID_NEW_COMPLEX_CHECKS_NOT_SUBSET, command_name,
+ ("'%s' has new complex access_checks checks that are not a subset of the old complex"
+ " access_check checks.") % (command_name), file)
+
+ def add_new_complex_privileges_not_subset_error(self, command_name: str, file: str) -> None:
+ """Add an error about the complex access_check privileges not being a subset of the old ones."""
+ self._add_error(
+ ERROR_ID_NEW_COMPLEX_PRIVILEGES_NOT_SUBSET, command_name,
+ ("'%s' has complex access_checks privileges that have changed the resource_pattern"
+ " or changed/added an action_type in the new command.") % (command_name), file)
+
+ def add_new_additional_complex_access_check_error(self, command_name: str, file: str) -> None:
+ """Add an error about an additional complex access_check being added."""
+ self._add_error(ERROR_ID_NEW_ADDITIONAL_COMPLEX_ACCESS_CHECK, command_name, (
+ "'%s' has additional complex access_checks in the new command that are not in the old command"
+ ) % (command_name), file)
+
+ def add_removed_access_check_field_error(self, command_name: str, file: str) -> None:
+ """Add an error the new command removing the access_check field."""
+ self._add_error(ERROR_ID_REMOVED_ACCESS_CHECK_FIELD, command_name, (
+ "'%s' has removed the access_check field in the new command when it exists in the old command"
+ ) % (command_name), file)
+
+ def add_added_access_check_field_error(self, command_name: str, file: str) -> None:
+ """Add an error the new command adding the access_check field when the api_version is '1'."""
+ self._add_error(ERROR_ID_ADDED_ACCESS_CHECK_FIELD, command_name, (
+ "'%s' has added the access_check field in the new command when it did not exist in the "
+ "old command and the api_version is '1'") % (command_name), file)
+
def _assert_unique_error_messages() -> None:
"""Assert that error codes are unique."""
diff --git a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
index 8b5e4d2e6bf..d4ef773c95b 100644
--- a/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/new/compatibility_test_fail_new.idl
@@ -1827,4 +1827,159 @@ commands:
cpp_name: InvalidReplySkippedCommand
strict: true
api_version: "1"
- reply_type: OkReply \ No newline at end of file
+ reply_type: OkReply
+
+ accessCheckTypeChange:
+ description: "new command fails because the access check type has changed"
+ command_name: accessCheckTypeChange
+ namespace: ignored
+ cpp_name: accessCheckTypeChange
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkThree
+ - check: checkOne
+
+ accessCheckTypeChangeTwo:
+ description: "new command fails because the access check type has changed"
+ command_name: accessCheckTypeChangeTwo
+ namespace: ignored
+ cpp_name: accessCheckTypeChangeTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ none: true
+
+ complexChecksNotSubset:
+ description: "new command fails because the complex checks are not a subset of the
+ old complex checks"
+ command_name: complexChecksNotSubset
+ namespace: ignored
+ cpp_name: complexChecksNotSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - check: checkThree
+ - check: checkOne
+
+ complexChecksNotSubsetTwo:
+ description: "new command fails because the complex checks are not a subset of the
+ old complex checks"
+ command_name: complexChecksNotSubsetTwo
+ namespace: ignored
+ cpp_name: complexChecksNotSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkThree
+ - check: checkOne
+ - check: checkTwo
+
+ complexResourcePatternChange:
+ description: "new command fails because there is a change in the complex resource patterns"
+ command_name: complexResourcePatternChange
+ namespace: ignored
+ cpp_name: complexResourcePatternChange
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: actionTypeOne
+
+ complexActionTypesNotSubset:
+ description: "new command fails because there is a change in the complex action types"
+ command_name: complexActionTypesNotSubset
+ namespace: ignored
+ cpp_name: complexActionTypesNotSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ complexActionTypesNotSubsetTwo:
+ description: "new command fails because there is a change in the complex action types"
+ command_name: complexActionTypesNotSubsetTwo
+ namespace: ignored
+ cpp_name: complexActionTypesNotSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeFour]
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeThree, actionTypeOne, actionTypeTwo]
+ - privilege:
+ resource_pattern: resourcePatternThree
+ action_type: [actionTypeOne, actionTypeFour]
+
+ additionalComplexAccessCheck:
+ description: "new command fails because there is an additional complex access check"
+ command_name: additionalComplexAccessCheck
+ namespace: ignored
+ cpp_name: additionalComplexAccessCheck
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: actionTypeOne
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: actionTypeOne
+
+ removedAccessCheckField:
+ description: "new command fails because it removes the access_check field"
+ command_name: removedAccessCheckField
+ namespace: ignored
+ cpp_name: removedAccessCheckField
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+
+ addedAccessCheckField:
+ description: "new command fails because it adds the access_check field when the api_version is '1'"
+ command_name: addedAccessCheckField
+ namespace: ignored
+ cpp_name: addedAccessCheckField
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ none: true
diff --git a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
index a0942756341..674dd813172 100644
--- a/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
+++ b/buildscripts/idl/tests/compatibility_test_fail/old/compatibility_test_fail_old.idl
@@ -1816,4 +1816,156 @@ commands:
cpp_name: InvalidReplySkippedCommand
strict: true
api_version: "1"
- reply_type: NotStructFieldReply \ No newline at end of file
+ reply_type: NotStructFieldReply
+
+ accessCheckTypeChange:
+ description: "new command fails because the access check type has changed"
+ command_name: accessCheckTypeChange
+ namespace: ignored
+ cpp_name: accessCheckTypeChange
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ simple:
+ check: oldCheck
+
+ accessCheckTypeChangeTwo:
+ description: "new command fails because the access check type has changed"
+ command_name: accessCheckTypeChangeTwo
+ namespace: ignored
+ cpp_name: accessCheckTypeChangeTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkThree
+ - check: checkOne
+
+ complexChecksNotSubset:
+ description: "new command fails because the complex checks are not a subset of the
+ old complex checks"
+ command_name: complexChecksNotSubset
+ namespace: ignored
+ cpp_name: complexChecksNotSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - check: checkOne
+ - check: checkTwo
+
+ complexChecksNotSubsetTwo:
+ description: "new command fails because the complex checks are not a subset of the
+ old complex checks"
+ command_name: complexChecksNotSubsetTwo
+ namespace: ignored
+ cpp_name: complexChecksNotSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - check: checkTwo
+
+ complexResourcePatternChange:
+ description: "new command fails because there is a change in the complex resource patterns"
+ command_name: complexResourcePatternChange
+ namespace: ignored
+ cpp_name: complexResourcePatternChange
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: actionTypeOne
+
+ complexActionTypesNotSubset:
+ description: "new command fails because there is a change in the complex action types"
+ command_name: complexActionTypesNotSubset
+ namespace: ignored
+ cpp_name: complexActionTypesNotSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: actionTypeOne
+ - check: checkOne
+
+ complexActionTypesNotSubsetTwo:
+ description: "new command fails because there is a change in the complex action types"
+ command_name: complexActionTypesNotSubsetTwo
+ namespace: ignored
+ cpp_name: complexActionTypesNotSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeFour]
+ - privilege:
+ resource_pattern: resourcePatternThree
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ additionalComplexAccessCheck:
+ description: "new command fails because there is an additional complex access check"
+ command_name: additionalComplexAccessCheck
+ namespace: ignored
+ cpp_name: additionalComplexAccessCheck
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: actionTypeOne
+
+ removedAccessCheckField:
+ description: "new command fails because it removes the access_check field"
+ command_name: removedAccessCheckField
+ namespace: ignored
+ cpp_name: removedAccessCheckField
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ none: true
+
+ addedAccessCheckField:
+ description: "new command fails because it adds the access_check field when the api_version is '1'"
+ command_name: addedAccessCheckField
+ namespace: ignored
+ cpp_name: addedAccessCheckField
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
diff --git a/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl b/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl
index 4f4eb378306..a47f01f1199 100644
--- a/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl
+++ b/buildscripts/idl/tests/compatibility_test_pass/new/compatibility_test_pass_new.idl
@@ -1186,3 +1186,78 @@ commands:
parameterStruct:
type: array<ArrayTypeStruct>
reply_type: ArrayTypeStruct
+
+ complexActionTypesSubset:
+ description: "new command passes when the action types are a subset"
+ command_name: complexActionTypesSubset
+ namespace: ignored
+ cpp_name: complexActionTypesSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: actionTypeOne
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ complexActionTypesSubsetTwo:
+ description: "new command passes when the action types are a subset"
+ command_name: complexActionTypesSubsetTwo
+ namespace: ignored
+ cpp_name: complexActionTypesSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+
+ complexChecksSubset:
+ description: "new command passes when the complex checks are a subset of the old checks"
+ command_name: complexChecksSubset
+ namespace: ignored
+ cpp_name: complexChecksSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - check: checkThree
+ - check: checkTwo
+
+ removedComplexPrivilege:
+ description: "new command passes when a complex privilege is removed"
+ command_name: removedComplexPrivilege
+ namespace: ignored
+ cpp_name: removedComplexPrivilege
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ addedAccessCheckField:
+ description: "new command passes because it adds the access_check field when the api_version is not '1'"
+ command_name: addedAccessCheckField
+ namespace: ignored
+ cpp_name: addedAccessCheckField
+ strict: true
+ api_version: ""
+ reply_type: OkReply
+ access_check:
+ none: true
diff --git a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
index 203e4496ead..4ec2621cd39 100644
--- a/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
+++ b/buildscripts/idl/tests/compatibility_test_pass/old/compatibility_test_pass_old.idl
@@ -1185,3 +1185,81 @@ commands:
parameterStruct:
type: array<ArrayTypeStruct>
reply_type: ArrayTypeStruct
+
+ complexActionTypesSubset:
+ description: "new command passes when the action types are a subset"
+ command_name: complexActionTypesSubset
+ namespace: ignored
+ cpp_name: complexActionTypesSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+
+ complexActionTypesSubsetTwo:
+ description: "new command passes when the action types are a subset"
+ command_name: complexActionTypesSubsetTwo
+ namespace: ignored
+ cpp_name: complexActionTypesSubsetTwo
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - check: checkTwo
+ - privilege:
+ resource_pattern: resourcePatternTwo
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ complexChecksSubset:
+ description: "new command passes when the complex checks are a subset of the old checks"
+ command_name: complexChecksSubset
+ namespace: ignored
+ cpp_name: complexChecksSubset
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - check: checkOne
+ - check: checkTwo
+ - check: checkThree
+
+ removedComplexPrivilege:
+ description: "new command passes when a complex privilege is removed"
+ command_name: removedComplexPrivilege
+ namespace: ignored
+ cpp_name: removedComplexPrivilege
+ strict: true
+ api_version: "1"
+ reply_type: OkReply
+ access_check:
+ complex:
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo]
+ - check: checkOne
+ - privilege:
+ resource_pattern: resourcePattern
+ action_type: [actionTypeOne, actionTypeTwo, actionTypeThree]
+
+ addedAccessCheckField:
+ description: "new command passes because it adds the access_check field when the api_version is not '1'"
+ command_name: addedAccessCheckField
+ namespace: ignored
+ cpp_name: addedAccessCheckField
+ strict: true
+ api_version: ""
+ reply_type: OkReply
diff --git a/buildscripts/idl/tests/test_compatibility.py b/buildscripts/idl/tests/test_compatibility.py
index dadb1fb4f6f..5a902c692d9 100644
--- a/buildscripts/idl/tests/test_compatibility.py
+++ b/buildscripts/idl/tests/test_compatibility.py
@@ -34,7 +34,7 @@ import sys
from os import path
sys.path.append(path.dirname(path.dirname(path.abspath(__file__))))
-#pylint: disable=wrong-import-position
+#pylint: disable=wrong-import-position,too-many-lines
import idl_check_compatibility
import idl_compatibility_errors
@@ -126,7 +126,7 @@ class TestIDLCompatibilityChecker(unittest.TestCase):
path.join(dir_path, "compatibility_test_fail/new"), ["src"])
self.assertTrue(error_collection.has_errors())
- self.assertTrue(error_collection.count() == 128)
+ self.assertTrue(error_collection.count() == 138)
invalid_api_version_new_error = error_collection.get_error_by_command_name(
"invalidAPIVersionNew")
@@ -970,6 +970,67 @@ class TestIDLCompatibilityChecker(unittest.TestCase):
str(new_command_type_variant_struct_recursive_with_array_error),
"newCommandTypeVariantStructRecursiveWithArray")
+ access_check_type_change_error = error_collection.get_error_by_command_name(
+ "accessCheckTypeChange")
+ self.assertTrue(access_check_type_change_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_ACCESS_CHECK_TYPE_NOT_EQUAL)
+ self.assertRegex(str(access_check_type_change_error), "accessCheckTypeChange")
+
+ access_check_type_change_two_error = error_collection.get_error_by_command_name(
+ "accessCheckTypeChangeTwo")
+ self.assertTrue(access_check_type_change_two_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_ACCESS_CHECK_TYPE_NOT_EQUAL)
+ self.assertRegex(str(access_check_type_change_two_error), "accessCheckTypeChangeTwo")
+
+ complex_checks_not_subset_error = error_collection.get_error_by_command_name(
+ "complexChecksNotSubset")
+ self.assertTrue(complex_checks_not_subset_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_COMPLEX_CHECKS_NOT_SUBSET)
+ self.assertRegex(str(complex_checks_not_subset_error), "complexChecksNotSubset")
+
+ complex_checks_not_subset_two_error = error_collection.get_error_by_command_name(
+ "complexChecksNotSubsetTwo")
+ self.assertTrue(complex_checks_not_subset_two_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_ADDITIONAL_COMPLEX_ACCESS_CHECK)
+ self.assertRegex(str(complex_checks_not_subset_two_error), "complexChecksNotSubsetTwo")
+
+ complex_resource_pattern_change_error = error_collection.get_error_by_command_name(
+ "complexResourcePatternChange")
+ self.assertTrue(complex_resource_pattern_change_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_COMPLEX_PRIVILEGES_NOT_SUBSET)
+ self.assertRegex(str(complex_resource_pattern_change_error), "complexResourcePatternChange")
+
+ complex_action_types_not_subset_error = error_collection.get_error_by_command_name(
+ "complexActionTypesNotSubset")
+ self.assertTrue(complex_action_types_not_subset_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_COMPLEX_PRIVILEGES_NOT_SUBSET)
+ self.assertRegex(str(complex_action_types_not_subset_error), "complexActionTypesNotSubset")
+
+ complex_action_types_not_subset_two_error = error_collection.get_error_by_command_name(
+ "complexActionTypesNotSubsetTwo")
+ self.assertTrue(complex_action_types_not_subset_two_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_COMPLEX_PRIVILEGES_NOT_SUBSET)
+ self.assertRegex(
+ str(complex_action_types_not_subset_two_error), "complexActionTypesNotSubsetTwo")
+
+ additional_complex_access_check_error = error_collection.get_error_by_command_name(
+ "additionalComplexAccessCheck")
+ self.assertTrue(additional_complex_access_check_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_NEW_ADDITIONAL_COMPLEX_ACCESS_CHECK)
+ self.assertRegex(str(additional_complex_access_check_error), "additionalComplexAccessCheck")
+
+ removed_access_check_field_error = error_collection.get_error_by_command_name(
+ "removedAccessCheckField")
+ self.assertTrue(removed_access_check_field_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_REMOVED_ACCESS_CHECK_FIELD)
+ self.assertRegex(str(removed_access_check_field_error), "removedAccessCheckField")
+
+ added_access_check_field_error = error_collection.get_error_by_command_name(
+ "addedAccessCheckField")
+ self.assertTrue(added_access_check_field_error.error_id ==
+ idl_compatibility_errors.ERROR_ID_ADDED_ACCESS_CHECK_FIELD)
+ self.assertRegex(str(added_access_check_field_error), "addedAccessCheckField")
+
def test_error_reply(self):
"""Tests the compatibility checker with the ErrorReply struct."""
dir_path = path.dirname(path.realpath(__file__))