Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions conf/st2.dev.conf
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ api_url = http://127.0.0.1:9101/
[system]
debug = True
base_path = /opt/stackstorm
validate_trigger_parameters = True

[garbagecollector]
logging = st2reactor/conf/logging.garbagecollector.conf
Expand Down
6 changes: 3 additions & 3 deletions contrib/examples/rules/sample_rule_file_watch.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@
name: sample_rule_file_watch
pack: "examples"
description: Sample rule custom trigger type - add a file to be watched by file_watch_sensor in linux pack.
enabled: false
enabled: true

trigger:
parameters:
file_path: /var/log/dmesg
type: linux.file_watch.file_path
file_path: /tmp/st2_test
type: linux.file_watch.line

criteria: {}

Expand Down
29 changes: 29 additions & 0 deletions contrib/linux/sensors/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
## NOTICE

File watch sensor has been updated to use trigger with parameters supplied via a rule approach. Tailing a file path supplied via a config file is now deprecated.

An example rule to supply a file path is as follows:

```
---
name: sample_rule_file_watch
pack: "examples"
description: Sample rule custom trigger type - add a file to be watched by file_watch_sensor in linux pack.
enabled: false

trigger:
parameters:
file_path: /tmp/st2_test
type: linux.file_watch.line

criteria: {}

action:
parameters:
cmd: echo "{{trigger}}"
ref: core.local

```

Trigger ``linux.file_watch.line`` still emits the same payload as it used to.
Just the way to provide the file_path to tail has changed.
15 changes: 10 additions & 5 deletions contrib/linux/sensors/file_watch_sensor.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@ class FileWatchSensor(Sensor):
def __init__(self, sensor_service, config=None):
super(FileWatchSensor, self).__init__(sensor_service=sensor_service,
config=config)
self._trigger_ref = 'linux.file_watch.line'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kami @dzimine This turned out to the root cause of all problems. Basically, I was using the trigger type as the "ref" which is incorrect. I should either be using "type"and "parameters" or the actual ref for the trigger object we create for this instance of type and parameters. For example,

Trigger type:

{ "_id" : ObjectId("59381e4ad9d7ed39ed238337"), "description" : "Trigger which indicates a new line has been detected", "tags" : [ ], "uid" : "trigger_type:linux:file_watch.line", "name" : "file_watch.line", "pack" : "linux", "payload_schema" : { "type" : "object", "properties" : { "file_name" : { "type" : "string" }, "line" : { "type" : "string" }, "file_path" : { "type" : "string" } } }, "parameters_schema" : { "additionalProperties" : false, "type" : "object", "properties" : { "file_path" : { "required" : true, "type" : "string", "description" : "Path to the file to monitor" } } } }

Trigger:

{ "_id" : ObjectId("59381e71d9d7ed39ed238524"), "uid" : "trigger:linux:7c883eb8-376d-40f4-a0f1-0cb3baa46ab9:851746484aeb7ece7f72c1e68c2d91a3", "name" : "7c883eb8-376d-40f4-a0f1-0cb3baa46ab9", "pack" : "linux", "type" : "linux.file_watch.line", "parameters" : { "file_path" : "/tmp/st2_test" }, "ref_count" : 1 }

And if you look at the rule, it's tied to the trigger ref we created and it has no knowledge of type and parameters.

{ "_id" : ObjectId("59381e71d9d7ed39ed238525"), "tags" : [ ], "uid" : "rule:examples:sample_rule_file_watch", "name" : "sample_rule_file_watch", "ref" : "examples.sample_rule_file_watch", "description" : "Sample rule custom trigger type - add a file to be watched by file_watch_sensor in linux pack.", "pack" : "examples", "type" : { "ref" : "standard", "parameters" : {  } }, "trigger" : "linux.7c883eb8-376d-40f4-a0f1-0cb3baa46ab9", "criteria" : {  }, "action" : { "ref" : "core.local", "parameters" : { "cmd" : "echo \"{{trigger}}\"" } }, "enabled" : true }

So for a single trigger type, for each instance of parameters, we end up creating a rule with a different "trigger". The trigger type information is only used to verify that the parameters someone supplies matches the schema. I have to verify if we throw an error if the parameters don't match. I'll validate that part.

