fix(alerts): handle more complex JSON structures in headers and/or pa…

…yload
newrelic · Jan 24, 2020 · 6958a44 · 6958a44
1 parent 3cacabc
commit 6958a44
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 19 deletions.
diff --git a/pkg/alerts/alerts_types.go b/pkg/alerts/alerts_types.go
@@ -41,10 +41,10 @@ type ChannelConfiguration struct {
 	UserID                string `json:"user_id,omitempty"`
 
 	// Payload is unmarshaled to type map[string]string
-	Payload MapStringString `json:"payload,omitempty"`
+	Payload MapStringInterface `json:"payload,omitempty"`
 
 	// Headers is unmarshaled to type map[string]string
-	Headers MapStringString `json:"headers,omitempty"`
+	Headers MapStringInterface `json:"headers,omitempty"`
 }
 
 // Condition represents a New Relic alert condition.
@@ -190,28 +190,28 @@ type SyntheticsCondition struct {
 	MonitorID  string `json:"monitor_id,omitempty"`
 }
 
-// MapStringString is used for custom unmarshaling of
+// MapStringInterface is used for custom unmarshaling of
 // fields that have potentially dynamic types.
 // E.g. when a field can be a string or an object/map
-type MapStringString map[string]string
-type mapStringStringProxy MapStringString
+type MapStringInterface map[string]interface{}
+type mapStringInterfaceProxy MapStringInterface
 
 // UnmarshalJSON is a custom unmarshal method to guard against
 // fields that can have more than one type returned from an API.
-func (c *MapStringString) UnmarshalJSON(data []byte) error {
-	var mapStrStr mapStringStringProxy
+func (c *MapStringInterface) UnmarshalJSON(data []byte) error {
+	var mapStrInterface mapStringInterfaceProxy
 
 	// Check for empty JSON string
 	if string(data) == `""` {
 		return nil
 	}
 
-	err := json.Unmarshal(data, &mapStrStr)
+	err := json.Unmarshal(data, &mapStrInterface)
 	if err != nil {
 		return err
 	}
 
-	*c = MapStringString(mapStrStr)
+	*c = MapStringInterface(mapStrInterface)
 
 	return nil
 }
diff --git a/pkg/alerts/channels_integration_test.go b/pkg/alerts/channels_integration_test.go
@@ -68,10 +68,10 @@ func TestIntegrationChannel(t *testing.T) {
 			Configuration: ChannelConfiguration{
 				BaseURL:     "https://test.com",
 				PayloadType: "application/json",
-				Headers: MapStringString{
+				Headers: MapStringInterface{
 					"x-test-header": "test-header",
 				},
-				Payload: MapStringString{
+				Payload: MapStringInterface{
 					"account_id": "123",
 				},
 			},
@@ -96,10 +96,10 @@ func TestIntegrationChannel(t *testing.T) {
 			Type: "webhook",
 			Configuration: ChannelConfiguration{
 				BaseURL: "https://test.com",
-				Headers: MapStringString{
+				Headers: MapStringInterface{
 					"": "",
 				},
-				Payload: MapStringString{
+				Payload: MapStringInterface{
 					"": "",
 				},
 				PayloadType: "application/json",
@@ -109,6 +109,40 @@ func TestIntegrationChannel(t *testing.T) {
 			},
 		}
 
+		// Currently the v2 API has minimal validation on the data
+		// structure for Headers and Payload, so we need to test
+		// as many scenarios as possible.
+		testChannelWebhookComplexHeadersPayload = Channel{
+			Name: "integration-test-webhook",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL:     "https://test.com",
+				PayloadType: "application/json",
+				Headers: MapStringInterface{
+					"x-test-header": "test-header",
+					"object": map[string]interface{}{
+						"key": "value",
+						"nestedObject": map[string]interface{}{
+							"k": "v",
+						},
+					},
+				},
+				Payload: MapStringInterface{
+					"account_id": "123",
+					"array": []interface{}{"string", 2},
+					"object": map[string]interface{}{
+						"key": "value",
+						"nestedObject": map[string]interface{}{
+							"k": "v",
+						},
+					},
+				},
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		}
+
 		channels = []Channel{
 			testChannelEmail,
 			testChannelOpsGenie,
@@ -117,6 +151,7 @@ func TestIntegrationChannel(t *testing.T) {
 			testChannelWebhook,
 			testChannelWebhookEmptyHeadersAndPayload,
 			testChannelWebhookWeirdHeadersAndPayload,
+			testChannelWebhookComplexHeadersPayload,
 		}
 	)
 

diff --git a/pkg/alerts/channels_test.go b/pkg/alerts/channels_test.go
@@ -76,8 +76,8 @@ var (
 		}
 	}`
 
-	// Tests serialization of `headers` and `payload` fields
-	testWebhookEmptyHeadersAndPayloadResponseJSON = `{
+	// Tests serialization of complex `headers` and `payload` fields
+	testWebhookComplexHeadersAndPayloadResponseJSON = `{
 		"channels": [
 			{
 				"id": 1,
@@ -110,6 +110,30 @@ var (
 				"links": {
 					"policy_ids": []
 				}
+			},
+			{
+				"id": 3,
+				"name": "webhook-COMPLEX-payload",
+				"type": "webhook",
+				"configuration": {
+					"base_url": "http://example.com",
+					"headers": {
+						"key": "value",
+						"invalidHeader": {
+							"is": "allowed by the API"
+						}
+					},
+					"payload": {
+						"array": ["test", 1],
+						"object": {
+							"key": "value"
+						}
+					},
+					"payload_type": "application/json"
+				},
+				"links": {
+					"policy_ids": []
+				}
 			}
 		]
 	}`
@@ -152,9 +176,9 @@ func TestListChannels(t *testing.T) {
 	assert.Equal(t, expected, actual)
 }
 
-func TestListChannelsWebhookWithEmptyHeadersAndPayload(t *testing.T) {
+func TestListChannelsWebhookWithComplexHeadersAndPayload(t *testing.T) {
 	t.Parallel()
-	alerts := newMockResponse(t, testWebhookEmptyHeadersAndPayloadResponseJSON, http.StatusOK)
+	alerts := newMockResponse(t, testWebhookComplexHeadersAndPayloadResponseJSON, http.StatusOK)
 
 	expected := []*Channel{
 		{
@@ -175,10 +199,10 @@ func TestListChannelsWebhookWithEmptyHeadersAndPayload(t *testing.T) {
 			Type: "webhook",
 			Configuration: ChannelConfiguration{
 				BaseURL: "http://example.com",
-				Headers: MapStringString{
+				Headers: MapStringInterface{
 					"": "",
 				},
-				Payload: MapStringString{
+				Payload: MapStringInterface{
 					"": "",
 				},
 				PayloadType: "application/json",
@@ -187,6 +211,30 @@ func TestListChannelsWebhookWithEmptyHeadersAndPayload(t *testing.T) {
 				PolicyIDs: []int{},
 			},
 		},
+		{
+			ID:   3,
+			Name: "webhook-COMPLEX-payload",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL: "http://example.com",
+				Headers: MapStringInterface{
+					"key": "value",
+					"invalidHeader": map[string]interface{}{
+						"is": "allowed by the API",
+					},
+				},
+				Payload: MapStringInterface{
+					"array": []interface{}{"test", float64(1)},
+					"object": map[string]interface{}{
+						"key": "value",
+					},
+				},
+				PayloadType: "application/json",
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		},
 	}
 
 	actual, err := alerts.ListChannels()