fix(alerts): custom unmarshal of channel configuration Headers and Payload fields

pkg/alerts/alerts_types.go

@@ -1,6 +1,10 @@
 package alerts
 
-import "github.com/newrelic/newrelic-client-go/internal/serialization"
+import (
+	"encoding/json"
+
+	"github.com/newrelic/newrelic-client-go/internal/serialization"
+)
 
 // Channel represents a New Relic alert notification channel
 type Channel struct {
@@ -18,25 +22,29 @@ type ChannelLinks struct {
 
 // ChannelConfiguration represents a Configuration type within Channels
 type ChannelConfiguration struct {
-	Recipients            string            `json:"recipients,omitempty"`
-	IncludeJSONAttachment string            `json:"include_json_attachment,omitempty"`
-	AuthToken             string            `json:"auth_token,omitempty"`
-	APIKey                string            `json:"api_key,omitempty"`
-	Teams                 string            `json:"teams,omitempty"`
-	Tags                  string            `json:"tags,omitempty"`
-	URL                   string            `json:"url,omitempty"`
-	Channel               string            `json:"channel,omitempty"`
-	Key                   string            `json:"key,omitempty"`
-	RouteKey              string            `json:"route_key,omitempty"`
-	ServiceKey            string            `json:"service_key,omitempty"`
-	BaseURL               string            `json:"base_url,omitempty"`
-	AuthUsername          string            `json:"auth_username,omitempty"`
-	AuthPassword          string            `json:"auth_password,omitempty"`
-	PayloadType           string            `json:"payload_type,omitempty"`
-	Region                string            `json:"region,omitempty"`
-	UserID                string            `json:"user_id,omitempty"`
-	Payload               map[string]string `json:"payload,omitempty"`
-	Headers               map[string]string `json:"headers,omitempty"`
+	Recipients            string `json:"recipients,omitempty"`
+	IncludeJSONAttachment string `json:"include_json_attachment,omitempty"`
+	AuthToken             string `json:"auth_token,omitempty"`
+	APIKey                string `json:"api_key,omitempty"`
+	Teams                 string `json:"teams,omitempty"`
+	Tags                  string `json:"tags,omitempty"`
+	URL                   string `json:"url,omitempty"`
+	Channel               string `json:"channel,omitempty"`
+	Key                   string `json:"key,omitempty"`
+	RouteKey              string `json:"route_key,omitempty"`
+	ServiceKey            string `json:"service_key,omitempty"`
+	BaseURL               string `json:"base_url,omitempty"`
+	AuthUsername          string `json:"auth_username,omitempty"`
+	AuthPassword          string `json:"auth_password,omitempty"`
+	PayloadType           string `json:"payload_type,omitempty"`
+	Region                string `json:"region,omitempty"`
+	UserID                string `json:"user_id,omitempty"`
+
+	// Payload is unmarshaled to type map[string]string
+	Payload MapStringString `json:"payload,omitempty"`
+
+	// Headers is unmarshaled to type map[string]string
+	Headers MapStringString `json:"headers,omitempty"`
 }
 
 // Condition represents a New Relic alert condition.
@@ -181,3 +189,29 @@ type SyntheticsCondition struct {
 	RunbookURL string `json:"runbook_url,omitempty"`
 	MonitorID  string `json:"monitor_id,omitempty"`
 }
+
+// MapStringString 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
+
+// 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
+
+	// Check for empty JSON string
+	if string(data) == `""` {
+		return nil
+	}
+
+	err := json.Unmarshal(data, &mapStrStr)
+	if err != nil {
+		return err
+	}
+
+	*c = MapStringString(mapStrStr)
+
+	return nil
+}

pkg/alerts/channels_integration_test.go

@@ -3,10 +3,8 @@
 package alerts
 
 import (
-	"os"
 	"testing"
 
-	"github.com/newrelic/newrelic-client-go/pkg/config"
 	"github.com/stretchr/testify/require"
 )
 
@@ -68,33 +66,43 @@ func TestIntegrationChannel(t *testing.T) {
 			Name: "integration-test-webhook",
 			Type: "webhook",
 			Configuration: ChannelConfiguration{
-				BaseURL:      "https://test.com",
-				AuthUsername: "devtoolkit",
-				AuthPassword: "123abc",
-				PayloadType:  "application/json",
-				Payload: map[string]string{
-					"account_id":               "$ACCOUNT_ID",
-					"account_name":             "$ACCOUNT_NAME",
-					"condition_id":             "$CONDITION_ID",
-					"condition_name":           "$CONDITION_NAME",
-					"current_state":            "$EVENT_STATE",
-					"details":                  "$EVENT_DETAILS",
-					"event_type":               "$EVENT_TYPE",
-					"incident_acknowledge_url": "$INCIDENT_ACKNOWLEDGE_URL",
-					"incident_id":              "$INCIDENT_ID",
-					"incident_url":             "$INCIDENT_URL",
-					"owner":                    "$EVENT_OWNER",
-					"policy_name":              "$POLICY_NAME",
-					"policy_url":               "$POLICY_URL",
-					"runbook_url":              "$RUNBOOK_URL",
-					"severity":                 "$SEVERITY",
-					"targets":                  "$TARGETS",
-					"timestamp":                "$TIMESTAMP",
-					"violation_chart_url":      "$VIOLATION_CHART_URL",
-				},
-				Headers: map[string]string{
+				BaseURL:     "https://test.com",
+				PayloadType: "application/json",
+				Headers: MapStringString{
 					"x-test-header": "test-header",
 				},
+				Payload: MapStringString{
+					"account_id": "123",
+				},
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		}
+
+		testChannelWebhookEmptyHeadersAndPayload = Channel{
+			Name: "integration-test-webhook-empty-headers-and-payload",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL: "https://test.com",
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		}
+
+		testChannelWebhookWeirdHeadersAndPayload = Channel{
+			Name: "integration-test-webhook-weird-headers-and-payload",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL: "https://test.com",
+				Headers: MapStringString{
+					"": "",
+				},
+				Payload: MapStringString{
+					"": "",
+				},
+				PayloadType: "application/json",
 			},
 			Links: ChannelLinks{
 				PolicyIDs: []int{},
@@ -107,54 +115,30 @@ func TestIntegrationChannel(t *testing.T) {
 			testChannelSlack,
 			testChannelVictorops,
 			testChannelWebhook,
+			testChannelWebhookEmptyHeadersAndPayload,
+			testChannelWebhookWeirdHeadersAndPayload,
 		}
 	)
 
-	client := newChannelsTestClient(t)
+	client := newIntegrationTestClient(t)
 
 	for _, channel := range channels {
 		// Test: Create
-		createResult := testCreateChannel(t, client, channel)
-
-		// Test: Read
-		readResult := testReadChannel(t, client, createResult)
-
-		// Test: Delete
-		testDeleteChannel(t, client, readResult)
-	}
-}
-
-func testCreateChannel(t *testing.T, client Alerts, channel Channel) *Channel {
-	result, err := client.CreateChannel(channel)
-
-	require.NoError(t, err)
+		created, err := client.CreateChannel(channel)
 
-	return result
-}
-
-func testReadChannel(t *testing.T, client Alerts, channel *Channel) *Channel {
-	result, err := client.GetChannel(channel.ID)
-
-	require.NoError(t, err)
-
-	return result
-}
+		require.NoError(t, err)
+		require.NotNil(t, created)
 
-func testDeleteChannel(t *testing.T, client Alerts, channel *Channel) {
-	p := *channel
-	_, err := client.DeleteChannel(p.ID)
+		// Test: Read
+		read, err := client.GetChannel(created.ID)
 
-	require.NoError(t, err)
-}
+		require.NoError(t, err)
+		require.NotNil(t, read)
 
-func newChannelsTestClient(t *testing.T) Alerts {
-	apiKey := os.Getenv("NEWRELIC_API_KEY")
+		// Test: Delete
+		deleted, err := client.DeleteChannel(read.ID)
 
-	if apiKey == "" {
-		t.Skipf("acceptance testing requires an API key")
+		require.NoError(t, err)
+		require.NotNil(t, deleted)
 	}
-
-	return New(config.Config{
-		APIKey: apiKey,
-	})
 }

pkg/alerts/channels_test.go

@@ -75,6 +75,44 @@ var (
 			"channel.policy_ids": "/v2/policies/{policy_id}"
 		}
 	}`
+
+	// Tests serialization of `headers` and `payload` fields
+	testWebhookEmptyHeadersAndPayloadResponseJSON = `{
+		"channels": [
+			{
+				"id": 1,
+				"name": "webhook-EMPTY-headers-and-payload",
+				"type": "webhook",
+				"configuration": {
+					"base_url": "http://example.com",
+					"headers": "",
+					"payload": "",
+					"payload_type": ""
+				},
+				"links": {
+					"policy_ids": []
+				}
+			},
+			{
+				"id": 2,
+				"name": "webhook-WEIRD-headers-and-payload",
+				"type": "webhook",
+				"configuration": {
+					"base_url": "http://example.com",
+					"headers": {
+						"": ""
+					},
+					"payload": {
+						"": ""
+					},
+					"payload_type": "application/json"
+				},
+				"links": {
+					"policy_ids": []
+				}
+			}
+		]
+	}`
 )
 
 func TestListChannels(t *testing.T) {
@@ -114,6 +152,50 @@ func TestListChannels(t *testing.T) {
 	assert.Equal(t, expected, actual)
 }
 
+func TestListChannelsWebhookWithEmptyHeadersAndPayload(t *testing.T) {
+	t.Parallel()
+	alerts := newMockResponse(t, testWebhookEmptyHeadersAndPayloadResponseJSON, http.StatusOK)
+
+	expected := []*Channel{
+		{
+			ID:   1,
+			Name: "webhook-EMPTY-headers-and-payload",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL:     "http://example.com",
+				PayloadType: "",
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		},
+		{
+			ID:   2,
+			Name: "webhook-WEIRD-headers-and-payload",
+			Type: "webhook",
+			Configuration: ChannelConfiguration{
+				BaseURL: "http://example.com",
+				Headers: MapStringString{
+					"": "",
+				},
+				Payload: MapStringString{
+					"": "",
+				},
+				PayloadType: "application/json",
+			},
+			Links: ChannelLinks{
+				PolicyIDs: []int{},
+			},
+		},
+	}
+
+	actual, err := alerts.ListChannels()
+
+	assert.NoError(t, err)
+	assert.NotNil(t, actual)
+	assert.Equal(t, expected, actual)
+}
+
 func TestGetChannel(t *testing.T) {
 	t.Parallel()
 	alerts := newMockResponse(t, testListChannelsResponseJSON, http.StatusOK)