In essence, the sensor should either use the actual "ref" for the trigger or a dict containing "type" and "parameters" so we can do a lookup and get the actual trigger ref ourselves. Everything from trigger instance to rules engine only understands trigger ref. They don't understand type and parameters.

Copy link
Contributor Author

@lakshmi-kannan lakshmi-kannan Jun 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to verify if we throw an error if the parameters don't match. I'll validate that part.

I verified by swapping "file_path" to "file_route" and the rule registered successfully without throwing errors :/. See trigger below

{ "_id" : ObjectId("593823d2d9d7ed49861671ad"), "uid" : "trigger:linux:745a7d8a-cc6e-458b-987d-a6b67de56761:15c31053a00d27a5438d559253101833", "name" : "745a7d8a-cc6e-458b-987d-a6b67de56761", "pack" : "linux", "type" : "linux.file_watch.line", "parameters" : { "file_route" : "/tmp/st2_test" }, "ref_count" : 1 }

This is a bug and it should be fixed. #3453

self._trigger = None
self._logger = self._sensor_service.get_logger(__name__)

self._file_paths = [] # stores a list of file paths we are monitoring
self._tail = None

def setup(self):
Expand All @@ -39,8 +37,12 @@ def add_trigger(self, trigger):
self._logger.error('Received trigger type without "file_path" field.')
return

self._tail.add_file(filename=file_path)
self._trigger = trigger.get('ref', None)

if not self._trigger:
raise Exception('Trigger %s did not contain a ref.' % trigger)

self._tail.add_file(filename=file_path)
self._logger.info('Added file "%s"' % (file_path))

def update_trigger(self, trigger):
Expand All @@ -54,14 +56,17 @@ def remove_trigger(self, trigger):
return

self._tail.remove_file(filename=file_path)
self._trigger = None

self._logger.info('Removed file "%s"' % (file_path))

def _handle_line(self, file_path, line):
trigger = self._trigger_ref
trigger = self._trigger
payload = {
'file_path': file_path,
'file_name': os.path.basename(file_path),
'line': line
}
self._logger.debug('Sending payload %s for trigger %s to sensor_service.',
payload, trigger)
self.sensor_service.dispatch(trigger=trigger, payload=payload)
12 changes: 5 additions & 7 deletions contrib/linux/sensors/file_watch_sensor.yaml
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
---
class_name: "FileWatchSensor"
enabled: false
enabled: true
entry_point: "file_watch_sensor.py"
description: "Sensor which monitors files for new lines"
trigger_types:
-
name: "file_watch.file_path"
name: "file_watch.line"
pack: "linux"
description: "Trigger which represents a file path to be monitored"
description: "Trigger which indicates a new line has been detected"
# This sensor can be supplied a path to a file to tail via a rule.
parameters_schema:
type: "object"
properties:
Expand All @@ -16,10 +17,7 @@
type: "string"
required: true
additionalProperties: false
-
name: "file_watch.line"
pack: "linux"
description: "Trigger which indicates a new line has been detected"
# This is the schema of the trigger payload the sensor generates
payload_schema:
type: "object"
properties:
Expand Down
2 changes: 2 additions & 0 deletions st2api/tests/unit/controllers/v1/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from st2api.controllers.v1.webhooks import WebhooksController, HooksHolder

from st2common.constants.triggers import WEBHOOK_TRIGGER_TYPES
from st2common.models.api.trigger import TriggerAPI
from st2common.models.db.trigger import TriggerDB
from st2common.transport.reactor import TriggerInstancePublisher

Expand All @@ -44,6 +45,7 @@

DUMMY_TRIGGER = TriggerDB(name='pr-merged', pack='git')
DUMMY_TRIGGER.type = WEBHOOK_TRIGGER_TYPES.keys()[0]
DUMMY_TRIGGER_API = TriggerAPI.from_model(DUMMY_TRIGGER)


class TestWebhooksController(FunctionalTest):
Expand Down
12 changes: 6 additions & 6 deletions st2api/tests/unit/controllers/v1/test_webhooks_rbac.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
from st2tests.fixturesloader import FixturesLoader
from tests.base import APIControllerWithRBACTestCase

