sensu-plugins / sensu-plugins-mailer

This plugin is an email handler for Sensu.
http://sensu-plugins.io
MIT License
17 stars 37 forks source link

Email going to wrong addresses #32

Closed oba11 closed 8 years ago

oba11 commented 8 years ago

Noticed that email started going to wrong subscriber which was a result of the addition here https://github.com/sensu-plugins/sensu-plugins-mailer/blob/master/bin/handler-mailer.rb#L121-L128.

Now email goes to event and client mail_to which is not an expected behaviour e.g

I have client with

{
  "client": {
    "name": "mon01",
    "address": "1.2.3.4",
    "subscriptions": [
      "graphite","all"
    ]
}

check json

{
  "mailer": {
    "mail_to": "email1@example.com",
    "smtp_address": "127.0.0.1",
    "smtp_port": "25",
    "subscriptions": {
      "graphite": {
        "mail_to": "email2@example.com"
      }
    }
  }
}

with a check of

"memory": {
      "command": "check-ram.rb",
      "subscribers": [ "all" ],
      "interval": 120,
      "handlers": [ "default", "mailer" ]
    },

Email now goes to both email1@example.com and email2@example.com which is wrong because email was supposed to go to only email1 and not both of them

Evesy commented 8 years ago

+1 on this

This change now means that users will receive alerts for checks they aren't interested in. Example scenario below:

Client

{
  "client": {
    "name": "host1",
    "address": "1.2.3.4",
    "subscriptions": [
      "system",
      "rabbitmq"
    ]
  }
}

Check

{
  "checks": {
    "rabbitmq_cluster_health": {
      "command": "/opt/sensu/embedded/bin/check-rabbitmq-cluster-health.rb -w localhost -P 15672",
      "handlers": [
        "default",
        "mail-default"
      ],
      "subscribers": [
        "rabbitmq"
      ],
      "interval": 30
    }
  }
}

Mail Handler Config

{
  "mail-default": {
    "admin_gui": "http://sensu.com:3000",
    "mail_from": "sensu@sensu.com",
    "mail_to": "email1@example.com",
    "smtp_address": "localhost",
    "smtp_port": "25",
    "smtp_domain": "localhost",
    "subscriptions": {
      "rabbitmq": {
        "mail_to": "email2@example.com"
      },
      "system": {
        "mail_to": "alerts@example.com"
      }
    }
  }
}

With the above configs if the rabbitmq_cluster_health check fails the mail handler is going to do both of the below:

Look at the checks subscribers (rabbitmq) and cross reference that with the mail handler json subscriptions, in which case it will add email2@example.com as a recipient. It will then look at the clients subscriptions and cross reference again with the mail handler json, in which case it's going to see both rabbitmq & system, adding both email2@example.com & alerts@example.com to the mail_to list

Now alerts@example.com, who is only interested in 'system' alerts is getting the emails when rabbit fails, which isn't right.

The PR this was implemented in is here: https://github.com/sensu-plugins/sensu-plugins-mailer/pull/18. This was added as a way for that user to alert based on their server environment (prod/non-prod) however I don't think that's the right way to go about it. They should be using filters to get that working (or even have a separate sensu instance for non-prod). I don't think this change was for the better as it has somewhat broken the alerting logic that was already in place.

oba11 commented 8 years ago

Will close this since the PR https://github.com/sensu-plugins/sensu-plugins-mailer/pull/40 is merged