Skip to content

Commit

Permalink
consumption - remove 4.0 flag and fix tests (#27511)
Browse files Browse the repository at this point in the history
* fix comsumption issue

* remove forcenew for threshold_type

* terrafmt

* link issue

* skip delete error on 404

* remove linked issue

* remove _disappear test and revert skipping 404 in deleting
  • Loading branch information
teowa authored Oct 24, 2024
1 parent 2a35668 commit c70167d
Show file tree
Hide file tree
Showing 6 changed files with 9 additions and 155 deletions.
119 changes: 2 additions & 117 deletions internal/services/consumption/consumption_budget_base.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/hashicorp/go-azure-helpers/lang/response"
"github.com/hashicorp/go-azure-sdk/resource-manager/consumption/2019-10-01/budgets"
"github.com/hashicorp/terraform-provider-azurerm/helpers/tf"
"github.com/hashicorp/terraform-provider-azurerm/internal/features"
"github.com/hashicorp/terraform-provider-azurerm/internal/sdk"
"github.com/hashicorp/terraform-provider-azurerm/internal/services/consumption/validate"
"github.com/hashicorp/terraform-provider-azurerm/internal/tf/pluginsdk"
Expand Down Expand Up @@ -152,14 +151,10 @@ func (br consumptionBudgetBaseResource) arguments(fields map[string]*pluginsdk.S
Required: true,
ValidateFunc: validation.IntBetween(0, 1000),
},
// Issue: https://github.com/Azure/azure-rest-api-specs/issues/16240
// Toggling between these two values doesn't work at the moment and also doesn't throw an error
// but it seems unlikely that a user would switch the threshold_type of their budgets frequently
"threshold_type": {
Type: pluginsdk.TypeString,
Optional: true,
Default: string(budgets.ThresholdTypeActual),
ForceNew: true, // TODO: remove this when the above issue is fixed
ValidateFunc: validation.StringInSlice([]string{
string(budgets.ThresholdTypeActual),
"Forecasted",
Expand Down Expand Up @@ -244,82 +239,6 @@ func (br consumptionBudgetBaseResource) arguments(fields map[string]*pluginsdk.S
},
}

if !features.FourPointOhBeta() {
output["filter"].Elem.(*pluginsdk.Resource).Schema["not"] = &pluginsdk.Schema{
Type: pluginsdk.TypeList,
Optional: true,
MaxItems: 1,
Deprecated: "This property has been deprecated as the API no longer supports it and will be removed in version 4.0 of the provider.",
AtLeastOneOf: []string{"filter.0.dimension", "filter.0.tag", "filter.0.not"},
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"dimension": {
Type: pluginsdk.TypeList,
MaxItems: 1,
Optional: true,
ExactlyOneOf: []string{"filter.0.not.0.tag"},
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
ValidateFunc: validation.StringInSlice(getDimensionNames(), false),
},
"operator": {
Type: pluginsdk.TypeString,
Optional: true,
Default: "In",
ValidateFunc: validation.StringInSlice([]string{
"In",
}, false),
},
"values": {
Type: pluginsdk.TypeList,
MinItems: 1,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},
},
},
},
"tag": {
Type: pluginsdk.TypeList,
MaxItems: 1,
Optional: true,
ExactlyOneOf: []string{"filter.0.not.0.dimension"},
Elem: &pluginsdk.Resource{
Schema: map[string]*pluginsdk.Schema{
"name": {
Type: pluginsdk.TypeString,
Required: true,
},
"operator": {
Type: pluginsdk.TypeString,
Optional: true,
Default: "In",
ValidateFunc: validation.StringInSlice([]string{
"In",
}, false),
},
"values": {
Type: pluginsdk.TypeList,
Required: true,
Elem: &pluginsdk.Schema{
Type: pluginsdk.TypeString,
ValidateFunc: validation.StringIsNotEmpty,
},
},
},
},
},
},
},
}
}

// Consumption Budgets for Management Groups have a different notification schema,
// here we override the notification schema in the base resource
for k, v := range fields {
Expand Down Expand Up @@ -384,7 +303,7 @@ func (br consumptionBudgetBaseResource) readFunc(scopeFieldName string) sdk.Reso
}

metadata.ResourceData.Set("name", id.BudgetName)
//lintignore:R001
// lintignore:R001
metadata.ResourceData.Set(scopeFieldName, id.Scope)

