fix(oncall): make integration labels idempotent (#2536)

* fix(oncall): make integration labels idempotent

The `labels` and `dynamic_labels` attributes caused spurious diffs on
every plan due to two issues:

1. flattenLabels() injected a synthetic "id" field (set to the label
   key name) that is never present in user configuration.

2. TypeList compares elements positionally, but the API may return
   labels in a different order than the config, causing labels to
   appear swapped or removed.

Fix by adding a DiffSuppressFunc that:
- Suppresses the synthetic "id" and map-size diffs from old state
- Compares labels as a set of {key, value} pairs, ignoring order

Also remove the synthetic "id" from flattenLabels() so new state
no longer includes it.

* fix(oncall): mark legacy id suppression for removal in next major version

Author: darkweaver87

internal/resources/oncall/resource_integration.go

@@ -237,8 +237,9 @@ func resourceIntegration() *common.Resource {
 						Type: schema.TypeString,
 					},
 				},
-				Optional:    true,
-				Description: "A list of string-to-string mappings for static labels. Each map must include one key named \"key\" and one key named \"value\" (using the `grafana_oncall_label` datasource).",
+				Optional:         true,
+				DiffSuppressFunc: labelsDiffSuppress,
+				Description:      "A list of string-to-string mappings for static labels. Each map must include one key named \"key\" and one key named \"value\" (using the `grafana_oncall_label` datasource).",
 			},
 			"dynamic_labels": {
 				Type: schema.TypeList,
@@ -248,8 +249,9 @@ func resourceIntegration() *common.Resource {
 						Type: schema.TypeString,
 					},
 				},
-				Optional:    true,
-				Description: "A list of string-to-string mappings for dynamic labels. Each map must include one key named \"key\" and one key named \"value\" (using the `grafana_oncall_label` datasource).",
+				Optional:         true,
+				DiffSuppressFunc: labelsDiffSuppress,
+				Description:      "A list of string-to-string mappings for dynamic labels. Each map must include one key named \"key\" and one key named \"value\" (using the `grafana_oncall_label` datasource).",
 			},
 		},
 	}
