Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azurerm_monitor_action_group - fix tests and doc of itsm_receiver #18190

Merged
merged 5 commits into from
Sep 6, 2022
Merged

azurerm_monitor_action_group - fix tests and doc of itsm_receiver #18190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
216c195
fix action_group itsmReceiver test and doc
teowa Aug 31, 2022
adfae97
fix datasource
teowa Aug 31, 2022
ccbc313
fix datasource workspace_id
teowa Aug 31, 2022
4e975c3
note itsm_receiver.ticket_configuration
teowa Sep 2, 2022
464c33d
add check in create/update
teowa Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions internal/services/monitor/monitor_action_group_data_source_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,9 @@ func TestAccDataSourceMonitorActionGroup_complete(t *testing.T) {
check.That(data.ResourceName).Key("email_receiver.1.email_address").HasValue("[email protected]"),
check.That(data.ResourceName).Key("email_receiver.1.use_common_alert_schema").HasValue("false"),
check.That(data.ResourceName).Key("itsm_receiver.#").HasValue("1"),
check.That(data.ResourceName).Key("itsm_receiver.0.workspace_id").HasValue("6eee3a18-aac3-40e4-b98e-1f309f329816"),
check.That(data.ResourceName).Key("itsm_receiver.0.workspace_id").HasValue("5def922a-3ed4-49c1-b9fd-05ec533819a3|55dfd1f8-7e59-4f89-bf56-4c82f5ace23c"),
katbyte marked this conversation as resolved.
Show resolved Hide resolved
check.That(data.ResourceName).Key("itsm_receiver.0.connection_id").HasValue("53de6956-42b4-41ba-be3c-b154cdf17b13"),
check.That(data.ResourceName).Key("itsm_receiver.0.ticket_configuration").HasValue("{}"),
check.That(data.ResourceName).Key("itsm_receiver.0.ticket_configuration").HasValue("{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\",\"UseTemplate\":false,\"WorkItemData\":\"{}\",\"CreateOneWIPerCI\":false}"),
check.That(data.ResourceName).Key("itsm_receiver.0.region").HasValue("southcentralus"),
check.That(data.ResourceName).Key("azure_app_push_receiver.#").HasValue("1"),
check.That(data.ResourceName).Key("azure_app_push_receiver.0.email_address").HasValue("[email protected]"),
Expand Down Expand Up @@ -217,9 +217,9 @@ resource "azurerm_monitor_action_group" "test" {

itsm_receiver {
name = "createorupdateticket"
workspace_id = "6eee3a18-aac3-40e4-b98e-1f309f329816"
workspace_id = "5def922a-3ed4-49c1-b9fd-05ec533819a3|55dfd1f8-7e59-4f89-bf56-4c82f5ace23c"
connection_id = "53de6956-42b4-41ba-be3c-b154cdf17b13"
ticket_configuration = "{}"
ticket_configuration = "{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\",\"UseTemplate\":false,\"WorkItemData\":\"{}\",\"CreateOneWIPerCI\":false}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we include Azure/azure-rest-api-specs#20488 in these workarounds here?

Copy link
Contributor Author

@teowa teowa Sep 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From TC, this ticket_configuration = "{}" is still OK in July but recently Service has changed API behavior (may change again in the future). And from further test, I find the "{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\"} can work well, these two are the required keys.

I think it's better to note this in the doc instead of checking in the create/update (it's too strict to check certain keys in json). And I will also include the issue in code comment.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@teowa - the user may or may not read the docs. Unless there is going to be a fix by the service team in the near term can we place put in a check to work around this bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added checks of the JSON blob contains PayloadRevision and WorkItemType in create/update method.

region = "southcentralus"
}

Expand Down
30 changes: 26 additions & 4 deletions internal/services/monitor/monitor_action_group_resource.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package monitor

import (
"encoding/json"
"fmt"
"time"

Expand Down Expand Up @@ -447,6 +448,11 @@ func resourceMonitorActionGroupCreateUpdate(d *pluginsdk.ResourceData, meta inte
return err
}

expandedItsmReceiver, err := expandMonitorActionGroupItsmReceiver(itsmReceiversRaw)
if err != nil {
return err
}

t := d.Get("tags").(map[string]interface{})
expandedTags := tags.Expand(t)

Expand All @@ -457,7 +463,7 @@ func resourceMonitorActionGroupCreateUpdate(d *pluginsdk.ResourceData, meta inte
Enabled: utils.Bool(enabled),
EmailReceivers: expandMonitorActionGroupEmailReceiver(emailReceiversRaw),
AzureAppPushReceivers: expandMonitorActionGroupAzureAppPushReceiver(azureAppPushReceiversRaw),
ItsmReceivers: expandMonitorActionGroupItsmReceiver(itsmReceiversRaw),
ItsmReceivers: expandedItsmReceiver,
SmsReceivers: expandMonitorActionGroupSmsReceiver(smsReceiversRaw),
WebhookReceivers: expandMonitorActionGroupWebHookReceiver(tenantId, webhookReceiversRaw),
AutomationRunbookReceivers: expandMonitorActionGroupAutomationRunbookReceiver(automationRunbookReceiversRaw),
Expand Down Expand Up @@ -584,20 +590,36 @@ func expandMonitorActionGroupEmailReceiver(v []interface{}) *[]insights.EmailRec
return &receivers
}

func expandMonitorActionGroupItsmReceiver(v []interface{}) *[]insights.ItsmReceiver {
func expandMonitorActionGroupItsmReceiver(v []interface{}) (*[]insights.ItsmReceiver, error) {
receivers := make([]insights.ItsmReceiver, 0)
for _, receiverValue := range v {
val := receiverValue.(map[string]interface{})
ticketConfiguration := utils.String(val["ticket_configuration"].(string))
receiver := insights.ItsmReceiver{
Name: utils.String(val["name"].(string)),
WorkspaceID: utils.String(val["workspace_id"].(string)),
ConnectionID: utils.String(val["connection_id"].(string)),
TicketConfiguration: utils.String(val["ticket_configuration"].(string)),
TicketConfiguration: ticketConfiguration,
Region: utils.String(azure.NormalizeLocation(val["region"].(string))),
}

// https://github.com/Azure/azure-rest-api-specs/issues/20488 ticket_configuration should have `PayloadRevision` and `WorkItemType` keys
if ticketConfiguration != nil {
j := make(map[string]interface{})
err := json.Unmarshal([]byte(*ticketConfiguration), &j)
if err != nil {
return nil, fmt.Errorf("`itsm_receiver.ticket_configuration` %s unmarshall json error: %+v", *ticketConfiguration, err)
}

_, existKeyPayloadRevision := j["PayloadRevision"]
_, existKeyWorkItemType := j["WorkItemType"]
if !(existKeyPayloadRevision && existKeyWorkItemType) {
return nil, fmt.Errorf("`itsm_receiver.ticket_configuration` should be JSON blob with `PayloadRevision` and `WorkItemType` keys")
}
}
receivers = append(receivers, receiver)
}
return &receivers
return &receivers, nil
}

func expandMonitorActionGroupAzureAppPushReceiver(v []interface{}) *[]insights.AzureAppPushReceiver {
Expand Down
4 changes: 2 additions & 2 deletions internal/services/monitor/monitor_action_group_resource_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,7 +499,7 @@ resource "azurerm_monitor_action_group" "test" {
name = "createorupdateticket"
workspace_id = "${data.azurerm_client_config.current.subscription_id}|${azurerm_log_analytics_workspace.test.workspace_id}"
connection_id = "53de6956-42b4-41ba-be3c-b154cdf17b13"
ticket_configuration = "{}"
ticket_configuration = "{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\",\"UseTemplate\":false,\"WorkItemData\":\"{}\",\"CreateOneWIPerCI\":false}"
region = "eastus"
}
}
Expand Down Expand Up @@ -912,7 +912,7 @@ resource "azurerm_monitor_action_group" "test" {
name = "createorupdateticket"
workspace_id = "${data.azurerm_client_config.current.subscription_id}|${azurerm_log_analytics_workspace.test.workspace_id}"
connection_id = "53de6956-42b4-41ba-be3c-b154cdf17b13"
ticket_configuration = "{}"
ticket_configuration = "{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\",\"UseTemplate\":false,\"WorkItemData\":\"{}\",\"CreateOneWIPerCI\":false}"
region = "eastus"
}

Expand Down
4 changes: 3 additions & 1 deletion website/docs/r/monitor_action_group.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ resource "azurerm_monitor_action_group" "example" {
name = "createorupdateticket"
workspace_id = "${data.azurerm_client_config.current.subscription_id}|${azurerm_log_analytics_workspace.example.workspace_id}"
connection_id = "53de6956-42b4-41ba-be3c-b154cdf17b13"
ticket_configuration = "{}"
ticket_configuration = "{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\",\"UseTemplate\":false,\"WorkItemData\":\"{}\",\"CreateOneWIPerCI\":false}"
region = "southcentralus"
}

Expand Down Expand Up @@ -199,6 +199,8 @@ The following arguments are supported:
* `ticket_configuration` - (Required) A JSON blob for the configurations of the ITSM action. CreateMultipleWorkItems option will be part of this blob as well.
* `region` - (Required) The region of the workspace.

-> **NOTE** `ticket_configuration` should be JSON blob with `PayloadRevision` and `WorkItemType` keys (e.g., `ticket_configuration="{\"PayloadRevision\":0,\"WorkItemType\":\"Incident\"}"`), and `ticket_configuration="{}"` will return an error, see more at this [REST API issue](https://github.com/Azure/azure-rest-api-specs/issues/20488)

---

`logic_app_receiver` supports the following:
Expand Down