Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.25k stars 754 forks source link

Iteration with a child resource Error: required properties: "scope" #6010

Closed alex-frankel closed 1 year ago

alex-frankel commented 2 years ago

Discussed in https://github.com/Azure/bicep/discussions/6007

Originally posted by **jackpye1997** February 17, 2022 I'm trying to deploy Sentinel analytic rules through a bicep module however, when I try to loop though an array to deploy the automation action child resources associated with the analytic rule it give me this error. ![image](https://user-images.githubusercontent.com/55981113/154493765-58b3537d-b544-4dd2-8a88-a77fd96edb8d.png) According to the documentation and as far as I'm aware this resource shouldn't need a scope just a parent parameter. Am I doing something wrong here or is this a bug? If the child resource is not encapsulated in a loop it doesn't error Module code below ```bicep param playbooks array = [] \\other params resource detection_rule 'Microsoft.SecurityInsights/alertRules@2021-03-01-preview' = { scope: workspace name: name kind: 'Scheduled' properties:{ displayName: empty(displayName) ? name : displayName description: description queryFrequency: queryFrequency_ISO_8601 queryPeriod: queryPeriod_ISO_8601 query: query enabled: enabled severity: severity entityMappings: ((!empty(entities)) ? entities : null) triggerOperator: triggerOperator triggerThreshold: triggerThreshold suppressionDuration: 'PT5H' suppressionEnabled: false tactics: tactics alertRuleTemplateName: empty(alertRuleTemplateName) ? null : alertRuleTemplateName incidentConfiguration: { createIncident: true groupingConfiguration: { matchingMethod: 'AllEntities' reopenClosedIncident: false lookbackDuration: 'PT5M' enabled: false } } } } resource detection_rule_playbooks 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = [for (playbook, i) in playbooks: { parent: detection_rule name: '${name}-${i}' properties: { triggerUri: listCallbackURL('${playbook}/triggers/When_a_response_to_an_Azure_Sentinel_alert_is_triggered', '2016-06-01').value logicAppResourceId: playbook } }] ```
slavizh commented 2 years ago

welcome to the wonderful world of Sentinel APIs. Resource within resource within resource as extension on other resource :)

miqm commented 2 years ago

@slavizh to clarify as it seems you know this resource - there should be a scope parameter because it's an extension resource? But the bicep does not allow it to set in conjunction with parent resource and a for loop?

slavizh commented 2 years ago

@miqm know it a little bit. I had little interactions with the Sentinel APIs and they are not good to my experience. I think the problem is within the initial design where instead of having their own top level resource and just referencing Log Analytics workspace by resource ID, they have opted to be an API with Log Analytics workspace. That is of course my opinion as outsider without knowing the full details. But yeah in short Microsoft.SecurityInsights/alertRules is extension resource to the Log Analytics workspace and Microsoft.SecurityInsights/alertRules/actions is extension resource to the Log Analytics workspace but also second level resource under Microsoft.SecurityInsights/alertRules. So either Bicep needs to put some logic that extension resources could be multi-level (not just one) or it needs to support both scope and parent at the same time which also requires changes in Bicep. For me the first option would seem better as the example code seems the right way to define this from code perspective.

majastrz commented 2 years ago

It seems like an issue with nested resources under extension resources (unclear if loop involvement affects the bug)

brwilkinson commented 2 years ago

@majastrz I had planned to review this, however did not do so during the past week.

If you are clear this is a bug, then I will unassign it from myself? otherwise I can still follow up with some further investigation, just let me know?

brwilkinson commented 2 years ago

Hi @jackpye1997 your discussion was converted to an issue, however I don't believe you were tagged.

Are you able to provide a full repro sample for this scenario?

you have other params however if you can provide a complete template including default parameter values so that we can run it directly without any param file, that would be most helpful.

param playbooks array = []
\\other params
jackpye1997 commented 2 years ago

Hi guys, thanks for tagging me

@majastrz - This issue still persists when child resource is not nested.

I've actually found a solution for this using the ARM templates generated when using the 'Export' feature for rules in sentinel although its not the prettiest. The following code works

