Azure / template-analyzer

Template scanner for security misconfiguration and best practices
MIT License
125 stars 38 forks source link

[BUG] - Analysis fails even when the ARM template (JSON) file is following the guidance #353

Open shailendragusain opened 8 months ago

shailendragusain commented 8 months ago

Describe the bug

Here's the sample ARM template

{
    "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
        "name": {
            "type": "string"
        },
        "resourceLocation": {
            "type": "string",
            "metadata": {
                "description": "Location where the resource to be created"
            },
            "defaultValue": "[resourceGroup().location]"
        },
        "resourceTags": {
            "type": "object",
            "defaultValue": {
                "Environemnt": "Dev",
                "Project": "Sample"
            }
        }
    },
    "resources": [
        {
            "type": "Microsoft.Network/virtualNetworks",
            "name": "[parameters('name')]",
            "apiVersion": "2020-06-01",
            "location": "[parameters('resourceLocation')]",
            "properties": {},
            "tags": "[parameters('resourceTags')]"
        }
    ]
}

When I run this template against the Template-Analyzer, I get the following error.

AZR-000222: Use a location parameter for regional resources
        Severity: High
        Recommendation: Consider updating the resource location property to use [parameters('location)].
        More information: https://azure.github.io/PSRule.Rules.Azure/en/rules/Azure.Template.ResourceLocation/
        Result: Failed 
        Line: 1
    Rules passed: 0

Expected behavior

It should not report AZR-000222: Use a location parameter for regional resources.

Reproduction Steps

Create an ARM template file with the following content,

{
    "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
    "contentVersion": "1.0.0.0",
    "parameters": {
        "name": {
            "type": "string"
        },
        "resourceLocation": {
            "type": "string",
            "metadata": {
                "description": "Location where the resource to be created"
            },
            "defaultValue": "[resourceGroup().location]"
        },
        "resourceTags": {
            "type": "object",
            "defaultValue": {
                "Environemnt": "Dev",
                "Project": "Sample"
            }
        }
    },
    "resources": [
        {
            "type": "Microsoft.Network/virtualNetworks",
            "name": "[parameters('name')]",
            "apiVersion": "2020-06-01",
            "location": "[parameters('resourceLocation')]",
            "properties": {},
            "tags": "[parameters('resourceTags')]"
        }
    ]
}

Run the template Analyzer command pointing to this file.

dotnet TemplateAnalyzer.dll analyze-template <path-to-template>.json --report-format Console --include-non-security-rules -v

Environment

I have tried this with Ubuntu and Mac OSX (M1 - Apple Silicon). NOTE: I am using dotnet-sdk-7.0 to run the TemplateAnalyzer on Unix machines.

JohnathonMohr commented 8 months ago

@BernieWhite I haven't investigated this on the PSRule side, but this appears to be AZR-000222 not considering whether the reference to resourceGroup().location is initializing a parameter value. The template with the violation appears to be following the guidance of this rule, yet it still fails.

BernieWhite commented 8 months ago

Thanks for reporting @shailendragusain @JohnathonMohr.

BernieWhite commented 8 months ago

@JohnathonMohr The issue is related to https://github.com/Azure/template-analyzer/blob/811b59edc234e7c6c8b6371632d026bb965c47ed/src/Analyzer.PowerShellRuleEngine/PowerShellRuleEngine.cs#L88

PSRule is passed an expanded template with all expressions replaced. PSRule is going to assume that the location was hardcoded which is the point of rule AZR-000222.

Since expanded resource are already passed via https://github.com/Azure/template-analyzer/blob/811b59edc234e7c6c8b6371632d026bb965c47ed/src/Analyzer.PowerShellRuleEngine/PowerShellRuleEngine.cs#L125

I don't think we need to pass an expanded template and we can use File.WriteAllText(tempTemplateFile, templateContext.OriginalTemplate.ToString());.

I'll raise a PR.

JohnathonMohr commented 8 months ago

I see, interesting. This rule requires not evaluating expressions. However, I'm sure other rules would benefit from having them evaluated.

The ideal case would be to run each rule against the desired template state. We might consider taking inventory of the rules and running PSRule twice (in parallel hopefully), once for each state, using the corresponding set of rules. Long term we could add metadata to each rule or something.