SERVER-55212 Check compatibility of strict field

7 files changed, 71 insertions, 1 deletions

diff --git a/buildscripts/idl/idl_check_compatibility.py b/buildscripts/idl/idl_check_compatibility.py
index 672bd45762b..c04d5218a85 100644
--- a/buildscripts/idl/idl_check_compatibility.py
+++ b/buildscripts/idl/idl_check_compatibility.py
@@ -840,6 +840,9 @@ def check_compatibility(old_idl_dir: str, new_idl_dir: str,
                     new_idl_file = new_command_file[old_cmd.command_name]
                     new_idl_file_path = new_command_file_path[old_cmd.command_name]
 
+                    if not old_cmd.strict and new_cmd.strict:
+                        ctxt.add_command_strict_true_error(new_cmd.command_name, new_idl_file_path)
+
                     # Check compatibility of command's parameters.
                     check_command_params_or_type_struct_fields(
                         ctxt, old_cmd, new_cmd, old_cmd.command_name, old_idl_file, new_idl_file,
diff --git a/buildscripts/idl/idl_compatibility_errors.py b/buildscripts/idl/idl_compatibility_errors.py
index ee88e7c5c01..70639fbd14b 100644
--- a/buildscripts/idl/idl_compatibility_errors.py
+++ b/buildscripts/idl/idl_compatibility_errors.py
@@ -108,6 +108,7 @@ 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"
+ERROR_ID_COMMAND_STRICT_TRUE_ERROR = "ID0068"
 
 # 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
@@ -306,6 +307,13 @@ class IDLCompatibilityContext(object):
         self._add_error(ERROR_ID_REMOVED_COMMAND, command_name,
                         "Old command '%s' was removed from new commands." % (command_name), file)
 
+    def add_command_strict_true_error(self, command_name: str, file: str) -> None:
+        """Add an error about a command that changes from strict: false to strict: true."""
+        self._add_error(
+            ERROR_ID_COMMAND_STRICT_TRUE_ERROR, command_name,
+            "'%s' changes from strict: false in the old version to strict: true in the new version."
+            % (command_name), file)
+
     def add_duplicate_command_name_error(self, command_name: str, dir_name: str, file: str) -> None:
         """Add an error about a duplicate command name within a directory."""
         self._add_error(ERROR_ID_DUPLICATE_COMMAND_NAME, command_name,
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 d4ef773c95b..3d1fe968cb6 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
@@ -526,6 +526,15 @@ commands:
         strict: true
         api_version: "1"
         reply_type: OkReply
+
+    strictFalseToTrueCommand:
+        description: "command fails because it changes from strict: false to strict: true"
+        command_name: strictFalseToTrueCommand
+        namespace: ignored
+        cpp_name: strictFalseToTrueCommand
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
     
     removedCommandParameter:
         description: "parameter will be removed from command in compatibility_test_fail/new
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 674dd813172..52fec6da542 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
@@ -517,6 +517,15 @@ commands:
         api_version: "1"
         reply_type: OkReply
 
+    strictFalseToTrueCommand:
+        description: "command fails because it changes from strict: false to strict: true"
+        command_name: strictFalseToTrueCommand
+        namespace: ignored
+        cpp_name: strictFalseToTrueCommand
+        strict: false
+        api_version: "1"
+        reply_type: OkReply
+
     removedCommandParameter:
         description: "parameter will be removed from command in compatibility_test_fail/new
                       which results in an error"
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 a47f01f1199..d5efb295f0a 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
@@ -398,6 +398,23 @@ commands:
         api_version: "1"
         reply_type: OkReply
 
+    strictFalseCommand:
+        description: "command passes with strict: false in both versions of the command"
+        command_name: strictFalseCommand
+        namespace: ignored
+        cpp_name: strictFalseCommand
+        strict: false
+        api_version: "1"
+        reply_type: OkReply
+
+    strictTrueToFalseCommand:
+        description: "command passes when it switches from strict: true to switch: false"
+        command_name: strictTrueToFalseCommand
+        namespace: ignored
+        cpp_name: strictTrueToFalseCommand
+        strict: false
+        api_version: "1"
+        reply_type: OkReply
 
     addedCommandParameter:
         description: "new optional command parameter is added and should pass"
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 4ec2621cd39..1d17dd4366a 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
@@ -396,6 +396,24 @@ commands:
         strict: true
         api_version: ""
         reply_type: OkReply
+
+    strictFalseCommand:
+        description: "command passes with strict: false in both versions of the command"
+        command_name: strictFalseCommand
+        namespace: ignored
+        cpp_name: strictFalseCommand
+        strict: false
+        api_version: "1"
+        reply_type: OkReply
+
+    strictTrueToFalseCommand:
+        description: "command passes when it switches from strict: true to switch: false"
+        command_name: strictTrueToFalseCommand
+        namespace: ignored
+        cpp_name: strictTrueToFalseCommand
+        strict: true
+        api_version: "1"
+        reply_type: OkReply
     
     addedCommandParameter:
         description: "new optional command parameter is added and should pass"
diff --git a/buildscripts/idl/tests/test_compatibility.py b/buildscripts/idl/tests/test_compatibility.py
index 5a902c692d9..1a2978a0299 100644
--- a/buildscripts/idl/tests/test_compatibility.py
+++ b/buildscripts/idl/tests/test_compatibility.py
@@ -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() == 138)
+        self.assertTrue(error_collection.count() == 139)
 
         invalid_api_version_new_error = error_collection.get_error_by_command_name(
             "invalidAPIVersionNew")
@@ -156,6 +156,12 @@ class TestIDLCompatibilityChecker(unittest.TestCase):
             idl_compatibility_errors.ERROR_ID_REMOVED_COMMAND)
         self.assertRegex(str(removed_command_error), "removedCommand")
 
+        strict_false_to_true_command_error = error_collection.get_error_by_command_name(
+            "strictFalseToTrueCommand")
+        self.assertTrue(strict_false_to_true_command_error.error_id ==
+                        idl_compatibility_errors.ERROR_ID_COMMAND_STRICT_TRUE_ERROR)
+        self.assertRegex(str(strict_false_to_true_command_error), "strictFalseToTrueCommand")
+
         removed_command_parameter_error = error_collection.get_error_by_command_name(
             "removedCommandParameter")
         self.assertTrue(removed_command_parameter_error.error_id ==