Repository: honeycombio/terraform-provider-honeycombio (language: python)

Issue:
Plan is always has changed with multiple recipients
If we have a trigger with multiple recipients than we would never have an empty plan after the apply.

**Versions**
```
    honeycombio = {
      source  = "honeycombio/honeycombio"
      version = "~> 0.3.0"
    }
```

**Steps to reproduce**

```
terraform {
  required_providers {
    honeycombio = {
      source  = "honeycombio/honeycombio"
      version = "~> 0.3.0"
    }
  }
}

provider "honeycombio" {
  api_key = "<api_key_here>"
}

locals {
  dataset = "prod"
}

data "honeycombio_query_specification" "example" {
  calculation {
    op     = "AVG"
    column = "duration_ms"
  }

  filter {
    column = "trace.parent_id"
    op     = "does-not-exist"
  }
}

resource "honeycombio_query" "example" {
  dataset    = local.dataset
  query_json = data.honeycombio_query_specification.example.json
}

data "honeycombio_trigger_recipient" "slack" {
  dataset = local.dataset
  type    = "slack"
  target  = "#alex-test"
}

resource "honeycombio_trigger" "example" {
  name        = "Dummy trigger"
  description = "Dummy trigger"

  query_id = honeycombio_query.example.id
  dataset  = local.dataset

  frequency = 6000 // in seconds, 10 minutes

  threshold {
    op    = ">"
    value = 100000000
  }

  recipient {
    type   = "pagerduty"
    id = "2TLD7x6yunN"
    target = null
  }

  recipient {
    type   = "slack"
    id = data.honeycombio_trigger_recipient.slack.id
    target = "#alex-test"
  }
}
```

Run:
* `terraform apply`
* `terraform apply` again plan is not empty

Same happens if you swap recipient blocks order.

**Additional context**

The next step issues happens if you try to apply the changes since we see the following error:

```
Terraform will perform the following actions:

  # honeycombio_trigger.example will be updated in-place
  ~ resource "honeycombio_trigger" "example" {
        id          = "yF8JrPtDZ7q"
        name        = "Pagerduty dummy"
        # (5 unchanged attributes hidden)

      ~ recipient {
          ~ id     = "2TLD7x6yunN" -> "yWmLrb34bsL"
          + target = "#alex-test"
          ~ type   = "pagerduty" -> "slack"
        }
      ~ recipient {
          ~ id     = "EYNPDUi675" -> "2TLD7x6yunN"
          ~ type   = "slack" -> "pagerduty"
            # (1 unchanged attribute hidden)
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

honeycombio_trigger.example: Modifying... [id=yF8JrPtDZ7q]
╷
│ Error: 422 Unprocessable Entity: recipient type pagerduty cannot have a target
│ 
│   with honeycombio_trigger.example,
│   on test.tf line 117, in resource "honeycombio_trigger" "example":
│  117: resource "honeycombio_trigger" "example" {
│ 
╵
```

Which I believe should be resolved by the recreation of the trigger when recipients are updated.


Discussion / hints:
I don't see an easy way to solve this :cry: 

Adding `ForceNew` to the recipient fields causes it to always recreate the resource, since the issue is with ordering:

```text
Terraform will perform the following actions:

  # honeycombio_trigger.example must be replaced
-/+ resource "honeycombio_trigger" "example" {
      - disabled    = false -> null
      ~ id          = "7Tzn6NBEXNq" -> (known after apply)
        name        = "Dummy trigger"
        # (4 unchanged attributes hidden)

      ~ recipient {
          ~ id     = "HpX5pnZxsxs" -> "pmtbi56yP9C" # forces replacement
          ~ target = "#alex-test" -> (known after apply)
          ~ type   = "slack" -> "pagerduty" # forces replacement
        }
      ~ recipient {
          ~ id     = "pmtbi56yP9C" -> "HpX5pnZxsxs" # forces replacement
          + target = "#alex-test" # forces replacement
          ~ type   = "pagerduty" -> "slack" # forces replacement
        }

        # (1 unchanged block hidden)
    }

Plan: 1 to add, 0 to change, 1 to destroy.
```

Using `schema.TypeSet` works, until you omit one of the fields. Because the omitted fields are `Computed`, they'll always show up as modified:

```text
Terraform will perform the following actions:

  # honeycombio_trigger.test will be updated in-place
  ~ resource "honeycombio_trigger" "test" {
        id        = "prd2NsdKiBE"
        name      = "testing trigger recipient"
        # (4 unchanged attributes hidden)

      - recipient {
          - id     = "f99vk5puRFW" -> null
          - target = "test+honeycomb0@example.com" -> null
          - type   = "email" -> null
        }
      + recipient {
          + id     = (known after apply)
          + target = "test+honeycomb0@example.com"
          + type   = "email"
        }

        # (1 unchanged block hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
```