@maxLength(256)
param name string
param description string
param workspace_name string
param query string
param queryFrequency string
param queryPeriod string
param displayName string = ''
param severity string
param enabled bool = true
param triggerThreshold int = 0
param triggerOperator string = 'GreaterThan'
param playbooks array = []
param alertRuleTemplateName string = ''
@allowed([
  'Collection'
  'CommandAndControl' 
  'CredentialAccess'
  'DefenseEvasion'
  'Discovery' 
  'Execution' 
  'Exfiltration'
  'Impact'
  'InitialAccess'
  'LateralMovement'
  'Persistence'
  'PreAttack'
  'PrivilegeEscalation'
])
param tactics array = []
// EXAMPLE ENTITIES PARAM
// [
//   {
//     entityType: 'Account'
//     fieldMappings: [
//       {
//         identifier: 'FullName'
//         columnName: 'AccountCustomEntity'
//       }
//     ]
//   }
//   {
//     entityType: 'IP'
//     fieldMappings: [
//       {
//         identifier: 'Address'
//         columnName: 'IPCustomEntity'
//       }
//     ]
//   }
// ]
param entities array = []

var queryFrequency_ISO_8601 =  contains(ISO_8601_time_components, toUpper(last(queryFrequency))) ? toUpper('PT${queryFrequency}') : toUpper('P${queryFrequency}')
var queryPeriod_ISO_8601 =  contains(ISO_8601_time_components, toUpper(last(queryPeriod))) ? toUpper('PT${queryPeriod}') : toUpper('P${queryPeriod}')

var ISO_8601_time_components  = [
  'H'
  'M'
  'S'
]
resource workspace 'Microsoft.OperationalInsights/workspaces@2021-06-01' existing = {
  name : workspace_name
}

resource detection_rule 'Microsoft.SecurityInsights/alertRules@2021-03-01-preview' = {
  scope: workspace
  name: name
  kind: 'Scheduled'
  properties:{
    displayName: empty(displayName) ? name : displayName
    description: description
    queryFrequency: queryFrequency_ISO_8601
    queryPeriod: queryPeriod_ISO_8601
    query: query
    enabled: enabled
    severity: severity
    entityMappings: ((!empty(entities)) ? entities : null)
    triggerOperator: triggerOperator
    triggerThreshold: triggerThreshold
    suppressionDuration: 'PT5H'
    suppressionEnabled: false
    tactics: tactics
    alertRuleTemplateName: empty(alertRuleTemplateName) ? null : alertRuleTemplateName
    incidentConfiguration: {
      createIncident: true
      groupingConfiguration: {
        matchingMethod: 'AllEntities'
        reopenClosedIncident: false
        lookbackDuration: 'PT5M'
        enabled: false
      }
    }
  }
}

resource detection_rule_playbooks 'Microsoft.OperationalInsights/workspaces/providers/alertRules/actions@2021-09-01-preview' = [for (playbook, index) in playbooks: if(playbooks != []) {
  name: '${workspace_name}/Microsoft.SecurityInsights/${name}/${name}${last(split(playbook, '/'))}'
  properties: {
    triggerUri: listCallbackURL('${playbook}/triggers/When_a_response_to_an_Azure_Sentinel_alert_is_triggered', '2016-06-01').value
    logicAppResourceId: playbook
  }
  dependsOn: [
    detection_rule
  ]
}]
brwilkinson commented 2 years ago

The alertRule action is scoped to both the OperationalInsights workspace and is also a child from the alertRule

There does appear to be an issue in natively defining both of these in Bicep.

A valid ActionId is as follows:

/subscriptions/b8f402aa-20f7-4888-b45c-3cf086dad9c3/resourceGroups/ACU1-BRW-AOA-RG-T5/providers/Microsoft.OperationalInsights/workspaces/acu1brwaoat5LogAnalytics/providers/Microsoft.SecurityInsights/alertRules/new2/actions/action1

The simplest way to define this is as follows.

resource workspace 'Microsoft.OperationalInsights/workspaces@2021-06-01' existing = {
  name: 'acu1brwaoat5LogAnalytics'
}

resource alertrule 'Microsoft.SecurityInsights/alertRules@2021-09-01-preview' = {
  name: 'new2'
  scope: workspace
  kind: 'Scheduled'
  properties: {
   ...
  }
}

resource logicApp 'Microsoft.Logic/workflows@2019-05-01' existing = {
  name: 'logic01'
}

resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  name: 'new2/action1'    // <---- This works correctly for parent
  scope: workspace    // <---- This works correctly for scope
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}

The above syntax works correctly βœ…


Below syntax options do not work correctly ❌

option 1

// scope and parent ?
resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  name: 'action1'
  parent: alertrule        //  <--- parent
  scope: workspace    //  <--- scope
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}

Has the following error:

