From 14b1dc3b75e69ba13bfb3c7f46770938dea15468 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 30 Jul 2019 21:08:02 +0200 Subject: [PATCH 01/30] Fix and combine integration tests targets. Make sure we only generate coverage for runners and orquesta integration tests when ENABLE_COVERAGE environment variable is set to "yes". Coverage adds a lot of overhead so this should speed up PR builds. --- Makefile | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 08fec2c261..b7f4a3780a 100644 --- a/Makefile +++ b/Makefile @@ -665,6 +665,28 @@ endif echo "Done running tests in" $$component; \ echo "==========================================================="; \ done + @echo + @echo "============== runners integration tests with coverage ==============" + @echo + @echo "The tests assume st2 is running on 127.0.0.1." + @for component in $(COMPONENTS_RUNNERS); do\ + echo "==========================================================="; \ + echo "Running tests in" $$component; \ + echo "==========================================================="; \ + . $(VIRTUALENV_DIR)/bin/activate; \ + COVERAGE_FILE=.coverage.integration.$$(echo $$component | tr '/' '.') \ + nosetests $(NOSE_OPTS) -s -v \ + $(NOSE_COVERAGE_FLAGS) $(NOSE_COVERAGE_PACKAGES) $$component/tests/integration || exit 1; \ + done + @echo + @echo "==================== Orquesta integration tests with coverage (HTML reports) ====================" + @echo "The tests assume st2 is running on 127.0.0.1." + @echo + . $(VIRTUALENV_DIR)/bin/activate; \ + COVERAGE_FILE=.coverage.integration.orquesta \ + nosetests $(NOSE_OPTS) -s -v \ + $(NOSE_COVERAGE_FLAGS) $(NOSE_COVERAGE_PACKAGES) st2tests/integration/orquesta || exit 1; \ + .PHONY: .combine-integration-tests-coverage .combine-integration-tests-coverage: .run-integration-tests-coverage @@ -960,7 +982,7 @@ ci-unit: .unit-tests-coverage-html sudo -E ./scripts/travis/prepare-integration.sh .PHONY: ci-integration -ci-integration: .ci-prepare-integration .itests-coverage-html .runners-itests-coverage-html .orquesta-itests-coverage-html +ci-integration: .ci-prepare-integration .itests-coverage-html .PHONY: ci-runners ci-runners: .ci-prepare-integration .runners-itests-coverage-html From 2c1d1a5462f3a492155f9d78fa7c31417b13a5ea Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 30 Jul 2019 22:22:19 +0200 Subject: [PATCH 02/30] Try to decrease wait delay, see if that helps. --- st2tests/integration/orquesta/base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2tests/integration/orquesta/base.py b/st2tests/integration/orquesta/base.py index dcb4206260..697a700b1c 100644 --- a/st2tests/integration/orquesta/base.py +++ b/st2tests/integration/orquesta/base.py @@ -84,7 +84,7 @@ def _execute_workflow(self, action, parameters=None, execute_async=True, @retrying.retry( retry_on_exception=retry_on_exceptions, - wait_fixed=3000, stop_max_delay=900000) + wait_fixed=500, stop_max_delay=900000) def _wait_for_state(self, ex, states): if isinstance(states, six.string_types): states = [states] @@ -113,7 +113,7 @@ def _get_children(self, ex): @retrying.retry( retry_on_exception=retry_on_exceptions, - wait_fixed=3000, stop_max_delay=900000) + wait_fixed=500, stop_max_delay=900000) def _wait_for_task(self, ex, task, status=None, num_task_exs=1): ex = self.st2client.executions.get_by_id(ex.id) From 8fc1423056daa486e37c95e1ca48c38367569869 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 31 Jul 2019 12:06:18 +0200 Subject: [PATCH 03/30] Add a work around for two tests which rely on longer retry delay. --- .../integration/orquesta/test_wiring_error_handling.py | 7 +++++++ .../integration/orquesta/test_wiring_pause_and_resume.py | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/st2tests/integration/orquesta/test_wiring_error_handling.py b/st2tests/integration/orquesta/test_wiring_error_handling.py index 7f70dad869..be49756b31 100644 --- a/st2tests/integration/orquesta/test_wiring_error_handling.py +++ b/st2tests/integration/orquesta/test_wiring_error_handling.py @@ -14,6 +14,7 @@ from __future__ import absolute_import +import eventlet from integration.orquesta import base from st2common.constants import action as ac_const @@ -244,6 +245,12 @@ def test_remediate_then_fail(self): ex = self._wait_for_completion(ex) # Assert that the log task is executed. + # NOTE: There is a race wheen execution gets in a desired state, but before the child + # tasks are written. To avoid that, we use longer sleep delay here. + # Better approach would be to try to retry a couple of times until expected num of + # tasks is reached (With some hard limit) before failing + eventlet.sleep(2) + self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_FAILED) self._wait_for_task(ex, 'log', ac_const.LIVEACTION_STATUS_SUCCEEDED) diff --git a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py index 6e8a3abe7e..a2bd3ec69f 100644 --- a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py +++ b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py @@ -15,6 +15,7 @@ from __future__ import absolute_import import os +import eventlet from integration.orquesta import base @@ -177,6 +178,12 @@ def test_pause_and_resume_cascade_from_subworkflow(self): self.assertFalse(os.path.exists(path)) # Wait for the workflow and task to be paused. + # NOTE: There is a race wheen execution gets in a desired state, but before the child + # tasks are written. To avoid that, we use longer sleep delay here. + # Better approach would be to try to retry a couple of times until expected num of + # tasks is reached (With some hard limit) before failing + eventlet.sleep(3) + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) From bbebd0284eba2e76ef3d3f255920554cb12b349a Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 31 Jul 2019 13:20:07 +0200 Subject: [PATCH 04/30] Allow wait_fixed and stop_max_delay to be provided on per method invocation basis. Update tests which have a race / rely on timing to use longer wait time to avoid failure. --- st2tests/integration/orquesta/base.py | 148 ++++++++++-------- .../orquesta/test_wiring_pause_and_resume.py | 76 +++++---- 2 files changed, 127 insertions(+), 97 deletions(-) diff --git a/st2tests/integration/orquesta/base.py b/st2tests/integration/orquesta/base.py index 697a700b1c..b7076fd4ce 100644 --- a/st2tests/integration/orquesta/base.py +++ b/st2tests/integration/orquesta/base.py @@ -32,6 +32,9 @@ action_constants.LIVEACTION_STATUS_RUNNING ] +DEFAULT_WAIT_FIXED = 3000 +DEFAULT_STOP_MAX_DELAY = 900000 + def retry_on_exceptions(exc): return isinstance(exc, AssertionError) @@ -82,86 +85,99 @@ def _execute_workflow(self, action, parameters=None, execute_async=True, return ex - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=500, stop_max_delay=900000) - def _wait_for_state(self, ex, states): - if isinstance(states, six.string_types): - states = [states] + def _wait_for_state(self, ex, states, wait_fixed=DEFAULT_WAIT_FIXED, + stop_max_delay=DEFAULT_STOP_MAX_DELAY): - for state in states: - if state not in action_constants.LIVEACTION_STATUSES: - raise ValueError('Status %s is not valid.' % state) + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) + def inner(ex, states): + if isinstance(states, six.string_types): + states = [states] - try: - ex = self.st2client.executions.get_by_id(ex.id) - self.assertIn(ex.status, states) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state "%s" and ' - 'does not match expected state(s). %s' % - (ex.status, ex.result) - ) - else: - raise + for state in states: + if state not in action_constants.LIVEACTION_STATUSES: + raise ValueError('Status %s is not valid.' % state) - return ex + try: + ex = self.st2client.executions.get_by_id(ex.id) + self.assertIn(ex.status, states) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state "%s" and ' + 'does not match expected state(s). %s' % + (ex.status, ex.result) + ) + else: + raise + + return ex + return inner(ex=ex, states=states) def _get_children(self, ex): return self.st2client.executions.query(parent=ex.id) - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=500, stop_max_delay=900000) - def _wait_for_task(self, ex, task, status=None, num_task_exs=1): - ex = self.st2client.executions.get_by_id(ex.id) - - task_exs = [ - task_ex for task_ex in self._get_children(ex) - if task_ex.context.get('orquesta', {}).get('task_name', '') == task - ] - - try: - self.assertEqual(len(task_exs), num_task_exs) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state and does not match expected number of ' - 'tasks. Expected: %s Actual: %s' % (str(num_task_exs), str(len(task_exs))) - ) - else: - raise + def _wait_for_task(self, ex, task, status=None, num_task_exs=1, + wait_fixed=DEFAULT_WAIT_FIXED, + stop_max_delay=DEFAULT_STOP_MAX_DELAY): + + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) + def inner(ex, task, status, num_task_exs): + ex = self.st2client.executions.get_by_id(ex.id) + + task_exs = [ + task_ex for task_ex in self._get_children(ex) + if task_ex.context.get('orquesta', {}).get('task_name', '') == task + ] - if status is not None: try: - self.assertTrue(all([task_ex.status == status for task_ex in task_exs])) + self.assertEqual(len(task_exs), num_task_exs) except: if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: raise Exception( - 'Execution is in completed state and not all tasks ' - 'match expected status "%s".' % status + 'Execution is in completed state and does not match expected number of ' + 'tasks. Expected: %s Actual: %s' % (str(num_task_exs), str(len(task_exs))) ) else: raise - return task_exs - - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=3000, stop_max_delay=900000) - def _wait_for_completion(self, ex): - ex = self._wait_for_state(ex, action_constants.LIVEACTION_COMPLETED_STATES) - - try: - self.assertTrue(hasattr(ex, 'result')) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state and does not ' - 'contain expected result.' - ) - else: - raise + if status is not None: + try: + self.assertTrue(all([task_ex.status == status for task_ex in task_exs])) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state and not all tasks ' + 'match expected status "%s".' % status + ) + else: + raise + + return task_exs + return inner(ex=ex, task=task, status=status, num_task_exs=num_task_exs) + + def _wait_for_completion(self, ex, wait_fixed=DEFAULT_WAIT_FIXED, + stop_max_delay=DEFAULT_STOP_MAX_DELAY): + + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) + def inner(ex): + ex = self._wait_for_state(ex, action_constants.LIVEACTION_COMPLETED_STATES) - return ex + try: + self.assertTrue(hasattr(ex, 'result')) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state and does not ' + 'contain expected result.' + ) + else: + raise + + return ex + return inner(ex=ex) diff --git a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py index a2bd3ec69f..08aaadbc1f 100644 --- a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py +++ b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py @@ -15,7 +15,6 @@ from __future__ import absolute_import import os -import eventlet from integration.orquesta import base @@ -162,38 +161,39 @@ def test_pause_and_resume_cascade_from_subworkflow(self): # Launch the workflow. The workflow will wait for the temp file to be deleted. params = {'tempfile': path} ex = self._execute_workflow('examples.orquesta-test-pause-subworkflow', params) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING) + + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING, wait_fixed=1500) + tk_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING, + wait_fixed=1500) # Pause the subworkflow before the temp file is deleted. The task will be # paused but workflow will still be running. tk_ac_ex = self.st2client.executions.pause(tk_exs[0].id) # Expecting the workflow is still running and task1 is pausing. - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING, + wait_fixed=1500) + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, + wait_fixed=1500) # Delete the temporary file. os.remove(path) self.assertFalse(os.path.exists(path)) - # Wait for the workflow and task to be paused. - # NOTE: There is a race wheen execution gets in a desired state, but before the child - # tasks are written. To avoid that, we use longer sleep delay here. - # Better approach would be to try to retry a couple of times until expected num of - # tasks is reached (With some hard limit) before failing - eventlet.sleep(3) - - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) # Resume the task. tk_ac_ex = self.st2client.executions.resume(tk_ac_ex.id) # Wait for completion. - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) - + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) + def test_pause_from_1_of_2_subworkflows_and_resume_subworkflow_when_workflow_paused(self): # Temp files are created during test setup. Ensure the temp files exist. path1 = self.temp_file_path_x @@ -305,8 +305,10 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): params = {'file1': path1, 'file2': path2} ex = self._execute_workflow('examples.orquesta-test-pause-subworkflows', params) ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING) - tk2_exs = self._wait_for_task(ex, 'task2', ac_const.LIVEACTION_STATUS_RUNNING) + tk1_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING, + wait_fixed=1500) + tk2_exs = self._wait_for_task(ex, 'task2', ac_const.LIVEACTION_STATUS_RUNNING, + wait_fixed=1500) # Pause the subworkflow before the temp file is deleted. The task will be # paused but workflow and the other subworkflow will still be running. @@ -314,8 +316,10 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Expecting the workflow is still running and task1 is pausing. ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) - tk2_ac_ex = self._wait_for_state(tk2_exs[0], ac_const.LIVEACTION_STATUS_RUNNING) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, + wait_fixed=1500) + tk2_ac_ex = self._wait_for_state(tk2_exs[0], ac_const.LIVEACTION_STATUS_RUNNING, + wait_fixed=1500) # Pause the other subworkflow before the temp file is deleted. The main # workflow will still running because pause is initiated downstream. @@ -323,8 +327,10 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Expecting workflow and subworkflows are pausing. ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, + wait_fixed=1500) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, + wait_fixed=1500) # Delete the temporary files for the subworkflows. os.remove(path1) @@ -334,25 +340,33 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Wait for subworkflows to pause. The main workflow will also # pause now because no other task is running. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) # Resume the subworkflow. tk1_ac_ex = self.st2client.executions.resume(tk1_ac_ex.id) # The subworkflow will succeed while the other subworkflow is still paused. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED, + wait_fixed=1500) # Resume the other subworkflow. tk2_ac_ex = self.st2client.executions.resume(tk2_ac_ex.id) # Wait for completion. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, + wait_fixed=1500) def test_pause_from_all_subworkflows_and_resume_from_parent_workflow(self): # Temp files are created during test setup. Ensure the temp files exist. From f9cd5488a2b91b9f895311e75db582a4d4230dfb Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 31 Jul 2019 13:42:27 +0200 Subject: [PATCH 05/30] WIP: Move slow mistral unit tests and Orquesta integration tests to a nightly build. This should substantially speed up PR builds. --- .travis.yml | 2 +- Makefile | 18 ++++++++++++++++++ tox.ini | 29 ++++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index e2ccc901fa..1e9e9f980a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -135,7 +135,7 @@ script: # as long as PR builds - if [ "${TRAVIS_PULL_REQUEST}" = "false" ] && [ "${IS_NIGHTLY_BUILD}" = "no" ]; then COMMAND_THRESHOLD=$(expr ${COMMAND_THRESHOLD} \* 2); fi; ./scripts/travis/time-command.sh "make ${TASK}" ${COMMAND_THRESHOLD} # Run any additional nightly checks only as part of a nightly (cron) build - - if [ "${IS_NIGHTLY_BUILD}" = "yes" ] && [ "${TASK}" = "ci-checks ci-packs-tests" ]; then make ci-checks-nightly; fi + - if [ "${IS_NIGHTLY_BUILD}" = "yes" ]; then make ${TASK}-nightly; fi # NOTE: We only generate and submit coverage report for master and version branches # NOTE: We put this here and not after_success so build is marked as failed if this step fails # See https://docs.travis-ci.com/user/customizing-the-build/#breaking-the-build diff --git a/Makefile b/Makefile index b7f4a3780a..870a5c41b4 100644 --- a/Makefile +++ b/Makefile @@ -939,6 +939,13 @@ ci-py3-unit: NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-unit -vv NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-packs -vv +.PHONY: ci-py3-unit-nightly +ci-py3-unit: + @echo + @echo "==================== ci-py3-unit ====================" + @echo + NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-unit-nightly -vv + .PHONY: ci-py3-integration ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration @@ -949,6 +956,17 @@ ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration @echo NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-integration -vv +.PHONY: ci-py3-integration +ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration-nightly + +.PHONY: .ci-py3-integration-nightly +.ci-py3-integration-nightly: + @echo + @echo "==================== ci-py3-integration ====================" + @echo + NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-integration-nightly -vv + + .PHONY: .rst-check .rst-check: @echo diff --git a/tox.ini b/tox.ini index fb193b3449..7d6578f4c4 100644 --- a/tox.ini +++ b/tox.ini @@ -46,12 +46,23 @@ commands = nosetests --rednose --immediate -sv contrib/runners/http_runner/tests/unit/ nosetests --rednose --immediate -sv contrib/runners/noop_runner/tests/unit/ nosetests --rednose --immediate -sv contrib/runners/local_runner/tests/unit/ - nosetests --rednose --immediate -sv contrib/runners/mistral_v2/tests/unit/ nosetests --rednose --immediate -sv contrib/runners/orquesta_runner/tests/unit/ nosetests --rednose --immediate -sv contrib/runners/python_runner/tests/unit/ nosetests --rednose --immediate -sv contrib/runners/winrm_runner/tests/unit/ -# Python 3 tasks +[testenv:py36-unit-nightly] +basepython = python3.6 +setenv = PYTHONPATH = {toxinidir}/external:{toxinidir}/st2common:{toxinidir}/st2api:{toxinidir}/st2actions:{toxinidir}/st2exporter:{toxinidir}/st2reactor:{toxinidir}/st2tests:{toxinidir}/contrib/runners/action_chain_runner:{toxinidir}/contrib/runners/local_runner:{toxinidir}/contrib/runners/python_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/noop_runner:{toxinidir}/contrib/runners/announcement_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/mistral_v2:{toxinidir}/contrib/runners/orquesta_runner:{toxinidir}/contrib/runners/inquirer_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/winrm_runner + VIRTUALENV_DIR = {envdir} +passenv = NOSE_WITH_TIMER TRAVIS +install_command = pip install -U --force-reinstall {opts} {packages} +deps = virtualenv + -r{toxinidir}/requirements.txt + -e{toxinidir}/st2client + -e{toxinidir}/st2common +commands = + nosetests --rednose --immediate -sv contrib/runners/mistral_v2/tests/unit/ + [testenv:py36-packs] basepython = python3.6 setenv = PYTHONPATH = {toxinidir}/external:{toxinidir}/st2common:{toxinidir}/st2api:{toxinidir}/st2actions:{toxinidir}/st2exporter:{toxinidir}/st2reactor:{toxinidir}/st2tests:{toxinidir}/contrib/runners/action_chain_runner:{toxinidir}/contrib/runners/local_runner:{toxinidir}/contrib/runners/python_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/noop_runner:{toxinidir}/contrib/runners/announcement_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/mistral_v2:{toxinidir}/contrib/runners/orquesta_runner:{toxinidir}/contrib/runners/inquirer_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/winrm_runner @@ -91,6 +102,18 @@ commands = nosetests --rednose --immediate -sv --exe contrib/runners/action_chain_runner/tests/integration/ nosetests --rednose --immediate -sv --exe contrib/runners/local_runner/tests/integration/ nosetests --rednose --immediate -sv --exe contrib/runners/mistral_v2/tests/integration/ - nosetests --rednose --immediate -sv --exe contrib/runners/orquesta_runner/tests/integration/ nosetests --rednose --immediate -sv --exe st2tests/integration/orquesta/ nosetests --rednose --immediate -sv --exe contrib/runners/python_runner/tests/integration/ + +[testenv:py36-integration-nightly] +basepython = python3.6 +setenv = PYTHONPATH = {toxinidir}/external:{toxinidir}/st2common:{toxinidir}/st2auth:{toxinidir}/st2api:{toxinidir}/st2actions:{toxinidir}/st2exporter:{toxinidir}/st2reactor:{toxinidir}/st2tests:{toxinidir}/contrib/runners/action_chain_runner:{toxinidir}/contrib/runners/local_runner:{toxinidir}/contrib/runners/python_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/noop_runner:{toxinidir}/contrib/runners/announcement_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/mistral_v2:{toxinidir}/contrib/runners/orquesta_runner:{toxinidir}/contrib/runners/inquirer_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/winrm_runner + VIRTUALENV_DIR = {envdir} +passenv = NOSE_WITH_TIMER TRAVIS +install_command = pip install -U --force-reinstall {opts} {packages} +deps = virtualenv + -r{toxinidir}/requirements.txt + -e{toxinidir}/st2client + -e{toxinidir}/st2common +commands = + nosetests --rednose --immediate -sv --exe contrib/runners/orquesta_runner/tests/integration/ From cd5fb034715103ace40c08557ec0dd079e4f15bf Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 6 Aug 2019 20:25:24 +0200 Subject: [PATCH 06/30] Add support for blacklisting hosts to the HTTP runner by adding new "hosts_blacklist" runner attribute. --- .../http_runner/http_runner/http_runner.py | 38 ++++++++++++- .../http_runner/http_runner/runner.yaml | 7 +++ .../tests/unit/test_http_runner.py | 57 +++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index 6915840814..3fc5abec5a 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -13,6 +13,7 @@ # limitations under the License. from __future__ import absolute_import + import ast import copy import json @@ -21,6 +22,7 @@ import requests from requests.auth import HTTPBasicAuth from oslo_config import cfg +from six.moves.urllib import parse as urlparse # pylint: disable=import-error from st2common.runners.base import ActionRunner from st2common.runners.base import get_metadata as get_runner_metadata @@ -55,6 +57,7 @@ RUNNER_VERIFY_SSL_CERT = 'verify_ssl_cert' RUNNER_USERNAME = 'username' RUNNER_PASSWORD = 'password' +RUNNER_HOSTS_BLACKLIST = 'hosts_blacklist' # Lookup constants for action params ACTION_AUTH = 'auth' @@ -93,6 +96,7 @@ def pre_run(self): self._http_proxy = self.runner_parameters.get(RUNNER_HTTP_PROXY, None) self._https_proxy = self.runner_parameters.get(RUNNER_HTTPS_PROXY, None) self._verify_ssl_cert = self.runner_parameters.get(RUNNER_VERIFY_SSL_CERT, None) + self._hosts_blacklist = self.runner_parameters.get(RUNNER_HOSTS_BLACKLIST, []) def run(self, action_parameters): client = self._get_http_client(action_parameters) @@ -147,7 +151,8 @@ def _get_http_client(self, action_parameters): headers=headers, cookies=self._cookies, auth=auth, timeout=timeout, allow_redirects=self._allow_redirects, proxies=proxies, files=files, verify=self._verify_ssl_cert, - username=self._username, password=self._password) + username=self._username, password=self._password, + hosts_blacklist=self._hosts_blacklist) @staticmethod def _get_result_status(status_code): @@ -158,7 +163,8 @@ def _get_result_status(status_code): class HTTPClient(object): def __init__(self, url=None, method=None, body='', params=None, headers=None, cookies=None, auth=None, timeout=60, allow_redirects=False, proxies=None, - files=None, verify=False, username=None, password=None): + files=None, verify=False, username=None, password=None, + hosts_blacklist=None): if url is None: raise Exception('URL must be specified.') @@ -188,12 +194,19 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.verify = verify self.username = username self.password = password + self.hosts_blacklist = hosts_blacklist or [] def run(self): results = {} resp = None json_content = self._is_json_content() + # Check if the provided URL is blacklisted + is_url_blacklisted = self._is_url_blacklisted(url=self.url) + + if is_url_blacklisted: + raise ValueError('URL "%s" is blacklisted' % (self.url)) + try: if json_content: # cast params (body) to dict @@ -301,6 +314,27 @@ def _cast_object(self, value): else: return value + def _is_url_blacklisted(self, url): + """ + Verify if the provided URL is blacklisted via hosts_blacklist runner parameter. + """ + if not self.hosts_blacklist: + # Blacklist is empty + return False + + parsed = urlparse.urlparse(url) + + # Remove the port and [] + host = parsed.netloc.replace('[', '').replace(']', '') + + if parsed.port is not None: + host = host.replace(':%s' % (parsed.port), '') + + if host in self.hosts_blacklist: + return True + + return False + def get_runner(): return HttpRunner(str(uuid.uuid4())) diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index 85f1e4c1c5..a194fd2be0 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -36,6 +36,13 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean + hosts_blacklist: + description: Optional list of hosts (network locations) to blacklist (e.g. example.com, + 127.0.0.1, etc.) + required: false + type: array + items: + type: string output_key: body output_schema: status_code: diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 7f02d987d1..9dc25d8c74 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -15,6 +15,8 @@ from __future__ import absolute_import +import re + import six import mock import unittest2 @@ -212,3 +214,58 @@ def test_http_unicode_body_data(self, mock_requests): expected_data = body self.assertEqual(call_kwargs['data'], expected_data) + + @mock.patch('http_runner.http_runner.requests') + def test_blacklisted_url_netloc(self, mock_requests): + # Black list is empty + self.assertEqual(mock_requests.request.call_count, 0) + + url = 'http://www.example.com' + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + # Blacklist is set + hosts_blacklist = [ + 'example.com', + '127.0.0.1', + '::1', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334' + ] + + # Blacklisted urls + urls = [ + 'https://example.com', + 'http://example.com', + 'http://example.com:81', + 'http://example.com:80', + 'http://example.com:9000', + 'http://[::1]:80/', + 'http://[::1]', + 'http://[::1]:9000', + 'http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]', + 'https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8000' + ] + + for url in urls: + expected_msg = r'URL "%s" is blacklisted' % (re.escape(url)) + client = HTTPClient(url=url, method='GET', hosts_blacklist=hosts_blacklist) + self.assertRaisesRegexp(ValueError, expected_msg, client.run) + + # Non blacklisted URLs + urls = [ + 'https://example2.com', + 'http://example3.com', + 'http://example4.com:81' + ] + + for url in urls: + mock_requests.request.reset_mock() + + self.assertEqual(mock_requests.request.call_count, 0) + + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) From 0abe410df14b7ab19cbf79678c0c9591c9d2a461 Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Tue, 6 Aug 2019 13:48:14 -0600 Subject: [PATCH 07/30] adding additional fields to return to get rbac to work --- st2api/st2api/controllers/v1/actionexecutions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index 5e67417302..d25419de89 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -84,6 +84,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): 'action.parameters', 'runner.runner_parameters', 'parameters' + # necessary for ActionExecutionDB to determine permissions in enterprise ldap + 'action.pack' + 'action.uid' ] # A list of attributes which can be specified using ?exclude_attributes filter From 8fb27e7d7814e7f83b596e26d06644f8d3b31e0b Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Tue, 6 Aug 2019 15:32:21 -0600 Subject: [PATCH 08/30] missing comma --- st2api/st2api/controllers/v1/actionexecutions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index d25419de89..2c60f3e4b2 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -83,9 +83,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): mandatory_include_fields_retrieve = [ 'action.parameters', 'runner.runner_parameters', - 'parameters' + 'parameters', # necessary for ActionExecutionDB to determine permissions in enterprise ldap - 'action.pack' + 'action.pack', 'action.uid' ] From 07a5fac5db6cd5d0f5f26ae6e48634084bbf059e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 7 Aug 2019 09:44:51 +0200 Subject: [PATCH 09/30] Rename existing runner parameter from "hosts_blacklist" to "url_hosts_blacklist" and also add support for whitelist approach using "url_hosts_whitelist" runner parameter. --- .../http_runner/http_runner/http_runner.py | 60 ++++++++++++++---- .../http_runner/http_runner/runner.yaml | 13 +++- .../tests/unit/test_http_runner.py | 63 +++++++++++++++++-- 3 files changed, 118 insertions(+), 18 deletions(-) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index 3fc5abec5a..b37e92c60e 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -57,7 +57,8 @@ RUNNER_VERIFY_SSL_CERT = 'verify_ssl_cert' RUNNER_USERNAME = 'username' RUNNER_PASSWORD = 'password' -RUNNER_HOSTS_BLACKLIST = 'hosts_blacklist' +RUNNER_URL_HOSTS_BLACKLIST = 'url_hosts_blacklist' +RUNNER_URL_HOSTS_WHITELIST = 'url_hosts_whitelist' # Lookup constants for action params ACTION_AUTH = 'auth' @@ -96,11 +97,17 @@ def pre_run(self): self._http_proxy = self.runner_parameters.get(RUNNER_HTTP_PROXY, None) self._https_proxy = self.runner_parameters.get(RUNNER_HTTPS_PROXY, None) self._verify_ssl_cert = self.runner_parameters.get(RUNNER_VERIFY_SSL_CERT, None) - self._hosts_blacklist = self.runner_parameters.get(RUNNER_HOSTS_BLACKLIST, []) + self._url_hosts_blacklist = self.runner_parameters.get(RUNNER_URL_HOSTS_BLACKLIST, []) + self._url_hosts_whitelist = self.runner_parameters.get(RUNNER_URL_HOSTS_WHITELIST, []) def run(self, action_parameters): client = self._get_http_client(action_parameters) + if self._url_hosts_blacklist and self._url_hosts_whitelist: + msg = ('"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive. Only one should be provided.') + raise ValueError(msg) + try: result = client.run() except requests.exceptions.Timeout as e: @@ -152,7 +159,8 @@ def _get_http_client(self, action_parameters): timeout=timeout, allow_redirects=self._allow_redirects, proxies=proxies, files=files, verify=self._verify_ssl_cert, username=self._username, password=self._password, - hosts_blacklist=self._hosts_blacklist) + url_hosts_blacklist=self._url_hosts_blacklist, + url_hosts_whitelist=self._url_hosts_whitelist) @staticmethod def _get_result_status(status_code): @@ -164,7 +172,7 @@ class HTTPClient(object): def __init__(self, url=None, method=None, body='', params=None, headers=None, cookies=None, auth=None, timeout=60, allow_redirects=False, proxies=None, files=None, verify=False, username=None, password=None, - hosts_blacklist=None): + url_hosts_blacklist=None, url_hosts_whitelist=None): if url is None: raise Exception('URL must be specified.') @@ -194,7 +202,8 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.verify = verify self.username = username self.password = password - self.hosts_blacklist = hosts_blacklist or [] + self.url_hosts_blacklist = url_hosts_blacklist or [] + self.url_hosts_whitelist = url_hosts_whitelist or [] def run(self): results = {} @@ -207,6 +216,11 @@ def run(self): if is_url_blacklisted: raise ValueError('URL "%s" is blacklisted' % (self.url)) + is_url_whitelisted = self._is_url_whitelisted(url=self.url) + + if not is_url_whitelisted: + raise ValueError('URL "%s" is not whitelisted' % (self.url)) + try: if json_content: # cast params (body) to dict @@ -316,24 +330,46 @@ def _cast_object(self, value): def _is_url_blacklisted(self, url): """ - Verify if the provided URL is blacklisted via hosts_blacklist runner parameter. + Verify if the provided URL is blacklisted via url_hosts_blacklist runner parameter. """ - if not self.hosts_blacklist: + if not self.url_hosts_blacklist: # Blacklist is empty return False + host = self._get_host_from_url(url=url) + + if host in self.url_hosts_blacklist: + return True + + return False + + def _is_url_whitelisted(self, url): + """ + Verify if the provided URL is whitelisted via url_hosts_whitelist runner parameter. + """ + if not self.url_hosts_whitelist: + return True + + host = self._get_host_from_url(url=url) + + if host in self.url_hosts_whitelist: + return True + + return False + + def _get_host_from_url(self, url): + """ + Return sanitized host (netloc) value from the provided url. + """ parsed = urlparse.urlparse(url) - # Remove the port and [] + # Remove port and [] host = parsed.netloc.replace('[', '').replace(']', '') if parsed.port is not None: host = host.replace(':%s' % (parsed.port), '') - if host in self.hosts_blacklist: - return True - - return False + return host def get_runner(): diff --git a/contrib/runners/http_runner/http_runner/runner.yaml b/contrib/runners/http_runner/http_runner/runner.yaml index a194fd2be0..168f68bbb2 100644 --- a/contrib/runners/http_runner/http_runner/runner.yaml +++ b/contrib/runners/http_runner/http_runner/runner.yaml @@ -36,9 +36,18 @@ CA bundle which comes from Mozilla. Verification using a custom CA bundle is not yet supported. Set to False to skip verification. type: boolean - hosts_blacklist: + url_hosts_blacklist: description: Optional list of hosts (network locations) to blacklist (e.g. example.com, - 127.0.0.1, etc.) + 127.0.0.1, ::1, etc.). If action will try to access that endpoint, an exception will be + thrown and action will be marked as failed. + required: false + type: array + items: + type: string + url_hosts_whitelist: + description: Optional list of hosts (network locations) to whitelist (e.g. example.com, + 127.0.0.1, ::1, etc.). If specified, actions will only be able to hit hosts on this + whitelist. required: false type: array items: diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 9dc25d8c74..0cbc9f8c84 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -216,7 +216,7 @@ def test_http_unicode_body_data(self, mock_requests): self.assertEqual(call_kwargs['data'], expected_data) @mock.patch('http_runner.http_runner.requests') - def test_blacklisted_url_netloc(self, mock_requests): + def test_blacklisted_url_url_hosts_blacklist_runner_parameter(self, mock_requests): # Black list is empty self.assertEqual(mock_requests.request.call_count, 0) @@ -227,7 +227,7 @@ def test_blacklisted_url_netloc(self, mock_requests): self.assertEqual(mock_requests.request.call_count, 1) # Blacklist is set - hosts_blacklist = [ + url_hosts_blacklist = [ 'example.com', '127.0.0.1', '::1', @@ -250,7 +250,7 @@ def test_blacklisted_url_netloc(self, mock_requests): for url in urls: expected_msg = r'URL "%s" is blacklisted' % (re.escape(url)) - client = HTTPClient(url=url, method='GET', hosts_blacklist=hosts_blacklist) + client = HTTPClient(url=url, method='GET', url_hosts_blacklist=url_hosts_blacklist) self.assertRaisesRegexp(ValueError, expected_msg, client.run) # Non blacklisted URLs @@ -265,7 +265,62 @@ def test_blacklisted_url_netloc(self, mock_requests): self.assertEqual(mock_requests.request.call_count, 0) - client = HTTPClient(url=url, method='GET') + client = HTTPClient(url=url, method='GET', url_hosts_blacklist=url_hosts_blacklist) + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + @mock.patch('http_runner.http_runner.requests') + def test_whitelisted_url_url_hosts_whitelist_runner_parameter(self, mock_requests): + # Whitelist is empty + self.assertEqual(mock_requests.request.call_count, 0) + + url = 'http://www.example.com' + client = HTTPClient(url=url, method='GET') + client.run() + + self.assertEqual(mock_requests.request.call_count, 1) + + # Whitelist is set + url_hosts_whitelist = [ + 'example.com', + '127.0.0.1', + '::1', + '2001:0db8:85a3:0000:0000:8a2e:0370:7334' + ] + + # Non whitelisted urls + urls = [ + 'https://www.google.com', + 'https://www.example2.com', + 'http://127.0.0.2' + ] + + for url in urls: + expected_msg = r'URL "%s" is not whitelisted' % (re.escape(url)) + client = HTTPClient(url=url, method='GET', url_hosts_whitelist=url_hosts_whitelist) + self.assertRaisesRegexp(ValueError, expected_msg, client.run) + + # Whitelisted URLS + urls = [ + 'https://example.com', + 'http://example.com', + 'http://example.com:81', + 'http://example.com:80', + 'http://example.com:9000', + 'http://[::1]:80/', + 'http://[::1]', + 'http://[::1]:9000', + 'http://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]', + 'https://[2001:0db8:85a3:0000:0000:8a2e:0370:7334]:8000' + ] + + for url in urls: + mock_requests.request.reset_mock() + + self.assertEqual(mock_requests.request.call_count, 0) + + client = HTTPClient(url=url, method='GET', url_hosts_whitelist=url_hosts_whitelist) client.run() self.assertEqual(mock_requests.request.call_count, 1) From c074677c99b969855717fbfce2f5669b03cc1407 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 7 Aug 2019 09:57:51 +0200 Subject: [PATCH 10/30] Add changelog entry. --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index adb08ce0b3..c02b0a25a8 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,13 @@ Changelog in development -------------- +Added +~~~~~ + +* Add support for blacklisting / whitelisting hosts to the HTTP runner by adding new + ``url_hosts_blacklist`` and ``url_hosts_whitelist`` runner attribute. (new feature) + #4757 + Changed ~~~~~~~ From 0fd85ba56a072f415a741217f33523bbc86a8153 Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Wed, 7 Aug 2019 09:34:23 -0600 Subject: [PATCH 11/30] updating CHANGELOG.rst --- CHANGELOG.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index adb08ce0b3..d2dc8d6314 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -13,7 +13,10 @@ Changed Fixed ~~~~~ - +* Fix rbac with execution view where the rbac is unable to verify the pack or uid of the execution + because it was not returned from the action execution db. This would result in an internal server + error when trying to view the results of a single execution. + Contributed by Joshua Meyer (@jdmeyer3) #4758 * Fixed logging middleware to output a ``content_length`` of ``0`` instead of ``Infinity`` when the type of data being returned is not supported. Previously, when the value was set to ``Infinity`` this would result in invalid JSON being output into structured From c07f6d6fc1c316ec3e29696e5e89c1f0bbc09210 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 14:00:46 +0200 Subject: [PATCH 12/30] Add a test case which checks behavior when mutually exclusive arguments are provided. --- contrib/runners/http_runner/http_runner/http_runner.py | 5 +++++ .../runners/http_runner/tests/unit/test_http_runner.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/contrib/runners/http_runner/http_runner/http_runner.py b/contrib/runners/http_runner/http_runner/http_runner.py index b37e92c60e..af5edbdf11 100644 --- a/contrib/runners/http_runner/http_runner/http_runner.py +++ b/contrib/runners/http_runner/http_runner/http_runner.py @@ -205,6 +205,11 @@ def __init__(self, url=None, method=None, body='', params=None, headers=None, co self.url_hosts_blacklist = url_hosts_blacklist or [] self.url_hosts_whitelist = url_hosts_whitelist or [] + if self.url_hosts_blacklist and self.url_hosts_whitelist: + msg = ('"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive. Only one should be provided.') + raise ValueError(msg) + def run(self): results = {} resp = None diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 0cbc9f8c84..7d3f26d48f 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -324,3 +324,11 @@ def test_whitelisted_url_url_hosts_whitelist_runner_parameter(self, mock_request client.run() self.assertEqual(mock_requests.request.call_count, 1) + + def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive(self): + url = 'http://www.example.com' + + expected_msg = (r'"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive.') + self.assertRaisesRegexp(ValueError, expected_msg, HTTPClient, url=url, method='GET', + url_hosts_blacklist=[url], url_hosts_whitelist=[url]) From 25b43cbae43ddfee5e145089d5b0399e320b0164 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 15:29:46 +0200 Subject: [PATCH 13/30] Add some additional test cases for HttpRunner class itself. Previously we don't had test cases for HTTPClient class, but not the actual runner. --- .../tests/unit/test_http_runner.py | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/contrib/runners/http_runner/tests/unit/test_http_runner.py b/contrib/runners/http_runner/tests/unit/test_http_runner.py index 7d3f26d48f..fa12ce27a6 100644 --- a/contrib/runners/http_runner/tests/unit/test_http_runner.py +++ b/contrib/runners/http_runner/tests/unit/test_http_runner.py @@ -21,9 +21,17 @@ import mock import unittest2 +from st2common.constants.action import LIVEACTION_STATUS_SUCCEEDED from http_runner.http_runner import HTTPClient +from http_runner.http_runner import HttpRunner + import st2tests.config as tests_config +__all__ = [ + 'HTTPClientTestCase', + 'HTTPRunnerTestCase' +] + if six.PY2: EXPECTED_DATA = '' @@ -35,7 +43,7 @@ class MockResult(object): close = mock.Mock() -class HTTPRunnerTestCase(unittest2.TestCase): +class HTTPClientTestCase(unittest2.TestCase): @classmethod def setUpClass(cls): tests_config.parse_args() @@ -332,3 +340,45 @@ def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive 'exclusive.') self.assertRaisesRegexp(ValueError, expected_msg, HTTPClient, url=url, method='GET', url_hosts_blacklist=[url], url_hosts_whitelist=[url]) + + +class HTTPRunnerTestCase(unittest2.TestCase): + @mock.patch('http_runner.http_runner.requests') + def test_get_success(self, mock_requests): + mock_result = MockResult() + + # Unknown content type, body should be returned raw + mock_result.text = 'foo bar ponies' + mock_result.headers = {'Content-Type': 'text/html'} + mock_result.status_code = 200 + + mock_requests.request.return_value = mock_result + + runner_parameters = { + 'url': 'http://www.example.com', + 'method': 'GET' + } + runner = HttpRunner('id') + runner.runner_parameters = runner_parameters + runner.pre_run() + + status, result, _ = runner.run({}) + self.assertEqual(status, LIVEACTION_STATUS_SUCCEEDED) + self.assertEqual(result['body'], 'foo bar ponies') + self.assertEqual(result['status_code'], 200) + self.assertEqual(result['parsed'], False) + + def test_url_host_blacklist_and_url_host_blacklist_params_are_mutually_exclusive(self): + runner_parameters = { + 'url': 'http://www.example.com', + 'method': 'GET', + 'url_hosts_blacklist': ['http://127.0.0.1'], + 'url_hosts_whitelist': ['http://127.0.0.1'], + } + runner = HttpRunner('id') + runner.runner_parameters = runner_parameters + runner.pre_run() + + expected_msg = (r'"url_hosts_blacklist" and "url_hosts_whitelist" parameters are mutually ' + 'exclusive.') + self.assertRaisesRegexp(ValueError, expected_msg, runner.run, {}) From 01c20bd65d0800874acc3dace0978db4fae776fc Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 15:45:29 +0200 Subject: [PATCH 14/30] Update Travis config to only run nightly build if a particular task exists. --- .travis.yml | 2 +- scripts/travis/run-make-task-if-exists.sh | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100755 scripts/travis/run-make-task-if-exists.sh diff --git a/.travis.yml b/.travis.yml index 1e9e9f980a..077eab2970 100644 --- a/.travis.yml +++ b/.travis.yml @@ -135,7 +135,7 @@ script: # as long as PR builds - if [ "${TRAVIS_PULL_REQUEST}" = "false" ] && [ "${IS_NIGHTLY_BUILD}" = "no" ]; then COMMAND_THRESHOLD=$(expr ${COMMAND_THRESHOLD} \* 2); fi; ./scripts/travis/time-command.sh "make ${TASK}" ${COMMAND_THRESHOLD} # Run any additional nightly checks only as part of a nightly (cron) build - - if [ "${IS_NIGHTLY_BUILD}" = "yes" ]; then make ${TASK}-nightly; fi + - if [ "${IS_NIGHTLY_BUILD}" = "yes" ]; then ./scripts/travis/run-make-task-if-exists.sh ${TASK}-nightly; fi # NOTE: We only generate and submit coverage report for master and version branches # NOTE: We put this here and not after_success so build is marked as failed if this step fails # See https://docs.travis-ci.com/user/customizing-the-build/#breaking-the-build diff --git a/scripts/travis/run-make-task-if-exists.sh b/scripts/travis/run-make-task-if-exists.sh new file mode 100755 index 0000000000..47544b7e87 --- /dev/null +++ b/scripts/travis/run-make-task-if-exists.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash + +# Script which runs make tasks if it exists. If it doesn't exist, it exits with +# 0 status code +TASK=$1 + +if [ ! "${TASK}" ]; then + echo "Missing TASK argument" + exit 2 +fi + +$(make -n ${TASK} &> /dev/null) + +if [ $? -eq 2 ]; then + # tASK DOESN'T EXIST + echo "Task ${TASK} doesn't exist, skipping execution..." + exit 0 +fi + +exec make ${TASK} From 036c680b4d9856eff81a7f8b535847c73c5ceec3 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 15:54:33 +0200 Subject: [PATCH 15/30] Use changes from master. --- st2tests/integration/orquesta/base.py | 147 ++++++++---------- .../orquesta/test_wiring_pause_and_resume.py | 67 +++----- 2 files changed, 90 insertions(+), 124 deletions(-) diff --git a/st2tests/integration/orquesta/base.py b/st2tests/integration/orquesta/base.py index b7076fd4ce..a1f16dbb81 100644 --- a/st2tests/integration/orquesta/base.py +++ b/st2tests/integration/orquesta/base.py @@ -32,7 +32,7 @@ action_constants.LIVEACTION_STATUS_RUNNING ] -DEFAULT_WAIT_FIXED = 3000 +DEFAULT_WAIT_FIXED = 500 DEFAULT_STOP_MAX_DELAY = 900000 @@ -85,99 +85,86 @@ def _execute_workflow(self, action, parameters=None, execute_async=True, return ex - def _wait_for_state(self, ex, states, wait_fixed=DEFAULT_WAIT_FIXED, - stop_max_delay=DEFAULT_STOP_MAX_DELAY): + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=DEFAULT_WAIT_FIXED, stop_max_delay=DEFAULT_STOP_MAX_DELAY) + def _wait_for_state(self, ex, states): + if isinstance(states, six.string_types): + states = [states] - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) - def inner(ex, states): - if isinstance(states, six.string_types): - states = [states] + for state in states: + if state not in action_constants.LIVEACTION_STATUSES: + raise ValueError('Status %s is not valid.' % state) - for state in states: - if state not in action_constants.LIVEACTION_STATUSES: - raise ValueError('Status %s is not valid.' % state) - - try: - ex = self.st2client.executions.get_by_id(ex.id) - self.assertIn(ex.status, states) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state "%s" and ' - 'does not match expected state(s). %s' % - (ex.status, ex.result) - ) - else: - raise + try: + ex = self.st2client.executions.get_by_id(ex.id) + self.assertIn(ex.status, states) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state "%s" and ' + 'does not match expected state(s). %s' % + (ex.status, ex.result) + ) + else: + raise - return ex - return inner(ex=ex, states=states) + return ex def _get_children(self, ex): return self.st2client.executions.query(parent=ex.id) - def _wait_for_task(self, ex, task, status=None, num_task_exs=1, - wait_fixed=DEFAULT_WAIT_FIXED, - stop_max_delay=DEFAULT_STOP_MAX_DELAY): - - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) - def inner(ex, task, status, num_task_exs): - ex = self.st2client.executions.get_by_id(ex.id) - - task_exs = [ - task_ex for task_ex in self._get_children(ex) - if task_ex.context.get('orquesta', {}).get('task_name', '') == task - ] + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=DEFAULT_WAIT_FIXED, stop_max_delay=DEFAULT_STOP_MAX_DELAY) + def _wait_for_task(self, ex, task, status=None, num_task_exs=1): + ex = self.st2client.executions.get_by_id(ex.id) + + task_exs = [ + task_ex for task_ex in self._get_children(ex) + if task_ex.context.get('orquesta', {}).get('task_name', '') == task + ] + + try: + self.assertEqual(len(task_exs), num_task_exs) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state and does not match expected number of ' + 'tasks. Expected: %s Actual: %s' % (str(num_task_exs), str(len(task_exs))) + ) + else: + raise + if status is not None: try: - self.assertEqual(len(task_exs), num_task_exs) + self.assertTrue(all([task_ex.status == status for task_ex in task_exs])) except: if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: raise Exception( - 'Execution is in completed state and does not match expected number of ' - 'tasks. Expected: %s Actual: %s' % (str(num_task_exs), str(len(task_exs))) + 'Execution is in completed state and not all tasks ' + 'match expected status "%s".' % status ) else: raise - if status is not None: - try: - self.assertTrue(all([task_ex.status == status for task_ex in task_exs])) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state and not all tasks ' - 'match expected status "%s".' % status - ) - else: - raise - - return task_exs - return inner(ex=ex, task=task, status=status, num_task_exs=num_task_exs) - - def _wait_for_completion(self, ex, wait_fixed=DEFAULT_WAIT_FIXED, - stop_max_delay=DEFAULT_STOP_MAX_DELAY): - - @retrying.retry( - retry_on_exception=retry_on_exceptions, - wait_fixed=wait_fixed, stop_max_delay=stop_max_delay) - def inner(ex): - ex = self._wait_for_state(ex, action_constants.LIVEACTION_COMPLETED_STATES) - - try: - self.assertTrue(hasattr(ex, 'result')) - except: - if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: - raise Exception( - 'Execution is in completed state and does not ' - 'contain expected result.' - ) - else: - raise + return task_exs + + @retrying.retry( + retry_on_exception=retry_on_exceptions, + wait_fixed=DEFAULT_WAIT_FIXED, stop_max_delay=DEFAULT_STOP_MAX_DELAY) + def _wait_for_completion(self, ex): + ex = self._wait_for_state(ex, action_constants.LIVEACTION_COMPLETED_STATES) + + try: + self.assertTrue(hasattr(ex, 'result')) + except: + if ex.status in action_constants.LIVEACTION_COMPLETED_STATES: + raise Exception( + 'Execution is in completed state and does not ' + 'contain expected result.' + ) + else: + raise - return ex - return inner(ex=ex) + return ex diff --git a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py index e0f65d495c..6e8a3abe7e 100644 --- a/st2tests/integration/orquesta/test_wiring_pause_and_resume.py +++ b/st2tests/integration/orquesta/test_wiring_pause_and_resume.py @@ -161,38 +161,31 @@ def test_pause_and_resume_cascade_from_subworkflow(self): # Launch the workflow. The workflow will wait for the temp file to be deleted. params = {'tempfile': path} ex = self._execute_workflow('examples.orquesta-test-pause-subworkflow', params) - - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING, wait_fixed=1500) - tk_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING, - wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) + tk_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING) # Pause the subworkflow before the temp file is deleted. The task will be # paused but workflow will still be running. tk_ac_ex = self.st2client.executions.pause(tk_exs[0].id) # Expecting the workflow is still running and task1 is pausing. - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING, - wait_fixed=1500) - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, - wait_fixed=1500) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) # Delete the temporary file. os.remove(path) self.assertFalse(os.path.exists(path)) - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) + # Wait for the workflow and task to be paused. + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) # Resume the task. tk_ac_ex = self.st2client.executions.resume(tk_ac_ex.id) # Wait for completion. - tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) + tk_ac_ex = self._wait_for_state(tk_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) def test_pause_from_1_of_2_subworkflows_and_resume_subworkflow_when_workflow_paused(self): # Temp files are created during test setup. Ensure the temp files exist. @@ -305,10 +298,8 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): params = {'file1': path1, 'file2': path2} ex = self._execute_workflow('examples.orquesta-test-pause-subworkflows', params) ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING, - wait_fixed=1500) - tk2_exs = self._wait_for_task(ex, 'task2', ac_const.LIVEACTION_STATUS_RUNNING, - wait_fixed=1500) + tk1_exs = self._wait_for_task(ex, 'task1', ac_const.LIVEACTION_STATUS_RUNNING) + tk2_exs = self._wait_for_task(ex, 'task2', ac_const.LIVEACTION_STATUS_RUNNING) # Pause the subworkflow before the temp file is deleted. The task will be # paused but workflow and the other subworkflow will still be running. @@ -316,10 +307,8 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Expecting the workflow is still running and task1 is pausing. ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, - wait_fixed=1500) - tk2_ac_ex = self._wait_for_state(tk2_exs[0], ac_const.LIVEACTION_STATUS_RUNNING, - wait_fixed=1500) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) + tk2_ac_ex = self._wait_for_state(tk2_exs[0], ac_const.LIVEACTION_STATUS_RUNNING) # Pause the other subworkflow before the temp file is deleted. The main # workflow will still running because pause is initiated downstream. @@ -327,10 +316,8 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Expecting workflow and subworkflows are pausing. ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_RUNNING) - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, - wait_fixed=1500) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING, - wait_fixed=1500) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSING) # Delete the temporary files for the subworkflows. os.remove(path1) @@ -340,33 +327,25 @@ def test_pause_from_all_subworkflows_and_resume_from_subworkflows(self): # Wait for subworkflows to pause. The main workflow will also # pause now because no other task is running. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) # Resume the subworkflow. tk1_ac_ex = self.st2client.executions.resume(tk1_ac_ex.id) # The subworkflow will succeed while the other subworkflow is still paused. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED, - wait_fixed=1500) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_PAUSED) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_PAUSED) # Resume the other subworkflow. tk2_ac_ex = self.st2client.executions.resume(tk2_ac_ex.id) # Wait for completion. - tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) - tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) - ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED, - wait_fixed=1500) + tk1_ac_ex = self._wait_for_state(tk1_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + tk2_ac_ex = self._wait_for_state(tk2_ac_ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) + ex = self._wait_for_state(ex, ac_const.LIVEACTION_STATUS_SUCCEEDED) def test_pause_from_all_subworkflows_and_resume_from_parent_workflow(self): # Temp files are created during test setup. Ensure the temp files exist. From 06f93a41ebcc53da356efa7404610d969b2f1b84 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 15:59:11 +0200 Subject: [PATCH 16/30] Orquesta integration tests have been optimized, no need to run them as part of the nightly builds now. --- tox.ini | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tox.ini b/tox.ini index 7d6578f4c4..1ebef4b8c3 100644 --- a/tox.ini +++ b/tox.ini @@ -104,16 +104,3 @@ commands = nosetests --rednose --immediate -sv --exe contrib/runners/mistral_v2/tests/integration/ nosetests --rednose --immediate -sv --exe st2tests/integration/orquesta/ nosetests --rednose --immediate -sv --exe contrib/runners/python_runner/tests/integration/ - -[testenv:py36-integration-nightly] -basepython = python3.6 -setenv = PYTHONPATH = {toxinidir}/external:{toxinidir}/st2common:{toxinidir}/st2auth:{toxinidir}/st2api:{toxinidir}/st2actions:{toxinidir}/st2exporter:{toxinidir}/st2reactor:{toxinidir}/st2tests:{toxinidir}/contrib/runners/action_chain_runner:{toxinidir}/contrib/runners/local_runner:{toxinidir}/contrib/runners/python_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/noop_runner:{toxinidir}/contrib/runners/announcement_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/remote_runner:{toxinidir}/contrib/runners/mistral_v2:{toxinidir}/contrib/runners/orquesta_runner:{toxinidir}/contrib/runners/inquirer_runner:{toxinidir}/contrib/runners/http_runner:{toxinidir}/contrib/runners/winrm_runner - VIRTUALENV_DIR = {envdir} -passenv = NOSE_WITH_TIMER TRAVIS -install_command = pip install -U --force-reinstall {opts} {packages} -deps = virtualenv - -r{toxinidir}/requirements.txt - -e{toxinidir}/st2client - -e{toxinidir}/st2common -commands = - nosetests --rednose --immediate -sv --exe contrib/runners/orquesta_runner/tests/integration/ From 40cffe9c40efa0dad2232b0a6ac6aa9942932c33 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:09:14 +0200 Subject: [PATCH 17/30] Update script so it knows how to handle scenario where multiple tasks are specified, but not all of them have corresponding nightly tasks. For example: TASK="foo1 foo2 foo3" and only "foo2-nightly" task exists. In this case, only "foo2-nightly" would run and other would be ignored. --- .travis.yml | 2 +- scripts/travis/run-make-task-if-exists.sh | 20 ---------- .../travis/run-nightly-make-task-if-exists.sh | 38 +++++++++++++++++++ 3 files changed, 39 insertions(+), 21 deletions(-) delete mode 100755 scripts/travis/run-make-task-if-exists.sh create mode 100755 scripts/travis/run-nightly-make-task-if-exists.sh diff --git a/.travis.yml b/.travis.yml index 077eab2970..f8c138407a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -135,7 +135,7 @@ script: # as long as PR builds - if [ "${TRAVIS_PULL_REQUEST}" = "false" ] && [ "${IS_NIGHTLY_BUILD}" = "no" ]; then COMMAND_THRESHOLD=$(expr ${COMMAND_THRESHOLD} \* 2); fi; ./scripts/travis/time-command.sh "make ${TASK}" ${COMMAND_THRESHOLD} # Run any additional nightly checks only as part of a nightly (cron) build - - if [ "${IS_NIGHTLY_BUILD}" = "yes" ]; then ./scripts/travis/run-make-task-if-exists.sh ${TASK}-nightly; fi + - if [ "${IS_NIGHTLY_BUILD}" = "yes" ]; then ./scripts/travis/run-nightly-make-task-if-exists.sh "${TASK}"; fi # NOTE: We only generate and submit coverage report for master and version branches # NOTE: We put this here and not after_success so build is marked as failed if this step fails # See https://docs.travis-ci.com/user/customizing-the-build/#breaking-the-build diff --git a/scripts/travis/run-make-task-if-exists.sh b/scripts/travis/run-make-task-if-exists.sh deleted file mode 100755 index 47544b7e87..0000000000 --- a/scripts/travis/run-make-task-if-exists.sh +++ /dev/null @@ -1,20 +0,0 @@ -#!/usr/bin/env bash - -# Script which runs make tasks if it exists. If it doesn't exist, it exits with -# 0 status code -TASK=$1 - -if [ ! "${TASK}" ]; then - echo "Missing TASK argument" - exit 2 -fi - -$(make -n ${TASK} &> /dev/null) - -if [ $? -eq 2 ]; then - # tASK DOESN'T EXIST - echo "Task ${TASK} doesn't exist, skipping execution..." - exit 0 -fi - -exec make ${TASK} diff --git a/scripts/travis/run-nightly-make-task-if-exists.sh b/scripts/travis/run-nightly-make-task-if-exists.sh new file mode 100755 index 0000000000..35e0ad9f21 --- /dev/null +++ b/scripts/travis/run-nightly-make-task-if-exists.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash + +# Script which runs a corresponding make nightly tasks if it exists. If a task corresponding +# nightly task doesn't exist, it's ignored. +# +# For example, let's say we have TASK="ci-checks ci-unit ci-pack-tests" and only +# "ci-checks-nightly" make task exists. +# In this scenario, only "ci-check-nightly" tasks would run and other would be ignored. + +TASK=$1 + +if [ ! "${TASK}" ]; then + echo "Missing TASK argument" + echo "Usage: $0 " + exit 2 +fi + +# Note: TASK could contain a list of multiple tasks +TASKS=($TASK) + +EXISTING_TASKS=() +for TASK_NAME in ${TASKS[@]}; do + $(make -n ${TASK_NAME}-nightly &> /dev/null) + + if [ $? -eq 0 ]; then + # Task {TASK}-nightly exists + EXISTING_TASKS+=("$TASK_NAME-nightly") + fi +done + +# Run only tasks which exist +if [ ${#EXISTING_TASKS[@]} -eq 0 ]; then + echo "No existing nightly tasks found..." + exit 0 +fi + +echo "Running the following nightly tasks: ${EXISTING_TASKS[@]}" +exec make ${EXISTING_TASKS[@]} From 6a324b41bd7221f3a0c54b3d1936fa94d8668d7e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:15:11 +0200 Subject: [PATCH 18/30] Fix typo. --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 870a5c41b4..f578486f87 100644 --- a/Makefile +++ b/Makefile @@ -940,7 +940,7 @@ ci-py3-unit: NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-packs -vv .PHONY: ci-py3-unit-nightly -ci-py3-unit: +ci-py3-unit-nightly: @echo @echo "==================== ci-py3-unit ====================" @echo From dc388e23d6df12663db7db530487111a06e2b0ba Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:31:08 +0200 Subject: [PATCH 19/30] Move Mistral tests to a nightly build. --- Makefile | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/Makefile b/Makefile index f578486f87..2c4bedfd91 100644 --- a/Makefile +++ b/Makefile @@ -22,9 +22,11 @@ BINARIES := bin # All components are prefixed by st2 and not .egg-info. COMPONENTS := $(shell ls -a | grep ^st2 | grep -v .egg-info) COMPONENTS_RUNNERS := $(wildcard contrib/runners/*) +COMPONENTS_RUNNERS := $(wildcard contrib/runners/*) COMPONENTS_WITHOUT_ST2TESTS := $(shell ls -a | grep ^st2 | grep -v .egg-info | grep -v st2tests | grep -v st2exporter) COMPONENTS_WITH_RUNNERS := $(COMPONENTS) $(COMPONENTS_RUNNERS) +COMPONENTS_WITH_RUNNERS_WITHOUT_MISTRAL_RUNNER := $(foreach component,$(filter-out contrib/runners/mistral_v2,$(COMPONENTS_WITH_RUNNERS)),$(component)) COMPONENTS_TEST_DIRS := $(wildcard st2*/tests) $(wildcard contrib/runners/*/tests) @@ -42,6 +44,7 @@ space_char := space_char += COMPONENT_PYTHONPATH = $(subst $(space_char),:,$(realpath $(COMPONENTS_WITH_RUNNERS))) COMPONENTS_TEST := $(foreach component,$(filter-out $(COMPONENT_SPECIFIC_TESTS),$(COMPONENTS_WITH_RUNNERS)),$(component)) +COMPONENTS_TEST_WITHOUT_MISTRAL_RUNNER := $(foreach component,$(filter-out $(COMPONENT_SPECIFIC_TESTS),$(COMPONENTS_WITH_RUNNERS_WITHOUT_MISTRAL_RUNNER)),$(component)) COMPONENTS_TEST_COMMA := $(subst $(slash),$(dot),$(subst $(space_char),$(comma),$(COMPONENTS_TEST))) COMPONENTS_TEST_MODULES := $(subst $(slash),$(dot),$(COMPONENTS_TEST_DIRS)) COMPONENTS_TEST_MODULES_COMMA := $(subst $(space_char),$(comma),$(COMPONENTS_TEST_MODULES)) @@ -90,6 +93,9 @@ endif .PHONY: all all: requirements configgen check tests +.PHONY: foo +all: requirements configgen check tests + .PHONY: .coverage_globs .coverage_globs: @for coverage_result in $$( \ @@ -109,6 +115,8 @@ play: @echo @echo COMPONENTS_WITH_RUNNERS=$(COMPONENTS_WITH_RUNNERS) @echo + @echo COMPONENTS_WITH_RUNNERS_WITHOUT_MISTRAL_RUNNER=$(COMPONENTS_WITH_RUNNERS_WITHOUT_MISTRAL_RUNNER) + @echo @echo COMPONENTS_TEST=$(COMPONENTS_TEST) @echo @echo COMPONENTS_TEST_COMMA=$(COMPONENTS_TEST_COMMA) @@ -119,6 +127,8 @@ play: @echo @echo COMPONENTS_TEST_MODULES_COMMA=$(COMPONENTS_TEST_MODULES_COMMA) @echo + @echo COMPONENTS_TEST_WITHOUT_MISTRAL_RUNNER=$(COMPONENTS_TEST_WITHOUT_MISTRAL_RUNNER) + @echo @echo COMPONENT_PYTHONPATH=$(COMPONENT_PYTHONPATH) @echo @echo TRAVIS_PULL_REQUEST=$(TRAVIS_PULL_REQUEST) @@ -202,6 +212,15 @@ check-python-packages-nightly: .PHONY: ci-checks-nightly ci-checks-nightly: check-python-packages-nightly +.PHONY: foo1-nightly +foo1-nightly: + echo "foo1" + +.PHONY: foo2-nightly +foo2-nightly: + echo "foo2" + exit 3 + .PHONY: checklogs checklogs: @echo @@ -573,7 +592,7 @@ endif @echo @echo "----- Dropping st2-test db -----" @mongo st2-test --eval "db.dropDatabase();" - for component in $(COMPONENTS_TEST); do\ + for component in $(COMPONENTS_TEST_WITHOUT_MISTRAL_RUNNER); do\ echo "==========================================================="; \ echo "Running tests in" $$component; \ echo "-----------------------------------------------------------"; \ @@ -947,7 +966,7 @@ ci-py3-unit-nightly: NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-unit-nightly -vv .PHONY: ci-py3-integration -ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration +ci-py3-integration: requirementci-unit-nightlyi-py3-integration .PHONY: .ci-py3-integration .ci-py3-integration: @@ -959,14 +978,6 @@ ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration .PHONY: ci-py3-integration ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration-nightly -.PHONY: .ci-py3-integration-nightly -.ci-py3-integration-nightly: - @echo - @echo "==================== ci-py3-integration ====================" - @echo - NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-integration-nightly -vv - - .PHONY: .rst-check .rst-check: @echo @@ -995,6 +1006,15 @@ ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration-nig .PHONY: ci-unit ci-unit: .unit-tests-coverage-html +.PHONY: ci-unit-nightly +ci-unit-nightly: + # NOTE: We run mistral runner checks only as part of a nightly build to speed up + # non nightly builds (Mistral will be deprecated in the future) + @echo + @echo "============== ci-unit-nightly ==============" + @echo + nosetests $(NOSE_OPTS) -s -v contrib/runners/mistral_v2/tests/unit + .PHONY: .ci-prepare-integration .ci-prepare-integration: sudo -E ./scripts/travis/prepare-integration.sh From 50221393369e50b58039fbd3578349453f45ee59 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:35:39 +0200 Subject: [PATCH 20/30] Add this line back. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 1ebef4b8c3..3832b40c02 100644 --- a/tox.ini +++ b/tox.ini @@ -102,5 +102,6 @@ commands = nosetests --rednose --immediate -sv --exe contrib/runners/action_chain_runner/tests/integration/ nosetests --rednose --immediate -sv --exe contrib/runners/local_runner/tests/integration/ nosetests --rednose --immediate -sv --exe contrib/runners/mistral_v2/tests/integration/ + nosetests --rednose --immediate -sv --exe contrib/runners/orquesta_runner/tests/integration/ nosetests --rednose --immediate -sv --exe st2tests/integration/orquesta/ nosetests --rednose --immediate -sv --exe contrib/runners/python_runner/tests/integration/ From 6aeaa0a02e421ac4be27810f69e7356ca7d175ce Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:36:09 +0200 Subject: [PATCH 21/30] Remove dummy / test tasks. --- Makefile | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/Makefile b/Makefile index 2c4bedfd91..bc53018418 100644 --- a/Makefile +++ b/Makefile @@ -22,7 +22,6 @@ BINARIES := bin # All components are prefixed by st2 and not .egg-info. COMPONENTS := $(shell ls -a | grep ^st2 | grep -v .egg-info) COMPONENTS_RUNNERS := $(wildcard contrib/runners/*) -COMPONENTS_RUNNERS := $(wildcard contrib/runners/*) COMPONENTS_WITHOUT_ST2TESTS := $(shell ls -a | grep ^st2 | grep -v .egg-info | grep -v st2tests | grep -v st2exporter) COMPONENTS_WITH_RUNNERS := $(COMPONENTS) $(COMPONENTS_RUNNERS) @@ -93,9 +92,6 @@ endif .PHONY: all all: requirements configgen check tests -.PHONY: foo -all: requirements configgen check tests - .PHONY: .coverage_globs .coverage_globs: @for coverage_result in $$( \ @@ -212,15 +208,6 @@ check-python-packages-nightly: .PHONY: ci-checks-nightly ci-checks-nightly: check-python-packages-nightly -.PHONY: foo1-nightly -foo1-nightly: - echo "foo1" - -.PHONY: foo2-nightly -foo2-nightly: - echo "foo2" - exit 3 - .PHONY: checklogs checklogs: @echo @@ -966,7 +953,7 @@ ci-py3-unit-nightly: NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-unit-nightly -vv .PHONY: ci-py3-integration -ci-py3-integration: requirementci-unit-nightlyi-py3-integration +ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration .PHONY: .ci-py3-integration .ci-py3-integration: @@ -975,9 +962,6 @@ ci-py3-integration: requirementci-unit-nightlyi-py3-integration @echo NOSE_WITH_TIMER=$(NOSE_WITH_TIMER) tox -e py36-integration -vv -.PHONY: ci-py3-integration -ci-py3-integration: requirements .ci-prepare-integration .ci-py3-integration-nightly - .PHONY: .rst-check .rst-check: @echo From 4347c7bb79da86d29d6c7c4b8bc584ce5c138401 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 16:46:45 +0200 Subject: [PATCH 22/30] Enable Slack notifications. Also enable it so we can test it's working. --- .travis.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.travis.yml b/.travis.yml index f8c138407a..148a7f2734 100644 --- a/.travis.yml +++ b/.travis.yml @@ -147,3 +147,15 @@ script: # Alternative: use strict pip pinning, including git-based pip packages before_cache: - if [ ${TRAVIS_PULL_REQUEST} = 'false' ] && [ "${IS_NIGHTLY_BUILD}" = "no" ]; then rm -rf virtualenv/; fi + +# We want to be notified when a master or nightly build fails +notifications: + # Post build failures to '#stackstorm' channel in 'stackstorm' Slack + slack: + rooms: + - secure: "rPA22aDgvNe0/S/2e+cp1rSDdDUPufLXnCbfnRzMPVDSQ2UPdLmEl9IeOoEHZmq92AZtzY8UnQaPFuoM0HAPrYDgKopn4n4KpOo+xUlJ92qdNj5qk3Z1TmQHwUYFvCkMvaR/CpX2liRr/YB3qM+1vFAMsYgmqrBX8vcEqNJQy/M=" + on_pull_requests: true + on_success: always + on_failure: always + #on_success: change # default: always + #on_failure: always # default: always From ea2971f478d61ebe19b67dfed57151357a057dec Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 8 Aug 2019 17:09:44 +0200 Subject: [PATCH 23/30] Remove test change. --- .travis.yml | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 148a7f2734..60617b9dc1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -154,8 +154,6 @@ notifications: slack: rooms: - secure: "rPA22aDgvNe0/S/2e+cp1rSDdDUPufLXnCbfnRzMPVDSQ2UPdLmEl9IeOoEHZmq92AZtzY8UnQaPFuoM0HAPrYDgKopn4n4KpOo+xUlJ92qdNj5qk3Z1TmQHwUYFvCkMvaR/CpX2liRr/YB3qM+1vFAMsYgmqrBX8vcEqNJQy/M=" - on_pull_requests: true - on_success: always - on_failure: always - #on_success: change # default: always - #on_failure: always # default: always + on_pull_requests: false + on_success: change # default: always + on_failure: always # default: always From 9a712d9939c00b24d6b0f35bae8f3427c46b6da9 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 9 Aug 2019 16:26:55 +0200 Subject: [PATCH 24/30] Update actionexecutions.py --- st2api/st2api/controllers/v1/actionexecutions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index 2c60f3e4b2..1ba3a3031e 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -84,9 +84,13 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): 'action.parameters', 'runner.runner_parameters', 'parameters', - # necessary for ActionExecutionDB to determine permissions in enterprise ldap + + # Attributes below are mandatory for RBAC installations 'action.pack', - 'action.uid' + 'action.uid', + + # Required when rbac.permission_isolation is enabled + 'context' ] # A list of attributes which can be specified using ?exclude_attributes filter From 79ef087b5c9b66edb86f9d62eeb1897291656e88 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 9 Aug 2019 16:30:58 +0200 Subject: [PATCH 25/30] Update APIControllerWithIncludeAndExcludeFilterTestCase class so it also works when RBAC is enabled. When RBAC is enabled, we simply use admin use when testing include and exclude attributes functionality. --- st2tests/st2tests/api.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/st2tests/st2tests/api.py b/st2tests/st2tests/api.py index 404816b964..fc5236d421 100644 --- a/st2tests/st2tests/api.py +++ b/st2tests/st2tests/api.py @@ -180,7 +180,13 @@ class APIControllerWithIncludeAndExcludeFilterTestCase(object): # _get_model_instance method method test_exact_object_count = True + # True if those tests are running with rbac enabled + rbac_enabled = False + def test_get_all_exclude_attributes_and_include_attributes_are_mutually_exclusive(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + url = self.get_all_path + '?include_attributes=id&exclude_attributes=id' resp = self.app.get(url, expect_errors=True) self.assertEqual(resp.status_int, 400) @@ -189,6 +195,9 @@ def test_get_all_exclude_attributes_and_include_attributes_are_mutually_exclusiv self.assertRegexpMatches(resp.json['faultstring'], expected_msg) def test_get_all_invalid_exclude_and_include_parameter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + # 1. Invalid exclude_attributes field url = self.get_all_path + '?exclude_attributes=invalid_field' resp = self.app.get(url, expect_errors=True) @@ -206,6 +215,9 @@ def test_get_all_invalid_exclude_and_include_parameter(self): self.assertRegexpMatches(resp.json['faultstring'], expected_msg) def test_get_all_include_attributes_filter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + mandatory_include_fields = self.controller_cls.mandatory_include_fields_response # Create any resources needed by those tests (if not already created inside setUp / @@ -249,6 +261,9 @@ def test_get_all_include_attributes_filter(self): self._delete_mock_models(object_ids) def test_get_all_exclude_attributes_filter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + # Create any resources needed by those tests (if not already created inside setUp / # setUpClass) object_ids = self._insert_mock_models() From c10f512513e9edd667c01a6740ae007903acfb90 Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Tue, 6 Aug 2019 13:48:14 -0600 Subject: [PATCH 26/30] adding additional fields to return to get rbac to work --- st2api/st2api/controllers/v1/actionexecutions.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index 5e67417302..d25419de89 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -84,6 +84,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): 'action.parameters', 'runner.runner_parameters', 'parameters' + # necessary for ActionExecutionDB to determine permissions in enterprise ldap + 'action.pack' + 'action.uid' ] # A list of attributes which can be specified using ?exclude_attributes filter From 9f621b5ed0d672386ac57248e0e6420089aa56cd Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Tue, 6 Aug 2019 15:32:21 -0600 Subject: [PATCH 27/30] missing comma --- st2api/st2api/controllers/v1/actionexecutions.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index d25419de89..2c60f3e4b2 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -83,9 +83,9 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): mandatory_include_fields_retrieve = [ 'action.parameters', 'runner.runner_parameters', - 'parameters' + 'parameters', # necessary for ActionExecutionDB to determine permissions in enterprise ldap - 'action.pack' + 'action.pack', 'action.uid' ] From 8920e6dd0307ba2946caad9c5a7eda86ca68f787 Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Wed, 7 Aug 2019 09:34:23 -0600 Subject: [PATCH 28/30] updating CHANGELOG.rst --- CHANGELOG.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index c02b0a25a8..4b6e5ec92e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -20,7 +20,10 @@ Changed Fixed ~~~~~ - +* Fix rbac with execution view where the rbac is unable to verify the pack or uid of the execution + because it was not returned from the action execution db. This would result in an internal server + error when trying to view the results of a single execution. + Contributed by Joshua Meyer (@jdmeyer3) #4758 * Fixed logging middleware to output a ``content_length`` of ``0`` instead of ``Infinity`` when the type of data being returned is not supported. Previously, when the value was set to ``Infinity`` this would result in invalid JSON being output into structured From c9fafb8b517d4bca5c9f57fa92af11f5912e5491 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 9 Aug 2019 16:26:55 +0200 Subject: [PATCH 29/30] Update actionexecutions.py --- st2api/st2api/controllers/v1/actionexecutions.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/st2api/st2api/controllers/v1/actionexecutions.py b/st2api/st2api/controllers/v1/actionexecutions.py index 2c60f3e4b2..1ba3a3031e 100644 --- a/st2api/st2api/controllers/v1/actionexecutions.py +++ b/st2api/st2api/controllers/v1/actionexecutions.py @@ -84,9 +84,13 @@ class ActionExecutionsControllerMixin(BaseRestControllerMixin): 'action.parameters', 'runner.runner_parameters', 'parameters', - # necessary for ActionExecutionDB to determine permissions in enterprise ldap + + # Attributes below are mandatory for RBAC installations 'action.pack', - 'action.uid' + 'action.uid', + + # Required when rbac.permission_isolation is enabled + 'context' ] # A list of attributes which can be specified using ?exclude_attributes filter From ea5cde1dd3a2de10039e2e640613ab0311fbd159 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 9 Aug 2019 16:30:58 +0200 Subject: [PATCH 30/30] Update APIControllerWithIncludeAndExcludeFilterTestCase class so it also works when RBAC is enabled. When RBAC is enabled, we simply use admin use when testing include and exclude attributes functionality. --- st2tests/st2tests/api.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/st2tests/st2tests/api.py b/st2tests/st2tests/api.py index 404816b964..fc5236d421 100644 --- a/st2tests/st2tests/api.py +++ b/st2tests/st2tests/api.py @@ -180,7 +180,13 @@ class APIControllerWithIncludeAndExcludeFilterTestCase(object): # _get_model_instance method method test_exact_object_count = True + # True if those tests are running with rbac enabled + rbac_enabled = False + def test_get_all_exclude_attributes_and_include_attributes_are_mutually_exclusive(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + url = self.get_all_path + '?include_attributes=id&exclude_attributes=id' resp = self.app.get(url, expect_errors=True) self.assertEqual(resp.status_int, 400) @@ -189,6 +195,9 @@ def test_get_all_exclude_attributes_and_include_attributes_are_mutually_exclusiv self.assertRegexpMatches(resp.json['faultstring'], expected_msg) def test_get_all_invalid_exclude_and_include_parameter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + # 1. Invalid exclude_attributes field url = self.get_all_path + '?exclude_attributes=invalid_field' resp = self.app.get(url, expect_errors=True) @@ -206,6 +215,9 @@ def test_get_all_invalid_exclude_and_include_parameter(self): self.assertRegexpMatches(resp.json['faultstring'], expected_msg) def test_get_all_include_attributes_filter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + mandatory_include_fields = self.controller_cls.mandatory_include_fields_response # Create any resources needed by those tests (if not already created inside setUp / @@ -249,6 +261,9 @@ def test_get_all_include_attributes_filter(self): self._delete_mock_models(object_ids) def test_get_all_exclude_attributes_filter(self): + if self.rbac_enabled: + self.use_user(self.users['admin']) + # Create any resources needed by those tests (if not already created inside setUp / # setUpClass) object_ids = self._insert_mock_models()