From 7f8a651aa74df966a7cce08804b0005ce2f171bc Mon Sep 17 00:00:00 2001 From: Nick Maludy Date: Fri, 12 Jul 2019 11:01:20 -0400 Subject: [PATCH 01/75] Fixed #4739. st2 cli now prints action ref for various commands --- CHANGELOG.rst | 4 ++++ st2client/st2client/commands/action.py | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b255f90db5..1f9a93c1fa 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,6 +15,10 @@ Fixed Contributed by Nick Maludy (@nmaludy Encore Technologies) * Fix the workflow execution cancelation to proceed even if the workflow execution is not found or completed. (bug fix) #4735 +* Fixed ``st2 execution get`` and ``st2 run`` not printing the ``action.ref`` + for non-workflow actions. (bug fix) #4739 + + Contributed by Nick Maludy (@nmaludy Encore Technologies) 3.1.0 - June 27, 2019 --------------------- diff --git a/st2client/st2client/commands/action.py b/st2client/st2client/commands/action.py index e6b594bde8..f458303bc0 100644 --- a/st2client/st2client/commands/action.py +++ b/st2client/st2client/commands/action.py @@ -296,7 +296,7 @@ def _add_common_options(self): detail_arg_grp = execution_details_arg_grp.add_mutually_exclusive_group() detail_arg_grp.add_argument('--attr', nargs='+', - default=['id', 'status', 'parameters', 'result'], + default=self.display_attributes, help=('List of attributes to include in the ' 'output. "all" or unspecified will ' 'return all attributes.')) @@ -1167,7 +1167,7 @@ def run_and_print(self, args, **kwargs): class ActionExecutionGetCommand(ActionRunCommandMixin, ResourceViewCommand): display_attributes = ['id', 'action.ref', 'context.user', 'parameters', 'status', - 'start_timestamp', 'end_timestamp', 'result', 'liveaction'] + 'start_timestamp', 'end_timestamp', 'result'] include_attributes = ['action.ref', 'action.runner_type', 'start_timestamp', 'end_timestamp'] @@ -1321,7 +1321,7 @@ def run(self, args, **kwargs): class ActionExecutionPauseCommand(ActionRunCommandMixin, ResourceViewCommand): display_attributes = ['id', 'action.ref', 'context.user', 'parameters', 'status', - 'start_timestamp', 'end_timestamp', 'result', 'liveaction'] + 'start_timestamp', 'end_timestamp', 'result'] def __init__(self, resource, *args, **kwargs): super(ActionExecutionPauseCommand, self).__init__( @@ -1364,7 +1364,7 @@ def _print_result(self, args, execution_id, execution, **kwargs): class ActionExecutionResumeCommand(ActionRunCommandMixin, ResourceViewCommand): display_attributes = ['id', 'action.ref', 'context.user', 'parameters', 'status', - 'start_timestamp', 'end_timestamp', 'result', 'liveaction'] + 'start_timestamp', 'end_timestamp', 'result'] def __init__(self, resource, *args, **kwargs): super(ActionExecutionResumeCommand, self).__init__( From cfe0aa748493a2392518817fb2dbc38622dbea02 Mon Sep 17 00:00:00 2001 From: jeansfelix Date: Thu, 12 Sep 2019 11:54:58 -0300 Subject: [PATCH 02/75] Fix the visibility of action secret parameters on rule --- st2common/st2common/models/db/rule.py | 55 ++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 7492b5abaa..3d9a988d79 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -14,11 +14,15 @@ from __future__ import absolute_import import mongoengine as me +from mongoengine.queryset import Q +import copy +from st2common.persistence.action import Action from st2common.models.db import MongoDBAccess from st2common.models.db import stormbase from st2common.constants.types import ResourceType - +from st2common.util.secrets import get_secret_parameters +from st2common.util.secrets import mask_secret_parameters class RuleTypeDB(stormbase.StormBaseDB): enabled = me.BooleanField( @@ -101,6 +105,55 @@ class RuleDB(stormbase.StormFoundationDB, stormbase.TagsMixin, stormbase.UIDFieldMixin.get_indexes()) } + def mask_secrets(self, value): + """ + Process the model dictionary and mask secret values. + + :type value: ``dict`` + :param value: Document dictionary. + + :rtype: ``dict`` + """ + result = copy.deepcopy(value) + + action_db = self._get_referenced_models(rule=result) + + secret_parameters = get_secret_parameters(parameters=action_db['parameters']) + result['action']['parameters'] = mask_secret_parameters(parameters=result['action']['parameters'], + secret_parameters=secret_parameters) + + return result + + def _get_referenced_models(self, rule): + """ + Return the action model referenced from rule. + + :type rule: ``dict`` + :param rule: rule + + :rtype: ``ActionDB`` + """ + + action_ref = rule.get('action', {}).get('ref', None) + + def ref_query_args(ref): + return {'ref': ref} + + action_db = self._get_entity(model_persistence=Action, + ref=action_ref, + query_args=ref_query_args) + + return action_db + + def _get_entity(self, model_persistence, ref, query_args): + q = Q(**query_args(ref)) + + if q: + return model_persistence._get_impl().model.objects(q).first() + + return None + + def __init__(self, *args, **values): super(RuleDB, self).__init__(*args, **values) self.ref = self.get_reference().ref From 0f1bae2715af88a2ef14114f8dc7b6c4943ac5f8 Mon Sep 17 00:00:00 2001 From: jeansfelix Date: Thu, 12 Sep 2019 15:02:22 -0300 Subject: [PATCH 03/75] Fix PEP8 --- st2common/st2common/models/db/rule.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 3d9a988d79..809d3c2f5f 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -13,10 +13,10 @@ # limitations under the License. from __future__ import absolute_import +import copy import mongoengine as me -from mongoengine.queryset import Q -import copy +from mongoengine.queryset import Q from st2common.persistence.action import Action from st2common.models.db import MongoDBAccess from st2common.models.db import stormbase @@ -24,6 +24,7 @@ from st2common.util.secrets import get_secret_parameters from st2common.util.secrets import mask_secret_parameters + class RuleTypeDB(stormbase.StormBaseDB): enabled = me.BooleanField( default=True, @@ -119,8 +120,9 @@ def mask_secrets(self, value): action_db = self._get_referenced_models(rule=result) secret_parameters = get_secret_parameters(parameters=action_db['parameters']) - result['action']['parameters'] = mask_secret_parameters(parameters=result['action']['parameters'], - secret_parameters=secret_parameters) + result['action']['parameters'] = mask_secret_parameters( + parameters=result['action']['parameters'], + secret_parameters=secret_parameters) return result @@ -153,7 +155,6 @@ def _get_entity(self, model_persistence, ref, query_args): return None - def __init__(self, *args, **values): super(RuleDB, self).__init__(*args, **values) self.ref = self.get_reference().ref From 1e8aa64d3b20fd8522742887626ee1fd279b631d Mon Sep 17 00:00:00 2001 From: Tristan Struthers Date: Tue, 17 Sep 2019 12:16:35 -0700 Subject: [PATCH 04/75] Add ActionResourceManager and implement GET entrypoint --- st2client/st2client/client.py | 3 ++- st2client/st2client/models/action.py | 1 + st2client/st2client/models/core.py | 12 ++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/st2client/st2client/client.py b/st2client/st2client/client.py index 280174edb5..fe1d4cc1b0 100644 --- a/st2client/st2client/client.py +++ b/st2client/st2client/client.py @@ -25,6 +25,7 @@ from st2client.models.core import ResourceManager from st2client.models.core import ActionAliasResourceManager from st2client.models.core import ActionAliasExecutionManager +from st2client.models.core import ActionResourceManager from st2client.models.core import ExecutionResourceManager from st2client.models.core import InquiryResourceManager from st2client.models.core import TriggerInstanceResourceManager @@ -118,7 +119,7 @@ def __init__(self, base_url=None, auth_url=None, api_url=None, stream_url=None, models.Token, self.endpoints['auth'], cacert=self.cacert, debug=self.debug) self.managers['RunnerType'] = ResourceManager( models.RunnerType, self.endpoints['api'], cacert=self.cacert, debug=self.debug) - self.managers['Action'] = ResourceManager( + self.managers['Action'] = ActionResourceManager( models.Action, self.endpoints['api'], cacert=self.cacert, debug=self.debug) self.managers['ActionAlias'] = ActionAliasResourceManager( models.ActionAlias, self.endpoints['api'], cacert=self.cacert, debug=self.debug) diff --git a/st2client/st2client/models/action.py b/st2client/st2client/models/action.py index d931c48427..6432192792 100644 --- a/st2client/st2client/models/action.py +++ b/st2client/st2client/models/action.py @@ -33,6 +33,7 @@ class RunnerType(core.Resource): class Action(core.Resource): _plural = 'Actions' _repr_attributes = ['name', 'pack', 'enabled', 'runner_type'] + _url_path = 'actions' class Execution(core.Resource): diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index f9d2a86838..6b70c3ffc0 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -376,6 +376,18 @@ def match_and_execute(self, instance, **kwargs): return instance +class ActionResourceManager(ResourceManager): + @add_auth_token_to_kwargs_from_env + def get_entrypoint(self, ref_or_id, **kwargs): + url = '/%s/views/entry_point/%s' % (self.resource.get_url_path_name(), ref_or_id) + + response = self.client.get(url, **kwargs) + if response.status_code != http_client.OK: + self.handle_error(response) + + return response.text + + class ExecutionResourceManager(ResourceManager): @add_auth_token_to_kwargs_from_env def re_run(self, execution_id, parameters=None, tasks=None, no_reset=None, user=None, delay=0, From d05c236be64b3c2b2850bc2f2aa68438d7c7dfda Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Sep 2019 12:17:26 +0200 Subject: [PATCH 05/75] Instead of calling eventlet directly, pass the calls through the concurrency abstraction which works with either gevent or eventlet. --- st2common/st2common/util/concurrency.py | 12 ++++++++++- st2reactor/st2reactor/cmd/timersengine.py | 7 ++++--- st2reactor/st2reactor/container/manager.py | 7 +++---- .../st2reactor/container/process_container.py | 16 +++++++------- .../st2reactor/garbage_collector/base.py | 21 ++++++++++--------- st2reactor/st2reactor/sensor/base.py | 6 ++++-- 6 files changed, 42 insertions(+), 27 deletions(-) diff --git a/st2common/st2common/util/concurrency.py b/st2common/st2common/util/concurrency.py index 62544eec3a..2735b8aa07 100644 --- a/st2common/st2common/util/concurrency.py +++ b/st2common/st2common/util/concurrency.py @@ -41,7 +41,8 @@ 'spawn', 'wait', 'cancel', - 'kill' + 'kill', + 'sleep' ] @@ -111,3 +112,12 @@ def kill(green_thread, *args, **kwargs): return green_thread.kill(*args, **kwargs) else: raise ValueError('Unsupported concurrency library') + + +def sleep(*args, **kwargs): + if CONCURRENCY_LIBRARY == 'eventlet': + return eventlet.sleep(*args, **kwargs) + elif CONCURRENCY_LIBRARY == 'gevent': + return gevent.sleep(*args, **kwargs) + else: + raise ValueError('Unsupported concurrency library') diff --git a/st2reactor/st2reactor/cmd/timersengine.py b/st2reactor/st2reactor/cmd/timersengine.py index 71d3fb29c5..b91dcaefcb 100644 --- a/st2reactor/st2reactor/cmd/timersengine.py +++ b/st2reactor/st2reactor/cmd/timersengine.py @@ -13,13 +13,14 @@ # limitations under the License. from __future__ import absolute_import + import os import sys -import eventlet from oslo_config import cfg from st2common import log as logging +from st2common.util import concurrency from st2common.constants.timer import TIMER_ENABLED_LOG_LINE, TIMER_DISABLED_LOG_LINE from st2common.logging.misc import get_logger_name_for_module from st2common.service_setup import setup as common_setup @@ -61,7 +62,7 @@ def _run_worker(): if cfg.CONF.timer.enable or cfg.CONF.timersengine.enable: local_tz = cfg.CONF.timer.local_timezone or cfg.CONF.timersengine.local_timezone timer = St2Timer(local_timezone=local_tz) - timer_thread = eventlet.spawn(_kickoff_timer, timer) + timer_thread = concurrency.spawn(_kickoff_timer, timer) LOG.info(TIMER_ENABLED_LOG_LINE) return timer_thread.wait() else: @@ -84,7 +85,7 @@ def main(): return _run_worker() except SystemExit as exit_code: sys.exit(exit_code) - except: + except Exception: LOG.exception('(PID=%s) TimerEngine quit due to exception.', os.getpid()) return 1 finally: diff --git a/st2reactor/st2reactor/container/manager.py b/st2reactor/st2reactor/container/manager.py index 917481ddd0..b655305a7b 100644 --- a/st2reactor/st2reactor/container/manager.py +++ b/st2reactor/st2reactor/container/manager.py @@ -13,12 +13,11 @@ # limitations under the License. from __future__ import absolute_import + import os import sys import signal -import eventlet - from st2common import log as logging from st2reactor.container.process_container import ProcessSensorContainer from st2common.services.sensor_watcher import SensorWatcher @@ -75,7 +74,7 @@ def _spin_container_and_wait(self, sensors): self._sensor_container = ProcessSensorContainer( sensors=sensors, single_sensor_mode=self._single_sensor_mode) - self._container_thread = eventlet.spawn(self._sensor_container.run) + self._container_thread = concurrency.spawn(self._sensor_container.run) LOG.debug('Starting sensor CUD watcher...') self._sensors_watcher.start() @@ -90,7 +89,7 @@ def _spin_container_and_wait(self, sensors): LOG.info('(PID:%s) SensorContainer stopped. Reason - %s', os.getpid(), sys.exc_info()[0].__name__) - eventlet.kill(self._container_thread) + concurrency.kill(self._container_thread) self._container_thread = None return exit_code diff --git a/st2reactor/st2reactor/container/process_container.py b/st2reactor/st2reactor/container/process_container.py index 81fe9654ac..5eee551cf8 100644 --- a/st2reactor/st2reactor/container/process_container.py +++ b/st2reactor/st2reactor/container/process_container.py @@ -13,6 +13,7 @@ # limitations under the License. from __future__ import absolute_import + import os import sys import time @@ -22,11 +23,10 @@ from collections import defaultdict import six -import eventlet -from eventlet.support import greenlets as greenlet from oslo_config import cfg from st2common import log as logging +from st2common.util import concurrency from st2common.constants.error_messages import PACK_VIRTUALENV_DOESNT_EXIST from st2common.constants.error_messages import PACK_VIRTUALENV_USES_PYTHON3 from st2common.constants.system import API_URL_ENV_VARIABLE_NAME @@ -129,6 +129,8 @@ def __init__(self, sensors, poll_interval=5, single_sensor_mode=False, dispatche def run(self): self._run_all_sensors() + success_exception_cls = concurrency.get_greenlet_exit_exception_class() + try: while not self._stopped: # Poll for all running processes @@ -140,8 +142,8 @@ def run(self): else: LOG.debug('No active sensors') - eventlet.sleep(self._poll_interval) - except greenlet.GreenletExit: + concurrency.sleep(self._poll_interval) + except success_exception_cls: # This exception is thrown when sensor container manager # kills the thread which runs process container. Not sure # if this is the best thing to do. @@ -180,8 +182,8 @@ def _poll_sensors_for_results(self, sensor_ids): # Try to respawn a dead process (maybe it was a simple failure which can be # resolved with a restart) - eventlet.spawn_n(self._respawn_sensor, sensor_id=sensor_id, sensor=sensor, - exit_code=status) + concurrency.spawn(self._respawn_sensor, sensor_id=sensor_id, sensor=sensor, + exit_code=status) else: sensor_start_time = self._sensor_start_times[sensor_id] sensor_respawn_count = self._sensor_respawn_counts[sensor_id] @@ -434,7 +436,7 @@ def _respawn_sensor(self, sensor_id, sensor, exit_code): self._sensor_respawn_counts[sensor_id] += 1 sleep_delay = (SENSOR_RESPAWN_DELAY * self._sensor_respawn_counts[sensor_id]) - eventlet.sleep(sleep_delay) + concurrency.sleep(sleep_delay) try: self._spawn_sensor_process(sensor=sensor) diff --git a/st2reactor/st2reactor/garbage_collector/base.py b/st2reactor/st2reactor/garbage_collector/base.py index ae9c747b6f..c1d4c14ab1 100644 --- a/st2reactor/st2reactor/garbage_collector/base.py +++ b/st2reactor/st2reactor/garbage_collector/base.py @@ -23,11 +23,10 @@ import random import six -import eventlet -from eventlet.support import greenlets as greenlet from oslo_config import cfg from st2common import log as logging +from st2common.util import concurrency from st2common.constants.exit_codes import SUCCESS_EXIT_CODE from st2common.constants.exit_codes import FAILURE_EXIT_CODE from st2common.constants.garbage_collection import DEFAULT_COLLECTION_INTERVAL @@ -81,11 +80,13 @@ def run(self): # Wait a couple of seconds before performing initial collection to prevent thundering herd # effect when restarting multiple services at the same time jitter_seconds = random.uniform(0, 3) - eventlet.sleep(jitter_seconds) + concurrency.sleep(jitter_seconds) + + success_exception_cls = concurrency.get_greenlet_exit_exception_class() try: self._main_loop() - except greenlet.GreenletExit: + except success_exception_cls: self._running = False return SUCCESS_EXIT_CODE except Exception as e: @@ -111,7 +112,7 @@ def _main_loop(self): LOG.info('Sleeping for %s seconds before next garbage collection...' % (self._collection_interval)) - eventlet.sleep(self._collection_interval) + concurrency.sleep(self._collection_interval) def _validate_ttl_values(self): """ @@ -142,7 +143,7 @@ def _perform_garbage_collection(self): if self._action_executions_ttl and self._action_executions_ttl >= MINIMUM_TTL_DAYS: LOG.info(proc_message, obj_type) self._purge_action_executions() - eventlet.sleep(self._sleep_delay) + concurrency.sleep(self._sleep_delay) else: LOG.debug(skip_message, obj_type) @@ -151,7 +152,7 @@ def _perform_garbage_collection(self): self._action_executions_output_ttl >= MINIMUM_TTL_DAYS_EXECUTION_OUTPUT: LOG.info(proc_message, obj_type) self._purge_action_executions_output() - eventlet.sleep(self._sleep_delay) + concurrency.sleep(self._sleep_delay) else: LOG.debug(skip_message, obj_type) @@ -159,7 +160,7 @@ def _perform_garbage_collection(self): if self._trigger_instances_ttl and self._trigger_instances_ttl >= MINIMUM_TTL_DAYS: LOG.info(proc_message, obj_type) self._purge_trigger_instances() - eventlet.sleep(self._sleep_delay) + concurrency.sleep(self._sleep_delay) else: LOG.debug(skip_message, obj_type) @@ -167,7 +168,7 @@ def _perform_garbage_collection(self): if self._purge_inquiries: LOG.info(proc_message, obj_type) self._timeout_inquiries() - eventlet.sleep(self._sleep_delay) + concurrency.sleep(self._sleep_delay) else: LOG.debug(skip_message, obj_type) @@ -175,7 +176,7 @@ def _perform_garbage_collection(self): if self._workflow_execution_max_idle > 0: LOG.info(proc_message, obj_type) self._purge_orphaned_workflow_executions() - eventlet.sleep(self._sleep_delay) + concurrency.sleep(self._sleep_delay) else: LOG.debug(skip_message, obj_type) diff --git a/st2reactor/st2reactor/sensor/base.py b/st2reactor/st2reactor/sensor/base.py index 617ed565a6..f7350c06e3 100644 --- a/st2reactor/st2reactor/sensor/base.py +++ b/st2reactor/st2reactor/sensor/base.py @@ -13,10 +13,12 @@ # limitations under the License. from __future__ import absolute_import + import abc import six -import eventlet + +from st2common.util import concurrency __all__ = [ 'Sensor', @@ -117,7 +119,7 @@ def poll(self): def run(self): while True: self.poll() - eventlet.sleep(self._poll_interval) + concurrency.sleep(self._poll_interval) def get_poll_interval(self): """ From ee52f63bdb63786a9ebd6ff5719a91640feba0e7 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Sep 2019 12:25:52 +0200 Subject: [PATCH 06/75] Add missing change. --- st2common/st2common/util/concurrency.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/st2common/st2common/util/concurrency.py b/st2common/st2common/util/concurrency.py index 2735b8aa07..5314f280e8 100644 --- a/st2common/st2common/util/concurrency.py +++ b/st2common/st2common/util/concurrency.py @@ -121,3 +121,12 @@ def sleep(*args, **kwargs): return gevent.sleep(*args, **kwargs) else: raise ValueError('Unsupported concurrency library') + + +def get_greenlet_exit_exception_class(): + if CONCURRENCY_LIBRARY == 'eventlet': + return eventlet.support.greenlets.GreenletExit + elif CONCURRENCY_LIBRARY == 'gevent': + return gevent.GreenletExit + else: + raise ValueError('Unsupported concurrency library') From b1360e61ea72f34d9a9c6642e86ff681356fdc54 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Sep 2019 20:14:02 +0200 Subject: [PATCH 07/75] Add a missing import. --- st2reactor/st2reactor/container/manager.py | 1 + 1 file changed, 1 insertion(+) diff --git a/st2reactor/st2reactor/container/manager.py b/st2reactor/st2reactor/container/manager.py index b655305a7b..6942c78a81 100644 --- a/st2reactor/st2reactor/container/manager.py +++ b/st2reactor/st2reactor/container/manager.py @@ -19,6 +19,7 @@ import signal from st2common import log as logging +from st2common.util import concurrency from st2reactor.container.process_container import ProcessSensorContainer from st2common.services.sensor_watcher import SensorWatcher from st2common.models.system.common import ResourceReference From e8e8e4902989e53b437f560aef3b306ad7b5b684 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Sep 2019 20:15:24 +0200 Subject: [PATCH 08/75] Add missing function to __all__. --- st2common/st2common/util/concurrency.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/util/concurrency.py b/st2common/st2common/util/concurrency.py index 5314f280e8..c40d2c311d 100644 --- a/st2common/st2common/util/concurrency.py +++ b/st2common/st2common/util/concurrency.py @@ -42,7 +42,8 @@ 'wait', 'cancel', 'kill', - 'sleep' + 'sleep', + 'get_greenlet_exit_exception_class' ] From 1b343eefd43c370b7786e5e91bbffc4eae071c8e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 19 Sep 2019 20:20:59 +0200 Subject: [PATCH 09/75] Also update test code in st2reactor to utilize our concurrency wrapper. --- .../integration/test_garbage_collector.py | 9 ++++---- .../tests/integration/test_rules_engine.py | 4 ++-- .../integration/test_sensor_container.py | 22 +++++++++---------- .../tests/integration/test_sensor_watcher.py | 5 +++-- .../tests/unit/test_process_container.py | 6 ++--- 5 files changed, 23 insertions(+), 23 deletions(-) diff --git a/st2reactor/tests/integration/test_garbage_collector.py b/st2reactor/tests/integration/test_garbage_collector.py index d91d8016f2..f321025911 100644 --- a/st2reactor/tests/integration/test_garbage_collector.py +++ b/st2reactor/tests/integration/test_garbage_collector.py @@ -19,9 +19,7 @@ import signal import datetime -import eventlet -from eventlet.green import subprocess - +from st2common.util import concurrency from st2common.constants import action as action_constants from st2common.util import date as date_utils from st2common.models.db.execution import ActionExecutionDB @@ -187,7 +185,7 @@ def test_garbage_collection(self): process = self._start_garbage_collector() # Give it some time to perform garbage collection and kill it - eventlet.sleep(15) + concurrency.sleep(15) process.send_signal(signal.SIGKILL) self.remove_process(process=process) @@ -235,7 +233,7 @@ def test_inquiry_garbage_collection(self): process = self._start_garbage_collector() # Give it some time to perform garbage collection and kill it - eventlet.sleep(15) + concurrency.sleep(15) process.send_signal(signal.SIGKILL) self.remove_process(process=process) @@ -254,6 +252,7 @@ def _create_inquiry(self, ttl, timestamp): executions.create_execution_object(liveaction_db) def _start_garbage_collector(self): + subprocess = concurrency.get_subprocess_module() process = subprocess.Popen(CMD_INQUIRY, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False, preexec_fn=os.setsid) self.add_process(process=process) diff --git a/st2reactor/tests/integration/test_rules_engine.py b/st2reactor/tests/integration/test_rules_engine.py index f61709e66c..ab7526925c 100644 --- a/st2reactor/tests/integration/test_rules_engine.py +++ b/st2reactor/tests/integration/test_rules_engine.py @@ -19,8 +19,7 @@ import signal import tempfile -from eventlet.green import subprocess - +from st2common.util import concurrency from st2common.constants.timer import TIMER_ENABLED_LOG_LINE from st2common.constants.timer import TIMER_DISABLED_LOG_LINE from st2tests.base import IntegrationTestCase @@ -134,6 +133,7 @@ def test_timer_disable_explicit(self): (TIMER_DISABLED_LOG_LINE)) def _start_times_engine(self, cmd): + subprocess = concurrency.get_subprocess_module() process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False, preexec_fn=os.setsid) self.add_process(process=process) diff --git a/st2reactor/tests/integration/test_sensor_container.py b/st2reactor/tests/integration/test_sensor_container.py index 018a825ac1..d3e7c04d2f 100644 --- a/st2reactor/tests/integration/test_sensor_container.py +++ b/st2reactor/tests/integration/test_sensor_container.py @@ -19,11 +19,10 @@ import signal import psutil -import eventlet -from eventlet.green import subprocess from oslo_config import cfg import st2tests.config +from st2common.util import concurrency from st2common.models.db import db_setup from st2reactor.container.process_container import PROCESS_EXIT_TIMEOUT from st2common.util.green.shell import run_command @@ -94,7 +93,7 @@ def test_child_processes_are_killed_on_sigint(self): process = self._start_sensor_container() # Give it some time to start up - eventlet.sleep(5) + concurrency.sleep(5) # Assert process has started and is running self.assertProcessIsRunning(process=process) @@ -110,7 +109,7 @@ def test_child_processes_are_killed_on_sigint(self): # SIGINT causes graceful shutdown so give it some time to gracefuly shut down the sensor # child processes - eventlet.sleep(PROCESS_EXIT_TIMEOUT + 1) + concurrency.sleep(PROCESS_EXIT_TIMEOUT + 1) # Verify parent and children processes have exited self.assertProcessExited(proc=pp) @@ -122,7 +121,7 @@ def test_child_processes_are_killed_on_sigterm(self): process = self._start_sensor_container() # Give it some time to start up - eventlet.sleep(3) + concurrency.sleep(3) # Verify container process and children sensor / wrapper processes are running pp = psutil.Process(process.pid) @@ -135,7 +134,7 @@ def test_child_processes_are_killed_on_sigterm(self): # SIGTERM causes graceful shutdown so give it some time to gracefuly shut down the sensor # child processes - eventlet.sleep(PROCESS_EXIT_TIMEOUT + 5) + concurrency.sleep(PROCESS_EXIT_TIMEOUT + 5) # Verify parent and children processes have exited self.assertProcessExited(proc=pp) @@ -147,7 +146,7 @@ def test_child_processes_are_killed_on_sigkill(self): process = self._start_sensor_container() # Give it some time to start up - eventlet.sleep(4) + concurrency.sleep(4) # Verify container process and children sensor / wrapper processes are running pp = psutil.Process(process.pid) @@ -159,7 +158,7 @@ def test_child_processes_are_killed_on_sigkill(self): process.send_signal(signal.SIGKILL) # Note: On SIGKILL processes should be killed instantly - eventlet.sleep(1) + concurrency.sleep(1) # Verify parent and children processes have exited self.assertProcessExited(proc=pp) @@ -175,7 +174,7 @@ def test_single_sensor_mode(self): pp = psutil.Process(process.pid) # Give it some time to start up - eventlet.sleep(4) + concurrency.sleep(4) stdout = process.stdout.read() self.assertTrue((b'--sensor-ref argument must be provided when running in single sensor ' @@ -191,7 +190,7 @@ def test_single_sensor_mode(self): pp = psutil.Process(process.pid) # Give it some time to start up - eventlet.sleep(8) + concurrency.sleep(8) # Container should exit and not respawn a sensor in single sensor mode stdout = process.stdout.read() @@ -200,12 +199,13 @@ def test_single_sensor_mode(self): self.assertTrue(b'Not respawning a sensor since running in single sensor mode') self.assertTrue(b'Process container quit with exit_code 110.') - eventlet.sleep(2) + concurrency.sleep(2) self.assertProcessExited(proc=pp) self.remove_process(process=process) def _start_sensor_container(self, cmd=DEFAULT_CMD): + subprocess = concurrency.get_subprocess_module() process = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False, preexec_fn=os.setsid) self.add_process(process=process) diff --git a/st2reactor/tests/integration/test_sensor_watcher.py b/st2reactor/tests/integration/test_sensor_watcher.py index 59ec452ce5..7a33cbb97e 100644 --- a/st2reactor/tests/integration/test_sensor_watcher.py +++ b/st2reactor/tests/integration/test_sensor_watcher.py @@ -13,10 +13,11 @@ # limitations under the License. from __future__ import absolute_import -import eventlet + from monotonic import monotonic from pyrabbit.api import Client +from st2common.util import concurrency from st2common.services.sensor_watcher import SensorWatcher from st2tests.base import IntegrationTestCase @@ -50,7 +51,7 @@ def delete_handler(sensor_db): start = monotonic() done = False while not done: - eventlet.sleep(0.01) + concurrency.sleep(0.01) sw_queues = self._get_sensor_watcher_amqp_queues(queue_name='st2.sensor.watch.covfefe') done = len(sw_queues) > 0 or ((monotonic() - start) < 5) diff --git a/st2reactor/tests/unit/test_process_container.py b/st2reactor/tests/unit/test_process_container.py index f353a0719b..f36b5277ea 100644 --- a/st2reactor/tests/unit/test_process_container.py +++ b/st2reactor/tests/unit/test_process_container.py @@ -16,11 +16,11 @@ import os import time -import eventlet from mock import (MagicMock, Mock, patch) import unittest2 from st2reactor.container.process_container import ProcessSensorContainer +from st2common.util import concurrency from st2common.models.db.pack import PackDB from st2common.persistence.pack import Pack @@ -35,8 +35,8 @@ class ProcessContainerTests(unittest2.TestCase): def test_no_sensors_dont_quit(self): process_container = ProcessSensorContainer(None, poll_interval=0.1) - process_container_thread = eventlet.spawn(process_container.run) - eventlet.sleep(0.5) + process_container_thread = concurrency.spawn(process_container.run) + concurrency.sleep(0.5) self.assertEqual(process_container.running(), 0) self.assertEqual(process_container.stopped(), False) process_container.shutdown() From 6478e3d31ffc3a4ae5ad7a3eafad64c8875733ae Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 23 Sep 2019 18:04:52 +0200 Subject: [PATCH 10/75] Update st2common.log.setup function so it takes new "st2_config_path" and supports logging config paths which are relative to st2.conf file directory. --- st2common/st2common/log.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/log.py b/st2common/st2common/log.py index cf1347ea81..15c25002a5 100644 --- a/st2common/st2common/log.py +++ b/st2common/st2common/log.py @@ -181,10 +181,22 @@ def _redirect_stderr(): sys.stderr = LoggingStream('STDERR') -def setup(config_file, redirect_stderr=True, excludes=None, disable_existing_loggers=False): +def setup(config_file, redirect_stderr=True, excludes=None, disable_existing_loggers=False, + st2_conf_path=None): """ Configure logging from file. + + :param st2_conf_path: Optional path to st2.conf file. If provided and "config_file" path is + relative to st2.conf path, the config_file path will get resolved to full + absolute path relative to st2.conf. + :type st2_conf_path: ``str`` """ + if st2_conf_path and config_file[:2] == './' and not os.path.isfile(config_file): + # Logging config path is relative to st2.conf, resolve it to full absolute path + directory = os.path.dirname(st2_conf_path) + config_file_name = os.path.basename(config_file) + config_file = os.path.join(directory, config_file_name) + try: logging.config.fileConfig(config_file, defaults=None, From b5780b8ac29fd0e82af5929e821ce23d3a07ee2a Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 24 Sep 2019 10:50:59 +0200 Subject: [PATCH 11/75] Allow user to pass "wrapper_script_path" and "create_token" arguments to the ProcessSensorContainer class. --- .../st2reactor/container/process_container.py | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/st2reactor/st2reactor/container/process_container.py b/st2reactor/st2reactor/container/process_container.py index 5eee551cf8..6ac36dd113 100644 --- a/st2reactor/st2reactor/container/process_container.py +++ b/st2reactor/st2reactor/container/process_container.py @@ -79,16 +79,26 @@ class ProcessSensorContainer(object): Sensor container which runs sensors in a separate process. """ - def __init__(self, sensors, poll_interval=5, single_sensor_mode=False, dispatcher=None): + def __init__(self, sensors, poll_interval=5, single_sensor_mode=False, dispatcher=None, + wrapper_script_path=WRAPPER_SCRIPT_PATH, create_token=True): """ :param sensors: A list of sensor dicts. :type sensors: ``list`` of ``dict`` :param poll_interval: How long to sleep between each poll for running / dead sensors. :type poll_interval: ``float`` + + :param wrapper_script_path: Path to the sensor wrapper script. + :type wrapper_script_path: ``str`` + + :param create_token: True to create temporary authentication token for the purpose for each + sensor process and add it to that process environment variables. + :type create_token: ``bool`` """ self._poll_interval = poll_interval self._single_sensor_mode = single_sensor_mode + self._wrapper_script_path = wrapper_script_path + self._create_token = create_token if self._single_sensor_mode: # For more immediate feedback we use lower poll interval when running in single sensor @@ -98,9 +108,7 @@ def __init__(self, sensors, poll_interval=5, single_sensor_mode=False, dispatche self._sensors = {} # maps sensor_id -> sensor object self._processes = {} # maps sensor_id -> sensor process - if not dispatcher: - dispatcher = TriggerDispatcher(LOG) - self._dispatcher = dispatcher + self._dispatcher = dispatcher or TriggerDispatcher(LOG) self._stopped = False self._exit_code = None # exit code with which this process should exit @@ -303,7 +311,7 @@ def _spawn_sensor_process(self, sensor): args = [ python_path, - WRAPPER_SCRIPT_PATH, + self._wrapper_script_path, '--pack=%s' % (sensor['pack']), '--file-path=%s' % (sensor['file_path']), '--class-name=%s' % (sensor['class_name']), @@ -329,22 +337,26 @@ def _spawn_sensor_process(self, sensor): else: env['PYTHONPATH'] = sandbox_python_path - # Include full api URL and API token specific to that sensor - ttl = cfg.CONF.auth.service_token_ttl - metadata = { - 'service': 'sensors_container', - 'sensor_path': sensor['file_path'], - 'sensor_class': sensor['class_name'] - } - temporary_token = create_token(username='sensors_container', ttl=ttl, metadata=metadata, - service=True) + if self._create_token: + # Include full api URL and API token specific to that sensor + LOG.debug('Creating temporary auth token for sensor %s' % (sensor['class_name'])) + + ttl = cfg.CONF.auth.service_token_ttl + metadata = { + 'service': 'sensors_container', + 'sensor_path': sensor['file_path'], + 'sensor_class': sensor['class_name'] + } + temporary_token = create_token(username='sensors_container', ttl=ttl, metadata=metadata, + service=True) + + env[API_URL_ENV_VARIABLE_NAME] = get_full_public_api_url() + env[AUTH_TOKEN_ENV_VARIABLE_NAME] = temporary_token.token - env[API_URL_ENV_VARIABLE_NAME] = get_full_public_api_url() - env[AUTH_TOKEN_ENV_VARIABLE_NAME] = temporary_token.token + # TODO 1: Purge temporary token when service stops or sensor process dies + # TODO 2: Store metadata (wrapper process id) with the token and delete + # tokens for old, dead processes on startup - # TODO 1: Purge temporary token when service stops or sensor process dies - # TODO 2: Store metadata (wrapper process id) with the token and delete - # tokens for old, dead processes on startup cmd = ' '.join(args) LOG.debug('Running sensor subprocess (cmd="%s")', cmd) From cebeccc3cd9da2f5f6b6a3a2975a13fdf4f9bf9c Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 24 Sep 2019 17:14:13 +0200 Subject: [PATCH 12/75] Move code for setting CLI arguments which are passed to the wrapper script to a method which can be overriden in derived classes. --- .../st2reactor/container/process_container.py | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/st2reactor/st2reactor/container/process_container.py b/st2reactor/st2reactor/container/process_container.py index 6ac36dd113..592447b811 100644 --- a/st2reactor/st2reactor/container/process_container.py +++ b/st2reactor/st2reactor/container/process_container.py @@ -304,26 +304,7 @@ def _spawn_sensor_process(self, sensor): msg = PACK_VIRTUALENV_USES_PYTHON3 % format_values raise Exception(msg) - trigger_type_refs = sensor['trigger_types'] or [] - trigger_type_refs = ','.join(trigger_type_refs) - - parent_args = json.dumps(sys.argv[1:]) - - args = [ - python_path, - self._wrapper_script_path, - '--pack=%s' % (sensor['pack']), - '--file-path=%s' % (sensor['file_path']), - '--class-name=%s' % (sensor['class_name']), - '--trigger-type-refs=%s' % (trigger_type_refs), - '--parent-args=%s' % (parent_args) - ] - - if sensor['poll_interval']: - args.append('--poll-interval=%s' % (sensor['poll_interval'])) - - sandbox_python_path = get_sandbox_python_path(inherit_from_parent=True, - inherit_parent_virtualenv=True) + args = self._get_args_for_wrapper_script(python_binary=python_path, sensor=sensor) if self._enable_common_pack_libs: pack_common_libs_path = get_pack_common_libs_path_for_pack_ref(pack_ref=pack_ref) @@ -332,6 +313,9 @@ def _spawn_sensor_process(self, sensor): env = os.environ.copy() + sandbox_python_path = get_sandbox_python_path(inherit_from_parent=True, + inherit_parent_virtualenv=True) + if self._enable_common_pack_libs and pack_common_libs_path: env['PYTHONPATH'] = pack_common_libs_path + ':' + sandbox_python_path else: @@ -473,6 +457,38 @@ def _should_respawn_sensor(self, sensor_id, sensor, exit_code): return True + def _get_args_for_wrapper_script(self, python_binary, sensor): + """ + Return CLI arguments passed to the sensor wrapper script. + + :param python_binary: Python binary used to execute wrapper script. + :type python_binary: ``str`` + + :param sensor: Sensor object dictionary. + :type sensor: ``dict`` + + :rtype: ``list`` + """ + trigger_type_refs = sensor['trigger_types'] or [] + trigger_type_refs = ','.join(trigger_type_refs) + + parent_args = json.dumps(sys.argv[1:]) + + args = [ + python_binary, + self._wrapper_script_path, + '--pack=%s' % (sensor['pack']), + '--file-path=%s' % (sensor['file_path']), + '--class-name=%s' % (sensor['class_name']), + '--trigger-type-refs=%s' % (trigger_type_refs), + '--parent-args=%s' % (parent_args) + ] + + if sensor['poll_interval']: + args.append('--poll-interval=%s' % (sensor['poll_interval'])) + + return args + def _get_sensor_id(self, sensor): """ Return unique identifier for the provider sensor dict. From eaa00767fa3d9cf549630ad03a43bb8b808c9702 Mon Sep 17 00:00:00 2001 From: Tristan Struthers Date: Tue, 24 Sep 2019 12:48:09 -0700 Subject: [PATCH 13/75] Update CHANGELOG to reflect new feature --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1aa027c819..a8e3c5a016 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -11,6 +11,7 @@ Added ``url_hosts_blacklist`` and ``url_hosts_whitelist`` runner attribute. (new feature) #4757 * Add ``user`` parameter to ``re_run`` method of st2client. #4785 +* Add ``get_entrypoint()`` method to ``ActionResourceManager`` attribute of st2client. #4791 Changed ~~~~~~~ From fcf6fe09abcb448ec6071f5a92b1d44621fb2a49 Mon Sep 17 00:00:00 2001 From: Tristan Struthers Date: Tue, 24 Sep 2019 12:48:33 -0700 Subject: [PATCH 14/75] Add unit tests for the new ActionResourceManager methods --- st2client/tests/unit/test_client_actions.py | 88 +++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 st2client/tests/unit/test_client_actions.py diff --git a/st2client/tests/unit/test_client_actions.py b/st2client/tests/unit/test_client_actions.py new file mode 100644 index 0000000000..b2d1f39d82 --- /dev/null +++ b/st2client/tests/unit/test_client_actions.py @@ -0,0 +1,88 @@ +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import json +import logging +import mock +import unittest2 + +from tests import base + +from st2client import client +from st2client.utils import httpclient + + +LOG = logging.getLogger(__name__) + + +EXECUTION = { + "id": 12345, + "action": { + "ref": "mock.foobar" + }, + "status": "failed", + "result": "non-empty" +} + +ENTRYPOINT = ( + "version: 1.0" + + "description: A basic workflow that runs an arbitrary linux command." + + "input:" + " - cmd" + " - timeout" + + "tasks:" + " task1:" + " action: core.local cmd=<% ctx(cmd) %> timeout=<% ctx(timeout) %>" + " next:" + " - when: <% succeeded() %>" + " publish:" + " - stdout: <% result().stdout %>" + " - stderr: <% result().stderr %>" + + "output:" + " - stdout: <% ctx(stdout) %>" +) + + +class TestActionResourceManager(unittest2.TestCase): + + @classmethod + def setUpClass(cls): + super(TestActionResourceManager, cls).setUpClass() + cls.client = client.Client() + + @mock.patch.object( + httpclient.HTTPClient, 'get', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(ENTRYPOINT), 200, 'OK'))) + def test_get_action_entry_point_by_ref(self): + self.client.actions.get_entrypoint(EXECUTION['action']['ref']) + + endpoint = '/actions/views/entry_point/%s' % EXECUTION['action']['ref'] + + httpclient.HTTPClient.get.assert_called_with(endpoint) + + @mock.patch.object( + httpclient.HTTPClient, 'get', + mock.MagicMock(return_value=base.FakeResponse(json.dumps(ENTRYPOINT), 200, 'OK'))) + def test_get_action_entry_point_by_id(self): + self.client.actions.get_entrypoint(EXECUTION['id']) + + endpoint = '/actions/views/entry_point/%s' % EXECUTION['id'] + + httpclient.HTTPClient.get.assert_called_with(endpoint) From 622fbfc49bb235e86acc468283d4de05d279cf4c Mon Sep 17 00:00:00 2001 From: Tristan Struthers Date: Tue, 24 Sep 2019 15:20:13 -0700 Subject: [PATCH 15/75] Assert correctness of return value and add 404 test --- st2client/tests/unit/test_client_actions.py | 22 ++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/st2client/tests/unit/test_client_actions.py b/st2client/tests/unit/test_client_actions.py index b2d1f39d82..ac7c3b191a 100644 --- a/st2client/tests/unit/test_client_actions.py +++ b/st2client/tests/unit/test_client_actions.py @@ -71,18 +71,34 @@ def setUpClass(cls): httpclient.HTTPClient, 'get', mock.MagicMock(return_value=base.FakeResponse(json.dumps(ENTRYPOINT), 200, 'OK'))) def test_get_action_entry_point_by_ref(self): - self.client.actions.get_entrypoint(EXECUTION['action']['ref']) + actual_entrypoint = self.client.actions.get_entrypoint(EXECUTION['action']['ref']) + actual_entrypoint = json.loads(actual_entrypoint) endpoint = '/actions/views/entry_point/%s' % EXECUTION['action']['ref'] - httpclient.HTTPClient.get.assert_called_with(endpoint) + self.assertEqual(ENTRYPOINT, actual_entrypoint) + @mock.patch.object( httpclient.HTTPClient, 'get', mock.MagicMock(return_value=base.FakeResponse(json.dumps(ENTRYPOINT), 200, 'OK'))) def test_get_action_entry_point_by_id(self): - self.client.actions.get_entrypoint(EXECUTION['id']) + actual_entrypoint = self.client.actions.get_entrypoint(EXECUTION['id']) + actual_entrypoint = json.loads(actual_entrypoint) endpoint = '/actions/views/entry_point/%s' % EXECUTION['id'] + httpclient.HTTPClient.get.assert_called_with(endpoint) + self.assertEqual(ENTRYPOINT, actual_entrypoint) + + @mock.patch.object( + httpclient.HTTPClient, 'get', + mock.MagicMock(return_value=base.FakeResponse( + json.dumps({}), 404, '404 Client Error: Not Found' + ))) + def test_get_non_existent_action_entry_point(self): + with self.assertRaises(Exception): + self.client.actions.get_entrypoint('nonexistentpack.nonexistentaction') + + endpoint = '/actions/views/entry_point/%s' % 'nonexistentpack.nonexistentaction' httpclient.HTTPClient.get.assert_called_with(endpoint) From 150aafe3467e89ade52af9b6897f332b8b72cae0 Mon Sep 17 00:00:00 2001 From: Tristan Struthers Date: Tue, 1 Oct 2019 18:50:56 -0700 Subject: [PATCH 16/75] Use assertRaisesRegexp --- st2client/tests/unit/test_client_actions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/tests/unit/test_client_actions.py b/st2client/tests/unit/test_client_actions.py index ac7c3b191a..5a5c798360 100644 --- a/st2client/tests/unit/test_client_actions.py +++ b/st2client/tests/unit/test_client_actions.py @@ -97,7 +97,7 @@ def test_get_action_entry_point_by_id(self): json.dumps({}), 404, '404 Client Error: Not Found' ))) def test_get_non_existent_action_entry_point(self): - with self.assertRaises(Exception): + with self.assertRaisesRegexp(Exception, '404 Client Error: Not Found'): self.client.actions.get_entrypoint('nonexistentpack.nonexistentaction') endpoint = '/actions/views/entry_point/%s' % 'nonexistentpack.nonexistentaction' From 4f7f7a1d7ca8fb9456cacbd795aff9582f14bb3b Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 14:51:42 -0300 Subject: [PATCH 17/75] Add tests when secrets masking works correctly when ?include_attributes and ?exclude_attributes --- st2api/tests/unit/controllers/v1/test_rule_views.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 7f98da158f..88c81f808a 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,6 +86,16 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) + def test_get_one_parameters_mask_with_include_parameters(self): + resp = self.app.get('/api/v1/rules?include_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + pass + + def test_get_one_parameters_mask_with_exclude_parameters(self): + resp = self.app.get('/api/v1/rules?exclude_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + pass + def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] return rule_ids From 36980e52f62780216d2a5f5764757946451a33fa Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 14:57:44 -0300 Subject: [PATCH 18/75] Change name method test_get_one_parameters_mask_with_include_parameters for test_get_all_parameters_mask_with_include_parameters and test_get_one_parameters_mask_with_exclude_parameters for test_get_all_parameters_mask_with_exclude_parameters --- st2api/tests/unit/controllers/v1/test_rule_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 88c81f808a..8236a7fa2d 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,12 +86,12 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) - def test_get_one_parameters_mask_with_include_parameters(self): + def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) pass - def test_get_one_parameters_mask_with_exclude_parameters(self): + def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) pass From 6b9eb4eb921628246f3e24e3e3961dda95170b12 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 15:11:32 -0300 Subject: [PATCH 19/75] add assertequal for length of json --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 8236a7fa2d..caa0c97352 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -89,11 +89,13 @@ def test_get_one_fail(self): def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) + self.assertEqual(len(resp.json), 3) pass def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) + self.assertEqual(len(resp.json), 3) pass def _insert_mock_models(self): From 42eb3937a86f876892f166836d093da640532349 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 9 Oct 2019 15:36:17 -0300 Subject: [PATCH 20/75] Add test model test_rule_with_secret_parameter_masked --- .../tests/unit/controllers/v1/test_rule_views.py | 2 -- st2common/tests/unit/test_db.py | 14 +++++++++++++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index caa0c97352..82f23061f4 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -90,13 +90,11 @@ def test_get_all_parameters_mask_with_include_parameters(self): resp = self.app.get('/api/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - pass def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/api/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - pass def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 8db54e7cf4..2ddea6f009 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -441,6 +441,17 @@ def test_trigger_lookup(self): 'Incorrect trigger returned.') ReactorModelTestCase._delete([saved, triggertype]) + def test_rule_with_secret_parameter_masked(self): + triggertype = ReactorModelTestCase._create_save_triggertype() + trigger = ReactorModelTestCase._create_save_trigger(triggertype) + runnertype = ActionModelTestCase._create_save_runnertype() + action = ActionModelTestCase._create_save_action(runnertype) + saved = ReactorModelTestCase._create_save_rule(trigger, action, False) + masked = RuleDB._mask_secrets(saved.action.) + + for value in masked['parameters']['p4'].values(): + self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', @@ -666,7 +677,8 @@ def _create_save_action(runnertype, metadata=False): created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False} + 'p3': {'type': 'boolean', 'default': False}, + 'p4': {'type': 'secret', 'default': "*****"} } return Action.add_or_update(created) From 76cd651a737c681b5fa4c036cdfcc0a518ee7fcd Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 16:00:06 -0300 Subject: [PATCH 21/75] Fix syntax error --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 2ddea6f009..68f1bbc93a 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action.) + masked = RuleDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) From 5defcdab528b76110169a46a4a99ce455aea5a4e Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 16:14:23 -0300 Subject: [PATCH 22/75] Fix api path --- st2api/tests/unit/controllers/v1/test_rule_views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 82f23061f4..ab4b54529f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -87,12 +87,12 @@ def test_get_one_fail(self): self.assertEqual(resp.status_int, http_client.NOT_FOUND) def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/api/v1/rules?include_attributes=action') + resp = self.app.get('/v1/rules?include_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) def test_get_all_parameters_mask_with_exclude_parameters(self): - resp = self.app.get('/api/v1/rules?exclude_attributes=action') + resp = self.app.get('/v1/rules?exclude_attributes=action') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) From 421321dac9a31bb216a35a659f4517a12af938c7 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:29:49 -0300 Subject: [PATCH 23/75] Add default value p4 parameter --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 68f1bbc93a..b25347ed99 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -672,7 +672,7 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None} + created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4' : None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, From 0cc4cb52e6d359044e22a16f33220d5537e0fce2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:44:39 -0300 Subject: [PATCH 24/75] Include import constant --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index b25347ed99..09010f90e0 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -21,7 +21,7 @@ import mongoengine.connection from oslo_config import cfg from pymongo.errors import ConnectionFailure - +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher From c07522aebd5dc066d48d01bc04651c49bd18c4e3 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 17:56:23 -0300 Subject: [PATCH 25/75] Fix whitespace --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 09010f90e0..616bdb77ce 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -672,7 +672,7 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4' : None} + created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, From 8b019496306b5c70442993c5ff43f3b6944c0e5d Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 18:34:17 -0300 Subject: [PATCH 26/75] Fix delete before finish test method --- st2common/tests/unit/test_db.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 616bdb77ce..af0550efef 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,10 +447,12 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action) + masked = RuleDB._mask_secrets(retrieved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) + ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) + @staticmethod def _create_save_triggertype(): From 9f3feaf1bf1937fa884f6a3ba55c580920d060b3 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 18:50:40 -0300 Subject: [PATCH 27/75] Fix parameters --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index af0550efef..aa70784f24 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -680,7 +680,7 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'secret', 'default': "*****"} + 'p4': {'type': 'string', 'default': '******', 'secret': True} } return Action.add_or_update(created) From 7ff661ea41bd82a45c17f111c381a7652c45bd76 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:01:42 -0300 Subject: [PATCH 28/75] Fix parameters --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index aa70784f24..baa091b07c 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(retrieved.action) + masked = RuleDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) From 4182f4bb5b622178e5d78219fdca13ec95d4823d Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:11:46 -0300 Subject: [PATCH 29/75] Remove blank lines --- st2common/tests/unit/test_db.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index baa091b07c..2d3768a469 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -448,12 +448,10 @@ def test_rule_with_secret_parameter_masked(self): action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) masked = RuleDB._mask_secrets(saved.action) - for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) - @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', From 46dd3fa05e0ad059ef55f4652dcf2af9f6c74365 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 10 Oct 2019 19:28:28 -0300 Subject: [PATCH 30/75] Simple string in secret true parameter --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 2d3768a469..b2f9291f18 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -678,7 +678,7 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'string', 'default': '******', 'secret': True} + 'p4': {'type': 'string', 'default': 'secret', 'secret': True} } return Action.add_or_update(created) From 7ad78a9b2a576ca7a3c4cf79917ecbe0a6354db2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:08:34 -0300 Subject: [PATCH 31/75] Add import RuleTypeDB --- st2common/tests/unit/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index b2f9291f18..4ea51c0eeb 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -34,7 +34,7 @@ from st2common.models.db.trigger import TriggerTypeDB, TriggerDB, TriggerInstanceDB from st2common.models.db.rule import RuleDB, ActionExecutionSpecDB from st2common.persistence.cleanup import db_cleanup -from st2common.persistence.rule import Rule +from st2common.persistence.rule import Rule, RuleTypeDB from st2common.persistence.trigger import TriggerType, Trigger, TriggerInstance from st2tests import DbTestCase @@ -447,7 +447,7 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleDB._mask_secrets(saved.action) + masked = RuleTypeDB._mask_secrets(saved.action) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) From 4c0fd03786da65cfde1eba4d1cdf70d29c8d1900 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:33:01 -0300 Subject: [PATCH 32/75] Fix test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 4ea51c0eeb..c80f9cb44d 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -34,7 +34,7 @@ from st2common.models.db.trigger import TriggerTypeDB, TriggerDB, TriggerInstanceDB from st2common.models.db.rule import RuleDB, ActionExecutionSpecDB from st2common.persistence.cleanup import db_cleanup -from st2common.persistence.rule import Rule, RuleTypeDB +from st2common.persistence.rule import Rule from st2common.persistence.trigger import TriggerType, Trigger, TriggerInstance from st2tests import DbTestCase @@ -447,10 +447,10 @@ def test_rule_with_secret_parameter_masked(self): runnertype = ActionModelTestCase._create_save_runnertype() action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - masked = RuleTypeDB._mask_secrets(saved.action) + retrieved = Rule.get_by_id(saved.id) for value in masked['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) - ReactorModelTestCase._delete([saved, trigger, action, runnertype, triggertype]) + ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) @staticmethod def _create_save_triggertype(): From 3271499cdbd030941b535c254f223895a8bd547b Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 10:43:17 -0300 Subject: [PATCH 33/75] Fix test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index c80f9cb44d..e927a2368a 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -448,7 +448,7 @@ def test_rule_with_secret_parameter_masked(self): action = ActionModelTestCase._create_save_action(runnertype) saved = ReactorModelTestCase._create_save_rule(trigger, action, False) retrieved = Rule.get_by_id(saved.id) - for value in masked['parameters']['p4'].values(): + for value in retrieved['parameters']['p4'].values(): self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) From 4ffaa6f9785ed904a411694db365abb1614a09d7 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:14:40 -0300 Subject: [PATCH 34/75] Remove test test_rule_with_secret_parameter_masked --- st2common/tests/unit/test_db.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index e927a2368a..d97e492855 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -441,17 +441,6 @@ def test_trigger_lookup(self): 'Incorrect trigger returned.') ReactorModelTestCase._delete([saved, triggertype]) - def test_rule_with_secret_parameter_masked(self): - triggertype = ReactorModelTestCase._create_save_triggertype() - trigger = ReactorModelTestCase._create_save_trigger(triggertype) - runnertype = ActionModelTestCase._create_save_runnertype() - action = ActionModelTestCase._create_save_action(runnertype) - saved = ReactorModelTestCase._create_save_rule(trigger, action, False) - retrieved = Rule.get_by_id(saved.id) - for value in retrieved['parameters']['p4'].values(): - self.assertEqual(value, MASKED_ATTRIBUTE_VALUE) - ReactorModelTestCase._delete([retrieved, trigger, action, runnertype, triggertype]) - @staticmethod def _create_save_triggertype(): created = TriggerTypeDB(pack='dummy_pack_1', name='triggertype-1', description='', From 4d7e552846dfd7c22793df6bc8041426c5c274ba Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:27:56 -0300 Subject: [PATCH 35/75] Remove parameters p4 --- st2common/tests/unit/test_db.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index d97e492855..ef0cbc9626 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -667,7 +667,6 @@ def _create_save_action(runnertype, metadata=False): 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, 'p3': {'type': 'boolean', 'default': False}, - 'p4': {'type': 'string', 'default': 'secret', 'secret': True} } return Action.add_or_update(created) From ee220c19cefea22fefed7d9a3d93eb5596797ff8 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:44:15 -0300 Subject: [PATCH 36/75] Fix syntax error --- st2common/tests/unit/test_db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index ef0cbc9626..8b281c305b 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -661,12 +661,12 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} + created.parameters = {'p1': None, 'p2': None, 'p3': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False}, + 'p3': {'type': 'boolean', 'default': False} } return Action.add_or_update(created) From b73e1c34737d2cec6f4c53cfce1284aac8912178 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 11:55:58 -0300 Subject: [PATCH 37/75] Remove unnecessary constant --- st2common/tests/unit/test_db.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index 8b281c305b..d7c72ae964 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -21,7 +21,6 @@ import mongoengine.connection from oslo_config import cfg from pymongo.errors import ConnectionFailure -from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.constants.triggers import TRIGGER_INSTANCE_PROCESSED from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher From a202c4bac50e90c52746dec3c92db92ee660d55e Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 14:16:15 -0300 Subject: [PATCH 38/75] Remove All tests test_get_all_parameters_mask_with_include_parameters and test_get_all_parameters_mask_with_exclude_parameters --- st2api/tests/unit/controllers/v1/test_rule_views.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index ab4b54529f..7f98da158f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -86,16 +86,6 @@ def test_get_one_fail(self): resp = self.app.get('/v1/rules/1', expect_errors=True) self.assertEqual(resp.status_int, http_client.NOT_FOUND) - def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/v1/rules?include_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) - self.assertEqual(len(resp.json), 3) - - def test_get_all_parameters_mask_with_exclude_parameters(self): - resp = self.app.get('/v1/rules?exclude_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) - self.assertEqual(len(resp.json), 3) - def _insert_mock_models(self): rule_ids = [rule['id'] for rule in self.rules.values()] return rule_ids From 1ca28547bea85e43097690dd4f50d1ada2191732 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 11 Oct 2019 14:55:17 -0300 Subject: [PATCH 39/75] Add tests with include_attribute and exclude_attributes --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 +- st2api/tests/unit/controllers/v1/test_rules.py | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index 7f98da158f..ae2305565d 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -63,7 +63,7 @@ def test_get_all(self): resp = self.app.get('/v1/rules/views') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - + def test_get_one_by_id(self): rule_id = str(RuleViewControllerTestCase.RULE_1.id) get_resp = self.__do_get_one(rule_id) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index ae8063095f..8c62996174 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -185,6 +185,14 @@ def test_get_all_enabled(self): self.__do_delete(self.__get_rule_id(post_resp_rule_1)) self.__do_delete(self.__get_rule_id(post_resp_rule_3)) + def test_get_all_parameters_mask_with_include_parameters(self): + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + + def test_get_all_parameters_mask_with_exclude_parameters(self): + resp = self.app.get('/v1/rules?exclude_attributes=action') + self.assertEqual(resp.status_int, http_client.OK) + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) @@ -466,7 +474,7 @@ def setUpClass(cls): cls.RULE_1 = cls.fixtures_loader.load_fixtures( fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - + def test_ref_count_trigger_increment(self): post_resp = self.__do_post(self.RULE_1) rule_1_id = self.__get_rule_id(post_resp) From a6440140a9744da64003a8079e744a0299b09bbe Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Wed, 16 Oct 2019 19:23:05 -0300 Subject: [PATCH 40/75] Model tests done, WIP API Tests --- st2api/st2api/controllers/v1/rule_views.py | 3 ++- st2api/st2api/controllers/v1/rules.py | 4 ++-- st2api/tests/unit/controllers/v1/test_rules.py | 18 ++++++++++++++---- st2common/st2common/models/db/rule.py | 5 ++--- st2common/tests/unit/test_db.py | 9 +++++++-- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/st2api/st2api/controllers/v1/rule_views.py b/st2api/st2api/controllers/v1/rule_views.py index c607dac0cb..04c1114bad 100644 --- a/st2api/st2api/controllers/v1/rule_views.py +++ b/st2api/st2api/controllers/v1/rule_views.py @@ -99,8 +99,9 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o return rules def get_one(self, ref_or_id, requester_user): + from_model_kwargs = {'mask_secrets': True} rule = self._get_one(ref_or_id, permission_type=PermissionType.RULE_VIEW, - requester_user=requester_user) + requester_user=requester_user, from_model_kwargs=from_model_kwargs) result = self._append_view_properties([rule.json])[0] rule.json = result return rule diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index d64fb73732..04215f2527 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -69,7 +69,7 @@ class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceCo def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, offset=0, limit=None, requester_user=None, **raw_filters): - from_model_kwargs = {'ignore_missing_trigger': True} + from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} return super(RuleController, self)._get_all(exclude_fields=exclude_attributes, include_fields=include_attributes, from_model_kwargs=from_model_kwargs, @@ -80,7 +80,7 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o requester_user=requester_user) def get_one(self, ref_or_id, requester_user): - from_model_kwargs = {'ignore_missing_trigger': True} + from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} return super(RuleController, self)._get_one(ref_or_id, from_model_kwargs=from_model_kwargs, requester_user=requester_user, permission_type=PermissionType.RULE_VIEW) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index 8c62996174..d52753f043 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -128,6 +128,11 @@ def setUpClass(cls): fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] + file_name = 'rule1.yaml' + RulesControllerTestCase.RULE_WITH_SECRET_PARAMETERS = RulesControllerTestCase.fixtures_loader.load_fixtures( + fixtures_pack=FIXTURES_PACK, + fixtures_dict={'rules': [file_name]})['rules'][file_name] + @classmethod def tearDownClass(cls): # Replace original configured value for trigger parameter validation @@ -186,13 +191,18 @@ def test_get_all_enabled(self): self.__do_delete(self.__get_rule_id(post_resp_rule_3)) def test_get_all_parameters_mask_with_include_parameters(self): - resp = self.app.get('/v1/rules?include_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + resp = self.app.get('/v1/rules?include-attributes=name') + self.assertEqual('name' in resp.json[0], True) + print(resp.json[0]) + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) resp = self.app.get('/v1/rules?exclude_attributes=action') - self.assertEqual(resp.status_int, http_client.OK) - + self.assertEqual('action' in resp.json[0], False) + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 809d3c2f5f..83e2365622 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -135,9 +135,8 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - - action_ref = rule.get('action', {}).get('ref', None) - + + action_ref = rule['action']['ref'] def ref_query_args(ref): return {'ref': ref} diff --git a/st2common/tests/unit/test_db.py b/st2common/tests/unit/test_db.py index d7c72ae964..462ab8fa3e 100644 --- a/st2common/tests/unit/test_db.py +++ b/st2common/tests/unit/test_db.py @@ -520,6 +520,10 @@ def _delete(model_objects): "p3": { "type": "boolean", "default": False + }, + "p4": { + "type": "string", + "secret": True } }, "additionalProperties": False @@ -660,12 +664,13 @@ def _create_save_action(runnertype, metadata=False): runner_type={'name': runnertype.name}) if not metadata: - created.parameters = {'p1': None, 'p2': None, 'p3': None} + created.parameters = {'p1': None, 'p2': None, 'p3': None, 'p4': None} else: created.parameters = { 'p1': {'type': 'string', 'required': True}, 'p2': {'type': 'number', 'default': 2868}, - 'p3': {'type': 'boolean', 'default': False} + 'p3': {'type': 'boolean', 'default': False}, + 'p4': {'type': 'string', 'secret': True} } return Action.add_or_update(created) From 8faa9aba1caf694b3723d55775370ab2f8be8b42 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 10:27:47 -0300 Subject: [PATCH 41/75] Fix get api rule include attribute --- st2api/tests/unit/controllers/v1/test_rules.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index d52753f043..f7b3d1a31b 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -192,9 +192,8 @@ def test_get_all_enabled(self): def test_get_all_parameters_mask_with_include_parameters(self): post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) - resp = self.app.get('/v1/rules?include-attributes=name') + resp = self.app.get('/v1/rules?include_attributes=name') self.assertEqual('name' in resp.json[0], True) - print(resp.json[0]) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): From a1b97c600e290807ea67b241a98bfb65718e8668 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:04:35 -0300 Subject: [PATCH 42/75] Include test key action in map rule --- st2common/st2common/models/db/rule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 83e2365622..d2a9dcae59 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -116,7 +116,8 @@ def mask_secrets(self, value): :rtype: ``dict`` """ result = copy.deepcopy(value) - + if('action' not in result): + return result action_db = self._get_referenced_models(rule=result) secret_parameters = get_secret_parameters(parameters=action_db['parameters']) From 07d4eb706a8b53ab21320e759365e251d5a254e0 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:27:44 -0300 Subject: [PATCH 43/75] Clean Lint errors --- st2api/tests/unit/controllers/v1/test_rule_views.py | 2 +- st2api/tests/unit/controllers/v1/test_rules.py | 9 ++------- st2common/st2common/models/db/rule.py | 1 - 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rule_views.py b/st2api/tests/unit/controllers/v1/test_rule_views.py index ae2305565d..7f98da158f 100644 --- a/st2api/tests/unit/controllers/v1/test_rule_views.py +++ b/st2api/tests/unit/controllers/v1/test_rule_views.py @@ -63,7 +63,7 @@ def test_get_all(self): resp = self.app.get('/v1/rules/views') self.assertEqual(resp.status_int, http_client.OK) self.assertEqual(len(resp.json), 3) - + def test_get_one_by_id(self): rule_id = str(RuleViewControllerTestCase.RULE_1.id) get_resp = self.__do_get_one(rule_id) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index f7b3d1a31b..6d5a84cfec 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -128,11 +128,6 @@ def setUpClass(cls): fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - file_name = 'rule1.yaml' - RulesControllerTestCase.RULE_WITH_SECRET_PARAMETERS = RulesControllerTestCase.fixtures_loader.load_fixtures( - fixtures_pack=FIXTURES_PACK, - fixtures_dict={'rules': [file_name]})['rules'][file_name] - @classmethod def tearDownClass(cls): # Replace original configured value for trigger parameter validation @@ -201,7 +196,7 @@ def test_get_all_parameters_mask_with_exclude_parameters(self): resp = self.app.get('/v1/rules?exclude_attributes=action') self.assertEqual('action' in resp.json[0], False) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) - + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) @@ -483,7 +478,7 @@ def setUpClass(cls): cls.RULE_1 = cls.fixtures_loader.load_fixtures( fixtures_pack=FIXTURES_PACK, fixtures_dict={'rules': [file_name]})['rules'][file_name] - + def test_ref_count_trigger_increment(self): post_resp = self.__do_post(self.RULE_1) rule_1_id = self.__get_rule_id(post_resp) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index d2a9dcae59..983dd38746 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -136,7 +136,6 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - action_ref = rule['action']['ref'] def ref_query_args(ref): return {'ref': ref} From b3a753131ed91f8bf94a8ff2872a05007ef8f883 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 11:41:55 -0300 Subject: [PATCH 44/75] Fix lin rule.py --- st2common/st2common/models/db/rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 983dd38746..e78ba58044 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,7 +137,7 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ action_ref = rule['action']['ref'] - def ref_query_args(ref): + def ref_query_args(ref): return {'ref': ref} action_db = self._get_entity(model_persistence=Action, From 3e5f0016e3ed5df3604f488b776190bb95c91c02 Mon Sep 17 00:00:00 2001 From: jdmeyer3 Date: Thu, 17 Oct 2019 10:11:14 -0600 Subject: [PATCH 45/75] fixing subprocess to use system buffer instead of being unbuffered --- st2common/st2common/util/green/shell.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/util/green/shell.py b/st2common/st2common/util/green/shell.py index fe286f07eb..0eda39b88a 100644 --- a/st2common/st2common/util/green/shell.py +++ b/st2common/st2common/util/green/shell.py @@ -107,7 +107,7 @@ def run_command(cmd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, # GreenPipe so it doesn't block LOG.debug('Creating subprocess.') process = concurrency.subprocess_popen(args=cmd, stdin=stdin, stdout=stdout, stderr=stderr, - env=env, cwd=cwd, shell=shell, preexec_fn=preexec_func) + env=env, cwd=cwd, shell=shell, preexec_fn=preexec_func, bufsize=-1) if read_stdout_func: LOG.debug('Spawning read_stdout_func function') From 8277c78fb8070bd59119a9e37ccff4e17f6b37bb Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 14:20:28 -0300 Subject: [PATCH 46/75] Fix test rule --- st2api/tests/unit/controllers/v1/test_rules.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index 6d5a84cfec..c6831393ab 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -187,8 +187,8 @@ def test_get_all_enabled(self): def test_get_all_parameters_mask_with_include_parameters(self): post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) - resp = self.app.get('/v1/rules?include_attributes=name') - self.assertEqual('name' in resp.json[0], True) + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual('action' in resp.json[0], True) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): From 61b8b58e583b47711fc810cb62016769c0f987c2 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 14:40:50 -0300 Subject: [PATCH 47/75] Fix Db rule --- st2common/st2common/models/db/rule.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index e78ba58044..983dd38746 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,7 +137,7 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ action_ref = rule['action']['ref'] - def ref_query_args(ref): + def ref_query_args(ref): return {'ref': ref} action_db = self._get_entity(model_persistence=Action, From f1c4de4e495e5bc1a3f5cdfe4741a88a763646e0 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 15:05:31 -0300 Subject: [PATCH 48/75] Fix method ref_query_args --- st2common/st2common/models/db/rule.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 983dd38746..ae8a1fe47e 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -139,7 +139,6 @@ def _get_referenced_models(self, rule): action_ref = rule['action']['ref'] def ref_query_args(ref): return {'ref': ref} - action_db = self._get_entity(model_persistence=Action, ref=action_ref, query_args=ref_query_args) From 8b85474f04e397715eadd001caaeb4734323ce57 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Thu, 17 Oct 2019 15:18:29 -0300 Subject: [PATCH 49/75] Fix Lint --- st2common/st2common/models/db/rule.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index ae8a1fe47e..f7b3c99a55 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -137,8 +137,10 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ action_ref = rule['action']['ref'] + def ref_query_args(ref): return {'ref': ref} + action_db = self._get_entity(model_persistence=Action, ref=action_ref, query_args=ref_query_args) From 4614d442ad418368fb2cfedb92cb328deb676115 Mon Sep 17 00:00:00 2001 From: Felipe Oliveira Date: Fri, 18 Oct 2019 17:48:39 -0300 Subject: [PATCH 50/75] Add changelog Added mask of rule action secret parameters #4788 --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1aa027c819..6845822aa9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -47,6 +47,11 @@ Fixed doesn't depend on internal pip API and so it works with latest pip version. (bug fix) #4750 * Fix dependency conflicts in pack CI runs: downgrade requests dependency back to 0.21.0, update internal dependencies and test expectations (amqp, pyyaml, prance, six) (bugfix) #4774 +* Fix mask of rule action secret parameters: + When we had an action on a rule with secret type parameters, the parameters were visible. This behavior + has been resolved in PR4788. + Contributed by @Nicodemos305 and @jeansfelix + 3.1.0 - June 27, 2019 --------------------- From 2bb212b0f6a977b99592e9a323ec0dd47ef722a7 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 21 Oct 2019 18:55:44 +0200 Subject: [PATCH 51/75] Allow user to specify additional options which are passed to "pip" command when installing pack dependencies into pack virtualenv using "pip" by seting action_runner.pip_opts config option. --- st2common/st2common/config.py | 4 ++++ st2common/st2common/util/virtualenvs.py | 13 ++++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 1b31b93e77..5ad2fe0971 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -374,6 +374,10 @@ def register_opts(ignore_errors=False): 'virtualenv_opts', default=['--system-site-packages'], help='List of virtualenv options to be passsed to "virtualenv" command that ' 'creates pack virtualenv.'), + cfg.ListOpt( + 'pip_opts', default=[], + help='List of pip options to be passed to "pip install" command when installing pack ' + 'dependencies into pack virtual environment.'), cfg.BoolOpt( 'stream_output', default=True, help='True to store and stream action output (stdout and stderr) in real-time.') diff --git a/st2common/st2common/util/virtualenvs.py b/st2common/st2common/util/virtualenvs.py index b1eacad31d..0ac1c5977c 100644 --- a/st2common/st2common/util/virtualenvs.py +++ b/st2common/st2common/util/virtualenvs.py @@ -158,7 +158,7 @@ def create_virtualenv(virtualenv_path, logger=None, include_pip=True, include_se python_binary = cfg.CONF.actionrunner.python_binary python3_binary = cfg.CONF.actionrunner.python3_binary virtualenv_binary = cfg.CONF.actionrunner.virtualenv_binary - virtualenv_opts = cfg.CONF.actionrunner.virtualenv_opts + virtualenv_opts = cfg.CONF.actionrunner.virtualenv_opts or [] if not os.path.isfile(python_binary): raise Exception('Python binary "%s" doesn\'t exist' % (python_binary)) @@ -237,6 +237,7 @@ def install_requirements(virtualenv_path, requirements_file_path, proxy_config=N """ logger = logger or LOG pip_path = os.path.join(virtualenv_path, 'bin/pip') + pip_opts = cfg.CONF.actionrunner.pip_opts or [] cmd = [pip_path] if proxy_config: @@ -253,7 +254,10 @@ def install_requirements(virtualenv_path, requirements_file_path, proxy_config=N if cert: cmd.extend(['--cert', cert]) - cmd.extend(['install', '-U', '-r', requirements_file_path]) + cmd.append('install') + cmd.extend(pip_opts) + cmd.extend(['-U', '-r', requirements_file_path]) + env = get_env_for_subprocess_command() logger.debug('Installing requirements from file %s with command %s.', @@ -278,6 +282,7 @@ def install_requirement(virtualenv_path, requirement, proxy_config=None, logger= """ logger = logger or LOG pip_path = os.path.join(virtualenv_path, 'bin/pip') + pip_opts = cfg.CONF.actionrunner.pip_opts or [] cmd = [pip_path] if proxy_config: @@ -294,7 +299,9 @@ def install_requirement(virtualenv_path, requirement, proxy_config=None, logger= if cert: cmd.extend(['--cert', cert]) - cmd.extend(['install', requirement]) + cmd.append('install') + cmd.extend(pip_opts) + cmd.extend([requirement]) env = get_env_for_subprocess_command() logger.debug('Installing requirement %s with command %s.', requirement, ' '.join(cmd)) From 0568f194a1bd3678debe8f952bc3260477e621b4 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 21 Oct 2019 19:11:04 +0200 Subject: [PATCH 52/75] Add changelog entry. --- CHANGELOG.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6d79ce87f8..729f24c59a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,6 +28,9 @@ Changed writing very large executions (executions with large results) to the database. #4767 * Improved development instructions in requirements.txt and dist_utils.py comment headers (improvement) #4774 +* Add new ``action_runner.pip_opts`` st2.conf config option which allows user to specify a list + of command line option which are passed to ``pip install`` command when installing pack + dependencies into a pack specific virtual environment. #4792 Fixed ~~~~~ From fba49352fe2fae4c976b955443721bd7f90c426e Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Mon, 21 Oct 2019 19:11:14 +0200 Subject: [PATCH 53/75] Re-generate sample config. --- conf/st2.conf.sample | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index d5d295d058..37985b34ae 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -8,6 +8,8 @@ enable = True emit_when = succeeded,failed,timeout,canceled,abandoned # comma separated list allowed here. [actionrunner] +# List of pip options to be passed to "pip install" command when installing pack dependencies into pack virtual environment. +pip_opts = # comma separated list allowed here. # Internal pool size for dispatcher used by regular actions. actions_pool_size = 60 # Default log level to use for Python runner actions. Can be overriden on invocation basis using "log_level" runner parameter. From 25ea404641d80b5e49243fbf6635ddea81407b64 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 15:57:37 +0100 Subject: [PATCH 54/75] Allow user to pass "bufsize" argument to st2common.util.green.shell.run_command() function. --- st2common/st2common/util/green/shell.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/util/green/shell.py b/st2common/st2common/util/green/shell.py index 0eda39b88a..d126675251 100644 --- a/st2common/st2common/util/green/shell.py +++ b/st2common/st2common/util/green/shell.py @@ -38,7 +38,8 @@ def run_command(cmd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=False, cwd=None, env=None, timeout=60, preexec_func=None, kill_func=None, read_stdout_func=None, read_stderr_func=None, - read_stdout_buffer=None, read_stderr_buffer=None, stdin_value=None): + read_stdout_buffer=None, read_stderr_buffer=None, stdin_value=None, + bufsize=0): """ Run the provided command in a subprocess and wait until it completes. @@ -82,6 +83,8 @@ def run_command(cmd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, using live read mode. :type read_stdout_func: ``func`` + :param bufsize: Buffer size argument to pass to subprocess.popen function. + :type bufsize: ``int`` :rtype: ``tuple`` (exit_code, stdout, stderr, timed_out) """ @@ -107,7 +110,8 @@ def run_command(cmd, stdin=None, stdout=subprocess.PIPE, stderr=subprocess.PIPE, # GreenPipe so it doesn't block LOG.debug('Creating subprocess.') process = concurrency.subprocess_popen(args=cmd, stdin=stdin, stdout=stdout, stderr=stderr, - env=env, cwd=cwd, shell=shell, preexec_fn=preexec_func, bufsize=-1) + env=env, cwd=cwd, shell=shell, preexec_fn=preexec_func, + bufsize=bufsize) if read_stdout_func: LOG.debug('Spawning read_stdout_func function') From 2f96d5db92ac270932cfa648acdda049e8b1cfe0 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 16:16:27 +0100 Subject: [PATCH 55/75] Add new actionrunner.stream_output_buf_size config option and use that value for "bufsize" argument inside the Python runner run_command function call. For performance reasons, default it to -1, but allow users to change it via config if they are experiencing performance or similar issues. --- .../python_runner/python_runner/python_runner.py | 10 +++++++--- st2common/st2common/config.py | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/contrib/runners/python_runner/python_runner/python_runner.py b/contrib/runners/python_runner/python_runner/python_runner.py index a1f42101b0..dae67f33e2 100644 --- a/contrib/runners/python_runner/python_runner/python_runner.py +++ b/contrib/runners/python_runner/python_runner/python_runner.py @@ -248,8 +248,11 @@ def run(self, action_parameters): if stdin_params: command_string = 'echo %s | %s' % (quote_unix(stdin_params), command_string) - LOG.debug('Running command: PATH=%s PYTHONPATH=%s %s' % (env['PATH'], env['PYTHONPATH'], - command_string)) + bufsize = cfg.CONF.actionrunner.stream_output_buffer_size + + LOG.debug('Running command (bufsize=%s): PATH=%s PYTHONPATH=%s %s' % (bufsize, env['PATH'], + env['PYTHONPATH'], + command_string)) exit_code, stdout, stderr, timed_out = run_command(cmd=args, stdin=stdin, stdout=subprocess.PIPE, @@ -261,7 +264,8 @@ def run(self, action_parameters): read_stderr_func=read_and_store_stderr, read_stdout_buffer=stdout, read_stderr_buffer=stderr, - stdin_value=stdin_params) + stdin_value=stdin_params, + bufsize=bufsize) LOG.debug('Returning values: %s, %s, %s, %s', exit_code, stdout, stderr, timed_out) LOG.debug('Returning.') return self._get_output_values(exit_code, stdout, stderr, timed_out) diff --git a/st2common/st2common/config.py b/st2common/st2common/config.py index 1b31b93e77..3bbd5725c1 100644 --- a/st2common/st2common/config.py +++ b/st2common/st2common/config.py @@ -376,7 +376,13 @@ def register_opts(ignore_errors=False): 'creates pack virtualenv.'), cfg.BoolOpt( 'stream_output', default=True, - help='True to store and stream action output (stdout and stderr) in real-time.') + help='True to store and stream action output (stdout and stderr) in real-time.'), + cfg.IntOpt( + 'stream_output_buffer_size', default=-1, + help=('Buffer size to use for real time action output streaming. 0 means unbuffered ' + '1 means line buffered, -1 means system default, which usually means fully ' + 'buffered and any other positive value means use a buffer of (approximately) ' + 'that size')) ] do_register_opts(action_runner_opts, group='actionrunner') From 711436fc167e4e209e0f5fda0b490b6f7e964c29 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 16:19:45 +0100 Subject: [PATCH 56/75] Add a simple test case which verifies that different values for "bufsize" argument don't hang or block the Python action runner streaming output functionality. --- .../tests/unit/test_pythonrunner.py | 32 +++++++++++++++++++ .../packs/pythonactions/actions/pascal_row.py | 2 ++ 2 files changed, 34 insertions(+) diff --git a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py index 9c242f237f..407af29c60 100644 --- a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py +++ b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py @@ -406,6 +406,38 @@ def test_action_stdout_and_stderr_is_stored_in_the_db(self, mock_spawn, mock_pop self.assertEqual(output_dbs[1].data, mock_stderr[1]) self.assertEqual(output_dbs[2].data, mock_stderr[2]) + def test_real_time_output_streaming_bufsize(self): + # Test various values for bufsize and verify it works / doesn't hang the process + cfg.CONF.set_override(name='stream_output', group='actionrunner', override=True) + + bufsize_values = [-100, -2, -1, 0, 1, 2, 1024, 2048, 4096, 10000] + + for index, bufsize in enumerate(bufsize_values, 1): + cfg.CONF.set_override(name='stream_output_buffer_size', override=bufsize, + group='actionrunner') + + output_dbs = ActionExecutionOutput.get_all() + self.assertEqual(len(output_dbs), (index - 1) * 3) + + runner = self._get_mock_runner_obj() + runner.runner_parameters = {'log_level': 'INFO'} + runner.entry_point = PASCAL_ROW_ACTION_PATH + runner.pre_run() + (_, output, _) = runner.run({'row_index': 2}) + + expected_stderr = ''.join([ + 'st2.actions.python.PascalRowAction: INFO test info log message\n', + 'st2.actions.python.PascalRowAction: ERROR test error log message\n' + ]) + + self.assertEqual(output['stdout'], 'Pascal row action\n') + self.assertEqual(output['stderr'], expected_stderr) + self.assertEqual(output['result'], [1, 2, 1]) + self.assertEqual(output['exit_code'], 0) + + output_dbs = ActionExecutionOutput.get_all() + self.assertEqual(len(output_dbs), (index) * 3) + @mock.patch('st2common.util.concurrency.subprocess_popen') def test_stdout_interception_and_parsing(self, mock_popen): values = {'delimiter': ACTION_OUTPUT_RESULT_DELIMITER} diff --git a/st2tests/st2tests/resources/packs/pythonactions/actions/pascal_row.py b/st2tests/st2tests/resources/packs/pythonactions/actions/pascal_row.py index 6f42b63042..030670452b 100644 --- a/st2tests/st2tests/resources/packs/pythonactions/actions/pascal_row.py +++ b/st2tests/st2tests/resources/packs/pythonactions/actions/pascal_row.py @@ -36,6 +36,8 @@ def run(self, **kwargs): @staticmethod def _compute_pascal_row(row_index=0): + print('Pascal row action') + if row_index == 'a': return False, 'This is suppose to fail don\'t worry!!' elif row_index == 'b': From f058a5d1e36cb901f6fd57759ce5926cdcfde245 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 16:28:42 +0100 Subject: [PATCH 57/75] Add changelog entry. --- CHANGELOG.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 6d79ce87f8..7199e85d5e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -28,6 +28,12 @@ Changed writing very large executions (executions with large results) to the database. #4767 * Improved development instructions in requirements.txt and dist_utils.py comment headers (improvement) #4774 +* Add new ``actionrunner.stream_output_buffer_size`` config option and default it to ``-1`` + (previously default value was ``0``). This should result in a better performance and smaller + CPU utilization for Python runner actions which produce a lot of output. + (improvement) + + Reported and contributed by Joshua Meyer (@jdmeyer3) #4803 Fixed ~~~~~ From dcfe02f1c009bd41b123e20419f512ff45f8fb70 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 17:04:44 +0100 Subject: [PATCH 58/75] Fix changelog entry. --- CHANGELOG.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f3608e2f7c..cb33bc1dcf 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -51,10 +51,10 @@ Fixed * Fix dependency conflicts in pack CI runs: downgrade requests dependency back to 0.21.0, update internal dependencies and test expectations (amqp, pyyaml, prance, six) (bugfix) #4774 * Fix mask of rule action secret parameters: - When we had an action on a rule with secret type parameters, the parameters were visible. This behavior - has been resolved in PR4788. - Contributed by @Nicodemos305 and @jeansfelix + When we had an action on a rule with secret type parameters, the parameters were visible. This + behavior has been resolved in #4788 + Contributed by @Nicodemos305 and @jeansfelix #4788 3.1.0 - June 27, 2019 --------------------- From bf421a304cc7a410693d5ae90203cf8f97c27510 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 17:58:16 +0100 Subject: [PATCH 59/75] Don't break the DB abstraction and use persistance class to query for the model in question. Also only retrieve fields we need for the masking operation. --- st2common/st2common/models/db/rule.py | 30 ++++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index f7b3c99a55..e7b84a5413 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -116,19 +116,27 @@ def mask_secrets(self, value): :rtype: ``dict`` """ result = copy.deepcopy(value) - if('action' not in result): + + action_ref = result.get('action', {}).get('ref', None) + + if not action_ref: return result - action_db = self._get_referenced_models(rule=result) - secret_parameters = get_secret_parameters(parameters=action_db['parameters']) + action_db = self._get_referenced_action_model(action_ref=action_ref) + + if not action_db: + return result + + secret_parameters = get_secret_parameters(parameters=action_db.parameters) result['action']['parameters'] = mask_secret_parameters( parameters=result['action']['parameters'], secret_parameters=secret_parameters) return result - def _get_referenced_models(self, rule): + def _get_referenced_action_model(self, action_ref): """ + Return Action object for the action referenced in a rule. Return the action model referenced from rule. :type rule: ``dict`` @@ -136,16 +144,14 @@ def _get_referenced_models(self, rule): :rtype: ``ActionDB`` """ - action_ref = rule['action']['ref'] - - def ref_query_args(ref): - return {'ref': ref} + # NOTE: We need to retrieve pack and name since that's needed for the PK + action_dbs = Action.query(only_fields=['pack', 'ref', 'name', 'parameters'], + ref=action_ref, limit=1) - action_db = self._get_entity(model_persistence=Action, - ref=action_ref, - query_args=ref_query_args) + if action_dbs: + return action_dbs[0] - return action_db + return None def _get_entity(self, model_persistence, ref, query_args): q = Q(**query_args(ref)) From 70f960bbfe3758d756e1ac1c861c96b49989bf3d Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:23:38 +0100 Subject: [PATCH 60/75] get one and get all rules api endpoint show also support ?show_secrets=True for admins. --- st2api/st2api/controllers/v1/rules.py | 17 ++++++++++++----- st2common/st2common/openapi.yaml.j2 | 8 ++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index 04215f2527..3d764d5847 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -20,6 +20,7 @@ from st2common import log as logging from st2common.exceptions.apivalidation import ValueValidationException from st2common.exceptions.triggers import TriggerDoesNotExistException +from st2api.controllers.base import BaseRestControllerMixin from st2api.controllers.resource import BaseResourceIsolationControllerMixin from st2api.controllers.resource import ContentPackResourceController from st2api.controllers.controller_transforms import transform_to_bool @@ -39,7 +40,7 @@ LOG = logging.getLogger(__name__) -class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceController): +class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, ContentPackResourceController): """ Implements the RESTful web endpoint that handles the lifecycle of Rules in the system. @@ -68,8 +69,11 @@ class RuleController(BaseResourceIsolationControllerMixin, ContentPackResourceCo mandatory_include_fields_retrieve = ['pack', 'name', 'trigger'] def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, offset=0, - limit=None, requester_user=None, **raw_filters): - from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} + limit=None, show_secrets=False, requester_user=None, **raw_filters): + from_model_kwargs = { + 'ignore_missing_trigger': True, + 'mask_secrets': self._get_mask_secrets(requester_user, show_secrets=show_secrets) + } return super(RuleController, self)._get_all(exclude_fields=exclude_attributes, include_fields=include_attributes, from_model_kwargs=from_model_kwargs, @@ -79,8 +83,11 @@ def get_all(self, exclude_attributes=None, include_attributes=None, sort=None, o raw_filters=raw_filters, requester_user=requester_user) - def get_one(self, ref_or_id, requester_user): - from_model_kwargs = {'ignore_missing_trigger': True, 'mask_secrets': True} + def get_one(self, ref_or_id, requester_user, show_secrets=False): + from_model_kwargs = { + 'ignore_missing_trigger': True, + 'mask_secrets': self._get_mask_secrets(requester_user, show_secrets=show_secrets) + } return super(RuleController, self)._get_one(ref_or_id, from_model_kwargs=from_model_kwargs, requester_user=requester_user, permission_type=PermissionType.RULE_VIEW) diff --git a/st2common/st2common/openapi.yaml.j2 b/st2common/st2common/openapi.yaml.j2 index 253f45cfde..3387564e8a 100644 --- a/st2common/st2common/openapi.yaml.j2 +++ b/st2common/st2common/openapi.yaml.j2 @@ -2645,6 +2645,10 @@ paths: in: query description: Enabled filter type: string + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context @@ -2706,6 +2710,10 @@ paths: description: Entity reference or id type: string required: true + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context From 3400a76aaef258b94e046e4d4be3689e9e064fd6 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:23:57 +0100 Subject: [PATCH 61/75] Add some test cases which verify secrets masking works correctly for rules API endpoint. --- .../tests/unit/controllers/v1/test_rules.py | 45 ++++++++++++++++++- .../fixtures/generic/actions/action1.yaml | 3 ++ .../fixtures/generic/rules/rule1.yaml | 1 + 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_rules.py b/st2api/tests/unit/controllers/v1/test_rules.py index c6831393ab..3651bef6ac 100644 --- a/st2api/tests/unit/controllers/v1/test_rules.py +++ b/st2api/tests/unit/controllers/v1/test_rules.py @@ -20,6 +20,7 @@ from st2common.constants.rules import RULE_TYPE_STANDARD, RULE_TYPE_BACKSTOP from st2common.constants.pack import DEFAULT_PACK_NAME +from st2common.constants.secrets import MASKED_ATTRIBUTE_VALUE from st2common.persistence.trigger import Trigger from st2common.models.system.common import ResourceReference from st2common.transport.publishers import PoolPublisher @@ -185,10 +186,20 @@ def test_get_all_enabled(self): self.__do_delete(self.__get_rule_id(post_resp_rule_1)) self.__do_delete(self.__get_rule_id(post_resp_rule_3)) - def test_get_all_parameters_mask_with_include_parameters(self): + def test_get_all_action_parameters_secrets_masking(self): post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) - resp = self.app.get('/v1/rules?include_attributes=action') + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules?include_attributes=action&show_secrets=true') self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], 'secret') + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) def test_get_all_parameters_mask_with_exclude_parameters(self): @@ -197,6 +208,36 @@ def test_get_all_parameters_mask_with_exclude_parameters(self): self.assertEqual('action' in resp.json[0], False) self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + def test_get_all_parameters_mask_with_include_parameters(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules?include_attributes=action') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules?include_attributes=action&show_secrets=true') + self.assertEqual('action' in resp.json[0], True) + self.assertEqual(resp.json[0]['action']['parameters']['action_secret'], 'secret') + + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + + def test_get_one_action_parameters_secrets_masking(self): + post_resp_rule_1 = self.__do_post(RulesControllerTestCase.RULE_1) + + # Verify parameter is masked by default + resp = self.app.get('/v1/rules/%s' % (post_resp_rule_1.json['id'])) + self.assertEqual(resp.json['action']['parameters']['action_secret'], + MASKED_ATTRIBUTE_VALUE) + + # Verify ?show_secrets=true works + resp = self.app.get('/v1/rules/%s?show_secrets=true' % (post_resp_rule_1.json['id'])) + self.assertEqual(resp.json['action']['parameters']['action_secret'], 'secret') + + self.__do_delete(self.__get_rule_id(post_resp_rule_1)) + def test_get_one_by_id(self): post_resp = self.__do_post(RulesControllerTestCase.RULE_1) rule_id = self.__get_rule_id(post_resp) diff --git a/st2tests/st2tests/fixtures/generic/actions/action1.yaml b/st2tests/st2tests/fixtures/generic/actions/action1.yaml index 26522c26e3..15422c71b7 100644 --- a/st2tests/st2tests/fixtures/generic/actions/action1.yaml +++ b/st2tests/st2tests/fixtures/generic/actions/action1.yaml @@ -18,6 +18,9 @@ parameters: async_test: default: false type: boolean + action_secret: + type: string + secret: true runnerdummy: default: actiondummy immutable: true diff --git a/st2tests/st2tests/fixtures/generic/rules/rule1.yaml b/st2tests/st2tests/fixtures/generic/rules/rule1.yaml index a8c0809138..0e4302e7ca 100644 --- a/st2tests/st2tests/fixtures/generic/rules/rule1.yaml +++ b/st2tests/st2tests/fixtures/generic/rules/rule1.yaml @@ -3,6 +3,7 @@ action: parameters: ip1: '{{trigger.t1_p}}' ip2: '{{trigger}}' + action_secret: 'secret' ref: wolfpack.action-1 criteria: trigger.t1_p: From 0343096689a78af373fa51b5cba01c4f78b21324 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:25:11 +0100 Subject: [PATCH 62/75] Fix lint, add missing change. --- st2api/st2api/controllers/v1/rules.py | 3 ++- st2common/st2common/openapi.yaml | 8 ++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/st2api/st2api/controllers/v1/rules.py b/st2api/st2api/controllers/v1/rules.py index 3d764d5847..4921b7b279 100644 --- a/st2api/st2api/controllers/v1/rules.py +++ b/st2api/st2api/controllers/v1/rules.py @@ -40,7 +40,8 @@ LOG = logging.getLogger(__name__) -class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, ContentPackResourceController): +class RuleController(BaseRestControllerMixin, BaseResourceIsolationControllerMixin, + ContentPackResourceController): """ Implements the RESTful web endpoint that handles the lifecycle of Rules in the system. diff --git a/st2common/st2common/openapi.yaml b/st2common/st2common/openapi.yaml index 91e299ff05..0896532f74 100644 --- a/st2common/st2common/openapi.yaml +++ b/st2common/st2common/openapi.yaml @@ -2649,6 +2649,10 @@ paths: in: query description: Enabled filter type: string + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context @@ -2710,6 +2714,10 @@ paths: description: Entity reference or id type: string required: true + - name: show_secrets + in: query + description: Show secrets in plain text + type: boolean x-parameters: - name: user in: context From e7d62d1e07102c6d78151eb6c0121042cda81fc1 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:34:53 +0100 Subject: [PATCH 63/75] Re-generate sample config. --- conf/st2.conf.sample | 2 ++ 1 file changed, 2 insertions(+) diff --git a/conf/st2.conf.sample b/conf/st2.conf.sample index d5d295d058..b3a7262a46 100644 --- a/conf/st2.conf.sample +++ b/conf/st2.conf.sample @@ -20,6 +20,8 @@ python3_prefix = None virtualenv_binary = /usr/bin/virtualenv # Python 3 binary which will be used by Python actions for packs which use Python 3 virtual environment. python3_binary = /usr/bin/python3 +# Buffer size to use for real time action output streaming. 0 means unbuffered 1 means line buffered, -1 means system default, which usually means fully buffered and any other positive value means use a buffer of (approximately) that size +stream_output_buffer_size = -1 # List of virtualenv options to be passsed to "virtualenv" command that creates pack virtualenv. virtualenv_opts = --system-site-packages # comma separated list allowed here. # True to store and stream action output (stdout and stderr) in real-time. From e446a46a26eb7b3288fe34e9424e229b42998589 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 19:43:34 +0100 Subject: [PATCH 64/75] Use a separate action for bufsize test case. --- .../tests/unit/test_pythonrunner.py | 21 +++++--------- .../actions/print_to_stdout_and_stderr.py | 29 +++++++++++++++++++ 2 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 st2tests/st2tests/resources/packs/pythonactions/actions/print_to_stdout_and_stderr.py diff --git a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py index 407af29c60..7dcad87d28 100644 --- a/contrib/runners/python_runner/tests/unit/test_pythonrunner.py +++ b/contrib/runners/python_runner/tests/unit/test_pythonrunner.py @@ -63,6 +63,8 @@ PRINT_CONFIG_ITEM_ACTION = os.path.join(tests_base.get_resources_path(), 'packs', 'pythonactions/actions/print_config_item_doesnt_exist.py') +PRINT_TO_STDOUT_STDERR_ACTION = os.path.join(tests_base.get_resources_path(), 'packs', + 'pythonactions/actions/print_to_stdout_and_stderr.py') # Note: runner inherits parent args which doesn't work with tests since test pass additional @@ -417,26 +419,19 @@ def test_real_time_output_streaming_bufsize(self): group='actionrunner') output_dbs = ActionExecutionOutput.get_all() - self.assertEqual(len(output_dbs), (index - 1) * 3) + self.assertEqual(len(output_dbs), (index - 1) * 4) runner = self._get_mock_runner_obj() - runner.runner_parameters = {'log_level': 'INFO'} - runner.entry_point = PASCAL_ROW_ACTION_PATH + runner.entry_point = PRINT_TO_STDOUT_STDERR_ACTION runner.pre_run() - (_, output, _) = runner.run({'row_index': 2}) + (_, output, _) = runner.run({'stdout_count': 2, 'stderr_count': 2}) - expected_stderr = ''.join([ - 'st2.actions.python.PascalRowAction: INFO test info log message\n', - 'st2.actions.python.PascalRowAction: ERROR test error log message\n' - ]) - - self.assertEqual(output['stdout'], 'Pascal row action\n') - self.assertEqual(output['stderr'], expected_stderr) - self.assertEqual(output['result'], [1, 2, 1]) + self.assertEqual(output['stdout'], 'stdout line 0\nstdout line 1\n') + self.assertEqual(output['stderr'], 'stderr line 0\nstderr line 1\n') self.assertEqual(output['exit_code'], 0) output_dbs = ActionExecutionOutput.get_all() - self.assertEqual(len(output_dbs), (index) * 3) + self.assertEqual(len(output_dbs), (index) * 4) @mock.patch('st2common.util.concurrency.subprocess_popen') def test_stdout_interception_and_parsing(self, mock_popen): diff --git a/st2tests/st2tests/resources/packs/pythonactions/actions/print_to_stdout_and_stderr.py b/st2tests/st2tests/resources/packs/pythonactions/actions/print_to_stdout_and_stderr.py new file mode 100644 index 0000000000..7c7a044748 --- /dev/null +++ b/st2tests/st2tests/resources/packs/pythonactions/actions/print_to_stdout_and_stderr.py @@ -0,0 +1,29 @@ +# Copyright 2019 Extreme Networks, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from __future__ import absolute_import + +import sys + +from st2common.runners.base_action import Action +from six.moves import range + + +class PrintToStdoutAndStderrAction(Action): + def run(self, stdout_count=3, stderr_count=3): + for index in range(0, stdout_count): + sys.stdout.write('stdout line %s\n' % (index)) + + for index in range(0, stderr_count): + sys.stderr.write('stderr line %s\n' % (index)) From 0b76bd505e0801bdf3e403c795dcbb84d5e0bf79 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 20:30:07 +0100 Subject: [PATCH 65/75] Store log files for each screen session inside launchdev script. This way we can troubleshoot various "fail to start" issues on CI. Without this change, we have no visibility if service fails to start before the actual service starts writting into it's own log file. Sadly we can't rely on new version of screen which supports -Logfile path.log flag so we need to use screen config per screen window so we can use different log files for different processes. --- scripts/travis/prepare-integration.sh | 8 ++++ tools/launchdev.sh | 40 ++++++++++--------- tools/screen-configs/st2actionrunner.conf | 6 +++ tools/screen-configs/st2api.conf | 6 +++ tools/screen-configs/st2auth.conf | 6 +++ tools/screen-configs/st2garbagecollector.conf | 6 +++ tools/screen-configs/st2notifier.conf | 6 +++ tools/screen-configs/st2resultstracker.conf | 6 +++ tools/screen-configs/st2rulesengine.conf | 6 +++ tools/screen-configs/st2scheduler.conf | 6 +++ tools/screen-configs/st2sensorcontainer.conf | 6 +++ tools/screen-configs/st2stream.conf | 6 +++ tools/screen-configs/st2timersengine.conf | 6 +++ tools/screen-configs/st2workflowengine.conf | 6 +++ 14 files changed, 101 insertions(+), 19 deletions(-) create mode 100644 tools/screen-configs/st2actionrunner.conf create mode 100644 tools/screen-configs/st2api.conf create mode 100644 tools/screen-configs/st2auth.conf create mode 100644 tools/screen-configs/st2garbagecollector.conf create mode 100644 tools/screen-configs/st2notifier.conf create mode 100644 tools/screen-configs/st2resultstracker.conf create mode 100644 tools/screen-configs/st2rulesengine.conf create mode 100644 tools/screen-configs/st2scheduler.conf create mode 100644 tools/screen-configs/st2sensorcontainer.conf create mode 100644 tools/screen-configs/st2stream.conf create mode 100644 tools/screen-configs/st2timersengine.conf create mode 100644 tools/screen-configs/st2workflowengine.conf diff --git a/scripts/travis/prepare-integration.sh b/scripts/travis/prepare-integration.sh index deb77d620f..c20e1b5fde 100755 --- a/scripts/travis/prepare-integration.sh +++ b/scripts/travis/prepare-integration.sh @@ -15,9 +15,17 @@ source ./virtualenv/bin/activate python ./st2client/setup.py develop st2 --version +# Clean up old screen log files +rm -f logs/screen-*.log + # start dev environment in screens ./tools/launchdev.sh start -x +# Give processes some time to start and check logs to see if all the services +# started or if there was any error / failure +sleep 10 +cat logs/screen-*.log + # This script runs as root on Travis which means other processes which don't run # as root can't write to logs/ directory and tests fail chmod 777 logs/ diff --git a/tools/launchdev.sh b/tools/launchdev.sh index 5cc6b13899..ad2e5d9dec 100755 --- a/tools/launchdev.sh +++ b/tools/launchdev.sh @@ -245,15 +245,18 @@ function st2start(){ screen -ls | grep mistral | cut -d. -f1 | awk '{print $1}' | xargs kill fi + # NOTE: We can't rely on latest version of screen with "-Logfile path" + # option so we need to use screen config file per screen window + # Run the st2 API server echo 'Starting screen session st2-api...' if [ "${use_gunicorn}" = true ]; then echo ' using gunicorn to run st2-api...' export ST2_CONFIG_PATH=${ST2_CONF} - screen -d -m -S st2-api ${VIRTUALENV}/bin/gunicorn \ + screen -L -c tools/screen-configs/st2api.conf -d -m -S st2-api ${VIRTUALENV}/bin/gunicorn \ st2api.wsgi:application -k eventlet -b "$BINDING_ADDRESS:9101" --workers 1 else - screen -d -m -S st2-api ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2api.conf -d -m -S st2-api ${VIRTUALENV}/bin/python \ ./st2api/bin/st2api \ --config-file $ST2_CONF fi @@ -262,10 +265,10 @@ function st2start(){ if [ "${use_gunicorn}" = true ]; then echo ' using gunicorn to run st2-stream' export ST2_CONFIG_PATH=${ST2_CONF} - screen -d -m -S st2-stream ${VIRTUALENV}/bin/gunicorn \ + screen -L -c tools/screen-configs/st2stream.conf -d -m -S st2-stream ${VIRTUALENV}/bin/gunicorn \ st2stream.wsgi:application -k eventlet -b "$BINDING_ADDRESS:9102" --workers 1 else - screen -d -m -S st2-stream ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2stream.conf -d -m -S st2-stream ${VIRTUALENV}/bin/python \ ./st2stream/bin/st2stream \ --config-file $ST2_CONF fi @@ -278,7 +281,7 @@ function st2start(){ WORKFLOW_ENGINE_NAME=st2-workflow-$i WORKFLOW_ENGINE_SCREENS+=($WORKFLOW_ENGINE_NAME) echo ' starting '$WORKFLOW_ENGINE_NAME'...' - screen -d -m -S $WORKFLOW_ENGINE_NAME ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2workflowengine.conf -d -m -S $WORKFLOW_ENGINE_NAME ${VIRTUALENV}/bin/python \ ./st2actions/bin/st2workflowengine \ --config-file $ST2_CONF done @@ -291,14 +294,14 @@ function st2start(){ RUNNER_NAME=st2-actionrunner-$i RUNNER_SCREENS+=($RUNNER_NAME) echo ' starting '$RUNNER_NAME'...' - screen -d -m -S $RUNNER_NAME ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2actionrunner.conf -d -m -S $RUNNER_NAME ${VIRTUALENV}/bin/python \ ./st2actions/bin/st2actionrunner \ --config-file $ST2_CONF done # Run the garbage collector service echo 'Starting screen session st2-garbagecollector' - screen -d -m -S st2-garbagecollector ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2garbagecollector.conf -d -m -S st2-garbagecollector ${VIRTUALENV}/bin/python \ ./st2reactor/bin/st2garbagecollector \ --config-file $ST2_CONF @@ -310,38 +313,38 @@ function st2start(){ SCHEDULER_NAME=st2-scheduler-$i SCHEDULER_SCREENS+=($SCHEDULER_NAME) echo ' starting '$SCHEDULER_NAME'...' - screen -d -m -S $SCHEDULER_NAME ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2scheduler.conf -d -m -S $SCHEDULER_NAME ${VIRTUALENV}/bin/python \ ./st2actions/bin/st2scheduler \ --config-file $ST2_CONF done # Run the sensor container server echo 'Starting screen session st2-sensorcontainer' - screen -d -m -S st2-sensorcontainer ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2sensorcontainer.conf -d -m -S st2-sensorcontainer ${VIRTUALENV}/bin/python \ ./st2reactor/bin/st2sensorcontainer \ --config-file $ST2_CONF # Run the rules engine server echo 'Starting screen session st2-rulesengine...' - screen -d -m -S st2-rulesengine ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2rulesengine.conf -d -m -S st2-rulesengine ${VIRTUALENV}/bin/python \ ./st2reactor/bin/st2rulesengine \ --config-file $ST2_CONF # Run the timer engine server echo 'Starting screen session st2-timersengine...' - screen -d -m -S st2-timersengine ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2timersengine.conf -d -m -S st2-timersengine ${VIRTUALENV}/bin/python \ ./st2reactor/bin/st2timersengine \ --config-file $ST2_CONF # Run the results tracker echo 'Starting screen session st2-resultstracker...' - screen -d -m -S st2-resultstracker ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2resultstracker.conf -d -m -S st2-resultstracker ${VIRTUALENV}/bin/python \ ./st2actions/bin/st2resultstracker \ --config-file $ST2_CONF # Run the actions notifier echo 'Starting screen session st2-notifier...' - screen -d -m -S st2-notifier ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2notifier.conf -d -m -S st2-notifier ${VIRTUALENV}/bin/python \ ./st2actions/bin/st2notifier \ --config-file $ST2_CONF @@ -350,10 +353,10 @@ function st2start(){ if [ "${use_gunicorn}" = true ]; then echo ' using gunicorn to run st2-auth...' export ST2_CONFIG_PATH=${ST2_CONF} - screen -d -m -S st2-auth ${VIRTUALENV}/bin/gunicorn \ + screen -L -c tools/screen-configs/st2auth.conf -d -m -S st2-auth ${VIRTUALENV}/bin/gunicorn \ st2auth.wsgi:application -k eventlet -b "$BINDING_ADDRESS:9100" --workers 1 else - screen -d -m -S st2-auth ${VIRTUALENV}/bin/python \ + screen -L -c tools/screen-configs/st2auth.conf -d -m -S st2-auth ${VIRTUALENV}/bin/python \ ./st2auth/bin/st2auth \ --config-file $ST2_CONF fi @@ -364,18 +367,17 @@ function st2start(){ sudo mkdir -p $EXPORTS_DIR sudo chown -R ${CURRENT_USER}:${CURRENT_USER_GROUP} $EXPORTS_DIR echo 'Starting screen session st2-exporter...' - screen -d -m -S st2-exporter ${VIRTUALENV}/bin/python \ + screen -L -d -m -S st2-exporter ${VIRTUALENV}/bin/python \ ./st2exporter/bin/st2exporter \ --config-file $ST2_CONF fi if [ "${include_mistral}" = true ]; then - LOGDIR=${ST2_REPO}/logs # Run mistral-server echo 'Starting screen session mistral-server...' - screen -d -m -S mistral-server ${MISTRAL_REPO}/.venv/bin/python \ + screen -L -Logfile logs/screen-mistral-server.log -d -m -S mistral-server ${MISTRAL_REPO}/.venv/bin/python \ ${MISTRAL_REPO}/.venv/bin/mistral-server \ --server engine,executor \ --config-file $MISTRAL_CONF \ @@ -383,7 +385,7 @@ function st2start(){ # Run mistral-api echo 'Starting screen session mistral-api...' - screen -d -m -S mistral-api ${MISTRAL_REPO}/.venv/bin/python \ + screen -L -Logfile logs/screen-mistral-server.log -d -m -S mistral-api ${MISTRAL_REPO}/.venv/bin/python \ ${MISTRAL_REPO}/.venv/bin/mistral-server \ --server api \ --config-file $MISTRAL_CONF \ diff --git a/tools/screen-configs/st2actionrunner.conf b/tools/screen-configs/st2actionrunner.conf new file mode 100644 index 0000000000..c7c8d900a1 --- /dev/null +++ b/tools/screen-configs/st2actionrunner.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2actionrunner.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2api.conf b/tools/screen-configs/st2api.conf new file mode 100644 index 0000000000..834313114b --- /dev/null +++ b/tools/screen-configs/st2api.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2api.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2auth.conf b/tools/screen-configs/st2auth.conf new file mode 100644 index 0000000000..0bff41afcb --- /dev/null +++ b/tools/screen-configs/st2auth.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2auth.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2garbagecollector.conf b/tools/screen-configs/st2garbagecollector.conf new file mode 100644 index 0000000000..dddb0f68f4 --- /dev/null +++ b/tools/screen-configs/st2garbagecollector.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2garbagecollector.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2notifier.conf b/tools/screen-configs/st2notifier.conf new file mode 100644 index 0000000000..104d873a39 --- /dev/null +++ b/tools/screen-configs/st2notifier.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2notifier.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2resultstracker.conf b/tools/screen-configs/st2resultstracker.conf new file mode 100644 index 0000000000..c5d454d4d0 --- /dev/null +++ b/tools/screen-configs/st2resultstracker.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2resultstracker.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2rulesengine.conf b/tools/screen-configs/st2rulesengine.conf new file mode 100644 index 0000000000..1fca4d9807 --- /dev/null +++ b/tools/screen-configs/st2rulesengine.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2rulesengine.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2scheduler.conf b/tools/screen-configs/st2scheduler.conf new file mode 100644 index 0000000000..1bbcbd81f6 --- /dev/null +++ b/tools/screen-configs/st2scheduler.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2scheduler.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2sensorcontainer.conf b/tools/screen-configs/st2sensorcontainer.conf new file mode 100644 index 0000000000..735550d07b --- /dev/null +++ b/tools/screen-configs/st2sensorcontainer.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2sensorcontainer.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2stream.conf b/tools/screen-configs/st2stream.conf new file mode 100644 index 0000000000..e6c6b8c09e --- /dev/null +++ b/tools/screen-configs/st2stream.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2stream.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2timersengine.conf b/tools/screen-configs/st2timersengine.conf new file mode 100644 index 0000000000..5d358d8fec --- /dev/null +++ b/tools/screen-configs/st2timersengine.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2timersengine.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on diff --git a/tools/screen-configs/st2workflowengine.conf b/tools/screen-configs/st2workflowengine.conf new file mode 100644 index 0000000000..28d110e29c --- /dev/null +++ b/tools/screen-configs/st2workflowengine.conf @@ -0,0 +1,6 @@ +logfile logs/screen-st2workflowengine.log +logfile flush 1 +log on +logtstamp after 1 +logtstamp string \"[ %t: %Y-%m-%d %c:%s ]\012\" +logtstamp on From b3d885ab9b34d4270ff7cd75fa99f3e5ddce405a Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 20:40:36 +0100 Subject: [PATCH 66/75] Add some log messages to make it easier to identify output. --- scripts/travis/prepare-integration.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/scripts/travis/prepare-integration.sh b/scripts/travis/prepare-integration.sh index c20e1b5fde..3557ce398e 100755 --- a/scripts/travis/prepare-integration.sh +++ b/scripts/travis/prepare-integration.sh @@ -23,8 +23,12 @@ rm -f logs/screen-*.log # Give processes some time to start and check logs to see if all the services # started or if there was any error / failure +echo "Giving screen processes some time to start..." sleep 10 + +echo " === START: Catting screen process log files. ===" cat logs/screen-*.log +echo " === END: Catting screen process log files. ===" # This script runs as root on Travis which means other processes which don't run # as root can't write to logs/ directory and tests fail From bfafd43fa55969c2b0d0705af609836463bbe0ec Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 22:43:08 +0100 Subject: [PATCH 67/75] Remove unused method, fix docstring. --- st2common/st2common/models/db/rule.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index e7b84a5413..0650363a5a 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -16,7 +16,6 @@ import copy import mongoengine as me -from mongoengine.queryset import Q from st2common.persistence.action import Action from st2common.models.db import MongoDBAccess from st2common.models.db import stormbase @@ -137,10 +136,9 @@ def mask_secrets(self, value): def _get_referenced_action_model(self, action_ref): """ Return Action object for the action referenced in a rule. - Return the action model referenced from rule. - :type rule: ``dict`` - :param rule: rule + :param action_ref: Action reference. + :type action_ref: ``str`` :rtype: ``ActionDB`` """ @@ -153,14 +151,6 @@ def _get_referenced_action_model(self, action_ref): return None - def _get_entity(self, model_persistence, ref, query_args): - q = Q(**query_args(ref)) - - if q: - return model_persistence._get_impl().model.objects(q).first() - - return None - def __init__(self, *args, **values): super(RuleDB, self).__init__(*args, **values) self.ref = self.get_reference().ref From ec6b7fdfdb421f7f242555f534f8e67c4ef59c95 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Tue, 29 Oct 2019 22:43:57 +0100 Subject: [PATCH 68/75] Add additional clarification. --- st2common/st2common/models/db/rule.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/st2common/st2common/models/db/rule.py b/st2common/st2common/models/db/rule.py index 0650363a5a..e56d66cea9 100644 --- a/st2common/st2common/models/db/rule.py +++ b/st2common/st2common/models/db/rule.py @@ -109,6 +109,9 @@ def mask_secrets(self, value): """ Process the model dictionary and mask secret values. + NOTE: This method results in one addition "get one" query where we retrieve corresponding + action model so we can correctly mask secret parameters. + :type value: ``dict`` :param value: Document dictionary. From 06628a7999f5bf0ec9db71163651a66ee8901c02 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Wed, 30 Oct 2019 08:08:42 +0100 Subject: [PATCH 69/75] Update changelog. --- CHANGELOG.rst | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 689df29023..2a6f4937ea 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -15,7 +15,7 @@ Added * Add support for `immutable_parameters` on Action Aliases. This feature allows default parameters to be supplied to the action on every execution of the alias. #4786 * Add ``get_entrypoint()`` method to ``ActionResourceManager`` attribute of st2client. - #4791 + #4791 Changed ~~~~~~~ @@ -61,11 +61,10 @@ Fixed doesn't depend on internal pip API and so it works with latest pip version. (bug fix) #4750 * Fix dependency conflicts in pack CI runs: downgrade requests dependency back to 0.21.0, update internal dependencies and test expectations (amqp, pyyaml, prance, six) (bugfix) #4774 -* Fix mask of rule action secret parameters: +* Fix secrets masking in action parameters section defined inside the rule when using + ``GET /v1/rules`` and ``GET /v1/rules/`` API endpoint. (bug fix) #4788 #4807 - When we had an action on a rule with secret type parameters, the parameters were visible. This - behavior has been resolved in #4788 - Contributed by @Nicodemos305 and @jeansfelix #4788 + Contributed by @Nicodemos305 and @jeansfelix 3.1.0 - June 27, 2019 --------------------- From 0fde265a9c4b33dbf5deaaaae07a776b7725771f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 31 Oct 2019 19:31:31 +0100 Subject: [PATCH 70/75] Fix a bug with authentication returning internal server error under gunicorn if auth.api_url config option was not set. NOTE: This issue would only affect deployments using gunicorn which don't have auth.api_url config option set. --- st2auth/st2auth/controllers/v1/auth.py | 4 +++- st2auth/tests/unit/controllers/v1/test_token.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/st2auth/st2auth/controllers/v1/auth.py b/st2auth/st2auth/controllers/v1/auth.py index e32266b9d2..3b3073e32b 100644 --- a/st2auth/st2auth/controllers/v1/auth.py +++ b/st2auth/st2auth/controllers/v1/auth.py @@ -22,6 +22,7 @@ from st2common.router import exc from st2common.router import Response from st2common.util import auth as auth_utils +from st2common.util import api as api_utils from st2common import log as logging import st2auth.handlers as handlers @@ -80,7 +81,8 @@ def post(self, request, **kwargs): def process_successful_response(token): resp = Response(json=token, status=http_client.CREATED) - resp.headers['X-API-URL'] = cfg.CONF.auth.api_url + # NOTE: gunicon fails and throws an error if header value is not a string (e.g. if it's None) + resp.headers['X-API-URL'] = api_utils.get_base_public_api_url() return resp diff --git a/st2auth/tests/unit/controllers/v1/test_token.py b/st2auth/tests/unit/controllers/v1/test_token.py index d1a5e46e5e..3a3eb237b1 100644 --- a/st2auth/tests/unit/controllers/v1/test_token.py +++ b/st2auth/tests/unit/controllers/v1/test_token.py @@ -87,6 +87,7 @@ def _test_token_post(self, path=TOKEN_V1_PATH): actual_expiry = isotime.parse(response.json['expiry']) self.assertLess(timestamp, actual_expiry) self.assertLess(actual_expiry, expected_expiry) + return response def test_token_post_unauthorized(self): response = self.app.post_json(TOKEN_V1_PATH, {}, expect_errors=True, extra_environ={ @@ -109,6 +110,22 @@ def test_token_post_new_user(self): def test_token_post_existing_user(self): self._test_token_post() + @mock.patch.object( + User, 'get_by_name', + mock.MagicMock(return_value=UserDB(name=USERNAME))) + def test_token_post_success_x_api_url_header_value(self): + # auth.api_url option is explicitly set + cfg.CONF.set_override('api_url', override='https://example.com', group='auth') + + resp = self._test_token_post() + self.assertEqual(resp.headers['X-API-URL'], 'https://example.com') + + # auth.api_url option is not set, url is inferred from listen host and port + cfg.CONF.set_override('api_url', override=None, group='auth') + + resp = self._test_token_post() + self.assertEqual(resp.headers['X-API-URL'], 'http://127.0.0.1:9101') + @mock.patch.object( User, 'get_by_name', mock.MagicMock(return_value=UserDB(name=USERNAME))) From b8b874fcadc471d2db88011c9ef9e912a6eaa87f Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Thu, 31 Oct 2019 20:03:50 +0100 Subject: [PATCH 71/75] Add changelog entry. --- CHANGELOG.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 2a6f4937ea..b26a4212d9 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -65,6 +65,11 @@ Fixed ``GET /v1/rules`` and ``GET /v1/rules/`` API endpoint. (bug fix) #4788 #4807 Contributed by @Nicodemos305 and @jeansfelix +* Fix a bug with authentication API endpoint (``POST /auth/v1/tokens``) returning internal + server error when running under gunicorn and when``auth.api_url`` config option was not set. + (bug fix) #4809 + + Reported by @guzzijones 3.1.0 - June 27, 2019 --------------------- From 03909b4c9be143e966c5718025da78f08eeebd46 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 1 Nov 2019 21:32:58 +0100 Subject: [PATCH 72/75] Update affected tests. --- st2client/tests/fixtures/execution_double_backslash.txt | 6 +++++- st2client/tests/fixtures/execution_get_default.txt | 6 +++++- st2client/tests/fixtures/execution_get_detail.txt | 4 ---- st2client/tests/fixtures/execution_get_has_schema.txt | 6 +++++- .../fixtures/execution_result_has_carriage_return.txt | 4 ---- st2client/tests/fixtures/execution_unescape_newline.txt | 6 +++++- st2client/tests/fixtures/execution_unicode.txt | 6 +++++- st2client/tests/unit/test_formatters.py | 7 ++++++- 8 files changed, 31 insertions(+), 14 deletions(-) diff --git a/st2client/tests/fixtures/execution_double_backslash.txt b/st2client/tests/fixtures/execution_double_backslash.txt index 2e589baa93..5437c7add4 100644 --- a/st2client/tests/fixtures/execution_double_backslash.txt +++ b/st2client/tests/fixtures/execution_double_backslash.txt @@ -1,7 +1,11 @@ id: 547e19561e2e2417d3dde333 -status: succeeded (1s elapsed) +action.ref: core.local +context.user: stanley parameters: cmd: echo 'C:\Users\ADMINI~1\AppData\Local\Temp\jking_vmware20_test' +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: failed: false return_code: 0 diff --git a/st2client/tests/fixtures/execution_get_default.txt b/st2client/tests/fixtures/execution_get_default.txt index 2613b61f2c..c29c2c221d 100644 --- a/st2client/tests/fixtures/execution_get_default.txt +++ b/st2client/tests/fixtures/execution_get_default.txt @@ -1,7 +1,11 @@ id: 547e19561e2e2417d3dde398 -status: succeeded (1s elapsed) +action.ref: core.ping +context.user: stanley parameters: cmd: 127.0.0.1 3 +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: localhost: failed: false diff --git a/st2client/tests/fixtures/execution_get_detail.txt b/st2client/tests/fixtures/execution_get_detail.txt index 9b9edfaabf..4645130ace 100644 --- a/st2client/tests/fixtures/execution_get_detail.txt +++ b/st2client/tests/fixtures/execution_get_detail.txt @@ -28,8 +28,4 @@ | | rtt min/avg/max/mdev = 0.015/0.023/0.030/0.006 ms" | | | } | | | } | -| liveaction | { | -| | "callback": {}, | -| | "id": "1" | -| | } | +-----------------+--------------------------------------------------------------+ diff --git a/st2client/tests/fixtures/execution_get_has_schema.txt b/st2client/tests/fixtures/execution_get_has_schema.txt index 2613b61f2c..c29c2c221d 100644 --- a/st2client/tests/fixtures/execution_get_has_schema.txt +++ b/st2client/tests/fixtures/execution_get_has_schema.txt @@ -1,7 +1,11 @@ id: 547e19561e2e2417d3dde398 -status: succeeded (1s elapsed) +action.ref: core.ping +context.user: stanley parameters: cmd: 127.0.0.1 3 +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: localhost: failed: false diff --git a/st2client/tests/fixtures/execution_result_has_carriage_return.txt b/st2client/tests/fixtures/execution_result_has_carriage_return.txt index 2484678896..efdbac7e8f 100644 --- a/st2client/tests/fixtures/execution_result_has_carriage_return.txt +++ b/st2client/tests/fixtures/execution_result_has_carriage_return.txt @@ -64,8 +64,4 @@ | | "stdout": "" | | | } | | | } | -| liveaction | { | -| | "callback": {}, | -| | "id": "1" | -| | } | +-----------------+--------------------------------------------------------------+ diff --git a/st2client/tests/fixtures/execution_unescape_newline.txt b/st2client/tests/fixtures/execution_unescape_newline.txt index 40ab18deca..4abac251a5 100644 --- a/st2client/tests/fixtures/execution_unescape_newline.txt +++ b/st2client/tests/fixtures/execution_unescape_newline.txt @@ -1,6 +1,10 @@ id: 547e19561e2e2417d3dde123 -status: succeeded (1s elapsed) +action.ref: core.stacktrace +context.user: stanley parameters: None +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: localhost: failed: false diff --git a/st2client/tests/fixtures/execution_unicode.txt b/st2client/tests/fixtures/execution_unicode.txt index f7209afaa9..7b7491d3b7 100644 --- a/st2client/tests/fixtures/execution_unicode.txt +++ b/st2client/tests/fixtures/execution_unicode.txt @@ -1,7 +1,11 @@ id: 547e19561e2e2417d3dde321 -status: succeeded (1s elapsed) +action.ref: core.local +context.user: stanley parameters: cmd: "echo '‡'" +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: failed: false return_code: 0 diff --git a/st2client/tests/unit/test_formatters.py b/st2client/tests/unit/test_formatters.py index e9ef0971b8..40289d6423 100644 --- a/st2client/tests/unit/test_formatters.py +++ b/st2client/tests/unit/test_formatters.py @@ -116,7 +116,9 @@ def test_execution_get_default_in_json(self): argv = ['execution', 'get', EXECUTION['id'], '-j'] content = self._get_execution(argv) self.assertEqual(json.loads(content), - jsutil.get_kvps(EXECUTION, ['id', 'status', 'parameters', 'result'])) + jsutil.get_kvps(EXECUTION, ['id', 'action.ref', 'context.user', + 'start_timestamp', 'end_timestamp', 'status', + 'parameters', 'result'])) def test_execution_get_detail(self): argv = ['execution', 'get', EXECUTION['id'], '-d'] @@ -181,6 +183,9 @@ def test_execution_get_detail_in_json(self): # Sufficient to check if output contains all expected keys. The entire result will not # match as content will contain characters which improve rendering. for k in six.iterkeys(EXECUTION): + if k in ['liveaction', 'callback']: + continue + if k in content: continue self.assertTrue(False, 'Missing key %s. %s != %s' % (k, EXECUTION, content_dict)) From 32446df488d04cc1ac9a917eb61ab82d7145c5a7 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 1 Nov 2019 21:33:55 +0100 Subject: [PATCH 73/75] Update changelog. --- CHANGELOG.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 3271a2a0f6..b468e22a3f 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -70,10 +70,12 @@ Fixed (bug fix) #4809 Reported by @guzzijones -* Fixed ``st2 execution get`` and ``st2 run`` not printing the ``action.ref`` - for non-workflow actions. (bug fix) #4739 +* Fixed ``st2 execution get`` and ``st2 run`` not printing the ``action.ref`` for non-workflow + actions. (bug fix) #4739 Contributed by Nick Maludy (@nmaludy Encore Technologies) +* Update ``st2 execution get`` command to always include ``context.user``, ``start_timestamp`` and + ``end_timestamp`` attributes. (improvement) #4739 3.1.0 - June 27, 2019 --------------------- From 5d3a3a85d9477b529039cb22f8b321b7a8e92229 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 1 Nov 2019 21:40:59 +0100 Subject: [PATCH 74/75] Fix test so it also passes with another common content type for python files. --- st2api/tests/unit/controllers/v1/test_action_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/st2api/tests/unit/controllers/v1/test_action_views.py b/st2api/tests/unit/controllers/v1/test_action_views.py index cd6491b42c..768802c254 100644 --- a/st2api/tests/unit/controllers/v1/test_action_views.py +++ b/st2api/tests/unit/controllers/v1/test_action_views.py @@ -240,7 +240,8 @@ def test_get_one_ref_python_content_type(self): try: get_resp = self.app.get('/v1/actions/views/entry_point/%s' % action_ref) self.assertEqual(get_resp.status_int, 200) - self.assertEqual(get_resp.headers['Content-Type'], 'application/x-python') + self.assertTrue(get_resp.headers['Content-Type'] in ['application/x-python', + 'text/x-python']) finally: self.app.delete('/v1/actions/%s' % action_id) From 1d1aa02944a848ff02a190af52c35d7189a1360a Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 1 Nov 2019 22:25:54 +0100 Subject: [PATCH 75/75] Update more affected tests. --- .../fixtures/execution_result_has_carriage_return_py3.txt | 4 ---- st2client/tests/fixtures/execution_unicode_py3.txt | 6 +++++- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/st2client/tests/fixtures/execution_result_has_carriage_return_py3.txt b/st2client/tests/fixtures/execution_result_has_carriage_return_py3.txt index 1dfa1c59ba..7a10e63a52 100644 --- a/st2client/tests/fixtures/execution_result_has_carriage_return_py3.txt +++ b/st2client/tests/fixtures/execution_result_has_carriage_return_py3.txt @@ -64,8 +64,4 @@ | | "stdout": "" | | | } | | | } | -| liveaction | { | -| | "callback": {}, | -| | "id": "1" | -| | } | +-----------------+--------------------------------------------------------------+ diff --git a/st2client/tests/fixtures/execution_unicode_py3.txt b/st2client/tests/fixtures/execution_unicode_py3.txt index ba96a99346..0db50aa746 100644 --- a/st2client/tests/fixtures/execution_unicode_py3.txt +++ b/st2client/tests/fixtures/execution_unicode_py3.txt @@ -1,7 +1,11 @@ id: 547e19561e2e2417d3dde321 -status: succeeded (1s elapsed) +action.ref: core.local +context.user: stanley parameters: cmd: "echo '\u2021'" +status: succeeded (1s elapsed) +start_timestamp: Tue, 02 Dec 2014 19:56:06 UTC +end_timestamp: Tue, 02 Dec 2014 19:56:07 UTC result: failed: false return_code: 0