hashicorp / terraform-provider-azurerm

Terraform provider for Azure Resource Manager
https://registry.terraform.io/providers/hashicorp/azurerm/latest/docs
Mozilla Public License 2.0
4.54k stars 4.61k forks source link

Support for azurerm_sentinel_alert_rule_action #11366

Closed NVolcz closed 2 years ago

NVolcz commented 3 years ago

Community Note

Description

It looks to be possible to unblock #6820 without the having Microsoft changing their API. To my understanding, #6820 is blocked because the Pull Request's azurerm_sentinel_alert_rule_action requires a logic_app_trigger_name property. The logic_app_trigger_name is used to fetch the triggerUri which is required by the Azure REST API when creating or updating an action rule but not returned by the API when reading. The blocking issue from this perspective is that the Azure API does not return the triggerUri when reading action rules which is required since the azurerm_sentinel_alert_rule_action resource requires logic_app_trigger_name which abstracts triggerUri.

I have tried to ask Microsoft about this issue and to my understanding, triggerUri is only used when creating the action rule then internally discarded, hence not possible to return it. We can therefor discard the logic_app_trigger_name property entirely and fetch the triggerUri with the logic_app_id.

Here is an example for how to create an action with PowerShell (It helped me understand the API):

$SentinelConnection = @{
    ResourceGroupName = "yourResourceGroupName"
    WorkspaceName = "yourWorkspaceName"
}

$LogicAppConnection = @{
    ResourceGroupName = "yourLogicAppsResourceGroupName"
    Name = "yourLogicAppName"
}

