Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chatops.post_message seems to be ignoring Slack pretext #162

Open
lingfish opened this issue Feb 21, 2019 · 23 comments
Open

chatops.post_message seems to be ignoring Slack pretext #162

lingfish opened this issue Feb 21, 2019 · 23 comments

Comments

@lingfish
Copy link

Originally logged here: StackStorm/st2#4561, @nzlosh suggested I might re-log it here.

SUMMARY

chatops.post_message seems to be ignoring Slack pretext.

ISSUE TYPE

Pick one below and delete the rest:

  • Bug Report
STACKSTORM VERSION

st2 2.10.1, on Python 2.7.6

OS / ENVIRONMENT / INSTALL METHOD

Standard docker install.

STEPS TO REPRODUCE
st2 run chatops.post_message channel=testing message='To get a list of commands type: ```!help```' extra='{"slack": {"color":"#f48527","pretext":"Hey <!channel>, Ready for ChatOps?","title": "Ansible and ChatOps. Get started :rocket:","title_link":"https://stackstorm.com/2015/06/24/ansible-chatops-get-started-%f0%9f%9a%80/","author_name":"by StackStorm - IFTTT for Ops","author_link":"https://stackstorm.com/","author_icon":"https://stackstorm.com/wp/wp-content/uploads/2015/01/favicon.png","image_url":"https://i.imgur.com/HWN8T78.png","fields":[{"title":"Documentation","value":"https://docs.stackstorm.com/chatops/","short":true}]}}' > /dev/null
EXPECTED RESULTS

A pretext line in the Slack channel.

ACTUAL RESULTS

Pretext line missing.

@blag
Copy link
Contributor

blag commented Feb 21, 2019

Is the rest of the Slack message appearing? With the correct color, title, title link, author, author link, author icon, image, and fields?

@blag blag added the bug label Feb 21, 2019
@lingfish
Copy link
Author

Yes, certainly is (sorry, should have mentioned that).

@lingfish
Copy link
Author

Any thoughts here? Anything I can do to help?

@blag
Copy link
Contributor

blag commented Feb 28, 2019

I don't think your usage of the slack key in the extra parameter is correct, or more to the point, I don't think you are using the pretext key correctly.

TL;DR: I think you are interpreting Slack's API incorrectly.