from tests.unit.controllers.v1.test_webhooks import DUMMY_TRIGGER
from tests.unit.controllers.v1.test_webhooks import DUMMY_TRIGGER, DUMMY_TRIGGER_API

http_client = six.moves.http_client

Expand Down Expand Up @@ -107,7 +107,7 @@ def test_get_all_no_permissions(self):
self.assertEqual(resp.json['faultstring'], expected_msg)

@mock.patch.object(HooksHolder, 'get_triggers_for_hook', mock.MagicMock(
return_value=[vars(DUMMY_TRIGGER)]))
return_value=[DUMMY_TRIGGER_API]))
def test_get_one_no_permissions(self):
user_db = self.users['no_permissions']
self.use_user(user_db)
Expand All @@ -124,9 +124,9 @@ def test_get_one_no_permissions(self):
self.assertEqual(resp.json['faultstring'], expected_msg)

@mock.patch.object(HooksHolder, 'get_all', mock.MagicMock(
return_value=[vars(DUMMY_TRIGGER)]))
return_value=[DUMMY_TRIGGER_API]))
@mock.patch.object(HooksHolder, 'get_triggers_for_hook', mock.MagicMock(
return_value=[vars(DUMMY_TRIGGER)]))
return_value=[DUMMY_TRIGGER_API]))
def test_get_all_permission_success_get_one_no_permission_failure(self):
user_db = self.users['webhook_list']
self.use_user(user_db)
Expand All @@ -148,9 +148,9 @@ def test_get_all_permission_success_get_one_no_permission_failure(self):
self.assertEqual(resp.json['faultstring'], expected_msg)

