#4932 - Fixes for field escaping in orquesta workflows

CHANGELOG.rst

@@ -4,6 +4,18 @@ Changelog
 in development
 --------------
 
+Fixed
+~~~~~
+* Fixed a bug where persisting Orquesta to the MongoDB database returned an error
+  ``message: key 'myvar.with.period' must not contain '.'``. This happened anytime an
+  ``input``, ``output``, ``publish`` or context ``var`` contained a key with a ``.`` within
+  the name (such as with hostnames and IP addresses). This was a regression introduced by
+  trying to improve performance. Fixing this bug means we are sacrificing performance of
+  serialization/deserialization in favor of correctness for persisting workflows and
+  their state to the MongoDB database. (bug fix) #4932
+
+  Contributed by Nick Maludy (@nmaludy Encore Technologies)
+
 
 3.2.0 - April 27, 2020
 ----------------------

Makefile

@@ -495,6 +495,8 @@ distclean: clean
 
 .PHONY: .requirements
 .requirements: virtualenv
+	# Print out pip version so we can see what version was restored from the Travis cache
+	$(VIRTUALENV_DIR)/bin/pip --version
 	# Generate all requirements to support current CI pipeline.
 	$(VIRTUALENV_DIR)/bin/python scripts/fixate-requirements.py --skip=virtualenv,virtualenv-osx -s st2*/in-requirements.txt contrib/runners/*/in-requirements.txt -f fixed-requirements.txt -o requirements.txt
 

contrib/examples/actions/orquesta-test-field-escaping.yaml

@@ -0,0 +1,8 @@
+---
+name: orquesta-test-field-escaping
+description: A workflow used to test escaping of things like vars, task names, results, outputs, etc.
+pack: examples
+runner_type: orquesta
+entry_point: workflows/tests/orquesta-test-field-escaping.yaml
+enabled: true
+parameters: {}

contrib/examples/actions/python-mock-core-remote.py

@@ -0,0 +1,23 @@
+from st2common.runners.base_action import Action
+
+
+class MockCoreRemoteAction(Action):
+
+    def run(self, cmd, hosts, hosts_dict):
+        if hosts_dict:
+            return hosts_dict
+
+        if not hosts:
+            return None
+
+        host_list = hosts.split(',')
+        results = {}
+        for h in hosts:
+            results[h] = {
+                'failed': False,
+                'return_code': 0,
+                'stderr': '',
+                'succeeded': True,
+                'stdout': cmd,
+            }
+        return results

contrib/examples/actions/python-mock-core-remote.yaml

@@ -0,0 +1,17 @@
+---
+name: python-mock-core-remote
+runner_type: python-script
+description: Action which mocks the core.remote action. It returns results with hostnames/IPs in them to ensure we are escaping the . properly. We set the output to the value of the command passed in
+enabled: true
+entry_point: python-mock-core-remote.py
+parameters:
+  cmd:
+    type: string
+    required: false
+  hosts:
+    type: string
+    required: false
+  hosts_dict:
+    type: object
+    description: "If specified, just returns this dict back out. Used for testing inputs and outputs of dicts with keys containing '.' character."
+    required: false

contrib/examples/actions/workflows/tests/orquesta-test-field-escaping.yaml

@@ -0,0 +1,26 @@
+version: 1.0
+
+description: A workflow used to test escaping of things like vars, task names, results, outputs, etc.
+
+vars:
+  - vars.key.with.periods: "vars.value.with.periods"
+  - vars.nested.with.periods:
+      nested.periods: "vars.nested.value.with.periods"
+
+tasks:
+  run:
+    action: examples.python-mock-core-remote
+    input:
+      hosts_dict:
+        hostname.domain.tld: "{{ ctx()['vars.key.with.periods'] }}"
+        hostname2.domain.tld:
+          stdout: "{{ ctx()['vars.nested.with.periods']['nested.periods'] }}"
+    next:
+      - when: "{{ succeeded() }}"
+        publish:
+          - published.hosts: "{{ result().result }}"
+          - published.field: "{{ result().result['hostname2.domain.tld']['stdout'] }}"
+
+output:
+  - wf.hostname.with.periods: "{{ ctx()['published.hosts'] }}"
+  - wf.output.with.periods: "{{ ctx()['published.field'] }}"

st2common/st2common/models/db/workflow.py

@@ -36,11 +36,11 @@ class WorkflowExecutionDB(stormbase.StormFoundationDB, stormbase.ChangeRevisionF
     RESOURCE_TYPE = types.ResourceType.EXECUTION
 
     action_execution = me.StringField(required=True)
-    spec = me.DictField()
+    spec = stormbase.EscapedDictField()
     graph = me.DictField()
     input = stormbase.EscapedDictField()
     notify = me.DictField()
-    context = me.DictField()
+    context = stormbase.EscapedDictField()
     state = stormbase.EscapedDictField()
     status = me.StringField(required=True)
     output = stormbase.EscapedDictField()
@@ -62,7 +62,7 @@ class TaskExecutionDB(stormbase.StormFoundationDB, stormbase.ChangeRevisionField
     task_name = me.StringField(required=True)
     task_id = me.StringField(required=True)
     task_route = me.IntField(required=True, min_value=0)
-    task_spec = me.DictField()
+    task_spec = stormbase.EscapedDictField()
     delay = me.IntField(min_value=0)
     itemized = me.BooleanField(default=False)
     items_count = me.IntField(min_value=0)

st2tests/integration/orquesta/test_wiring.py

@@ -156,3 +156,23 @@ def test_config_context_renders(self):
 
         self.assertEqual(ex.status, ac_const.LIVEACTION_STATUS_SUCCEEDED)
         self.assertDictEqual(ex.result, expected_result)
+
+    def test_field_escaping(self):
+        wf_name = 'examples.orquesta-test-field-escaping'
+
+        ex = self._execute_workflow(wf_name)
+        ex = self._wait_for_completion(ex)
+
+        expected_output = {
+            'wf.hostname.with.periods': {
+                'hostname.domain.tld': 'vars.value.with.periods',
+                'hostname2.domain.tld': {
+                    'stdout': 'vars.nested.value.with.periods',
+                },
+            },
+            'wf.output.with.periods': 'vars.nested.value.with.periods',
+        }
+        expected_result = {'output': expected_output}
+
+        self.assertEqual(ex.status, ac_const.LIVEACTION_STATUS_SUCCEEDED)
+        self.assertDictEqual(ex.result, expected_result)