@@ -795,6 +797,9 @@ func expandDefaultRoute(input []any) *onCallAPI.DefaultRoute {
 	defaultRoute := onCallAPI.DefaultRoute{}
 
 	for _, r := range input {
+		if r == nil {
+			continue
+		}
 		inputMap := r.(map[string]any)
 		id := inputMap["id"].(string)
 		defaultRoute.ID = id
@@ -825,6 +830,9 @@ func expandLabels(input []any) []*onCallAPI.Label {
 	labelsData := make([]*onCallAPI.Label, 0, 1)
 
 	for _, r := range input {
+		if r == nil {
+			continue
+		}
 		inputMap := r.(map[string]any)
 		key, keyExists := inputMap["key"]
 		value, valueExists := inputMap["value"]
@@ -841,15 +849,65 @@ func expandLabels(input []any) []*onCallAPI.Label {
 }
 
 func flattenLabels(labels []*onCallAPI.Label) []map[string]string {
-	flattenedLabels := make([]map[string]string, 0, 1)
+	flattenedLabels := make([]map[string]string, 0, len(labels))
 
 	for _, l := range labels {
 		flattenedLabels = append(flattenedLabels, map[string]string{
-			"id":    l.Key.Name,
 			"key":   l.Key.Name,
 			"value": l.Value.Name,
 		})
 	}
 
 	return flattenedLabels
 }
+
+// labelsDiffSuppress suppresses spurious diffs on the labels and dynamic_labels attributes.
+// It handles two cases:
+//  1. The synthetic "id" field that flattenLabels previously stored in state.
+//  2. Label ordering: the API may return labels in a different order than the config, but TypeList
+//     compares positionally. We suppress the diff when the set of {key,value} pairs is identical.
+func labelsDiffSuppress(k, old, new string, d *schema.ResourceData) bool {
+	// TODO: remove in next major version.
+	// Suppress the synthetic "id" field that was previously stored in state by
+	// the old flattenLabels. Users upgrading still have "id" in their state and
+	// would see a spurious diff showing its removal without this.
+	if strings.HasSuffix(k, ".id") {
+		return true
+	}
+
+	// For count, map-size, key, and value diffs: suppress if the set of {key, value} pairs is identical.
+	// This also handles the map-size diff caused by the legacy "id" field (3 keys → 2 keys).
+	attr := "labels"
+	if strings.HasPrefix(k, "dynamic_labels") {
+		attr = "dynamic_labels"
+	}
+
+	oldRaw, newRaw := d.GetChange(attr)
+	return labelsSetEqual(oldRaw.([]any), newRaw.([]any))
+}
+
+// labelsSetEqual compares two label lists as sets of {key, value} pairs, ignoring order and the "id" field.
+func labelsSetEqual(a, b []any) bool {
+	if len(a) != len(b) {
+		return false
+	}
+
+	setA := make(map[string]string, len(a))
+	for _, item := range a {
+		m := item.(map[string]any)
+		key, _ := m["key"].(string)
+		value, _ := m["value"].(string)
+		setA[key] = value
+	}
+
+	for _, item := range b {
+		m := item.(map[string]any)
+		key, _ := m["key"].(string)
+		value, _ := m["value"].(string)
+		if setA[key] != value {
+			return false
+		}
+	}
+
+	return true
+}

internal/resources/oncall/resource_integration_labels_test.go

@@ -0,0 +1,131 @@
+package oncall
+
+import (
+	"testing"
+
+	onCallAPI "github.com/grafana/amixr-api-go-client"
+)
+
+func TestFlattenLabels_NoIDField(t *testing.T) {
+	labels := []*onCallAPI.Label{
+		{Key: onCallAPI.KeyValueName{Name: "severity"}, Value: onCallAPI.KeyValueName{Name: "critical"}},
+	}
+
+	result := flattenLabels(labels)
+
+	if len(result) != 1 {
+		t.Fatalf("expected 1 label, got %d", len(result))
+	}
+	if _, exists := result[0]["id"]; exists {
+		t.Error("flattenLabels should not include an 'id' field; it causes spurious diffs because the config never specifies it")
+	}
+	if result[0]["key"] != "severity" {
+		t.Errorf("expected key 'severity', got %q", result[0]["key"])
+	}
+	if result[0]["value"] != "critical" {
+		t.Errorf("expected value 'critical', got %q", result[0]["value"])
+	}
+}
+
+func TestFlattenLabels_Empty(t *testing.T) {
+	result := flattenLabels(nil)
+	if len(result) != 0 {
+		t.Fatalf("expected 0 labels, got %d", len(result))
+	}
+}
+
+func TestExpandLabels_NilElement(t *testing.T) {
+	input := []any{
+		nil,
+		map[string]any{"key": "severity", "value": "critical"},
+		nil,
+	}
+
+	result := expandLabels(input)
+
+	if len(result) != 1 {
+		t.Fatalf("expected 1 label, got %d", len(result))
+	}
+	if result[0].Key.Name != "severity" {
+		t.Errorf("expected key 'severity', got %q", result[0].Key.Name)
+	}
+	if result[0].Value.Name != "critical" {
+		t.Errorf("expected value 'critical', got %q", result[0].Value.Name)
+	}
+}
+
+func TestExpandLabels_AllNil(t *testing.T) {
+	input := []any{nil, nil}
+
+	result := expandLabels(input)
+
+	if len(result) != 0 {
+		t.Fatalf("expected 0 labels, got %d", len(result))
+	}
+}
+
+func TestLabelsSetEqual_SameOrder(t *testing.T) {
+	a := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+		map[string]any{"key": "team", "value": "platform"},
+	}
+	b := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+		map[string]any{"key": "team", "value": "platform"},
+	}
+	if !labelsSetEqual(a, b) {
+		t.Error("expected labels with same content and order to be equal")
+	}
+}
+
+func TestLabelsSetEqual_DifferentOrder(t *testing.T) {
+	a := []any{
+		map[string]any{"key": "team", "value": "platform"},
+		map[string]any{"key": "severity", "value": "critical"},
+	}
+	b := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+		map[string]any{"key": "team", "value": "platform"},
+	}
+	if !labelsSetEqual(a, b) {
+		t.Error("expected labels with same content in different order to be equal")
+	}
+}
+
+func TestLabelsSetEqual_IgnoresIDField(t *testing.T) {
+	// Old state has "id" field, new config does not
+	a := []any{
+		map[string]any{"id": "severity", "key": "severity", "value": "critical"},
+	}
+	b := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+	}
+	if !labelsSetEqual(a, b) {
+		t.Error("expected labels to be equal even when old state has 'id' field")
+	}
+}
+
+func TestLabelsSetEqual_DifferentValues(t *testing.T) {
+	a := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+	}
+	b := []any{
+		map[string]any{"key": "severity", "value": "warning"},
+	}
+	if labelsSetEqual(a, b) {
+		t.Error("expected labels with different values to NOT be equal")
+	}
+}
+
+func TestLabelsSetEqual_DifferentCount(t *testing.T) {
+	a := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+	}
+	b := []any{
+		map[string]any{"key": "severity", "value": "critical"},
+		map[string]any{"key": "team", "value": "platform"},
+	}
+	if labelsSetEqual(a, b) {
+		t.Error("expected labels with different count to NOT be equal")
+	}
+}