It seems the behaviour of `Optional` and `Computed` attributes within sets [is undefined](https://discuss.hashicorp.com/t/optional-computed-attribute-in-embedded-resource/17211/3), and since the `id`, `type` and `target` fields are optional and computed if not set, they always show up as a diff. It might be possible to write a `DiffSuppressFunc` to remove these, but they're invoked per-attribute and not per-resource.

I had some luck splitting `recipients` into separate `recipients_id` and `recipients_type_and_target` attributes, but I wasn't able to suppress the "objects have changed outside of Terraform" warning. Since Terraform doesn't load the configuration when performing a read, there's no way to identify which objects were specified via which attribute.

I don't think we can write a separate `trigger_recipients` resource either, as the current API requires sending the full list of recipients.

Any ideas, @jharley?
@LukeCarrier I just opened #132 as a potential (hacky) solution here. Can you confirm it resolves things for you, please?

If so, we'll get a patch release out ASAP
👋  @jharley, unfortunately,  #132 has fixed just a subsequent issue described in `Additional context` session not the issue mentioned in the ticket. We still have changes in the plan which makes plans unreadable :( 

Fix patch:
diff --git a/.vscode/launch.json b/.vscode/launch.json
new file mode 100644
index 00000000..d8e8a6a2
--- /dev/null
+++ b/.vscode/launch.json
@@ -0,0 +1,16 @@
+{
+  "version": "0.2.0",
+  "configurations": [
+    {
+      "name": "Debug Terraform Provider",
+      "type": "go",
+      "request": "launch",
+      "mode": "debug",
+      "program": "${workspaceFolder}",
+      "env": {},
+      "args": [
+        "-debug"
+      ]
+    }
+  ]
+}
diff --git a/honeycombio/resource_trigger.go b/honeycombio/resource_trigger.go
index cdf051e6..40a98994 100644
--- a/honeycombio/resource_trigger.go
+++ b/honeycombio/resource_trigger.go
@@ -162,7 +162,11 @@ func resourceTriggerRead(ctx context.Context, d *schema.ResourceData, meta inter
 
 	d.Set("frequency", t.Frequency)
 
-	err = d.Set("recipient", flattenTriggerRecipients(t.Recipients))
+	declaredRecipients, ok := d.Get("recipient").([]interface{})
+	if !ok {
+		return diag.Errorf("failed to parse recipients for Trigger %s", t.ID)
+	}
+	err = d.Set("recipient", flattenTriggerRecipients(matchRecipientsWithSchema(t.Recipients, declaredRecipients)))
 	if err != nil {
 		return diag.FromErr(err)
 	}
@@ -261,3 +265,53 @@ func flattenTriggerRecipients(rs []honeycombio.TriggerRecipient) []map[string]in
 
 	return result
 }
+
+// Matches read recipients against those declared in HCL and returns
+// the Trigger recipients in a stable order grouped by recipient type.
+//
+// This cannot currently be handled efficiently by a DiffSuppressFunc.
+// See: https://github.com/hashicorp/terraform-plugin-sdk/issues/477
+func matchRecipientsWithSchema(readRecipients []honeycombio.TriggerRecipient, declaredRecipients []interface{}) []honeycombio.TriggerRecipient {
+	result := make([]honeycombio.TriggerRecipient, len(declaredRecipients))
+
+	rMap := make(map[string]honeycombio.TriggerRecipient, len(readRecipients))
+	for _, recipient := range readRecipients {
+		rMap[recipient.ID] = recipient
+	}
+
+	// Build up result, with each readRecipient in the same position as it
+	// appears in declaredRecipients, by looking at each declaredRecipient and
+	// finding its matching readRecipient (via rMap).
+	//
+	// If the declaredRecipient has an ID, this is easy: just look it up and
+	// put it in it's place. Otherwise, try to match it to a readRecipient with
+	// the same type and target. If we can't find it at all, it must be new, so
+	// put it at the end.
+	for i, declaredRcpt := range declaredRecipients {
+		declaredRcpt := declaredRcpt.(map[string]interface{})
+
+		if declaredRcpt["id"] != "" {
+			if v, ok := rMap[declaredRcpt["id"].(string)]; ok {
+				// matched recipient declared by ID
+				result[i] = v
+				delete(rMap, v.ID)
+			}
+		} else {
+			// group result recipients by type
+			for key, rcpt := range rMap {
+				if string(rcpt.Type) == declaredRcpt["type"] && rcpt.Target == declaredRcpt["target"] {
+					result[i] = rcpt
+					delete(rMap, key)
+					break
+				}
+			}
+		}
+	}
+
+	// append unmatched read recipients to the result
+	for _, rcpt := range rMap {
+		result = append(result, rcpt)
+	}
+
+	return result
+}


Test patch:
diff --git a/honeycombio/resource_trigger_test.go b/honeycombio/resource_trigger_test.go
index b53ced01..44906a89 100644
--- a/honeycombio/resource_trigger_test.go
+++ b/honeycombio/resource_trigger_test.go
@@ -105,6 +105,69 @@ func TestAccHoneycombioTrigger_triggerRecipientById(t *testing.T) {
 	})
 }
 
+func TestAccHoneycombioTrigger_recipientOrderingNoDiff(t *testing.T) {
+	dataset := testAccDataset()
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:          testAccPreCheck(t),
+		ProviderFactories: testAccProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: fmt.Sprintf(`
+data "honeycombio_query_specification" "test" {
+  calculation {
+    op     = "COUNT"
+  }
+}
+
+resource "honeycombio_query" "test" {
+  dataset    = "%s"
+  query_json = data.honeycombio_query_specification.test.json
+}
+
+resource "honeycombio_trigger" "test" {
+  name = "Ensure mixed order recipients don't cause infinite diffs"
+
+  query_id = honeycombio_query.test.id
+  dataset  = "%s"
+
+  threshold {
+    op    = ">"
+    value = 1000
+  }
+
+  recipient {
+    type   = "slack"
+    target = "#test2"
+  }
+
+  recipient {
+    type   = "slack"
+    target = "#test"
+  }
+
+  recipient {
+    type   = "email"
+    target = "bob@example.com"
+  }
+
+  recipient {
+    type   = "email"
+    target = "alice@example.com"
+  }
+
+  recipient {
+    type   = "marker"
+    target = "trigger fired"
+  }
+}
+`, dataset, dataset),
+			},
+		},
+	})
+
+}
+
 func testAccTriggerConfigWithFrequency(dataset string, frequency int) string {
 	return fmt.Sprintf(`
 data "honeycombio_query_specification" "test" {