New Resource: azurerm_monitor_action_group
#1714
Conversation
tombuildsstuff
changed the title
azurerm_monitor_action_group
There was a problem hiding this comment.
hi @metacpp
Thanks for re-opening this PR :)
I've taken a look through and left some comments in-line (most of which were from the original PR) so that we can look towards shipping this.
Thanks!
| @@ -122,6 +122,7 @@ func Provider() terraform.ResourceProvider { | |||
| }, | |||
|
|
|||
| ResourcesMap: map[string]*schema.Resource{ | |||
| "azurerm_action_group": resourceArmActionGroup(), | |||
There was a problem hiding this comment.
given this a resource within Azure Monitor this should be azurerm_monitor_action_group (we should probably update the naming for the azurerm_metric_alertrule and azurerm_autoscale_setting resources too, but we should fix this for new resources going forward)
|
|
||
| "short_name": { | ||
| Type: schema.TypeString, | ||
| Required: true, |
There was a problem hiding this comment.
is this only required for the SMS Receiver? if so - should this be optional?
|
|
||
| if v, ok := d.GetOk("webhook_receiver"); ok { | ||
| parameters.ActionGroup.WebhookReceivers = expandActionGroupWebHookReceiver(v.([]interface{})) | ||
| } |
There was a problem hiding this comment.
we can assign all of these directly:
emailReceiversRaw := d.Get("email_receiver").([]interface{})
emailReceivers = expandActionGroupEmailReceiver(emailReceiversRaw)
smsReceiversRaw := d.Get("sms_receiver").([]interface{})
smsReceivers = expandActionGroupSmsReceiver(smsReceiversRaw)
webhookReceiversRaw := d.Get("webhook_receiver").([]interface{})
webhookReceivers = expandActionGroupWebHookReceiver(webhookReceiversRaw)
parameters := insights.ActionGroupResource{
Location: utils.String(location),
ActionGroup: &insights.ActionGroup{
GroupShortName: utils.String(shortName),
Enabled: utils.Bool(enabled),
EmailReceivers: emailReceivers,
SmsReceivers: smsReceivers,
WebhookReceivers: webhookReceivers,
},
Tags: expandedTags,
}
|
|
||
| id, err := parseAzureResourceID(d.Id()) | ||
| if err != nil { | ||
| return fmt.Errorf("Error parsing action group resource ID \"%s\" during get: %+v", d.Id(), err) |
There was a problem hiding this comment.
since this is an unrecoverable state for users, this there's no point in wrapping it and we can return err directly here.
| } | ||
|
|
||
| d.Set("short_name", *resp.GroupShortName) | ||
| d.Set("enabled", *resp.Enabled) |
There was a problem hiding this comment.
we can remove the pointer deference here and we should be accessing these on the .Properties block to match the Azure API Response
| @@ -688,6 +688,10 @@ | |||
| <li<%= sidebar_current("docs-azurerm-resource-autoscale-setting") %>> | |||
There was a problem hiding this comment.
now there's multiple items in here, can we update this to: docs-azurerm-resource-monitor
| --- | ||
| layout: "azurerm" | ||
| page_title: "Azure Resource Manager: azurerm_action_group" | ||
| sidebar_current: "docs-azurerm-resource-action-group" |
There was a problem hiding this comment.
this should be docs-azurerm-resource-monitor-action-group
| * `enabled` - (Optional) Whether this action group is enabled. If an action group is not enabled, then none of its receivers will receive communications. Defaults to `true`. | ||
| * `email_receiver` - (Optional) The list of `email_receiver` blocks as defined below that are part of this action group. | ||
| * `sms_receiver` - (Optional) The list of `sms_receiver` blocks as defined below that are part of this action group. | ||
| * `webhook_receiver` - (Optional) The list of `webhook_receiver` blocks as defined below that are part of this action group. |
There was a problem hiding this comment.
we should update these to match the newer format:
* `email_receiver` - (Optional) One or more `email_receiver` blocks as defined below.
* `sms_receiver ` - (Optional) One or more `sms_receiver ` blocks as defined below.
* `webhook_receiver ` - (Optional) One or more `webhook_receiver ` blocks as defined below.
| The following arguments are supported: | ||
|
|
||
| * `name` - (Required) The name of the Action Group. Changing this forces a new resource to be created. | ||
| * `location` - (Required) The location of this Action Group. The only possible value is `Global`. |
There was a problem hiding this comment.
if this is the only available value, then it should be hard-coded (and not available for users to set)
|
|
||
| The following attributes are exported: | ||
|
|
||
| * `id` - The Route ID. |
There was a problem hiding this comment.
The Route ID -> The ID of the Monitor Action Group
|
Close this since @metacpp could not provide comments. I will create a new one. |
|
I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
In this PR, I introduced a new simple action group resource of Azure monitor.
I enabled three receivers (Email, SMS and Webhook) which should cover most of the use cases.
This PR is a first step of #1252 , because the resource (signal based alert) proposed in it relies on Action Group.
(fixes #1252)
This PR is a follow up on #1419, and @JunyiYi will take care of it.