check commands structure with None values (#4451)

Author: vladak

tools/src/main/python/opengrok_tools/utils/commandsequence.py

@@ -136,23 +136,34 @@ def check_command_property(command):
     w.r.t. individual commands.
     :param command: command element
     """
-
     if not isinstance(command, dict):
         raise CommandConfigurationException("command '{}' is not a dictionary".format(command))
 
-    command_value = command.get(COMMAND_PROPERTY)
-    call_value = command.get(CALL_PROPERTY)
-    if command_value is None and call_value is None:
-        raise CommandConfigurationException(f"command dictionary has unknown key: {command}")
+    for key in command.keys():
+        if key not in [COMMAND_PROPERTY, CALL_PROPERTY]:
+            raise CommandConfigurationException(f"command dictionary has unknown key: {command}")
 
-    if command_value and not isinstance(command_value, dict):
-        raise CommandConfigurationException("command value not a dictionary: {}".
-                                            format(command_value))
-    if call_value:
-        check_call_config(call_value)
+    # There is difference between dictionary key that is missing and dictionary key that has None value.
+    # The get() function cannot distinguish between these two. Given the latter is sometimes a result
+    # of YAML parsing, use the below code to catch it.
+    try:
+        command_value = command[COMMAND_PROPERTY]
+        if command_value is None:
+            raise CommandConfigurationException("empty command value")
 
-    if command_value:
         check_command_config(command_value)
+        return
+    except KeyError:
+        pass
+
+    try:
+        call_value = command[CALL_PROPERTY]
+        if call_value is None:
+            raise CommandConfigurationException("empty call value")
+
+        check_call_config(call_value)
+    except KeyError:
+        pass
 
 
 class ApiCall:

tools/src/test/python/test_command_sequence.py

@@ -33,7 +33,7 @@
 
 from opengrok_tools.utils.commandsequence import CommandSequence, \
     CommandSequenceBase, CommandConfigurationException
-from opengrok_tools.utils.patterns import PROJECT_SUBST, CALL_PROPERTY
+from opengrok_tools.utils.patterns import PROJECT_SUBST, CALL_PROPERTY, COMMAND_PROPERTY
 
 
 def test_str():
@@ -74,6 +74,14 @@ def test_invalid_configuration_commands_no_dict2():
     assert str(exc_info.value).find("is not a dictionary") != -1
 
 
+@pytest.mark.parametrize('type', [COMMAND_PROPERTY, CALL_PROPERTY])
+def test_invalid_configuration_commands_none_value(type):
+    with pytest.raises(CommandConfigurationException) as exc_info:
+        CommandSequence(CommandSequenceBase("foo", [{type: None}]))
+
+    assert str(exc_info.value).find("empty") != -1
+
+
 def test_timeout_propagation():
     """
     Make sure the timeouts propagate from CommandSequenceBase to CommandSequence.