st2 icon indicating copy to clipboard operation
st2 copied to clipboard

Rule should match if optional action parameters are not specified in payload

Open pietervogelaar opened this issue 8 years ago • 20 comments

/opt/stackstorm/packs/my_application/actions/deploy.yaml

---
name: 'deploy'
description: 'Workflow to deploy an application'
runner_type: 'mistral-v2'
entry_point: 'workflows/deploy.yaml'
enabled: true
parameters:
  repo_url:
    description: The repository URL of the (compiled) application
    required: true
    position: 0
  server_application:
    description: The application/purpose of a server to deploy this application to
    required: true
    position: 1
  server_role:
    description: The role of a server to deploy this application to. Possible values are "tomcat", "apache", "postgres", "rabbitmq", etc
    required: true
    position: 2
  server_environment:
    description: The environment of a server to deploy this application to. Possible values are "prd", "acc", "tst" or "dev"
    required: true
    position: 3
  target_dir:
    description: The target directory for the application code
    position: 4
  pre_update_command:
    description: 'This command is executed locally before code is updated'
    position: 5
  post_update_command:
    description: 'This command is executed locally after code is updated'
    position: 6

/opt/stackstorm/packs/my_application/triggers/deploytrigger.yaml

---
name: deploytrigger
description: 'Application deployment trigger.'

/opt/stackstorm/packs/my_application/rules/deploy.yaml

---
name: deploy
description: "Deployment rule that can be called for an application deployment from Jenkins or something similar."
enabled: true

trigger:
  type: my_application.deploytrigger

action:
  ref: "my_application.deploy"
  parameters:
    repo_url: "{{trigger.repo_url}}"
    server_application: "{{trigger.server_application}}"
    server_role: "{{trigger.server_role}}"
    server_environment: "{{trigger.server_environment}}"
    target_dir: "{{trigger.target_dir}}"
    pre_update_command: "{{trigger.pre_update_command}}"
    post_update_command: "{{trigger.post_update_command}}"

/tmp/trigger-instance.yaml

---
trigger: "my_application.deploytrigger"
payload:
  repo_url: "ssh://[email protected]/my/helloworldapp.git"
  server_application: "shared"
  server_role: "apache"
  server_environment: "tst"
  target_dir: "/srv/helloworldapp"
  pre_update_command: ~
  post_update_command: ~

A rule DOES match if I run the rule tester with all the workflow/action parameters defined in the trigger instance payload:

st2-rule-tester --config-file /etc/st2/st2.conf --rule=/opt/stackstorm/packs/my_application/rules/deploy.yaml --trigger-instance=/tmp/trigger-instance.yaml --debug --verbose

However if I DON'T specify the optional workflow parameters "target_dir", "pre_update_command" and "post_update_command" there is NO rule match. It would be nice if optional workflow/action parameters are not required in the trigger payload.

pietervogelaar avatar Feb 14 '17 09:02 pietervogelaar

Thanks for opening / reporting this.

We are a bit busy with a release at the moment, but will have a look as soon as possible.

Kami avatar Feb 14 '17 18:02 Kami

@pietervogelaar Did you try with explicitly specifying criteria: {} in the rule YAML? The check on rule match doesn't really depend on the parameters at all. So I am puzzled at this behavior. We'll have a look post release but meanwhile if you can try my suggestion and report, it would be super useful.

lakshmi-kannan avatar Feb 17 '17 21:02 lakshmi-kannan

@lakshmi-kannan I added criteria: {}, but that didn't make a difference. So even with criteria: {}, all parameters including the optional must be specified to get a matching rule.

pietervogelaar avatar Feb 20 '17 09:02 pietervogelaar

Today I discovered another example which seems to have the same issue.

In a certain action I have the following notify syntax:

notify:
  on-complete:
    message: "Get hosts action succeeded. Action was started by {{action_context.user}}."
    routes:
      - 'chatops'
    data:
      channel: 'it'
      user: 'pietervogelaar'

The rule I have is:

---
name: 'notify_chatops_message'
description: 'Notification rule to send a message to stream for chatops.'
enabled: true

trigger:
  type: 'core.st2.generic.notifytrigger'
  parameters: {}

criteria:
  trigger.channel:
    pattern: 'chatops'
    type: 'equals'

action:
  ref: 'chatops.post_message'
  parameters:
    message: "{{trigger.message}}"
    channel: "{{trigger.data.channel}}"
    user: "{{trigger.data.user}}"

If I don't specify data.channel and data.user at the action notify, the rule won't match. So again, required and optional parameters must be specified to get a rule match.

pietervogelaar avatar Feb 21 '17 12:02 pietervogelaar

Thanks from reporting this.

I didn't really dig in yet (for that, we would need to see rules engine log), but from what you posted it seems that the rule does indeed match, but the problem is with the action execution - action is not executed (it fails) because a mandatory parameter references in the action.parameters section is not available on the trigger.

Kami avatar Feb 21 '17 12:02 Kami

I'm not sure what you mean, I also tried this:

---
name: 'notify_chatops_message'
description: 'Notification rule to send a message to stream for chatops.'
enabled: true

trigger:
  type: 'core.st2.generic.notifytrigger'
  parameters: {}

criteria:
  trigger.channel:
    pattern: 'chatops'
    type: 'equals'

action:
  ref: 'chatops.post_message'
  parameters:
    message: "{{trigger.message}}"
    channel: "{% if trigger.data.channel %}{{trigger.data.channel}}{% endif %}"
    user: "{% if trigger.data.user %}{{trigger.data.user}}{% endif %}"

But this didn't make a difference.

pietervogelaar avatar Feb 21 '17 12:02 pietervogelaar

Please run rules engine service in foreground with --debug flag and gist the log so we can troubleshoot what is going on - https://docs.stackstorm.com/troubleshooting/debug_mode.html

Kami avatar Feb 21 '17 12:02 Kami

Only channel and message are required parameters in the chatops.post_message action. The user parameter is optional. If i DO specify message and channel, and only leave out user in the action notify it still doesn't match.

pietervogelaar avatar Feb 21 '17 12:02 pietervogelaar

2017-02-21 14:02:01,947 49482640 ERROR enforcer [-] Failed kicking off execution for rule RuleDB(action=ActionExecutionSpecDB@49949776(ref="chatops.post_message", parameters="{u'message': u'{{trigger.message}}', u'user': u'{{trigger.data.user}}', u'channel': u'{{trigger.data.channel}}'}"), criteria={u'trigger.channel': {u'pattern': u'chatops', u'type': u'equals'}}, description="Notification rule to send a message to stream for chatops.", enabled=True, id=58ac2efcf970ac2b3b65f0b6, name="notify_chatops_message", pack="crv_chatops", ref="crv_chatops.notify_chatops_message", tags=[], trigger="core.st2.generic.notifytrigger", type=RuleTypeSpecDB@49949904(ref="standard", parameters="{}"), uid="rule:crv_chatops:notify_chatops_message").
Traceback (most recent call last):
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2reactor/rules/enforcer.py", line 74, in enforce
    execution_db = self._do_enforce()
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2reactor/rules/enforcer.py", line 98, in _do_enforce
    params = self.get_resolved_parameters()
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2reactor/rules/enforcer.py", line 62, in get_resolved_parameters
    return self.data_transformer(self.rule.action.parameters)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2reactor/rules/datatransform.py", line 37, in __call__
    return jinja_utils.render_values(mapping=mapping, context=context)
  File "/opt/stackstorm/st2/lib/python2.7/site-packages/st2common/util/jinja.py", line 143, in render_values
    raise e