@mock.patch.object(HooksHolder, 'get_all', mock.MagicMock(
return_value=[vars(DUMMY_TRIGGER)]))
return_value=[DUMMY_TRIGGER_API]))
@mock.patch.object(HooksHolder, 'get_triggers_for_hook', mock.MagicMock(
return_value=[vars(DUMMY_TRIGGER)]))
return_value=[DUMMY_TRIGGER_API]))
def test_get_one_permission_success_get_all_no_permission_failure(self):
user_db = self.users['webhook_view']
self.use_user(user_db)
Expand Down
2 changes: 2 additions & 0 deletions st2common/st2common/models/db/trigger.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class TriggerTypeDB(stormbase.StormBaseDB,
RESOURCE_TYPE = ResourceType.TRIGGER_TYPE
UID_FIELDS = ['pack', 'name']

ref = me.StringField(required=False)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made it required=False so people upgrading don't have issues. At some point, we should make it required.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also see why this "ref" change is required. #3454

name = me.StringField(required=True)
pack = me.StringField(required=True, unique_with='name')
payload_schema = me.DictField()
Expand Down Expand Up @@ -75,6 +76,7 @@ class TriggerDB(stormbase.StormBaseDB, stormbase.ContentPackResourceMixin,
RESOURCE_TYPE = ResourceType.TRIGGER
UID_FIELDS = ['pack', 'name']

ref = me.StringField(required=False)
name = me.StringField(required=True)
pack = me.StringField(required=True, unique_with='name')
type = me.StringField()
Expand Down
59 changes: 59 additions & 0 deletions st2common/st2common/services/rules.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Licensed to the StackStorm, Inc ('StackStorm') under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You 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.

import six

from st2common import log as logging
from st2common.persistence.rule import Rule


LOG = logging.getLogger(__name__)

__all__ = [
'get_rules_given_trigger',
'get_rules_with_trigger_ref'
]


def get_rules_given_trigger(trigger):

if isinstance(trigger, six.string_types):
return get_rules_with_trigger_ref(trigger_ref=trigger)

if isinstance(trigger, dict):
trigger_ref = trigger.get('ref', None)
if trigger_ref:
return get_rules_with_trigger_ref(trigger_ref=trigger_ref)
else:
raise ValueError('Trigger dict %s is missing ``ref``.' % trigger)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parenthesis around % (trigger) would make it a bit less error prone in the future.


raise ValueError('Unknown type %s for trigger. Cannot do rule lookups.' % type(trigger))


def get_rules_with_trigger_ref(trigger_ref=None, enabled=True):
"""
Get rules in DB corresponding to given trigger_ref as a string reference.
:param trigger_ref: Reference to trigger.
:type trigger_ref: ``str``
:rtype: ``list`` of ``RuleDB``
"""

if not trigger_ref:
return None

LOG.debug('Querying rules with trigger %s', trigger_ref)
return Rule.query(trigger=trigger_ref, enabled=enabled)
2 changes: 2 additions & 0 deletions st2reactor/st2reactor/container/sensor_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ def dispatch(self, trigger, payload=None, trace_tag=None):
"""
# empty strings
trace_context = TraceContext(trace_tag=trace_tag) if trace_tag else None
self._logger.debug('Added trace_context %s to trigger %s.', trace_context, trigger)
self.dispatch_with_context(trigger, payload=payload, trace_context=trace_context)

def dispatch_with_context(self, trigger, payload=None, trace_context=None):
Expand Down Expand Up @@ -118,6 +119,7 @@ def dispatch_with_context(self, trigger, payload=None, trace_context=None):
'dispatching a trigger "%s" (%s)' % (trigger, str(payload)))
return None

self._logger.debug('Dispatching trigger %s with payload %s.', trigger, payload)
self._dispatcher.dispatch(trigger, payload=payload, trace_context=trace_context)

##################################
Expand Down
2 changes: 1 addition & 1 deletion st2reactor/st2reactor/container/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ def create_trigger_instance(trigger, payload, occurrence_time, raise_on_no_trigg
trigger_db = TriggerService.get_trigger_db_given_type_and_params(type=trigger_type,
parameters=parameters)

if trigger_db is None:
if not trigger_db:
LOG.debug('No trigger in db for %s', trigger)
if raise_on_no_trigger:
raise StackStormDBObjectNotFoundError('Trigger not found for %s', trigger)
Expand Down
38 changes: 26 additions & 12 deletions st2reactor/st2reactor/rules/engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.

from st2common import log as logging
from st2common.persistence.rule import Rule
from st2common.services.rules import get_rules_given_trigger
from st2common.services.triggers import get_trigger_db_by_ref
from st2reactor.rules.enforcer import RuleEnforcer
from st2reactor.rules.matcher import RulesMatcher
Expand All @@ -27,24 +27,38 @@ def handle_trigger_instance(self, trigger_instance):
# Find matching rules for trigger instance.
matching_rules = self.get_matching_rules_for_trigger(trigger_instance)

# Create rule enforcers.
enforcers = self.create_rule_enforcers(trigger_instance, matching_rules)
if matching_rules:
# Create rule enforcers.
enforcers = self.create_rule_enforcers(trigger_instance, matching_rules)

# Enforce the rules.
self.enforce_rules(enforcers)
# Enforce the rules.
self.enforce_rules(enforcers)
else:
LOG.info('No matching rules found for trigger instance %s.', trigger_instance['id'])

def get_matching_rules_for_trigger(self, trigger_instance):
trigger = trigger_instance.trigger
trigger = get_trigger_db_by_ref(trigger_instance.trigger)
rules = Rule.query(trigger=trigger_instance.trigger, enabled=True)
LOG.info('Found %d rules defined for trigger %s (type=%s)', len(rules), trigger['name'],
trigger['type'])

trigger_db = get_trigger_db_by_ref(trigger_instance.trigger)

if not trigger_db:
LOG.error('No matching trigger found in db for trigger instance %s.', trigger_instance)
return None

rules = get_rules_given_trigger(trigger=trigger)

LOG.info('Found %d rules defined for trigger %s', len(rules),
trigger_db.get_reference().ref)

if len(rules) < 1:
return rules

matcher = RulesMatcher(trigger_instance=trigger_instance,
trigger=trigger, rules=rules)
trigger=trigger_db, rules=rules)

matching_rules = matcher.get_matching_rules()
LOG.info('Matched %s rule(s) for trigger_instance %s (type=%s)', len(matching_rules),
trigger['name'], trigger['type'])
LOG.info('Matched %s rule(s) for trigger_instance %s (trigger=%s)', len(matching_rules),
trigger_instance['id'], trigger_db.ref)
return matching_rules

def create_rule_enforcers(self, trigger_instance, matching_rules):
Expand Down