if model := resp.Model; model != nil {
Expand Down Expand Up @@ -418,7 +337,7 @@ func (br consumptionBudgetBaseResource) deleteFunc() sdk.ResourceFunc {
return err
}

if _, err = client.Delete(ctx, *id); err != nil {
if _, err := client.Delete(ctx, *id); err != nil {
return fmt.Errorf("deleting %s: %+v", *id, err)
}

Expand Down Expand Up @@ -700,22 +619,6 @@ func expandConsumptionBudgetFilter(i []interface{}) *budgets.BudgetFilter {

filter := budgets.BudgetFilter{}

if !features.FourPointOhBeta() {
notBlock := input["not"].([]interface{})
if len(notBlock) != 0 && notBlock[0] != nil {
not := notBlock[0].(map[string]interface{})

tags := expandConsumptionBudgetFilterTag(not["tag"].([]interface{}))
dimensions := expandConsumptionBudgetFilterDimensions(not["dimension"].([]interface{}))

if len(dimensions) != 0 {
filter.Not = &dimensions[0]
} else if len(tags) != 0 {
filter.Not = &tags[0]
}
}
}

tags := expandConsumptionBudgetFilterTag(input["tag"].(*pluginsdk.Set).List())
dimensions := expandConsumptionBudgetFilterDimensions(input["dimension"].(*pluginsdk.Set).List())

Expand Down Expand Up @@ -756,24 +659,6 @@ func flattenConsumptionBudgetFilter(input *budgets.BudgetFilter) []interface{} {

filterBlock := make(map[string]interface{})

if !features.FourPointOhBeta() {
notBlock := make(map[string]interface{})

if input.Not != nil {
if input.Not.Dimensions != nil {
notBlock["dimension"] = []interface{}{flattenConsumptionBudgetComparisonExpression(input.Not.Dimensions)}
}

if input.Not.Tags != nil {
notBlock["tag"] = []interface{}{flattenConsumptionBudgetComparisonExpression(input.Not.Tags)}
}

if len(notBlock) != 0 {
filterBlock["not"] = []interface{}{notBlock}
}
}
}

if input.And != nil {
for _, v := range *input.And {
if v.Dimensions != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,10 @@ func (r ManagementGroupConsumptionBudget) Arguments() map[string]*pluginsdk.Sche
Required: true,
ValidateFunc: validation.IntBetween(0, 1000),
},
// Issue: https://github.com/Azure/azure-rest-api-specs/issues/16240
// Toggling between these two values doesn't work at the moment and also doesn't throw an error
// but it seems unlikely that a user would switch the threshold_type of their budgets frequently
"threshold_type": {
Type: pluginsdk.TypeString,
Optional: true,
Default: string(budgets.ThresholdTypeActual),
ForceNew: true, // TODO: remove this when the above issue is fixed
ValidateFunc: validation.StringInSlice([]string{
string(budgets.ThresholdTypeActual),
"Forecasted",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,6 @@ func TestAccConsumptionBudgetResourceGroup_completeUpdate(t *testing.T) {
})
}

func TestAccConsumptionBudgetResourceGroup_disappears(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_consumption_budget_resource_group", "test")
r := ConsumptionBudgetResourceGroupResource{}

data.ResourceTest(t, r, []acceptance.TestStep{
data.DisappearsStep(acceptance.DisappearsStepData{
Config: r.basic,
TestResource: r,
}),
})
}

func (ConsumptionBudgetResourceGroupResource) Exists(ctx context.Context, clients *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
id, err := budgets.ParseScopedBudgetID(state.ID)
if err != nil {
Expand Down Expand Up @@ -404,12 +392,10 @@ resource "azurerm_consumption_budget_resource_group" "test" {
}
notification {
enabled = true
threshold = 90.0
operator = "EqualTo"
// We don't update the value of threshold_type because toggling between the two seems to be broken
// See the comment on threshold_type in the schema for more details
threshold_type = "Forecasted"
enabled = true
threshold = 90.0
operator = "EqualTo"
threshold_type = "Actual"
contact_emails = [
"[email protected]",
Expand Down Expand Up @@ -439,16 +425,3 @@ resource "azurerm_consumption_budget_resource_group" "test" {
}
`, data.RandomInteger, data.Locations.Primary, data.RandomInteger, data.RandomInteger, consumptionBudgetTestStartDate().Format(time.RFC3339))
}

func (t ConsumptionBudgetResourceGroupResource) Destroy(ctx context.Context, client *clients.Client, state *pluginsdk.InstanceState) (*bool, error) {
id, err := budgets.ParseScopedBudgetID(state.ID)
if err != nil {
return nil, err
}

if _, err = client.Consumption.BudgetsClient.Delete(ctx, *id); err != nil {
return nil, fmt.Errorf("deleting %s: %+v", *id, err)
}

return utils.Bool(true), nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ A `notification` block supports the following:

* `contact_emails` - (Required) Specifies a list of email addresses to send the budget notification to when the threshold is exceeded.

* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`. Changing this forces a new resource to be created.
* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`.

* `enabled` - (Optional) Should the notification be enabled? Defaults to `true`.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ A `notification` block supports the following:

* `threshold` - (Required) Threshold value associated with a notification. Notification is sent when the cost exceeded the threshold. It is always percent and has to be between 0 and 1000.

* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`. Changing this forces a new resource to be created.
* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`.

* `contact_emails` - (Optional) Specifies a list of email addresses to send the budget notification to when the threshold is exceeded.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ A `notification` block supports the following:

* `threshold` - (Required) Threshold value associated with a notification. Notification is sent when the cost exceeded the threshold. It is always percent and has to be between 0 and 1000.

* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`. Changing this forces a new resource to be created.
* `threshold_type` - (Optional) The type of threshold for the notification. This determines whether the notification is triggered by forecasted costs or actual costs. The allowed values are `Actual` and `Forecasted`. Default is `Actual`.

* `contact_emails` - (Optional) Specifies a list of email addresses to send the budget notification to when the threshold is exceeded.

Expand Down

0 comments on commit c70167d

Please sign in to comment.