UndefinedError: 'dict object' has no attribute 'user' (_rule_db={'description': u'Notification rule to send a message to stream for chatops.', 'tags': [], 'ref': u'crv_chatops.notify_chatops_message', 'enabled': True, 'name': u'notify_chatops_message', 'trigger': u'core.st2.generic.notifytrigger', 'criteria': {u'trigger.channel': {u'pattern': u'chatops', u'type': u'equals'}}, 'action': 'ActionExecutionSpecDB@49949776(ref="chatops.post_message", parameters="{u\'message\': u\'{{trigger.message}}\', u\'user\': u\'{{trigger.data.user}}\', u\'channel\': u\'{{trigger.data.channel}}\'}")', 'pack': u'crv_chatops', 'type': 'RuleTypeSpecDB@49949904(ref="standard", parameters="{}")', 'id': '58ac2efcf970ac2b3b65f0b6', 'uid': u'rule:crv_chatops:notify_chatops_message'},_trigger_instance_db={'status': 'processing', 'occurrence_time': '2017-02-21 13:02:01.854296+00:00', 'trigger': u'core.st2.generic.notifytrigger', 'id': '58ac3a49f970ac4a48be4840', 'payload': {'status': u'succeeded', 'start_timestamp': '2017-02-21T13:02:00.679561+00:00', 'route': u'chatops', 'runner_ref': u'python-script', 'channel': u'chatops', 'action_ref': u'crv_core.get_hosts', 'data': {'result': '{"result": ["training-base-04t.adinfra.example.org", "training-base-06t.adinfra.example.org", "training-base-10t.adinfra.example.org", "training-base-12t.adinfra.example.org", "training-base-14t.adinfra.example.org", "training-base-16t.adinfra.example.org", "training-base-18t.adinfra.example.org", "training-base-20t.adinfra.example.org", "training-base-22t.adinfra.example.org", "training-base-24t.adinfra.example.org", "training-base-26t.adinfra.example.org", "training-base-30t.adinfra.example.org", "training-base-32t.adinfra.example.org", "training-base-34t.adinfra.example.org"], "exit_code": 0, "stderr": "", "stdout": ""}', u'channel': u'it'}, 'message': u'Get hosts action succeeded. Action was started by VogelaarP.', 'execution_id': '58ac3a48f970ac4b38ca1590', 'end_timestamp': '2017-02-21T13:02:01.756399+00:00'}})

pietervogelaar avatar Feb 21 '17 13:02 pietervogelaar

Thanks for including the logs - it looks like my assumption was correct.

The rule does indeed match, but the execution fails because parameter value Jinja conditional refers to the value which is not available on the Trigger.

There are two solution - the correct one would be to make sure that the trigger always includes all the attributes which are then accessed inside the action (and they should be None if value is not set).

Another option would be for us, to potentially change the code so the conditional Jinja expression would return None (need to verify that one, will look into it), but this could make some things harder to debug and we would really be masking the "root cause" which is the previously mentioned trigger not containing all the attributes.

Kami avatar Feb 21 '17 13:02 Kami

I added jinja if conditions to fix the python error:

---
name: 'notify_chatops_message'
description: 'Notification rule to send a message to stream for chatops.'
enabled: true

trigger:
  type: 'core.st2.generic.notifytrigger'
  parameters: {}

criteria:
  trigger.channel:
    pattern: 'chatops'
    type: 'equals'

action:
  ref: 'chatops.post_message'
  parameters:
    message: "{{trigger.message}}"
    channel: "{% if trigger.data and trigger.data.channel %}{{trigger.data.channel}}{% endif %}"
    user: "{% if trigger.data and trigger.data.user %}{{trigger.data.user}}{% endif %}"

Now I only get the following:

2017-02-21 14:15:12,585 49482320 INFO engine [-] Found 0 rules defined for trigger st2.generic.actiontrigger (type=core.st2.generic.actiontrigger)
2017-02-21 14:15:12,585 49482320 DEBUG matcher [-] [1st_pass] 0 rule(s) found to enforce for st2.generic.actiontrigger.
2017-02-21 14:15:12,586 49482320 DEBUG matcher [-] [2nd_pass] 0 rule(s) found to enforce for st2.generic.actiontrigger.
2017-02-21 14:15:12,586 49482320 INFO matcher [-] 0 rule(s) found to enforce for st2.generic.actiontrigger.
2017-02-21 14:15:12,586 49482320 INFO engine [-] Matched 0 rule(s) for trigger_instance st2.generic.actiontrigger (type=core.st2.generic.actiontrigger)

Which brings us back to the originating problem that there SHOULD be a match.

pietervogelaar avatar Feb 21 '17 13:02 pietervogelaar

The log messages you posted seem to refer to a different trigger (generic action trigger and not notifytrigger). The changes you made to the action section shouldn't affect the match itself, just the parameter resolution during the action execution part.

I confirmed and the option I mentioned above should work and it's quite easy to implement - allowing user to refer to attributes which are not defined inside the trigger and Jinja simply returning None for the value (in this case the single if you posted above would work).

Having said that, it looks like that way it currently works is intentional - silently failing and returning "None" could potentially be dangerous and mask the root cause aka real issues with other systems which are injecting triggers into the system (e.g. sensor not correctly population all the trigger instance fields, webhook not containing all the required attributes, etc.). In addition to that, I'm also a proponent of "fail loudly and early" approach - imo, it's better for rules engine to throw (yes, we could improve the error message) then "silently" setting the value to None which could cause all kinds of issues down the chain and make things hard to debug.