In the linked st2 issue, the following extra parameter data is verified as working correctly with Mattermost (which copies Slack's attachments API):

{
  "color": "#474C5D",
  "mattermost": {
    "attachments": [
      {
        "pretext": "---",
        "actions": [
          {
            "integration": {
              "url": "https://example.com",
              "context": {
                "some": "value"
              }
            },
            "name": "button name"
          }
        ],
        "text": "example"
      }
    ]
  }
}

Note the attachments key, and the fact that pretext appears within an object in a list of attachments. That is what I believe Slack is expecting.

This is the JSON that you are sending in the extra parameter:

{
  "slack": {
    "color": "#f48527",
    "pretext": "Hey <!channel>, Ready for ChatOps?",
    "title": "Ansible and ChatOps. Get started :rocket:",
    "title_link": "https://stackstorm.com/2015/06/24/ansible-chatops-get-started-%f0%9f%9a%80/",
    "author_name": "by StackStorm - IFTTT for Ops",
    "author_link": "https://stackstorm.com/",
    "author_icon": "https://stackstorm.com/wp/wp-content/uploads/2015/01/favicon.png",
    "image_url": "https://i.imgur.com/HWN8T78.png",
    "fields": [
      {
        "title": "Documentation",
        "value": "https://docs.stackstorm.com/chatops/",
        "short": true
      }
    ]
  }
}

See how it's missing the attachments key?

Perusing Slack's messy yet useful documentation, I have not found that pretext is a supported key outside of the attachments API. If you have found otherwise, please point me to it.

In fact, using Slack's own message formatter, I have found that the way you are trying to use pretext is not supported:

We found an error in line 2
Here's what went wrong: we don't recognize the property pretext on message

Using that same formatter, I have verified that pretext is supported as a key of an attachment.

So please try using this data instead:

{
  "color": "#474C5D",
  "slack": {
    "attachments": [
      {
        "color": "#f48527",
        "pretext": "Hey <!channel>, Ready for ChatOps?",
        "title": "Ansible and ChatOps. Get started :rocket:",
        "title_link": "https://stackstorm.com/2015/06/24/ansible-chatops-get-started-%f0%9f%9a%80/",
        "author_name": "by StackStorm - IFTTT for Ops",
        "author_link": "https://stackstorm.com/",
        "author_icon": "https://stackstorm.com/wp/wp-content/uploads/2015/01/favicon.png",
        "image_url": "https://i.imgur.com/HWN8T78.png",
        "fields": [
          {
            "title": "Documentation",
            "value": "https://docs.stackstorm.com/chatops/",
            "short": true
          }
        ]
      }
    ]
  }
}

@nzlosh
Copy link
Contributor

nzlosh commented Mar 1, 2019

@blag Requiring attachments isn't consistent with StackStorm ChatOps documentation https://docs.stackstorm.com/chatops/aliases.html#passing-attachment-api-parameters-slack-only

Slack formats ChatOps output as attachments, and you can configure the API parameters in the result.extra.slack field.

You can see in the example that extra.slack doest not contain the attachments key, but starts with keys color, fields and image directly below slack.

I assume the Mattermost adapter code handles things differently which is why attachments is required?

What is the correct way Slack extra paramters should be handled? I know you've been working on updating hubot components, has there been changes in the slack adapter that mean the st2 documentation is no longer correct?

@lingfish
Copy link
Author

lingfish commented Mar 1, 2019

Yeah, basically what @nzlosh said. The CLI example I originally pasted is actually from an example somewhere in the st2 repos, but that said, the doco does indicate that using:

    "slack_extra": {
      "slack": {

Uses attachments in the "backend" when sent to Slack.

An example of my trigger instance (which clearly shows up as a card/attachment in Slack):

    "slack_extra": {
      "slack": {
        "title": "xxx",
        "color": "danger",
        "text": "An alert",
        "author_name": "ServiceNow",
        "title_link": "https://xxx",
        "mrkdwn_in": [
          "fields"
        ],
        "fields": [
          {
            "short": true,
            "value": "xxx",
            "title": "CI"
          },
          {
            "short": true,
            "value": "2 - High",
            "title": "Priority"
          },
          {
            "short": true,
            "value": "01-Mar-2019 00:17:29",
            "title": "First notified"
          },
          {
            "short": true,
            "value": "Active",
            "title": "External status"
          },
          {
            "short": false,
            "value": "yyy",
            "title": "Last update"
          }
        ],
        "pretext": "Something",
        "fallback": "xxx",
        "author_icon": "https://zzz"
      }
    }

@lingfish
Copy link
Author

Hi @blag, any further thoughts here?

@blag
Copy link
Contributor

blag commented Apr 15, 2019

@lingfish Can you find where the docs stated to use slack_extra? That's definitely a bug in our documentation.

Regarding how to us attachments with Mattermost, using extra.mattermost.attachments should work:

Can you see what data is being sent to the Mattermost server?

Adding end-to-end tests to ChatOps with Mattermost is on my list of things to do after 3.0 is released.

@lingfish
Copy link
Author

Ok, so a few confusing things going on here, including from me ;)

Firstly, I'm using Slack, not Mattermost.

Secondly, the payload being sent is actually this:

{
  "extra": {
    "slack": {

Not what I pasted above; there is no slack_extra -- sorry about that! I must have gotten confused, because I have a rule that looks like this:

---
    name: "notify_slack"
    pack: "xx_st2"
    description: "Send to Slack."
    enabled: true

    trigger:
      type: "xx_st2.evt_trigger"
      parameters: {}
    action:
      ref: "chatops.post_message"
      parameters:
        message: "{{trigger.message}}"
        channel: "testing"
        extra: "{{trigger.slack_extra}}"

My sensor sends that extra data using self._sensor_service.dispatch.

In the doco, it states:

Everything that’s specified in extra.slack will be passed as-is to the Slack Attachment API.

This implies that anything supplied in extra.slack, gets put into an attachments section in the backend, right?

Inserting the above JSON, from extra.slack, directly into Slack's attachment tester, inside the attachments list, renders it perfectly, including the pretext.

{
    "text": "I am a test message http://slack.com",
    "attachments": [
{
        "title": "xxx",
        "color": "danger",
        "text": "An alert",
        "author_name": "ServiceNow",
        "title_link": "https://xxx",
        "mrkdwn_in": [
          "fields"
        ],
        "fields": [
          {
            "short": true,
            "value": "xxx",
            "title": "CI"
          },
          {
            "short": true,
            "value": "2 - High",
            "title": "Priority"
          },
          {
            "short": true,
            "value": "01-Mar-2019 00:17:29",
            "title": "First notified"
          },
          {
            "short": true,
            "value": "Active",
            "title": "External status"
          },
          {
            "short": false,
            "value": "yyy",
            "title": "Last update"
          }
        ],
        "pretext": "Something",
        "fallback": "xxx",
        "author_icon": "https://zzz"
      }
    ]
}

@blag
Copy link
Contributor

blag commented Apr 16, 2019

Sorry for the confusion regarding Mattermost vs. Slack!

Unfortunately

I think you have found a bug in our documentation. This statement from our documentation:

Everything that’s specified in extra.slack will be passed as-is to the Slack Attachment API.

is not 100% true. The dictionary that is passed to the Slack Attachment API is NOT passed as-is, it is completely mangled by the hubot-stackstorm Slack adapter.

But there is still hope!

There are two paths in the Slack adapter for handling extra.slack, and they handle attachments slightly differently.

Originally, sending custom attachments was not supported in the adapter (see here). I added the ability to specify your own attachments in 14275e9626021d28a1657acf823de5deec3e6742.

However, I needed to preserve backwards compatibility, so there are now two code paths for sending Slack messages with attachments:

  1. if (data.extra && data.extra.slack && data.extra.slack.attachments)
  2. if (data.extra && data.extra.slack)

But the second (original) code path unconditionally mangled the pretext:

      content.pretext = i === 0 ? pretext + split_message.pretext : null;
      content.text = chunks[i];
      content.fallback = chunks[i];
      robot.adapter.client.send(envelope, {'attachments': [content]});

Really, a better description of what it does is it completely ignores any pretext that is set by you, the user.

Furthermore, if you look through the first code path, you'll notice that it is comparatively incredibly simple:

    var messages_to_send = messages.buildMessages(data.extra.slack);

    var sendMessage = function (i) {
      robot.adapter.client.send(envelope, messages_to_send[i]);

      if (messages_to_send.length > ++i) {
        setTimeout(function(){sendMessage(i);}, 300);
      }
    };

    sendMessage(0);

The first code path does not mangle the pretext at all, so it is a "safer" code path for you, since you wish to specify your own pretext.

What this means for you

Please fully specify the attachments you wish to send, so the code takes the first (safer) code path:

---
    name: "notify_slack"
    pack: "xx_st2"
    description: "Send to Slack."
    enabled: true

    trigger:
      type: "xx_st2.evt_trigger"
      parameters: {}
    action:
      ref: "chatops.post_message"
      parameters:
        message: "{{trigger.message}}"
        channel: "testing"
        extra:
          slack:
            attachments: "{{ trigger.slack_extra_attachments }}"  # Should be an array, see the Slack Attachments API

The value of the extra.slack.attachments key is sent directly to Slack unchanged (except for some intelligent chunking if the message would not fit Slack's message size limit).

I cannot really properly "fix" this bug, because doing so would create issues for other users who do expect pretext to be set/mangled by the Slack adapter, but the above is a way for you to workaround the issue.

Other considerations

Additional pretext mangling elsewhere in the code

There is some additional pretext mangling earlier in the adapter, before either code path:

  if (data.whisper && data.user) {
    recipient = data.user;
    envelope = {
      "user": data.user
    };
  } else {
    recipient = data.channel;
    pretext = (data.user && !data.whisper) ? util.format('@%s: ', data.user) : "";
    envelope = {
      "room": data.channel,
      "id": data.channel,
      "user": data.user,
    };
  }

but that pretext value is not used by the first code path, so the workaround I specified will still workaround this.

Slack message size limits

Ensure that your total message size does not exceed Slack's message size limit (which, IIRC, is 4000 characters in the JSON). If the Slack adapter thinks your message is too large, it will apply some (hopefully) intelligent chunking to break up your message while preserving the intended formatting as much as possible.

Documentation fix

Tomorrow I will put together a pull request updating the documentation so it doesn't mislead users such as yourself. Thank you for pointing out that inaccuracy. :)

@lingfish
Copy link
Author

Love it, thanks @blag! That was the most epic and awesome dev issue comment I've read :)

Pity about the constraints though. I'll try your suggestion soon.

Meanwhile, Slack are at it again, improving etc. Now they have a thing called blocks. Perhaps if/when this project moves to it, that non-breaking change can be implemented.

@lingfish
Copy link
Author

lingfish commented May 2, 2019

Hi again @blag ... So, I've tried it as suggested and hit a bit of a wall. It seems that no jinja parsing is done, if done as above?

  "extra": {
    "slack": {
      "attachments": "[{u'title': u'something', u'color': u'#a0a0a0', u'text': u'something', ... ]"
    }
  }

It seems to have just pprint'd the Python representation of the list of dicts?

I've tried variants using to_json_string and yaml, no difference.

Thoughts?

@lingfish
Copy link
Author

lingfish commented May 6, 2019

Pinging @blag 😉

@blag
Copy link
Contributor

blag commented May 7, 2019

@lingfish Can you post your entire alias (feel free to redact/change any sensitive information)? It's difficult to troubleshoot when I don't know what you've written.

@lingfish
Copy link
Author

lingfish commented May 8, 2019

Do you mean my rule? There is no alias. If rule, here it is:

---
    name: "notify_slack"
    pack: "xxx_st2"
    description: "Send to Slack."
    enabled: true

    trigger:
      type: "xxx_st2.evt_trigger"
      parameters: {}

    action:
      ref: "chatops.post_message"
      parameters:
        message: "{{trigger.message}}"
        channel: "testing"
        extra:
          slack:
            attachments: "{{ trigger.slack_extra }}"

@blag
Copy link
Contributor

blag commented May 8, 2019

I'm having a hard time reconstructing what you are doing. Can you post a snapshot of everything you have right now?

Attempting to reconstruct what's going on...

"extra": {
  "slack": {
    "attachments": "[{u'title': u'something', u'color': u'#a0a0a0', u'text': u'something', ... ]"
  }
}

^ This appears to be the data that is being passed into chatops.post_message. Is that correct?

So if this is your rule YAML (copied/pasted from above):

---
    name: "notify_slack"
    pack: "xxx_st2"
    description: "Send to Slack."
    enabled: true

    trigger:
      type: "xxx_st2.evt_trigger"
      parameters: {}

    action:
      ref: "chatops.post_message"
      parameters:
        message: "{{trigger.message}}"
        channel: "testing"
        extra:
          slack:
            attachments: "{{ trigger.slack_extra }}"

It would stand to reason that your trigger data is:

"slack_extra": [
  {
    "title": "something",
    "color": "#a0a0a0",
    "text": "something",
    ...
  }
]

Which doesn't look right.

I would double check what your trigger instance data is. Make sure it is an object/dictionary and not a string. Once you've verified it, please post it here verbatim so I can follow along.

Once you have verified the trigger instance data is what you expect it to be, you can also try to use the from_json_string Jinja filter in your rule:

---
name: "notify_slack"
pack: "xxx_st2"
description: "Send to Slack."
enabled: true

trigger:
  type: "xxx_st2.evt_trigger"
  parameters: {}

action:
  ref: "chatops.post_message"
  parameters:
    message: "{{trigger.message}}"
    channel: "testing"
    extra:
      slack:
        attachments: "{{ trigger.slack_extra | from_json_string }}"

@lingfish
Copy link
Author

lingfish commented May 9, 2019

Sorry, you're right, more data needed from me.

Ok, so... the rule above is right.

Heavily redacted below, but done carefully to not lose quotes etc.

Yes, that's the data that is being passed into chatops.post_message. Here is the action output:

{
  "extra": {
    "slack": {
      "attachments": "[{u'title': u'zzz', u'color': u'danger', u'text': u'NOTIFICATION', u'author_name': u'zzz', u'title_link': u'zzz', u'mrkdwn_in': [u'fields'], u'fields': [{u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'Active', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'1', u'title': u'zzz'}, {u'short': False, u'value': u'zzz', u'title': u'zzz'}], u'pretext': u'', u'fallback': u'zzz', u'author_icon': u'zzz'}]"
    }
  },
  "whisper": false,
  "user": null,
  "output": {
    "whisper": false,
    "message": "NOTIFICATION",
    "user": null,
    "channel": "testing",
    "extra": {
      "slack": {
        "attachments": "[{u'title': u'zzz', u'color': u'danger', u'text': u'NOTIFICATION', u'author_name': u'zzz', u'title_link': u'zzz', u'mrkdwn_in': [u'fields'], u'fields': [{u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'zzz', u'title': u'zzz'}, {u'short': True, u'value': u'1', u'title': u'zzz'}, {u'short': False, u'value': u'zzz', u'title': u'zzz'}], u'pretext': u'', u'fallback': u'zzz', u'author_icon': u'zzz'}]"
      }
    }
  },
  "message": "NOTIFICATION",
  "channel": "testing"
}

Here is the trigger instance:

{
  "status": "processed",
  "occurrence_time": "2019-05-09T13:16:03.000000Z",
  "trigger": "pack.trigger",
  "id": "5cd39b73e8a12c310f1e5ddc",
  "payload": {
    "param": "zzz",
    "param": false,
    "param": 1,
    "param": "zzz",
    "param": "zzz",
    "param": "zzz",
    "param": "zzz",
    "param": "zzz",
    "slack_extra": [
      {
        "title": "zzz",
        "color": "danger",
        "text": "NOTIFICATION",
        "author_name": "zzz",
        "title_link": "zzz",
        "mrkdwn_in": [
          "fields"
        ],
        "fields": [
          {
            "short": true,
            "value": "zzz",
            "title": "zzz"
          },
          {
            "short": true,
            "value": "zzz",
            "title": "zzz"
          },
          {
            "short": true,
            "value": "zzz",
            "title": "zzz"
          },
          {
            "short": true,
            "value": "zzz",
            "title": "zzz"
          },
          {
            "short": true,
            "value": "zzz",
            "title": "zzz"
          },
          {
            "short": true,
            "value": "1",
            "title": "zzz"
          },
          {
            "short": false,
            "value": "zzz",
            "title": "zzz"
          }
        ],
        "pretext": "",
        "fallback": "zzz",
        "author_icon": "zzz"
      }
    ],
    "short_description": "zzz",
    "message": "zzz",
    "cmdb_ci": {
      "link": "zzz",
      "display_value": "zzz"
    }
  }
}

So the trigger instance data looks like an object to me... but as stated above, the attachments field just looks like the data as if Python pretty-printed.

The snippet of my Python code for the sensor:

        payload['slack_extra'] = [
            {   
                'fallback': 'zzz {}: {}'.format(record['zzz'], record['yyy']),
                'author_name': 'zzz',
                'author_icon': 'zzz',
                'title': record['zzz'],
      ...

Finally, I had already tried from_json_string, but did so again, and I get this:

{
  "traceback": "  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2reactor/rules/enforcer.py\", line 200, in _invoke_action\n    additional_contexts=additional_contexts)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2reactor/rules/enforcer.py\", line 82, in get_resolved_parameters\n    additional_contexts=additional_contexts)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2common/util/param.py\", line 297, in render_live_params\n    context = _resolve_dependencies(G)\n  File \"/opt/stackstorm/st2/local/lib/python2.7/site-packages/st2common/util/param.py\", line 215, in _resolve_dependencies\n    raise ParamException(msg)\n",
  "error": "Failed to render parameter \"extra\": Expecting property name enclosed in double quotes: line 1 column 3 (char 2)"
}

@blag
Copy link
Contributor

blag commented May 9, 2019

Can you post the YAML metadata file for your sensor?

@lingfish
Copy link
Author

lingfish commented May 9, 2019

Sure, for this though you'll need non-redacted fields, right?

If so, can I mail it or similar, commercially-sensitive stuff in there.

@blag
Copy link
Contributor

blag commented May 9, 2019

You are welcome to either redact field values (use your best judgment on that) and post it here or you can DM me via our Slack community (I'm also blag on that).

@lingfish
Copy link
Author

lingfish commented May 9, 2019

Alright, so upon trying to modify the sensor schema, I get an error for object, and noticing it's technically an array, tried that too.

Failed to validate payload [...] is not of type u'object', 'null'

When specifying array, it just goes through again as a string :(

@lingfish
Copy link
Author

Also just for fun, from the sensor I've send an object {} instead of an array of objects, changed the schema accordingly, and it passes, but still just a flat string.

@blag
Copy link
Contributor

blag commented May 15, 2019

Okay, so since the data being passed to chatops.post_message is a string, it appears that that is not the cause of the issue.

The issue is farther back in the trigger chain - in the rule itself. Because of that, this bug is in the st2 project itself and the original issue StackStorm/st2#4561 is the proper place to handle this.

@lingfish Is that acceptable to you? Sorry to push you back over to that issue, but it looks like there's nothing to do/fix in this project.

I'll keep this open for a week or until you respond, whichever comes first. Then I'll close it and post the information we've figured out here back into the original st2 issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants