diff --git a/CHANGELOG.rst b/CHANGELOG.rst index a1e7e6c805..93d826bdc0 100644 --- a/CHANGELOG.rst +++ b/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 ---------------------- diff --git a/Makefile b/Makefile index e900c3ae9a..7390776e84 100644 --- a/Makefile +++ b/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 diff --git a/contrib/examples/actions/orquesta-test-field-escaping.yaml b/contrib/examples/actions/orquesta-test-field-escaping.yaml new file mode 100644 index 0000000000..a5b5de5004 --- /dev/null +++ b/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: {} diff --git a/contrib/examples/actions/python-mock-core-remote.py b/contrib/examples/actions/python-mock-core-remote.py new file mode 100644 index 0000000000..cd4d44500e --- /dev/null +++ b/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 diff --git a/contrib/examples/actions/python-mock-core-remote.yaml b/contrib/examples/actions/python-mock-core-remote.yaml new file mode 100644 index 0000000000..f72702f235 --- /dev/null +++ b/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 diff --git a/contrib/examples/actions/workflows/tests/orquesta-test-field-escaping.yaml b/contrib/examples/actions/workflows/tests/orquesta-test-field-escaping.yaml new file mode 100644 index 0000000000..c3ea12e248 --- /dev/null +++ b/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'] }}" diff --git a/contrib/runners/action_chain_runner/action_chain_runner/__init__.py b/contrib/runners/action_chain_runner/action_chain_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/action_chain_runner/action_chain_runner/__init__.py +++ b/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' diff --git a/contrib/runners/announcement_runner/announcement_runner/__init__.py b/contrib/runners/announcement_runner/announcement_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/announcement_runner/announcement_runner/__init__.py +++ b/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' diff --git a/contrib/runners/http_runner/http_runner/__init__.py b/contrib/runners/http_runner/http_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/http_runner/http_runner/__init__.py +++ b/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' diff --git a/contrib/runners/inquirer_runner/inquirer_runner/__init__.py b/contrib/runners/inquirer_runner/inquirer_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/inquirer_runner/inquirer_runner/__init__.py +++ b/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' diff --git a/contrib/runners/local_runner/local_runner/__init__.py b/contrib/runners/local_runner/local_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/local_runner/local_runner/__init__.py +++ b/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' diff --git a/contrib/runners/mistral_v2/mistral_v2/__init__.py b/contrib/runners/mistral_v2/mistral_v2/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/mistral_v2/mistral_v2/__init__.py +++ b/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' diff --git a/contrib/runners/noop_runner/noop_runner/__init__.py b/contrib/runners/noop_runner/noop_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/noop_runner/noop_runner/__init__.py +++ b/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' diff --git a/contrib/runners/orquesta_runner/orquesta_runner/__init__.py b/contrib/runners/orquesta_runner/orquesta_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/orquesta_runner/orquesta_runner/__init__.py +++ b/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' diff --git a/contrib/runners/python_runner/python_runner/__init__.py b/contrib/runners/python_runner/python_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/python_runner/python_runner/__init__.py +++ b/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' diff --git a/contrib/runners/remote_runner/remote_runner/__init__.py b/contrib/runners/remote_runner/remote_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/remote_runner/remote_runner/__init__.py +++ b/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' diff --git a/contrib/runners/winrm_runner/winrm_runner/__init__.py b/contrib/runners/winrm_runner/winrm_runner/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/contrib/runners/winrm_runner/winrm_runner/__init__.py +++ b/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' diff --git a/st2actions/st2actions/__init__.py b/st2actions/st2actions/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2actions/st2actions/__init__.py +++ b/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' diff --git a/st2api/st2api/__init__.py b/st2api/st2api/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2api/st2api/__init__.py +++ b/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' diff --git a/st2auth/st2auth/__init__.py b/st2auth/st2auth/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2auth/st2auth/__init__.py +++ b/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' diff --git a/st2client/st2client/__init__.py b/st2client/st2client/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2client/st2client/__init__.py +++ b/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' diff --git a/st2common/bin/st2-run-pack-tests b/st2common/bin/st2-run-pack-tests index 5244b03272..bed2826760 100755 --- a/st2common/bin/st2-run-pack-tests +++ b/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 diff --git a/st2common/st2common/__init__.py b/st2common/st2common/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2common/st2common/__init__.py +++ b/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' diff --git a/st2common/st2common/models/db/workflow.py b/st2common/st2common/models/db/workflow.py index b642df8eb8..81722ece51 100644 --- a/st2common/st2common/models/db/workflow.py +++ b/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) diff --git a/st2common/tests/unit/test_dist_utils.py b/st2common/tests/unit/test_dist_utils.py index 9c24320c21..9cd2b1bcf8 100644 --- a/st2common/tests/unit/test_dist_utils.py +++ b/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) diff --git a/st2debug/st2debug/__init__.py b/st2debug/st2debug/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2debug/st2debug/__init__.py +++ b/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' diff --git a/st2reactor/st2reactor/__init__.py b/st2reactor/st2reactor/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2reactor/st2reactor/__init__.py +++ b/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' diff --git a/st2stream/st2stream/__init__.py b/st2stream/st2stream/__init__.py index 80eeded44a..d0382c39e3 100644 --- a/st2stream/st2stream/__init__.py +++ b/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' diff --git a/st2tests/integration/orquesta/test_wiring.py b/st2tests/integration/orquesta/test_wiring.py index 72af88fd55..729482d71f 100644 --- a/st2tests/integration/orquesta/test_wiring.py +++ b/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)