That's my current take, but it would also be good to have others (cc @lakshmi-kannan @m4dcoder) to chime in and with enough arguments I could be convinced otherwise :)

Kami avatar Feb 21 '17 13:02 Kami

In short, the solution for you right now is to make sure trigger instance contains values for all the attributes (parameters) which are referenced inside the action.parameters section.

Depending on the trigger and rule in question, this might need fixing / updating a sensor or 3rd party integration or other part of the system if the problem is that not all the required attributes are specified (e.g. changing where core.st2.generic.notifytrigger is dispatched).

Edit: Looked at the example again, you are referring to trigger.data.channel which should be set on the action basis inside the notify: section.

Kami avatar Feb 21 '17 13:02 Kami

Okay, for now I will just have to specify all the parameters for the trigger instance. It still feels not completely right, because optional action parameters should not have to be specified.

pietervogelaar avatar Feb 21 '17 13:02 pietervogelaar

@pietervogelaar I created a sample pack to repro your report but I couldn't. Here is a pack with a rule that just passes one argument for an action (action has two arguments, one required and one optional). The rule matches and the action execution gets kicked off.

git clone https://github.com/lakshmi-kannan/st2_optional_args_pack /opt/stackstorm/packs/optional_args

st2ctl reload --register-all

Here is my sample payload:

(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯ cat payload.json                                                                       
{
  "arg1": 5
}
(virtualenv)vagrant@st2dev /m/s/s/st2 ❯❯❯

Here's relevant st2 information from my repro: https://gist.github.com/lakshmi-kannan/a6e601b9eab33db386b3e6bfd4d94728.

lakshmi-kannan avatar Feb 23 '17 00:02 lakshmi-kannan

In my case it's a mistral workflow, instead of an direct action. Could that be different?

pietervogelaar avatar Feb 23 '17 08:02 pietervogelaar

I got the same problem (missing parameter passing from notify section) and wondered why it wasn't triggered at all, and without any error message to help pinpoint the cause. (FE: "cannot render parameter template" or something like that)

vincent-legoll avatar Oct 19 '17 13:10 vincent-legoll

I am facing a similar issue at the moment. I have an action that requires 1 parameter and has 10 optional parameters. I have created a webhook that triggers the execution of this action based on the payload. However, I want to be able to provide a payload with a dt of keys. Currently, my rule considers all parameters, and if any of them are not provided in the payload, the action cannot be started at all.

i found a workaround:

pack: "mypack"
enabled: true
trigger:
  parameters:
    url: "some-webhook"
  type: core.st2.webhook
action:
  parameters:
    required_parameter: "{{trigger.body.required_parameter}}"
    optional_parameter: "{{trigger.body.optional_parameter | default(None) | use_none}}"
  ref: mypack.action-with-optional-params

but this only works with optional parameters of the type "string" and not with integers. This is the error that I got from the rulesengine when trying to set None for an OPTIONAL integer parameter :TypeError: int() argument must be a string, a bytes-like object or a number, not 'NoneType'

It all comes down to the fact that I can't send None or any value that would represent an undefined state to an action. The parameter is expected to be excluded from the payload.

kaluginadaria avatar Mar 13 '24 15:03 kaluginadaria

Have you tried setting the "optional_parameter" default value to an integer? Something like:

optional_parameter: "{{trigger.body.optional_parameter | default(0), true }}"

If the action you are attempting to run is something you created internally, then it may be a option to make the parameter "required" but set the default value on the action definition. Something along the lines:

parameters:
  req_param:
    type: integer
    description: A required action input parameter with a default value when not specified
    required: true
    default: 0

At least in this manner, you can add code to the action to process and address the default value when it is not provided.

jamesdreid avatar Mar 18 '24 11:03 jamesdreid

@jamesdreid, thank you for your suggestion. In my case, the value 0 is also considered valid and is different from "undefined". As a solution, I set my parameters to a string type (even though they are numbers) and used the following in the rule definition:

optional_parameter: "{{trigger.body.optional_parameter | default(None) | use_none}}"

Then, I processed "None" as a string and excluded it in the action code.

kaluginadaria avatar Mar 20 '24 15:03 kaluginadaria