Sensu-go: [Feature Request] token substitution for handlers

Created on 12 Dec 2018  Â·  12Comments  Â·  Source: sensu/sensu-go

being able to pass attributes from events to handlers would make handlers more dynamic and easier to maintain. i am by no means fluent in how events and handlers communicate, so the following is based on my interpretation of how handlers work. feel free to point out something i'm missing!

Current Behavior

from my understanding, token substitution only applies to checks when they are scheduled to run on the agent, pulling attributes from the entity.

using slack and pagerduty handlers as an example, they require tokens to send events to the proper channel or team. right now as i understand it, a handler must be configured for each individual token being used for their respective handler command. so, if there were 4 different slack channels to alert to and 4 different pagerduty teams, that would require 8 different handlers.

to further complicate the matter, filters are also applied at the hander. so if i had specific filtering needs depending for different checks then i need to make a handler with filters for each of the 8 different slack/pd commands i'm running. so let's say i have 4 different filtering scenario needs that is 32 different handlers that need to be created and updated as things change.

Possible Solution

allowing attributes from events to be applied to handlers with token substitution in order to make the application of handler command flags and filters more dynamic and easier to manage.

example event data pulled from documentation with some added labels to event metadata and check metadata:

{
  "type": "Event",
  "api_version": "core/v2",
  "metadata": {
    "name": "webserver01",
    "namespace": "default",
    "labels": {
      "pd_team": "ops",
      "slack_key": "webhook-key",
      "slack_channel": "#alerts"
    },
    "annotations": null
  },
  "spec": {
    "timestamp": 1542667666,
    "entity": {
      "entity_class": "agent",
      "system": {
        "hostname": "webserver01",
        "os": "linux",
        "platform": "centos",
        "platform_family": "rhel",
        "platform_version": "7.4.1708",
        "network": {
          "interfaces": [
            {
              "name": "lo",
              "addresses": [
                "127.0.0.1/8",
                "::1/128"
              ]
            },
            {
              "name": "enp0s3",
              "mac": "08:00:27:11:ad:d2",
              "addresses": [
                "10.0.2.15/24",
                "fe80::26a5:54ec:cf0d:9704/64"
              ]
            },
            {
              "name": "enp0s8",
              "mac": "08:00:27:bc:be:60",
              "addresses": [
                "172.28.128.3/24",
                "fe80::a00:27ff:febc:be60/64"
              ]
            }
          ]
        },
        "arch": "amd64"
      },
      "subscriptions": [
        "testing",
        "entity:webserver01"
      ],
      "last_seen": 1542667635,
      "deregister": false,
      "deregistration": {},
      "user": "agent",
      "redact": [
        "password",
        "passwd",
        "pass",
        "api_key",
        "api_token",
        "access_key",
        "secret_key",
        "private_key",
        "secret"
      ]
    },
    "check": {
      "check_hooks": null,
      "duration": 2.033888684,
      "executed": 1522170513,
      "command": "http_check.sh http://localhost:80",
      "handlers": [
        "slack"
      ],
      "high_flap_threshold": 0,
      "interval": 20,
      "low_flap_threshold": 0,
      "publish": true,
      "runtime_assets": [],
      "subscriptions": [
        "testing"
      ],
      "proxy_entity_name": "",
      "check_hooks": null,
      "stdin": false,
      "ttl": 0,
      "timeout": 0,
      "duration": 0.010849143,
      "executed": 1542667666,
      "history": [
        {
          "status": 1,
          "executed": 1542667666
        }
      ],
      "issued": 1542667666,
      "output": "",
      "state": "failing",
      "status": 1,
      "total_state_change": 0,
      "last_ok": 0,
      "occurrences": 1,
      "occurrences_watermark": 1,
      "output_metric_format": "",
      "output_metric_handlers": [],
      "env_vars": null,
      "metadata": {
        "name": "check-nginx",
        "namespace": "default",
        "labels": {
             "pd_team": "ops",
              "filters": [ "is_incident", "occurrences-3", "hourly_interval" ]
         },
        "annotations": null
      }
    }
  }
}

the first example is the slack handler pulling labels from event metadata:

{
  "api_version": "core/v2",
  "type": "Handler",
  "metadata": {
      "namespace": "default",
      "name": "slack"
  },
  "spec": {
      "type": "pipe",
      "command": "slack-handler --channel '{{ .metadata.labels.slack_channel' --timeout 20 --webhook-url 'https://hooks.slack.com/services/{{ .metadata.label.slack_key }}'",
      "timeout": 30,
      "filters": [
          "is_incident",
          "occurrences-3"
      ]
  }
}

the second example is the pagerduty handler pulling labels from event check metadata which includes filters:

{
  "api_version": "core/v2",
  "type": "Handler",
  "metadata": {
      "namespace": "default",
      "name": "pagerduty"
  },
  "spec": {
      "type": "pipe",
      "command": "pagerduty-handler --token {{ .check.metadata.labels.pd_team }}",
      "timeout": 30,
      "filters": [
          {{ .check.metadata.labels.filters }}
      ]
  }
}

the filters seems like it could be the more challenging task of the two as you would need a good way to pull all filters from the event and insert them into the handler dynamically.

that pretty much sums up what i was thinking though...

backend feature needs-discussion wontfix

Most helpful comment

This feature would be awesome!

All 12 comments

One problem with this is that (currently) your proposed changes to the check labels will not unmarshal, so the check is invalid:

        "labels": {
             "pd_team": "ops",
              "filters": [ "is_incident", "occurrences-3", "hourly_interval" ]
         },

