Skip to content

Commit

Permalink
fix: better handle the case where a recipient is modified outside of …
Browse files Browse the repository at this point in the history 
…Terraform (#433)

If a recipient used by a Trigger or Burn Alert was updated outside of
Terraform (i.e. the UI or via the API in some other way) the provider
would get rather cranky and spit out an error like:

> Could not find Recipient xysef83ryH in state

This adjusts that behaviour to instead accept that the recipient is
defined remotely and allow the rest of Terraform's diffing process to
handle the difference and present that plan to the user.

Effects `r/burn_alert` and `r/trigger`

Co-authored-by: Dean Strelau <[email protected]>
  • Loading branch information
@jharley @dstrelau
jharley and dstrelau authored Feb 16, 2024
1 parent b154ae1 commit 3ca8f66
Show file tree
Hide file tree
Showing 6 changed files with 311 additions and 65 deletions.
28 changes: 1 addition & 27 deletions internal/provider/burn_alert_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ import (
"errors"
"strings"

"golang.org/x/exp/slices"

"github.com/honeycombio/terraform-provider-honeycombio/internal/helper/validation"

"github.com/hashicorp/terraform-plugin-framework-validators/float64validator"
Expand Down Expand Up @@ -316,6 +314,7 @@ func (r *burnAlertResource) Read(ctx context.Context, req resource.ReadRequest,
state.ID = types.StringValue(burnAlert.ID)
state.AlertType = types.StringValue(string(burnAlert.AlertType))
state.SLOID = types.StringValue(burnAlert.SLO.ID)
state.Recipients = reconcileReadNotificationRecipientState(burnAlert.Recipients, state.Recipients)

// Process any attributes that could be nil and add them to the state values
if burnAlert.ExhaustionMinutes != nil {
Expand All @@ -330,31 +329,6 @@ func (r *burnAlertResource) Read(ctx context.Context, req resource.ReadRequest,
state.BudgetRateWindowMinutes = types.Int64Value(int64(*burnAlert.BudgetRateWindowMinutes))
}

recipients := make([]models.NotificationRecipientModel, len(burnAlert.Recipients))
if state.Recipients != nil {
// match the recipients to those in the state sorting out type+target vs ID
for i, r := range burnAlert.Recipients {
idx := slices.IndexFunc(state.Recipients, func(s models.NotificationRecipientModel) bool {
if !s.ID.IsNull() {
return s.ID.ValueString() == r.ID
}
return s.Type.ValueString() == string(r.Type) && s.Target.ValueString() == r.Target
})
if idx < 0 {
// this should never happen?! But if it does, we'll just skip it and hope to get a reproducible case
resp.Diagnostics.AddError(
"Error Reading Honeycomb Burn Alert",
"Could not find Recipient "+r.ID+" in state",
)
continue
}
recipients[i] = state.Recipients[idx]
}
} else {
recipients = flattenNotificationRecipients(burnAlert.Recipients)
}
state.Recipients = recipients

// Set the burn alert's attributes in state
resp.Diagnostics.Append(resp.State.Set(ctx, state)...)
}
Expand Down
55 changes: 55 additions & 0 deletions internal/provider/burn_alert_resource_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,46 @@ func TestAcc_BurnAlertResource_recreateOnNotFound(t *testing.T) {
})
}

func TestAcc_BurnAlertResource_HandlesRecipientChangedOutsideOfTerraform(t *testing.T) {
c := testAccClient(t)
ctx := context.Background()
dataset, sloID := burnAlertAccTestSetup(t)

// setup a slack recipient to be used in the burn alert, and modified outside of terraform
channel := "#" + acctest.RandString(8)
rcpt, err := c.Recipients.Create(ctx, &client.Recipient{
Type: client.RecipientTypeSlack,
Details: client.RecipientDetails{
SlackChannel: channel,
},
})
require.NoError(t, err, "failed to create test recipient")
t.Cleanup(func() {
//nolint:errcheck
c.Recipients.Delete(ctx, rcpt.ID)
})

resource.Test(t, resource.TestCase{
PreCheck: testAccPreCheck(t),
ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory,
Steps: []resource.TestStep{
{
Config: testAccConfigBurnAlertWithSlackRecipient(dataset, sloID, channel),
},
{
PreConfig: func() {
// update the channel name outside of Terraform
channel += "-1"
rcpt.Details.SlackChannel = channel
_, err := c.Recipients.Update(ctx, rcpt)
require.NoError(t, err, "failed to update test recipient")
},
Config: testAccConfigBurnAlertWithSlackRecipient(dataset, sloID, channel),
},
},
})
}

// Checks that the exhaustion time burn alert exists, has the correct attributes, and has the correct state
func testAccEnsureSuccessExhaustionTimeAlert(t *testing.T, burnAlert *client.BurnAlert, exhaustionMinutes int, pagerdutySeverity, sloID string) resource.TestCheckFunc {
return resource.ComposeAggregateTestCheckFunc(
Expand Down Expand Up @@ -764,3 +804,18 @@ resource "honeycombio_burn_alert" "test" {
}
}`, dataset, sloID)
}

func testAccConfigBurnAlertWithSlackRecipient(dataset, sloID, channel string) string {
return fmt.Sprintf(`
resource "honeycombio_burn_alert" "test" {
exhaustion_minutes = 60
dataset = "%[1]s"
slo_id = "%[2]s"
recipient {
type = "slack"
target = "%[3]s"
}
}`, dataset, sloID, channel)
}
56 changes: 45 additions & 11 deletions internal/provider/notification_recipients.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier"
"github.com/hashicorp/terraform-plugin-framework/schema/validator"
"github.com/hashicorp/terraform-plugin-framework/types"
"golang.org/x/exp/slices"

"github.com/honeycombio/terraform-provider-honeycombio/client"
"github.com/honeycombio/terraform-provider-honeycombio/internal/helper"
Expand Down Expand Up @@ -84,6 +85,34 @@ func notificationRecipientSchema(allowedTypes []client.RecipientType) schema.Set
}
}

func reconcileReadNotificationRecipientState(remote []client.NotificationRecipient, state []models.NotificationRecipientModel) []models.NotificationRecipientModel {
if state == nil {
// if we don't have any state, we can't reconcile anything so just return the remote recipients
return flattenNotificationRecipients(remote)
}

recipients := make([]models.NotificationRecipientModel, len(remote))
// match the remote recipients to those in the state
// in an effort to preserve the id vs type+target distinction
for i, r := range remote {
idx := slices.IndexFunc(state, func(s models.NotificationRecipientModel) bool {
if !s.ID.IsNull() {
return s.ID.ValueString() == r.ID
}
return s.Type.ValueString() == string(r.Type) && s.Target.ValueString() == r.Target
})
if idx < 0 {
// if we didn't find a match, use the recipient as specified in remote
recipients[i] = notificationRecipientToModel(r)
} else {
// if we found a match, use the stored recipient
recipients[i] = state[idx]
}
}

return recipients
}

func expandNotificationRecipients(n []models.NotificationRecipientModel) []client.NotificationRecipient {
recipients := make([]client.NotificationRecipient, len(n))

Expand All @@ -106,19 +135,24 @@ func expandNotificationRecipients(n []models.NotificationRecipientModel) []clien

func flattenNotificationRecipients(n []client.NotificationRecipient) []models.NotificationRecipientModel {
recipients := make([]models.NotificationRecipientModel, len(n))

for i, r := range n {
rcpt := models.NotificationRecipientModel{
ID: types.StringValue(r.ID),
Type: types.StringValue(string(r.Type)),
Target: types.StringValue(r.Target),
}
if r.Details != nil {
rcpt.Details = make([]models.NotificationRecipientDetailsModel, 1)
rcpt.Details[0].PDSeverity = types.StringValue(string(r.Details.PDSeverity))
}
recipients[i] = rcpt
recipients[i] = notificationRecipientToModel(r)
}

return recipients
}

func notificationRecipientToModel(r client.NotificationRecipient) models.NotificationRecipientModel {
rcpt := models.NotificationRecipientModel{
ID: types.StringValue(r.ID),
Type: types.StringValue(string(r.Type)),
Target: types.StringValue(r.Target),
}
if r.Details != nil {
rcpt.Details = []models.NotificationRecipientDetailsModel{
{PDSeverity: types.StringValue(string(r.Details.PDSeverity))},
}
}

return rcpt
}
140 changes: 140 additions & 0 deletions internal/provider/notification_recipients_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
package provider

import (
"testing"

"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/stretchr/testify/assert"

"github.com/honeycombio/terraform-provider-honeycombio/client"
"github.com/honeycombio/terraform-provider-honeycombio/internal/models"
)

func Test_reconcileReadNotificationRecipientState(t *testing.T) {
type args struct {
remote []client.NotificationRecipient
state []models.NotificationRecipientModel
}
tests := []struct {
name string
args args
want []models.NotificationRecipientModel
}{
{
name: "both empty",
args: args{},
want: []models.NotificationRecipientModel{},
},
{
name: "empty state",
args: args{
remote: []client.NotificationRecipient{
{ID: "abcd12345", Type: client.RecipientTypeEmail, Target: "[email protected]"},
},
state: []models.NotificationRecipientModel{},
},
want: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345"), Type: types.StringValue("email"), Target: types.StringValue("[email protected]")},
},
},
{
name: "empty remote",
args: args{
remote: []client.NotificationRecipient{},
state: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345"), Type: types.StringValue("email"), Target: types.StringValue("[email protected]")},
},
},
want: []models.NotificationRecipientModel{},
},
{
name: "remote and state reconciled",
args: args{
remote: []client.NotificationRecipient{
{ID: "abcd12345", Type: client.RecipientTypeEmail, Target: "[email protected]"},
{ID: "efgh67890", Type: client.RecipientTypeSlack, Target: "#test-channel"},
},
state: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")}, // defined by ID
{Type: types.StringValue("slack"), Target: types.StringValue("#test-channel")}, // defined by type+target
},
},
want: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")},
{Type: types.StringValue("slack"), Target: types.StringValue("#test-channel")},
},
},
{
name: "remote has additional recipients",
args: args{
remote: []client.NotificationRecipient{
{ID: "abcd12345", Type: client.RecipientTypeEmail, Target: "[email protected]"},
{ID: "efgh67890", Type: client.RecipientTypeSlack, Target: "#test-channel"},
{ID: "qrsty3847", Type: client.RecipientTypeSlack, Target: "#test-alerts"},
{
ID: "ijkl13579",
Type: client.RecipientTypePagerDuty,
Target: "test-pagerduty",
Details: &client.NotificationRecipientDetails{
PDSeverity: client.PDSeverityWARNING,
}},
},
state: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")}, // defined by ID
{Type: types.StringValue("slack"), Target: types.StringValue("#test-channel")}, // defined by type+target
},
},
want: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")},
{Type: types.StringValue("slack"), Target: types.StringValue("#test-channel")},
{ID: types.StringValue("qrsty3847"), Type: types.StringValue("slack"), Target: types.StringValue("#test-alerts")},
{
ID: types.StringValue("ijkl13579"),
Type: types.StringValue("pagerduty"),
Target: types.StringValue("test-pagerduty"),
Details: []models.NotificationRecipientDetailsModel{
{PDSeverity: types.StringValue("warning")},
},
},
},
},
{
name: "state has additional recipients",
args: args{
remote: []client.NotificationRecipient{
{ID: "efgh67890", Type: client.RecipientTypeSlack, Target: "#test-foo"},
},
state: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")},
{Type: types.StringValue("slack"), Target: types.StringValue("#test-foo")},
{ID: types.StringValue("ijkl13579"), Details: []models.NotificationRecipientDetailsModel{{PDSeverity: types.StringValue("warning")}}},
},
},
want: []models.NotificationRecipientModel{
{Type: types.StringValue("slack"), Target: types.StringValue("#test-foo")},
},
},
{
name: "state has totally unmatched recipients",
args: args{
remote: []client.NotificationRecipient{
{ID: "efgh67890", Type: client.RecipientTypeSlack, Target: "#test-foo"},
},
state: []models.NotificationRecipientModel{
{ID: types.StringValue("abcd12345")},
{Type: types.StringValue("slack"), Target: types.StringValue("#test-channel")},
{ID: types.StringValue("ijkl13579"), Details: []models.NotificationRecipientDetailsModel{{PDSeverity: types.StringValue("warning")}}},
},
},
want: []models.NotificationRecipientModel{
{ID: types.StringValue("efgh67890"), Type: types.StringValue("slack"), Target: types.StringValue("#test-foo")},
},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, reconcileReadNotificationRecipientState(tt.args.remote, tt.args.state))
})
}
}
28 changes: 1 addition & 27 deletions internal/provider/trigger_resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"regexp"
"strings"

"golang.org/x/exp/slices"

"github.com/hashicorp/terraform-plugin-framework-validators/int64validator"
"github.com/hashicorp/terraform-plugin-framework-validators/listvalidator"
"github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator"
Expand Down Expand Up @@ -288,31 +286,7 @@ func (r *triggerResource) Read(ctx context.Context, req resource.ReadRequest, re
state.Threshold = flattenTriggerThreshold(trigger.Threshold)
state.Frequency = types.Int64Value(int64(trigger.Frequency))
state.EvaluationSchedule = flattenTriggerEvaluationSchedule(trigger)

recipients := make([]models.NotificationRecipientModel, len(trigger.Recipients))
if state.Recipients != nil {
// match the Trigger's recipients to those in the state sorting out type+target vs ID
for i, r := range trigger.Recipients {
idx := slices.IndexFunc(state.Recipients, func(s models.NotificationRecipientModel) bool {
if !s.ID.IsNull() {
return s.ID.ValueString() == r.ID
}
return s.Type.ValueString() == string(r.Type) && s.Target.ValueString() == r.Target
})
if idx < 0 {
// this should never happen?! But if it does, we'll just skip it and hope to get a reproducible case
resp.Diagnostics.AddError(
"Error Reading Honeycomb Trigger",
"Could not find Recipient "+r.ID+" in state",
)
continue
}
recipients[i] = state.Recipients[idx]
}
} else {
recipients = flattenNotificationRecipients(trigger.Recipients)
}
state.Recipients = recipients
state.Recipients = reconcileReadNotificationRecipientState(trigger.Recipients, state.Recipients)

resp.Diagnostics.Append(resp.State.Set(ctx, state)...)
}
Expand Down
Loading

0 comments on commit 3ca8f66

Please sign in to comment.