Azure/alz-monitor

Missing "policyDefinitionReferenceId" in PolicySetDefinitions

wendlandreas opened this issue · 5 comments

The policy definitions in the initiatives (policy set definitions) are missing the field "policyDefinitionReferenceId".
The result is that in the Azure portal in some places randomly generated policyDefinitionReferenceIds are visible, for example on the "policies" tab of the initiative definition (but also in other places like when selection the policy in "create remediation task").

image

The fix should be easy, each policy in the policy set definition should contain a "policyDefinitionReferenceId" with a descriptive name.

{
  "policyDefinitionReferenceId": "ALZM_DeployActivitylogServiceHealthSecurityAdvisory",
  "policyDefinitionId": "[concat('/providers/Microsoft.Management/managementGroups/',managementGroup().name, '/providers/Microsoft.Authorization/policyDefinitions/Deploy_activitylog_ServiceHealth_SecurityAdvisory')]",
  "parameters": {
    "enabled": {
      "value": "[[parameters('svcHlthSecAdvisoryAlertState')]"
    },
    "alertResourceGroupName": {
      "value": "[[parameters('ALZMonitorResourceGroupName')]"
    },
    "alertResourceGroupTags": {
      "value": "[[parameters('ALZMonitorResourceGroupTags')]"
    },
    "alertResourceGroupLocation": {
      "value": "[[parameters('ALZMonitorResourceGroupLocation')]"
    }
  }            
}

Hi @wendlandreas thank you for taking the time to contribute to this. This is certainly something that we need to fix. I created a user story in our internal backlog. Of course we would welcome any contributions you might want to make, so if you have the bandwidth and inclination feel free to create a pull request towards this. :-)

Hi @jfaurskov,
I can create a PR, no problem.
Do you have any naming convention?

Awesome! Let's go with the same naming convention that is used in our other projects, i.e. the prefix to parameters, so the Service health security advisory should be svcHlthSecAdvisory (from svcHlthSecAdvisoryAlertState), service health Incident is SvcHlthIncident (from SvcHlthIncidentAlertState) and so on. Does that make sense?

Sure sounds good.
I will create a pull request as soon as I'm done.

resolved in #190