fix: Zenoss config handling (#2549)

* fix: respect configuration values

* fix: collector property is configuration item

* fix: remove URL and auth info from handler config

* style: replace ServiceNow with Zenoss in code comments

* refactor: remove redundant URL parsing

* fix: more strict URL validation

* fix: default value of collector property

* test: fix Zenoss tests

* fix: unified json props names

* test: fix zenoss json props names

* style: go fmt

alert.go

@@ -581,7 +581,6 @@ func newAlertNode(et *ExecutingTask, n *pipeline.AlertNode, d NodeDiagnostic) (a
 
 	for _, s := range n.ZenossHandlers {
 		c := zenoss.HandlerConfig{
-			URL:           s.URL,
 			Action:        s.Action,
 			Method:        s.Method,
 			Type:          s.Type,

etc/kapacitor/kapacitor.conf

@@ -370,6 +370,8 @@ default-retention-policy = ""
   type = "rpc"
   # Temporary request transaction ID.
   tid = 1
+  # Collector name.
+  collector = "Kapacitor"
   # Alert level to event severity mapping.
   severity-map = { OK = "Clear", Info = "Info", Warning = "Warning", Critical = "Critical" }
 

integrations/streamer_test.go

@@ -11239,6 +11239,7 @@ stream
 		c := zenoss.NewConfig()
 		c.Enabled = true
 		c.URL = ts.URL + "/zport/dmd/evconsole_router"
+		c.Collector = ""
 		svc := zenoss.NewService(c, diagService.NewZenossHandler())
 		tm.ZenossService = svc
 	}

pipeline/alert.go

@@ -2423,6 +2423,10 @@ type ZenossHandler struct {
 	// If empty uses value from the configuration.
 	TID int64 `json:"tid"`
 
+	// Collector name.
+	// If empty uses value from the configuration.
+	Collector string `json:"collector"`
+
 	// Summary of the event.
 	Summary string `json:"summary"`
 
@@ -2441,9 +2445,6 @@ type ZenossHandler struct {
 	// Message related to the event.
 	Message string `json:"message"`
 
-	// Collector.
-	Collector string `json:"collector"`
-
 	// Custom fields.
 	// tick:ignore
 	CustomFieldsMap map[string]interface{} `tick:"CustomField" json:"customField"`

pipeline/tick/alert.go

@@ -263,7 +263,7 @@ func (n *AlertNode) Build(a *pipeline.AlertNode) (ast.Node, error) {
 			Dot("action", h.Action).
 			Dot("method", h.Method).
 			Dot("type", h.Type).
-			Dot("TID", h.TID).
+			Dot("tid", h.TID).
 			// standard event data element fields
 			Dot("summary", h.Summary).
 			Dot("device", h.Device).

server/server_test.go

@@ -8834,6 +8834,7 @@ func TestServer_UpdateConfig(t *testing.T) {
 			section: "zenoss",
 			setDefaults: func(c *server.Config) {
 				c.Zenoss.URL = "https://tenant.zenoss.io:8080/zport/dmd/evconsole_router"
+				c.Zenoss.Collector = "Kapacitor"
 			},
 			expDefaultSection: client.ConfigSection{
 				Link: client.Link{Relation: client.Self, Href: "/kapacitor/v1/config/zenoss"},
@@ -8850,6 +8851,7 @@ func TestServer_UpdateConfig(t *testing.T) {
 						"method":             "add_event",
 						"type":               "rpc",
 						"tid":                float64(1),
+						"collector":          "Kapacitor",
 						"severity-map": map[string]interface{}{
 							"ok": "Clear", "info": "Info", "warning": "Warning", "critical": "Critical",
 						},
@@ -8872,6 +8874,7 @@ func TestServer_UpdateConfig(t *testing.T) {
 					"method":             "add_event",
 					"type":               "rpc",
 					"tid":                float64(1),
+					"collector":          "Kapacitor",
 					"severity-map": map[string]interface{}{
 						"ok": "Clear", "info": "Info", "warning": "Warning", "critical": "Critical",
 					},
@@ -8910,6 +8913,7 @@ func TestServer_UpdateConfig(t *testing.T) {
 								"method":             "kapa_handler",
 								"type":               "rpc",
 								"tid":                float64(1),
+								"collector":          "Kapacitor",
 								"severity-map": map[string]interface{}{
 									"ok": float64(0), "info": float64(2), "warning": float64(3), "critical": float64(5),
 								},
@@ -8932,6 +8936,7 @@ func TestServer_UpdateConfig(t *testing.T) {
 							"method":             "kapa_handler",
 							"type":               "rpc",
 							"tid":                float64(1),
+							"collector":          "Kapacitor",
 							"severity-map": map[string]interface{}{
 								"ok": float64(0), "info": float64(2), "warning": float64(3), "critical": float64(5),
 							},
@@ -9451,20 +9456,20 @@ func TestServer_ListServiceTests(t *testing.T) {
 				Link: client.Link{Relation: client.Self, Href: "/kapacitor/v1/service-tests/zenoss"},
 				Name: "zenoss",
 				Options: client.ServiceTestOptions{
-					"alert_id":        "1001",
-					"level":           "CRITICAL",
-					"message":         "test zenoss message",
-					"action":          "EventsRouter",
-					"method":          "add_event",
-					"type":            "rpc",
-					"tid":             float64(1),
-					"summary":         "",
-					"device":          "",
-					"component":       "",
-					"event_class_key": "",
-					"event_class":     "",
-					"collector":       "",
-					"custom_fields":   map[string]interface{}{},
+					"alert_id":      "1001",
+					"level":         "CRITICAL",
+					"message":       "test zenoss message",
+					"action":        "EventsRouter",
+					"method":        "add_event",
+					"type":          "rpc",
+					"tid":           float64(1),
+					"summary":       "",
+					"device":        "",
+					"component":     "",
+					"eventclasskey": "",
+					"eventclass":    "",
+					"collector":     "Kapacitor",
+					"custom_fields": map[string]interface{}{},
 				},
 			},
 		},
@@ -11174,6 +11179,7 @@ func TestServer_AlertHandlers(t *testing.T) {
 
 				c.Zenoss.Enabled = true
 				c.Zenoss.URL = ts.URL + "/zport/dmd/evconsole_router"
+				c.Zenoss.Collector = ""
 				return ctxt, nil
 			},
 			result: func(ctxt context.Context) error {

services/zenoss/config.go

@@ -59,9 +59,9 @@ type Config struct {
 	Enabled bool `toml:"enabled" override:"enabled"`
 	// Zenoss events API URL.
 	URL string `toml:"url" override:"url"`
-	// ServiceNow authentication username.
+	// Zenoss authentication username.
 	Username string `toml:"username" override:"username"`
-	// ServiceNow authentication password.
+	// Zenoss authentication password.
 	Password string `toml:"password" override:"password,redact"`
 	// Action (router name).
 	Action string `toml:"action" override:"action"`
@@ -71,9 +71,11 @@ type Config struct {
 	Type string `toml:"type" override:"type"`
 	// Event TID.
 	TID int64 `toml:"tid" override:"tid"`
+	// Collector name.
+	Collector string `toml:"collector" override:"collector"`
 	// Level to severity map.
 	SeverityMap SeverityMap `toml:"severity-map" override:"severity-map"`
-	// Whether all alerts should automatically post to ServiceNow.
+	// Whether all alerts should automatically post to Zenoss.
 	Global bool `toml:"global" override:"global"`
 	// Whether all alerts should automatically use stateChangesOnly mode.
 	// Only applies if global is also set.
@@ -87,6 +89,7 @@ func NewConfig() Config {
 		Method:      "add_event",
 		Type:        "rpc",
 		TID:         1,
+		Collector:   "Kapacitor",
 		SeverityMap: SeverityMap{OK: "Clear", Info: "Info", Warning: "Warning", Critical: "Critical"},
 	}
 }
@@ -95,7 +98,7 @@ func (c Config) Validate() error {
 	if c.Enabled && c.URL == "" {
 		return errors.New("must specify events URL")
 	}
-	if _, err := url.Parse(c.URL); err != nil {
+	if _, err := url.ParseRequestURI(c.URL); err != nil {
 		return errors.Wrapf(err, "invalid url %q", c.URL)
 	}
 	if c.SeverityMap.OK == nil || c.SeverityMap.Info == nil || c.SeverityMap.Warning == nil || c.SeverityMap.Critical == nil {

services/zenoss/service.go

@@ -8,7 +8,6 @@ import (
 	"io/ioutil"
 	"math"
 	"net/http"
-	neturl "net/url"
 	"strings"
 	"sync/atomic"
 	text "text/template"
@@ -84,21 +83,23 @@ type testOptions struct {
 	Summary       string                 `json:"summary"`
 	Device        string                 `json:"device"`
 	Component     string                 `json:"component"`
-	EventClassKey string                 `json:"event_class_key"`
-	EventClass    string                 `json:"event_class"`
+	EventClassKey string                 `json:"eventclasskey"`
+	EventClass    string                 `json:"eventclass"`
 	Collector     string                 `json:"collector"`
 	CustomFields  map[string]interface{} `json:"custom_fields"`
 }
 
 func (s *Service) TestOptions() interface{} {
+	c := s.config()
 	return &testOptions{
 		AlertID:      "1001",
 		Level:        alert.Critical,
 		Message:      "test zenoss message",
-		Action:       "EventsRouter",
-		Method:       "add_event",
-		Type:         "rpc",
-		TID:          1,
+		Action:       c.Action,
+		Method:       c.Method,
+		Type:         c.Type,
+		TID:          c.TID,
+		Collector:    c.Collector,
 		CustomFields: map[string]interface{}{},
 	}
 }
@@ -108,7 +109,6 @@ func (s *Service) Test(options interface{}) error {
 	if !ok {
 		return fmt.Errorf("unexpected options type %T", options)
 	}
-	c := s.config()
 	hc := &HandlerConfig{
 		Action:        o.Action,
 		Method:        o.Method,
@@ -132,11 +132,11 @@ func (s *Service) Test(options interface{}) error {
 		Message: o.Message,
 	}
 
-	return s.Alert(c.URL, state, data, hc)
+	return s.Alert(state, data, hc)
 }
 
-func (s *Service) Alert(url string, state *alert.EventState, data *alert.EventData, hc *HandlerConfig) error {
-	postUrl, postBody, err := s.preparePost(url, state, data, hc)
+func (s *Service) Alert(state *alert.EventState, data *alert.EventData, hc *HandlerConfig) error {
+	postUrl, postBody, err := s.preparePost(state, data, hc)
 	if err != nil {
 		return err
 	}
@@ -199,20 +199,12 @@ type Event struct {
 	TID    int64                    `json:"tid"`
 }
 
-func (s *Service) preparePost(url string, state *alert.EventState, data *alert.EventData, hc *HandlerConfig) (string, io.Reader, error) {
+func (s *Service) preparePost(state *alert.EventState, data *alert.EventData, hc *HandlerConfig) (string, io.Reader, error) {
 	c := s.config()
 	if !c.Enabled {
 		return "", nil, errors.New("service is not enabled")
 	}
 
-	if url == "" {
-		url = c.URL
-	}
-	u, err := neturl.Parse(url)
-	if err != nil {
-		return "", nil, err
-	}
-
 	// fallback to config values for handler options not set
 	action := hc.Action
 	if action == "" {
@@ -308,6 +300,9 @@ func (s *Service) preparePost(url string, state *alert.EventState, data *alert.E
 	if summary == "" { // fallback to default (ie. alert message)
 		summary = state.Message
 	}
+	if collector == "" {
+		collector = c.Collector
+	}
 
 	eventData := &EventData{
 		Summary:    cutoff(summary, 256),
@@ -357,7 +352,7 @@ func (s *Service) preparePost(url string, state *alert.EventState, data *alert.E
 		return "", nil, errors.Wrap(err, "error marshaling event struct")
 	}
 
-	return u.String(), bytes.NewBuffer(postBytes), nil
+	return c.URL, bytes.NewBuffer(postBytes), nil
 }
 
 func (s *Service) addCustomData(basicData, customData map[string]interface{}) {
@@ -396,18 +391,6 @@ func (s *Service) toMap(data *EventData) (map[string]interface{}, error) {
 }
 
 type HandlerConfig struct {
-	// web service URL used to post messages.
-	// If empty uses the service URL from the configuration.
-	URL string `mapstructure:"url"`
-
-	// Username for BASIC authentication.
-	// If empty uses username from the configuration.
-	Username string `mapstructure:"username"`
-
-	// Password for BASIC authentication.
-	// If empty uses password from the configuration.
-	Password string `mapstructure:"password"`
-
 	// Action (router name).
 	// If empty uses action from the configuration.
 	Action string `mapstructure:"action"`
@@ -465,7 +448,6 @@ func (s *Service) Handler(c HandlerConfig, ctx ...keyvalue.T) alert.Handler {
 
 func (h *handler) Handle(event alert.Event) {
 	if err := h.s.Alert(
-		h.c.URL,
 		&event.State,
 		&event.Data,
 		&h.c,