SERVER-54532 Extend buildscripts/idl/idl_check_compatibility.py to check for additions and changes in complex

5 files changed, 525 insertions, 4 deletions

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__))