Provide deployment of alert policy definitions from Azure Resources section of AMBA website #467
Conversation
There was a problem hiding this comment.
Hi @judyer28,
I have identified a few areas for your review.
I am questioning the necessity of maintaining the policy-arm folder along with its templates. There seems to be considerable code duplication merely for the deployment of individual policy definitions, which is quite a limited use case. The following points elaborate on this:
- The deployment does not include the Policy Assignment.
- There is a limit of 200 Policy or Initiative assignments at any given scope. Deploying and assigning policies individually will quickly meet this limit, which we should avoid recommending.
I strongly advocate for policies to be incorporated into an initiative and assigned per initiative. This responsibility should lie with the pattern owners.
| } | ||
| }, | ||
| "variables": { | ||
| "pidDeploymentName": "[take(concat('pid-8bb7cf8a-bcf7-4264-abcb-703ace2fc84d-', uniqueString(managementGroup().id, parameters('currentDateTimeUtcNow'))), 64)]", |
There was a problem hiding this comment.
Can you review, there are template validation errors in all templates:
Template validation failed: The template variable 'pidDeploymentName' is not valid: The template function 'managementGroup' is not valid. Please see https://aka.ms/arm-template-expressions for usage details.. Please see https://aka.ms/arm-template-expressions for usage details.
There was a problem hiding this comment.
I just tried to deploy this specific file and it deployed ok for me. Can you tell me how you are trying to deploy it? I just did a Custom Deployment in the portal, loaded the file and deployed without error.
There was a problem hiding this comment.
@arjenhuitema I looked at this again and I think it may be an issue with the template validator in VS Code. As far as I can tell 'managementGoup' is a valid template function. See here https://learn.microsoft.com/en-us/azure/azure-resource-manager/templates/template-functions-scope#managementgroup
There was a problem hiding this comment.
Loaded template file shows no error on line 39 when loaded into portal:
There was a problem hiding this comment.
Appears to be known issue: microsoft/vscode-azurearmtools#1428
| "displayName": "[parameters('displayName')]", | ||
| "description": "[parameters('description')]", | ||
| "metadata": { | ||
| "version": "1.0.0", |
There was a problem hiding this comment.
These should be tagged as "version": "1.0.0-preview",
Also the metadata should include "preview": true,
There was a problem hiding this comment.
Why should it have these? I do not see it in the docs. https://learn.microsoft.com/en-us/azure/templates/microsoft.authorization/2021-06-01/policydefinitions?pivots=deployment-language-arm-template. What is preview used for?
There was a problem hiding this comment.
"Preview" indicates the Policy is still in preview, applicable to built-in policies as well. Generally, the versions end with "-preview." For example: https://www.azadvertizer.net/azpolicyadvertizer/4485d24b-a9d3-4206-b691-1fad83bc5007.html. Although not strictly required, these policies were auto-generated and remain in their initial release phase. They have yet to be thoroughly vetted.
There was a problem hiding this comment.
@arjenhuitema I added the version and preview information back into the template
...Services/servers/templates/policy-arm/memorymetric_0289bc11-db65-4a58-91ed-fda8637322ec.json
Show resolved
Hide resolved
...Services/servers/templates/policy-arm/memorymetric_0289bc11-db65-4a58-91ed-fda8637322ec.json
Show resolved
Hide resolved
|
@arjenhuitema thanks for taking the time to review and provide feedback. Regarding:
This feature is not meant for those users looking to apply a specific pattern such as ALZ. In those cases, users should go to the pattern section and follow the guidance there. This feature is meant for users not following a specific pattern and want to pick and choose specific alerts to deploy from the Azure Resources section of the AMBA website. This is a first step for those users to deploy the policy definitions. They can then include them in their own custom initiatives and assign them as they see fit. Future iterations of this could include automation for the custom initiatives and assignments. |
|
@arjenhuitema I believe I have addressed all comments. Please let me know if there is anything else needed. |
Overview/Summary
Added a Deploy to Azure button for policy definitions for individual Azure resource type alerts. Also, removed '[[' from the Policy section of each alert so that users can easily copy and paste policy definition snippet to Azure portal.
This PR fixes/adds/changes/removes
Breaking Changes
As part of this Pull Request I have
mainbranch