From 2245f5455ca3f7e080a4cab2db99f2a50517e44c Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 7 Nov 2023 14:24:27 -0600 Subject: [PATCH 01/11] client: Add support for budget rate burn alerts --- client/burn_alert.go | 31 ++++- client/burn_alert_test.go | 239 +++++++++++++++++++++++++++++--------- 2 files changed, 207 insertions(+), 63 deletions(-) diff --git a/client/burn_alert.go b/client/burn_alert.go index c61283a2..c6b0ce77 100644 --- a/client/burn_alert.go +++ b/client/burn_alert.go @@ -41,12 +41,31 @@ type SLORef struct { } type BurnAlert struct { - ID string `json:"id,omitempty"` - ExhaustionMinutes int `json:"exhaustion_minutes"` - SLO SLORef `json:"slo"` - CreatedAt time.Time `json:"created_at,omitempty"` - UpdatedAt time.Time `json:"updated_at,omitempty"` - Recipients []NotificationRecipient `json:"recipients,omitempty"` + ID string `json:"id,omitempty"` + AlertType string `json:"alert_type"` + ExhaustionMinutes *int `json:"exhaustion_minutes,omitempty"` + BudgetRateWindowMinutes *int `json:"budget_rate_window_minutes,omitempty"` + BudgetRateDecreaseThresholdPerMillion *int `json:"budget_rate_decrease_threshold_per_million,omitempty"` + SLO SLORef `json:"slo"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` + Recipients []NotificationRecipient `json:"recipients,omitempty"` +} + +// BurnAlertAlertType represents a burn alert alert type +type BurnAlertAlertType string + +const ( + BurnAlertAlertTypeExhaustionTime BurnAlertAlertType = "exhaustion_time" + BurnAlertAlertTypeBudgetRate BurnAlertAlertType = "budget_rate" +) + +// BurnAlertAlertTypes returns a list of valid burn alert alert types +func BurnAlertAlertTypes() []BurnAlertAlertType { + return []BurnAlertAlertType{ + BurnAlertAlertTypeExhaustionTime, + BurnAlertAlertTypeBudgetRate, + } } func (s *burnalerts) ListForSLO(ctx context.Context, dataset string, sloId string) ([]BurnAlert, error) { diff --git a/client/burn_alert_test.go b/client/burn_alert_test.go index 85d759de..db78b68a 100644 --- a/client/burn_alert_test.go +++ b/client/burn_alert_test.go @@ -2,6 +2,7 @@ package client import ( "context" + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -10,7 +11,6 @@ import ( func TestBurnAlerts(t *testing.T) { ctx := context.Background() - var burnAlert *BurnAlert var err error c := newTestClient(t) @@ -39,69 +39,194 @@ func TestBurnAlerts(t *testing.T) { c.DerivedColumns.Delete(ctx, dataset, sli.ID) }) - t.Run("Create", func(t *testing.T) { - data := &BurnAlert{ - ExhaustionMinutes: int(24 * 60), // 24 hours - SLO: SLORef{ID: slo.ID}, - Recipients: []NotificationRecipient{ - { - Type: "email", - Target: "testalert@example.com", - }, + var defaultBurnAlert *BurnAlert + exhaustionMinutes24Hours := 24 * 60 + defaultBurnAlertCreateRequest := BurnAlert{ + ExhaustionMinutes: &exhaustionMinutes24Hours, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", }, - } - - burnAlert, err = c.BurnAlerts.Create(ctx, dataset, data) - - assert.NoError(t, err, "failed to create BurnAlert") - assert.NotNil(t, burnAlert.ID, "BurnAlert ID is empty") - assert.NotNil(t, burnAlert.CreatedAt, "created at is empty") - assert.NotNil(t, burnAlert.UpdatedAt, "updated at is empty") - // copy dynamic fields before asserting equality - data.ID = burnAlert.ID - data.CreatedAt = burnAlert.CreatedAt - data.UpdatedAt = burnAlert.UpdatedAt - data.Recipients[0].ID = burnAlert.Recipients[0].ID - assert.Equal(t, data, burnAlert) - }) - - t.Run("Get", func(t *testing.T) { - getBA, err := c.BurnAlerts.Get(ctx, dataset, burnAlert.ID) - assert.NoError(t, err, "failed to get BurnAlert by ID") - assert.Equal(t, burnAlert, getBA) - }) - - t.Run("Update", func(t *testing.T) { - burnAlert.ExhaustionMinutes = int(4 * 60) // 4 hours + }, + } + exhaustionMinutes1Hour := 60 + defaultBurnAlertUpdateRequest := BurnAlert{ + ExhaustionMinutes: &exhaustionMinutes1Hour, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", + }, + }, + } - result, err := c.BurnAlerts.Update(ctx, dataset, burnAlert) + var exhaustionTimeBurnAlert *BurnAlert + exhaustionMinutes0Minutes := 0 + exhaustionTimeBurnAlertCreateRequest := BurnAlert{ + AlertType: string(BurnAlertAlertTypeExhaustionTime), + ExhaustionMinutes: &exhaustionMinutes0Minutes, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", + }, + }, + } + exhaustionMinutes4Hours := 4 * 60 + exhaustionTimeBurnAlertUpdateRequest := BurnAlert{ + AlertType: string(BurnAlertAlertTypeExhaustionTime), + ExhaustionMinutes: &exhaustionMinutes4Hours, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", + }, + }, + } - assert.NoError(t, err, "failed to update BurnAlert") - // copy dynamic field before asserting equality - burnAlert.UpdatedAt = result.UpdatedAt - assert.Equal(t, burnAlert, result) - }) + var budgetRateBurnAlert *BurnAlert + budgetRateWindowMinutes1Hour := 60 + budgetRateDecreaseThresholdPerMillion1Percent := 10000 + budgetRateBurnAlertCreateRequest := BurnAlert{ + AlertType: string(BurnAlertAlertTypeBudgetRate), + BudgetRateWindowMinutes: &budgetRateWindowMinutes1Hour, + BudgetRateDecreaseThresholdPerMillion: &budgetRateDecreaseThresholdPerMillion1Percent, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", + }, + }, + } + budgetRateWindowMinutes2Hours := 2 * 60 + budgetRateDecreaseThresholdPerMillion5Percent := 10000 + budgetRateBurnAlertUpdateRequest := BurnAlert{ + AlertType: string(BurnAlertAlertTypeBudgetRate), + BudgetRateWindowMinutes: &budgetRateWindowMinutes2Hours, + BudgetRateDecreaseThresholdPerMillion: &budgetRateDecreaseThresholdPerMillion5Percent, + SLO: SLORef{ID: slo.ID}, + Recipients: []NotificationRecipient{ + { + Type: "email", + Target: "testalert@example.com", + }, + }, + } - t.Run("ListForSLO", func(t *testing.T) { - results, err := c.BurnAlerts.ListForSLO(ctx, dataset, slo.ID) - burnAlert.Recipients = []NotificationRecipient{} - assert.NoError(t, err, "failed to list burn alerts for SLO") - assert.NotZero(t, len(results)) - assert.Equal(t, burnAlert.ID, results[0].ID, "newly created BurnAlert not in list of SLO's burn alerts") - }) + testCases := map[string]struct { + alertType string + createRequest BurnAlert + updateRequest BurnAlert + burnAlert *BurnAlert + }{ + "default - exhaustion_time": { + alertType: string(BurnAlertAlertTypeExhaustionTime), + createRequest: defaultBurnAlertCreateRequest, + updateRequest: defaultBurnAlertUpdateRequest, + burnAlert: defaultBurnAlert, + }, + "exhaustion_time": { + alertType: string(BurnAlertAlertTypeExhaustionTime), + createRequest: exhaustionTimeBurnAlertCreateRequest, + updateRequest: exhaustionTimeBurnAlertUpdateRequest, + burnAlert: exhaustionTimeBurnAlert, + }, + "budget_rate": { + alertType: string(BurnAlertAlertTypeBudgetRate), + createRequest: budgetRateBurnAlertCreateRequest, + updateRequest: budgetRateBurnAlertUpdateRequest, + burnAlert: budgetRateBurnAlert, + }, + } - t.Run("Delete", func(t *testing.T) { - err = c.BurnAlerts.Delete(ctx, dataset, burnAlert.ID) + for testName, testCase := range testCases { + var burnAlert *BurnAlert + var err error + + t.Run(fmt.Sprintf("Create: %s", testName), func(t *testing.T) { + data := &testCase.createRequest + burnAlert, err = c.BurnAlerts.Create(ctx, dataset, data) + + assert.NoError(t, err, "failed to create BurnAlert") + assert.NotNil(t, burnAlert.ID, "BurnAlert ID is empty") + assert.NotNil(t, burnAlert.CreatedAt, "created at is empty") + assert.NotNil(t, burnAlert.UpdatedAt, "updated at is empty") + assert.Equal(t, testCase.alertType, burnAlert.AlertType) + + // copy dynamic fields before asserting equality + data.AlertType = burnAlert.AlertType + data.ID = burnAlert.ID + data.CreatedAt = burnAlert.CreatedAt + data.UpdatedAt = burnAlert.UpdatedAt + data.Recipients[0].ID = burnAlert.Recipients[0].ID + assert.Equal(t, data, burnAlert) + }) + + t.Run(fmt.Sprintf("Get: %s", testName), func(t *testing.T) { + result, err := c.BurnAlerts.Get(ctx, dataset, burnAlert.ID) + assert.NoError(t, err, "failed to get BurnAlert by ID") + assert.Equal(t, burnAlert, result) + }) + + t.Run(fmt.Sprintf("Update: %s", testName), func(t *testing.T) { + data := &testCase.updateRequest + data.ID = burnAlert.ID + + burnAlert, err = c.BurnAlerts.Update(ctx, dataset, data) + + assert.NoError(t, err, "failed to update BurnAlert") + + // copy dynamic field before asserting equality + data.AlertType = burnAlert.AlertType + data.ID = burnAlert.ID + data.CreatedAt = burnAlert.CreatedAt + data.UpdatedAt = burnAlert.UpdatedAt + data.Recipients[0].ID = burnAlert.Recipients[0].ID + assert.Equal(t, burnAlert, data) + }) + + t.Run(fmt.Sprintf("ListForSLO: %s", testName), func(t *testing.T) { + results, err := c.BurnAlerts.ListForSLO(ctx, dataset, slo.ID) + + assert.NoError(t, err, "failed to list burn alerts for SLO") + assert.NotZero(t, len(results)) + assert.Equal(t, burnAlert.ID, results[0].ID, "newly created BurnAlert not in list of SLO's burn alerts") + }) + + t.Run(fmt.Sprintf("Delete - %s", testName), func(t *testing.T) { + err = c.BurnAlerts.Delete(ctx, dataset, burnAlert.ID) + + assert.NoError(t, err, "failed to delete BurnAlert") + }) + + t.Run(fmt.Sprintf("Fail to GET a deleted burn alert: %s", testName), func(t *testing.T) { + _, err := c.BurnAlerts.Get(ctx, dataset, burnAlert.ID) + + var de DetailedError + assert.Error(t, err) + assert.ErrorAs(t, err, &de) + assert.True(t, de.IsNotFound()) + }) + } +} - assert.NoError(t, err, "failed to delete BurnAlert") - }) +func TestBurnAlerts_BurnAlertAlertTypes(t *testing.T) { + expectedAlertTypes := []BurnAlertAlertType{ + BurnAlertAlertTypeExhaustionTime, + BurnAlertAlertTypeBudgetRate, + } - t.Run("Fail to Get deleted Burn Alert", func(t *testing.T) { - _, err := c.BurnAlerts.Get(ctx, dataset, burnAlert.ID) + t.Run("returns expected burn alert alert types", func(t *testing.T) { + actualAlertTypes := BurnAlertAlertTypes() - var de DetailedError - assert.Error(t, err) - assert.ErrorAs(t, err, &de) - assert.True(t, de.IsNotFound()) + assert.NotEmpty(t, actualAlertTypes) + assert.Equal(t, len(expectedAlertTypes), len(actualAlertTypes)) + assert.ElementsMatch(t, expectedAlertTypes, actualAlertTypes) }) } From 67c1b19d31c9c5cc1ee4002d1b5846eba3e90da5 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 7 Nov 2023 16:15:59 -0600 Subject: [PATCH 02/11] model: Add new burn alert fields to burn alert resource model --- internal/models/burn_alerts.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/models/burn_alerts.go b/internal/models/burn_alerts.go index 6a1578f2..5d894c5a 100644 --- a/internal/models/burn_alerts.go +++ b/internal/models/burn_alerts.go @@ -3,9 +3,12 @@ package models import "github.com/hashicorp/terraform-plugin-framework/types" type BurnAlertResourceModel struct { - ID types.String `tfsdk:"id"` - Dataset types.String `tfsdk:"dataset"` - SLOID types.String `tfsdk:"slo_id"` - ExhaustionMinutes types.Int64 `tfsdk:"exhaustion_minutes"` - Recipients []NotificationRecipientModel `tfsdk:"recipient"` + ID types.String `tfsdk:"id"` + AlertType types.String `tfsdk:"alert_type"` + BudgetRateWindowMinutes types.Int64 `tfsdk:"budget_rate_window_minutes"` + BudgetRateDecreasePercent types.Float64 `tfsdk:"budget_rate_decrease_percent"` + Dataset types.String `tfsdk:"dataset"` + SLOID types.String `tfsdk:"slo_id"` + ExhaustionMinutes types.Int64 `tfsdk:"exhaustion_minutes"` + Recipients []NotificationRecipientModel `tfsdk:"recipient"` } From 5c007921d2dda61f9ad465d8d9a4882a455c216f Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Thu, 9 Nov 2023 12:35:30 -0600 Subject: [PATCH 03/11] r/burn_alert: Add documentation for budget rate burn alerts --- docs/resources/burn_alert.md | 82 ++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/docs/resources/burn_alert.md b/docs/resources/burn_alert.md index 9ecc2fad..9e2730a5 100644 --- a/docs/resources/burn_alert.md +++ b/docs/resources/burn_alert.md @@ -1,11 +1,13 @@ # Resource: honeycombio_burn_alert -Creates a burn alert. For more information about burn alerts, check out [Define Burn Alerts](https://docs.honeycomb.io/working-with-your-data/slos/slo-process/#define-burn-alerts). +Creates a burn alert. -## Example Usage +For more information about burn alerts, +check out [Define Burn Alerts](https://docs.honeycomb.io/working-with-your-data/slos/burn-alerts). -### Basic Example +## Example Usage +### Basic Example - Exhaustion Time Burn Alert ```hcl variable "dataset" { type = string @@ -16,10 +18,12 @@ variable "slo_id" { } resource "honeycombio_burn_alert" "example_alert" { - dataset = var.dataset - slo_id = var.slo_id + alert_type = "exhaustion_time" exhaustion_minutes = 480 + dataset = var.dataset + slo_id = var.slo_id + # one or more recipients recipient { type = "email" @@ -33,7 +37,33 @@ resource "honeycombio_burn_alert" "example_alert" { } ``` -### Example with PagerDuty Recipient and Severity +### Basic Example - Budget Rate Burn Alert +```hcl +variable "dataset" { + type = string +} + +variable "slo_id" { + type = string +} + +resource "honeycombio_burn_alert" "example_alert" { + alert_type = "budget_rate" + budget_rate_window_minutes = 480 + budget_rate_decrease_percent = 1 + + dataset = var.dataset + slo_id = var.slo_id + + # one or more recipients + recipient { + type = "webhook" + target = "name of the webhook" + } +} +``` + +### Example - Exhaustion Time Burn Alert with PagerDuty Recipient and Severity ```hcl variable "dataset" { type = string @@ -43,7 +73,7 @@ variable "slo_id" { type = string } -data "honeycombio_recipient" "pd-prod" { +data "honeycombio_recipient" "pd_prod" { type = "pagerduty" detail_filter { @@ -53,12 +83,12 @@ data "honeycombio_recipient" "pd-prod" { } resource "honeycombio_burn_alert" "example_alert" { + exhaustion_minutes = 60 dataset = var.dataset slo_id = var.slo_id - exhaustion_minutes = 60 recipient { - id = data.honeycombio_recipient.pd-prod.id + id = data.honeycombio_recipient.pd_prod.id notification_details { pagerduty_severity = "critical" @@ -71,10 +101,26 @@ resource "honeycombio_burn_alert" "example_alert" { ## Argument Reference The following arguments are supported: - * `slo_id` - (Required) ID of the SLO this burn alert is associated with. * `dataset` - (Required) The dataset this burn alert is associated with. -* `exhaustion_minutes` - (Required) The amount of time, in minutes, remaining before the SLO's error budget will be exhausted and the alert will fire. +* `alert_type` - (Optional) Type of the burn alert. Valid values are `exhaustion_time` and `budget_rate`. + Defaults to `exhaustion_time`. +* `budget_rate_window_minutes` - (Optional) The time period, in minutes, over which a budget rate will be calculated. + Must be between 60 and the associated SLO's time period. + Required when `alert_type` is `budget_rate`. + Must not be provided when `alert_type` is `exhaustion_time`. +* `budget_rate_decrease_percent` - (Optional) The percent the budget has decreased over the budget rate window. + The alert will fire when this budget decrease threshold is reached. + Must be between 0.0001% and 100%, with no more than 4 numbers past the decimal point. + Required when `alert_type` is `budget_rate`. + Must not be provided when `alert_type` is `exhaustion_time`. +* `exhaustion_minutes` - (Optional) The amount of time, in minutes, remaining before the SLO's error budget will be exhausted and + the alert will fire. + Must be 0 or greater. + Required when `alert_type` is `exhaustion_time`. + Must not be provided when `alert_type` is `budget_rate`. +# TODO: This is a required attribute in the API. It doesn't appear to be required in the provider but +# I think we should probs mark is required in the docs * `recipient` - (Optional) Zero or more configuration blocks (described below) with the recipients to notify when the alert fires. Each burn alert configuration may have one or more `recipient` blocks, which each accept the following arguments. A recipient block can either refer to an existing recipient (a recipient that is already present in another burn alert or trigger) or a new recipient. When specifying an existing recipient, only `id` may be set. If you pass in a recipient without its ID and only include the type and target, Honeycomb will make a best effort to match to an existing recipient. To retrieve the ID of an existing recipient, refer to the [`honeycombio_recipient`](../data-sources/recipient.md) data source. @@ -84,12 +130,12 @@ Each burn alert configuration may have one or more `recipient` blocks, which eac * `id` - (Optional) The ID of an already existing recipient. Should not be used in combination with `type` and `target`. * `notification_details` - (Optional) a block of additional details to send along with the notification. The only supported option currently is `pagerduty_severity` which has a default value of `critical` but can be set to one of `info`, `warning`, `error`, or `critical` and must be used in combination with a PagerDuty recipient. -Type | Target -----------|------------------------- -email | an email address -pagerduty | _N/A_ -slack | name of the channel -webhook | name of the webhook +| Type | Target | +|-----------|---------------------| +| email | an email address | +| pagerduty | _N/A_ | +| slack | name of the channel | +| webhook | name of the webhook | ## Attribute Reference @@ -99,7 +145,7 @@ In addition to all arguments above, the following attributes are exported: ## Import -Burn Alerts can be imported using a combination of the dataset name and their ID, e.g. +Burn alerts can be imported using a combination of the dataset name and their ID, e.g. ``` $ terraform import honeycombio_burn_alert.my_alert my-dataset/bj9BwOb1uKz From 5b1a64ad5acf12936388a7b2dae40346510c58c5 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Mon, 13 Nov 2023 14:45:48 -0600 Subject: [PATCH 04/11] r/burn_alert: Change documentation to note that recipients are required --- docs/resources/burn_alert.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/resources/burn_alert.md b/docs/resources/burn_alert.md index 9e2730a5..8006f85a 100644 --- a/docs/resources/burn_alert.md +++ b/docs/resources/burn_alert.md @@ -119,9 +119,7 @@ The following arguments are supported: Must be 0 or greater. Required when `alert_type` is `exhaustion_time`. Must not be provided when `alert_type` is `budget_rate`. -# TODO: This is a required attribute in the API. It doesn't appear to be required in the provider but -# I think we should probs mark is required in the docs -* `recipient` - (Optional) Zero or more configuration blocks (described below) with the recipients to notify when the alert fires. +* `recipient` - (Required) Zero or more configuration blocks (described below) with the recipients to notify when the alert fires. Each burn alert configuration may have one or more `recipient` blocks, which each accept the following arguments. A recipient block can either refer to an existing recipient (a recipient that is already present in another burn alert or trigger) or a new recipient. When specifying an existing recipient, only `id` may be set. If you pass in a recipient without its ID and only include the type and target, Honeycomb will make a best effort to match to an existing recipient. To retrieve the ID of an existing recipient, refer to the [`honeycombio_recipient`](../data-sources/recipient.md) data source. From fc343af272010cc8f5749284a75f2beb71ccc31b Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 14 Nov 2023 14:24:18 -0600 Subject: [PATCH 05/11] internal/helper: Add helpers for converting between percent and PPM --- internal/helper/type_float.go | 21 +++++++++++++ internal/helper/type_float_test.go | 50 ++++++++++++++++++++++++++++++ internal/helper/type_int.go | 11 +++++++ internal/helper/type_int_test.go | 29 +++++++++++++++++ 4 files changed, 111 insertions(+) create mode 100644 internal/helper/type_float.go create mode 100644 internal/helper/type_float_test.go create mode 100644 internal/helper/type_int.go create mode 100644 internal/helper/type_int_test.go diff --git a/internal/helper/type_float.go b/internal/helper/type_float.go new file mode 100644 index 00000000..e304d670 --- /dev/null +++ b/internal/helper/type_float.go @@ -0,0 +1,21 @@ +package helper + +import ( + "fmt" +) + +// The SLO and burn alert APIs use 'X Per Million' to avoid the problems with floats. +// In the name of offering a nicer UX with percentages, we handle the conversion +// back and forth to allow things like 99.98 to be provided in the HCL and +// handle the conversion to and from 999800 + +// FloatToPPM converts a floating point percentage to a parts per million value +func FloatToPPM(f float64) int { + return int(f * 10000) +} + +// FloatToPercentString converts a floating point percentage to a string, with a maximum of +// 4 decimal places, no trailing zeros +func FloatToPercentString(f float64) string { + return fmt.Sprintf("%g", f) +} diff --git a/internal/helper/type_float_test.go b/internal/helper/type_float_test.go new file mode 100644 index 00000000..225bd9a2 --- /dev/null +++ b/internal/helper/type_float_test.go @@ -0,0 +1,50 @@ +package helper + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTypeFloat_FloatToPPM(t *testing.T) { + testCases := []struct { + input float64 + expected int + }{ + {input: 100, expected: 1000000}, + {input: 99.00, expected: 990000}, + {input: 9.9999, expected: 99999}, + {input: 9, expected: 90000}, + {input: 0.1, expected: 1000}, + {input: 0.010, expected: 100}, + {input: 0.001, expected: 10}, + {input: 0.0001, expected: 1}, + } + for _, testCase := range testCases { + t.Run(fmt.Sprintf("returns correct value for input %g", testCase.input), func(t *testing.T) { + assert.Equal(t, testCase.expected, FloatToPPM(testCase.input)) + }) + } +} + +func TestTypeFloat_FloatToPercentString(t *testing.T) { + testCases := []struct { + input float64 + expected string + }{ + {input: 100, expected: "100"}, + {input: 99.00, expected: "99"}, + {input: 9.9999, expected: "9.9999"}, + {input: 9, expected: "9"}, + {input: 0.1, expected: "0.1"}, + {input: 0.010, expected: "0.01"}, + {input: 0.001, expected: "0.001"}, + {input: 0.0001, expected: "0.0001"}, + } + for _, testCase := range testCases { + t.Run(fmt.Sprintf("returns correct value for input %g", testCase.input), func(t *testing.T) { + assert.Equal(t, testCase.expected, FloatToPercentString(testCase.input)) + }) + } +} diff --git a/internal/helper/type_int.go b/internal/helper/type_int.go new file mode 100644 index 00000000..2899aa42 --- /dev/null +++ b/internal/helper/type_int.go @@ -0,0 +1,11 @@ +package helper + +// The SLO and burn alert APIs use 'X Per Million' to avoid the problems with floats. +// In the name of offering a nicer UX with percentages, we handle the conversion +// back and forth to allow things like 99.98 to be provided in the HCL and +// handle the conversion to and from 999800 + +// converts a parts per million value to a floating point percentage +func PPMToFloat(t int) float64 { + return float64(t) / 10000 +} diff --git a/internal/helper/type_int_test.go b/internal/helper/type_int_test.go new file mode 100644 index 00000000..7fa2ab7d --- /dev/null +++ b/internal/helper/type_int_test.go @@ -0,0 +1,29 @@ +package helper + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestTypeInt_PPMToFloat(t *testing.T) { + testCases := []struct { + input int + expected float64 + }{ + {input: 1000000, expected: 100}, + {input: 990000, expected: 99}, + {input: 99999, expected: 9.9999}, + {input: 90000, expected: 9}, + {input: 1000, expected: 0.1}, + {input: 100, expected: 0.01}, + {input: 10, expected: 0.001}, + {input: 1, expected: 0.0001}, + } + for _, testCase := range testCases { + t.Run(fmt.Sprintf("returns correct value for input %d", testCase.input), func(t *testing.T) { + assert.Equal(t, testCase.expected, PPMToFloat(testCase.input)) + }) + } +} From 6f5b0b4b6f72ac694abbcd61377917358cf11e50 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 14 Nov 2023 14:01:48 -0600 Subject: [PATCH 06/11] honeycombio/type_helpers: Remove tpmToFloat/floatToTPM and replace usages in r/slo with equivalent helpers from internal/helper --- honeycombio/resource_slo.go | 5 +++-- honeycombio/type_helpers.go | 15 --------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/honeycombio/resource_slo.go b/honeycombio/resource_slo.go index 0a7dc7d4..8530abac 100644 --- a/honeycombio/resource_slo.go +++ b/honeycombio/resource_slo.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" honeycombio "github.com/honeycombio/terraform-provider-honeycombio/client" + "github.com/honeycombio/terraform-provider-honeycombio/internal/helper" ) func newSLO() *schema.Resource { @@ -117,7 +118,7 @@ func resourceSLORead(ctx context.Context, d *schema.ResourceData, meta interface d.Set("name", s.Name) d.Set("description", s.Description) d.Set("sli", s.SLI.Alias) - d.Set("target_percentage", tpmToFloat(s.TargetPerMillion)) + d.Set("target_percentage", helper.PPMToFloat(s.TargetPerMillion)) d.Set("time_period", s.TimePeriodDays) return nil @@ -159,7 +160,7 @@ func expandSLO(d *schema.ResourceData) (*honeycombio.SLO, error) { Name: d.Get("name").(string), Description: d.Get("description").(string), TimePeriodDays: d.Get("time_period").(int), - TargetPerMillion: floatToTPM(d.Get("target_percentage").(float64)), + TargetPerMillion: helper.FloatToPPM(d.Get("target_percentage").(float64)), SLI: honeycombio.SLIRef{Alias: d.Get("sli").(string)}, } return s, nil diff --git a/honeycombio/type_helpers.go b/honeycombio/type_helpers.go index fda56210..5fdf2a31 100644 --- a/honeycombio/type_helpers.go +++ b/honeycombio/type_helpers.go @@ -29,21 +29,6 @@ func coerceValueToType(i string) interface{} { return i } -// The SLO API uses 'Target Per Million' to avoid the problems with floats. -// In the name of offering a nicer UX with percentages, we handle the conversion -// back and fourth to allow things like 99.98 to be provided in the HCL and -// handle the conversion to and from 999800 - -// converts a floating point percentage to a 'Target Per Million' SLO value -func floatToTPM(f float64) int { - return int(f * 10000) -} - -// converts a SLO 'Target Per Million' value to a floating point percentage -func tpmToFloat(t int) float64 { - return float64(t) / 10000 -} - func expandRecipient(t honeycombio.RecipientType, d *schema.ResourceData) (*honeycombio.Recipient, error) { r := &honeycombio.Recipient{ ID: d.Id(), From 6da3c1d67821bbef460550e29b7ff1d089022149 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 14 Nov 2023 15:48:03 -0600 Subject: [PATCH 07/11] internal/helper/validation: Add a custom validator to check the number of places past the decimal point --- .../helper/validation/precision_at_most.go | 62 +++++++++++++++ .../validation/precision_at_most_test.go | 76 +++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 internal/helper/validation/precision_at_most.go create mode 100644 internal/helper/validation/precision_at_most_test.go diff --git a/internal/helper/validation/precision_at_most.go b/internal/helper/validation/precision_at_most.go new file mode 100644 index 00000000..32456307 --- /dev/null +++ b/internal/helper/validation/precision_at_most.go @@ -0,0 +1,62 @@ +package validation + +import ( + "context" + "fmt" + "strings" + + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/honeycombio/terraform-provider-honeycombio/internal/helper" + + "github.com/hashicorp/terraform-plugin-framework-validators/helpers/validatordiag" +) + +var _ validator.Float64 = precisionAtMostValidator{} + +// precisionAtMostValidator validates that a float attribute's precision is at most a certain value. +type precisionAtMostValidator struct { + max int64 +} + +func (validator precisionAtMostValidator) Description(_ context.Context) string { + return fmt.Sprintf("precision for value must be at most %d", validator.max) +} + +func (validator precisionAtMostValidator) MarkdownDescription(ctx context.Context) string { + return validator.Description(ctx) +} + +// ValidateFloat64 performs the validation. +func (validator precisionAtMostValidator) ValidateFloat64(ctx context.Context, request validator.Float64Request, response *validator.Float64Response) { + if request.ConfigValue.IsNull() || request.ConfigValue.IsUnknown() { + return + } + + value := request.ConfigValue.ValueFloat64() + valueAsString := helper.FloatToPercentString(value) + splitValue := strings.Split(valueAsString, ".") + + // If length is less than 2, then we don't have a decimal point + // so we know the precision is valid + if len(splitValue) < 2 { + return + } + + afterDecimal := splitValue[1] + valuePrecision := int64(len(afterDecimal)) + if valuePrecision > validator.max { + response.Diagnostics.Append(validatordiag.InvalidAttributeValueDiagnostic( + request.Path, + validator.Description(ctx), + fmt.Sprintf("%f", value), + )) + } +} + +// Float64PrecisionAtMost returns an AttributeValidator which ensures that any configured +// attribute value has a precision which is at most the given max +func Float64PrecisionAtMost(max int64) validator.Float64 { + return precisionAtMostValidator{ + max: max, + } +} diff --git a/internal/helper/validation/precision_at_most_test.go b/internal/helper/validation/precision_at_most_test.go new file mode 100644 index 00000000..d022c81d --- /dev/null +++ b/internal/helper/validation/precision_at_most_test.go @@ -0,0 +1,76 @@ +package validation_test + +import ( + "context" + "testing" + + "github.com/hashicorp/terraform-plugin-framework/path" + "github.com/hashicorp/terraform-plugin-framework/schema/validator" + "github.com/hashicorp/terraform-plugin-framework/types" + "github.com/honeycombio/terraform-provider-honeycombio/internal/helper/validation" +) + +func TestValidation_PrecisionAtMost(t *testing.T) { + t.Parallel() + + type testCase struct { + value types.Float64 + maxPrecision int64 + expectError bool + } + + tests := map[string]testCase{ + "unknown value": { + value: types.Float64Unknown(), + maxPrecision: 5, + }, + "null value": { + value: types.Float64Null(), + maxPrecision: 4, + }, + "valid value at precision limit": { + value: types.Float64Value(0.001), + maxPrecision: 3, + }, + "valid value under precision limit": { + value: types.Float64Value(5), + maxPrecision: 2, + }, + "valid large value under precision limit": { + value: types.Float64Value(123), + maxPrecision: 2, + }, + // trailing zeros won't impact the conversion from percent to ppm so there's no need to fail + "valid value over precision limit with trailing zeros doesn't fail because the trailing zeros don't matter": { + value: types.Float64Value(99.00000), + maxPrecision: 4, + }, + "invalid value over precision limit": { + value: types.Float64Value(99.99999), + maxPrecision: 4, + expectError: true, + }, + } + + for name, test := range tests { + name, test := name, test + t.Run(name, func(t *testing.T) { + t.Parallel() + request := validator.Float64Request{ + Path: path.Root("test"), + PathExpression: path.MatchRoot("test"), + ConfigValue: test.value, + } + response := validator.Float64Response{} + validation.Float64PrecisionAtMost(test.maxPrecision).ValidateFloat64(context.TODO(), request, &response) + + if !response.Diagnostics.HasError() && test.expectError { + t.Fatal("expected error, got no error") + } + + if response.Diagnostics.HasError() && !test.expectError { + t.Fatalf("got unexpected error: %s", response.Diagnostics) + } + }) + } +} From 97eaa2bf50509a36c3541b1124643e47a1fcf6ec Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Tue, 7 Nov 2023 16:16:44 -0600 Subject: [PATCH 08/11] r/burn_alert: Add budget rate burn alert attributes to the schema and validate them --- internal/provider/burn_alert_resource.go | 123 ++++++++++++++++++++++- 1 file changed, 119 insertions(+), 4 deletions(-) diff --git a/internal/provider/burn_alert_resource.go b/internal/provider/burn_alert_resource.go index 7d1438fb..1202bbc7 100644 --- a/internal/provider/burn_alert_resource.go +++ b/internal/provider/burn_alert_resource.go @@ -5,12 +5,17 @@ import ( "errors" "strings" + "github.com/honeycombio/terraform-provider-honeycombio/internal/helper/validation" "golang.org/x/exp/slices" + "github.com/hashicorp/terraform-plugin-framework-validators/float64validator" "github.com/hashicorp/terraform-plugin-framework-validators/int64validator" + "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" + "github.com/hashicorp/terraform-plugin-framework/path" "github.com/hashicorp/terraform-plugin-framework/resource" "github.com/hashicorp/terraform-plugin-framework/resource/schema" "github.com/hashicorp/terraform-plugin-framework/resource/schema/planmodifier" + "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringdefault" "github.com/hashicorp/terraform-plugin-framework/resource/schema/stringplanmodifier" "github.com/hashicorp/terraform-plugin-framework/schema/validator" "github.com/hashicorp/terraform-plugin-framework/types" @@ -22,9 +27,10 @@ import ( // Ensure the implementation satisfies the expected interfaces. var ( - _ resource.Resource = &burnAlertResource{} - _ resource.ResourceWithConfigure = &burnAlertResource{} - _ resource.ResourceWithImportState = &burnAlertResource{} + _ resource.Resource = &burnAlertResource{} + _ resource.ResourceWithConfigure = &burnAlertResource{} + _ resource.ResourceWithImportState = &burnAlertResource{} + _ resource.ResourceWithValidateConfig = &burnAlertResource{} ) type burnAlertResource struct { @@ -56,6 +62,31 @@ func (*burnAlertResource) Schema(_ context.Context, _ resource.SchemaRequest, re stringplanmodifier.UseStateForUnknown(), }, }, + "alert_type": schema.StringAttribute{ + Optional: true, + Computed: true, + Description: "The alert type of this Burn Alert.", + Default: stringdefault.StaticString(string(client.BurnAlertAlertTypeExhaustionTime)), + Validators: []validator.String{ + stringvalidator.OneOf(helper.AsStringSlice(client.BurnAlertAlertTypes())...), + }, + }, + "budget_rate_decrease_percent": schema.Float64Attribute{ + Optional: true, + Description: "The percent the budget has decreased over the budget rate window.", + Validators: []validator.Float64{ + float64validator.AtLeast(0.0001), + float64validator.AtMost(100), + validation.Float64PrecisionAtMost(4), + }, + }, + "budget_rate_window_minutes": schema.Int64Attribute{ + Optional: true, + Description: "The time period, in minutes, over which a budget rate will be calculated.", + Validators: []validator.Int64{ + int64validator.AtLeast(60), + }, + }, "dataset": schema.StringAttribute{ Required: true, Description: "The dataset this Burn Alert is associated with.", @@ -64,7 +95,7 @@ func (*burnAlertResource) Schema(_ context.Context, _ resource.SchemaRequest, re }, }, "exhaustion_minutes": schema.Int64Attribute{ - Required: true, + Optional: true, Description: "The amount of time, in minutes, remaining before the SLO's error budget will be exhausted and the alert will fire.", Validators: []validator.Int64{ int64validator.AtLeast(0), @@ -84,6 +115,90 @@ func (*burnAlertResource) Schema(_ context.Context, _ resource.SchemaRequest, re } } +func validateAttributesWhenAlertTypeIsExhaustionTime(data models.BurnAlertResourceModel, resp *resource.ValidateConfigResponse) { + // Check that the alert_type is exhaustion_time or that it is not configured(which means we default to exhaustion_time) + if data.AlertType.IsNull() || data.AlertType.IsUnknown() || data.AlertType.ValueString() == string(client.BurnAlertAlertTypeExhaustionTime) { + // When the alert_type is exhaustion_time, exhaustion_minutes is required + if data.ExhaustionMinutes.IsNull() || data.ExhaustionMinutes.IsUnknown() { + resp.Diagnostics.AddAttributeError( + path.Root("exhaustion_minutes"), + "Missing required argument", + "The argument \"exhaustion_minutes\" is required, but no definition was found.", + ) + } + + // When the alert_type is exhaustion_time, budget_rate_decrease_percent must not be configured + if !(data.BudgetRateDecreasePercent.IsNull() || data.BudgetRateDecreasePercent.IsUnknown()) { + resp.Diagnostics.AddAttributeError( + path.Root("budget_rate_decrease_percent"), + "Conflicting configuration arguments", + "\"budget_rate_decrease_percent\": must not be configured when \"alert_type\" is \"exhaustion_time\"", + ) + } + + // When the alert_type is exhaustion_time, budget_rate_window_minutes must not be configured + if !(data.BudgetRateWindowMinutes.IsNull() || data.BudgetRateWindowMinutes.IsUnknown()) { + resp.Diagnostics.AddAttributeError( + path.Root("budget_rate_window_minutes"), + "Conflicting configuration arguments", + "\"budget_rate_window_minutes\": must not be configured when \"alert_type\" is \"exhaustion_time\"", + ) + } + } +} + +func validateAttributesWhenAlertTypeIsBudgetRate(data models.BurnAlertResourceModel, resp *resource.ValidateConfigResponse) { + // Check if the alert_type is budget_rate + if data.AlertType.ValueString() == string(client.BurnAlertAlertTypeBudgetRate) { + // When the alert_type is budget_rate, budget_rate_decrease_percent is required + if data.BudgetRateDecreasePercent.IsNull() || data.BudgetRateDecreasePercent.IsUnknown() { + resp.Diagnostics.AddAttributeError( + path.Root("budget_rate_decrease_percent"), + "Missing required argument", + "The argument \"budget_rate_decrease_percent\" is required, but no definition was found.", + ) + } + + // When the alert_type is budget_rate, budget_rate_window_minutes is required + if data.BudgetRateWindowMinutes.IsNull() || data.BudgetRateWindowMinutes.IsUnknown() { + resp.Diagnostics.AddAttributeError( + path.Root("budget_rate_window_minutes"), + "Missing required argument", + "The argument \"budget_rate_window_minutes\" is required, but no definition was found.", + ) + } + + // When the alert_type is budget_rate, exhaustion_minutes must not be configured + if !(data.ExhaustionMinutes.IsNull() || data.ExhaustionMinutes.IsUnknown()) { + resp.Diagnostics.AddAttributeError( + path.Root("exhaustion_minutes"), + "Conflicting configuration arguments", + "\"exhaustion_minutes\": must not be configured when \"alert_type\" is \"budget_rate\"", + ) + } + } +} + +func (r *burnAlertResource) ValidateConfig(ctx context.Context, req resource.ValidateConfigRequest, resp *resource.ValidateConfigResponse) { + var data models.BurnAlertResourceModel + + resp.Diagnostics.Append(req.Config.Get(ctx, &data)...) + + if resp.Diagnostics.HasError() { + return + } + + // When alert_type is exhaustion_time, check that exhaustion_minutes + // is configured and budget rate attributes are not configured + validateAttributesWhenAlertTypeIsExhaustionTime(data, resp) + + // When alert_type is budget_rate, check that budget rate + // attributes are configured and exhaustion_minutes is not configured + validateAttributesWhenAlertTypeIsBudgetRate(data, resp) + + return +} + func (r *burnAlertResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { // import ID is of the format / // note that the dataset name can also contain '/' From 7286d0b391e4d73f798ca5b7ecf78b84778f2c19 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Wed, 15 Nov 2023 11:14:33 -0600 Subject: [PATCH 09/11] r/burn_alert: Import - fix small typo in error message --- internal/provider/burn_alert_resource.go | 2 +- internal/provider/burn_alert_resource_test.go | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/internal/provider/burn_alert_resource.go b/internal/provider/burn_alert_resource.go index 1202bbc7..d61ceb0f 100644 --- a/internal/provider/burn_alert_resource.go +++ b/internal/provider/burn_alert_resource.go @@ -206,7 +206,7 @@ func (r *burnAlertResource) ImportState(ctx context.Context, req resource.Import if len(idSegments) < 2 { resp.Diagnostics.AddError( "Invalid Import ID", - "The supplied ID must be wrtten as /.", + "The supplied ID must be written as /.", ) return } diff --git a/internal/provider/burn_alert_resource_test.go b/internal/provider/burn_alert_resource_test.go index 4b465c3d..313b26cd 100644 --- a/internal/provider/burn_alert_resource_test.go +++ b/internal/provider/burn_alert_resource_test.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "regexp" "testing" "github.com/hashicorp/terraform-plugin-testing/helper/acctest" @@ -85,6 +86,33 @@ func TestAcc_BurnAlertResourceUpgradeFromVersion015(t *testing.T) { }) } +func TestAcc_BurnAlertResource_Import_validateImportID(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} + + exhaustionMinutes := 240 + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + // Create resource for importing + { + Config: testAccConfigBurnAlertDefault_basic(exhaustionMinutes, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionMinutes, "info", sloID), + }, + // Import with invalid import ID + { + ResourceName: "honeycombio_burn_alert.test", + ImportStateIdPrefix: fmt.Sprintf("%v.", dataset), + ImportState: true, + ExpectError: regexp.MustCompile(`Error: Invalid Import ID`), + }, + }, + }) +} + func testAccEnsureBurnAlertExists(t *testing.T, name string) resource.TestCheckFunc { return func(s *terraform.State) error { resourceState, ok := s.RootModule().Resources[name] From 1eb0dfd4ae578a6260967a799c24cc30ffc7f843 Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Wed, 15 Nov 2023 11:15:11 -0600 Subject: [PATCH 10/11] r/burn_alert: Create/Read/Update/Delete - Add support for budget rate burn alerts --- internal/provider/burn_alert_resource.go | 134 ++++-- internal/provider/burn_alert_resource_test.go | 403 +++++++++++++++++- 2 files changed, 490 insertions(+), 47 deletions(-) diff --git a/internal/provider/burn_alert_resource.go b/internal/provider/burn_alert_resource.go index d61ceb0f..ae9feafb 100644 --- a/internal/provider/burn_alert_resource.go +++ b/internal/provider/burn_alert_resource.go @@ -222,41 +222,76 @@ func (r *burnAlertResource) ImportState(ctx context.Context, req resource.Import func (r *burnAlertResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { var plan, config models.BurnAlertResourceModel + // Read in the config and plan data resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) if resp.Diagnostics.HasError() { return } - burnAlert, err := r.client.BurnAlerts.Create(ctx, plan.Dataset.ValueString(), &client.BurnAlert{ - ExhaustionMinutes: int(plan.ExhaustionMinutes.ValueInt64()), - SLO: client.SLORef{ID: plan.SLOID.ValueString()}, - Recipients: expandNotificationRecipients(plan.Recipients), - }) + // Get attributes from config and construct the create request + createRequest := &client.BurnAlert{ + AlertType: plan.AlertType.ValueString(), + Recipients: expandNotificationRecipients(plan.Recipients), + SLO: client.SLORef{ID: plan.SLOID.ValueString()}, + } + + // Process any attributes that could be nil and add them to the create request + exhaustionMinutes := int(plan.ExhaustionMinutes.ValueInt64()) + // Must convert from float to PPM because the API only accepts PPM + budgetRateDecreasePercentAsPPM := helper.FloatToPPM(plan.BudgetRateDecreasePercent.ValueFloat64()) + budgetRateWindowMinutes := int(plan.BudgetRateWindowMinutes.ValueInt64()) + if plan.AlertType.ValueString() == string(client.BurnAlertAlertTypeExhaustionTime) { + createRequest.ExhaustionMinutes = &exhaustionMinutes + } + if plan.AlertType.ValueString() == string(client.BurnAlertAlertTypeBudgetRate) { + createRequest.BudgetRateDecreaseThresholdPerMillion = &budgetRateDecreasePercentAsPPM + createRequest.BudgetRateWindowMinutes = &budgetRateWindowMinutes + } + + // Create the new burn alert + burnAlert, err := r.client.BurnAlerts.Create(ctx, plan.Dataset.ValueString(), createRequest) if helper.AddDiagnosticOnError(&resp.Diagnostics, "Creating Honeycomb Burn Alert", err) { return } + // Get attributes from the new burn alert and construct the state values var state models.BurnAlertResourceModel - state.Dataset = plan.Dataset state.ID = types.StringValue(burnAlert.ID) - state.ExhaustionMinutes = types.Int64Value(int64(burnAlert.ExhaustionMinutes)) - state.SLOID = types.StringValue(burnAlert.SLO.ID) + state.AlertType = types.StringValue(burnAlert.AlertType) + state.Dataset = plan.Dataset // we created them as authored so to avoid matching type-target or ID we can just use the same value state.Recipients = config.Recipients + state.SLOID = types.StringValue(burnAlert.SLO.ID) + + // Process any attributes that could be nil and add them to the state values + if burnAlert.ExhaustionMinutes != nil { + state.ExhaustionMinutes = types.Int64Value(int64(*burnAlert.ExhaustionMinutes)) + } + if burnAlert.BudgetRateDecreaseThresholdPerMillion != nil { + // Must convert from PPM back to float to match what the user has in their config + budgetRateDecreaseThresholdPerMillionAsPercent := helper.PPMToFloat(*burnAlert.BudgetRateDecreaseThresholdPerMillion) + state.BudgetRateDecreasePercent = types.Float64Value(budgetRateDecreaseThresholdPerMillionAsPercent) + } + if burnAlert.BudgetRateWindowMinutes != nil { + state.BudgetRateWindowMinutes = types.Int64Value(int64(*burnAlert.BudgetRateWindowMinutes)) + } + // Set the new burn alert's attributes in state resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } func (r *burnAlertResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { var state models.BurnAlertResourceModel + // Read in the state data resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } + // Read the burn alert, using the values from state var detailedErr *client.DetailedError - ba, err := r.client.BurnAlerts.Get(ctx, state.Dataset.ValueString(), state.ID.ValueString()) + burnAlert, err := r.client.BurnAlerts.Get(ctx, state.Dataset.ValueString(), state.ID.ValueString()) if errors.As(err, &detailedErr) { if detailedErr.IsNotFound() { resp.State.RemoveResource(ctx) @@ -276,14 +311,28 @@ func (r *burnAlertResource) Read(ctx context.Context, req resource.ReadRequest, return } - state.ID = types.StringValue(ba.ID) - state.ExhaustionMinutes = types.Int64Value(int64(ba.ExhaustionMinutes)) - state.SLOID = types.StringValue(ba.SLO.ID) + // Get attributes from the burn alert and construct the state values + state.ID = types.StringValue(burnAlert.ID) + state.AlertType = types.StringValue(burnAlert.AlertType) + state.SLOID = types.StringValue(burnAlert.SLO.ID) + + // Process any attributes that could be nil and add them to the state values + if burnAlert.ExhaustionMinutes != nil { + state.ExhaustionMinutes = types.Int64Value(int64(*burnAlert.ExhaustionMinutes)) + } + if burnAlert.BudgetRateDecreaseThresholdPerMillion != nil { + // Must convert from PPM back to float to match what the user has in their config + budgetRateDecreaseThresholdPerMillionAsPercent := helper.PPMToFloat(*burnAlert.BudgetRateDecreaseThresholdPerMillion) + state.BudgetRateDecreasePercent = types.Float64Value(budgetRateDecreaseThresholdPerMillionAsPercent) + } + if burnAlert.BudgetRateWindowMinutes != nil { + state.BudgetRateWindowMinutes = types.Int64Value(int64(*burnAlert.BudgetRateWindowMinutes)) + } - recipients := make([]models.NotificationRecipientModel, len(ba.Recipients)) + 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 ba.Recipients { + 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 @@ -301,54 +350,91 @@ func (r *burnAlertResource) Read(ctx context.Context, req resource.ReadRequest, recipients[i] = state.Recipients[idx] } } else { - recipients = flattenNotificationRecipients(ba.Recipients) + recipients = flattenNotificationRecipients(burnAlert.Recipients) } state.Recipients = recipients + // Set the burn alert's attributes in state resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } func (r *burnAlertResource) Update(ctx context.Context, req resource.UpdateRequest, resp *resource.UpdateResponse) { var plan, config models.BurnAlertResourceModel + // Read in the config and plan data resp.Diagnostics.Append(req.Config.Get(ctx, &config)...) resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) if resp.Diagnostics.HasError() { return } - _, err := r.client.BurnAlerts.Update(ctx, plan.Dataset.ValueString(), &client.BurnAlert{ - ID: plan.ID.ValueString(), - ExhaustionMinutes: int(plan.ExhaustionMinutes.ValueInt64()), - SLO: client.SLORef{ID: plan.SLOID.ValueString()}, - Recipients: expandNotificationRecipients(plan.Recipients), - }) + // Get attributes from config and construct the update request + updateRequest := &client.BurnAlert{ + ID: plan.ID.ValueString(), + AlertType: plan.AlertType.ValueString(), + Recipients: expandNotificationRecipients(plan.Recipients), + SLO: client.SLORef{ID: plan.SLOID.ValueString()}, + } + + // Process any attributes that could be nil and add them to the update request + exhaustionMinutes := int(plan.ExhaustionMinutes.ValueInt64()) + // Must convert from float to PPM because the API only accepts PPM + budgetRateDecreasePercentAsPPM := helper.FloatToPPM(plan.BudgetRateDecreasePercent.ValueFloat64()) + budgetRateWindowMinutes := int(plan.BudgetRateWindowMinutes.ValueInt64()) + if plan.AlertType.ValueString() == string(client.BurnAlertAlertTypeExhaustionTime) { + updateRequest.ExhaustionMinutes = &exhaustionMinutes + } + if plan.AlertType.ValueString() == string(client.BurnAlertAlertTypeBudgetRate) { + updateRequest.BudgetRateDecreaseThresholdPerMillion = &budgetRateDecreasePercentAsPPM + updateRequest.BudgetRateWindowMinutes = &budgetRateWindowMinutes + } + + // Update the burn alert + _, err := r.client.BurnAlerts.Update(ctx, plan.Dataset.ValueString(), updateRequest) if helper.AddDiagnosticOnError(&resp.Diagnostics, "Updating Honeycomb Burn Alert", err) { return } + // Read the updated burn alert burnAlert, err := r.client.BurnAlerts.Get(ctx, plan.Dataset.ValueString(), plan.ID.ValueString()) if helper.AddDiagnosticOnError(&resp.Diagnostics, "Updating Honeycomb Burn Alert", err) { return } + // Get attributes from the updated burn alert and construct the state values var state models.BurnAlertResourceModel - state.Dataset = plan.Dataset state.ID = types.StringValue(burnAlert.ID) - state.ExhaustionMinutes = types.Int64Value(int64(burnAlert.ExhaustionMinutes)) - state.SLOID = types.StringValue(burnAlert.SLO.ID) + state.AlertType = types.StringValue(burnAlert.AlertType) + state.Dataset = plan.Dataset // we created them as authored so to avoid matching type-target or ID we can just use the same value state.Recipients = config.Recipients + state.SLOID = types.StringValue(burnAlert.SLO.ID) + + // Process any attributes that could be nil and add them to the state values + if burnAlert.ExhaustionMinutes != nil { + state.ExhaustionMinutes = types.Int64Value(int64(*burnAlert.ExhaustionMinutes)) + } + if burnAlert.BudgetRateDecreaseThresholdPerMillion != nil { + // Must convert from PPM back to float to match what the user has in their config + budgetRateDecreaseThresholdPerMillionAsPercent := helper.PPMToFloat(*burnAlert.BudgetRateDecreaseThresholdPerMillion) + state.BudgetRateDecreasePercent = types.Float64Value(budgetRateDecreaseThresholdPerMillionAsPercent) + } + if burnAlert.BudgetRateWindowMinutes != nil { + state.BudgetRateWindowMinutes = types.Int64Value(int64(*burnAlert.BudgetRateWindowMinutes)) + } + // Set the updated burn alert's attributes in state resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } func (r *burnAlertResource) Delete(ctx context.Context, req resource.DeleteRequest, resp *resource.DeleteResponse) { var state models.BurnAlertResourceModel + // Read in the state data resp.Diagnostics.Append(req.State.Get(ctx, &state)...) if resp.Diagnostics.HasError() { return } + // Delete the burn alert, using the values from state err := r.client.BurnAlerts.Delete(ctx, state.Dataset.ValueString(), state.ID.ValueString()) if helper.AddDiagnosticOnError(&resp.Diagnostics, "Deleting Honeycomb Burn Alert", err) { return diff --git a/internal/provider/burn_alert_resource_test.go b/internal/provider/burn_alert_resource_test.go index 313b26cd..d860bbb4 100644 --- a/internal/provider/burn_alert_resource_test.go +++ b/internal/provider/burn_alert_resource_test.go @@ -9,36 +9,194 @@ import ( "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/plancheck" "github.com/hashicorp/terraform-plugin-testing/terraform" + "github.com/honeycombio/terraform-provider-honeycombio/internal/helper" "github.com/stretchr/testify/require" "github.com/honeycombio/terraform-provider-honeycombio/client" ) -func TestAcc_BurnAlertResource(t *testing.T) { +func TestAcc_BurnAlertResource_defaultBasic(t *testing.T) { dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} + + // Create + exhaustionMinutes := 240 + + // Update + updatedExhaustionMinutes := 480 + budgetRateWindowMinutes := 60 + budgetRateDecreasePercent := 0.0001 resource.Test(t, resource.TestCase{ PreCheck: testAccPreCheck(t), ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), Steps: []resource.TestStep{ + // Create - basic { - Config: testAccConfigBasicBurnAlertTest(dataset, sloID, "info"), - Check: resource.ComposeAggregateTestCheckFunc( - testAccEnsureBurnAlertExists(t, "honeycombio_burn_alert.test"), - resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "slo_id", sloID), - resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "exhaustion_minutes", "240"), - resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.#", "1"), - ), + Config: testAccConfigBurnAlertDefault_basic(exhaustionMinutes, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionMinutes, "info", sloID), }, - // then update the PD Severity from info -> critical (the default) + // Update - PD Severity from info -> critical (the default) { - Config: testAccConfigBasicBurnAlertTest(dataset, sloID, "critical"), + Config: testAccConfigBurnAlertDefault_basic(exhaustionMinutes, dataset, sloID, "critical"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionMinutes, "critical", sloID), }, + // Import { - ResourceName: "honeycombio_burn_alert.test", - ImportStateIdPrefix: fmt.Sprintf("%v/", dataset), - ImportState: true, + ResourceName: "honeycombio_burn_alert.test", + ImportStateIdPrefix: fmt.Sprintf("%v/", dataset), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"recipient"}, + }, + // Update - exhaustion time to exhaustion time + { + Config: testAccConfigBurnAlertDefault_basic(updatedExhaustionMinutes, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, updatedExhaustionMinutes, "info", sloID), + }, + // Update - exhaustion time to budget rate + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, budgetRateDecreasePercent, dataset, sloID, "info"), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, "info", sloID), + }, + }, + }) +} + +func TestAcc_BurnAlertResource_exhaustionTimeBasic(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} + + // Create + exhaustionMinutes := 0 + + // Update + updatedExhaustionMinutes := 240 + budgetRateWindowMinutes := 60 + budgetRateDecreasePercent := 5.1 + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + // Create - basic + { + Config: testAccConfigBurnAlertDefault_basic(exhaustionMinutes, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionMinutes, "info", sloID), + }, + // Update - PD Severity from info -> critical (the default) + { + Config: testAccConfigBurnAlertDefault_basic(exhaustionMinutes, dataset, sloID, "critical"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionMinutes, "critical", sloID), + }, + // Import + { + ResourceName: "honeycombio_burn_alert.test", + ImportStateIdPrefix: fmt.Sprintf("%v/", dataset), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"recipient"}, + }, + // Update - exhaustion time to exhaustion time + { + Config: testAccConfigBurnAlertDefault_basic(updatedExhaustionMinutes, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, updatedExhaustionMinutes, "info", sloID), + }, + // Update - exhaustion time to budget rate + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, budgetRateDecreasePercent, dataset, sloID, "info"), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, "info", sloID), + }, + }, + }) +} + +func TestAcc_BurnAlertResource_budgetRateBasic(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} + + // Create + budgetRateWindowMinutes := 60 + budgetRateDecreasePercent := float64(5) + + // Update + updatedBudgetRateWindowMinutes := 100 + updatedBudgetRateDecreasePercent := 0.1 + exhaustionTime := 0 + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + // Create - basic + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, budgetRateDecreasePercent, dataset, sloID, "info"), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, "info", sloID), + }, + // Update - PD Severity from info -> critical (the default) + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, budgetRateDecreasePercent, dataset, sloID, "critical"), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, "critical", sloID), + }, + // Import + { + ResourceName: "honeycombio_burn_alert.test", + ImportStateIdPrefix: fmt.Sprintf("%v/", dataset), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"recipient"}, + }, + // Update - budget rate to budget rate + { + Config: testAccConfigBurnAlertBudgetRate_basic(updatedBudgetRateWindowMinutes, updatedBudgetRateDecreasePercent, dataset, sloID, "info"), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, updatedBudgetRateWindowMinutes, updatedBudgetRateDecreasePercent, "info", sloID), + }, + // Update - budget rate to exhaustion time + { + Config: testAccConfigBurnAlertExhaustionTime_basic(exhaustionTime, dataset, sloID, "info"), + Check: testAccEnsureSuccessExhaustionTimeAlert(t, burnAlert, exhaustionTime, "info", sloID), + }, + }, + }) +} + +// Check that creating a budget rate alert with a +// budget_rate_decrease_percent with trailing zeros works, +// doesn't produce spurious plans after apply, and imports successfully +func TestAcc_BurnAlertResource_budgetRateTrailingZeros(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} + + // Create + budgetRateWindowMinutes := 60 + budgetRateDecreasePercent := float64(5.00000) + pdSeverity := "info" + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + // Create - trailing zeros on budget_rate_decrease_percent + { + Config: testAccConfigBurnAlertBudgetRate_trailingZeros(dataset, sloID), + Check: testAccEnsureSuccessBudgetRateAlert(t, burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, pdSeverity, sloID), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()}, + }, + }, + // Import + { + ResourceName: "honeycombio_burn_alert.test", + ImportStateIdPrefix: fmt.Sprintf("%v/", dataset), + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{"recipient"}, }, }, }) @@ -51,8 +209,9 @@ func TestAcc_BurnAlertResource(t *testing.T) { // See: https://developer.hashicorp.com/terraform/plugin/framework/migrating/testing#testing-migration func TestAcc_BurnAlertResourceUpgradeFromVersion015(t *testing.T) { dataset, sloID := burnAlertAccTestSetup(t) + burnAlert := &client.BurnAlert{} - config := testAccConfigBasicBurnAlertTest(dataset, sloID, "info") + config := testAccConfigBurnAlertDefault_basic(60, dataset, sloID, "info") resource.Test(t, resource.TestCase{ Steps: []resource.TestStep{ @@ -65,7 +224,7 @@ func TestAcc_BurnAlertResourceUpgradeFromVersion015(t *testing.T) { }, Config: config, Check: resource.ComposeTestCheckFunc( - testAccEnsureBurnAlertExists(t, "honeycombio_burn_alert.test"), + testAccEnsureBurnAlertExists(t, "honeycombio_burn_alert.test", burnAlert), ), // These tests pull in older versions of the provider that don't // support setting the API host easily. We'll skip them for now @@ -113,7 +272,51 @@ func TestAcc_BurnAlertResource_Import_validateImportID(t *testing.T) { }) } -func testAccEnsureBurnAlertExists(t *testing.T, name string) resource.TestCheckFunc { +// 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( + // Check that the burn alert exists + testAccEnsureBurnAlertExists(t, "honeycombio_burn_alert.test", burnAlert), + + // Check that the burn alert has the correct attributes + testAccEnsureAttributesCorrectExhaustionTime(burnAlert, exhaustionMinutes, sloID), + + // Check that the burn alert has the correct values in state + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "slo_id", sloID), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "alert_type", "exhaustion_time"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "exhaustion_minutes", fmt.Sprintf("%d", exhaustionMinutes)), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.#", "1"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.0.notification_details.#", "1"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.0.notification_details.0.pagerduty_severity", pagerdutySeverity), + // Budget rate attributes should not be set + resource.TestCheckNoResourceAttr("honeycombio_burn_alert.test", "budget_rate_window_minutes"), + resource.TestCheckNoResourceAttr("honeycombio_burn_alert.test", "budget_rate_decrease_percent"), + ) +} + +// Checks that the budget rate burn alert exists, has the correct attributes, and has the correct state +func testAccEnsureSuccessBudgetRateAlert(t *testing.T, burnAlert *client.BurnAlert, budgetRateWindowMinutes int, budgetRateDecreasePercent float64, pagerdutySeverity, sloID string) resource.TestCheckFunc { + return resource.ComposeAggregateTestCheckFunc( + // Check that the burn alert exists + testAccEnsureBurnAlertExists(t, "honeycombio_burn_alert.test", burnAlert), + + // Check that the burn alert has the correct attributes + testAccEnsureAttributesCorrectBudgetRate(burnAlert, budgetRateWindowMinutes, budgetRateDecreasePercent, sloID), + + // Check that the burn alert has the correct values in state + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "slo_id", sloID), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "alert_type", "budget_rate"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "budget_rate_window_minutes", fmt.Sprintf("%d", budgetRateWindowMinutes)), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "budget_rate_decrease_percent", helper.FloatToPercentString(budgetRateDecreasePercent)), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.#", "1"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.0.notification_details.#", "1"), + resource.TestCheckResourceAttr("honeycombio_burn_alert.test", "recipient.0.notification_details.0.pagerduty_severity", pagerdutySeverity), + // Exhaustion time attributes should not be set + resource.TestCheckNoResourceAttr("honeycombio_burn_alert.test", "exhaustion_minutes"), + ) +} + +func testAccEnsureBurnAlertExists(t *testing.T, name string, burnAlert *client.BurnAlert) resource.TestCheckFunc { return func(s *terraform.State) error { resourceState, ok := s.RootModule().Resources[name] if !ok { @@ -121,11 +324,90 @@ func testAccEnsureBurnAlertExists(t *testing.T, name string) resource.TestCheckF } client := testAccClient(t) - _, err := client.BurnAlerts.Get(context.Background(), resourceState.Primary.Attributes["dataset"], resourceState.Primary.ID) + alert, err := client.BurnAlerts.Get(context.Background(), resourceState.Primary.Attributes["dataset"], resourceState.Primary.ID) if err != nil { return fmt.Errorf("failed to fetch created Burn Alert: %w", err) } + *burnAlert = *alert + + return nil + } +} + +func testAccEnsureAttributesCorrectExhaustionTime(burnAlert *client.BurnAlert, exhaustionMinutes int, sloID string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if burnAlert.AlertType != "exhaustion_time" { + return fmt.Errorf("incorrect alert_type: %s", burnAlert.AlertType) + } + + if burnAlert.ExhaustionMinutes == nil { + return fmt.Errorf("incorrect exhaustion_minutes: expected not to be nil") + } + if *burnAlert.ExhaustionMinutes != exhaustionMinutes { + return fmt.Errorf("incorrect exhaustion_minutes: %d", *burnAlert.ExhaustionMinutes) + } + + if burnAlert.SLO.ID != sloID { + return fmt.Errorf("incorrect SLO ID: %s", burnAlert.SLO.ID) + } + + // TODO: more in-depth checking of recipients + + return nil + } +} + +func testAccEnsureAttributesCorrectBudgetRate(burnAlert *client.BurnAlert, budgetRateWindowMinutes int, budgetRateDecreasePercent float64, sloID string) resource.TestCheckFunc { + return func(s *terraform.State) error { + if burnAlert.AlertType != "budget_rate" { + return fmt.Errorf("incorrect alert_type: %s", burnAlert.AlertType) + } + + if burnAlert.BudgetRateWindowMinutes == nil { + return fmt.Errorf("incorrect budget_rate_window_minutes: expected not to be nil") + } + if *burnAlert.BudgetRateWindowMinutes != budgetRateWindowMinutes { + return fmt.Errorf("incorrect budget_rate_window_minutes: %d", *burnAlert.BudgetRateWindowMinutes) + } + + if burnAlert.BudgetRateDecreaseThresholdPerMillion == nil { + return fmt.Errorf("incorrect budget_rate_decrease_percent: expected not to be nil") + } + // Must convert from PPM back to float to compare with config + budgetRateDecreaseThresholdPerMillionAsPercent := helper.PPMToFloat(*burnAlert.BudgetRateDecreaseThresholdPerMillion) + if budgetRateDecreaseThresholdPerMillionAsPercent != budgetRateDecreasePercent { + return fmt.Errorf("incorrect budget_rate_decrease_percent: %f", budgetRateDecreaseThresholdPerMillionAsPercent) + } + + if burnAlert.SLO.ID != sloID { + return fmt.Errorf("incorrect SLO ID: %s", burnAlert.SLO.ID) + } + + // TODO: more in-depth checking of recipients + + return nil + } +} + +func testAccEnsureBurnAlertDestroyed(t *testing.T) resource.TestCheckFunc { + return func(s *terraform.State) error { + for _, resourceState := range s.RootModule().Resources { + if resourceState.Type != "honeycomb_burn_alert" { + continue + } + + if resourceState.Primary.ID == "" { + return fmt.Errorf("no ID set for burn alert") + } + + client := testAccClient(t) + _, err := client.BurnAlerts.Get(context.Background(), resourceState.Primary.Attributes["dataset"], resourceState.Primary.ID) + if err == nil { + return fmt.Errorf("burn alert %s was not deleted on destroy", resourceState.Primary.ID) + } + } + return nil } } @@ -161,7 +443,7 @@ func burnAlertAccTestSetup(t *testing.T) (string, string) { return dataset, slo.ID } -func testAccConfigBasicBurnAlertTest(dataset, sloID, pdseverity string) string { +func testAccConfigBurnAlertDefault_basic(exhaustionMinutes int, dataset, sloID, pdseverity string) string { return fmt.Sprintf(` resource "honeycombio_pagerduty_recipient" "test" { integration_key = "08b9d4cacd68933151a1ef1028b67da2" @@ -169,16 +451,91 @@ resource "honeycombio_pagerduty_recipient" "test" { } resource "honeycombio_burn_alert" "test" { - dataset = "%[1]s" - slo_id = "%[2]s" - exhaustion_minutes = 4 * 60 + exhaustion_minutes = %[1]d + + dataset = "%[2]s" + slo_id = "%[3]s" + + recipient { + id = honeycombio_pagerduty_recipient.test.id + + notification_details { + pagerduty_severity = "%[4]s" + } + } +}`, exhaustionMinutes, dataset, sloID, pdseverity) +} + +func testAccConfigBurnAlertExhaustionTime_basic(exhaustionMinutes int, dataset, sloID, pdseverity string) string { + return fmt.Sprintf(` +resource "honeycombio_pagerduty_recipient" "test" { + integration_key = "08b9d4cacd68933151a1ef1028b67da2" + integration_name = "testacc-basic" +} + +resource "honeycombio_burn_alert" "test" { + alert_type = "exhaustion_time" + exhaustion_minutes = %[1]d + + dataset = "%[2]s" + slo_id = "%[3]s" + + recipient { + id = honeycombio_pagerduty_recipient.test.id + + notification_details { + pagerduty_severity = "%[4]s" + } + } +}`, exhaustionMinutes, dataset, sloID, pdseverity) +} + +func testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes int, budgetRateDecreasePercent float64, dataset, sloID, pdseverity string) string { + return fmt.Sprintf(` +resource "honeycombio_pagerduty_recipient" "test" { + integration_key = "08b9d4cacd68933151a1ef1028b67da2" + integration_name = "testacc-basic" +} + +resource "honeycombio_burn_alert" "test" { + alert_type = "budget_rate" + budget_rate_window_minutes = %[1]d + budget_rate_decrease_percent = %[2]s + + dataset = "%[3]s" + slo_id = "%[4]s" + + recipient { + id = honeycombio_pagerduty_recipient.test.id + + notification_details { + pagerduty_severity = "%[5]s" + } + } +}`, budgetRateWindowMinutes, helper.FloatToPercentString(budgetRateDecreasePercent), dataset, sloID, pdseverity) +} + +func testAccConfigBurnAlertBudgetRate_trailingZeros(dataset, sloID string) string { + return fmt.Sprintf(` +resource "honeycombio_pagerduty_recipient" "test" { + integration_key = "08b9d4cacd68933151a1ef1028b67da2" + integration_name = "testacc-basic" +} + +resource "honeycombio_burn_alert" "test" { + alert_type = "budget_rate" + budget_rate_window_minutes = 60 + budget_rate_decrease_percent = 5.00000 + + dataset = "%[1]s" + slo_id = "%[2]s" recipient { id = honeycombio_pagerduty_recipient.test.id notification_details { - pagerduty_severity = "%[3]s" + pagerduty_severity = "info" } } -}`, dataset, sloID, pdseverity) +}`, dataset, sloID) } From f6ebbc74b4105a59974f08fc01023d7d5307d60c Mon Sep 17 00:00:00 2001 From: Krista LaFentres Date: Wed, 15 Nov 2023 11:15:33 -0600 Subject: [PATCH 11/11] r/burn_alert: Schema validation tests --- internal/provider/burn_alert_resource_test.go | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/internal/provider/burn_alert_resource_test.go b/internal/provider/burn_alert_resource_test.go index d860bbb4..aafc3caa 100644 --- a/internal/provider/burn_alert_resource_test.go +++ b/internal/provider/burn_alert_resource_test.go @@ -272,6 +272,105 @@ func TestAcc_BurnAlertResource_Import_validateImportID(t *testing.T) { }) } +func TestAcc_BurnAlertResource_validateDefault(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + { + Config: testAccConfigBurnAlertDefault_basic(-1, dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`exhaustion_minutes value must be at least`), + }, + { + Config: testAccConfigBurnAlertDefault_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`argument "exhaustion_minutes" is required`), + }, + { + Config: testAccConfigBurnAlertDefault_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`"budget_rate_window_minutes": must not be configured when "alert_type"`), + }, + { + Config: testAccConfigBurnAlertDefault_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`"budget_rate_decrease_percent": must not be configured when`), + }, + }, + }) +} + +func TestAcc_BurnAlertResource_validateExhaustionTime(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + { + Config: testAccConfigBurnAlertExhaustionTime_basic(-1, dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`exhaustion_minutes value must be at least`), + }, + { + Config: testAccConfigBurnAlertExhaustionTime_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`argument "exhaustion_minutes" is required`), + }, + { + Config: testAccConfigBurnAlertExhaustionTime_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`"budget_rate_window_minutes": must not be configured when "alert_type"`), + }, + { + Config: testAccConfigBurnAlertExhaustionTime_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID), + ExpectError: regexp.MustCompile(`"budget_rate_decrease_percent": must not be configured when`), + }, + }, + }) +} + +func TestAcc_BurnAlertResource_validateBudgetRate(t *testing.T) { + dataset, sloID := burnAlertAccTestSetup(t) + + budgetRateWindowMinutes := 60 + budgetRateDecreasePercent := float64(1) + + resource.Test(t, resource.TestCase{ + PreCheck: testAccPreCheck(t), + ProtoV5ProviderFactories: testAccProtoV5MuxServerFactory, + CheckDestroy: testAccEnsureBurnAlertDestroyed(t), + Steps: []resource.TestStep{ + { + Config: testAccConfigBurnAlertBudgetRate_basic(0, budgetRateDecreasePercent, dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`budget_rate_window_minutes value must be at least`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, float64(0), dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`budget_rate_decrease_percent value must be at least`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, float64(200), dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`budget_rate_decrease_percent value must be at most`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes, float64(1.123456789), dataset, sloID, "info"), + ExpectError: regexp.MustCompile(`budget_rate_decrease_percent precision for value must be at most`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_validateAttributesWhenAlertTypeIsBudgetRate(dataset, sloID), + ExpectError: regexp.MustCompile(`argument "budget_rate_decrease_percent" is required`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_validateAttributesWhenAlertTypeIsBudgetRate(dataset, sloID), + ExpectError: regexp.MustCompile(`argument "budget_rate_window_minutes" is required`), + }, + { + Config: testAccConfigBurnAlertBudgetRate_validateAttributesWhenAlertTypeIsBudgetRate(dataset, sloID), + ExpectError: regexp.MustCompile(`"exhaustion_minutes": must not be configured when "alert_type"`), + }, + }, + }) +} + // 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( @@ -466,6 +565,22 @@ resource "honeycombio_burn_alert" "test" { }`, exhaustionMinutes, dataset, sloID, pdseverity) } +func testAccConfigBurnAlertDefault_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID string) string { + return fmt.Sprintf(` +resource "honeycombio_burn_alert" "test" { + budget_rate_window_minutes = 60 + budget_rate_decrease_percent = 1 + + dataset = "%[1]s" + slo_id = "%[2]s" + + recipient { + type = "email" + target = "test@example.com" + } +}`, dataset, sloID) +} + func testAccConfigBurnAlertExhaustionTime_basic(exhaustionMinutes int, dataset, sloID, pdseverity string) string { return fmt.Sprintf(` resource "honeycombio_pagerduty_recipient" "test" { @@ -490,6 +605,23 @@ resource "honeycombio_burn_alert" "test" { }`, exhaustionMinutes, dataset, sloID, pdseverity) } +func testAccConfigBurnAlertExhaustionTime_validateAttributesWhenAlertTypeIsExhaustionTime(dataset, sloID string) string { + return fmt.Sprintf(` +resource "honeycombio_burn_alert" "test" { + alert_type = "exhaustion_time" + budget_rate_window_minutes = 60 + budget_rate_decrease_percent = 1 + + dataset = "%[1]s" + slo_id = "%[2]s" + + recipient { + type = "email" + target = "test@example.com" + } +}`, dataset, sloID) +} + func testAccConfigBurnAlertBudgetRate_basic(budgetRateWindowMinutes int, budgetRateDecreasePercent float64, dataset, sloID, pdseverity string) string { return fmt.Sprintf(` resource "honeycombio_pagerduty_recipient" "test" { @@ -539,3 +671,19 @@ resource "honeycombio_burn_alert" "test" { } }`, dataset, sloID) } + +func testAccConfigBurnAlertBudgetRate_validateAttributesWhenAlertTypeIsBudgetRate(dataset, sloID string) string { + return fmt.Sprintf(` +resource "honeycombio_burn_alert" "test" { + alert_type = "budget_rate" + exhaustion_minutes = 60 + + dataset = "%[1]s" + slo_id = "%[2]s" + + recipient { + type = "email" + target = "test@example.com" + } +}`, dataset, sloID) +}