New Resource: azurerm_monitor_alert_processing_rule_action_group, azurerm_monitor_alert_processing_rule_suppression

[teowa]

fix #16272, fix #17678

1. Add a new resource alert_processing_rule_action_group and alert_processing_rule_suppression due to REST API breaking changes. Please refer to: service doc and REST API specs.
2. Older version of this resource is implemented in two parts: azurerm_monitor_action_rule_action_group ,azurerm_monitor_action_rule_suppression. Please refer to azurerm_monitor_action_rule_action_group TF doc, azurerm_monitor_action_rule_suppression TF doc, and REST API specs. Due to schema breaking change (e.g. scope.type is removed, schedule time format change and support more than one recurrence) , developing a new resource is required.

test

> TF_ACC=1 go test -v ./internal/services/monitor -run=TestAccMonitorAlertProcessingRule_ -timeout 60m -ldflags="-X=github.com/hashicorp/terraform-provider-azurerm/version.ProviderVersion=acc"
=== RUN   TestAccMonitorAlertProcessingRule_basic
=== PAUSE TestAccMonitorAlertProcessingRule_basic
=== RUN   TestAccMonitorAlertProcessingRule_requiresImport
=== PAUSE TestAccMonitorAlertProcessingRule_requiresImport
=== RUN   TestAccMonitorAlertProcessingRule_complete
=== PAUSE TestAccMonitorAlertProcessingRule_complete
=== RUN   TestAccMonitorAlertProcessingRule_update
=== PAUSE TestAccMonitorAlertProcessingRule_update
=== CONT  TestAccMonitorAlertProcessingRule_basic
=== CONT  TestAccMonitorAlertProcessingRule_update
=== CONT  TestAccMonitorAlertProcessingRule_complete
=== CONT  TestAccMonitorAlertProcessingRule_requiresImport
--- PASS: TestAccMonitorAlertProcessingRule_basic (240.84s)
--- PASS: TestAccMonitorAlertProcessingRule_complete (241.75s)
--- PASS: TestAccMonitorAlertProcessingRule_requiresImport (262.58s)
--- PASS: TestAccMonitorAlertProcessingRule_update (506.86s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/monitor       506.899s

[teowa]

have changed some code and run a new test:

--- PASS: TestAccMonitorAlertProcessingRule_basic (220.94s)
--- PASS: TestAccMonitorAlertProcessingRule_complete (223.45s)
--- PASS: TestAccMonitorAlertProcessingRule_requiresImport (229.45s)
--- PASS: TestAccMonitorAlertProcessingRule_update (1425.12s)
PASS
ok      github.com/hashicorp/terraform-provider-azurerm/internal/services/monitor       1425.203s

[dkuzmenok]

@katbyte
Would be great to merge this one. Existing azurerm_monitor_action_rule_suppression does not allow to properly manage recurrent suppressions (which is frequently used in enterprise monitoring...).

[katbyte]

Can you explain why you feel the resources need to be replaced rather then updated due to these changes?

[teowa]

Can you explain why you feel the resources need to be replaced rather then updated due to these changes?

If to update existing azurerm_monitor_action_rule_action_group and azurerm_monitor_action_rule_suppression, there will be breaking changes:

1. Required parameter scope.type is deprecated.
2. Required parameter suppression.recurrence_type is deprecated.
3. start_date_utc (e.g., "2019-01-01T01:02:03Z") splits into datetime (e.g., "2019-01-01T01:02:03") and time_zone (e.g., "Pacific Standard Time"), see this REST API example.
4. recurrence_weekly is changed from list type recurrence_weekly = ["Sunday", "Monday", "Friday", "Saturday"] to one or more object block ,see this REST API example, the HCL should be like below, the same with recurrence_monthly.

weekly {
  days_of_week = ["Sunday", "Saturday"]
}
weekly {
  start_time   = "17:00:00"
  end_time     = "18:00:00"
  days_of_week = ["Monday"]
}

[dkuzmenok]

@katbyte @teowa My few cents on this one.

Previous API version was not fully usable and had issues with usage scenarios, that is why it was reworked.
So if we leave two resources - then we would have 2 resources for old and new API.

I would prefer one of 2 ways:

1. Deprecate existing resource and have a new resource for new API.
2. Replace existing resource with a new API implementation.

[katbyte]

@teowa - how is 1 resource replace two?

[teowa]

@teowa - how is 1 resource replace two?

In action block schema, action.type="AddActionGroups" corresponds to the old azurerm_monitor_action_rule_action_group, otherwise action.type="RemoveAllActionGroups" corresponds to the old azurerm_monitor_action_rule_suppression.

In current one, the only diff is that if set action.type="AddActionGroups" then action.add_action_group_ids must be set ( but in old ones, azurerm_monitor_action_rule_action_group does not support schedule with REST API 2019-05-05-preview, now the action.type="AddActionGroups" also support schedule with REST API 2021-08-08). And for consistency, I am willing to split the new resource into two.

[tunglvNEV]

@katbyte @teowa I am waiting for update new resource: azurerm_monitor_alert_processing_rule. Please !

[teowa]

@teowa - how is 1 resource replace two?

@katbyte In new API version, the properties of two resource are almost the same except one need action group ids. So I think it is better to deprecate the old two resources and use the new one instead.

In Portal:

1. add action group

2. suppress

[katbyte]

That may be the case but as per the discussion in #6563 these were split out for clarity of what the resource was doing. I don't see any reason to change that now. How about we refine the name of the resources as the old ones seem.. inaccurate and then we can safely deprecate the old ones and not worry about breaking changes. perhaps azurerm_monitor_alert_rule_apply_action_group and azurerm_monitor_alert_rule_suppression ?

WDYT?

[teowa]

That may be the case but as per the discussion in #6563 these were split out for clarity of what the resource was doing. I don't see any reason to change that now. How about we refine the name of the resources as the old ones seem.. inaccurate and then we can safely deprecate the old ones and not worry about breaking changes. perhaps azurerm_monitor_alert_rule_apply_action_group and azurerm_monitor_alert_rule_suppression ?

WDYT?

Make sense. I will refine the code.

[teowa]

[katbyte]

LGTM 🌩️

[github-actions]

This functionality has been released in v3.24.0 of the Terraform Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.