[WIP] Hotfix/4932 orquesta field escaping

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
 
@@ -519,10 +521,10 @@ requirements: virtualenv .requirements .sdist-requirements install-runners
 	# .st2client-install-check target and .travis.yml to match
 	# Make sure we use latest version of pip
 	$(VIRTUALENV_DIR)/bin/pip --version
-	$(VIRTUALENV_DIR)/bin/pip install --upgrade "pip>=19.3.1"
+	$(VIRTUALENV_DIR)/bin/pip install --upgrade "pip==20.0.2"
 	# setuptools >= 41.0.1 is required for packs.install in dev envs
 	# setuptools >= 42     is required so setup.py install respects dependencies' python_requires
-	$(VIRTUALENV_DIR)/bin/pip install --upgrade "setuptools>=42"
+	$(VIRTUALENV_DIR)/bin/pip install --upgrade "setuptools==44.1.0"
 	$(VIRTUALENV_DIR)/bin/pip install --upgrade "pbr==5.4.3"  # workaround for pbr issue
 
 	# Fix for Travis CI race

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'] }}"

contrib/runners/action_chain_runner/action_chain_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/announcement_runner/announcement_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/http_runner/http_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/inquirer_runner/inquirer_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/local_runner/local_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/mistral_v2/mistral_v2/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/noop_runner/noop_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/orquesta_runner/orquesta_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/python_runner/python_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/remote_runner/remote_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

contrib/runners/winrm_runner/winrm_runner/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2actions/st2actions/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2api/st2api/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2auth/st2auth/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2client/st2client/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2common/bin/st2-run-pack-tests

@@ -198,7 +198,7 @@ if [ "${CREATE_VIRTUALENV}" = true ]; then
         activate_virtualenv
 
         # Make sure virtualenv is using latest pip version
-        ${VIRTUALENV_DIR}/bin/pip install --upgrade "pip>=19.0.1,<20.0"
+        ${VIRTUALENV_DIR}/bin/pip install --upgrade "pip==20.0.2"
     fi
 
     if [ ! -d "${VIRTUALENV_DIR}" ]; then

st2common/st2common/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

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)

st2common/tests/unit/test_dist_utils.py

@@ -30,7 +30,6 @@
 from dist_utils import fetch_requirements
 from dist_utils import apply_vagrant_workaround
 from dist_utils import get_version_string
-from dist_utils_old import fetch_requirements as old_fetch_requirements
 
 __all__ = [
     'DistUtilsTestCase'
@@ -138,18 +137,7 @@ def test_fetch_requirements(self):
         self.assertEqual(reqs, expected_reqs)
         self.assertEqual(links, expected_links)
 
-        # Verify output of old and new function is the same
-        reqs_old, links_old = old_fetch_requirements(REQUIREMENTS_PATH_1)
-
-        self.assertEqual(reqs_old, expected_reqs)
-        self.assertEqual(links_old, expected_links)
-
-        self.assertEqual(reqs_old, reqs)
-        self.assertEqual(links_old, links)
-
         # Also test it on requirements.txt in repo root
         reqs, links = fetch_requirements(REQUIREMENTS_PATH_2)
-        reqs_old, links_old = old_fetch_requirements(REQUIREMENTS_PATH_2)
-
-        self.assertEqual(reqs_old, reqs)
-        self.assertEqual(links_old, links)
+        self.assertGreater(len(reqs), 0)
+        self.assertGreater(len(links), 0)

st2debug/st2debug/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2reactor/st2reactor/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

st2stream/st2stream/__init__.py

@@ -12,4 +12,4 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-__version__ = '3.2dev'
+__version__ = '3.3dev'

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)