[{
    "owner": "_generated_diagnostic_collection_name_#2",
    "code": "BCP164",
    "severity": 8,
    "message": "A child resource's scope is computed based on the scope of its ancestor resource. This means that using the \"scope\" property on a child resource is unsupported.",
    "source": "bicep"
}]

option 2

// using the scope of the parent ?
resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  name: 'action1'
  scope: alertrule
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}

Has the following error:

[{
    "owner": "_generated_diagnostic_collection_name_#2",
    "code": "BCP169",
    "severity": 8,
    "message": "Expected resource name to contain 1 \"/\" character(s). The number of name segments must match the number of segments in the resource type.",
    "source": "bicep"
}]
anthony-c-martin commented 2 years ago

@brwilkinson this internal doc might be helpful in understanding my thinking a bit (I wrote the doc and designed + implemented scope and parent, so any confusion is my responsibility)

Using the example from that doc (which unfortunately isn't going to be visible for non-MS employees), for an extension resourceId:

{rootScope}/providers/{parentNamespace}/{parentType}/{parentName}/providers/{extensionNamespace}/{extensionType}/{extensionName}

So in the Bicep file we essentially have the following resources being created:

tl;dr: it doesn't make sense to allow both setting a parent and a scope for a child resource, because the scope will always have to match that of the parent resource.

brwilkinson commented 2 years ago

Hi @anthony-c-martin

So are you saying that this is the intented/preferred configuration/setting here?

resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  name: 'new2/action1'    // <---- This works correctly for parent
  scope: workspace    // <---- This works correctly for scope
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}

Given the scope is the workspace, however as an extension resource it still requires 2 arguments:

We don't have a native way to provide both arguments in this case. However, in other cases in Bicep i.e. for non extension resources, I believe we have attempted to discourage people from using alertRuleParentParam1/actionParam2 the slashes in the name.

anthony-c-martin commented 2 years ago

My preferred solution would be:

resource alertRule 'Microsoft.SecurityInsights/alertRules@2021-09-01-preview' existing = {
  scope: workspace
  name: 'new2'
}

resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  parent: alertRule
  name: 'action1'
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}

Using slashes does work, but you're right - I'd like to discourage that where possible.

anthony-c-martin commented 2 years ago

By the way, in case it's not clear - I'm agreeing that this does seem like a legitimate bug to me.

brwilkinson commented 2 years ago

@anthony-c-martin sure sounds good βœ…

I see what you mean:

the scope will always have to match that of the parent resource.

resource logicApp 'Microsoft.Logic/workflows@2019-05-01' existing = {
  name: 'logic01'
}

resource workspace 'Microsoft.OperationalInsights/workspaces@2021-06-01' existing = {
  name: 'acu1brwaoat5LogAnalytics'
}

resource alertrule 'Microsoft.SecurityInsights/alertRules@2021-09-01-preview' = {
  name: 'new2'
  scope: workspace
  kind: 'Scheduled'
  #disable-next-line BCP035
  properties: {
    //  ...
  }
}

resource action 'Microsoft.SecurityInsights/alertRules/actions@2021-09-01-preview' = {
  name: 'action1'
  parent: alertRule  //<--- The name "alertRule" does not exist in the current context.bicep(BCP057)
  properties: {
    logicAppResourceId: logicApp.id
    triggerUri: logicApp.listCallbackUrl().value
  }
}
alex-frankel commented 1 year ago

Given this has been open for some time with not a lot of other reports, I'm moving this to 1.0. It's a very odd resource type combination, so while we should fix it, I think 1.0 is appropriate.

Kaloszer commented 1 year ago

Same issue seems to happen if you try to add an extension resource of watchlistItems ontop of a watchlist. Repro:

param watchlistItems array
param watchlistName string
param workspaceName string

resource workspace 'Microsoft.OperationalInsights/workspaces@2022-10-01' existing = {
  name: workspaceName
}

var firstColumnName = !empty(watchlistItems) ? watchlistItems[0][0] : ''

resource watchlist 'Microsoft.SecurityInsights/watchlists@2023-02-01-preview' = {
  scope: workspace
  name: watchlistName
  properties: {
    provider: 'Microsoft'
    displayName: watchlistName
    itemsSearchKey: firstColumnName
  }

  resource watchlistItemsDeployment 'watchlistItems@2023-02-01-preview' = [for item in watchlistItems: {
    scope: watchlist
    name: guid(item)
    properties: {
      itemsKeyValue: item
    }
  }]

}

image image