ObjectMeta.Labels is a map[string]string: https://github.com/sensu/sensu-go/blob/master/api/core/v2/meta.pb.go#L39
afaict there is no clean way to add structured JSON data.

I think this limitation would need to be resolved first. I commented about it previously here https://github.com/sensu/sensu-go/issues/2674#issuecomment-462795078 but I think it warrants its own issue for discussion so I just raised one: #2749

This feature would be awesome!

I agree this feature would be really useful, and consistent with the idea of token substitution in general.

@jspaleta can you or someone add a 1.x-parity label this was possible in previous implementations.

Having reviewed documentation and run some manual tests, I don't think the 1.x-parity label is warranted for this feature request. Token substitution in 1.x is limited to check definitions and is not supported in handler definitions.

Having reviewed documentation and run some manual tests, I don't think the 1.x-parity label is warranted for this feature request. Token substitution in 1.x is limited to check definitions and is not supported in handler definitions.

hmm I thought you could use it in a handler though I don't have an example at this org. I recall doing this to map a pagerduty service to a team? I could be remembering wrong.

I think I might have been thinking it was as we could simply "abuse" the fact that we had json files on disk and could accomplish the same without the need of it being a first class citizen: https://github.com/sensu-plugins/sensu-plugins-pagerduty/blob/5.0.0/bin/handler-pagerduty.rb#L27-L32 and https://github.com/sensu-plugins/sensu-plugins-pagerduty/blob/5.0.0/bin/handler-pagerduty.rb#L86-L88 then in your handler definition you pass in -j MY_TEAM which then would return the api key for you. IMO its still a feature parity since the traditional interface through files was removed for defining handler config but I don't want to argue about. This "style" of setup I have used in multiple orgs and was 100% supported.

Custom attributes can still be defined in Handler definitions, they just live in metadata.labels and metadata.annotations now (and all resources support this, so custom labels & annotations set on the entity are in the event payload and accessible to the Handler plugin - just not as tokens). And you can also pass custom config via env_vars now as well.

type: Handler 
api_version: core/v2 
metadata: 
  name: example 
  labels: 
    foo: bar 
  annotations:
    foo: bar
spec:
  type: pipe 
  command: echo 'Hello $EXAMPLE_FOO world'
  env_vars: 
  - "EXAMPLE_FOO=environment"

One of the bigger picture concerns here – and the reason why we never added token substitution to Handlers in Sensu 1.x – is that there are various security risks related to executing command containing data from an event payload (i.e. something something "remote code execution" something). The difference between Handler token substitution and Check token substitution is that the agent is just applying local configuration data (i.e. read from /etc/sensu/agent.yml) and performing the substitution, which is quite a bit different than arbitrary event data transmitted via an event (which could come from a Sensu Agent, or from some external system via the Sensu APIs).

We haven't closed this issue because we do want to do _something_ to satisfy the desired UX, it just might not be "Handler token substitution" per se.

FWIW, I was one of the original advocates for handler token substitution before I realized the implications. So this issue is near and dear to my heart. We just need to find a safer way to provide the desired UX.

Does that make sense?

I missed the part around allowing it from the events what filters (which can be arbitrary js). I definitely like having env vars be more native than relying on automation have to account for platform specific methods depending such as /etc/default/sensu, init scripts, etc to accomplish the same.

Hi all!

I definitely agree that handler token substitution sounds awesome, however there are some serious security implications associated to that, as stated by Caleb, which make it a nonstarter in my mind.

That's why I believe it's important to keep an eye on the problem we are trying to solve here, because token substitution is a means to an end, not an end in itself. I think Craig summed it up well in its first sentence:

being able to pass attributes from events to handlers would make handlers more dynamic and easier to maintain

Turns out a solution already exists within the sensu-plugin-sdk (previously sensu-plugins-go-library), which consists in configuration options override via the events annotations.

We just released the version 1.2.0 of the sensu-slack-handler which implements that pattern. To get started, make sure to upgrade the slack asset and then, simply update your checks and/or entities annotations to specify your custom Slack configuration, e.g.:

# /etc/sensu/agent.yml example
annotations:
  sensu.io/plugins/slack/config/channel: '#monitoring'
  sensu.io/plugins/slack/config/webhook-url: https://hooks.slack.com/services/XXXXX/YYYYY/ZZZZ

Your Slack handler configuration would remain the same, which simply defines default values in case entities and/or checks do not override them:

api_version: core/v2
type: Handler
metadata:
  namespace: default
  name: slack
spec:
  type: pipe
  command: sensu-slack-handler --webhook-url https://hooks.slack.com/services/AAAAA/BBBBB/CCCCC --channel '#general'
  runtime_assets:
    - sensu/sensu-slack-handler
  timeout: 10

The obvious downside of this approach is that each handlers need to provide support for that, hopefully using the sensu-slack-handler but I do believe this is something achievable relatively easily.

I'm more than interested in hearing some feedback about this approach, especially from users that tried it in their environment!

This issue has been mentioned on Sensu Community. There might be relevant details there:

https://discourse.sensu.io/t/how-can-i-read-sensu-event-data-in-a-handler-sensugo/1772/1

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

palourde picture palourde  Â·  3Comments

palourde picture palourde  Â·  4Comments

treydock picture treydock  Â·  4Comments

palourde picture palourde  Â·  3Comments

hezor picture hezor  Â·  4Comments