Alerting: Fix contact point update and diff logic (#2305)

* Fix contact point update and diff logic

This includes two fixes:
- Fixed an issue where notifiers without required fields (WeCom, Webex, Slack) were incorrectly deleted during updates.
- Fixed a state drift issue where, if all notifiers of a given type existed in state but not in the API, the missing notifiers were not synced to state during refresh and did not show up in diff.

Author: JacobsonMT

docs/resources/contact_point.md

@@ -492,14 +492,17 @@ Read-Only:
 <a id="nestedblock--webex"></a>
 ### Nested Schema for `webex`
 
+Required:
+
+- `room_id` (String) ID of the Webex Teams room where to send the messages.
+- `token` (String, Sensitive) The bearer token used to authorize the client.
+
 Optional:
 
 - `api_url` (String) The URL to send webhook requests to.
 - `disable_resolve_message` (Boolean) Whether to disable sending resolve messages. Defaults to `false`.
 - `message` (String) The templated title of the message to send.
-- `room_id` (String) ID of the Webex Teams room where to send the messages.
 - `settings` (Map of String, Sensitive) Additional custom properties to attach to the notifier. Defaults to `map[]`.
-- `token` (String, Sensitive) The bearer token used to authorize the client.
 
 Read-Only:
 

internal/resources/grafana/resource_alerting_contact_point.go

@@ -274,11 +274,8 @@ func unpackContactPoints(data *schema.ResourceData) []statePair {
 				deleted = true
 				processedUIDs[uid.(string)] = true
 			}
-			for fieldName, fieldSchema := range n.schema().Schema {
-				if !fieldSchema.Computed && fieldSchema.Required && !reflect.ValueOf(pointMap[fieldName]).IsZero() {
-					deleted = false
-					break
-				}
+			if nonEmptyNotifier(n, pointMap) {
+				deleted = false
 			}
 
 			// Add the point/receiver to the result
@@ -306,6 +303,25 @@ func unpackContactPoints(data *schema.ResourceData) []statePair {
 	return result
 }
 
+type HasData interface {
+	HasData(data map[string]any) bool
+}
+
+func nonEmptyNotifier(n notifier, data map[string]any) bool {
+	if customEmpty, ok := n.(HasData); ok {
+		return customEmpty.HasData(data)
+	}
+	for fieldName, fieldSchema := range n.schema().Schema {
+		// We only check required fields to determine if the point is zeroed. This is because some optional fields,
+		// such as nested schema.Set can be troublesome to check for zero values. Notifiers that lack required fields
+		// (ex. wecom) can define a custom IsEmpty method to handle this.
+		if !fieldSchema.Computed && fieldSchema.Required && !reflect.ValueOf(data[fieldName]).IsZero() {
+			return true
+		}
+	}
+	return false
+}
+
 func unpackPointConfig(n notifier, data interface{}, name string) *models.EmbeddedContactPoint {
 	pt := n.unpack(data, name)
 	settings := pt.Settings.(map[string]interface{})
@@ -340,8 +356,8 @@ func packContactPoints(ps []*models.EmbeddedContactPoint, data *schema.ResourceD
 	}
 	data.Set("disable_provenance", disableProvenance)
 
-	for n, pts := range pointsPerNotifier {
-		data.Set(n.meta().field, pts)
+	for _, n := range notifiers {
+		data.Set(n.meta().field, pointsPerNotifier[n])
 	}
 
 	return nil

internal/resources/grafana/resource_alerting_contact_point_notifiers.go

@@ -1447,6 +1447,12 @@ type slackNotifier struct{}
 
 var _ notifier = (*slackNotifier)(nil)
 
+func (s slackNotifier) HasData(data map[string]any) bool {
+	// Slack has no simple required fields as they require mutual exclusivity. We rely on `Required` to test for
+	// deletions on update, so instead we define a custom HasData method.
+	return data["url"] != "" || data["token"] != ""
+}
+
 func (s slackNotifier) meta() notifierMeta {
 	return notifierMeta{
 		field:        "slack",
@@ -2050,7 +2056,7 @@ func (w webexNotifier) schema() *schema.Resource {
 	r := commonNotifierResource()
 	r.Schema["token"] = &schema.Schema{
 		Type:        schema.TypeString,
-		Optional:    true,
+		Required:    true,
 		Sensitive:   true,
 		Description: "The bearer token used to authorize the client.",
 	}
@@ -2066,7 +2072,7 @@ func (w webexNotifier) schema() *schema.Resource {
 	}
 	r.Schema["room_id"] = &schema.Schema{
 		Type:        schema.TypeString,
-		Optional:    true,
+		Required:    true,
 		Description: "ID of the Webex Teams room where to send the messages.",
 	}
 	return r
@@ -2250,6 +2256,12 @@ type wecomNotifier struct{}
 
 var _ notifier = (*wecomNotifier)(nil)
 
+func (w wecomNotifier) HasData(data map[string]any) bool {
+	// WeCom has no simple required fields as they require mutual exclusivity. We rely on `Required` to test for
+	// deletions on update, so instead we define a custom HasData method.
+	return data["url"] != "" || data["secret"] != ""
+}
+
 func (w wecomNotifier) meta() notifierMeta {
 	return notifierMeta{
 		field:        "wecom",

internal/resources/grafana/resource_alerting_contact_point_test.go

@@ -320,6 +320,19 @@ func TestAccContactPoint_notifiers(t *testing.T) {
 					resource.TestCheckResourceAttr("grafana_contact_point.receiver_types", "wecom.0.to_user", "to_user"),
 				),
 			},
+			// The following test ensures that the plan remains empty for notifiers when updates are made to
+			// other notifiers in the contact point. This is a regression test to catch certain notifiers without
+			// required fields being deleted on update.
+			{
+				Config: testutils.TestAccExampleWithReplace(t, "resources/grafana_contact_point/_acc_receiver_types.tf", map[string]string{
+					`victor-ops-url`: `updated-victor-ops-url`, // VictorOps is a "safe" update as it's not being tested.
+				}),
+				ExpectNonEmptyPlan: false,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("grafana_contact_point.receiver_types", "victorops.#", "1"),
+					resource.TestCheckResourceAttr("grafana_contact_point.receiver_types", "victorops.0.url", "http://updated-victor-ops-url"),
+				),
+			},
 			// Test blank fields in settings should be omitted.
 			{
 				Config: testutils.TestAccExample(t, "resources/grafana_contact_point/_acc_default_settings.tf"),