$LogicAppResourceId = Get-AzLogicApp @LogicAppConnection
$LogicAppTriggerUri = Get-AzLogicAppTriggerCallbackUrl @LogicAppConnection -TriggerName "When_a_response_to_an_Azure_Sentinel_alert_is_triggered"
$AnalyticsRule = Get-AzSentinelAlertRule @SentinelConnection | Where-Object {$PSItem.DisplayName -eq "yourRuleName"}
$AlertRuleAction = New-AzSentinelAlertRuleAction @SentinelConnection `
-AlertRuleId $AnalyticsRule.Name `
-LogicAppResourceId ($LogicAppResourceId.Id) `
-TriggerUri ($LogicAppTriggerUri.Value)  

New or Affected Resource(s)

Potential Terraform Configuration

A new suggestion for unblocking the pull request:

resource "azurerm_resource_group" "example" {
  name     = "example-resources"
  location = "West Europe"
}

resource "azurerm_log_analytics_workspace" "example" {
  name                = "example-workspace"
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
  sku                 = "pergb2018"
}

resource "azurerm_sentinel_alert_rule_scheduled" "example" {
  name                       = "example-alert-rule"
  log_analytics_workspace_id = azurerm_log_analytics_workspace.example.id
  display_name               = "example"
  severity                   = "High"
  query                      = <<QUERY
AzureActivity |
  where OperationName == "Create or Update Virtual Machine" or OperationName =="Create Deployment" |
  where ActivityStatus == "Succeeded" |
  make-series dcount(ResourceId) default=0 on EventSubmissionTimestamp in range(ago(7d), now(), 1d) by Caller
QUERY
}

resource "azurerm_logic_app_workflow" "example" {
  name                = "example-workflow"
  location            = azurerm_resource_group.example.location
  resource_group_name = azurerm_resource_group.example.name
}

resource "azurerm_logic_app_trigger_custom" "example" {
  name         = "When_a_response_to_an_Azure_Sentinel_alert_is_triggered"
  logic_app_id = azurerm_logic_app_workflow.example.id
  body = <<BODY
{
    "type": "ApiConnectionWebhook",
    "inputs": {
        "body": {
            "callback_url": "@{listCallbackUrl()}"
        },
        "host": {
            "connection": {
                "name": "@parameters('$connections')['azuresentinel']['connectionId']"
            }
        },
        "path": "/subscribe"
    }
}
BODY
}

resource "azurerm_sentinel_alert_rule_action" "example" {
  name                   = "example-alert-rule-action"
  sentinel_alert_rule_id = azurerm_sentinel_alert_rule_scheduled.example.id
  logic_app_id           = azurerm_logic_app_trigger_custom.example.logic_app_id
}

References

magodo commented 3 years ago

@NVolcz I've followed your PR comment and this issue, while in the cmdlet you provided above, you'll set -TriggerUri ($LogicAppTriggerUri.Value) when creating the alert rule action, isn't this the same as we pass in the logic_app_trigger_name?

Meanwhile, I've submit another PR #11502 to support the automation rule, which shall supersedes the alert rule action.

NVolcz commented 3 years ago

Great to hear about the PR for the automation rules! It will be super useful! There is however a difference in functionality between automation rules and playbooks being triggered by the alert rules. Automation are triggered when an incident has been created and alert rules triggers playbooks when an alert has been created.

It looks to me that logic_app_trigger_name would be the equivalent of When_a_response_to_an_Azure_Sentinel_alert_is_triggered in the my powershell script above and ($LogicAppTriggerUri.Value) would be something like https://prod-17.westeurope.logic.azure.com:443/workflows/SOMEID?api-version=2016-06-01&SOMESECRETS

magodo commented 3 years ago

@NVolcz Thank you for pointing the diff! It makes sense to implement azurerm_sentinel_alert_rule_action along with the automation rule.

The name you used is the default name that is created on Portal (the workflow desiner). Whilst the name can be something else, if it is created via other means (e.g. via azurerm_logic_app_trigger_custom). Therefore, we still need the users to provide the name, tbn.

NVolcz commented 3 years ago

I am not sure if it is possible to change the name. I believe there only can be a single trigger of the same type, couldn't we simply enumerate the logic app's triggers and select the trigger of the correct type?

NVolcz commented 3 years ago

I double checked my suggestion in the previous comment and it seems to not be possible to do it the way I initially thought. There seems to be no obvious way to find the type of the logic app trigger. The Sentinel blade in the Azure portal seems to be filtering logic apps some logic apps. I can't create an action rule for logic app s that only have recurrence triggers. Also, if I create an action rule for a logic app with multiple trigger, the Azure portal selects the last one.

kaovd commented 3 years ago

Great to hear about the PR for the automation rules! It will be super useful! There is however a difference in functionality between automation rules and playbooks being triggered by the alert rules. Automation are triggered when an incident has been created and alert rules triggers playbooks when an alert has been created.

pretty sure this is superseded now by Automation Rules, incidents are created for everything regardless. Classic playbook alert triggers would trigger on the sentinel alert when it fired. That alert then creates an incident in sentinel. Effectively, whatever it is, it ends up in an incident and can be used in automation rules. When you are in the logic app using the preview incident trigger, If the alert is a Azure Sentinel KQL alert, you additionally get some properties that you would of had in the old standard alert trigger (as it is possible for an incident to contain multiple alerts depending on your config).

With the way its going logic app alert triggers are soon to be legacy from the looks of it with Ms's pretty much straight up refusal to fix their API. While this is nice to have I don't see urgency needing this anymore as automation rules do handle both. Although if else would be good to hear your thoughts

NVolcz commented 3 years ago

I am not sure that alert rules will be superseded by Automation rules. It is for example possible to create security alerts without incidents, it is possible to trigger a logic app workflow on this.

kaovd commented 3 years ago

@NVolcz Alerts should pretty much always result in Incidents and while having them both is great have a feeling down the line MS will probably switch.

Anyways none of this is getting pulled in the first place as we are still eternally blocked on the Logic App connections not working properly, Provider maintainers require tests do not use arm templates which was the only way to preform tests for Logic App triggers in the automation PR.
https://github.com/hashicorp/terraform-provider-azurerm/issues/1691

NVolcz commented 3 years ago

Will take a look at #1691 and see if there is anything I can help with there. @kaovd, Unless you have a credible source for future Microsoft plans, I would be careful making assumptions about the alert action. Many customers are actively using this today, so I have a hard time imagining this going away anytime soon.

kaovd commented 3 years ago

Will take a look at #1691 and see if there is anything I can help with there. @kaovd, Unless you have a credible source for future Microsoft plans, I would be careful making assumptions about the alert action. Many customers are actively using this today, so I have a hard time imagining this going away anytime soon.

Im not saying that we shouldnt have both just in my experience incident Automation rules make alert triggers redundant although thats not really the issue here

Ive considered the effort costs of forking some of this functionality as it seems silly these vital components of functionality for us are blocked due to having an arm template in a test although for now I don't suppose script provisioning the patch for that property is too bad

From what I hear at MS, while there is a lot of us asking about this stuff currently we are more small individual voices. Would certainly be nice if there was a interest group around as obviously if there was a centralized voice might make MS focus more on getting SecurityInsights packages out faster as dosent seem it gets much love as other RPs atm

magodo commented 2 years ago

@NVolcz I've got credit from the sentinel service team that they're already aware of the issue as stated in the https://github.com/Azure/azure-rest-api-specs/issues/9424. While they have no plan to fix that. On the other hand, they are planning to deprecate the alert rule action in favor of the automation rule, as the latter is a superset of the alert rule action.

As said above, currently there is nothing we can do to move this issue forward. Also, we already have the alternative solution for your use case (though need to include some arm template for other reason). Do you mind we close this issue for now?

magodo commented 2 years ago

@NVolcz I'm gonna close this issue as explained above. Please feel free reopen it if you have any any use case that can't be covered by the azurerm_automation_rule.

github-actions[bot] commented 2 years ago

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.